Merge pull request scala#10484 from lrytz/moreQuickFixes

Add quick fixes to more warnings and errors

Author: lrytz

src/compiler/scala/tools/nsc/Reporting.scala

@@ -69,6 +69,8 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio
       if (settings.quickfix.isSetByUser && settings.quickfix.value.isEmpty) {
         globalError(s"Missing message filter for `-quickfix`; see `-quickfix:help` or use `-quickfix:any` to apply all available quick fixes.")
         Nil
+      } else if (settings.quickFixSilent) {
+        Nil
       } else {
         val parsed = settings.quickfix.value.map(WConf.parseFilter(_, rootDirPrefix))
         val msgs = parsed.collect { case Left(msg) => msg }
@@ -167,7 +169,7 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio
 
       val quickfixed = {
         if (!skipRewriteAction(action) && registerTextEdit(warning)) s"[rewritten by -quickfix] ${warning.msg}"
-        else if (warning.actions.exists(_.edits.nonEmpty)) s"[quick fix available] ${warning.msg}"
+        else if (warning.actions.exists(_.edits.nonEmpty) && !settings.quickFixSilent) s"${warning.msg} [quickfixable]"
         else warning.msg
       }
 
@@ -352,11 +354,14 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio
     def warning(pos: Position, msg: String, category: WarningCategory, site: Symbol, origin: String): Unit =
       issueIfNotSuppressed(Message.Origin(pos, msg, category, siteName(site), origin, actions = Nil))
 
+    def codeAction(title: String, pos: Position, newText: String, desc: String, expected: Option[(String, CompilationUnit)] = None) =
+      CodeAction(title, pos, newText, desc, expected.forall(e => e._1 == e._2.sourceAt(pos)))
+
     // Remember CodeActions that match `-quickfix` and report the error through the reporter
     def error(pos: Position, msg: String, actions: List[CodeAction]): Unit = {
       val quickfixed = {
         if (registerErrorTextEdit(pos, msg, actions)) s"[rewritten by -quickfix] $msg"
-        else if (actions.exists(_.edits.nonEmpty)) s"[quick fix available] $msg"
+        else if (actions.exists(_.edits.nonEmpty) && !settings.quickFixSilent) s"$msg [quickfixable]"
         else msg
       }
       reporter.error(pos, quickfixed, actions)

src/compiler/scala/tools/nsc/ast/parser/Parsers.scala

@@ -17,18 +17,12 @@ package scala.tools.nsc
 package ast.parser
 
 import scala.annotation.tailrec
-import scala.collection.mutable, mutable.ListBuffer
-import scala.reflect.internal.{ModifierFlags => Flags, Precedence}
-import scala.reflect.internal.util.{
-  CodeAction,
-  FreshNameCreator,
-  ListOfNil,
-  Position,
-  SourceFile,
-  TextEdit,
-}
-import Tokens._
+import scala.collection.mutable
+import scala.collection.mutable.ListBuffer
+import scala.reflect.internal.util.{CodeAction, FreshNameCreator, ListOfNil, Position, SourceFile}
+import scala.reflect.internal.{Precedence, ModifierFlags => Flags}
 import scala.tools.nsc.Reporting.WarningCategory
+import scala.tools.nsc.ast.parser.Tokens._
 
 /** Historical note: JavaParsers started life as a direct copy of Parsers
  *  but at a time when that Parsers had been replaced by a different one.
@@ -329,7 +323,7 @@ self =>
       def source = parser.source
     }
     val treeBuilder = new ParserTreeBuilder
-    import treeBuilder.{global => _, unit => _, source => _, fresh => _, _}
+    import treeBuilder.{fresh => _, global => _, source => _, unit => _, _}
 
     implicit def fresh: FreshNameCreator = unit.fresh
 
@@ -633,11 +627,11 @@ self =>
       else deprecationWarning(offset, msg, since, actions)
 
     // deprecation or migration under -Xsource:3, with different messages
-    def hardMigrationWarning(offset: Offset, depr: => String, migr: => String, since: String, actions: List[CodeAction]): Unit =
-      if (currentRun.isScala3) warning(offset, migr, WarningCategory.Scala3Migration, actions)
-      else deprecationWarning(offset, depr, since, actions)
+    def hardMigrationWarning(offset: Offset, depr: => String, migr: => String, since: String, actions: String => List[CodeAction]): Unit =
+      if (currentRun.isScala3) warning(offset, migr, WarningCategory.Scala3Migration, actions(migr))
+      else deprecationWarning(offset, depr, since, actions(depr))
     def hardMigrationWarning(offset: Offset, depr: => String, migr: => String, since: String): Unit =
-      hardMigrationWarning(offset, depr, migr, since, Nil)
+      hardMigrationWarning(offset, depr, migr, since, _ => Nil)
 
     def expectedMsgTemplate(exp: String, fnd: String) = s"$exp expected but $fnd found."
     def expectedMsg(token: Token): String =
@@ -748,12 +742,18 @@ self =>
     def isWildcardType = in.token == USCORE || isScala3WildcardType
     def isScala3WildcardType = isRawIdent && in.name == raw.QMARK
     def checkQMarkDefinition() =
-      if (isScala3WildcardType)
-        syntaxError(in.offset, "using `?` as a type name requires backticks.")
+      if (isScala3WildcardType) {
+        val msg = "using `?` as a type name requires backticks."
+        syntaxError(in.offset, msg,
+          runReporting.codeAction("add backticks", r2p(in.offset, in.offset, in.offset + 1), "`?`", msg, expected = Some(("?", unit))))
+      }
+
     def checkKeywordDefinition() =
-      if (isRawIdent && scala3Keywords.contains(in.name))
-        deprecationWarning(in.offset,
-          s"Wrap `${in.name}` in backticks to use it as an identifier, it will become a keyword in Scala 3.", "2.13.7")
+      if (isRawIdent && scala3Keywords.contains(in.name)) {
+        val msg = s"Wrap `${in.name}` in backticks to use it as an identifier, it will become a keyword in Scala 3."
+        deprecationWarning(in.offset, msg, "2.13.7",
+          runReporting.codeAction("add backticks", r2p(in.offset, in.offset, in.offset + in.name.length), s"`${in.name}`", msg, expected = Some((in.name.toString, unit))))
+      }
 
     def isIdent = in.token == IDENTIFIER || in.token == BACKQUOTED_IDENT
     def isMacro = in.token == IDENTIFIER && in.name == nme.MACROkw
@@ -816,8 +816,7 @@ self =>
         val wrn = sm"""|$msg
                        |Use '-Wconf:msg=lambda-parens:s' to silence this warning."""
         def actions =
-          if (tree.pos.isRange)
-            List(CodeAction("lambda parameter", Some(msg), List(TextEdit(tree.pos, s"(${unit.sourceAt(tree.pos)})"))))
+          if (tree.pos.isRange) runReporting.codeAction("add parentheses", tree.pos, s"(${unit.sourceAt(tree.pos)})", msg)
           else Nil
         migrationWarning(tree.pos.point, wrn, "2.13.11", actions)
         List(convertToParam(tree))
@@ -1029,11 +1028,17 @@ self =>
 
     def finishBinaryOp(isExpr: Boolean, opinfo: OpInfo, rhs: Tree): Tree = {
       import opinfo._
-      if (targs.nonEmpty)
-        migrationWarning(offset, "type application is not allowed for infix operators", "2.13.11")
       val operatorPos: Position = Position.range(rhs.pos.source, offset, offset, offset + operator.length)
       val pos                   = lhs.pos.union(rhs.pos).union(operatorPos).withEnd(in.lastOffset).withPoint(offset)
 
+      if (targs.nonEmpty) {
+        val qual = unit.sourceAt(lhs.pos)
+        val fun = s"${CodeAction.maybeWrapInParens(qual)}.${unit.sourceAt(operatorPos.withEnd(rhs.pos.start))}".trim
+        val fix = s"$fun${CodeAction.wrapInParens(unit.sourceAt(rhs.pos))}"
+        val msg = "type application is not allowed for infix operators"
+        migrationWarning(offset, msg, "2.13.11",
+          runReporting.codeAction("use selection", pos, fix, msg))
+      }
       atPos(pos)(makeBinop(isExpr, lhs, operator, rhs, operatorPos, targs))
     }
 
@@ -1090,7 +1095,9 @@ self =>
         if (in.token == ARROW)
           atPos(start, in.skipToken()) { makeSafeFunctionType(ts, typ()) }
         else if (ts.isEmpty) {
-          syntaxError(start, "Illegal literal type (), use Unit instead")
+          val msg = "Illegal literal type (), use Unit instead"
+          syntaxError(start, msg,
+            runReporting.codeAction("use `Unit`", r2p(start, start, start + 2), "Unit", msg, expected = Some(("()", unit))))
           EmptyTree
         }
         else {
@@ -1174,7 +1181,9 @@ self =>
               if (lookingAhead(in.token == RPAREN)) {
                 in.nextToken()
                 in.nextToken()
-                syntaxError(start, "Illegal literal type (), use Unit instead")
+                val msg = "Illegal literal type (), use Unit instead"
+                syntaxError(start, msg,
+                  runReporting.codeAction("use `Unit`", r2p(start, start, start + 2), "Unit", msg, expected = Some(("()", unit))))
                 EmptyTree
               }
               else
@@ -1475,9 +1484,9 @@ self =>
         else withPlaceholders(interpolatedString(inPattern), isAny = true) // interpolator params are Any* by definition
       }
       else if (in.token == SYMBOLLIT) {
-        def msg(what: String) =
-          s"""symbol literal is $what; use Symbol("${in.strVal}") instead"""
-        deprecationWarning(in.offset, msg("deprecated"), "2.13.0")
+        val msg = s"""symbol literal is deprecated; use Symbol("${in.strVal}") instead"""
+        deprecationWarning(in.offset, msg, "2.13.0",
+          runReporting.codeAction("replace symbol literal", r2p(in.offset, in.offset, in.offset + 1 + in.strVal.length), s"""Symbol("${in.strVal}")""", msg, expected = Some((s"'${in.strVal}", unit))))
         Apply(scalaDot(nme.Symbol), List(finish(in.strVal)))
       }
       else finish(in.token match {
@@ -2095,17 +2104,15 @@ self =>
       val hasEq = in.token == EQUALS
 
       if (hasVal) {
-        def actions = {
-          val pos = r2p(valOffset, valOffset, valOffset + 4)
-          if (unit.sourceAt(pos) != "val ") Nil else
-            List(CodeAction("val in for comprehension", None, List(TextEdit(pos, ""))))
-        }
+        def actions(msg: String) = runReporting.codeAction("remove `val` keyword", r2p(valOffset, valOffset, valOffset + 4), "", msg, expected = Some(("val ", unit)))
         def msg(what: String, instead: String): String = s"`val` keyword in for comprehension is $what: $instead"
         if (hasEq) {
           val without = "instead, bind the value without `val`"
           hardMigrationWarning(in.offset, msg("deprecated", without), msg("unsupported", without), "2.10.0", actions)
+        } else {
+          val m = msg("unsupported", "just remove `val`")
+          syntaxError(in.offset, m, actions(m))
         }
-        else syntaxError(in.offset, msg("unsupported", "just remove `val`"), actions)
       }
 
       if (hasEq && eqOK && !hasCase) in.nextToken()
@@ -2969,7 +2976,11 @@ self =>
     def funDefOrDcl(start: Int, mods: Modifiers): Tree = {
       in.nextToken()
       if (in.token == THIS) {
-        def missingEquals() = hardMigrationWarning(in.lastOffset, "procedure syntax is deprecated for constructors: add `=`, as in method definition", "2.13.2")
+        def missingEquals() = {
+          val msg = "procedure syntax is deprecated for constructors: add `=`, as in method definition"
+          hardMigrationWarning(in.lastOffset, msg, "2.13.2",
+            runReporting.codeAction("replace procedure syntax", o2p(in.lastOffset), " =", msg))
+        }
         atPos(start, in.skipToken()) {
           val vparamss = paramClauses(nme.CONSTRUCTOR, classContextBounds map (_.duplicate), ofCaseClass = false)
           newLineOptWhenFollowedBy(LBRACE)
@@ -3006,8 +3017,8 @@ self =>
         var restype = fromWithinReturnType(typedOpt())
         def msg(what: String, instead: String) =
           s"procedure syntax is $what: instead, add `$instead` to explicitly declare `$name`'s return type"
-        def declActions = List(CodeAction("procedure syntax (decl)", None, List(TextEdit(o2p(in.lastOffset), ": Unit"))))
-        def defnActions = List(CodeAction("procedure syntax (defn)", None, List(TextEdit(o2p(in.lastOffset), ": Unit ="))))
+        def declActions(msg: String) = runReporting.codeAction("add result type", o2p(in.lastOffset), ": Unit", msg)
+        def defnActions(msg: String) = runReporting.codeAction("replace procedure syntax", o2p(in.lastOffset), ": Unit =", msg)
         val rhs =
           if (isStatSep || in.token == RBRACE) {
             if (restype.isEmpty) {
@@ -3035,7 +3046,11 @@ self =>
         if (nme.isEncodedUnary(name) && vparamss.nonEmpty) {
           def instead = DefDef(newmods, name.toTermName.decodedName, tparams, vparamss.drop(1), restype, rhs)
           def unaryMsg(what: String) = s"unary prefix operator definition with empty parameter list is $what: instead, remove () to declare as `$instead`"
-          def warnNilary() = hardMigrationWarning(nameOffset, unaryMsg("deprecated"), unaryMsg("unsupported"), "2.13.4")
+          def action(msg: String) = {
+            val o = nameOffset + name.decode.length
+            runReporting.codeAction("remove ()", r2p(o, o, o + 2), "", msg, expected = Some(("()", unit)))
+          }
+          def warnNilary() = hardMigrationWarning(nameOffset, unaryMsg("deprecated"), unaryMsg("unsupported"), "2.13.4", action)
           vparamss match {
             case List(List())                               => warnNilary()
             case List(List(), x :: xs) if x.mods.isImplicit => warnNilary()
@@ -3313,7 +3328,9 @@ self =>
      */
     def templateOpt(mods: Modifiers, name: Name, constrMods: Modifiers, vparamss: List[List[ValDef]], tstart: Offset): Template = {
       def deprecatedUsage(): Boolean = {
-        deprecationWarning(in.offset, "Using `<:` for `extends` is deprecated", since = "2.12.5")
+        val msg = "Using `<:` for `extends` is deprecated"
+        deprecationWarning(in.offset, msg, since = "2.12.5",
+          runReporting.codeAction("use `extends`", r2p(in.offset, in.offset, in.offset + 2), "extends", msg, expected = Some(("<:", unit))))
         true
       }
       val (parents, self, body) =

src/compiler/scala/tools/nsc/ast/parser/Scanners.scala

@@ -167,6 +167,8 @@ trait Scanners extends ScannersCommon {
   }
 
   abstract class Scanner extends CharArrayReader with TokenData with ScannerData with ScannerCommon with DocScanner {
+    def unit: CompilationUnit
+
     /** A switch whether operators at the start of lines can be infix operators. */
     private var allowLeadingInfixOperators = true
 
@@ -817,10 +819,14 @@ trait Scanners extends ScannersCommon {
         case _ =>
           def fetchOther() = {
             if (ch == '\u21D2') {
-              deprecationWarning("The unicode arrow `⇒` is deprecated, use `=>` instead. If you still wish to display it as one character, consider using a font with programming ligatures such as Fira Code.", "2.13.0")
+              val msg = "The unicode arrow `⇒` is deprecated, use `=>` instead. If you still wish to display it as one character, consider using a font with programming ligatures such as Fira Code."
+              deprecationWarning(msg, "2.13.0",
+                runReporting.codeAction("replace unicode arrow", unit.position(offset).withEnd(offset + 1), "=>", msg, expected = Some(("⇒", unit))))
               nextChar(); token = ARROW
             } else if (ch == '\u2190') {
-              deprecationWarning("The unicode arrow `←` is deprecated, use `<-` instead. If you still wish to display it as one character, consider using a font with programming ligatures such as Fira Code.", "2.13.0")
+              val msg = "The unicode arrow `←` is deprecated, use `<-` instead. If you still wish to display it as one character, consider using a font with programming ligatures such as Fira Code."
+              deprecationWarning(msg, "2.13.0",
+                runReporting.codeAction("replace unicode arrow", unit.position(offset).withEnd(offset + 1), "<-", msg, expected = Some(("←", unit))))
               nextChar(); token = LARROW
             } else if (isUnicodeIdentifierStart(ch)) {
               putChar(ch)
@@ -1370,7 +1376,9 @@ trait Scanners extends ScannersCommon {
       // 1l is an acknowledged bad practice
       def lintel(): Unit = {
         val msg = "Lowercase el for long is not recommended because it is easy to confuse with numeral 1; use uppercase L instead"
-        if (ch == 'l') deprecationWarning(numberOffset + cbuf.length, msg, since="2.13.0")
+        val o = numberOffset + cbuf.length
+        if (ch == 'l') deprecationWarning(o, msg, since="2.13.0",
+          runReporting.codeAction("use uppercase L", unit.position(o).withEnd(o + 1), "L", msg, expected = Some(("l", unit))))
       }
       // after int: 5e7f, 42L, 42.toDouble but not 42b.
       def restOfNumber(): Unit = {
@@ -1573,6 +1581,8 @@ trait Scanners extends ScannersCommon {
    *  Useful for looking inside source files that are not currently compiled to see what's there
    */
   class SourceFileScanner(val source: SourceFile) extends Scanner {
+    def unit = global.currentUnit
+
     val buf = source.content
 
     // suppress warnings, throw exception on errors
@@ -1584,7 +1594,7 @@ trait Scanners extends ScannersCommon {
 
   /** A scanner over a given compilation unit
    */
-  class UnitScanner(val unit: CompilationUnit, patches: List[BracePatch]) extends SourceFileScanner(unit.source) {
+  class UnitScanner(override val unit: CompilationUnit, patches: List[BracePatch]) extends SourceFileScanner(unit.source) {
     def this(unit: CompilationUnit) = this(unit, List())
 
     override def warning(off: Offset, msg: String, category: WarningCategory): Unit =

src/compiler/scala/tools/nsc/settings/StandardScalaSettings.scala

@@ -64,8 +64,11 @@ trait StandardScalaSettings { _: MutableSettings =>
         |  -quickfix:msg=Auto-application   apply quick fixes where the message contains "Auto-application"
         |
         |Use `-Wconf:any:warning-verbose` to display applicable message filters with each warning.
+        |
+        |Use `-quickfix:silent` to omit the `[quickfixable]` tag in compiler messages.
         |""".stripMargin),
     prepend = true)
+  def quickFixSilent: Boolean = quickfix.value == List("silent")
   val release =
     ChoiceSetting("-release", "release", "Compile for a version of the Java API and target class file.", AllTargetVersions, normalizeTarget(javaSpecVersion))
       .withPostSetHook { setting =>

src/compiler/scala/tools/nsc/typechecker/Adaptations.scala

@@ -98,9 +98,13 @@ trait Adaptations {
         if (settings.lintArgDiscard && discardedArgs) context.warning(t.pos, adaptWarningMessage(
           s"adapted the argument list to expected Unit type: arguments will be discarded"),
           WarningCategory.LintAdaptedArgs)
-        else if (settings.warnAdaptedArgs && !isInfix) context.warning(t.pos, adaptWarningMessage(
-          s"adapted the argument list to the expected ${args.size}-tuple: add additional parens instead"),
-          WarningCategory.LintAdaptedArgs)
+        else if (settings.warnAdaptedArgs && !isInfix) {
+          val msg = adaptWarningMessage(
+            s"adapted the argument list to the expected ${args.size}-tuple: add additional parens instead")
+          val pos = wrappingPos(args)
+          context.warning(t.pos, msg, WarningCategory.LintAdaptedArgs,
+            runReporting.codeAction("add wrapping parentheses", pos, s"(${currentUnit.sourceAt(pos)})", msg))
+        }
         true // keep adaptation
       }
       if (args.nonEmpty)