From 8808e63a6d694b1a7b058b8637d1e9a5899934cd Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Tue, 23 May 2023 08:31:51 +0200 Subject: [PATCH 01/12] feat: update bridge to account for actions --- .../tools/dotc/reporting/CodeAction.scala | 9 +++ .../dotty/tools/dotc/reporting/Message.scala | 4 +- .../dotty/tools/dotc/reporting/messages.scala | 19 +++++- .../dotty/tools/dotc/rewrites/Rewrites.scala | 3 + .../src/dotty/tools/dotc/typer/Typer.scala | 4 +- project/Dependencies.scala | 2 +- sbt-bridge/src/dotty/tools/xsbt/Action.java | 28 +++++++++ .../dotty/tools/xsbt/DelegatingReporter.java | 6 +- sbt-bridge/src/dotty/tools/xsbt/Problem.java | 39 +++++++++++- sbt-bridge/src/dotty/tools/xsbt/TextEdit.java | 23 +++++++ .../src/dotty/tools/xsbt/WorkspaceEdit.java | 20 ++++++ sbt-test/compilerReporter/simple/Source.scala | 5 +- .../simple/project/Reporter.scala | 61 ++++++++++++++----- 13 files changed, 201 insertions(+), 22 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/reporting/CodeAction.scala create mode 100644 sbt-bridge/src/dotty/tools/xsbt/Action.java create mode 100644 sbt-bridge/src/dotty/tools/xsbt/TextEdit.java create mode 100644 sbt-bridge/src/dotty/tools/xsbt/WorkspaceEdit.java diff --git a/compiler/src/dotty/tools/dotc/reporting/CodeAction.scala b/compiler/src/dotty/tools/dotc/reporting/CodeAction.scala new file mode 100644 index 000000000000..fc51bc038c5a --- /dev/null +++ b/compiler/src/dotty/tools/dotc/reporting/CodeAction.scala @@ -0,0 +1,9 @@ +package dotty.tools.dotc.reporting + +import dotty.tools.dotc.rewrites.Rewrites.ActionPatch + +case class CodeAction( + title: String, + description: java.util.Optional[String], + patches: java.util.List[ActionPatch] +) diff --git a/compiler/src/dotty/tools/dotc/reporting/Message.scala b/compiler/src/dotty/tools/dotc/reporting/Message.scala index a536c5871b2a..03db3661be12 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Message.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Message.scala @@ -10,7 +10,6 @@ import printing.Formatting.hl import config.SourceVersion import scala.language.unsafeNulls - import scala.annotation.threadUnsafe /** ## Tips for error message generation @@ -415,6 +414,9 @@ abstract class Message(val errorId: ErrorMessageID)(using Context) { self => */ def showAlways = false + def actions(using Context): java.util.List[CodeAction] = + java.util.Collections.emptyList + override def toString = msg } diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 970c44d54cbc..9adb5b2b930b 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -29,6 +29,9 @@ import transform.SymUtils._ import scala.util.matching.Regex import java.util.regex.Matcher.quoteReplacement import cc.CaptureSet.IdentityCaptRefMap +import dotty.tools.dotc.rewrites.Rewrites.ActionPatch +import dotty.tools.dotc.util.Spans.Span +import dotty.tools.dotc.util.SourcePosition /** Messages * ======== @@ -1846,12 +1849,26 @@ class FailureToEliminateExistential(tp: Type, tp1: Type, tp2: Type, boundSyms: L |are only approximated in a best-effort way.""" } -class OnlyFunctionsCanBeFollowedByUnderscore(tp: Type)(using Context) +class OnlyFunctionsCanBeFollowedByUnderscore(tp: Type, tree: untpd.PostfixOp)(using Context) extends SyntaxMsg(OnlyFunctionsCanBeFollowedByUnderscoreID) { def msg(using Context) = i"Only function types can be followed by ${hl("_")} but the current expression has type $tp" def explain(using Context) = i"""The syntax ${hl("x _")} is no longer supported if ${hl("x")} is not a function. |To convert to a function value, you need to explicitly write ${hl("() => x")}""" + + override def actions(using Context) = + val untpd.PostfixOp(qual, Ident(nme.WILDCARD)) = tree: @unchecked + import scala.language.unsafeNulls + import scala.jdk.CollectionConverters.* + List( + CodeAction(title = "Rewrite to function value", + description = java.util.Optional.empty(), + patches = List( + ActionPatch(SourcePosition(tree.source, Span(tree.span.start)), "(() => "), + ActionPatch(SourcePosition(tree.source, Span(qual.span.end, tree.span.end)), ")") + ).asJava + ) + ).asJava } class MissingEmptyArgumentList(method: String)(using Context) diff --git a/compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala b/compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala index f2dfac88d464..c5e4851bc3f3 100644 --- a/compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala +++ b/compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala @@ -7,6 +7,7 @@ import core.Contexts._ import collection.mutable import scala.annotation.tailrec import dotty.tools.dotc.reporting.Reporter +import dotty.tools.dotc.util.SourcePosition; import java.io.OutputStreamWriter import java.nio.charset.StandardCharsets.UTF_8 @@ -19,6 +20,8 @@ object Rewrites { def delta = replacement.length - (span.end - span.start) } + case class ActionPatch(srcPos: SourcePosition, replacement: String) + private class Patches(source: SourceFile) { private[Rewrites] val pbuf = new mutable.ListBuffer[Patch]() diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index cfb464e4fec8..0a5cbd271921 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2941,7 +2941,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer case closure(_, _, _) => case _ => val recovered = typed(qual)(using ctx.fresh.setExploreTyperState()) - report.errorOrMigrationWarning(OnlyFunctionsCanBeFollowedByUnderscore(recovered.tpe.widen), tree.srcPos, from = `3.0`) + report.errorOrMigrationWarning( + OnlyFunctionsCanBeFollowedByUnderscore(recovered.tpe.widen, tree), tree.srcPos, from = `3.0` + ) if (migrateTo3) { // Under -rewrite, patch `x _` to `(() => x)` patch(Span(tree.span.start), "(() => ") diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 54bc6ecadfe0..9e8e9477a0e2 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -28,6 +28,6 @@ object Dependencies { "com.vladsch.flexmark" % "flexmark-ext-yaml-front-matter" % flexmarkVersion, ) - val newCompilerInterface = "org.scala-sbt" % "compiler-interface" % "1.8.0" + val newCompilerInterface = "org.scala-sbt" % "compiler-interface" % "1.9.0-RC3" val oldCompilerInterface = "org.scala-sbt" % "compiler-interface" % "1.3.5" } diff --git a/sbt-bridge/src/dotty/tools/xsbt/Action.java b/sbt-bridge/src/dotty/tools/xsbt/Action.java new file mode 100644 index 000000000000..2a1818fef78c --- /dev/null +++ b/sbt-bridge/src/dotty/tools/xsbt/Action.java @@ -0,0 +1,28 @@ +package dotty.tools.xsbt; + +import java.util.Optional; + +final public class Action implements xsbti.Action { + private final String _title; + private final Optional _description; + private final WorkspaceEdit _edit; + + public Action(String title, Optional description, WorkspaceEdit edit) { + super(); + this._title = title; + this._description = description; + this._edit = edit; + } + + public String title() { + return _title; + } + + public Optional description() { + return _description; + } + + public WorkspaceEdit edit() { + return _edit; + } +} diff --git a/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java b/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java index 25b934000144..2d441a156f69 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java +++ b/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java @@ -3,11 +3,14 @@ */ package dotty.tools.xsbt; +import java.util.List; + import scala.Tuple2; import scala.collection.mutable.HashMap; import dotty.tools.dotc.core.Contexts.Context; import dotty.tools.dotc.reporting.AbstractReporter; +import dotty.tools.dotc.reporting.CodeAction; import dotty.tools.dotc.reporting.Diagnostic; import dotty.tools.dotc.reporting.Message; import dotty.tools.dotc.util.SourceFile; @@ -43,12 +46,13 @@ public void doReport(Diagnostic dia, Context ctx) { messageBuilder.append(message.message()); String diagnosticCode = String.valueOf(message.errorId().errorNumber()); boolean shouldExplain = Diagnostic.shouldExplain(dia, ctx); + List actions = message.actions(ctx); if (shouldExplain && !message.explanation().isEmpty()) { rendered.append(explanation(message, ctx)); messageBuilder.append(System.lineSeparator()).append(explanation(message, ctx)); } - delegate.log(new Problem(position, messageBuilder.toString(), severity, rendered.toString(), diagnosticCode)); + delegate.log(new Problem(position, messageBuilder.toString(), severity, rendered.toString(), diagnosticCode, actions)); } private static Severity severityOf(int level) { diff --git a/sbt-bridge/src/dotty/tools/xsbt/Problem.java b/sbt-bridge/src/dotty/tools/xsbt/Problem.java index 29d64cc26c4a..cf41e01801de 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/Problem.java +++ b/sbt-bridge/src/dotty/tools/xsbt/Problem.java @@ -1,6 +1,13 @@ package dotty.tools.xsbt; +import java.util.List; import java.util.Optional; +import static java.util.stream.Collectors.toList; + +import dotty.tools.dotc.reporting.CodeAction; +import dotty.tools.dotc.rewrites.Rewrites.ActionPatch; +import dotty.tools.dotc.util.SourcePosition; + import xsbti.Position; import xsbti.Severity; @@ -10,14 +17,16 @@ final public class Problem implements xsbti.Problem { private final Severity _severity; private final Optional _rendered; private final String _diagnosticCode; + private final List _actions; - public Problem(Position position, String message, Severity severity, String rendered, String diagnosticCode) { + public Problem(Position position, String message, Severity severity, String rendered, String diagnosticCode, List actions) { super(); this._position = position; this._message = message; this._severity = severity; this._rendered = Optional.of(rendered); this._diagnosticCode = diagnosticCode; + this._actions = actions; } public String category() { @@ -56,6 +65,34 @@ public Optional diagnosticCode() { } } + public List actions() { + if (_actions.isEmpty()) { + return java.util.Collections.emptyList(); + } else { + return _actions + .stream() + .map(action -> new Action(action.title(), action.description(), toWorkspaceEdit(action.patches()))) + .collect(toList()); + } + } + + private static WorkspaceEdit toWorkspaceEdit(List patches) { + return new WorkspaceEdit( + patches + .stream() + .map(patch -> new TextEdit(positionOf(patch.srcPos()), patch.replacement())) + .collect(toList()) + ); + } + + private static Position positionOf(SourcePosition pos) { + if (pos.exists()){ + return new PositionBridge(pos, pos.source()); + } else { + return PositionBridge.noPosition; + } + } + @Override public String toString() { return "Problem(" + _position + ", " + _message + ", " + _severity + ", " + _rendered + ", " + _diagnosticCode + ")"; diff --git a/sbt-bridge/src/dotty/tools/xsbt/TextEdit.java b/sbt-bridge/src/dotty/tools/xsbt/TextEdit.java new file mode 100644 index 000000000000..df717446b2f2 --- /dev/null +++ b/sbt-bridge/src/dotty/tools/xsbt/TextEdit.java @@ -0,0 +1,23 @@ +package dotty.tools.xsbt; + +import xsbti.Position; + +final public class TextEdit implements xsbti.TextEdit { + private final Position _position; + private final String _newText; + + public TextEdit(Position position, String newText) { + super(); + this._position = position; + this._newText = newText; + } + + public Position position() { + return _position; + } + + public String newText() { + return _newText; + } + +} diff --git a/sbt-bridge/src/dotty/tools/xsbt/WorkspaceEdit.java b/sbt-bridge/src/dotty/tools/xsbt/WorkspaceEdit.java new file mode 100644 index 000000000000..153de63e3765 --- /dev/null +++ b/sbt-bridge/src/dotty/tools/xsbt/WorkspaceEdit.java @@ -0,0 +1,20 @@ +package dotty.tools.xsbt; + +import java.util.List; + +import xsbti.TextEdit; + +final public class WorkspaceEdit implements xsbti.WorkspaceEdit { + + private final List _changes; + + public WorkspaceEdit(List changes) { + super(); + this._changes = changes; + } + + public List changes() { + return _changes; + } + +} diff --git a/sbt-test/compilerReporter/simple/Source.scala b/sbt-test/compilerReporter/simple/Source.scala index 6f06785990c3..fcfd8672475b 100644 --- a/sbt-test/compilerReporter/simple/Source.scala +++ b/sbt-test/compilerReporter/simple/Source.scala @@ -7,4 +7,7 @@ trait Wr { object Er { val a = er1 -} \ No newline at end of file + + def f: Int = 1 + val x = f _ +} diff --git a/sbt-test/compilerReporter/simple/project/Reporter.scala b/sbt-test/compilerReporter/simple/project/Reporter.scala index 6c3b60cebb3a..70805cf0c359 100644 --- a/sbt-test/compilerReporter/simple/project/Reporter.scala +++ b/sbt-test/compilerReporter/simple/project/Reporter.scala @@ -27,27 +27,58 @@ object Reporter { check := (Compile / compile).failure.map(_ => { val problems = reporter.problems println(problems.toList) - assert(problems.size == 1) - // make sure position reported by zinc are proper - val mainProblem = problems.head + problems match { + case Array(err, warning) => + // Checking the error reported + val eline = err.position().line() + assert(eline.isPresent() == true) + assert(eline.get() == 9) - val line = mainProblem.position().line() - assert(line.isPresent() == true) - assert(line.get() == 9) + val ediagnosticCode = err.diagnosticCode() + assert(ediagnosticCode.isPresent() == true) + val ecode = ediagnosticCode.get().code() + assert(ecode == "6") - val diagnosticCode = mainProblem.diagnosticCode() - assert(diagnosticCode.isPresent() == true) - val code = diagnosticCode.get() - assert(diagnosticCode.get().code() == "6") + val epointer = err.position().pointer() + assert(epointer.isPresent() == true) + assert(epointer.get() == 10) - val pointer = mainProblem.position().pointer() - assert(pointer.isPresent() == true) - assert(pointer.get() == 10) + assert(err.position.offset.isPresent) - assert(problems.forall(_.position.offset.isPresent)) + assert(err.severity == Severity.Error) // not found: er1, - assert(problems.count(_.severity == Severity.Error) == 1) // not found: er1, + // Checking the warning reported + + val wline = warning.position().line() + assert(wline.isPresent() == true) + assert(wline.get() == 12) + + val wdiagnosticCode = warning.diagnosticCode() + assert(wdiagnosticCode.isPresent() == true) + val wcode = wdiagnosticCode.get().code() + assert(wcode == "99") + + val wpointer = warning.position().pointer() + assert(wpointer.isPresent() == true) + assert(wpointer.get() == 12) + + assert(warning.position.offset.isPresent) + + assert(warning.severity == Severity.Warn) // Only function types can be followed by _ but the current expression has type Int + + //val actions = warning.actions() + + //assert(actions.size == 1) + + //val action = actions.head + + //assert(action.title() == "wrong") + + case somethingElse => + assert(false, s"Only expected to have a single error and a single warning, but instead got: ${somethingElse.toString}") + + } }).value ) } From 56d4885b1df5e976ea8e92df7e875f0934fe4122 Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Tue, 23 May 2023 13:56:03 +0200 Subject: [PATCH 02/12] dep: bump sbt to 1.9.0-RC3 for tests --- .../src/scala/dotty/communitybuild/projects.scala | 2 +- project/build.properties | 2 +- .../compilerReporter/simple/project/Reporter.scala | 14 ++++++++++---- .../project/build.properties | 2 +- .../project/build.properties | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/community-build/src/scala/dotty/communitybuild/projects.scala b/community-build/src/scala/dotty/communitybuild/projects.scala index 1349c3adc3b9..caaee008b0cc 100644 --- a/community-build/src/scala/dotty/communitybuild/projects.scala +++ b/community-build/src/scala/dotty/communitybuild/projects.scala @@ -140,7 +140,7 @@ final case class SbtCommunityProject( case Some(ivyHome) => List(s"-Dsbt.ivy.home=$ivyHome") case _ => Nil extraSbtArgs ++ sbtProps ++ List( - "-sbt-version", "1.8.2", + "-sbt-version", "1.9.0-RC3", "-Dsbt.supershell=false", s"-Ddotty.communitybuild.dir=$communitybuildDir", s"--addPluginSbtFile=$sbtPluginFilePath" diff --git a/project/build.properties b/project/build.properties index 46e43a97ed86..b71cb664803a 100644 --- a/project/build.properties +++ b/project/build.properties @@ -1 +1 @@ -sbt.version=1.8.2 +sbt.version=1.9.0-RC3 diff --git a/sbt-test/compilerReporter/simple/project/Reporter.scala b/sbt-test/compilerReporter/simple/project/Reporter.scala index 70805cf0c359..a22b5cfb904d 100644 --- a/sbt-test/compilerReporter/simple/project/Reporter.scala +++ b/sbt-test/compilerReporter/simple/project/Reporter.scala @@ -2,6 +2,8 @@ import sbt._ import Keys._ import KeyRanks.DTask +import scala.jdk.CollectionConverters.* + object Reporter { import xsbti.{Reporter, Problem, Position, Severity} @@ -67,13 +69,17 @@ object Reporter { assert(warning.severity == Severity.Warn) // Only function types can be followed by _ but the current expression has type Int - //val actions = warning.actions() + val actions = warning.actions().asScala.toList + + assert(actions.size == 1) + + val action = actions.head - //assert(actions.size == 1) + assert(action.title() == "Rewrite to function value") - //val action = actions.head + val edits = action.edit().changes().asScala.toList - //assert(action.title() == "wrong") + assert(edits.size == 2) case somethingElse => assert(false, s"Only expected to have a single error and a single warning, but instead got: ${somethingElse.toString}") diff --git a/tests/cmdTest-sbt-tests/sourcepath-with-inline-api-hash/project/build.properties b/tests/cmdTest-sbt-tests/sourcepath-with-inline-api-hash/project/build.properties index 46e43a97ed86..b71cb664803a 100644 --- a/tests/cmdTest-sbt-tests/sourcepath-with-inline-api-hash/project/build.properties +++ b/tests/cmdTest-sbt-tests/sourcepath-with-inline-api-hash/project/build.properties @@ -1 +1 @@ -sbt.version=1.8.2 +sbt.version=1.9.0-RC3 diff --git a/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/build.properties b/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/build.properties index 46e43a97ed86..b71cb664803a 100644 --- a/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/build.properties +++ b/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/build.properties @@ -1 +1 @@ -sbt.version=1.8.2 +sbt.version=1.9.0-RC3 From 38567468e1c1237fe33329b6f47a15e829bd95d4 Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Tue, 23 May 2023 14:06:26 +0200 Subject: [PATCH 03/12] docs: add in a few docs in the newly added classes --- compiler/src/dotty/tools/dotc/reporting/CodeAction.scala | 7 +++++++ compiler/src/dotty/tools/dotc/reporting/Message.scala | 3 +++ compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala | 8 ++++++++ sbt-bridge/src/dotty/tools/xsbt/Problem.java | 4 ++++ 4 files changed, 22 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/reporting/CodeAction.scala b/compiler/src/dotty/tools/dotc/reporting/CodeAction.scala index fc51bc038c5a..df3faa64516a 100644 --- a/compiler/src/dotty/tools/dotc/reporting/CodeAction.scala +++ b/compiler/src/dotty/tools/dotc/reporting/CodeAction.scala @@ -2,6 +2,13 @@ package dotty.tools.dotc.reporting import dotty.tools.dotc.rewrites.Rewrites.ActionPatch +/** A representation of a code action / fix that can be used by tooling to + * apply a fix to their code. + * + * @param title The title of the fix, often showed to a user in their editor. + * @param description An optional description of the fix. + * @param patches The patches that this fix contains. + */ case class CodeAction( title: String, description: java.util.Optional[String], diff --git a/compiler/src/dotty/tools/dotc/reporting/Message.scala b/compiler/src/dotty/tools/dotc/reporting/Message.scala index 03db3661be12..bb445beee38f 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Message.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Message.scala @@ -414,6 +414,9 @@ abstract class Message(val errorId: ErrorMessageID)(using Context) { self => */ def showAlways = false + /** A list of actions attatched to this message to address the issue this + * message represents. + */ def actions(using Context): java.util.List[CodeAction] = java.util.Collections.emptyList diff --git a/compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala b/compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala index c5e4851bc3f3..9b6b115a7b17 100644 --- a/compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala +++ b/compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala @@ -20,6 +20,14 @@ object Rewrites { def delta = replacement.length - (span.end - span.start) } + /** A special type of Patch that instead of just a span, contains the + * full SourcePosition. This is useful when being used by + * [[dotty.tools.dotc.reporting.CodeAction]] or if the patch doesn't + * belong to the same file that the actual issue it's addressing is in. + * + * @param srcPos The SourcePosition of the patch. + * @param replacement The Replacement that should go in that position. + */ case class ActionPatch(srcPos: SourcePosition, replacement: String) private class Patches(source: SourceFile) { diff --git a/sbt-bridge/src/dotty/tools/xsbt/Problem.java b/sbt-bridge/src/dotty/tools/xsbt/Problem.java index cf41e01801de..5200c1af154c 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/Problem.java +++ b/sbt-bridge/src/dotty/tools/xsbt/Problem.java @@ -69,6 +69,10 @@ public List actions() { if (_actions.isEmpty()) { return java.util.Collections.emptyList(); } else { + // Same as with diagnosticCode, we need to ensure we don't create the actual + // Action until we are here to ensure that when using an older version of sbt/zinc + // with the new versions of the compiler, this doesn't blow up because this is + // never getting called. return _actions .stream() .map(action -> new Action(action.title(), action.description(), toWorkspaceEdit(action.patches()))) From df434daeaba61189f8b18003e91db69bd0be4c74 Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Tue, 23 May 2023 17:12:09 +0200 Subject: [PATCH 04/12] test: add in a CodeActionTest suite --- .../tools/dotc/reporting/CodeActionTest.scala | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala new file mode 100644 index 000000000000..24c20fc5ad50 --- /dev/null +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -0,0 +1,83 @@ +package dotty.tools.dotc.reporting + +import dotty.tools.DottyTest +import dotty.tools.dotc.rewrites.Rewrites +import dotty.tools.dotc.rewrites.Rewrites.ActionPatch +import dotty.tools.dotc.util.SourceFile + +import scala.annotation.tailrec +import scala.jdk.CollectionConverters.* +import scala.runtime.Scala3RunTime.assertFailed + +import org.junit.Assert._ +import org.junit.Test + +/** This is a test suite that is meant to test the actions attached to the + * diagnostic for a given code snippet. + */ +class CodeActionTest extends DottyTest: + + @Test def convertToFunctionValue = + checkCodeAction( + """|object Test: + | def x: Int = 3 + | val test = x _ + |""".stripMargin, + "Rewrite to function value", + """|object Test: + | def x: Int = 3 + | val test = (() => x) + |""".stripMargin + ) + + // Make sure we're not using the default reporter, which is the ConsoleReporter, + // meaning they will get reported in the test run and that's it. + private def newContext = + val rep = new StoreReporter(null) with UniqueMessagePositions with HideNonSensicalMessages + initialCtx.setReporter(rep).withoutColors + + private def checkCodeAction(code: String, title: String, expected: String) = + ctx = newContext + val source = SourceFile.virtual("test", code).content + val runCtx = checkCompile("typer", code) { (_, _) => () } + val diagnostics = runCtx.reporter.removeBufferedMessages + assertEquals(diagnostics.size, 1) + + val diagnostic = diagnostics.head + val actions = diagnostic.msg.actions.asScala.toList + assertEquals(actions.size, 1) + + // TODO account for more than 1 action + val action = actions.head + assertEquals(action.title, title) + val patches = action.patches.asScala.toList + if patches.nonEmpty then + patches.reduceLeft: (p1, p2) => + assert(p1.srcPos.span.end <= p2.srcPos.span.start, s"overlapping patches $p1 and $p2") + p2 + else assertFailed("Expected a patch attatched to this action, but it was empty") + + val delta = patches + .map: patch => + patch.replacement.length - (patch.srcPos.end - patch.srcPos.start) + .sum + + val result = new Array[Char](source.length + delta) + + @tailrec def loop(ps: List[ActionPatch], inIdx: Int, outIdx: Int): Unit = + def copy(upTo: Int): Int = + val untouched = upTo - inIdx + System.arraycopy(source, inIdx, result, outIdx, untouched) + outIdx + untouched + + ps match + case patch @ ActionPatch(srcPos, replacement) :: ps1 => + val outNew = copy(srcPos.start) + replacement.copyToArray(result, outNew) + loop(ps1, srcPos.end, outNew + replacement.length) + case Nil => + val outNew = copy(source.length) + assert(outNew == result.length, s"$outNew != ${result.length}") + + loop(patches, 0, 0) + assertEquals(result.mkString, expected) From d046e2f234f87939af935074a375cc4ff31c6a16 Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Tue, 23 May 2023 21:26:36 +0200 Subject: [PATCH 05/12] feat: add in an action for missing empty argument list --- .../dotty/tools/dotc/reporting/messages.scala | 15 +++++++++++--- .../tools/dotc/typer/ErrorReporting.scala | 2 +- .../src/dotty/tools/dotc/typer/Typer.scala | 2 +- .../tools/dotc/reporting/CodeActionTest.scala | 20 ++++++++++++++++--- 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 9adb5b2b930b..3a0bee78d0c1 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -32,6 +32,7 @@ import cc.CaptureSet.IdentityCaptRefMap import dotty.tools.dotc.rewrites.Rewrites.ActionPatch import dotty.tools.dotc.util.Spans.Span import dotty.tools.dotc.util.SourcePosition +import scala.jdk.CollectionConverters.* /** Messages * ======== @@ -1858,8 +1859,6 @@ class OnlyFunctionsCanBeFollowedByUnderscore(tp: Type, tree: untpd.PostfixOp)(us override def actions(using Context) = val untpd.PostfixOp(qual, Ident(nme.WILDCARD)) = tree: @unchecked - import scala.language.unsafeNulls - import scala.jdk.CollectionConverters.* List( CodeAction(title = "Rewrite to function value", description = java.util.Optional.empty(), @@ -1871,7 +1870,7 @@ class OnlyFunctionsCanBeFollowedByUnderscore(tp: Type, tree: untpd.PostfixOp)(us ).asJava } -class MissingEmptyArgumentList(method: String)(using Context) +class MissingEmptyArgumentList(method: String, tree: tpd.Tree)(using Context) extends SyntaxMsg(MissingEmptyArgumentListID) { def msg(using Context) = i"$method must be called with ${hl("()")} argument" def explain(using Context) = { @@ -1886,6 +1885,16 @@ class MissingEmptyArgumentList(method: String)(using Context) |In Dotty, this idiom is an error. The application syntax has to follow exactly the parameter syntax. |Excluded from this rule are methods that are defined in Java or that override methods defined in Java.""" } + + override def actions(using Context) = + List( + CodeAction(title = "Insert ()", + description = java.util.Optional.empty(), + patches = List( + ActionPatch(SourcePosition(tree.source, tree.span.endPos), "()"), + ).asJava + ) + ).asJava } class DuplicateNamedTypeParameter(name: Name)(using Context) diff --git a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala index 126d109889e1..25cbfdfec600 100644 --- a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala +++ b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala @@ -55,7 +55,7 @@ object ErrorReporting { val meth = err.exprStr(methPart(tree)) val info = if tree.symbol.exists then tree.symbol.info else mt if isCallableWithSingleEmptyArgumentList(info) then - report.error(MissingEmptyArgumentList(meth), tree.srcPos) + report.error(MissingEmptyArgumentList(meth, tree), tree.srcPos) else report.error(MissingArgumentList(meth, tree.symbol), tree.srcPos) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 0a5cbd271921..2e284a3d32ff 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3953,7 +3953,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer def isAutoApplied(sym: Symbol): Boolean = sym.isConstructor || sym.matchNullaryLoosely - || Feature.warnOnMigration(MissingEmptyArgumentList(sym.show), tree.srcPos, version = `3.0`) + || Feature.warnOnMigration(MissingEmptyArgumentList(sym.show, tree), tree.srcPos, version = `3.0`) && { patch(tree.span.endPos, "()"); true } /** If this is a selection prototype of the form `.apply(...): R`, return the nested diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index 24c20fc5ad50..300dbe54e3cd 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -30,6 +30,20 @@ class CodeActionTest extends DottyTest: |""".stripMargin ) + @Test def insertBracesForEmptyArgument = + checkCodeAction( + """|object Test: + | def foo(): Unit = () + | val x = foo + |""".stripMargin, + "Insert ()", + """|object Test: + | def foo(): Unit = () + | val x = foo() + |""".stripMargin + + ) + // Make sure we're not using the default reporter, which is the ConsoleReporter, // meaning they will get reported in the test run and that's it. private def newContext = @@ -41,11 +55,11 @@ class CodeActionTest extends DottyTest: val source = SourceFile.virtual("test", code).content val runCtx = checkCompile("typer", code) { (_, _) => () } val diagnostics = runCtx.reporter.removeBufferedMessages - assertEquals(diagnostics.size, 1) + assertEquals(1, diagnostics.size) val diagnostic = diagnostics.head val actions = diagnostic.msg.actions.asScala.toList - assertEquals(actions.size, 1) + assertEquals(1, actions.size) // TODO account for more than 1 action val action = actions.head @@ -80,4 +94,4 @@ class CodeActionTest extends DottyTest: assert(outNew == result.length, s"$outNew != ${result.length}") loop(patches, 0, 0) - assertEquals(result.mkString, expected) + assertEquals(expected, result.mkString) From 1618a0b46fdd4021e91b887b00ec3516acbb887c Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Tue, 23 May 2023 21:52:19 +0200 Subject: [PATCH 06/12] feat: add in an action to remove duplicated modifiers --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 3 +-- .../src/dotty/tools/dotc/reporting/messages.scala | 13 ++++++++++++- .../dotty/tools/dotc/reporting/CodeActionTest.scala | 11 +++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index d5dce6dc5212..cec7a72f3e8e 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -3059,8 +3059,7 @@ object Parsers { val mod = atSpan(in.skipToken()) { modOfToken(tok, name) } if mods.isOneOf(mod.flags) then - syntaxError(RepeatedModifier(mod.flags.flagsString), mod.span) - + syntaxError(RepeatedModifier(mod.flags.flagsString, source, mod.span), mod.span) addMod(mods, mod) } diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 3a0bee78d0c1..6e59ab0cd41a 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -33,6 +33,7 @@ import dotty.tools.dotc.rewrites.Rewrites.ActionPatch import dotty.tools.dotc.util.Spans.Span import dotty.tools.dotc.util.SourcePosition import scala.jdk.CollectionConverters.* +import dotty.tools.dotc.util.SourceFile /** Messages * ======== @@ -497,7 +498,7 @@ extends SyntaxMsg(ObjectMayNotHaveSelfTypeID) { } } -class RepeatedModifier(modifier: String)(implicit ctx:Context) +class RepeatedModifier(modifier: String, source: SourceFile, span: Span)(implicit ctx:Context) extends SyntaxMsg(RepeatedModifierID) { def msg(using Context) = i"""Repeated modifier $modifier""" @@ -516,6 +517,16 @@ extends SyntaxMsg(RepeatedModifierID) { | |""" } + + override def actions(using Context) = + List( + CodeAction(title = s"""Remove repeated modifier: "$modifier"""", + description = java.util.Optional.empty(), + patches = List( + ActionPatch(SourcePosition(source, span), "") + ).asJava + ) + ).asJava } class InterpolatedStringError()(implicit ctx:Context) diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index 300dbe54e3cd..ef2eff6d6f18 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -44,6 +44,17 @@ class CodeActionTest extends DottyTest: ) + @Test def removeRepeatModifier = + checkCodeAction( + """|final final class Test + |""".stripMargin, + """Remove repeated modifier: "final"""", + // TODO look into trying to remove the extra space that is left behind + """|final class Test + |""".stripMargin + + ) + // Make sure we're not using the default reporter, which is the ConsoleReporter, // meaning they will get reported in the test run and that's it. private def newContext = From 75f2db92cdf74a4fd7fa1cb0e7c2e41eacd699e1 Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Tue, 23 May 2023 21:59:46 +0200 Subject: [PATCH 07/12] fix: add unsafeNull imports back in --- compiler/src/dotty/tools/dotc/reporting/messages.scala | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 6e59ab0cd41a..2e7a8173793d 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -519,6 +519,7 @@ extends SyntaxMsg(RepeatedModifierID) { } override def actions(using Context) = + import scala.language.unsafeNulls List( CodeAction(title = s"""Remove repeated modifier: "$modifier"""", description = java.util.Optional.empty(), @@ -1869,6 +1870,7 @@ class OnlyFunctionsCanBeFollowedByUnderscore(tp: Type, tree: untpd.PostfixOp)(us |To convert to a function value, you need to explicitly write ${hl("() => x")}""" override def actions(using Context) = + import scala.language.unsafeNulls val untpd.PostfixOp(qual, Ident(nme.WILDCARD)) = tree: @unchecked List( CodeAction(title = "Rewrite to function value", @@ -1898,6 +1900,7 @@ class MissingEmptyArgumentList(method: String, tree: tpd.Tree)(using Context) } override def actions(using Context) = + import scala.language.unsafeNulls List( CodeAction(title = "Insert ()", description = java.util.Optional.empty(), From a9911494856ff8cf1e76833781b5a7780bc02c85 Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Tue, 6 Jun 2023 13:03:30 +0200 Subject: [PATCH 08/12] dep: bump sbt to 1.9.0 --- community-build/src/scala/dotty/communitybuild/projects.scala | 2 +- project/Dependencies.scala | 2 +- project/build.properties | 2 +- .../sourcepath-with-inline-api-hash/project/build.properties | 2 +- .../sourcepath-with-inline/project/build.properties | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/community-build/src/scala/dotty/communitybuild/projects.scala b/community-build/src/scala/dotty/communitybuild/projects.scala index caaee008b0cc..0ba5d32a78a8 100644 --- a/community-build/src/scala/dotty/communitybuild/projects.scala +++ b/community-build/src/scala/dotty/communitybuild/projects.scala @@ -140,7 +140,7 @@ final case class SbtCommunityProject( case Some(ivyHome) => List(s"-Dsbt.ivy.home=$ivyHome") case _ => Nil extraSbtArgs ++ sbtProps ++ List( - "-sbt-version", "1.9.0-RC3", + "-sbt-version", "1.9.0", "-Dsbt.supershell=false", s"-Ddotty.communitybuild.dir=$communitybuildDir", s"--addPluginSbtFile=$sbtPluginFilePath" diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 9e8e9477a0e2..f22601346803 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -28,6 +28,6 @@ object Dependencies { "com.vladsch.flexmark" % "flexmark-ext-yaml-front-matter" % flexmarkVersion, ) - val newCompilerInterface = "org.scala-sbt" % "compiler-interface" % "1.9.0-RC3" + val newCompilerInterface = "org.scala-sbt" % "compiler-interface" % "1.9.0" val oldCompilerInterface = "org.scala-sbt" % "compiler-interface" % "1.3.5" } diff --git a/project/build.properties b/project/build.properties index b71cb664803a..40b3b8e7b655 100644 --- a/project/build.properties +++ b/project/build.properties @@ -1 +1 @@ -sbt.version=1.9.0-RC3 +sbt.version=1.9.0 diff --git a/tests/cmdTest-sbt-tests/sourcepath-with-inline-api-hash/project/build.properties b/tests/cmdTest-sbt-tests/sourcepath-with-inline-api-hash/project/build.properties index b71cb664803a..40b3b8e7b655 100644 --- a/tests/cmdTest-sbt-tests/sourcepath-with-inline-api-hash/project/build.properties +++ b/tests/cmdTest-sbt-tests/sourcepath-with-inline-api-hash/project/build.properties @@ -1 +1 @@ -sbt.version=1.9.0-RC3 +sbt.version=1.9.0 diff --git a/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/build.properties b/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/build.properties index b71cb664803a..40b3b8e7b655 100644 --- a/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/build.properties +++ b/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/build.properties @@ -1 +1 @@ -sbt.version=1.9.0-RC3 +sbt.version=1.9.0 From f1a16f4b001905ce0558f8472ff45b4abbeb3120 Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Thu, 22 Jun 2023 14:06:22 +0200 Subject: [PATCH 09/12] refactor: use scala types --- .../tools/dotc/reporting/CodeAction.scala | 4 ++-- .../dotty/tools/dotc/reporting/Message.scala | 5 ++--- .../dotty/tools/dotc/reporting/messages.scala | 18 +++++++++--------- .../tools/dotc/reporting/CodeActionTest.scala | 4 ++-- .../dotty/tools/xsbt/DelegatingReporter.java | 3 ++- sbt-bridge/src/dotty/tools/xsbt/Problem.java | 5 ++++- 6 files changed, 21 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/CodeAction.scala b/compiler/src/dotty/tools/dotc/reporting/CodeAction.scala index df3faa64516a..b2b18cc00104 100644 --- a/compiler/src/dotty/tools/dotc/reporting/CodeAction.scala +++ b/compiler/src/dotty/tools/dotc/reporting/CodeAction.scala @@ -11,6 +11,6 @@ import dotty.tools.dotc.rewrites.Rewrites.ActionPatch */ case class CodeAction( title: String, - description: java.util.Optional[String], - patches: java.util.List[ActionPatch] + description: Option[String], + patches: List[ActionPatch] ) diff --git a/compiler/src/dotty/tools/dotc/reporting/Message.scala b/compiler/src/dotty/tools/dotc/reporting/Message.scala index bb445beee38f..2ef839b5b402 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Message.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Message.scala @@ -414,11 +414,10 @@ abstract class Message(val errorId: ErrorMessageID)(using Context) { self => */ def showAlways = false - /** A list of actions attatched to this message to address the issue this + /** A list of actions attached to this message to address the issue this * message represents. */ - def actions(using Context): java.util.List[CodeAction] = - java.util.Collections.emptyList + def actions(using Context): List[CodeAction] = List.empty override def toString = msg } diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 2e7a8173793d..094568ae5de5 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -522,12 +522,12 @@ extends SyntaxMsg(RepeatedModifierID) { import scala.language.unsafeNulls List( CodeAction(title = s"""Remove repeated modifier: "$modifier"""", - description = java.util.Optional.empty(), + description = None, patches = List( ActionPatch(SourcePosition(source, span), "") - ).asJava + ) ) - ).asJava + ) } class InterpolatedStringError()(implicit ctx:Context) @@ -1874,13 +1874,13 @@ class OnlyFunctionsCanBeFollowedByUnderscore(tp: Type, tree: untpd.PostfixOp)(us val untpd.PostfixOp(qual, Ident(nme.WILDCARD)) = tree: @unchecked List( CodeAction(title = "Rewrite to function value", - description = java.util.Optional.empty(), + description = None, patches = List( ActionPatch(SourcePosition(tree.source, Span(tree.span.start)), "(() => "), ActionPatch(SourcePosition(tree.source, Span(qual.span.end, tree.span.end)), ")") - ).asJava + ) ) - ).asJava + ) } class MissingEmptyArgumentList(method: String, tree: tpd.Tree)(using Context) @@ -1903,12 +1903,12 @@ class MissingEmptyArgumentList(method: String, tree: tpd.Tree)(using Context) import scala.language.unsafeNulls List( CodeAction(title = "Insert ()", - description = java.util.Optional.empty(), + description = None, patches = List( ActionPatch(SourcePosition(tree.source, tree.span.endPos), "()"), - ).asJava + ) ) - ).asJava + ) } class DuplicateNamedTypeParameter(name: Name)(using Context) diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index ef2eff6d6f18..d6261ca682bd 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -69,13 +69,13 @@ class CodeActionTest extends DottyTest: assertEquals(1, diagnostics.size) val diagnostic = diagnostics.head - val actions = diagnostic.msg.actions.asScala.toList + val actions = diagnostic.msg.actions.toList assertEquals(1, actions.size) // TODO account for more than 1 action val action = actions.head assertEquals(action.title, title) - val patches = action.patches.asScala.toList + val patches = action.patches.toList if patches.nonEmpty then patches.reduceLeft: (p1, p2) => assert(p1.srcPos.span.end <= p2.srcPos.span.start, s"overlapping patches $p1 and $p2") diff --git a/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java b/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java index 2d441a156f69..dba1889b393f 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java +++ b/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java @@ -7,6 +7,7 @@ import scala.Tuple2; import scala.collection.mutable.HashMap; +import scala.jdk.javaapi.CollectionConverters; import dotty.tools.dotc.core.Contexts.Context; import dotty.tools.dotc.reporting.AbstractReporter; @@ -46,7 +47,7 @@ public void doReport(Diagnostic dia, Context ctx) { messageBuilder.append(message.message()); String diagnosticCode = String.valueOf(message.errorId().errorNumber()); boolean shouldExplain = Diagnostic.shouldExplain(dia, ctx); - List actions = message.actions(ctx); + List actions = CollectionConverters.asJava(message.actions(ctx)); if (shouldExplain && !message.explanation().isEmpty()) { rendered.append(explanation(message, ctx)); messageBuilder.append(System.lineSeparator()).append(explanation(message, ctx)); diff --git a/sbt-bridge/src/dotty/tools/xsbt/Problem.java b/sbt-bridge/src/dotty/tools/xsbt/Problem.java index 5200c1af154c..9bde5ce76ebb 100644 --- a/sbt-bridge/src/dotty/tools/xsbt/Problem.java +++ b/sbt-bridge/src/dotty/tools/xsbt/Problem.java @@ -8,6 +8,9 @@ import dotty.tools.dotc.rewrites.Rewrites.ActionPatch; import dotty.tools.dotc.util.SourcePosition; +import scala.jdk.javaapi.CollectionConverters; +import scala.jdk.javaapi.OptionConverters; + import xsbti.Position; import xsbti.Severity; @@ -75,7 +78,7 @@ public List actions() { // never getting called. return _actions .stream() - .map(action -> new Action(action.title(), action.description(), toWorkspaceEdit(action.patches()))) + .map(action -> new Action(action.title(), OptionConverters.toJava(action.description()), toWorkspaceEdit(CollectionConverters.asJava(action.patches())))) .collect(toList()); } } From 6af13b344c16bcf9f258fb0a6f6990a9571edbaa Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Thu, 22 Jun 2023 14:37:45 +0200 Subject: [PATCH 10/12] refactor: make sure the patch logic isn't duplicated --- .../src/dotty/tools/dotc/typer/Typer.scala | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 2e284a3d32ff..6aaced118ac8 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2941,13 +2941,13 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer case closure(_, _, _) => case _ => val recovered = typed(qual)(using ctx.fresh.setExploreTyperState()) - report.errorOrMigrationWarning( - OnlyFunctionsCanBeFollowedByUnderscore(recovered.tpe.widen, tree), tree.srcPos, from = `3.0` - ) + val msg = OnlyFunctionsCanBeFollowedByUnderscore(recovered.tpe.widen, tree) + report.errorOrMigrationWarning(msg, tree.srcPos, from = `3.0`) if (migrateTo3) { // Under -rewrite, patch `x _` to `(() => x)` - patch(Span(tree.span.start), "(() => ") - patch(Span(qual.span.end, tree.span.end), ")") + msg.actions + .flatMap(_.patches) + .map(actionPatch => patch(actionPatch.srcPos.span, actionPatch.replacement)) return typed(untpd.Function(Nil, qual), pt) } } @@ -3951,10 +3951,17 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer def adaptNoArgsUnappliedMethod(wtp: MethodType, functionExpected: Boolean, arity: Int): Tree = { /** Is reference to this symbol `f` automatically expanded to `f()`? */ def isAutoApplied(sym: Symbol): Boolean = + lazy val msg = MissingEmptyArgumentList(sym.show, tree) + sym.isConstructor || sym.matchNullaryLoosely - || Feature.warnOnMigration(MissingEmptyArgumentList(sym.show, tree), tree.srcPos, version = `3.0`) - && { patch(tree.span.endPos, "()"); true } + || Feature.warnOnMigration(msg, tree.srcPos, version = `3.0`) + && { + msg.actions + .flatMap(_.patches) + .map(actionPatch => patch(actionPatch.srcPos.span, actionPatch.replacement)) + true + } /** If this is a selection prototype of the form `.apply(...): R`, return the nested * function prototype `(...)R`. Otherwise `pt`. From 6c9b967c40d7e0157b0dc1162bb5c269f2d5ded6 Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Thu, 22 Jun 2023 16:20:08 +0200 Subject: [PATCH 11/12] refactor(test): simplify the code action testing Instead of keeping track of the delta, we simply reverse the patches and apply them. This shouldn't be problematic because we do a check beforehand ensuring that there aren't overlapping patches. This greatly simplifies the way these are applied for the tests while keeping the same behavior. --- .../tools/dotc/reporting/CodeActionTest.scala | 29 ++++--------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index d6261ca682bd..1740542b46b0 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -82,27 +82,10 @@ class CodeActionTest extends DottyTest: p2 else assertFailed("Expected a patch attatched to this action, but it was empty") - val delta = patches - .map: patch => - patch.replacement.length - (patch.srcPos.end - patch.srcPos.start) - .sum + val result = patches.reverse.foldLeft(code): (newCode, patch) => + import scala.language.unsafeNulls + val start = newCode.substring(0, patch.srcPos.start) + val ending = newCode.substring(patch.srcPos.end, newCode.length) + start + patch.replacement + ending - val result = new Array[Char](source.length + delta) - - @tailrec def loop(ps: List[ActionPatch], inIdx: Int, outIdx: Int): Unit = - def copy(upTo: Int): Int = - val untouched = upTo - inIdx - System.arraycopy(source, inIdx, result, outIdx, untouched) - outIdx + untouched - - ps match - case patch @ ActionPatch(srcPos, replacement) :: ps1 => - val outNew = copy(srcPos.start) - replacement.copyToArray(result, outNew) - loop(ps1, srcPos.end, outNew + replacement.length) - case Nil => - val outNew = copy(source.length) - assert(outNew == result.length, s"$outNew != ${result.length}") - - loop(patches, 0, 0) - assertEquals(expected, result.mkString) + assertEquals(expected, result) From 079a1bfc3fdd08263c891da3b0b5103eaae77b0b Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Wed, 5 Jul 2023 13:22:00 +0200 Subject: [PATCH 12/12] refactor: add in a `Rewrites.applyAction` helper This also has a small change where instead of applying all the actions if we are doing a rewrite, we only actually take the first one --- compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala | 9 +++++++++ compiler/src/dotty/tools/dotc/typer/Typer.scala | 9 +++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala b/compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala index 9b6b115a7b17..5bea0fb66ed0 100644 --- a/compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala +++ b/compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala @@ -11,6 +11,7 @@ import dotty.tools.dotc.util.SourcePosition; import java.io.OutputStreamWriter import java.nio.charset.StandardCharsets.UTF_8 +import dotty.tools.dotc.reporting.CodeAction /** Handles rewriting of Scala2 files to Dotty */ object Rewrites { @@ -99,6 +100,14 @@ object Rewrites { report.echo(s"[patched file ${source.file.path}]") rewrites.patched(source).writeBack() } + + /** Given a CodeAction take the patches and apply them. + * + * @param action The CodeAction containing the patches + */ + def applyAction(action: CodeAction)(using Context): Unit = + action.patches.foreach: actionPatch => + patch(actionPatch.srcPos.span, actionPatch.replacement) } /** A completely encapsulated class representing rewrite state, used diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 6aaced118ac8..3d65b05b40b5 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -54,6 +54,7 @@ import cc.CheckCaptures import config.Config import scala.annotation.constructorOnly +import dotty.tools.dotc.rewrites.Rewrites object Typer { @@ -2946,8 +2947,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer if (migrateTo3) { // Under -rewrite, patch `x _` to `(() => x)` msg.actions - .flatMap(_.patches) - .map(actionPatch => patch(actionPatch.srcPos.span, actionPatch.replacement)) + .headOption + .foreach(Rewrites.applyAction) return typed(untpd.Function(Nil, qual), pt) } } @@ -3958,8 +3959,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer || Feature.warnOnMigration(msg, tree.srcPos, version = `3.0`) && { msg.actions - .flatMap(_.patches) - .map(actionPatch => patch(actionPatch.srcPos.span, actionPatch.replacement)) + .headOption + .foreach(Rewrites.applyAction) true }