Warnings clean up (#322)

* writeFile() unused argument * Removing unused parameters in pattern matches * Redundant braces removed * Excessive "case" removed * Excessive "new" removed * Remove extra parentheses * var -> val * Excessive braces removed * Added @tailrec
scapegoat-scala · Mar 25, 2020 · 14fbac4 · 14fbac4
1 parent 4b6c244
commit 14fbac4
Show file tree

Hide file tree

Showing 21 changed files with 55 additions and 61 deletions.
diff --git a/src/main/scala/com/sksamuel/scapegoat/Inspection.scala b/src/main/scala/com/sksamuel/scapegoat/Inspection.scala
@@ -54,6 +54,7 @@ case class InspectionContext(global: Global, feedback: Feedback) {
 
     private val SuppressWarnings = typeOf[SuppressWarnings]
 
+    @scala.annotation.tailrec
     private def inspectionClass(klass: Class[_]): Class[_] = Option(klass.getEnclosingClass) match {
       case None    => klass
       case Some(k) => inspectionClass(k)

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/LonelySealedTrait.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/LonelySealedTrait.scala
@@ -30,7 +30,7 @@ class LonelySealedTrait
 
     private def inspectParents(parents: List[Tree]): Unit = {
       parents.foreach {
-        case parent =>
+        parent =>
           for (c <- parent.tpe.baseClasses)
             implementedClasses.add(c.name.toString)
       }

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/MaxParameters.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/MaxParameters.scala
@@ -27,7 +27,7 @@ class MaxParameters
         tree match {
           case DefDef(_, name, _, _, _, _) if name == nme.CONSTRUCTOR =>
           case DefDef(mods, _, _, _, _, _) if mods.isSynthetic        =>
-          case DefDef(_, name, _, vparamss, _, _) if countExceeds(vparamss, 10) =>
+          case DefDef(_, _, _, vparamss, _, _) if countExceeds(vparamss, 10) =>
             context.warn(tree.pos, self)
           case _ => continue(tree)
         }

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/RedundantFinalModifierOnMethod.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/RedundantFinalModifierOnMethod.scala
@@ -26,7 +26,7 @@ class RedundantFinalModifierOnMethod
           case dd: DefDef if dd.symbol != null && dd.symbol.isSynthetic                                  =>
           case DefDef(mods, _, _, _, _, _) if mods.hasFlag(Flags.ACCESSOR)                               =>
           case DefDef(_, nme.CONSTRUCTOR, _, _, _, _)                                                    =>
-          case dd @ DefDef(mods, _, _, _, _, _)
+          case DefDef(mods, _, _, _, _, _)
               if mods.isFinal &&
               (tree.symbol.enclClass.isFinal ||
               tree.symbol.enclClass.isCase ||

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/TypeShadowing.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/TypeShadowing.scala
@@ -23,7 +23,7 @@ class TypeShadowing
         tparams.foreach(tparam => types.add(tparam.name.toString))
         trees.foreach {
           case dd: DefDef if dd.symbol != null && dd.symbol.isSynthetic =>
-          case dd @ DefDef(_, name, deftparams, _, _, _) =>
+          case dd @ DefDef(_, _, deftparams, _, _, _) =>
             deftparams.foreach { tparam =>
               if (types.contains(tparam.name.toString))
                 context.warn(dd.pos, self)

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/VariableShadowing.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/VariableShadowing.scala
@@ -30,7 +30,7 @@ class VariableShadowing
         tree match {
           case Block(_, _) =>
             enter(); continue(tree); exit()
-          case ClassDef(_, _, _, Template(_, _, body)) =>
+          case ClassDef(_, _, _, Template(_, _, _)) =>
             enter(); continue(tree); exit()
           case ModuleDef(_, _, Template(_, _, _)) =>
             enter(); continue(tree); exit()

diff --git a/...n/scala/com/sksamuel/scapegoat/inspections/collections/ExistsSimplifiableToContains.scala b/...n/scala/com/sksamuel/scapegoat/inspections/collections/ExistsSimplifiableToContains.scala
@@ -49,7 +49,7 @@ class ExistsSimplifiableToContains
               List(
                 Function(
                   List(ValDef(_, TermName(iterationVariable), _, _)),
-                  subtree @ Apply(Select(name, Equals), List(x))
+                  subtree @ Apply(Select(_, Equals), List(x))
                 )
               )
               )

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/collections/MapGetAndGetOrElse.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/collections/MapGetAndGetOrElse.scala
@@ -25,8 +25,8 @@ class MapGetAndGetOrElse
       override def inspect(tree: Tree): Unit = {
         tree match {
           case Apply(
-              TypeApply(Select(Apply(Select(left, TermName("get")), List(key)), TermName("getOrElse")), _),
-              List(defaultValue)
+              TypeApply(Select(Apply(Select(left, TermName("get")), List(_)), TermName("getOrElse")), _),
+              List(_)
               ) if isMap(left) =>
             context.warn(tree.pos, self, tree.toString.take(500))
           case _ => continue(tree)

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/collections/PreferSeqEmpty.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/collections/PreferSeqEmpty.scala
@@ -23,7 +23,7 @@ class PreferSeqEmpty
       override def inspect(tree: Tree): Unit = {
         tree match {
           case a @ Apply(TypeApply(Select(Select(_, SeqTerm), ApplyTerm), _), List())
-              if (!a.tpe.toString.startsWith("scala.collection.mutable.")) =>
+              if !a.tpe.toString.startsWith("scala.collection.mutable.") =>
             context.warn(tree.pos, self, tree.toString.take(500))
           case _ => continue(tree)
         }

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/collections/ReverseFunc.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/collections/ReverseFunc.scala
@@ -31,12 +31,12 @@ class ReverseFunc
 
       override def inspect(tree: Tree): Unit = {
         tree match {
-          case Select(Select(c, TermName("reverse")), TermName(FuncReplace(func, replace)))
+          case Select(Select(c, TermName("reverse")), TermName(FuncReplace(_, _)))
               if c.tpe <:< typeOf[Traversable[Any]] =>
             context.warn(tree.pos, self, tree.toString.take(500))
           case Select(
               Apply(arrayOps1, List(Select(Apply(arrayOps2, List(_)), TermName("reverse")))),
-              TermName(FuncReplace(func, replace))
+              TermName(FuncReplace(_, _))
               ) if arrayOps1.toString.contains("ArrayOps") && arrayOps2.toString.contains("ArrayOps") =>
             context.warn(tree.pos, self, tree.toString.take(500))
           case _ => continue(tree)

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/collections/ReverseTailReverse.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/collections/ReverseTailReverse.scala
@@ -32,7 +32,7 @@ class ReverseTailReverse
               ),
               TermName("reverse")
               )
-              if (arrayOps0.toString.contains("ArrayOps"))
+              if arrayOps0.toString.contains("ArrayOps")
               && arrayOps1.toString.contains("ArrayOps")
               && arrayOps2.toString.contains("ArrayOps") =>
             context.warn(tree.pos, self, tree.toString.take(500))

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/collections/ReverseTakeReverse.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/collections/ReverseTakeReverse.scala
@@ -35,9 +35,9 @@ class ReverseTakeReverse
               ),
               TermName("reverse")
               )
-              if (arrayOps0.toString.contains("ArrayOps"))
-              && (arrayOps1.toString.contains("ArrayOps"))
-              && (arrayOps2.toString.contains("ArrayOps")) =>
+              if arrayOps0.toString.contains("ArrayOps")
+              && arrayOps1.toString.contains("ArrayOps")
+              && arrayOps2.toString.contains("ArrayOps") =>
             context.warn(tree.pos, self, tree.toString.take(500))
           case _ => continue(tree)
         }

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/controlflow/RepeatedIfElseBody.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/controlflow/RepeatedIfElseBody.scala
@@ -21,7 +21,7 @@ class RepeatedIfElseBody
 
       private def twoBlocksStartWithTheSame(oneBlock: Block, another: Block): Boolean = {
         (oneBlock.children.headOption, another.children.headOption) match {
-          case (Some(statement1), Some(statement2)) if (statement1.toString == statement2.toString) => true
+          case (Some(statement1), Some(statement2)) if statement1.toString == statement2.toString => true
           case _                                                                                    => false
         }
       }

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/math/UseExpM1.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/math/UseExpM1.scala
@@ -19,7 +19,7 @@ class UseExpM1
       override def inspect(tree: Tree): Unit = {
         tree match {
           case Apply(
-              Select(Apply(Select(pack, TermName("exp")), List(_)), nme.SUB),
+              Select(Apply(Select(_, TermName("exp")), List(_)), nme.SUB),
               List(Literal(Constant(1)))
               ) =>
             context.warn(tree.pos, self)

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/string/InvalidRegex.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/string/InvalidRegex.scala
@@ -28,7 +28,7 @@ class InvalidRegex
             try {
               regex.toString.r
             } catch {
-              case e: PatternSyntaxException =>
+              case _: PatternSyntaxException =>
                 context.warn(tree.pos, self)
             }
           case _ => continue(tree)

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/style/ParameterlessMethodReturnsUnit.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/style/ParameterlessMethodReturnsUnit.scala
@@ -19,7 +19,7 @@ class ParameterlessMethodReturnsUnit
 
       override def inspect(tree: Tree): Unit = {
         tree match {
-          case DefDef(_, name, _, vparamss, tpt, _) if tpt.tpe.toString == "Unit" && vparamss.isEmpty =>
+          case DefDef(_, _, _, vparamss, tpt, _) if tpt.tpe.toString == "Unit" && vparamss.isEmpty =>
             context.warn(tree.pos, self, tree.toString.take(300))
           case _ => continue(tree)
         }

diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/unneccesary/VarCouldBeVal.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/unneccesary/VarCouldBeVal.scala
@@ -61,7 +61,7 @@ class VarCouldBeVal
       override final def inspect(tree: Tree): Unit = {
         tree match {
           case DefDef(_, _, _, _, _, Block(stmt, expr)) =>
-            for ((unwritten, definitionTree) <- containsUnwrittenVar(stmt :+ expr)) {
+            for ((_, definitionTree) <- containsUnwrittenVar(stmt :+ expr)) {
               context.warn(definitionTree.pos, self, tree.toString.take(200))
             }
           case _ => continue(tree)

diff --git a/src/main/scala/com/sksamuel/scapegoat/io/HtmlReportWriter.scala b/src/main/scala/com/sksamuel/scapegoat/io/HtmlReportWriter.scala
@@ -94,35 +94,30 @@ object HtmlReportWriter {
 
   def warnings(reporter: Feedback) = {
     reporter.warningsWithMinimalLevel.map {
-      case warning =>
+      warning =>
         val source = warning.sourceFileNormalized + ":" + warning.line
         <div class="warning">
           <div class="source">
             {source}
           </div>
           <div class="title">
-            {
-          warning.level match {
-            case Levels.Info    => <span class="label label-info">Info</span>
+            {warning.level match {
+            case Levels.Info => <span class="label label-info">Info</span>
             case Levels.Warning => <span class="label label-warning">Warning</span>
-            case Levels.Error   => <span class="label label-danger">Error</span>
-          }
-        }
-            &nbsp;{warning.text}
-            &nbsp;<span class="inspection">{warning.inspection}</span>
+            case Levels.Error => <span class="label label-danger">Error</span>
+          }}&nbsp;{warning.text}&nbsp; <span class="inspection">
+            {warning.inspection}
+          </span>
           </div>
           <div>
             {warning.explanation}
-          </div>
-            {
-          warning.snippet match {
-            case None =>
-            case Some(snippet) =>
-              <div class="snippet">
-                  {snippet}
-                </div>
-          }
-        }
+          </div>{warning.snippet match {
+          case None =>
+          case Some(snippet) =>
+            <div class="snippet">
+              {snippet}
+            </div>
+        }}
         </div>
     }
   }

diff --git a/src/main/scala/com/sksamuel/scapegoat/io/IOUtils.scala b/src/main/scala/com/sksamuel/scapegoat/io/IOUtils.scala
@@ -22,20 +22,20 @@ object IOUtils {
 
   def writeHTMLReport(targetDir: File, reporter: Feedback): File = {
     val html = HtmlReportWriter.generate(reporter).toString()
-    writeFile(targetDir, reporter, html, HtmlFile)
+    writeFile(targetDir, html, HtmlFile)
   }
 
   def writeXMLReport(targetDir: File, reporter: Feedback): File = {
-    val html = XmlReportWriter.toXML(reporter).toString()
-    writeFile(targetDir, reporter, html, XmlFile)
+    val xml = XmlReportWriter.toXML(reporter).toString()
+    writeFile(targetDir, xml, XmlFile)
   }
 
   def writeScalastyleReport(targetDir: File, reporter: Feedback): File = {
-    val html = ScalastyleReportWriter.toXML(reporter).toString()
-    writeFile(targetDir, reporter, html, ScalastyleXmlFile)
+    val xml = ScalastyleReportWriter.toXML(reporter).toString()
+    writeFile(targetDir, xml, ScalastyleXmlFile)
   }
 
-  private def writeFile(targetDir: File, reporter: Feedback, data: String, fileName: String) = {
+  private def writeFile(targetDir: File, data: String, fileName: String) = {
     targetDir.mkdirs()
     val file = new File(targetDir.getAbsolutePath + "/" + fileName)
     serialize(file, data)

diff --git a/src/main/scala/com/sksamuel/scapegoat/plugin.scala b/src/main/scala/com/sksamuel/scapegoat/plugin.scala
@@ -75,35 +75,33 @@ class ScapegoatPlugin(val global: Global) extends Plugin {
         component.disableScalastyleXML = false
     }
     forProperty("overrideLevels:") foreach {
-      case option =>
+      option =>
         component.feedback.levelOverridesByInspectionSimpleName = option
           .drop("overrideLevels:".length)
           .split(":")
           .map {
-            case nameLevel =>
+            nameLevel =>
               nameLevel.split("=") match {
                 case Array(insp, level) => insp -> Levels.fromName(level)
                 case _ =>
                   throw new IllegalArgumentException(
                     s"Malformed argument to 'overrideLevels': '$nameLevel'. " +
-                    "Expecting 'name=level' where 'name' is the simple name of " +
-                    "an inspection and 'level' is the simple name of a " +
-                    "com.sksamuel.scapegoat.Level constant, e.g. 'Warning'."
+                      "Expecting 'name=level' where 'name' is the simple name of " +
+                      "an inspection and 'level' is the simple name of a " +
+                      "com.sksamuel.scapegoat.Level constant, e.g. 'Warning'."
                   )
               }
           }
           .toMap
     }
     forProperty("sourcePrefix:") match {
-      case Some(option) => {
+      case Some(option) =>
         component.sourcePrefix = option.drop("sourcePrefix:".length)
-      }
       case None => component.sourcePrefix = "src/main/scala/"
     }
     forProperty("minimalLevel:") match {
-      case Some(level) => {
+      case Some(level) =>
         component.minimalLevel = Levels.fromName(level)
-      }
       case None => component.minimalLevel = Levels.Info
     }
     forProperty("dataDir:") match {
@@ -164,7 +162,7 @@ class ScapegoatComponent(val global: Global, inspections: Seq[Inspection])
   var ignoredFiles: List[String] = Nil
   var consoleOutput: Boolean = false
   var verbose: Boolean = false
-  var debug: Boolean = false
+  val debug: Boolean = false
   var summary: Boolean = true
   var disableXML = true
   var disableHTML = true
@@ -248,7 +246,7 @@ class ScapegoatComponent(val global: Global, inspections: Seq[Inspection])
         if (debug) {
           reporter.echo(s"[debug] Scapegoat analysis [$unit] .....")
         }
-        val context = new InspectionContext(global, feedback)
+        val context = InspectionContext(global, feedback)
         activeInspections
           .filter(_.isEnabled)
           .foreach { inspection =>

diff --git a/...t/scala/com/sksamuel/scapegoat/inspections/collections/UnsafeTraversableMethodsTest.scala b/...t/scala/com/sksamuel/scapegoat/inspections/collections/UnsafeTraversableMethodsTest.scala
@@ -28,14 +28,14 @@ class UnsafeTraversableMethodsTest
   )
 
   unsafeMethodUsages.foreach { unsafeMethodUsage =>
-    s"Traversable.${unsafeMethodUsage} use" - {
+    s"Traversable.$unsafeMethodUsage use" - {
       "should report warning" in {
 
         val code = s"""class Test {
-                      Seq(1).${unsafeMethodUsage}
-                      List(1).${unsafeMethodUsage}
-                      Vector(1).${unsafeMethodUsage}
-                      Iterable(1).${unsafeMethodUsage}
+                      Seq(1).$unsafeMethodUsage
+                      List(1).$unsafeMethodUsage
+                      Vector(1).$unsafeMethodUsage
+                      Iterable(1).$unsafeMethodUsage
                     } """.stripMargin
 
         compileCodeSnippet(code)