Skip to content

Commit 8af6da5

Browse files
authored
Backport "feat: hook up actions in sbt-bridge to start forwarding actionable diagnostics" to LTS (#19060)
Backports #17561 to the LTS branch. PR submitted by the release tooling. [skip ci]
2 parents 55cbec5 + 23fb8d3 commit 8af6da5

File tree

20 files changed

+375
-35
lines changed

20 files changed

+375
-35
lines changed

community-build/src/scala/dotty/communitybuild/projects.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ final case class SbtCommunityProject(
140140
case Some(ivyHome) => List(s"-Dsbt.ivy.home=$ivyHome")
141141
case _ => Nil
142142
extraSbtArgs ++ sbtProps ++ List(
143-
"-sbt-version", "1.8.2",
143+
"-sbt-version", "1.9.0",
144144
"-Dsbt.supershell=false",
145145
s"-Ddotty.communitybuild.dir=$communitybuildDir",
146146
s"--addPluginSbtFile=$sbtPluginFilePath"

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3029,8 +3029,7 @@ object Parsers {
30293029
val mod = atSpan(in.skipToken()) { modOfToken(tok, name) }
30303030

30313031
if mods.isOneOf(mod.flags) then
3032-
syntaxError(RepeatedModifier(mod.flags.flagsString), mod.span)
3033-
3032+
syntaxError(RepeatedModifier(mod.flags.flagsString, source, mod.span), mod.span)
30343033
addMod(mods, mod)
30353034
}
30363035

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package dotty.tools.dotc.reporting
2+
3+
import dotty.tools.dotc.rewrites.Rewrites.ActionPatch
4+
5+
/** A representation of a code action / fix that can be used by tooling to
6+
* apply a fix to their code.
7+
*
8+
* @param title The title of the fix, often showed to a user in their editor.
9+
* @param description An optional description of the fix.
10+
* @param patches The patches that this fix contains.
11+
*/
12+
case class CodeAction(
13+
title: String,
14+
description: Option[String],
15+
patches: List[ActionPatch]
16+
)

compiler/src/dotty/tools/dotc/reporting/Message.scala

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import printing.Formatting.hl
1010
import config.SourceVersion
1111

1212
import scala.language.unsafeNulls
13-
1413
import scala.annotation.threadUnsafe
1514

1615
/** ## Tips for error message generation
@@ -384,6 +383,11 @@ abstract class Message(val errorId: ErrorMessageID)(using Context) { self =>
384383
*/
385384
def showAlways = false
386385

386+
/** A list of actions attached to this message to address the issue this
387+
* message represents.
388+
*/
389+
def actions(using Context): List[CodeAction] = List.empty
390+
387391
override def toString = msg
388392
}
389393

compiler/src/dotty/tools/dotc/reporting/messages.scala

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ import transform.SymUtils._
2929
import scala.util.matching.Regex
3030
import java.util.regex.Matcher.quoteReplacement
3131
import cc.CaptureSet.IdentityCaptRefMap
32+
import dotty.tools.dotc.rewrites.Rewrites.ActionPatch
33+
import dotty.tools.dotc.util.Spans.Span
34+
import dotty.tools.dotc.util.SourcePosition
35+
import scala.jdk.CollectionConverters.*
36+
import dotty.tools.dotc.util.SourceFile
3237

3338
/** Messages
3439
* ========
@@ -493,7 +498,7 @@ extends SyntaxMsg(ObjectMayNotHaveSelfTypeID) {
493498
}
494499
}
495500

496-
class RepeatedModifier(modifier: String)(implicit ctx:Context)
501+
class RepeatedModifier(modifier: String, source: SourceFile, span: Span)(implicit ctx:Context)
497502
extends SyntaxMsg(RepeatedModifierID) {
498503
def msg(using Context) = i"""Repeated modifier $modifier"""
499504

@@ -512,6 +517,17 @@ extends SyntaxMsg(RepeatedModifierID) {
512517
|
513518
|"""
514519
}
520+
521+
override def actions(using Context) =
522+
import scala.language.unsafeNulls
523+
List(
524+
CodeAction(title = s"""Remove repeated modifier: "$modifier"""",
525+
description = None,
526+
patches = List(
527+
ActionPatch(SourcePosition(source, span), "")
528+
)
529+
)
530+
)
515531
}
516532

517533
class InterpolatedStringError()(implicit ctx:Context)
@@ -1846,15 +1862,28 @@ class FailureToEliminateExistential(tp: Type, tp1: Type, tp2: Type, boundSyms: L
18461862
|are only approximated in a best-effort way."""
18471863
}
18481864

1849-
class OnlyFunctionsCanBeFollowedByUnderscore(tp: Type)(using Context)
1865+
class OnlyFunctionsCanBeFollowedByUnderscore(tp: Type, tree: untpd.PostfixOp)(using Context)
18501866
extends SyntaxMsg(OnlyFunctionsCanBeFollowedByUnderscoreID) {
18511867
def msg(using Context) = i"Only function types can be followed by ${hl("_")} but the current expression has type $tp"
18521868
def explain(using Context) =
18531869
i"""The syntax ${hl("x _")} is no longer supported if ${hl("x")} is not a function.
18541870
|To convert to a function value, you need to explicitly write ${hl("() => x")}"""
1871+
1872+
override def actions(using Context) =
1873+
import scala.language.unsafeNulls
1874+
val untpd.PostfixOp(qual, Ident(nme.WILDCARD)) = tree: @unchecked
1875+
List(
1876+
CodeAction(title = "Rewrite to function value",
1877+
description = None,
1878+
patches = List(
1879+
ActionPatch(SourcePosition(tree.source, Span(tree.span.start)), "(() => "),
1880+
ActionPatch(SourcePosition(tree.source, Span(qual.span.end, tree.span.end)), ")")
1881+
)
1882+
)
1883+
)
18551884
}
18561885

1857-
class MissingEmptyArgumentList(method: String)(using Context)
1886+
class MissingEmptyArgumentList(method: String, tree: tpd.Tree)(using Context)
18581887
extends SyntaxMsg(MissingEmptyArgumentListID) {
18591888
def msg(using Context) = i"$method must be called with ${hl("()")} argument"
18601889
def explain(using Context) = {
@@ -1869,6 +1898,17 @@ class MissingEmptyArgumentList(method: String)(using Context)
18691898
|In Dotty, this idiom is an error. The application syntax has to follow exactly the parameter syntax.
18701899
|Excluded from this rule are methods that are defined in Java or that override methods defined in Java."""
18711900
}
1901+
1902+
override def actions(using Context) =
1903+
import scala.language.unsafeNulls
1904+
List(
1905+
CodeAction(title = "Insert ()",
1906+
description = None,
1907+
patches = List(
1908+
ActionPatch(SourcePosition(tree.source, tree.span.endPos), "()"),
1909+
)
1910+
)
1911+
)
18721912
}
18731913

18741914
class DuplicateNamedTypeParameter(name: Name)(using Context)

compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@ import core.Contexts._
77
import collection.mutable
88
import scala.annotation.tailrec
99
import dotty.tools.dotc.reporting.Reporter
10+
import dotty.tools.dotc.util.SourcePosition;
1011

1112
import java.io.OutputStreamWriter
1213
import java.nio.charset.StandardCharsets.UTF_8
14+
import dotty.tools.dotc.reporting.CodeAction
1315

1416
/** Handles rewriting of Scala2 files to Dotty */
1517
object Rewrites {
@@ -19,6 +21,16 @@ object Rewrites {
1921
def delta = replacement.length - (span.end - span.start)
2022
}
2123

24+
/** A special type of Patch that instead of just a span, contains the
25+
* full SourcePosition. This is useful when being used by
26+
* [[dotty.tools.dotc.reporting.CodeAction]] or if the patch doesn't
27+
* belong to the same file that the actual issue it's addressing is in.
28+
*
29+
* @param srcPos The SourcePosition of the patch.
30+
* @param replacement The Replacement that should go in that position.
31+
*/
32+
case class ActionPatch(srcPos: SourcePosition, replacement: String)
33+
2234
private class Patches(source: SourceFile) {
2335
private[Rewrites] val pbuf = new mutable.ListBuffer[Patch]()
2436

@@ -88,6 +100,14 @@ object Rewrites {
88100
report.echo(s"[patched file ${source.file.path}]")
89101
rewrites.patched(source).writeBack()
90102
}
103+
104+
/** Given a CodeAction take the patches and apply them.
105+
*
106+
* @param action The CodeAction containing the patches
107+
*/
108+
def applyAction(action: CodeAction)(using Context): Unit =
109+
action.patches.foreach: actionPatch =>
110+
patch(actionPatch.srcPos.span, actionPatch.replacement)
91111
}
92112

93113
/** A completely encapsulated class representing rewrite state, used

compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ object ErrorReporting {
5555
val meth = err.exprStr(methPart(tree))
5656
val info = if tree.symbol.exists then tree.symbol.info else mt
5757
if isCallableWithSingleEmptyArgumentList(info) then
58-
report.error(MissingEmptyArgumentList(meth), tree.srcPos)
58+
report.error(MissingEmptyArgumentList(meth, tree), tree.srcPos)
5959
else
6060
report.error(MissingArgumentList(meth, tree.symbol), tree.srcPos)
6161

compiler/src/dotty/tools/dotc/typer/Typer.scala

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import cc.CheckCaptures
5454
import config.Config
5555

5656
import scala.annotation.constructorOnly
57+
import dotty.tools.dotc.rewrites.Rewrites
5758

5859
object Typer {
5960

@@ -2877,11 +2878,13 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
28772878
case closure(_, _, _) =>
28782879
case _ =>
28792880
val recovered = typed(qual)(using ctx.fresh.setExploreTyperState())
2880-
report.errorOrMigrationWarning(OnlyFunctionsCanBeFollowedByUnderscore(recovered.tpe.widen), tree.srcPos, from = `3.0`)
2881+
val msg = OnlyFunctionsCanBeFollowedByUnderscore(recovered.tpe.widen, tree)
2882+
report.errorOrMigrationWarning(msg, tree.srcPos, from = `3.0`)
28812883
if (migrateTo3) {
28822884
// Under -rewrite, patch `x _` to `(() => x)`
2883-
patch(Span(tree.span.start), "(() => ")
2884-
patch(Span(qual.span.end, tree.span.end), ")")
2885+
msg.actions
2886+
.headOption
2887+
.foreach(Rewrites.applyAction)
28852888
return typed(untpd.Function(Nil, qual), pt)
28862889
}
28872890
}
@@ -3878,10 +3881,17 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
38783881
def adaptNoArgsUnappliedMethod(wtp: MethodType, functionExpected: Boolean, arity: Int): Tree = {
38793882
/** Is reference to this symbol `f` automatically expanded to `f()`? */
38803883
def isAutoApplied(sym: Symbol): Boolean =
3884+
lazy val msg = MissingEmptyArgumentList(sym.show, tree)
3885+
38813886
sym.isConstructor
38823887
|| sym.matchNullaryLoosely
3883-
|| Feature.warnOnMigration(MissingEmptyArgumentList(sym.show), tree.srcPos, version = `3.0`)
3884-
&& { patch(tree.span.endPos, "()"); true }
3888+
|| Feature.warnOnMigration(msg, tree.srcPos, version = `3.0`)
3889+
&& {
3890+
msg.actions
3891+
.headOption
3892+
.foreach(Rewrites.applyAction)
3893+
true
3894+
}
38853895

38863896
/** If this is a selection prototype of the form `.apply(...): R`, return the nested
38873897
* function prototype `(...)R`. Otherwise `pt`.
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
package dotty.tools.dotc.reporting
2+
3+
import dotty.tools.DottyTest
4+
import dotty.tools.dotc.rewrites.Rewrites
5+
import dotty.tools.dotc.rewrites.Rewrites.ActionPatch
6+
import dotty.tools.dotc.util.SourceFile
7+
8+
import scala.annotation.tailrec
9+
import scala.jdk.CollectionConverters.*
10+
import scala.runtime.Scala3RunTime.assertFailed
11+
12+
import org.junit.Assert._
13+
import org.junit.Test
14+
15+
/** This is a test suite that is meant to test the actions attached to the
16+
* diagnostic for a given code snippet.
17+
*/
18+
class CodeActionTest extends DottyTest:
19+
20+
@Test def convertToFunctionValue =
21+
checkCodeAction(
22+
"""|object Test:
23+
| def x: Int = 3
24+
| val test = x _
25+
|""".stripMargin,
26+
"Rewrite to function value",
27+
"""|object Test:
28+
| def x: Int = 3
29+
| val test = (() => x)
30+
|""".stripMargin
31+
)
32+
33+
@Test def insertBracesForEmptyArgument =
34+
checkCodeAction(
35+
"""|object Test:
36+
| def foo(): Unit = ()
37+
| val x = foo
38+
|""".stripMargin,
39+
"Insert ()",
40+
"""|object Test:
41+
| def foo(): Unit = ()
42+
| val x = foo()
43+
|""".stripMargin
44+
45+
)
46+
47+
@Test def removeRepeatModifier =
48+
checkCodeAction(
49+
"""|final final class Test
50+
|""".stripMargin,
51+
"""Remove repeated modifier: "final"""",
52+
// TODO look into trying to remove the extra space that is left behind
53+
"""|final class Test
54+
|""".stripMargin
55+
56+
)
57+
58+
// Make sure we're not using the default reporter, which is the ConsoleReporter,
59+
// meaning they will get reported in the test run and that's it.
60+
private def newContext =
61+
val rep = new StoreReporter(null) with UniqueMessagePositions with HideNonSensicalMessages
62+
initialCtx.setReporter(rep).withoutColors
63+
64+
private def checkCodeAction(code: String, title: String, expected: String) =
65+
ctx = newContext
66+
val source = SourceFile.virtual("test", code).content
67+
val runCtx = checkCompile("typer", code) { (_, _) => () }
68+
val diagnostics = runCtx.reporter.removeBufferedMessages
69+
assertEquals(1, diagnostics.size)
70+
71+
val diagnostic = diagnostics.head
72+
val actions = diagnostic.msg.actions.toList
73+
assertEquals(1, actions.size)
74+
75+
// TODO account for more than 1 action
76+
val action = actions.head
77+
assertEquals(action.title, title)
78+
val patches = action.patches.toList
79+
if patches.nonEmpty then
80+
patches.reduceLeft: (p1, p2) =>
81+
assert(p1.srcPos.span.end <= p2.srcPos.span.start, s"overlapping patches $p1 and $p2")
82+
p2
83+
else assertFailed("Expected a patch attatched to this action, but it was empty")
84+
85+
val result = patches.reverse.foldLeft(code): (newCode, patch) =>
86+
import scala.language.unsafeNulls
87+
val start = newCode.substring(0, patch.srcPos.start)
88+
val ending = newCode.substring(patch.srcPos.end, newCode.length)
89+
start + patch.replacement + ending
90+
91+
assertEquals(expected, result)

project/Dependencies.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,6 @@ object Dependencies {
2828
"com.vladsch.flexmark" % "flexmark-ext-yaml-front-matter" % flexmarkVersion,
2929
)
3030

31-
val newCompilerInterface = "org.scala-sbt" % "compiler-interface" % "1.8.0"
31+
val newCompilerInterface = "org.scala-sbt" % "compiler-interface" % "1.9.0"
3232
val oldCompilerInterface = "org.scala-sbt" % "compiler-interface" % "1.3.5"
3333
}

project/build.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
sbt.version=1.8.2
1+
sbt.version=1.9.0
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package dotty.tools.xsbt;
2+
3+
import java.util.Optional;
4+
5+
final public class Action implements xsbti.Action {
6+
private final String _title;
7+
private final Optional<String> _description;
8+
private final WorkspaceEdit _edit;
9+
10+
public Action(String title, Optional<String> description, WorkspaceEdit edit) {
11+
super();
12+
this._title = title;
13+
this._description = description;
14+
this._edit = edit;
15+
}
16+
17+
public String title() {
18+
return _title;
19+
}
20+
21+
public Optional<String> description() {
22+
return _description;
23+
}
24+
25+
public WorkspaceEdit edit() {
26+
return _edit;
27+
}
28+
}

0 commit comments

Comments
 (0)