From 728c18b2a367bad5258452f425e7592b4d8171ac Mon Sep 17 00:00:00 2001 From: Thibault Duplessis Date: Mon, 28 Sep 2020 23:35:17 +0200 Subject: [PATCH] -Wvalue-discard and explicit discard with .unit WIP 250 warnings to go --- app/Env.scala | 2 +- app/controllers/Account.scala | 2 +- app/controllers/Auth.scala | 4 +-- app/controllers/Fishnet.scala | 2 +- app/controllers/LilaController.scala | 2 +- modules/activity/src/main/Env.scala | 26 +++++++-------- modules/analyse/src/main/Analyser.scala | 7 ++-- modules/api/src/main/EventStream.scala | 24 +++++++------- modules/bookmark/src/main/Env.scala | 4 +-- modules/bot/src/main/GameStateStream.scala | 26 ++++++++------- modules/challenge/src/main/Env.scala | 2 +- modules/chat/src/main/ChatApi.scala | 8 ++--- modules/clas/src/main/Env.scala | 2 +- modules/coach/src/main/Env.scala | 10 +++--- modules/common/src/main/Bus.scala | 32 +++++++++++-------- modules/common/src/main/Lilaisms.scala | 1 + .../src/main/base/PimpedPrimitives.scala | 7 ++++ modules/irwin/src/main/IrwinApi.scala | 2 +- project/BuildSettings.scala | 4 +-- 19 files changed, 91 insertions(+), 76 deletions(-) diff --git a/app/Env.scala b/app/Env.scala index 8cdda8df44..d8f8bbd114 100644 --- a/app/Env.scala +++ b/app/Env.scala @@ -166,7 +166,7 @@ final class Env( // GC can be aborted by reverting the initial SB mark user.repo.isTroll(userId) foreach { troll => if (troll) scheduler.scheduleOnce(1.second) { - closeAccount(userId, self = false) + closeAccount(userId, self = false).unit } } } diff --git a/app/controllers/Account.scala b/app/controllers/Account.scala index ed159f35c1..f48889afee 100644 --- a/app/controllers/Account.scala +++ b/app/controllers/Account.scala @@ -417,7 +417,7 @@ final class Account( case Some(user) => env.report.api.reopenReports(lila.report.Suspect(user)) >> auth.authenticateUser(user) >>- - lila.mon.user.auth.reopenConfirm("success").increment() + lila.mon.user.auth.reopenConfirm("success").increment().discard.unit } } diff --git a/app/controllers/Auth.scala b/app/controllers/Auth.scala index 0c50153002..e35ed1d092 100644 --- a/app/controllers/Auth.scala +++ b/app/controllers/Auth.scala @@ -387,7 +387,7 @@ final class Auth( env.security.store.closeAllSessionsOf(user.id) >> env.push.webSubscriptionApi.unsubscribeByUser(user) >> authenticateUser(user) >>- - lila.mon.user.auth.passwordResetConfirm("success").increment() + lila.mon.user.auth.passwordResetConfirm("success").increment().unit }(rateLimitedFu) } } @@ -444,7 +444,7 @@ final class Auth( case Some(user) => authLog(user.username, "-", "Magic link") authenticateUser(user) >>- - lila.mon.user.auth.magicLinkConfirm("success").increment() + lila.mon.user.auth.magicLinkConfirm("success").increment().unit } } diff --git a/app/controllers/Fishnet.scala b/app/controllers/Fishnet.scala index 92860f6742..d06b05e03b 100644 --- a/app/controllers/Fishnet.scala +++ b/app/controllers/Fishnet.scala @@ -18,7 +18,7 @@ final class Fishnet(env: Env) extends LilaController(env) { def acquire(slow: Boolean = false) = ClientAction[JsonApi.Request.Acquire] { _ => client => api.acquire(client, slow) addEffect { jobOpt => - lila.mon.fishnet.http.request(jobOpt.isDefined).increment() + lila.mon.fishnet.http.request(jobOpt.isDefined).increment().unit } map Right.apply } diff --git a/app/controllers/LilaController.scala b/app/controllers/LilaController.scala index 8ca389194e..3b5536629f 100644 --- a/app/controllers/LilaController.scala +++ b/app/controllers/LilaController.scala @@ -649,7 +649,7 @@ abstract private[controllers] class LilaController(val env: Env) fuccess(BadRequest(errorsAsJson(err))) protected def pageHit(req: RequestHeader): Unit = - if (HTTPRequest isHuman req) lila.mon.http.path(req.path).increment() + if (HTTPRequest isHuman req) lila.mon.http.path(req.path).increment().unit protected def pageHit(implicit ctx: lila.api.Context): Unit = pageHit(ctx.req) diff --git a/modules/activity/src/main/Env.scala b/modules/activity/src/main/Env.scala index ae212e4c12..ca9a1c7a0b 100644 --- a/modules/activity/src/main/Env.scala +++ b/modules/activity/src/main/Env.scala @@ -43,20 +43,20 @@ final class Env( "startStudy", "streamStart" ) { - case lila.game.actorApi.FinishGame(game, _, _) if !game.aborted => write game game - case lila.forum.actorApi.CreatePost(post) => write.forumPost(post) - case res: lila.puzzle.Puzzle.UserResult => write puzzle res - case prog: lila.practice.PracticeProgress.OnComplete => write practice prog - case lila.simul.Simul.OnStart(simul) => write simul simul - case CorresMoveEvent(move, Some(userId), _, _, false) => write.corresMove(move.gameId, userId) - case lila.hub.actorApi.plan.MonthInc(userId, months) => write.plan(userId, months) - case lila.hub.actorApi.relation.Follow(from, to) => write.follow(from, to) + case lila.game.actorApi.FinishGame(game, _, _) if !game.aborted => write.game(game).unit + case lila.forum.actorApi.CreatePost(post) => write.forumPost(post).unit + case res: lila.puzzle.Puzzle.UserResult => write.puzzle(res).unit + case prog: lila.practice.PracticeProgress.OnComplete => write.practice(prog).unit + case lila.simul.Simul.OnStart(simul) => write.simul(simul).unit + case CorresMoveEvent(move, Some(userId), _, _, false) => write.corresMove(move.gameId, userId).unit + case lila.hub.actorApi.plan.MonthInc(userId, months) => write.plan(userId, months).unit + case lila.hub.actorApi.relation.Follow(from, to) => write.follow(from, to).unit case lila.study.actorApi.StartStudy(id) => // wait some time in case the study turns private - system.scheduler.scheduleOnce(5 minutes) { write study id } - case lila.hub.actorApi.team.CreateTeam(id, _, userId) => write.team(id, userId) - case lila.hub.actorApi.team.JoinTeam(id, userId) => write.team(id, userId) - case lila.hub.actorApi.streamer.StreamStart(userId) => write.streamStart(userId) - case lila.user.User.GDPRErase(user) => write erase user + system.scheduler.scheduleOnce(5 minutes) { write.study(id).unit }.unit + case lila.hub.actorApi.team.CreateTeam(id, _, userId) => write.team(id, userId).unit + case lila.hub.actorApi.team.JoinTeam(id, userId) => write.team(id, userId).unit + case lila.hub.actorApi.streamer.StreamStart(userId) => write.streamStart(userId).unit + case lila.user.User.GDPRErase(user) => write.erase(user).unit } } diff --git a/modules/analyse/src/main/Analyser.scala b/modules/analyse/src/main/Analyser.scala index 840fee80fc..8d40a47afc 100644 --- a/modules/analyse/src/main/Analyser.scala +++ b/modules/analyse/src/main/Analyser.scala @@ -27,15 +27,14 @@ final class Analyser( sendAnalysisProgress(analysis, complete = true) >>- { Bus.publish(actorApi.AnalysisReady(game, analysis), "analysisReady") Bus.publish(InsertGame(game), "gameSearchInsert") - requesterApi save analysis + requesterApi.save(analysis).unit } } } case Some(_) => analysisRepo.save(analysis) >> - sendAnalysisProgress(analysis, complete = true) >>- { - requesterApi save analysis - } + sendAnalysisProgress(analysis, complete = true) >>- + requesterApi.save(analysis).unit } def progress(analysis: Analysis): Funit = sendAnalysisProgress(analysis, complete = false) diff --git a/modules/api/src/main/EventStream.scala b/modules/api/src/main/EventStream.scala index c20d484d3e..aa727f29da 100644 --- a/modules/api/src/main/EventStream.scala +++ b/modules/api/src/main/EventStream.scala @@ -86,26 +86,28 @@ final class EventStream( lastSetSeenAt = DateTime.now } - context.system.scheduler.scheduleOnce(6 second) { - if (online) { - // gotta send a message to check if the client has disconnected - queue offer None - self ! SetOnline + context.system.scheduler + .scheduleOnce(6 second) { + if (online) { + // gotta send a message to check if the client has disconnected + queue offer None + self ! SetOnline + } } - } + .unit - case StartGame(game) => queue offer gameJson("gameStart")(game).some + case StartGame(game) => queue.offer(gameJson("gameStart")(game).some).unit - case FinishGame(game, _, _) => queue offer gameJson("gameFinish")(game).some + case FinishGame(game, _, _) => queue.offer(gameJson("gameFinish")(game).some).unit case lila.challenge.Event.Create(c) if isMyChallenge(c) => - queue offer challengeJson("challenge")(c).some + queue.offer(challengeJson("challenge")(c).some).unit case lila.challenge.Event.Decline(c) if isMyChallenge(c) => - queue offer challengeJson("challengeDeclined")(c).some + queue.offer(challengeJson("challengeDeclined")(c).some).unit case lila.challenge.Event.Cancel(c) if isMyChallenge(c) => - queue offer challengeJson("challengeCanceled")(c).some + queue.offer(challengeJson("challengeCanceled")(c).some).unit // pretend like the rematch is a challenge case lila.hub.actorApi.round.RematchOffer(gameId) => diff --git a/modules/bookmark/src/main/Env.scala b/modules/bookmark/src/main/Env.scala index 8be7bb386f..ecec1608ee 100644 --- a/modules/bookmark/src/main/Env.scala +++ b/modules/bookmark/src/main/Env.scala @@ -36,8 +36,8 @@ final class Env( system.actorOf( Props(new Actor { def receive = { - case Toggle(gameId, userId) => api.toggle(gameId, userId) - case Remove(gameId) => api removeByGameId gameId + case Toggle(gameId, userId) => api.toggle(gameId, userId).unit + case Remove(gameId) => api.removeByGameId(gameId).unit } }), name = config.actorName diff --git a/modules/bot/src/main/GameStateStream.scala b/modules/bot/src/main/GameStateStream.scala index 5fd23c8745..5b56462d51 100644 --- a/modules/bot/src/main/GameStateStream.scala +++ b/modules/bot/src/main/GameStateStream.scala @@ -94,24 +94,26 @@ final class GameStateStream( Bus.publish(Tell(init.game.id, BotConnected(as, v = false)), "roundSocket") } queue.complete() - lila.mon.bot.gameStream("stop").increment() + lila.mon.bot.gameStream("stop").increment().unit } def receive = { - case MoveGameEvent(g, _, _) if g.id == id && !g.finished => pushState(g) + case MoveGameEvent(g, _, _) if g.id == id && !g.finished => pushState(g).unit case lila.chat.actorApi.ChatLine(chatId, UserLine(username, _, text, false, false)) => - pushChatLine(username, text, chatId.value.lengthIs == Game.gameIdSize) - case FinishGame(g, _, _) if g.id == id => onGameOver(g.some) - case AbortedBy(pov) if pov.gameId == id => onGameOver(pov.game.some) - case lila.game.actorApi.BoardDrawOffer(pov) if pov.gameId == id => pushState(pov.game) + pushChatLine(username, text, chatId.value.lengthIs == Game.gameIdSize).unit + case FinishGame(g, _, _) if g.id == id => onGameOver(g.some).unit + case AbortedBy(pov) if pov.gameId == id => onGameOver(pov.game.some).unit + case lila.game.actorApi.BoardDrawOffer(pov) if pov.gameId == id => pushState(pov.game).unit case SetOnline => onlineApiUsers.setOnline(user.id) - context.system.scheduler.scheduleOnce(6 second) { - // gotta send a message to check if the client has disconnected - queue offer None - self ! SetOnline - Bus.publish(Tell(id, QuietFlag), "roundSocket") - } + context.system.scheduler + .scheduleOnce(6 second) { + // gotta send a message to check if the client has disconnected + queue offer None + self ! SetOnline + Bus.publish(Tell(id, QuietFlag), "roundSocket") + } + .unit case NewConnectionDetected => newConnectionDetected = true self ! PoisonPill diff --git a/modules/challenge/src/main/Env.scala b/modules/challenge/src/main/Env.scala index d5b6e156a5..da5c50fd82 100644 --- a/modules/challenge/src/main/Env.scala +++ b/modules/challenge/src/main/Env.scala @@ -51,6 +51,6 @@ final class Env( lazy val jsonView = wire[JsonView] system.scheduler.scheduleWithFixedDelay(10 seconds, 3 seconds) { () => - api.sweep + api.sweep.unit } } diff --git a/modules/chat/src/main/ChatApi.scala b/modules/chat/src/main/ChatApi.scala index 06436d3810..17fddff0d6 100644 --- a/modules/chat/src/main/ChatApi.scala +++ b/modules/chat/src/main/ChatApi.scala @@ -104,7 +104,7 @@ final class ChatApi( } } publish(chatId, actorApi.ChatLine(chatId, line), busChan) - lila.mon.chat.message(publicSource.fold("player")(_.parentName), line.troll).increment() + lila.mon.chat.message(publicSource.fold("player")(_.parentName), line.troll).increment().unit } } } @@ -132,7 +132,7 @@ final class ChatApi( } def service(chatId: Chat.Id, text: String, busChan: BusChan.Select, isVolatile: Boolean): Unit = - (if (isVolatile) volatile _ else system _)(chatId, text, busChan) + (if (isVolatile) volatile _ else system _)(chatId, text, busChan).unit def timeout( chatId: Chat.Id, @@ -146,7 +146,7 @@ final class ChatApi( coll.byId[UserChat](chatId.value) zip userRepo.byId(modId) zip userRepo.byId(userId) flatMap { case Some(chat) ~ Some(mod) ~ Some(user) if isMod(mod) || scope == ChatTimeout.Scope.Local => doTimeout(chat, mod, user, reason, scope, text, busChan) - case _ => fuccess(none) + case _ => funit } def userModInfo(username: String): Fu[Option[UserModInfo]] = @@ -256,7 +256,7 @@ final class ChatApi( makeLine(chatId, color, text) ?? { line => pushLine(chatId, line) >>- { publish(chatId, actorApi.ChatLine(chatId, line), busChan) - lila.mon.chat.message("anonPlayer", troll = false).increment() + lila.mon.chat.message("anonPlayer", troll = false).increment().unit } } diff --git a/modules/clas/src/main/Env.scala b/modules/clas/src/main/Env.scala index 0fe2250857..a0897dfcea 100644 --- a/modules/clas/src/main/Env.scala +++ b/modules/clas/src/main/Env.scala @@ -35,7 +35,7 @@ final class Env( lila.common.Bus.subscribeFuns( "finishGame" -> { case lila.game.actorApi.FinishGame(game, _, _) => - progressApi.onFinishGame(game) + progressApi.onFinishGame(game).unit }, "clas" -> { case lila.hub.actorApi.clas.IsTeacherOf(teacher, student, promise) => promise completeWith api.clas.isTeacherOfStudent(teacher, Student.Id(student)) diff --git a/modules/coach/src/main/Env.scala b/modules/coach/src/main/Env.scala index 069f133b1f..cb81263b69 100644 --- a/modules/coach/src/main/Env.scala +++ b/modules/coach/src/main/Env.scala @@ -43,18 +43,18 @@ final class Env( lila.common.Bus.subscribeFun("adjustCheater", "finishGame", "shadowban", "setPermissions") { case lila.hub.actorApi.mod.Shadowban(userId, true) => api.toggleApproved(userId, value = false) - api.reviews deleteAllBy userId + api.reviews.deleteAllBy(userId).unit case lila.hub.actorApi.mod.MarkCheater(userId, true) => api.toggleApproved(userId, value = false) - api.reviews deleteAllBy userId + api.reviews.deleteAllBy(userId).unit case lila.hub.actorApi.mod.SetPermissions(userId, permissions) => - api.toggleApproved(userId, permissions.has(Permission.Coach.dbKey)) + api.toggleApproved(userId, permissions.has(Permission.Coach.dbKey)).unit case lila.game.actorApi.FinishGame(game, white, black) if game.rated => if (game.perfType.exists(lila.rating.PerfType.standard.contains)) { white ?? api.setRating black ?? api.setRating - } - case lila.user.User.GDPRErase(user) => api.reviews deleteAllBy user.id + }.unit + case lila.user.User.GDPRErase(user) => api.reviews.deleteAllBy(user.id).unit } def cli = diff --git a/modules/common/src/main/Bus.scala b/modules/common/src/main/Bus.scala index b5ffa7a6f7..277d893a73 100644 --- a/modules/common/src/main/Bus.scala +++ b/modules/common/src/main/Bus.scala @@ -81,22 +81,26 @@ final private class EventBus[Event, Channel, Subscriber]( private val entries = new ConcurrentHashMap[Channel, Set[Subscriber]](initialCapacity) def subscribe(subscriber: Subscriber, channel: Channel): Unit = - entries.compute( - channel, - (_: Channel, subs: Set[Subscriber]) => { - Option(subs).fold(Set(subscriber))(_ + subscriber) - } - ) + entries + .compute( + channel, + (_: Channel, subs: Set[Subscriber]) => { + Option(subs).fold(Set(subscriber))(_ + subscriber) + } + ) + .unit def unsubscribe(subscriber: Subscriber, channel: Channel): Unit = - entries.computeIfPresent( - channel, - (_: Channel, subs: Set[Subscriber]) => { - val newSubs = subs - subscriber - if (newSubs.isEmpty) null - else newSubs - } - ) + entries + .computeIfPresent( + channel, + (_: Channel, subs: Set[Subscriber]) => { + val newSubs = subs - subscriber + if (newSubs.isEmpty) null + else newSubs + } + ) + .unit def publish(event: Event, channel: Channel): Unit = Option(entries get channel) foreach { diff --git a/modules/common/src/main/Lilaisms.scala b/modules/common/src/main/Lilaisms.scala index 9d58107562..d742bad6e3 100644 --- a/modules/common/src/main/Lilaisms.scala +++ b/modules/common/src/main/Lilaisms.scala @@ -32,6 +32,7 @@ trait Lilaisms @inline implicit def toPimpedJsObject(jo: JsObject) = new PimpedJsObject(jo) @inline implicit def toPimpedJsValue(jv: JsValue) = new PimpedJsValue(jv) + @inline implicit def toAugmentedAny(b: Any) = new AugmentedAny(b) @inline implicit def toPimpedBoolean(b: Boolean) = new PimpedBoolean(b) @inline implicit def toPimpedInt(i: Int) = new PimpedInt(i) @inline implicit def toPimpedLong(l: Long) = new PimpedLong(l) diff --git a/modules/common/src/main/base/PimpedPrimitives.scala b/modules/common/src/main/base/PimpedPrimitives.scala index dcd5c7dbb2..85811a69d2 100644 --- a/modules/common/src/main/base/PimpedPrimitives.scala +++ b/modules/common/src/main/base/PimpedPrimitives.scala @@ -4,6 +4,13 @@ import java.lang.Math.{ max, min } import ornicar.scalalib.Zero +final class AugmentedAny(private val self: Any) extends AnyVal { + + // sugar for -Wvalue-discard + @scala.annotation.nowarn + def unit: Unit = () +} + final class PimpedBoolean(private val self: Boolean) extends AnyVal { /** diff --git a/modules/irwin/src/main/IrwinApi.scala b/modules/irwin/src/main/IrwinApi.scala index 6a93af8754..95ba9a9f84 100644 --- a/modules/irwin/src/main/IrwinApi.scala +++ b/modules/irwin/src/main/IrwinApi.scala @@ -39,7 +39,7 @@ final class IrwinApi( reportColl.update.one($id(report._id), report, upsert = true) >> markOrReport(report) >> notification(report) >>- - lila.mon.mod.irwin.ownerReport(report.owner).increment() + lila.mon.mod.irwin.ownerReport(report.owner).increment().pp def get(user: User): Fu[Option[IrwinReport]] = reportColl.find($id(user.id)).one[IrwinReport] diff --git a/project/BuildSettings.scala b/project/BuildSettings.scala index 9381c47e37..22437e7ddf 100644 --- a/project/BuildSettings.scala +++ b/project/BuildSettings.scala @@ -89,11 +89,11 @@ object BuildSettings { // "-Wnumeric-widen", "-Wunused:imports", "-Wunused:locals", - "-Wunused:patvars" + "-Wunused:patvars", // "-Wunused:privates", // unfortunately doesn't work with macros // "-Wunused:implicits", // "-Wunused:params" - // "-Wvalue-discard", + "-Wvalue-discard" ) val srcMain = Seq(