Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve regex integration with DisableSyntax #907

Merged
merged 8 commits into from
Nov 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions docs/rules/DisableSyntax.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
10 changes: 10 additions & 0 deletions scalafix-core/src/main/scala/scalafix/config/CustomMessage.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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]] {
Expand Down
25 changes: 25 additions & 0 deletions scalafix-core/src/main/scala/scalafix/config/Regex.scala
Original file line number Diff line number Diff line change
@@ -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")
}
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -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)
)
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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._
Expand All @@ -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)
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
*/
Expand Down Expand Up @@ -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

Expand Down