From eb56b4163aa9500207ab970349de651a92c8aa8e Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Thu, 29 Jun 2023 23:56:13 +0900 Subject: [PATCH 1/9] =?UTF-8?q?feat:=20case=20class=E3=81=AB=E5=AF=BE?= =?UTF-8?q?=E3=81=97=E3=81=A6=E6=9A=97=E9=BB=99=E3=81=AEtoString=E3=82=92?= =?UTF-8?q?=E5=91=BC=E3=82=93=E3=81=A7=E3=81=84=E3=82=8B=E7=AE=87=E6=89=80?= =?UTF-8?q?=E3=82=92=E3=83=81=E3=82=A7=E3=83=83=E3=82=AF=E3=81=99=E3=82=8B?= =?UTF-8?q?=E3=83=AA=E3=83=B3=E3=83=88=E3=82=92=E4=BD=9C=E6=88=90?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kisaragi Marine --- build.sbt | 7 +- .../fix/ImplicitToStringCallOnCaseClass.scala | 79 +++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala diff --git a/build.sbt b/build.sbt index 63373d4518..f0b1ddb383 100644 --- a/build.sbt +++ b/build.sbt @@ -15,6 +15,8 @@ ThisBuild / description := "ギガンティック☆整地鯖の独自要素を // Scalafixが要求するため、semanticdbは有効化する ThisBuild / semanticdbEnabled := true ThisBuild / semanticdbVersion := scalafixSemanticdb.revision +ThisBuild / scalafixScalaBinaryVersion := + CrossVersion.binaryScalaVersion(scalaVersion.value) // endregion @@ -68,6 +70,9 @@ val providedDependencies = Seq( "org.typelevel" %% "simulacrum" % "1.0.1" ).map(_ % "provided") +val scalafixCoreDep = + "ch.epfl.scala" %% "scalafix-core" % _root_.scalafix.sbt.BuildInfo.scalafixVersion % ScalafixConfig + val testDependencies = Seq( "org.scalamock" %% "scalamock" % "5.2.0", "org.scalatest" %% "scalatest" % "3.2.16", @@ -189,7 +194,7 @@ Compile / PB.targets := Seq(scalapb.gen() -> (Compile / sourceManaged).value / " lazy val root = (project in file(".")).settings( name := "SeichiAssist", assembly / assemblyOutputPath := baseDirectory.value / "target" / "build" / "SeichiAssist.jar", - libraryDependencies := providedDependencies ++ testDependencies ++ dependenciesToEmbed, + libraryDependencies := (providedDependencies :+ scalafixCoreDep) ++ testDependencies ++ dependenciesToEmbed, excludeDependencies := Seq(ExclusionRule(organization = "org.bukkit", name = "bukkit")), unmanagedBase := baseDirectory.value / "localDependencies", scalacOptions ++= Seq( diff --git a/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala b/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala new file mode 100644 index 0000000000..e73a91f0df --- /dev/null +++ b/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala @@ -0,0 +1,79 @@ +package fix + +import scalafix.v1._ + +import scala.meta._ + +/** + * Lints on string interpolation where its variable part contains `case class` without `toString`. + */ +class ImplicitToStringCallOnCaseClass extends SemanticRule("ImplicitToStringCallOnCaseClass") { + override def fix(implicit doc: SemanticDocument): Patch = { + val s = doc.tree.collect { + // string interpolation in standard library + case inter @ Term.Interpolate((prefix, _, args)) => + if (prefix.value != "s") { + return Patch.empty + } + + Patch.fromIterable( + args.collect { arg => + val tp = arg.symbol + val info = tp.info.get + // does `tp` point to some `case class`? + val isCaseClass = info.isCase && info.isClass + + // lazily evaluated since most classes are not `case class` + lazy val isToStringOverriden = info.overriddenSymbols.exists(overridenMethodSym => overridenMethodSym.value == "toString" && { + val sig = overridenMethodSym.info.get.signature + sig match { + case MethodSignature(List(), List(), returnType) => + val ret = simpleDealias(returnType) + ret match { + case TypeRef(_, symbol, _) => symbol.value == "java.lang.String" + case _ => false + } + case _ => false + } + }) + + if (!isCaseClass || isToStringOverriden) { + return Patch.empty + } + + Patch.lint(new Diagnostic { + override def message: String = "Case class value shouldn't interpolated, please use `toString` if it is really intended snippet" + + // points to arg + override def position: _root_.scala.meta.Position = arg.pos + }) + } + ) + case _ => Patch.empty + } + + Patch.fromIterable(s) + } + + def getType(symbol: Symbol)(implicit sym: Symtab): SemanticType = + symbol.info.get.signature match { + case MethodSignature(_, _, returnType) => + returnType + } + + def simpleDealias(tpe: SemanticType)(implicit sym: Symtab): SemanticType = { + def dealiasSymbol(symbol: Symbol): Symbol = + symbol.info.get.signature match { + case TypeSignature(_, lowerBound@TypeRef(_, dealiased, _), upperBound) + if lowerBound == upperBound => + dealiased + case _ => + symbol + } + + tpe match { + case TypeRef(prefix, symbol, typeArguments) => + TypeRef(prefix, dealiasSymbol(symbol), typeArguments.map(simpleDealias)) + } + } +} From 47af477514eb8f48f453240f319026bac2a71a31 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 30 Jun 2023 19:23:51 +0900 Subject: [PATCH 2/9] refactor: delete unused Signed-off-by: Kisaragi Marine --- .../scala/fix/ImplicitToStringCallOnCaseClass.scala | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala b/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala index e73a91f0df..fecb4da27c 100644 --- a/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala +++ b/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala @@ -55,12 +55,6 @@ class ImplicitToStringCallOnCaseClass extends SemanticRule("ImplicitToStringCall Patch.fromIterable(s) } - def getType(symbol: Symbol)(implicit sym: Symtab): SemanticType = - symbol.info.get.signature match { - case MethodSignature(_, _, returnType) => - returnType - } - def simpleDealias(tpe: SemanticType)(implicit sym: Symtab): SemanticType = { def dealiasSymbol(symbol: Symbol): Symbol = symbol.info.get.signature match { @@ -74,6 +68,7 @@ class ImplicitToStringCallOnCaseClass extends SemanticRule("ImplicitToStringCall tpe match { case TypeRef(prefix, symbol, typeArguments) => TypeRef(prefix, dealiasSymbol(symbol), typeArguments.map(simpleDealias)) + case other => other } } } From bd03bf02ec68cc572033b7b03e0318395d16b316 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 30 Jun 2023 21:26:06 +0900 Subject: [PATCH 3/9] refactor: delete unused binding Signed-off-by: Kisaragi Marine --- src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala b/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala index fecb4da27c..a74c237533 100644 --- a/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala +++ b/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala @@ -11,7 +11,7 @@ class ImplicitToStringCallOnCaseClass extends SemanticRule("ImplicitToStringCall override def fix(implicit doc: SemanticDocument): Patch = { val s = doc.tree.collect { // string interpolation in standard library - case inter @ Term.Interpolate((prefix, _, args)) => + case Term.Interpolate((prefix, _, args)) => if (prefix.value != "s") { return Patch.empty } From 8b3d093c413b9bd998e6a3c65fee1168b72b8da1 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 30 Jun 2023 21:26:49 +0900 Subject: [PATCH 4/9] chore: suppress IntelliJ warning Signed-off-by: Kisaragi Marine --- src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala b/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala index a74c237533..4b1026bd0f 100644 --- a/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala +++ b/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala @@ -7,6 +7,7 @@ import scala.meta._ /** * Lints on string interpolation where its variable part contains `case class` without `toString`. */ +// noinspection ScalaUnusedSymbol; referred from scalafix implicitly class ImplicitToStringCallOnCaseClass extends SemanticRule("ImplicitToStringCallOnCaseClass") { override def fix(implicit doc: SemanticDocument): Patch = { val s = doc.tree.collect { From 4eac5dbdef02ca89e038f2402e8c9f9cd8c84339 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 30 Jun 2023 21:27:11 +0900 Subject: [PATCH 5/9] chore: add hint for future reference Signed-off-by: Kisaragi Marine --- src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala b/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala index 4b1026bd0f..5afe8a2df6 100644 --- a/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala +++ b/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala @@ -8,6 +8,7 @@ import scala.meta._ * Lints on string interpolation where its variable part contains `case class` without `toString`. */ // noinspection ScalaUnusedSymbol; referred from scalafix implicitly +// NOTE: see AST on https://xuwei-k.github.io/scalameta-ast/ or https://astexplorer.net class ImplicitToStringCallOnCaseClass extends SemanticRule("ImplicitToStringCallOnCaseClass") { override def fix(implicit doc: SemanticDocument): Patch = { val s = doc.tree.collect { From 429ab07eef6fc36059d28f14b99e839acd061b49 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 30 Jun 2023 21:30:51 +0900 Subject: [PATCH 6/9] refactor: rename the lint Signed-off-by: Kisaragi Marine --- ...a => WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename src/scalafix/scala/fix/{ImplicitToStringCallOnCaseClass.scala => WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala} (95%) diff --git a/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala b/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala similarity index 95% rename from src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala rename to src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala index 5afe8a2df6..055987938b 100644 --- a/src/scalafix/scala/fix/ImplicitToStringCallOnCaseClass.scala +++ b/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala @@ -9,7 +9,7 @@ import scala.meta._ */ // noinspection ScalaUnusedSymbol; referred from scalafix implicitly // NOTE: see AST on https://xuwei-k.github.io/scalameta-ast/ or https://astexplorer.net -class ImplicitToStringCallOnCaseClass extends SemanticRule("ImplicitToStringCallOnCaseClass") { +class WarnUnoverriddenImplicitToStringCallsOnCaseClass extends SemanticRule("WarnUnoverriddenImplicitToStringCallsOnCaseClass") { override def fix(implicit doc: SemanticDocument): Patch = { val s = doc.tree.collect { // string interpolation in standard library From 7222d2037ae320490b3720b6bf83197f65bdd563 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 30 Jun 2023 21:34:37 +0900 Subject: [PATCH 7/9] =?UTF-8?q?refactor:=20toString=E3=81=AE=E6=88=BB?= =?UTF-8?q?=E3=82=8A=E5=80=A4=E3=81=AE=E5=9E=8B=E3=81=AE=E3=83=81=E3=82=A7?= =?UTF-8?q?=E3=83=83=E3=82=AF=E3=82=92=E3=82=B3=E3=83=B3=E3=83=91=E3=82=A4?= =?UTF-8?q?=E3=83=A9=E3=81=AB=E4=BB=BB=E3=81=9B=E3=81=A6=E3=83=AA=E3=83=B3?= =?UTF-8?q?=E3=83=88=E3=81=A7=E3=81=AF=E7=84=A1=E8=A6=96=E3=81=99=E3=82=8B?= =?UTF-8?q?=E3=82=88=E3=81=86=E3=81=AB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kisaragi Marine --- ...ddenImplicitToStringCallsOnCaseClass.scala | 26 +++---------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala b/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala index 055987938b..6a764c73dc 100644 --- a/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala +++ b/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala @@ -29,12 +29,9 @@ class WarnUnoverriddenImplicitToStringCallsOnCaseClass extends SemanticRule("War lazy val isToStringOverriden = info.overriddenSymbols.exists(overridenMethodSym => overridenMethodSym.value == "toString" && { val sig = overridenMethodSym.info.get.signature sig match { - case MethodSignature(List(), List(), returnType) => - val ret = simpleDealias(returnType) - ret match { - case TypeRef(_, symbol, _) => symbol.value == "java.lang.String" - case _ => false - } + // もしtoString()のreturn typeがStringかそのサブタイプにならないような型であれば、 + // scalafixが走る前にコンパイルが落ちるのでここで改めて考慮する必要はない + case MethodSignature(List(), List(), _) => true case _ => false } }) @@ -56,21 +53,4 @@ class WarnUnoverriddenImplicitToStringCallsOnCaseClass extends SemanticRule("War Patch.fromIterable(s) } - - def simpleDealias(tpe: SemanticType)(implicit sym: Symtab): SemanticType = { - def dealiasSymbol(symbol: Symbol): Symbol = - symbol.info.get.signature match { - case TypeSignature(_, lowerBound@TypeRef(_, dealiased, _), upperBound) - if lowerBound == upperBound => - dealiased - case _ => - symbol - } - - tpe match { - case TypeRef(prefix, symbol, typeArguments) => - TypeRef(prefix, dealiasSymbol(symbol), typeArguments.map(simpleDealias)) - case other => other - } - } } From 234cb258060f6e60b834fd3a1db25ab2d0f979b1 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 30 Jun 2023 21:36:17 +0900 Subject: [PATCH 8/9] chore: accept grammer suggestion Signed-off-by: Kisaragi Marine --- .../fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala b/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala index 6a764c73dc..063115c3ab 100644 --- a/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala +++ b/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala @@ -41,7 +41,8 @@ class WarnUnoverriddenImplicitToStringCallsOnCaseClass extends SemanticRule("War } Patch.lint(new Diagnostic { - override def message: String = "Case class value shouldn't interpolated, please use `toString` if it is really intended snippet" + override def message: String = "Case class value shouldn't be interpolated, use `toString` " + + "if you wish to interpolate the String representation into the string" // points to arg override def position: _root_.scala.meta.Position = arg.pos From 4812ba8afad41b8ca92661170cb5abad569c1b59 Mon Sep 17 00:00:00 2001 From: Kisaragi <48310258+KisaragiEffective@users.noreply.github.com> Date: Sat, 1 Jul 2023 00:19:59 +0900 Subject: [PATCH 9/9] chore: apply suggestions from code review Co-authored-by: Kory <6561358+kory33@users.noreply.github.com> --- ...arnUnoverriddenImplicitToStringCallsOnCaseClass.scala | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala b/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala index 063115c3ab..b9cb98285b 100644 --- a/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala +++ b/src/scalafix/scala/fix/WarnUnoverriddenImplicitToStringCallsOnCaseClass.scala @@ -27,10 +27,11 @@ class WarnUnoverriddenImplicitToStringCallsOnCaseClass extends SemanticRule("War // lazily evaluated since most classes are not `case class` lazy val isToStringOverriden = info.overriddenSymbols.exists(overridenMethodSym => overridenMethodSym.value == "toString" && { - val sig = overridenMethodSym.info.get.signature - sig match { - // もしtoString()のreturn typeがStringかそのサブタイプにならないような型であれば、 - // scalafixが走る前にコンパイルが落ちるのでここで改めて考慮する必要はない + overridenMethodSym.info.get.signature match { + // def toString[](): の形の override を見つけたい。 + // もし toString() の return type が String のサブタイプにならないような型であれば + // scalafix が走る前にコンパイルが落ちるので、ここで改めて return type が + // String のサブタイプであるかは考慮する必要はない case MethodSignature(List(), List(), _) => true case _ => false }