Skip to content

Commit 8aa5176

Browse files
DuhemmDuhemm
committed
Ask user when renaming overriding symbols
When renaming a symbol that is overriding another symbol, we now ask the user whether we should rename the base symbol or only this member.
1 parent e6a70f4 commit 8aa5176

File tree

5 files changed

+104
-27
lines changed

5 files changed

+104
-27
lines changed

‎language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala‎

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -369,9 +369,7 @@ class DottyLanguageServer extends LanguageServer
369369
val uriTrees = driver.openedTrees(uri)
370370
val pos = sourcePosition(driver, uri, params.getPosition)
371371
val path = Interactive.pathTo(uriTrees, pos)
372-
val syms = Interactive.enclosingSourceSymbols(path, pos).flatMap { sym =>
373-
sym :: sym.allOverriddenSymbols.toList
374-
}
372+
val syms = Interactive.enclosingSourceSymbols(path, pos)
375373
val newName = params.getNewName
376374

377375
def findRenamedReferences(trees: List[SourceTree], syms: List[Symbol], withName: Name): List[SourceTree] = {
@@ -392,13 +390,25 @@ class DottyLanguageServer extends LanguageServer
392390
findRenamedReferences(uriTrees, syms, nameTree.name)
393391

394392
case _ =>
395-
val names = syms.map(_.name.sourceModuleName).toSet
393+
val (include, allSymbols) =
394+
if (syms.exists(_.allOverriddenSymbols.nonEmpty)) {
395+
showMessageRequest(MessageType.Info,
396+
RENAME_OVERRIDDEN_QUESTION,
397+
List(
398+
RENAME_OVERRIDDEN -> (() => (Include.all, syms.flatMap(s => s :: s.allOverriddenSymbols.toList))),
399+
RENAME_NO_OVERRIDDEN -> (() => (Include.all.except(Include.overridden), syms)))
400+
).get.getOrElse((Include.empty, List.empty[Symbol]))
401+
} else {
402+
(Include.all, syms)
403+
}
404+
405+
val names = allSymbols.map(_.name.sourceModuleName).toSet
396406
val trees = names.flatMap(name => driver.allTreesContaining(name.toString)).toList
397-
syms.flatMap { sym =>
407+
allSymbols.flatMap { sym =>
398408
Interactive.findTreesMatching(trees,
399-
Include.all,
400-
sym,
401-
t => names.exists(Interactive.sameName(t.name, _)))
409+
include,
410+
sym,
411+
t => names.exists(Interactive.sameName(t.name, _)))
402412
}
403413
}
404414

@@ -495,12 +505,41 @@ class DottyLanguageServer extends LanguageServer
495505
override def resolveCodeLens(params: CodeLens) = null
496506
override def resolveCompletionItem(params: CompletionItem) = null
497507
override def signatureHelp(params: TextDocumentPositionParams) = null
508+
509+
/**
510+
* Send a `window/showMessageRequest` to the client, asking to choose between `choices`, and
511+
* perform the associated operation.
512+
*
513+
* @param tpe The type of the request
514+
* @param message The message accompanying the request
515+
* @param choices The choices and their associated operation
516+
* @return A future that will complete with the result of executing the action corresponding to
517+
* the user's response.
518+
*/
519+
private def showMessageRequest[T](tpe: MessageType,
520+
message: String,
521+
choices: List[(String, () => T)]): CompletableFuture[Option[T]] = {
522+
val options = choices.map((title, _) => new MessageActionItem(title))
523+
val request = new ShowMessageRequestParams(options.asJava)
524+
request.setMessage(message)
525+
request.setType(tpe)
526+
527+
client.showMessageRequest(request).thenApply { (answer: MessageActionItem) =>
528+
choices.find(_._1 == answer.getTitle).map {
529+
case (_, action) => action()
530+
}
531+
}
532+
}
498533
}
499534

