Method checkers: report method position, not class

beautiful-scala · Jul 15, 2021 · 6dc457a · 6dc457a
1 parent 2a98b4f
commit 6dc457a
Show file tree

Hide file tree

Showing 10 changed files with 38 additions and 37 deletions.
diff --git a/src/main/scala/org/scalastyle/scalariform/AbstractMethodChecker.scala b/src/main/scala/org/scalastyle/scalariform/AbstractMethodChecker.scala
@@ -101,18 +101,18 @@ abstract class AbstractMethodChecker extends ScalariformChecker {
     val it = for {
       t <- localvisit(ast.immediateChildren.head)
       f <- traverse(t)
-      if matches(f)
-    } yield PositionError(f.position.get, params(f))
+      p <- matches(f)
+    } yield PositionError(p, params(f))
 
-    it.toList
+    it
   }
 
   private def traverse(t: BaseClazz[AstNode]): ListType = {
     val l = t.subs.flatMap(traverse)
-    if (matches(t)) t :: l else l
+    if (matches(t).isDefined) t :: l else l
   }
 
-  def matches(t: BaseClazz[AstNode]): Boolean
+  def matches(t: BaseClazz[AstNode]): Option[Int]
 
   protected def getParams(p: ParamClauses): List[Param] =
     p.paramClausesAndNewlines
