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

Add support to disable symbol via regex #669

Merged
merged 24 commits into from
Apr 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,89 @@ package scalafix
package internal.config

import metaconfig._
import metaconfig.annotation.{Description, ExampleValue}
import metaconfig.generic.Surface
import org.langmeta.Symbol

import scalafix.internal.util.SymbolOps
import metaconfig.annotation.{Description, ExampleValue}
import metaconfig.generic.Surface

case class DisabledSymbol(
@ExampleValue("scala.sys.process.Process")
@Description(
"A single symbol to ban. Cannot be used together with the regex option.")
symbol: Option[Symbol.Global],
@Description("Custom message.")
message: Option[String],
@Description("Custom id for error messages.")
id: Option[String],
@Description(
"Regex to ban symbols matching a given include/exclude patterns." +
"Cannot be used together with the symbol option." +
"Include and exclude filters can be either a list of regex strings or a single regex string.")
@ExampleValue("""
|{
| includes = [
| "java.io.*"
| "scala.io.*"
| ]
| excludes = "java.io.InputStream"
|}""".stripMargin)
regex: Option[FilterMatcher]) {

def matches(symbol: Symbol): Boolean = {
this.symbol match {
case Some(s) => SymbolOps.isSameNormalized(symbol, s)
case None =>
regex match {
case Some(r) => r.matches(symbol.toString)
case None => sys.error("impossible")
}
}
}
}

object DisabledSymbol {
implicit val surface: Surface[DisabledSymbol] =
generic.deriveSurface[DisabledSymbol]

private def normalizeMessage(msg: String): String =
if (msg.isMultiline) {
"\n" + msg.stripMargin
} else {
msg
}
implicit val reader: ConfDecoder[DisabledSymbol] =
ConfDecoder.instance[DisabledSymbol] {
case c: Conf.Obj =>
(c.getOption[Symbol.Global]("symbol") |@|
c.getOption[String]("message") |@|
c.getOption[String]("id") |@|
c.getOption[FilterMatcher]("regex"))
.andThen {
case (((Some(_), b), c), Some(_)) =>
Configured.notOk(ConfError.message(
"Cannot specify both symbol and regex, only one of them is allowed."))
case (((a @ Some(_), b), c), None) =>
Configured.ok(DisabledSymbol(a, b.map(normalizeMessage), c, None))
case (((None, b), c), d @ Some(_)) =>
Configured.ok(DisabledSymbol(None, b.map(normalizeMessage), c, d))
case (((None, b), c), None) =>
Configured.notOk(
ConfError.message("Either symbol or regex must be specified."))
}
case s: Conf.Str =>
symbolGlobalReader
.read(s)
.map(sym => DisabledSymbol(Some(sym), None, None, None))
}
}

case class UnlessInsideBlock(
@Description("The symbol that indicates a 'safe' block.")
safeBlock: Symbol.Global,
safeBlock: DisabledSymbol,
@Description(
"The unsafe symbols that are banned unless inside a 'safe' block")
symbols: List[CustomMessage[Symbol.Global]])
symbols: List[DisabledSymbol])

