From 82ff17b1136a9c8c2bfe81c40fb7fed1ae75cb9c 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 +++ .../src/test/scala/tests/bazel/BazelLspSuite.scala | 6 ++++-- .../src/main/scala/tests/BaseRenameLspSuite.scala | 12 ++++++++---- .../src/main/scala/tests/BaseWorksheetLspSuite.scala | 3 ++- tests/unit/src/main/scala/tests/TestingServer.scala | 1 + .../test/scala/tests/BaseNonCompilingLspSuite.scala | 6 ++++-- .../scala/tests/BuildServerConnectionLspSuite.scala | 3 ++- .../src/test/scala/tests/DefinitionLspSuite.scala | 3 ++- .../src/test/scala/tests/DiagnosticsLspSuite.scala | 3 ++- .../unit/src/test/scala/tests/DidFocusLspSuite.scala | 9 ++++++--- .../src/test/scala/tests/FingerprintsLspSuite.scala | 7 +++++-- .../unit/src/test/scala/tests/TreeViewLspSuite.scala | 3 ++- .../bestEffort/BestEffortCompilationSuite.scala | 1 + 13 files changed, 42 insertions(+), 18 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/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..8e520f79c9b 100644 --- a/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala @@ -782,12 +782,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" ) 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..9e2f6a1f338 100644 --- a/tests/unit/src/test/scala/tests/FingerprintsLspSuite.scala +++ b/tests/unit/src/test/scala/tests/FingerprintsLspSuite.scala @@ -33,12 +33,15 @@ 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/Names.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..a5b3263a05e 100644 --- a/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala +++ b/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala @@ -173,9 +173,10 @@ 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") } + _ <- server.didSave("a/src/main/scala/a/Zero.scala")(identity) _ = assertEquals( server.client.workspaceTreeViewChanges, s"metalsPackages projects-$folder:${server.buildTarget("a")}!/_root_/", 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