500535
object DottyLanguageServer {
501536
/** Configuration file normally generated by sbt-dotty */
502537
final val IDE_CONFIG_FILE = ".dotty-ide.json"
503538

539+
final val RENAME_OVERRIDDEN_QUESTION = "Do you want to rename the base member, or only this member?"
540+
final val RENAME_OVERRIDDEN= "Rename the base member"
541+
final val RENAME_NO_OVERRIDDEN = "Rename only this member"
542+
504543
/** Convert an lsp4j.Position to a SourcePosition */
505544
def sourcePosition(driver: InteractiveDriver, uri: URI, pos: lsp4j.Position): SourcePosition = {
506545
val actualPosition =

‎language-server/test/dotty/tools/languageserver/RenameTest.scala‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -216,19 +216,23 @@ class RenameTest {
216216
}
217217

218218
@Test def renameOverridden: Unit = {
219-
def testRename(m: CodeMarker) =
219+
def testRename(m: CodeMarker, expectations: Set[CodeRange], withOverridden: Option[Boolean]) =
220220
withSources(
221221
code"""class A { def ${m1}foo${m2}: Int = 0 }
222222
class B extends A { override def ${m3}foo${m4}: Int = 1 }
223223
class C extends A { override def ${m5}foo${m6}: Int = 2 }"""
224-
).rename(m, "NewName", Set(m1 to m2, m3 to m4, m5 to m6))
225-
226-
testRename(m1)
227-
testRename(m2)
228-
testRename(m3)
229-
testRename(m4)
230-
testRename(m5)
231-
testRename(m6)
224+
).rename(m, "NewName", expectations, withOverridden)
225+
226+
testRename(m1, Set(m1 to m2, m3 to m4, m5 to m6), withOverridden = None)
227+
testRename(m2, Set(m1 to m2, m3 to m4, m5 to m6), withOverridden = None)
228+
testRename(m3, Set(m1 to m2, m3 to m4, m5 to m6), withOverridden = Some(true))
229+
testRename(m4, Set(m1 to m2, m3 to m4, m5 to m6), withOverridden = Some(true))
230+
testRename(m5, Set(m1 to m2, m3 to m4, m5 to m6), withOverridden = Some(true))
231+
testRename(m6, Set(m1 to m2, m3 to m4, m5 to m6), withOverridden = Some(true))
232+
testRename(m3, Set(m3 to m4), withOverridden = Some(false))
233+
testRename(m4, Set(m3 to m4), withOverridden = Some(false))
234+
testRename(m5, Set(m5 to m6), withOverridden = Some(false))
235+
testRename(m6, Set(m5 to m6), withOverridden = Some(false))
232236

233237
}
234238

‎language-server/test/dotty/tools/languageserver/util/CodeTester.scala‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,14 +89,20 @@ class CodeTester(projects: List[Project]) {
8989
* Performs a workspace-wide renaming of the symbol under `marker`, verifies that the positions to
9090
* update match `expected`.
9191
*
92-
* @param marker The position from which to ask for renaming.
93-
* @param newName The new name to give to the symbol.
94-
* @param expected The expected positions to change.
92+
* @param marker The position from which to ask for renaming.
93+
* @param newName The new name to give to the symbol.
94+
* @param expected The expected positions to change.
95+
* @param withOverridden If `None`, do not expect the server to ask whether to include overridden
96+
* symbol. Otherwise, wait for this question from the server and include
97+
* overridden symbols if this is true.
9598
*
9699
* @see dotty.tools.languageserver.util.actions.CodeRename
97100
*/
98-
def rename(marker: CodeMarker, newName: String, expected: Set[CodeRange]): this.type =
99-
doAction(new CodeRename(marker, newName, expected)) // TODO apply changes to the sources and positions
101+
def rename(marker: CodeMarker,
102+
newName: String,
103+
expected: Set[CodeRange],
104+
withOverridden: Option[Boolean] = None): this.type =
105+
doAction(new CodeRename(marker, newName, expected, withOverridden)) // TODO apply changes to the sources and positions
100106

101107
/**
102108
* Queries for all the symbols referenced in the source file in `marker`, verifies that they match

‎language-server/test/dotty/tools/languageserver/util/actions/CodeRename.scala‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,13 @@ package dotty.tools.languageserver.util.actions
22

33
import dotty.tools.languageserver.util.embedded.CodeMarker
44
import dotty.tools.languageserver.util.{CodeRange, PositionContext}
5+
import dotty.tools.languageserver.DottyLanguageServer.{RENAME_OVERRIDDEN, RENAME_NO_OVERRIDDEN}
56

6-
import org.junit.Assert.{assertEquals, assertNull}
7+
import org.junit.Assert.{assertEquals, assertNull, fail}
8+
9+
import org.eclipse.lsp4j.{MessageActionItem, ShowMessageRequestParams}
10+
11+
import java.util.concurrent.CompletableFuture
712

813
import scala.collection.JavaConverters._
914

@@ -17,10 +22,31 @@ import scala.collection.JavaConverters._
1722
*/
1823
class CodeRename(override val marker: CodeMarker,
1924
newName: String,
20-
expected: Set[CodeRange]) extends ActionOnMarker {
25+
expected: Set[CodeRange],
26+
withOverridden: Option[Boolean]) extends ActionOnMarker {
27+
28+
private final val TIMEOUT_MS = 10000
2129

2230
override def execute(): Exec[Unit] = {
23-
val results = server.rename(marker.toRenameParams(newName)).get()
31+
val query = server.rename(marker.toRenameParams(newName))
32+
33+
withOverridden.foreach { includeOverridden =>
34+
var question: (ShowMessageRequestParams, CompletableFuture[MessageActionItem]) = null
35+
val startTime = System.currentTimeMillis()
36+
do {
37+
Thread.sleep(50)
38+
question = client.requests.get.headOption.orNull
39+
} while (question == null && System.currentTimeMillis() - startTime < TIMEOUT_MS)
40+
41+
if (question == null) fail("The server didn't ask about overridden symbols.")
42+
43+
val answerStr = if (includeOverridden) RENAME_OVERRIDDEN else RENAME_NO_OVERRIDDEN
44+
val action = question._1.getActions.asScala.find(_.getTitle == answerStr).get
45+
question._2.complete(action)
46+
}
47+
48+
val results = query.get()
49+
2450
val changes = results.getChanges.asScala.mapValues(_.asScala.toSet.map(ch => (ch.getNewText, ch.getRange)))
2551
val expectedChanges = expected.groupBy(_.file.uri).mapValues(_.map(range => (newName, range.toRange)))
2652

‎language-server/test/dotty/tools/languageserver/util/server/TestClient.scala‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ class TestClient extends WorksheetClient {
2323
val diagnostics = new Log[PublishDiagnosticsParams]
2424
val telemetry = new Log[Any]
2525
val worksheetOutput = new Log[WorksheetRunOutput]
26+
val requests = new Log[(ShowMessageRequestParams, CompletableFuture[MessageActionItem])]
2627

2728
override def logMessage(message: MessageParams) = {
2829
log += message
@@ -37,8 +38,9 @@ class TestClient extends WorksheetClient {
3738
}
3839

3940
override def showMessageRequest(requestParams: ShowMessageRequestParams) = {
40-
log += requestParams
41-
new CompletableFuture[MessageActionItem]
41+
val reply = new CompletableFuture[MessageActionItem]
42+
requests += ((requestParams, reply))
43+
reply
4244
}
4345

4446
override def publishDiagnostics(diagnosticsParams: PublishDiagnosticsParams) = {

0 commit comments

Comments
 (0)