diff --git a/README.md b/README.md index 4769df31..5662deb1 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ To suppress warnings globally for the project, use `disabledInspections` or `ove ### Inspections -There are currently 122 inspections for Scala 2, and 2 for Scala 3. +There are currently 123 inspections for Scala 2, and 3 for Scala 3. An overview list is given, followed by a more detailed description of each inspection after the list (todo: finish rest of detailed descriptions) | Name | Brief Description | Default Level | Scala 2 | Scala 3 | @@ -168,6 +168,7 @@ An overview list is given, followed by a more detailed description of each inspe | ArraysToString | Checks for explicit toString calls on arrays | Warning | Yes | No | | AsInstanceOf | Checks for use of `asInstanceOf` | Warning | Yes | No | | AvoidOperatorOverload | Checks for mental symbolic method names | Info | Yes | No | +| AvoidRequire | Use of require | Warning | Yes | Yes | | AvoidSizeEqualsZero | Traversable.size can be slow for some data structure, prefer .isEmpty | Warning | Yes | No | | AvoidSizeNotEqualsZero | Traversable.size can be slow for some data structure, prefer .nonEmpty | Warning | Yes | No | | AvoidToMinusOne | Checks for loops that use `x to n-1` instead of `x until n` | Info | Yes | No | diff --git a/src/main/scala-2/com/sksamuel/scapegoat/Inspections.scala b/src/main/scala-2/com/sksamuel/scapegoat/Inspections.scala index 605990eb..c9a0df26 100644 --- a/src/main/scala-2/com/sksamuel/scapegoat/Inspections.scala +++ b/src/main/scala-2/com/sksamuel/scapegoat/Inspections.scala @@ -27,6 +27,7 @@ object Inspections extends App { def inspections: Seq[Inspection] = Seq( + new AvoidRequire, new ArrayEquals, new ArraysInFormat, new ArraysToString, diff --git a/src/main/scala-2/com/sksamuel/scapegoat/inspections/AvoidRequire.scala b/src/main/scala-2/com/sksamuel/scapegoat/inspections/AvoidRequire.scala new file mode 100644 index 00000000..83254e29 --- /dev/null +++ b/src/main/scala-2/com/sksamuel/scapegoat/inspections/AvoidRequire.scala @@ -0,0 +1,31 @@ +package com.sksamuel.scapegoat.inspections + +import com.sksamuel.scapegoat.{Inspection, InspectionContext, Inspector, Levels} + +class AvoidRequire extends + Inspection( + text = "Use of require", + defaultLevel = Levels.Warning, + description = "Use require in code.", + explanation = "Using require throws an untyped Exception." + ) { + + def inspector(ctx: InspectionContext): Inspector = + new Inspector(ctx) { + override def postTyperTraverser: context.Traverser = + new context.Traverser { + + import context.global._ + + override def inspect(tree: Tree): Unit = { + tree match { + case Select(lhs, TermName("require")) if lhs.tpe.typeSymbol.fullName == "scala.Predef" => + context.warn(tree.pos, self, tree.toString.take(200)) + case _ => + continue(tree) + } + } + } + } + +} diff --git a/src/main/scala-3/com/sksamuel/scapegoat/Inspections.scala b/src/main/scala-3/com/sksamuel/scapegoat/Inspections.scala index c5dae87c..76a189c2 100644 --- a/src/main/scala-3/com/sksamuel/scapegoat/Inspections.scala +++ b/src/main/scala-3/com/sksamuel/scapegoat/Inspections.scala @@ -1,5 +1,6 @@ package com.sksamuel.scapegoat +import com.sksamuel.scapegoat.inspections.AvoidRequire import com.sksamuel.scapegoat.inspections.option._ import com.sksamuel.scapegoat.inspections.traits._ @@ -7,6 +8,7 @@ object Inspections { final val inspections: List[Inspection] = List( new OptionGet, + new AvoidRequire, new AbstractTrait ) diff --git a/src/main/scala-3/com/sksamuel/scapegoat/inspections/AvoidRequire.scala b/src/main/scala-3/com/sksamuel/scapegoat/inspections/AvoidRequire.scala new file mode 100644 index 00000000..9b85d58e --- /dev/null +++ b/src/main/scala-3/com/sksamuel/scapegoat/inspections/AvoidRequire.scala @@ -0,0 +1,39 @@ +package com.sksamuel.scapegoat.inspections + +import com.sksamuel.scapegoat.* +import dotty.tools.dotc.ast.Trees.* +import dotty.tools.dotc.ast.tpd +import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.core.Symbols.Symbol +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." + ) { + + import tpd.* + + def inspect(feedback: Feedback[SourcePosition], tree: tpd.Tree)(using ctx: Context): Unit = { + val traverser = new InspectionTraverser { + def traverse(tree: Tree)(using Context): Unit = { + tree match { + 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 == "" => + feedback.warn(tree.sourcePos, self, tree.asSnippet) + case x => + } + case _ => traverseChildren(tree) + } + } + } + traverser.traverse(tree) + } +} \ No newline at end of file diff --git a/src/test/scala-2/com/sksamuel/scapegoat/inspections/AvoidRequireTest.scala b/src/test/scala-2/com/sksamuel/scapegoat/inspections/AvoidRequireTest.scala new file mode 100644 index 00000000..7ea62577 --- /dev/null +++ b/src/test/scala-2/com/sksamuel/scapegoat/inspections/AvoidRequireTest.scala @@ -0,0 +1,62 @@ +package com.sksamuel.scapegoat.inspections + +import com.sksamuel.scapegoat.{Inspection, InspectionTest} + +class AvoidRequireTest extends InspectionTest { + override val inspections = Seq[Inspection](new AvoidRequire) + + "require use" - { + "should return warning in method" in { + val code = + """ + object Test { + def test(x: Int): Int = { + require(x == 1) + x + } + } + """.stripMargin + + compileCodeSnippet(code) + compiler.scapegoat.feedback.warnings.size shouldBe 1 + } + + "should return warning in class" in { + val code = + """ + class Test(val x: Int) { + require(x == 1, "oops") + } + """.stripMargin + + compileCodeSnippet(code) + compiler.scapegoat.feedback.warnings.size shouldBe 1 + } + + "should not return warning on own require method" in { + val code = + """ + object T { + def require(x: Boolean): Boolean = false + + def foo(): Boolean = { + require(1 == 1) + } + } + """.stripMargin + + compileCodeSnippet(code) + compiler.scapegoat.feedback.warnings.size shouldBe 0 + } + + "should not return warning if no require" in { + val code = + """ + class Test(val x: Int) { } + """.stripMargin + + compileCodeSnippet(code) + compiler.scapegoat.feedback.warnings.size shouldBe 0 + } + } +} diff --git a/src/test/scala-3/com/sksamuel/scapegoat/inspections/AvoidRequireTest.scala b/src/test/scala-3/com/sksamuel/scapegoat/inspections/AvoidRequireTest.scala new file mode 100644 index 00000000..ced2702b --- /dev/null +++ b/src/test/scala-3/com/sksamuel/scapegoat/inspections/AvoidRequireTest.scala @@ -0,0 +1,62 @@ +package com.sksamuel.scapegoat.inspections + +import com.sksamuel.scapegoat.InspectionTest + +class AvoidRequireTest extends InspectionTest(classOf[AvoidRequire]) { + + "require use" - { + "should return warning in method" in { + val code = + """ + object Test { + def test(x: Int): Int = { + require(x == 1) + x + } + } + """.stripMargin + + val feedback = runner.compileCodeSnippet(code) + feedback.warnings.assertable.size shouldBe 1 + } + + "should return warning in class" in { + val code = + """ + class Test(val x: Int) { + require(x == 1, "oops") + } + """.stripMargin + + val feedback = runner.compileCodeSnippet(code) + feedback.warnings.assertable.size shouldBe 1 + } + + "should not return warning on own require method" in { + val code = + """ + object T { + def require(x: Boolean): Boolean = false + + def foo(): Boolean = { + require(1 == 1) + } + } + """.stripMargin + + val feedback = runner.compileCodeSnippet(code) + feedback.warnings.assertable.size shouldBe 0 + } + + "should not return warning if no require" in { + val code = + """ + class Test(val x: Int) { } + """.stripMargin + + val feedback = runner.compileCodeSnippet(code) + feedback.warnings.assertable.size shouldBe 0 + } + } + +}