Skip to content

Commit

Permalink
fix: Make ResetWorkspace more stable
Browse files Browse the repository at this point in the history
Now user is asked for confirmation before clearing .bloop directory.
Adds additional checks for non-existing files, since it used to fail both on `.toList` and `delete`.
Also, the ResetWorkspaceLspSuite was flaky before.
  • Loading branch information
jkciesluk committed Jul 14, 2023
1 parent 41a8dd1 commit c9635e1
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 25 deletions.
26 changes: 26 additions & 0 deletions metals/src/main/scala/scala/meta/internal/metals/BloopDir.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package scala.meta.internal.metals

import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.io.AbsolutePath

object BloopDir {
def clear(workspace: AbsolutePath): Unit = {
val bloopDir = workspace.resolve(".bloop")
bloopDir.list.foreach { f =>
if (f.exists && f.isDirectory) f.deleteRecursively()
}
val remainingDirs =
bloopDir.list.filter(f => f.exists && f.isDirectory).toList
if (remainingDirs.isEmpty) {
scribe.info(
"Deleted directories inside .bloop"
)
} else {
val str = remainingDirs.mkString(", ")
scribe.error(
s"Couldn't delete directories inside .bloop, remaining: $str"
)
}

}
}
20 changes: 20 additions & 0 deletions metals/src/main/scala/scala/meta/internal/metals/Messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -888,6 +888,26 @@ object Messages {
}
}

object ResetWorkspace {
val message: String =
"Are you sure you want to clear all caches and compiled artifacts and reset workspace?"
val resetWorkspace: String = "Reset workspace"
val cancel: String = "Cancel"
def params(): ShowMessageRequestParams = {
val params = new ShowMessageRequestParams(
List(
resetWorkspace,
cancel,
)
.map(new MessageActionItem(_))
.asJava
)
params.setMessage(message)
params.setType(MessageType.Warning)
params
}
}

object NewScalaProject {
def selectTheTemplate: String = "Select the template to use"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import java.net.URI
import java.nio.charset.StandardCharsets
import java.nio.file.FileAlreadyExistsException
import java.nio.file.Files
import java.nio.file.NoSuchFileException
import java.nio.file.Path
import java.nio.file.StandardCopyOption
import java.nio.file.StandardOpenOption
Expand Down Expand Up @@ -524,8 +525,23 @@ object MetalsEnrichments
Files.delete(path.dealias.toNIO)
}

// This method used to fail on both `.toList` and `delete`
// so we want to be extra careful and check if file exists on every step
def deleteRecursively(): Unit = {
path.listRecursive.toList.reverse.foreach(f => if (f.exists) f.delete())
path.listRecursive
.filter(_.exists)
.toList
.reverse
.foreach(_.deleteIfExists())
}

def deleteIfExists(): Unit = {
if (path.exists) {
try path.delete()
catch {
case _: NoSuchFileException => ()
}
}
}

def appendText(text: String): Unit = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2576,29 +2576,12 @@ class MetalsLspService(
}