@@ -132,10 +132,8 @@ abstract class AbstractMethodChecker extends ScalariformChecker {
   protected def getParamTypes(pc: ParamClauses): List[String] =
     getParams(pc).map(p => typename(p.paramTypeOpt.get._2))
 
-  protected def matchFunDefOrDcl(t: BaseClazz[AstNode], fn: FunDefOrDcl => Boolean): Boolean =
-    t match {
-      case f: FunDefOrDclClazz => fn(f.t); case _ => false
-    }
+  protected def matchFunDefOrDcl(t: BaseClazz[AstNode], fn: FunDefOrDcl => Boolean): Option[Int] =
+    t.subs.collectFirst { case f: FunDefOrDclClazz if fn(f.t) => f.t.nameToken.offset }
 
   protected def methodMatch(name: String, paramTypesMatch: List[String] => Boolean)(t: FunDefOrDcl): Boolean =
     t.nameToken.text == name && paramTypesMatch(getParamTypes(t.paramClauses))

diff --git a/src/main/scala/org/scalastyle/scalariform/CovariantEqualsChecker.scala b/src/main/scala/org/scalastyle/scalariform/CovariantEqualsChecker.scala
@@ -17,17 +17,16 @@
 package org.scalastyle.scalariform
 
 import org.scalastyle.Checker
+
 import scalariform.parser.AstNode
 import scalariform.parser.FunDefOrDcl
 
 class CovariantEqualsChecker extends AbstractMethodChecker {
   val errorKey = "covariant.equals"
 
-  def matches(t: BaseClazz[AstNode]): Boolean = {
-    val equalsObject = t.subs.exists(matchFunDefOrDcl(_, isEqualsObject))
-    val equalsOther = t.subs.exists(matchFunDefOrDcl(_, isEqualsOther))
-
-    !equalsObject && equalsOther
+  def matches(t: BaseClazz[AstNode]): Option[Int] = {
+    val equalsObject = matchFunDefOrDcl(t, isEqualsObject).isEmpty
+    if (equalsObject) matchFunDefOrDcl(t, isEqualsOther) else None
   }
 
   private def isEqualsOther(t: FunDefOrDcl): Boolean =

diff --git a/src/main/scala/org/scalastyle/scalariform/EqualsHashCodeChecker.scala b/src/main/scala/org/scalastyle/scalariform/EqualsHashCodeChecker.scala
@@ -22,11 +22,15 @@ import scalariform.parser.FunDefOrDcl
 class EqualsHashCodeChecker extends AbstractMethodChecker {
   val errorKey = "equals.hash.code"
 
-  def matches(t: BaseClazz[AstNode]): Boolean = {
-    val hc = t.subs.exists(matchFunDefOrDcl(_, isHashCode))
-    val eq = t.subs.exists(matchFunDefOrDcl(_, isEqualsObject))
+  def matches(t: BaseClazz[AstNode]): Option[Int] = {
+    val hc = matchFunDefOrDcl(t, isHashCode)
+    val eq = matchFunDefOrDcl(t, isEqualsObject)
 
-    (hc && !eq) || (!hc && eq)
+    if (hc.isDefined) {
+      if (eq.isDefined) None else hc
+    } else {
+      if (eq.isDefined) eq else None
+    }
   }
 
   private def isHashCode(t: FunDefOrDcl): Boolean = methodMatch("hashCode", noParameter() _)(t)

diff --git a/src/main/scala/org/scalastyle/scalariform/NoCloneChecker.scala b/src/main/scala/org/scalastyle/scalariform/NoCloneChecker.scala
@@ -22,8 +22,8 @@ import scalariform.parser.FunDefOrDcl
 class NoCloneChecker extends AbstractMethodChecker {
   val errorKey = "no.clone"
 
-  def matches(t: BaseClazz[AstNode]): Boolean =
-    t.subs.exists(matchFunDefOrDcl(_, isClone))
+  def matches(t: BaseClazz[AstNode]): Option[Int] =
+    matchFunDefOrDcl(t, isClone)
 
   private def isClone(t: FunDefOrDcl): Boolean = methodMatch("clone", noParameter() _)(t)
 }
diff --git a/src/main/scala/org/scalastyle/scalariform/NoFinalizeChecker.scala b/src/main/scala/org/scalastyle/scalariform/NoFinalizeChecker.scala
@@ -22,8 +22,8 @@ import scalariform.parser.FunDefOrDcl
 class NoFinalizeChecker extends AbstractMethodChecker {
   val errorKey = "no.finalize"
 
-  def matches(t: BaseClazz[AstNode]): Boolean =
-    t.subs.exists(matchFunDefOrDcl(_, isFinalize))
+  def matches(t: BaseClazz[AstNode]): Option[Int] =
+    matchFunDefOrDcl(t, isFinalize)
 
   private def isFinalize(t: FunDefOrDcl): Boolean = methodMatch("finalize", noParameter() _)(t)
 }
diff --git a/src/main/scala/org/scalastyle/scalariform/NumberOfMethodsInTypeChecker.scala b/src/main/scala/org/scalastyle/scalariform/NumberOfMethodsInTypeChecker.scala
@@ -25,8 +25,8 @@ class NumberOfMethodsInTypeChecker extends AbstractMethodChecker {
   override def params(t: BaseClazz[AstNode]): List[String] =
     List("" + getInt("maxMethods", DefaultMaxMethods))
 
-  def matches(t: BaseClazz[AstNode]): Boolean = {
+  def matches(t: BaseClazz[AstNode]): Option[Int] = {
     val maxMethods = getInt("maxMethods", DefaultMaxMethods)
-    t.subs.size > maxMethods
+    if (t.subs.size > maxMethods) t.position else None
   }
 }
diff --git a/src/test/scala/org/scalastyle/scalariform/CovariantEqualsCheckerTest.scala b/src/test/scala/org/scalastyle/scalariform/CovariantEqualsCheckerTest.scala
@@ -48,7 +48,7 @@ class CovariantEqualsNoObjectKO {
 }
 """
 
-    assertErrors(List(columnError(4, 6)), source)
+    assertErrors(List(columnError(5, 6)), source)
   }
 
   @Test def testObjectOK(): Unit = {
@@ -73,6 +73,6 @@ object CovariantEqualsNoObjectKO {
 }
 """
 
-    assertErrors(List(columnError(4, 7)), source)
+    assertErrors(List(columnError(5, 6)), source)
   }
 }
diff --git a/src/test/scala/org/scalastyle/scalariform/EqualsHashCodeCheckerTest.scala b/src/test/scala/org/scalastyle/scalariform/EqualsHashCodeCheckerTest.scala
@@ -48,7 +48,7 @@ class HashCodeOnlyKO {
 }
 """
 
-    assertErrors(List(columnError(4, 6)), source)
+    assertErrors(List(columnError(5, 6)), source)
   }
 
   @Test def testEqualsOnlyKO(): Unit = {
@@ -60,7 +60,7 @@ class EqualsOnlyKO {
 }
 """
 
-    assertErrors(List(columnError(4, 6)), source)
+    assertErrors(List(columnError(5, 6)), source)
   }
 
   @Test def testEqualsOnlyAnyKO(): Unit = {
@@ -72,7 +72,7 @@ class EqualsOnlyKO {
 }
 """
 
-    assertErrors(List(columnError(4, 6)), source)
+    assertErrors(List(columnError(5, 6)), source)
   }
 
   @Test def testEqualsWrongSignatureOK(): Unit = {
@@ -112,7 +112,7 @@ class OuterKO {
 }
 """
 
-    assertErrors(List(columnError(4, 6), columnError(6, 8)), source)
+    assertErrors(List(columnError(5, 6), columnError(7, 8)), source)
   }
 
   @Test def testOuterOKInnerKO(): Unit = {
@@ -128,7 +128,7 @@ class OuterOK {
 }
 """
 
-    assertErrors(List(columnError(6, 8)), source)
+    assertErrors(List(columnError(7, 8)), source)
   }
 
   @Test def testObjectInnerKO(): Unit = {
@@ -142,7 +142,7 @@ object Object {
 }
 """
 
-    assertErrors(List(columnError(5, 8)), source)
+    assertErrors(List(columnError(6, 8)), source)
   }
 
   @Test def testMultipleClasses(): Unit = {
@@ -158,6 +158,6 @@ class Class2 {
 }
 """
 
-    assertErrors(List(columnError(4, 6), columnError(8, 6)), source)
+    assertErrors(List(columnError(5, 6), columnError(9, 6)), source)
   }
 }
diff --git a/src/test/scala/org/scalastyle/scalariform/NoCloneCheckerTest.scala b/src/test/scala/org/scalastyle/scalariform/NoCloneCheckerTest.scala
@@ -47,7 +47,7 @@ class CloneKO {
 }
 """
 
-    assertErrors(List(columnError(4, 6)), source)
+    assertErrors(List(columnError(5, 6)), source)
   }
 
   @Test def testObjectOK(): Unit = {
@@ -71,6 +71,6 @@ object CloneKO {
 }
 """
 
-    assertErrors(List(columnError(4, 7)), source)
+    assertErrors(List(columnError(5, 6)), source)
   }
 }
diff --git a/src/test/scala/org/scalastyle/scalariform/NoFinalizeCheckerTest.scala b/src/test/scala/org/scalastyle/scalariform/NoFinalizeCheckerTest.scala
@@ -47,7 +47,7 @@ class CloneKO {
 }
 """
 
-    assertErrors(List(columnError(4, 6)), source)
+    assertErrors(List(columnError(5, 6)), source)
   }
 
   @Test def testObjectOK(): Unit = {
@@ -71,6 +71,6 @@ object CloneKO {
 }
 """
 
-    assertErrors(List(columnError(4, 7)), source)
+    assertErrors(List(columnError(5, 6)), source)
   }
 }