Add require check (#888)

* require check for scala 2 * add check for scala 3 * add . to the end for description * reorder imports for scalafix * fix review --------- Co-authored-by: Bendix Sältz <[email protected]>
scapegoat-scala · Oct 21, 2024 · 249a070 · 249a070
1 parent 5808954
commit 249a070
Show file tree

Hide file tree

Showing 7 changed files with 199 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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
@@ -1,12 +1,14 @@
 package com.sksamuel.scapegoat
 
+import com.sksamuel.scapegoat.inspections.AvoidRequire
 import com.sksamuel.scapegoat.inspections.option._
 import com.sksamuel.scapegoat.inspections.traits._
 
 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
@@ -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 == "<root>" =>
+                feedback.warn(tree.sourcePos, self, tree.asSnippet)
+              case x =>
+            }
+          case _ => traverseChildren(tree)
+        }
+      }
+    }
+    traverser.traverse(tree)
+  }
+}
diff --git a/src/test/scala-2/com/sksamuel/scapegoat/inspections/AvoidRequireTest.scala 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
@@ -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
+    }
+  }
+
+}