Merge pull request scalacenter#907 from Jacoby6000/feature/better-regex

Improve regex integration with DisableSyntax

docs/rules/DisableSyntax.md

@@ -31,3 +31,41 @@ println(
 scalafix.website.rule("DisableSyntax", DisableSyntaxConfig.default)
 )
 ```
+
+## Regex
+
+Regex patterns have 3 available ways to be configured.  The example below shows 1 of each way.
+
+```hocon
+DisableSyntax.regex = [
+  {
+    id = offensive
+    pattern = "[Pp]imp"
+    message = "Please consider a less offensive word than ${0} such as Extension"
+  }
+  "Await\\.result"
+  {
+    id = magicNumbers
+    regex = {
+      pattern = "(?:(?:[,(]\\s)|(?:^\\s*))+(\\d+(\\.\\d+)?)"
+      captureGroup = 1
+    }
+    message = "Numbers ({$1} in this instance) should always have a named parameter attached, or be assigned to a val."
+  }
+]
+```
+
+1. The first way has an object providing an `id`, `pattern`, and `message`.
+2. The second way is just the pattern.  When this is used, the `id` is set equal 
+   to the pattern, and a generic message is provided for you.
+3. The third way allows you to specify what capture-group the problematic piece 
+   of code is in, in case your regex is complicated and also matches characters 
+   not useful in an error message.
+
+### Error Messages  
+
+Error messages have access to the capture groups of the regex. To access the 
+capture groups of the regex, use `{$n}` where `n` is the index of the capture 
+group you wish to appear in that part of the message.
+
+You can see this used in the 3rd example.

scalafix-core/src/main/scala/scalafix/config/CustomMessage.scala

@@ -13,6 +13,16 @@ class CustomMessage[T](
 object CustomMessage {
   implicit val SymbolDecoder: ConfDecoder[CustomMessage[Symbol.Global]] =
     decoder[Symbol.Global](field = "symbol")
+
+  implicit def CustomMessageEitherDecoder[A, B](
+      implicit AB: ConfDecoder[Either[CustomMessage[A], CustomMessage[B]]])
+    : ConfDecoder[CustomMessage[Either[A, B]]] =
+    AB.map {
+      case Right(msg) =>
+        new CustomMessage(Right(msg.value), msg.message, msg.id)
+      case Left(msg) => new CustomMessage(Left(msg.value), msg.message, msg.id)
+    }
+
   def decoder[T](field: String)(
       implicit ev: ConfDecoder[T]): ConfDecoder[CustomMessage[T]] =
     ConfDecoder.instance[CustomMessage[T]] {

scalafix-core/src/main/scala/scalafix/config/Regex.scala

@@ -0,0 +1,25 @@
+package scalafix.config
+
+import metaconfig.Conf
+import metaconfig.ConfDecoder
+import java.util.regex.Pattern
+import scalafix.internal.config.ScalafixMetaconfigReaders._
+
+class Regex(
+    val pattern: Pattern,
+    val captureGroup: Option[Int]
+)
+
+object Regex {
+  implicit val regexDecoder: ConfDecoder[Regex] =
+    ConfDecoder.instance[Regex] {
+      case obj: Conf.Obj =>
+        (obj.get[Pattern]("pattern") |@| obj.getOption[Int]("captureGroup"))
+          .map {
+            case (pattern, groupIndex) => new Regex(pattern, groupIndex)
+          }
+    }
+
+  implicit val customMessageRegexDecoder: ConfDecoder[CustomMessage[Regex]] =
+    CustomMessage.decoder[Regex](field = "regex")
+}

scalafix-core/src/main/scala/scalafix/internal/config/ScalafixConfig.scala

@@ -1,6 +1,8 @@
 package scalafix.internal.config
 
 import metaconfig._
+import metaconfig.Configured.Ok
+import metaconfig.Configured.NotOk
 import metaconfig.generic.Surface
 import scala.meta._
 import scala.meta.dialects.Scala212

scalafix-core/src/main/scala/scalafix/internal/config/ScalafixMetaconfigReaders.scala

@@ -9,6 +9,7 @@ import metaconfig.Conf
 import metaconfig.ConfDecoder
 import metaconfig.ConfError
 import metaconfig.Configured
+import metaconfig.Configured.NotOk
 import metaconfig.Configured.Ok
 import scala.meta.Ref
 import scala.meta._
@@ -223,4 +224,23 @@ trait ScalafixMetaconfigReaders {
   implicit lazy val CustomMessagePattern: ConfDecoder[CustomMessage[Pattern]] =
     CustomMessage.decoder(field = "pattern")
 
+  implicit def EitherConfDecoder[A, B](
+      implicit A: ConfDecoder[A],
+      B: ConfDecoder[B]): ConfDecoder[Either[A, B]] = {
+    def wrapLeft(a: A): Either[A, B] = Left(a)
+    def wrapRight(b: B): Either[A, B] = Right(b)
+    ConfDecoder.instance[Either[A, B]] {
+      case conf =>
+        B.map(wrapRight).orElse(A.map(wrapLeft)).read(conf) match {
+          case ok @ Ok(_) => ok
+          case NotOk(err) =>
+            NotOk(
+              ConfError
+                .message(
+                  "Failed to decode configuration for either of the following types:")
+                .combine(err)
+            )
+        }
+    }
+  }
 }

scalafix-rules/src/main/scala/scalafix/internal/rule/DisableSyntax.scala

@@ -1,6 +1,7 @@
 package scalafix.internal.rule
 
 import metaconfig.Configured
+import java.util.regex.Matcher
 import scala.meta._
 import scalafix.v0.LintCategory
 import scalafix.v1._
@@ -20,19 +21,40 @@ final class DisableSyntax(config: DisableSyntaxConfig)
       .map(new DisableSyntax(_))
 
   private def checkRegex(doc: SyntacticDocument): Seq[Diagnostic] = {
-    def pos(offset: Int): Position =
-      Position.Range(doc.input, offset, offset)
+    def pos(matcher: Matcher, groupIndex: Int): Position =
+      if (matcher.group(groupIndex) == null)
+        Position.Range(doc.input, matcher.start, matcher.end)
+      else
+        Position.Range(
+          doc.input,
+          matcher.start(groupIndex),
+          matcher.end(groupIndex))
+
+    def messageSubstitution(matcher: Matcher, message: String): String =
+      (0 to matcher.groupCount).foldLeft(message) {
+        case (msg, idx) =>
+          val groupText = matcher.group(idx)
+          if (groupText != null) msg.replace(s"{$$$idx}", matcher.group(idx))
+          else msg
+      }
+
     val regexDiagnostics = Seq.newBuilder[Diagnostic]
     config.regex.foreach { regex =>
-      val matcher = regex.value.matcher(doc.input.chars)
-      val pattern = regex.value.pattern
+      val (matcher, pattern, groupIndex) = regex.value match {
+        case Right(pat) => (pat.matcher(doc.input.chars), pat.pattern, 0)
+        case Left(reg) =>
+          val pattern = reg.pattern
+          val groupIndex = reg.captureGroup.getOrElse(0)
+          (pattern.matcher(doc.input.chars), pattern.pattern, groupIndex)
+      }
+
       val message = regex.message.getOrElse(s"$pattern is disabled")
       while (matcher.find()) {
         regexDiagnostics +=
           Diagnostic(
             id = regex.id.getOrElse(pattern),
-            message = message,
-            position = pos(matcher.start)
+            message = messageSubstitution(matcher, message),
+            position = pos(matcher, groupIndex)
           )
       }
     }

scalafix-rules/src/main/scala/scalafix/internal/rule/DisableSyntaxConfig.scala

@@ -13,6 +13,7 @@ import metaconfig.annotation.ExampleValue
 import metaconfig.annotation.Hidden
 import pprint.TPrint
 import scalafix.config.CustomMessage
+import scalafix.config.Regex
 
 case class DisableSyntaxConfig(
     @Hidden
@@ -83,11 +84,11 @@ case class DisableSyntaxConfig(
       """|[
          |  {
          |    id = "offensive"
-         |    pattern = "[P|p]imp"
+         |    pattern = "[Pp]imp"
          |    message = "Please consider a less offensive word such as 'extension' or 'enrichment'"
          |  }
          |]""".stripMargin)
-    regex: List[CustomMessage[Pattern]] = Nil
+    regex: List[CustomMessage[Either[Regex, Pattern]]] = Nil
 ) {
 
   def isDisabled(keyword: String): Boolean = keyword match {

scalafix-tests/input/src/main/scala/test/disableSyntax/DisableSyntaxBase.scala

@@ -12,9 +12,17 @@ DisableSyntax.noFinalize = true
 DisableSyntax.regex = [
   {
     id = offensive
-    pattern = "[P|p]imp"
+    pattern = "[Pp]imp"
     message = "Please consider a less offensive word such as Extension"
   }
+  {
+    id = magicNumbers
+    regex = {
+      pattern = "(?:(?:[,(]\\s)|(?:^\\s*))+(\\d+(\\.\\d+)?)"
+      captureGroup = 1
+    }
+    message = "Numbers ({$1} in this instance) should always have a named parameter attached, or be assigned to a val."
+  }
   "Await\\.result"
 ]
 */
@@ -52,8 +60,15 @@ case object DisableSyntaxBase {
     def -(other: String): String = s"$value - $other"
   }
 
+  5 /* assert: DisableSyntax.magicNumbers
+  ^
+Numbers (5 in this instance) should always have a named parameter attached, or be assigned to a val.
+   */
+
+  val fortyTwo = 42
+  val someDays = 75.days
   // actually 7.5 million years
-  Await.result(Future(42), 75.days) // assert: DisableSyntax.Await\.result
+  Await.result(Future(fortyTwo), someDays) // assert: DisableSyntax.Await\.result
 
   override def finalize(): Unit = println("exit") // assert: DisableSyntax.noFinalize