Checkers: report the failing token and offset

Author: Albert Meltzer

src/main/scala/org/scalastyle/scalariform/CyclomaticComplexityChecker.scala

@@ -49,7 +49,7 @@ class CyclomaticComplexityChecker extends CombinedChecker {
       f <- traverse(t)
       value = matches(f, ast.lines, maximum)
       if value > maximum
-    } yield PositionError(t.position.get, List("" + value, "" + maximum))
+    } yield PositionError(f.position.get, List("" + value, "" + maximum))
 
     it
   }

src/main/scala/org/scalastyle/scalariform/EmptyInterpolatedStringChecker.scala

@@ -30,7 +30,7 @@ class EmptyInterpolatedStringChecker extends ScalariformChecker {
       List(left, right) <- ast.tokens.sliding(2)
       if left.tokenType == INTERPOLATION_ID && typesSupportingVariables.contains(left.text) &&
       interpolationRegex.findFirstIn(right.text).isEmpty
-    } yield PositionError(right.offset)
+    } yield PositionError(left.offset)
 
     it.toList
   }

src/main/scala/org/scalastyle/scalariform/ImportsChecker.scala

@@ -213,13 +213,13 @@ class ImportOrderChecker extends ScalariformChecker {
     val CompilationUnit(statements, _) = ast
     statements.immediateChildren.flatMap { n =>
       val result = n match {
-        case ImportClause(_, BlockImportExpr(prefix, selectors), _, _) =>
+        case ImportClause(_, e @ BlockImportExpr(prefix, selectors), _, _) =>
           val text = exprToText(prefix.contents, if (lexicographic) selectors.tokens else Nil)
-          checkImport(text, n.firstToken.offset) ++ checkSelectors(selectors)
+          checkImport(text, e.firstToken.offset) ++ checkSelectors(selectors)
 
-        case ImportClause(_, Expr(contents), _, _) =>
+        case ImportClause(_, e @ Expr(contents), _, _) =>
           val text = exprToText(contents)
-          checkImport(text, n.firstToken.offset)
+          checkImport(text, e.firstToken.offset)
 
         case _ =>
           Nil
@@ -281,13 +281,13 @@ class ImportOrderChecker extends ScalariformChecker {
     val ImportSelectors(_, first, others, _) = selectors
 
     val errors = new ListBuffer[ScalastyleError]()
-    val names = Seq(first.contents.head.tokens.head.text) ++
-      others.map(_._2.contents.head.tokens.head.text)
+    val names = Seq(first.contents.head.tokens.head) ++
+      others.map(_._2.contents.head.tokens.head)
 
     if (names.size > 1) {
       names.sliding(2).foreach { case Seq(left, right) =>
-        if (compareNames(left, right, isImport = false) > 0)
-          errors += newError(selectors.firstToken.offset, "wrongOrderInSelector", right, left)
+        if (compareNames(left.text, right.text, isImport = false) > 0)
+          errors += newError(left.offset, "wrongOrderInSelector", right.text, left.text)
       }
     }
 

src/main/scala/org/scalastyle/scalariform/NoWhitespaceBracketChecker.scala

@@ -30,7 +30,7 @@ class NoWhitespaceBeforeLeftBracketChecker extends ScalariformChecker {
     val it = for {
       List(left, right) <- ast.tokens.sliding(2)
       if right.tokenType == LBRACKET && charsBetweenTokens(left, right) > 0
-    } yield PositionError(left.offset)
+    } yield PositionError(right.offset)
 
     it.toList
   }

src/main/scala/org/scalastyle/scalariform/NonASCIICharacterChecker.scala

@@ -18,7 +18,7 @@ package org.scalastyle.scalariform
 
 import java.util.regex.Pattern
 
-import _root_.scalariform.lexer.{Token, Tokens}
+import _root_.scalariform.lexer.{Comment, Token, Tokens}
 import _root_.scalariform.parser.CompilationUnit
 import org.scalastyle.PositionError
 import org.scalastyle.ScalariformChecker
@@ -35,7 +35,12 @@ class NonASCIICharacterChecker extends ScalariformChecker {
     getBoolean("allowStringLiterals", defaultAllowStringLiterals)
 
   override def verify(ast: CompilationUnit): List[ScalastyleError] =
-    ast.tokens.filter(hasNonAsciiChars).map(x => PositionError(x.offset))
+    ast.tokens.flatMap(getTokenNonAsciiChars(_).map(PositionError(_)))
+
+  private def getTokenNonAsciiChars(x: Token): Option[Int] =
+    x.associatedWhitespaceAndComments.tokens
+      .collectFirst { case c: Comment if hasNonAsciiChars(c.token) => c.token.offset }
+      .orElse(if (hasNonAsciiChars(x)) Some(x.offset) else None)
 
   private def hasNonAsciiChars(x: Token) = {
     x.rawText.trim.nonEmpty &&

src/main/scala/org/scalastyle/scalariform/ScalaDocChecker.scala

@@ -35,6 +35,7 @@ import _root_.scalariform.parser.TypeDefOrDcl
 import _root_.scalariform.parser.TypeParamClause
 import org.scalastyle.CombinedAst
 import org.scalastyle.CombinedChecker
+import org.scalastyle.LineColumn
 import org.scalastyle.LineError
 import org.scalastyle.Lines
 import org.scalastyle.ScalastyleError
@@ -106,28 +107,34 @@ class ScalaDocChecker extends CombinedChecker {
    * ``FullDefOrDcl`` -> ``PatDefOrDcl``, with the ScalaDoc attached to the ``FulDefOrDcl``, which
    * finds its way to us here in ``fallback``.
    */
-  private def findScalaDoc(token: Token, lines: Lines, fallback: HiddenTokens): Option[ScalaDoc] = {
+  private def findScalaDoc(
+    token: Token,
+    lines: Lines,
+    fallback: HiddenTokens
+  )(f: ScalaDoc => List[ScalastyleError]): List[ScalastyleError] = {
     def toScalaDoc(ht: HiddenTokens): Option[ScalaDoc] =
       ht.rawTokens
         .find(_.isScalaDocComment)
         .map { commentToken =>
-          val commentOffset = lines.toLineColumn(commentToken.offset).map(_.column).getOrElse(0)
-          ScalaDoc.apply(commentToken, commentOffset)
+          val lineColumn = lines.toLineColumn(commentToken.offset).getOrElse(LineColumn(0, 0))
+          ScalaDoc(commentToken.rawText, lineColumn)
         }
 
-    toScalaDoc(token.associatedWhitespaceAndComments).orElse(toScalaDoc(fallback))
+    toScalaDoc(token.associatedWhitespaceAndComments).orElse(toScalaDoc(fallback)).map(f).getOrElse {
+      List(LineError(lines.toLine(token.offset).get, List(Missing)))
+    }
   }
 
-  private def indentErrors(line: Int, style: DocIndentStyle)(scalaDoc: ScalaDoc): List[ScalastyleError] =
+  private def indentErrors(style: DocIndentStyle)(scalaDoc: ScalaDoc): List[ScalastyleError] =
     if (style == AnyDocStyle || style == scalaDoc.indentStyle)
       Nil
     else if (SingleLineStyle == scalaDoc.indentStyle)
       Nil
     else
-      List(LineError(line, List(InvalidDocStyle)))
+      List(LineError(scalaDoc.line, List(InvalidDocStyle)))
 
   // parse the parameters and report errors for the parameters (constructor or method)
-  private def paramErrors(line: Int, paramClausesOpt: Option[ParamClauses])(
+  private def paramErrors(paramClausesOpt: Option[ParamClauses])(
     scalaDoc: ScalaDoc
   ): List[ScalastyleError] = {
     def params(xs: List[Token]): List[String] =
@@ -150,18 +157,16 @@ class ScalaDocChecker extends CombinedChecker {
     val extraScalaDocParams = scalaDoc.params.filterNot(param => paramNames.contains(param.name))
     val validScalaDocParams = scalaDoc.params.filter(param => paramNames.contains(param.name))
 
+    val line = scalaDoc.line
     missingScalaDocParams.map(missing => LineError(line, List(missingParam(missing)))) ++
       extraScalaDocParams.map(extra => LineError(line, List(extraParam(extra.name)))) ++
       validScalaDocParams.filter(_.text.isEmpty).map(empty => LineError(line, List(emptyParam(empty.name))))
-
-//      if (!scalaDoc.params.forall(p => paramNames.exists(name => p.name == name && !p.text.isEmpty))) List(LineError(line, List(MalformedParams)))
-//      else Nil
   }
 
   // parse the type parameters and report errors for the parameters (constructor or method)
   // scalastyle:off cyclomatic.complexity
 
-  private def tparamErrors(line: Int, tparamClausesOpt: Option[TypeParamClause])(
+  private def tparamErrors(tparamClausesOpt: Option[TypeParamClause])(
     scalaDoc: ScalaDoc
   ): List[ScalastyleError] = {
     def tparams(xs: List[Token], bracketDepth: Int): List[String] =
@@ -203,6 +208,7 @@ class ScalaDocChecker extends CombinedChecker {
 
     val tparamNames = tparamClausesOpt.map(tc => tparams(tc.tokens, 0)).getOrElse(Nil)
 
+    val line = scalaDoc.line
     if (tparamNames.size != scalaDoc.typeParams.size)
       // bad param sizes
       List(LineError(line, List(MalformedTypeParams)))
@@ -216,13 +222,13 @@ class ScalaDocChecker extends CombinedChecker {
   // scalastyle:on cyclomatic.complexity
 
   // parse the parameters and report errors for the return types
-  private def returnErrors(line: Int, returnTypeOpt: Option[(Token, Type)])(
+  private def returnErrors(returnTypeOpt: Option[(Token, Type)])(
     scalaDoc: ScalaDoc
   ): List[ScalastyleError] = {
     val needsReturn = returnTypeOpt.exists { case (_, tpe) => tpe.firstToken.text != "Unit" }
 
     if (needsReturn && scalaDoc.returns.isEmpty)
-      List(LineError(line, List(MalformedReturn)))
+      List(LineError(scalaDoc.line, List(MalformedReturn)))
     else
       Nil
   }
@@ -286,20 +292,17 @@ class ScalaDocChecker extends CombinedChecker {
         // class Foo, class Foo[A](a: A);
         // case class Foo(), case class Foo[A](a: A);
         // object Foo;
-        val (_, line) = lines.findLineAndIndex(t.firstToken.offset).get
 
         // we are checking parameters and type parameters
         val errors =
           if (shouldSkip(t))
             Nil
           else {
-            findScalaDoc(t.firstToken, lines, fallback)
-              .map { scalaDoc =>
-                paramErrors(line, t.paramClausesOpt)(scalaDoc) ++
-                  tparamErrors(line, t.typeParamClauseOpt)(scalaDoc) ++
-                  indentErrors(line, indentStyle)(scalaDoc)
-              }
-              .getOrElse(List(LineError(line, List(Missing))))
+            findScalaDoc(t.firstToken, lines, fallback) { scalaDoc =>
+              paramErrors(t.paramClausesOpt)(scalaDoc) ++
+                tparamErrors(t.typeParamClauseOpt)(scalaDoc) ++
+                indentErrors(indentStyle)(scalaDoc)
+            }
           }
 
         // and we descend, because we're interested in seeing members of the types
@@ -309,50 +312,43 @@ class ScalaDocChecker extends CombinedChecker {
         )
       case t: FunDefOrDcl =>
         // def foo[A, B](a: Int): B = ...
-        val (_, line) = lines.findLineAndIndex(t.firstToken.offset).get
-
         // we are checking parameters, type parameters and returns
         val errors =
           if (shouldSkip(t))
             Nil
           else {
-            findScalaDoc(t.firstToken, lines, fallback)
-              .map { scalaDoc =>
-                paramErrors(line, Some(t.paramClauses))(scalaDoc) ++
-                  tparamErrors(line, t.typeParamClauseOpt)(scalaDoc) ++
-                  returnErrors(line, t.returnTypeOpt)(scalaDoc) ++
-                  indentErrors(line, indentStyle)(scalaDoc)
-              }
-              .getOrElse(List(LineError(line, List(Missing))))
+            findScalaDoc(t.firstToken, lines, fallback) { scalaDoc =>
+              paramErrors(Some(t.paramClauses))(scalaDoc) ++
+                tparamErrors(t.typeParamClauseOpt)(scalaDoc) ++
+                returnErrors(t.returnTypeOpt)(scalaDoc) ++
+                indentErrors(indentStyle)(scalaDoc)
+            }
           }
 
         // we don't descend any further
         errors
       case t: TypeDefOrDcl =>
         // type Foo = ...
-        val (_, line) = lines.findLineAndIndex(t.firstToken.offset).get
-
         // no params here
         val errors =
           if (shouldSkip(t)) Nil
           else
-            findScalaDoc(t.firstToken, lines, fallback)
-              .map(scalaDoc => indentErrors(line, indentStyle)(scalaDoc))
-              .getOrElse(List(LineError(line, List(Missing))))
+            findScalaDoc(t.firstToken, lines, fallback) { scalaDoc =>
+              indentErrors(indentStyle)(scalaDoc)
+            }
 
         // we don't descend any further
         errors
 
       case t: PatDefOrDcl =>
         // val a = ..., var a = ...
-        val (_, line) = lines.findLineAndIndex(t.valOrVarToken.offset).get
         val errors =
           if (shouldSkip(t))
             Nil
           else {
-            findScalaDoc(t.firstToken, lines, fallback)
-              .map(scalaDoc => indentErrors(line, indentStyle)(scalaDoc))
-              .getOrElse(List(LineError(line, List(Missing))))
+            findScalaDoc(t.firstToken, lines, fallback) { scalaDoc =>
+              indentErrors(indentStyle)(scalaDoc)
+            }
           }
 
         // we don't descend any further
@@ -440,13 +436,14 @@ object ScalaDocChecker {
 
     /**
      * Take the ``raw`` and parse an instance of ``ScalaDoc``
-     * @param raw the token containing the ScalaDoc
-     * @param offset column number of scaladoc's first string
+     * @param text the ScalaDoc text
+     * @param lineColumn line and column number of scaladoc's first string
      * @return the parsed instance
      */
     // scalastyle:off cyclomatic.complexity
-    def apply(raw: Token, offset: Int): ScalaDoc = {
-      val strings = raw.rawText.split("\\n").toList
+    def apply(raw: String, lineColumn: LineColumn): ScalaDoc = {
+      val offset = lineColumn.column
+      val strings = raw.split("\\n").toList
 
       val indentStyle = {
         def getStyle(xs: List[String], style: DocIndentStyle): DocIndentStyle =
@@ -494,7 +491,7 @@ object ScalaDocChecker {
       val typeParams = combineScalaDocFor(lines, "tparam", ScalaDocParameter)
       val returns = combineScalaDocFor(lines, "return", _ + _).headOption
 
-      ScalaDoc(raw.rawText, params, typeParams, returns, None, indentStyle)
+      ScalaDoc(lineColumn.line, raw, params, typeParams, returns, None, indentStyle)
     }
     // scalastyle:on cyclomatic.complexity
   }
@@ -508,6 +505,7 @@ object ScalaDocChecker {
 
   /**
    * Models the parsed ScalaDoc
+   * @param line scaladoc line
    * @param text arbitrary text
    * @param params the parameters
    * @param typeParams the type parameters
@@ -516,6 +514,7 @@ object ScalaDocChecker {
    * @param indentStyle doc indent style
    */
   private case class ScalaDoc(
+    line: Int,
     text: String,
     params: List[ScalaDocParameter],
     typeParams: List[ScalaDocParameter],

src/main/scala/org/scalastyle/scalariform/SimplifyBooleanExpressionChecker.scala

@@ -38,20 +38,20 @@ class SimplifyBooleanExpressionChecker extends ScalariformChecker {
     val it1 = for {
       List(left, right) <- ast.tokens.sliding(2);
       if (left.text == "!" && isBoolean(right))
-    } yield PositionError(left.offset)
+    } yield PositionError(right.offset)
 
     val it2 = for {
       t <- localvisit(ast);
-      if (matches(t))
-    } yield PositionError(t.position.get)
+      p <- matches(t)
+    } yield PositionError(p)
 
     (it1.toList ::: it2.toList).sortWith((a, b) => a.position < b.position)
   }
 
-  private def matches[T <: AstNode](t: Clazz[T]): Boolean = {
+  private def matches[T <: AstNode](t: Clazz[T]): Option[Int] = {
     t match {
-      case t: InfixExprClazz => matchesInfixOp(t.id) && (boolean(t.left) || boolean(t.right))
-      case _                 => false
+      case t: InfixExprClazz if matchesInfixOp(t.id) => boolean(t.left).orElse(boolean(t.right))
+      case _                                         => None
     }
   }
 
@@ -74,9 +74,10 @@ class SimplifyBooleanExpressionChecker extends ScalariformChecker {
       case t: Any           => visit(t, localvisit)
     }
 
-  private def boolean(expr: List[Clazz[_]]) =
-    expr.size == 1 && expr(0)
-      .isInstanceOf[GeneralTokensClazz] && expr(0).asInstanceOf[GeneralTokensClazz].bool
+  private def boolean(expr: List[Clazz[_]]) = expr match {
+    case List(t: GeneralTokensClazz) if t.bool => t.position
+    case _                                     => None
+  }
 
   private def isBoolean(t: GeneralTokens): Boolean = t.tokens.size == 1 && isBoolean(t.tokens(0))
   private def isBoolean(t: Token): Boolean = Set(TRUE, FALSE).contains(t.tokenType)

src/main/scala/org/scalastyle/scalariform/TodoCommentChecker.scala

@@ -34,13 +34,13 @@ class TodoCommentChecker extends CombinedChecker {
   def verify(ast: CombinedAst): List[ScalastyleError] = {
     val words = getString("words", defaultWords)
     val split = words.split("\\|").map(Pattern.quote).mkString("|")
-    val regex = ("""(?i)(//|/\*|/\*\*|\*)\s?(""" + split + """)(:?)\s+""").r
+    val regex = ("""(?i)(?:^//|^/\*|^/\*\*|\*)\s?(""" + split + """):?\s+""").r.unanchored
 
     for {
       t  <- ast.compilationUnit.tokens
       at <- t.associatedWhitespaceAndComments
       if Tokens.COMMENTS.contains(at.token.tokenType)
-      if at.text.split("\n").exists(s => regex.findFirstIn(s).isDefined)
-    } yield PositionError(at.token.offset, List(words))
+      tag <- at.text.split("\n").view.collectFirst { case regex(tag) => tag }
+    } yield PositionError(at.token.offset, List(tag))
   }
 }

src/test/scala/org/scalastyle/scalariform/CyclomaticComplexityCheckerTest.scala

@@ -119,7 +119,7 @@ class Foobar {
 """
 
     assertErrors(
-      List(columnError(5, 6, List("4", "3")), columnError(21, 6, List("4", "3"))),
+      List(columnError(5, 6, List("4", "3")), columnError(23, 8, List("4", "3"))),
       source,
       Map("maximum" -> "3")
     )