object UnlessInsideBlock {
implicit val surface: Surface[UnlessInsideBlock] =
Expand All @@ -23,8 +94,8 @@ object UnlessInsideBlock {
implicit val reader: ConfDecoder[UnlessInsideBlock] =
ConfDecoder.instanceF[UnlessInsideBlock] {
case c: Conf.Obj =>
(c.get[Symbol.Global]("safeBlock") |@|
c.get[List[CustomMessage[Symbol.Global]]]("symbols")).map {
(c.get[DisabledSymbol]("safeBlock") |@|
c.get[List[DisabledSymbol]]("symbols")).map {
case (a, b) => UnlessInsideBlock(a, b)
}
case _ => Configured.NotOk(ConfError.message("Wrong config format"))
Expand All @@ -42,7 +113,7 @@ case class DisableConfig(
| message = "use pattern-matching instead"
| }
|]""".stripMargin)
symbols: List[CustomMessage[Symbol.Global]] = Nil,
symbols: List[DisabledSymbol] = Nil,
@Description(
"List of symbols to disable if inferred. Does not report an error for symbols written explicitly in source code.")
@ExampleValue("""
Expand All @@ -52,7 +123,7 @@ case class DisableConfig(
| message = "use explicit toString be calling +"
| }
|]""".stripMargin)
ifSynthetic: List[CustomMessage[Symbol.Global]] = Nil,
ifSynthetic: List[DisabledSymbol] = Nil,
@Description(
"List of symbols to disable unless they are in the given block.")
@ExampleValue("""
Expand All @@ -70,38 +141,10 @@ case class DisableConfig(
""".stripMargin)
unlessInside: List[UnlessInsideBlock] = Nil
) {
private def normalizeSymbol(symbol: Symbol.Global): String =
SymbolOps.normalize(symbol).syntax

lazy val allUnlessBlocks: List[Symbol.Global] =
lazy val allSafeBlocks: List[DisabledSymbol] =
unlessInside.map(_.safeBlock)
private lazy val allCustomMessages: List[CustomMessage[Symbol.Global]] =
lazy val allDisabledSymbols: List[DisabledSymbol] =
symbols ++ ifSynthetic ++ unlessInside.flatMap(_.symbols)
lazy val allSymbols: List[Symbol.Global] = allCustomMessages.map(_.value)
lazy val allSymbolsInSynthetics: List[Symbol.Global] =
ifSynthetic.map(_.value)

private val syntheticSymbols: Set[String] =
ifSynthetic.map(u => normalizeSymbol(u.value)).toSet

private val messageBySymbol: Map[String, CustomMessage[Symbol.Global]] =
allCustomMessages.map(m => normalizeSymbol(m.value) -> m).toMap

private val symbolsByUnless: Map[String, List[Symbol.Global]] =
unlessInside
.groupBy(u => normalizeSymbol(u.safeBlock))
.mapValues(_.flatMap(_.symbols.map(_.value)))

def symbolsInSafeBlock(unless: Symbol.Global): List[Symbol.Global] =
symbolsByUnless.getOrElse(normalizeSymbol(unless), List.empty)

def customMessage(
symbol: Symbol.Global): Option[CustomMessage[Symbol.Global]] =
messageBySymbol.get(normalizeSymbol(symbol))

def isSynthetic(symbol: Symbol.Global): Boolean = {
syntheticSymbols.contains(normalizeSymbol(symbol))
}
}

object DisableConfig {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ case class FilterMatcher(
}
case c =>
(
c.getOrElse("include")(includeFilters) |@|
c.getOrElse("exclude")(excludeFilters)
c.getOrElse("include", "includes")(includeFilters) |@|
c.getOrElse("exclude", "excludes")(excludeFilters)
).map { case (a, b) => FilterMatcher(a, b) }
}
def matches(file: AbsolutePath): Boolean = matches(file.toString())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import metaconfig.{Conf, Configured}

import scala.meta._
import scala.meta.transversers.Traverser
import scalafix.config.CustomMessage
import scalafix.internal.config.DisableConfig
import scalafix.internal.config.{DisableConfig, DisabledSymbol}
import scalafix.internal.util.SymbolOps
import scalafix.lint.{LintCategory, LintMessage}
import scalafix.rule.{Rule, RuleCtx, SemanticRule}
import scalafix.util.{SemanticdbIndex, SymbolMatcher}
import scalafix.util.SemanticdbIndex

object Disable {

Expand Down Expand Up @@ -46,6 +46,20 @@ object Disable {
buf.toList
}
}

final class DisableSymbolMatcher(symbols: List[DisabledSymbol])(
implicit index: SemanticdbIndex) {
def findMatch(symbol: Symbol): Option[DisabledSymbol] =
symbols.find(_.matches(symbol))

def unapply(tree: Tree): Option[(Tree, DisabledSymbol)] =
index
.symbol(tree)
.flatMap(findMatch(_).map(ds => (tree, ds)))

def unapply(symbol: Symbol): Option[(Symbol, DisabledSymbol)] =
findMatch(symbol).map(ds => (symbol, ds))
}
}

final case class Disable(index: SemanticdbIndex, config: DisableConfig)
Expand All @@ -61,31 +75,24 @@ final case class Disable(index: SemanticdbIndex, config: DisableConfig)
override def description: String =
"Linter that reports an error on a configurable set of symbols."

private lazy val disabledSymbolInSynthetics: SymbolMatcher =
SymbolMatcher.normalized(config.allSymbolsInSynthetics: _*)

private lazy val disabledBlock: SymbolMatcher =
SymbolMatcher.normalized(config.allUnlessBlocks: _*)

override def init(config: Conf): Configured[Rule] =
config
.getOrElse("disable", "Disable")(DisableConfig.default)
.map(Disable(index, _))

private val safeBlock = new DisableSymbolMatcher(config.allSafeBlocks)
private val disabledSymbolInSynthetics =
new DisableSymbolMatcher(config.ifSynthetic)

private def createLintMessage(
symbol: Symbol.Global,
signature: Signature,
custom: Option[CustomMessage[Symbol.Global]],
disabled: DisabledSymbol,
pos: Position,
details: String = ""): LintMessage = {
val message = config
.customMessage(symbol)
.flatMap(_.message)
.getOrElse(s"${signature.name} is disabled$details")
val message = disabled.message.getOrElse(
s"${symbol.signature.name} is disabled$details")

val id = custom
.flatMap(_.id)
.getOrElse(signature.name)
val id = disabled.id.getOrElse(symbol.signature.name)

errorCategory
.copy(id = id)
Expand All @@ -94,38 +101,59 @@ final case class Disable(index: SemanticdbIndex, config: DisableConfig)

private def checkTree(ctx: RuleCtx): Seq[LintMessage] = {
def filterBlockedSymbolsInBlock(
blockedSymbols: List[Symbol.Global],
block: Tree): List[Symbol.Global] = {
blockedSymbols: List[DisabledSymbol],
block: Tree): List[DisabledSymbol] =
ctx.index.symbol(block) match {
case Some(symbolBlock: Symbol.Global) =>
blockedSymbols.filter(sym =>
!config.symbolsInSafeBlock(symbolBlock).contains(sym))
val symbolsInMatchedBlocks =
config.unlessInside.flatMap(
u =>
if (u.safeBlock.matches(symbolBlock)) u.symbols
else List.empty)
val res = blockedSymbols.filterNot(symbolsInMatchedBlocks.contains)
res
case _ => blockedSymbols
}

def skipTermSelect(term: Term): Boolean = term match {
case _: Term.Name => true
case Term.Select(q, _) => skipTermSelect(q)
case _ => false
}

def handleName(t: Name, blockedSymbols: List[DisabledSymbol])
: Either[LintMessage, List[DisabledSymbol]] = {
val isBlocked = new DisableSymbolMatcher(blockedSymbols)
ctx.index.symbol(t) match {
case Some(isBlocked(s: Symbol.Global, disabled)) =>
SymbolOps.normalize(s) match {
case g: Symbol.Global if g.signature.name != "<init>" =>
Left(createLintMessage(g, disabled, t.pos))
case _ => Right(blockedSymbols)
}
case _ => Right(blockedSymbols)
}
}

new ContextTraverser(config.allSymbols)({
new ContextTraverser(config.allDisabledSymbols)({
case (_: Import, _) => Right(List.empty)
case (Term.Select(q, name), blockedSymbols) if skipTermSelect(q) =>
handleName(name, blockedSymbols)
case (Type.Select(q, name), blockedSymbols) if skipTermSelect(q) =>
handleName(name, blockedSymbols)
case (
Term.Apply(Term.Select(disabledBlock(block), Term.Name("apply")), _),
Term
.Apply(Term.Select(block @ safeBlock(_, _), Term.Name("apply")), _),
blockedSymbols) =>
Right(filterBlockedSymbolsInBlock(blockedSymbols, block)) // <Block>.apply
case (Term.Apply(disabledBlock(block), _), blockedSymbols) =>
case (Term.Apply(block @ safeBlock(_, _), _), blockedSymbols) =>
Right(filterBlockedSymbolsInBlock(blockedSymbols, block)) // <Block>(...)
case (_: Defn.Def, _) =>
Right(config.allSymbols) // reset blocked symbols in def
Right(config.allDisabledSymbols) // reset blocked symbols in def
case (_: Term.Function, _) =>
Right(config.allSymbols) // reset blocked symbols in (...) => (...)
case (t: Name, blockedSymbols) => {
val isBlocked = SymbolMatcher.normalized(blockedSymbols: _*)
ctx.index.symbol(t) match {
case Some(isBlocked(s: Symbol.Global)) =>
Left(
createLintMessage(s, s.signature, config.customMessage(s), t.pos)
)
case _ => Right(blockedSymbols)
}
}
Right(config.allDisabledSymbols) // reset blocked symbols in (...) => (...)
case (t: Name, blockedSymbols) =>
handleName(t, blockedSymbols)
}).result(ctx.tree)
}

Expand All @@ -134,7 +162,7 @@ final case class Disable(index: SemanticdbIndex, config: DisableConfig)
document <- ctx.index.documents.view
ResolvedName(
pos,
disabledSymbolInSynthetics(symbol @ Symbol.Global(_, signature)),
disabledSymbolInSynthetics(symbol @ Symbol.Global(_, _), disabled),
false
) <- document.synthetics.view.flatMap(_.names)
} yield {
Expand All @@ -147,12 +175,7 @@ final case class Disable(index: SemanticdbIndex, config: DisableConfig)
case _ =>
"" -> pos
}
createLintMessage(
symbol,
signature,
config.customMessage(symbol),
caret,
details)
createLintMessage(symbol, disabled, caret, details)
}
}

Expand Down
46 changes: 46 additions & 0 deletions scalafix-tests/input/src/main/scala/test/DisableRegex.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
rules = Disable
Disable.symbols = [
{
regex = "scala.collection.mutable.*"
message = "use immutable structures"
}
]

Disable.unlessInside = [
{
safeBlock = "test.DisableRegex.IO"
symbols = [
{
regex = {
includes = "java.io.*"
excludes = "java.io.InputStream"
}
message = "input/output are only allowed within IO block"
}
]
}
]
*/
package test

object DisableRegex {
import scala.collection.mutable._ // ok
import scala.collection.mutable // ok

@SuppressWarnings(Array("Disable.ListBuffer"))
val buffer = new ListBuffer[Int]()

@SuppressWarnings(Array("Disable.HashMap"))
val abc = new mutable.HashMap[String, String]()

object IO { // IO we deserve
def apply[T](run: => T): Nothing = ???
}

System.out.println("I love to debug by console messages.") // assert: Disable.println
IO {
System.out.println("[IMPORTANT MESSAGE]") // ok
}
System.in.read() // ok
}