private def clearBloopDir(): Unit = {
try {
val bloopDir = folder.resolve(".bloop")
bloopDir.list.foreach { f =>
if (f.exists && f.isDirectory) f.deleteRecursively()
}
val remainingDirs =
bloopDir.list.filter(f => f.exists && f.isDirectory).toList
if (remainingDirs.isEmpty) {
scribe.info(
"Deleted directories inside .bloop"
)
} else {
val str = remainingDirs.mkString(", ")
scribe.error(
s"Couldn't delete directories inside .bloop, remaining: $str"
)
}
} catch {
try BloopDir.clear(folder)
catch {
case e: Throwable =>
languageClient.showMessage(Messages.ResetWorkspaceFailed)
scribe.error("Error while deleting directories inside .bloop", e)
}

}

def resetWorkspace(): Future[Unit] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ class WorkspaceLspService(
.discoverMainClasses(unresolvedParams)
.asJavaObject
case ServerCommands.ResetWorkspace() =>
foreachSeq(_.resetWorkspace())
maybeResetWorkspace().asJavaObject
case ServerCommands.RunScalafix(params) =>
val uri = params.getTextDocument().getUri()
getServiceFor(uri).runScalafix(uri).asJavaObject
Expand Down Expand Up @@ -1227,6 +1227,28 @@ class WorkspaceLspService(
def workspaceSymbol(query: String): Seq[SymbolInformation] =
folderServices.flatMap(_.workspaceSymbol(query))

private def maybeResetWorkspace(): Future[Unit] = {
languageClient
.showMessageRequest(Messages.ResetWorkspace.params())
.asScala
.flatMap { response =>
if (response != null)
response.getTitle match {
case Messages.ResetWorkspace.resetWorkspace =>
Future
.sequence(folderServices.map(_.resetWorkspace()))
.ignoreValue
case _ => Future.unit
}
else {
Future.unit
}
}
.recover { e =>
scribe.warn("Error requesting workspace reset", e)
}
}

}

case class Folder(path: AbsolutePath, visibleName: Option[String]) {
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/src/main/scala/tests/TestingClient.scala
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers)
actions.find(_.getTitle == "a.Main").get
}
var importScalaCliScript = new MessageActionItem(ImportScalaScript.dismiss)
var resetWorkspace = new MessageActionItem(ResetWorkspace.cancel)

val resources = new ResourceOperations(buffers)
val diagnostics: TrieMap[AbsolutePath, Seq[Diagnostic]] =
Expand Down Expand Up @@ -339,6 +340,8 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers)
switchBuildTool
} else if (ImportScalaScript.params() == params) {
importScalaCliScript
} else if (ResetWorkspace.params() == params) {
resetWorkspace
} else {
throw new IllegalArgumentException(params.toString)
}
Expand Down
29 changes: 25 additions & 4 deletions tests/unit/src/test/scala/tests/ResetWorkspaceLspSuite.scala
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
package tests

import scala.meta.internal.metals.Messages
import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.internal.metals.ServerCommands

import org.eclipse.lsp4j.MessageActionItem

class ResetWorkspaceLspSuite extends BaseLspSuite(s"reset-workspace") {

test("basic") {
def bloopDir = server.workspace.resolve(".bloop")
def classFileExists = bloopDir.exists && bloopDir.listRecursive.exists(f =>
f.isFile && !f.filename.endsWith(".json")
)
cleanWorkspace()
for {
_ <- initialize(
"""
Expand All @@ -16,8 +25,12 @@ class ResetWorkspaceLspSuite extends BaseLspSuite(s"reset-workspace") {
|/a/src/main/scala/a/A.scala
|package a
|import scala.util.Success
|import b.B
|
|object A {
| val x: String = 42
| case class Foo(x: Int)
| case class Bar(y: String)
| val foo = Foo(B.x)
|}
|/a/src/main/scala/b/B.scala
|package b
Expand All @@ -27,23 +40,31 @@ class ResetWorkspaceLspSuite extends BaseLspSuite(s"reset-workspace") {
|}
""".stripMargin
)

_ <- server.didOpen("a/src/main/scala/a/A.scala")
_ = assertNoDiff(
client.workspaceDiagnostics,
"",
)
_ = assert(classFileExists)
_ = client.resetWorkspace =
new MessageActionItem(Messages.ResetWorkspace.resetWorkspace)
_ <- server.executeCommand(ServerCommands.ResetWorkspace)
_ = assertNoDiff(
client.workspaceShowMessages,
"",
)
_ <- server.didSave("a/src/main/scala/b/B.scala")(
_.replaceAll("val x: Int = 42", "val x: String = 42")
)
_ = assertNoDiff(
client.workspaceDiagnostics,
"""|a/src/main/scala/a/A.scala:4:19: error: type mismatch;
"""|a/src/main/scala/b/B.scala:4:19: error: type mismatch;
| found : Int(42)
| required: String
| val x: String = 42
| ^^
|""".stripMargin,
)

} yield ()
}
}

0 comments on commit c9635e1

Please sign in to comment.