From 58acdbeb3991b8a4b238f2774aabfb1bac2503ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Mon, 11 Dec 2017 15:05:28 +0100 Subject: [PATCH 1/3] Fix #493, handle synthetics and symbol signatures in Disable. Previously, Disable reported an error on the positions of any ResolvedName to a disabled symbol, including Input.Synthetic and Input.Denotation which are invisible and don't appear in the source file. Now Disable ignores symbol signatures and for synthetics it points a caret at the position where the synthetic got expanded and includes the pretty-printed representation of the synthetic to help make sense of the error message. --- .../scalafix/internal/rule/Disable.scala | 37 +++++++++++++------ .../input/src/main/scala/test/Disable.scala | 12 +++++- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/scalafix-core/shared/src/main/scala/scalafix/internal/rule/Disable.scala b/scalafix-core/shared/src/main/scala/scalafix/internal/rule/Disable.scala index 05ed81cf9..06575b51d 100644 --- a/scalafix-core/shared/src/main/scala/scalafix/internal/rule/Disable.scala +++ b/scalafix-core/shared/src/main/scala/scalafix/internal/rule/Disable.scala @@ -30,21 +30,34 @@ final case class Disable(index: SemanticdbIndex, config: DisableConfig) .getOrElse("disable", "Disable")(DisableConfig.default) .map(Disable(index, _)) - override def check(ctx: RuleCtx): Seq[LintMessage] = - ctx.index.names.collect { + override def check(ctx: RuleCtx): Seq[LintMessage] = { + val buffer = List.newBuilder[LintMessage] + def add(name: ResolvedName): Unit = name match { case ResolvedName( pos, disabledSymbol(symbol @ Symbol.Global(_, signature)), - false) => { - - val message = - config - .customMessage(symbol) - .getOrElse(s"${signature.name} is disabled") - - errorCategory + false) => + val (details, caret) = pos.input match { + case synthetic @ Input.Synthetic(_, input, start, end) => + // For synthetics the caret should point to the original position + // but display the inferred code. + s" and it got inferred as `${synthetic.text}`" -> + Position.Range(input, start, end) + case _ => + "" -> pos + } + val message = config + .customMessage(symbol) + .getOrElse(s"${signature.name} is disabled$details") + buffer += errorCategory .copy(id = signature.name) - .at(message, pos) - } + .at(message, caret) + case _ => + } + ctx.index.documents.foreach { document => + document.names.foreach(add) + document.synthetics.foreach(_.names.foreach(add)) } + buffer.result() + } } diff --git a/scalafix-tests/input/src/main/scala/test/Disable.scala b/scalafix-tests/input/src/main/scala/test/Disable.scala index 88b356a85..774425f12 100644 --- a/scalafix-tests/input/src/main/scala/test/Disable.scala +++ b/scalafix-tests/input/src/main/scala/test/Disable.scala @@ -13,10 +13,14 @@ Disable.symbols = [ |... |// scalafix:on Option.get""" } + "scala.collection.mutable.ListBuffer" + "scala.Predef.any2stringadd" ] */ package test +import scala.collection.mutable.ListBuffer + case object Disable { case class B() @@ -52,4 +56,10 @@ If you must Option.get, wrap the code block with ... // scalafix:on Option.get */ -} \ No newline at end of file + val l: ListBuffer[Int] = + scala.collection.mutable.ListBuffer.empty[Int] // scalafix:ok Disable.ListBuffer + List(1) + "any2stringadd" /* assert: Disable.any2stringadd + ^ +any2stringadd is disabled and it got inferred as `scala.Predef.any2stringadd[List[Int]](*)` + */ +} From 7896ccd26987154019374a6085e99e4c4765d291 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Mon, 11 Dec 2017 20:28:31 +0100 Subject: [PATCH 2/3] Refactor Disable to use views. --- .../scalafix/internal/rule/Disable.scala | 53 +++++++++---------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/scalafix-core/shared/src/main/scala/scalafix/internal/rule/Disable.scala b/scalafix-core/shared/src/main/scala/scalafix/internal/rule/Disable.scala index 06575b51d..2d4ff8c30 100644 --- a/scalafix-core/shared/src/main/scala/scalafix/internal/rule/Disable.scala +++ b/scalafix-core/shared/src/main/scala/scalafix/internal/rule/Disable.scala @@ -31,33 +31,32 @@ final case class Disable(index: SemanticdbIndex, config: DisableConfig) .map(Disable(index, _)) override def check(ctx: RuleCtx): Seq[LintMessage] = { - val buffer = List.newBuilder[LintMessage] - def add(name: ResolvedName): Unit = name match { - case ResolvedName( - pos, - disabledSymbol(symbol @ Symbol.Global(_, signature)), - false) => - val (details, caret) = pos.input match { - case synthetic @ Input.Synthetic(_, input, start, end) => - // For synthetics the caret should point to the original position - // but display the inferred code. - s" and it got inferred as `${synthetic.text}`" -> - Position.Range(input, start, end) - case _ => - "" -> pos - } - val message = config - .customMessage(symbol) - .getOrElse(s"${signature.name} is disabled$details") - buffer += errorCategory - .copy(id = signature.name) - .at(message, caret) - case _ => + for { + document <- ctx.index.documents.view + ResolvedName( + pos, + disabledSymbol(symbol @ Symbol.Global(_, signature)), + false + ) <- { + document.names.view ++ + document.synthetics.view.flatMap(_.names) + } + } yield { + val (details, caret) = pos.input match { + case synthetic @ Input.Synthetic(_, input, start, end) => + // For synthetics the caret should point to the original position + // but display the inferred code. + s" and it got inferred as `${synthetic.text}`" -> + Position.Range(input, start, end) + case _ => + "" -> pos + } + val message = config + .customMessage(symbol) + .getOrElse(s"${signature.name} is disabled$details") + errorCategory + .copy(id = signature.name) + .at(message, caret) } - ctx.index.documents.foreach { document => - document.names.foreach(add) - document.synthetics.foreach(_.names.foreach(add)) - } - buffer.result() } } From a992b02f8e2ef2f2df4c3eb646ae2c5eb79cd3ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Mon, 11 Dec 2017 20:29:53 +0100 Subject: [PATCH 3/3] Assert instead of scalafix:ok --- scalafix-tests/input/src/main/scala/test/Disable.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scalafix-tests/input/src/main/scala/test/Disable.scala b/scalafix-tests/input/src/main/scala/test/Disable.scala index 774425f12..3adbe6c02 100644 --- a/scalafix-tests/input/src/main/scala/test/Disable.scala +++ b/scalafix-tests/input/src/main/scala/test/Disable.scala @@ -56,8 +56,7 @@ If you must Option.get, wrap the code block with ... // scalafix:on Option.get */ - val l: ListBuffer[Int] = - scala.collection.mutable.ListBuffer.empty[Int] // scalafix:ok Disable.ListBuffer + val l: ListBuffer[Int] = scala.collection.mutable.ListBuffer.empty[Int] // assert: Disable.ListBuffer List(1) + "any2stringadd" /* assert: Disable.any2stringadd ^ any2stringadd is disabled and it got inferred as `scala.Predef.any2stringadd[List[Int]](*)`