From 9fd01d944d761682d0f323f0eea028877047637a Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Tue, 10 Dec 2024 12:02:14 +0100 Subject: [PATCH] bugfix: Make sure everything uses didChange first --- .../meta/internal/metals/MetalsLspService.scala | 3 +++ .../test/scala/tests/bazel/BazelLspSuite.scala | 6 ++++-- .../test/scala/tests/sbt/SbtServerSuite.scala | 12 ++++++++---- .../main/scala/tests/BaseRenameLspSuite.scala | 12 ++++++++---- .../main/scala/tests/BaseWorksheetLspSuite.scala | 16 +++++++++++----- .../src/main/scala/tests/TestingServer.scala | 1 + .../scala/tests/BaseNonCompilingLspSuite.scala | 6 ++++-- .../tests/BuildServerConnectionLspSuite.scala | 3 ++- .../test/scala/tests/DefinitionLspSuite.scala | 3 ++- .../test/scala/tests/DiagnosticsLspSuite.scala | 3 ++- .../src/test/scala/tests/DidFocusLspSuite.scala | 9 ++++++--- .../test/scala/tests/FingerprintsLspSuite.scala | 6 ++++-- .../src/test/scala/tests/TreeViewLspSuite.scala | 4 ++-- .../bestEffort/BestEffortCompilationSuite.scala | 1 + 14 files changed, 58 insertions(+), 27 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala index b57fad1eefb..c9a001572f8 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -884,6 +884,9 @@ abstract class MetalsLspService( ): CompletableFuture[Unit] = { val path = params.getTextDocument.getUri.toAbsolutePath savedFiles.add(path) + if (!buffers.contains(path)) { + buffers.put(path, path.toInput.text) + } Future .sequence( List( diff --git a/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala b/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala index 2f1fd0cb7f8..1bcdfcf0b95 100644 --- a/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala +++ b/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala @@ -234,7 +234,7 @@ class BazelLspSuite BazelBuildLayout(workspaceLayout, V.bazelScalaVersion, bazelVersion) ) _ <- server.didOpen("Hello.scala") - _ <- server.didSave("Hello.scala") { _ => + _ <- server.didChange("Hello.scala") { _ => """|package examples.scala3 | |sealed trait A @@ -251,6 +251,7 @@ class BazelLspSuite |} |""".stripMargin } + _ <- server.didSave("Hello.scala")(identity) _ = assertNoDiff( server.client.workspaceDiagnostics, """|Hello.scala:11:3: warning: match may not be exhaustive. @@ -262,12 +263,13 @@ class BazelLspSuite |""".stripMargin, ) // warnings should not disappear after updating - _ <- server.didSave("Hello.scala") { text => + _ <- server.didChange("Hello.scala") { text => s"""|$text | |class Additional |""".stripMargin } + _ <- server.didSave("Hello.scala")(identity) _ = assertNoDiff( server.client.workspaceDiagnostics, """|Hello.scala:11:3: warning: match may not be exhaustive. diff --git a/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala b/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala index 3cb60b7492c..d1038e642c0 100644 --- a/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala +++ b/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala @@ -194,11 +194,12 @@ class SbtServerSuite ) // reload build after build.sbt changes _ <- server.executeCommand(ServerCommands.ResetNotifications) - _ <- server.didSave("build.sbt") { text => + _ <- server.didChange("build.sbt") { text => s"""$text |ibraryDependencies += "com.lihaoyi" %% "sourcecode" % "0.3.0" |""".stripMargin } + _ <- server.didSave("build.sbt")(identity) _ = { assertNoDiff( client.workspaceErrorShowMessages, @@ -209,9 +210,10 @@ class SbtServerSuite // This is a little hacky but up above this promise is succeeded already, so down // below it won't wait until it reconnects to Sbt and indexed like we want _ = server.server.indexingPromise = Promise() - _ <- server.didSave("build.sbt") { text => + _ <- server.didChange("build.sbt") { text => text.replace("ibraryDependencies", "libraryDependencies") } + _ <- server.didSave("build.sbt")(identity) _ = { assert(client.workspaceErrorShowMessages.isEmpty) } @@ -231,7 +233,7 @@ class SbtServerSuite | ^^^ |""".stripMargin, ) - _ <- server.didSave("build.sbt") { text => + _ <- server.didChange("build.sbt") { text => text.replace( "val a = project.in(file(\"a\"))", """|val a = project.in(file("a")).settings( @@ -240,15 +242,17 @@ class SbtServerSuite |""".stripMargin, ) } + _ <- server.didSave("build.sbt")(identity) _ = { assert(client.workspaceErrorShowMessages.isEmpty) } - _ <- server.didSave("a/src/main/scala/A.scala") { _ => + _ <- server.didChange("a/src/main/scala/A.scala") { _ => """|object A{ | val a: scala.meta.Defn.Class = ??? |} |""".stripMargin } + _ <- server.didSave("a/src/main/scala/A.scala")(identity) _ <- server.assertHoverAtLine( "a/src/main/scala/A.scala", " val a: scala.meta.Defn.C@@lass = ???", diff --git a/tests/unit/src/main/scala/tests/BaseRenameLspSuite.scala b/tests/unit/src/main/scala/tests/BaseRenameLspSuite.scala index 019674e6226..dea263e64a5 100644 --- a/tests/unit/src/main/scala/tests/BaseRenameLspSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseRenameLspSuite.scala @@ -97,7 +97,8 @@ abstract class BaseRenameLspSuite(name: String) extends BaseLspSuite(name) { // possible breaking changes for testing _ <- Future.sequence { openedFiles.map { file => - server.didSave(file) { code => breakingChange(code) } + server.didChange(file) { code => breakingChange(code) } + server.didSave(file)(identity) } } _ = if (!expectedError) assertNoDiagnostics() @@ -124,9 +125,12 @@ abstract class BaseRenameLspSuite(name: String) extends BaseLspSuite(name) { Future .sequence { files.map { case (file, code) => - server.didSave(file)(_ => - code.replaceAll(allMarkersRegex, "") - ) + for { + _ <- server.didChange(file)(_ => + code.replaceAll(allMarkersRegex, "") + ) + _ <- server.didSave(file)(identity) + } yield () }.toList } diff --git a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala index d2868f5a300..efdb1d5daf0 100644 --- a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala @@ -285,12 +285,14 @@ abstract class BaseWorksheetLspSuite( _ <- server.didOpen("a/src/main/scala/Main.worksheet.sc") _ <- cancelled.future _ = client.onWorkDoneProgressStart = (_, _) => {} - _ <- server.didSave("a/src/main/scala/Main.worksheet.sc")( + _ <- server.didChange("a/src/main/scala/Main.worksheet.sc")( _.replace("Stream", "// Stream") ) - _ <- server.didSave("a/src/main/scala/Main.worksheet.sc")( + _ <- server.didSave("a/src/main/scala/Main.worksheet.sc")(identity) + _ <- server.didChange("a/src/main/scala/Main.worksheet.sc")( _.replace("42", "43") ) + _ <- server.didSave("a/src/main/scala/Main.worksheet.sc")(identity) _ = assertNoDiff( client.syntheticDecorations, """| @@ -469,9 +471,11 @@ abstract class BaseWorksheetLspSuite( |a.Util.increase(1) // : Int = 2 |""".stripMargin, ) - _ <- server.didSave("a/src/main/scala/a/Util.scala")( + _ <- server.didChange("a/src/main/scala/a/Util.scala")( _.replace("n + 1", "n + 2") ) + _ <- server.didSave("a/src/main/scala/a/Util.scala")(identity) + _ <- server.didFocus("a/src/main/scala/a/Main.worksheet.sc") _ <- server.didSave("a/src/main/scala/a/Main.worksheet.sc")(identity) _ = assertNoDiff( client.workspaceDecorations("a/src/main/scala/a/Main.worksheet.sc"), @@ -782,12 +786,13 @@ abstract class BaseWorksheetLspSuite( ) ), ) - _ <- server.didSave("a/src/main/scala/foo/Main.worksheet.sc")( + _ <- server.didChange("a/src/main/scala/foo/Main.worksheet.sc")( _.replace( "Hi(1, 2, 3)", "Hi(7, 8, 9)", ) ) + _ <- server.didSave("a/src/main/scala/foo/Main.worksheet.sc")(identity) export = server.exportEvaluation( "a/src/main/scala/foo/Main.worksheet.sc" ) @@ -951,9 +956,10 @@ abstract class BaseWorksheetLspSuite( Nil, ) _ <- client.applyCodeAction(0, codeActions, server) - _ <- server.didSave(path) { _ => + _ <- server.didChange(path) { _ => server.bufferContents(path) } + _ <- server.didSave(path)(identity) // Assert if indentation is correct. See `AutoImports.renderImport` _ = assertNoDiff( server.bufferContents(path), diff --git a/tests/unit/src/main/scala/tests/TestingServer.scala b/tests/unit/src/main/scala/tests/TestingServer.scala index 98042207732..dff22794d71 100644 --- a/tests/unit/src/main/scala/tests/TestingServer.scala +++ b/tests/unit/src/main/scala/tests/TestingServer.scala @@ -785,6 +785,7 @@ final case class TestingServer( val abspath = toPath(filename) val oldText = abspath.toInputFromBuffers(buffers).text val newText = fn(oldText) + buffers.put(abspath, newText) didChange(filename, newText) } diff --git a/tests/unit/src/test/scala/tests/BaseNonCompilingLspSuite.scala b/tests/unit/src/test/scala/tests/BaseNonCompilingLspSuite.scala index 3b48fe4cb66..8e3299ed796 100644 --- a/tests/unit/src/test/scala/tests/BaseNonCompilingLspSuite.scala +++ b/tests/unit/src/test/scala/tests/BaseNonCompilingLspSuite.scala @@ -105,9 +105,10 @@ abstract class BaseNonCompilingLspSuite(name: String) |} |""".stripMargin input = newText.replace("<<", "").replace(">>", "") - _ <- server.didSave("a/src/main/scala/a/A.scala") { _ => + _ <- server.didChange("a/src/main/scala/a/A.scala") { _ => newText.replace("<<", "").replace(">>", "") } + _ <- server.didSave("a/src/main/scala/a/A.scala")(identity) _ <- server .assertCodeAction( @@ -119,9 +120,10 @@ abstract class BaseNonCompilingLspSuite(name: String) kind = Nil, ) // make sure that the now change UniqueObject is not suggested - _ <- server.didSave("a/src/main/scala/a/A.scala") { _ => + _ <- server.didChange("a/src/main/scala/a/A.scala") { _ => input.replace("UniqueObjectOther", "UniqueObject") } + _ <- server.didSave("a/src/main/scala/a/A.scala")(identity) _ <- server .assertCodeAction( diff --git a/tests/unit/src/test/scala/tests/BuildServerConnectionLspSuite.scala b/tests/unit/src/test/scala/tests/BuildServerConnectionLspSuite.scala index db8eda7209e..ec10c067ea7 100644 --- a/tests/unit/src/test/scala/tests/BuildServerConnectionLspSuite.scala +++ b/tests/unit/src/test/scala/tests/BuildServerConnectionLspSuite.scala @@ -26,9 +26,10 @@ class BuildServerConnectionLspSuite _ = server.server.bspSession.get.cancel() _ = assertNoDiagnostics() _ <- server.executeCommand(ServerCommands.ConnectBuildServer) - _ <- server.didSave("a/src/main/scala/a/A.scala")( + _ <- server.didChange("a/src/main/scala/a/A.scala")( _.replace("val n = 42", "val n: String = 42") ) + _ <- server.didSave("a/src/main/scala/a/A.scala")(identity) _ = assertNoDiff( client.workspaceDiagnostics, """|a/src/main/scala/a/A.scala:3:19: error: type mismatch; diff --git a/tests/unit/src/test/scala/tests/DefinitionLspSuite.scala b/tests/unit/src/test/scala/tests/DefinitionLspSuite.scala index 074ae5f2f9a..e8b1e5259ec 100644 --- a/tests/unit/src/test/scala/tests/DefinitionLspSuite.scala +++ b/tests/unit/src/test/scala/tests/DefinitionLspSuite.scala @@ -266,9 +266,10 @@ class DefinitionLspSuite |} """.stripMargin, ) - _ <- server.didSave("a/src/main/scala/a/Main.scala")( + _ <- server.didChange("a/src/main/scala/a/Main.scala")( _.replace("max(1, 2)", "max") ) + _ <- server.didSave("a/src/main/scala/a/Main.scala")(identity) _ = assertNoDiff( server.workspaceDefinitions, """|/a/src/main/scala/a/Main.scala diff --git a/tests/unit/src/test/scala/tests/DiagnosticsLspSuite.scala b/tests/unit/src/test/scala/tests/DiagnosticsLspSuite.scala index 8dfab761903..1a57d70d500 100644 --- a/tests/unit/src/test/scala/tests/DiagnosticsLspSuite.scala +++ b/tests/unit/src/test/scala/tests/DiagnosticsLspSuite.scala @@ -355,9 +355,10 @@ class DiagnosticsLspSuite extends BaseLspSuite("diagnostics") { | ^^ |""".stripMargin, ) - _ <- server.didSave("a/src/main/scala/a/A.scala")( + _ <- server.didChange("a/src/main/scala/a/A.scala")( _.replace("val n: Int = \"\"", "val n: Int = \" ") ) + _ <- server.didSave("a/src/main/scala/a/A.scala")(identity) _ = assertNoDiff( client.workspaceDiagnostics, """|a/src/main/scala/a/A.scala:2:16: error: unclosed string literal diff --git a/tests/unit/src/test/scala/tests/DidFocusLspSuite.scala b/tests/unit/src/test/scala/tests/DidFocusLspSuite.scala index 8b9c4069de7..1b6d62b34e7 100644 --- a/tests/unit/src/test/scala/tests/DidFocusLspSuite.scala +++ b/tests/unit/src/test/scala/tests/DidFocusLspSuite.scala @@ -185,23 +185,26 @@ class DidFocusWhileCompilingLspSuite |""".stripMargin } _ = fakeTime.elapseSeconds(10) - _ <- server.didSave("a/src/main/scala/a/A.scala")( + _ <- server.didChange("a/src/main/scala/a/A.scala")( _.replace("1", "\"\"") ) + _ <- server.didSave("a/src/main/scala/a/A.scala")(identity) _ = assertNoDiff( client.workspaceDiagnostics, xMismatch, ) - _ <- server.didSave("b/src/main/scala/b/B.scala")( + _ <- server.didChange("b/src/main/scala/b/B.scala")( _.replace("2", "\"\"") ) + _ <- server.didSave("b/src/main/scala/b/B.scala")(identity) _ = assertNoDiff( client.workspaceDiagnostics, xMismatch, ) - _ <- server.didSave("a/src/main/scala/a/A.scala")( + _ <- server.didChange("a/src/main/scala/a/A.scala")( _.replace("Int", "String") ) + _ <- server.didSave("a/src/main/scala/a/A.scala")(identity) _ = assertNoDiff( client.workspaceDiagnostics, """|b/src/main/scala/b/B.scala:3:16: error: type mismatch; diff --git a/tests/unit/src/test/scala/tests/FingerprintsLspSuite.scala b/tests/unit/src/test/scala/tests/FingerprintsLspSuite.scala index 266b3ff6f29..67f038f29ec 100644 --- a/tests/unit/src/test/scala/tests/FingerprintsLspSuite.scala +++ b/tests/unit/src/test/scala/tests/FingerprintsLspSuite.scala @@ -33,12 +33,14 @@ class FingerprintsLspSuite extends BaseLspSuite("fingerprints") { _ <- server.executeCommand(ServerCommands.CascadeCompile) _ = assertNoDiff(client.workspaceDiagnostics, "") _ = server.assertReferenceDefinitionBijection() - _ <- server.didSave("a/src/main/scala/a/Names.scala")(text => + _ <- server.didChange("a/src/main/scala/a/Names.scala")(text => text.replace("+ surname", "+ surname2") ) - _ <- server.didSave("a/src/main/scala/a/Adresses.scala")(text => + _ <- server.didSave("a/src/main/scala/a/Names.scala")(identity) + _ <- server.didChange("a/src/main/scala/a/Adresses.scala")(text => text.replace("+ number", "+ number2") ) + _ <- server.didSave("a/src/main/scala/a/Adresses.scala")(identity) _ = assertNoDiff( client.workspaceDiagnostics, """|a/src/main/scala/a/Adresses.scala:5:44: error: not found: value number2 diff --git a/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala b/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala index 994631ee87e..cea3e637c65 100644 --- a/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala +++ b/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala @@ -173,7 +173,7 @@ class TreeViewLspSuite extends BaseLspSuite("tree-view") { |c symbol-variable |""".stripMargin, ) - _ <- server.didSave("a/src/main/scala/a/Zero.scala") { text => + _ <- server.didChange("a/src/main/scala/a/Zero.scala") { text => text.replace("val a = 1", "val a = 1\nval b = 1.0") } _ = assertEquals( @@ -191,7 +191,7 @@ class TreeViewLspSuite extends BaseLspSuite("tree-view") { s"projects-$folder:${server.buildTarget("a")}!/_root_/", isCollapsed = true, ) - _ <- server.didSave("a/src/main/scala/a/Zero.scala") { text => + _ <- server.didChange("a/src/main/scala/a/Zero.scala") { text => text.replace("val a = 1", "val a = 1\nval c = 1.0") } _ = assertEmpty( diff --git a/tests/unit/src/test/scala/tests/bestEffort/BestEffortCompilationSuite.scala b/tests/unit/src/test/scala/tests/bestEffort/BestEffortCompilationSuite.scala index c9857d9f243..8d99ebba2b4 100644 --- a/tests/unit/src/test/scala/tests/bestEffort/BestEffortCompilationSuite.scala +++ b/tests/unit/src/test/scala/tests/bestEffort/BestEffortCompilationSuite.scala @@ -232,6 +232,7 @@ class BestEffortCompilationSuite |""".stripMargin } _ <- server.didOpen("a/src/main/scala/a/A.scala") + _ <- server.didOpen("b/src/main/scala/b/B.scala") _ = assertNoDiff( client.workspaceDiagnostics, """|a/src/main/scala/a/A.scala:5:3: error: value completionUnknown is not a member of object B