diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 455eb021..7067d589 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -12,8 +12,8 @@ jobs: scala: - 2.12.19 - 2.12.20 - - 2.13.14 - 2.13.15 + - 2.13.16 - 3.3.4 - 3.6.2 steps: @@ -57,6 +57,8 @@ jobs: - uses: codecov/codecov-action@v4 - name: Run Scalafix check run: sbt fixCheck + - name: Check formatting + run: sbt scalafmtCheckAll - name: Run Scapegoat run: | sbt 'set version := "99.0-SNAPSHOT"; publishLocal' diff --git a/build.sbt b/build.sbt index 792ddd71..4d65e443 100644 --- a/build.sbt +++ b/build.sbt @@ -20,7 +20,7 @@ developers := List( ) scalaVersion := "3.6.2" -crossScalaVersions := Seq("2.12.19", "2.12.20", "2.13.14", "2.13.15", "3.3.4", "3.6.2") +crossScalaVersions := Seq("2.12.19", "2.12.20", "2.13.15", "2.13.16", "3.3.4", "3.6.2") autoScalaLibrary := false crossVersion := CrossVersion.full crossTarget := { @@ -118,7 +118,7 @@ libraryDependencies ++= (if (scalaBinaryVersion.value == "3") { "org.scala-lang" % "scala-compiler" % scalaVersion.value % "provided", "org.scala-lang" % "scala-compiler" % scalaVersion.value % "test", compilerPlugin( - "org.scalameta" % "semanticdb-scalac" % "4.12.3" cross CrossVersion.full + "org.scalameta" % "semanticdb-scalac" % "4.12.4" cross CrossVersion.full ) ) }) diff --git a/src/main/scala-2/com/sksamuel/scapegoat/inspections/VariableShadowing.scala b/src/main/scala-2/com/sksamuel/scapegoat/inspections/VariableShadowing.scala index 3e4db375..b26adfa8 100644 --- a/src/main/scala-2/com/sksamuel/scapegoat/inspections/VariableShadowing.scala +++ b/src/main/scala-2/com/sksamuel/scapegoat/inspections/VariableShadowing.scala @@ -31,7 +31,9 @@ class VariableShadowing tree match { case Block(_, _) | ClassDef(_, _, _, Template(_, _, _)) | ModuleDef(_, _, Template(_, _, _)) | Function(_, _) => - enter(); continue(tree); exit() + enter() + continue(tree) + exit() case DefDef(_, name, _, vparamss, _, rhs) => enter() // For case classes there's a synthetic constructor (not marked as ) which takes @@ -40,13 +42,13 @@ class VariableShadowing vparamss.foreach(_.foreach(inspect)) inspect(rhs) exit() - case ValDef(_, TermName(name), _, _) => + case ValDef(_, TermName(name), _, _) if name != "_" => if (isDefined(name)) context.warn(tree.pos, self, tree.toString.take(200)) contexts.headOption.foreach(_.append(name.trim)) case Match(_, cases) => cases.foreach { - case CaseDef(Bind(name, _), _, _) => - if (isDefined(name.toString)) context.warn(tree.pos, self, tree.toString.take(200)) + case CaseDef(Bind(name, _), _, _) if isDefined(name.toString) => + context.warn(tree.pos, self, tree.toString.take(200)) case _ => // do nothing } continue(tree) diff --git a/src/main/scala-3/com/sksamuel/scapegoat/Plugin.scala b/src/main/scala-3/com/sksamuel/scapegoat/Plugin.scala index 6433efcf..f3e37a40 100644 --- a/src/main/scala-3/com/sksamuel/scapegoat/Plugin.scala +++ b/src/main/scala-3/com/sksamuel/scapegoat/Plugin.scala @@ -49,7 +49,7 @@ class ScapegoatPhase(var configuration: Configuration, override val inspections: } else { if (configuration.verbose) { runCtx.reporter.report( - Info(s"Running with ${activeInspections.size} active inspections", NoSourcePosition) + Info(s"[scapegoat] Running with ${activeInspections.size} active inspections", NoSourcePosition) ) } diff --git a/src/main/scala-3/com/sksamuel/scapegoat/inspections/AvoidRequire.scala b/src/main/scala-3/com/sksamuel/scapegoat/inspections/AvoidRequire.scala index 9b85d58e..5c496247 100644 --- a/src/main/scala-3/com/sksamuel/scapegoat/inspections/AvoidRequire.scala +++ b/src/main/scala-3/com/sksamuel/scapegoat/inspections/AvoidRequire.scala @@ -9,12 +9,12 @@ import dotty.tools.dotc.core.Types.TermRef import dotty.tools.dotc.util.SourcePosition class AvoidRequire - extends Inspection( - text = "Use of require", - defaultLevel = Levels.Warning, - description = "Use require in code.", - explanation = "Using require throws an untyped Exception." - ) { + extends Inspection( + text = "Use of require", + defaultLevel = Levels.Warning, + description = "Use require in code.", + explanation = "Using require throws an untyped Exception." + ) { import tpd.* @@ -25,10 +25,10 @@ class AvoidRequire case Apply(ident: Ident, _) if ident.name.toString == "require" => ident.tpe.normalizedPrefix match { case TermRef(tx, nm: Symbol) - if nm.toString == "object Predef" && - tx.normalizedPrefix.typeSymbol.name.toString == "" => + if nm.toString == "object Predef" && + tx.normalizedPrefix.typeSymbol.name.toString == "" => feedback.warn(tree.sourcePos, self, tree.asSnippet) - case x => + case _ => } case _ => traverseChildren(tree) } @@ -36,4 +36,4 @@ class AvoidRequire } traverser.traverse(tree) } -} \ No newline at end of file +} diff --git a/src/test/scala-2/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala b/src/test/scala-2/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala index ff88a40e..2d6aa5b3 100644 --- a/src/test/scala-2/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala +++ b/src/test/scala-2/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala @@ -5,222 +5,206 @@ import com.sksamuel.scapegoat.{Inspection, InspectionTest} /** @author Stephen Samuel */ class VariableShadowingTest extends InspectionTest { - override val inspections = Seq[Inspection](new VariableShadowing) + override val inspections: Seq[Inspection] = Seq(new VariableShadowing) "VariableShadowing" - { "should report warning" - { "when variable shadows in nested def" in { - val code = - """class Test { - | def foo = { - | val a = 1 - | def boo = { - | val a = 2 - | println(a) - | } - | } - |} """.stripMargin + val code = """class Test { + | def foo = { + | val a = 1 + | def boo = { + | val a = 2 + | println(a) + | } + | } + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 1 } + "when variable defined in def shadows field" in { - val code = - """class Test { - | val a = 1 - | def foo = { - | val a = 2 - | println(a) - | } - |} """.stripMargin + val code = """class Test { + | val a = 1 + | def foo = { + | val a = 2 + | println(a) + | } + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 1 } + "when variable defined as case bind shadows field" in { - val code = - """class Test { - | val a = 1 - | def foo(b : Int) = b match { - | case a => println(a) - | } - |} """.stripMargin + val code = """class Test { + | val a = 1 + | def foo(b : Int) = b match { + | case a => println(a) + | } + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 1 } + "when variable defined in nested def in case shadows field" in { - val code = - """class Test { - | val a = 1 - | def foo(b : Int) = b match { - | case f => - | def boo() = { - | val a = 2 - | println(a) - | } - | } - |} """.stripMargin + val code = """class Test { + | val a = 1 + | def foo(b : Int) = b match { + | case f => + | def boo() = { + | val a = 2 + | println(a) + | } + | } + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 1 } + "when variable defined in def shadows parameter" in { - val code = - """class Test { - | def foo(a : Int) = { - | val a = 1 - | println(a) - | } - |} """.stripMargin + val code = """class Test { + | def foo(a : Int) = { + | val a = 1 + | println(a) + | } + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 1 } + "when variable defined in nested def shadows outer def parameter" in { - val code = - """class Test { - | def foo(a : Int) = { - | println(a) - | def boo = { - | val a = 2 - | println(a) - | } - | } - |} """.stripMargin + val code = """class Test { + | def foo(a : Int) = { + | println(a) + | def boo = { + | val a = 2 + | println(a) + | } + | } + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 1 } } + "should not report warning" - { "when sibling defs define same variable" in { - val code = - """class Test { - | def foo = { - | val a = 1 - | println(a) - | } - | def boo = { - | val a = 2 - | println(a) - | } - |} """.stripMargin + val code = """class Test { + | def foo = { + | val a = 1 + | println(a) + | } + | def boo = { + | val a = 2 + | println(a) + | } + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 0 } "when two if branches define the same variable" in { - val code = - """class Test { - | if (1 > 0) { - | val something = 4 - | println(something+1) - | } else { - | val something = 2 - | println(something+2) - | } - |}""".stripMargin + val code = """class Test { + | if (1 > 0) { + | val something = 4 + | println(something+1) + | } else { + | val something = 2 + | println(something+2) + | } + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 0 } "when two sibling cases define the same local variable" in { - val code = - """class Test { - | val possibility: Option[Int] = Some(3) - | possibility match { - | case Some(x) => - | val y = x + 1 - | println(y) - | case None => - | val y = 0 - | println(y) - | } - |}""".stripMargin + val code = """class Test { + | val possibility: Option[Int] = Some(3) + | possibility match { + | case Some(x) => + | val y = x + 1 + | println(y) + | case None => + | val y = 0 + | println(y) + | } + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 0 } "when visiting a match case, especially not visit it twice" in { - val code = - """class Test { - | val possibility: Option[Int] = Some(3) - | possibility match { - | case Some(x) => - | val y = x + 1 - | println(y) - | case None => println("None") - | } - |}""".stripMargin + val code = """class Test { + | val possibility: Option[Int] = Some(3) + | possibility match { + | case Some(x) => + | val y = x + 1 + | println(y) + | case None => println("None") + | } + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 0 } "when sibling case classes use the same argument name" in { - val code = - """ - |final case class A(value: String) - |final case class B(value: String) - |final case class C(value: Int) - |""".stripMargin + val code = """final case class A(value: String) + |final case class B(value: String) + |final case class C(value: Int) + |""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 0 } "when the same variable is used in two sibling for loops (#342)" in { - val code = - """ - |object Test { - | for (i <- 1 to 10) println(i.toString) - | for (i <- 1 to 10) println(i.toString) - |} - |""".stripMargin + val code = """object Test { + | for (i <- 1 to 10) println(i.toString) + | for (i <- 1 to 10) println(i.toString) + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 0 } "when using for-comprehension (#343)" in { - val code = - """ - |object Test { - | for { - | c <- "Hello, world!" - | if c != ',' - | } println(c) - |} - |""".stripMargin + val code = """object Test { + | for { + | c <- "Hello, world!" + | if c != ',' + | } println(c) + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 0 } + "when using Builder-pattern method chaining (#398)" in { - val code = - """ - |object Test { - | - | def f(x: Int): String = x.toString - | def g(y: String): Int = y.toInt - | - | val a = Seq(1, 2, 3) - | System.out.println( - | a - | .map(s => f(s)) - | .map(s => g(s)) - | ) - |} - |""".stripMargin + val code = """object Test { + | def f(x: Int): String = x.toString + | def g(y: String): Int = y.toInt + | + | val a = Seq(1, 2, 3) + | System.out.println( + | a + | .map(s => f(s)) + | .map(s => g(s)) + | ) + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 0 } "when passing function as an argument (#419)" in { val code = - """ - |class TestVariableShadowing { + """class TestVariableShadowing { | private def testCallbackFunction1(shadowedArg: Long): Boolean = shadowedArg > 10 | private def testCallbackFunction2(arg: Long): Boolean = arg < 10 | @@ -232,12 +216,21 @@ class VariableShadowingTest extends InspectionTest { | case l2: Double => testCaller(l2.toLong, testCallbackFunction2) | case l3 => false | } - |} - |""".stripMargin + |}""".stripMargin compileCodeSnippet(code) compiler.scapegoat.feedback.warnings.size shouldBe 0 } + "when using underscores as variable name" in { + val code = """object Test { + | for { + | _ <- Right("something") + | _ <- Right("something2") + | } yield () + |}""".stripMargin + compileCodeSnippet(code) + compiler.scapegoat.feedback.warnings.size shouldBe 0 + } } } }