Skip to content

Commit f5eb029

Browse files
sebthomsebthom
authored
feat: support linked inline editing on rename (#1440)
Fixes #891
1 parent 024d833 commit f5eb029

File tree

9 files changed

+825
-21
lines changed

9 files changed

+825
-21
lines changed

‎org.eclipse.lsp4e.test/META-INF/MANIFEST.MF‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
22
Bundle-ManifestVersion: 2
33
Bundle-Name: Tests for language server bundle (Incubation)
44
Bundle-SymbolicName: org.eclipse.lsp4e.test;singleton:=true
5-
Bundle-Version: 0.16.4.qualifier
5+
Bundle-Version: 0.16.5.qualifier
66
Fragment-Host: org.eclipse.lsp4e
77
Bundle-Vendor: Eclipse LSP4E
88
Bundle-RequiredExecutionEnvironment: JavaSE-21

‎org.eclipse.lsp4e.test/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
</parent>
99
<artifactId>org.eclipse.lsp4e.test</artifactId>
1010
<packaging>eclipse-test-plugin</packaging>
11-
<version>0.16.4-SNAPSHOT</version>
11+
<version>0.16.5-SNAPSHOT</version>
1212

1313
<properties>
1414
<os-jvm-flags /> <!-- for the default case -->

‎org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/rename/LSPInlineRenameLinkedModeTest.java‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2025 Vegard IT GmbH and others.
3+
* This program and the accompanying materials are made
4+
* available under the terms of the Eclipse Public License 2.0
5+
* which is available at https://www.eclipse.org/legal/epl-2.0/
6+
*
7+
* SPDX-License-Identifier: EPL-2.0
8+
*
9+
* Contributors:
10+
* Sebastian Thomschke (Vegard IT GmbH) - initial implementation
11+
*******************************************************************************/
12+
package org.eclipse.lsp4e.test.rename;
13+
14+
import static org.eclipse.lsp4e.test.utils.TestUtils.waitForAndAssertCondition;
15+
import static org.junit.jupiter.api.Assertions.*;
16+
17+
import java.lang.reflect.Method;
18+
import java.util.HashMap;
19+
import java.util.List;
20+
21+
import org.eclipse.core.resources.IFile;
22+
import org.eclipse.core.runtime.preferences.InstanceScope;
23+
import org.eclipse.jface.text.IDocument;
24+
import org.eclipse.jface.text.IRegion;
25+
import org.eclipse.jface.text.ITextViewer;
26+
import org.eclipse.jface.text.Region;
27+
import org.eclipse.jface.text.link.LinkedPosition;
28+
import org.eclipse.lsp4e.LSPEclipseUtils;
29+
import org.eclipse.lsp4e.LanguageServerPlugin;
30+
import org.eclipse.lsp4e.LanguageServerWrapper;
31+
import org.eclipse.lsp4e.operations.rename.LSPInlineRenameLinkedMode;
32+
import org.eclipse.lsp4e.test.utils.AbstractTestWithProject;
33+
import org.eclipse.lsp4e.test.utils.TestUtils;
34+
import org.eclipse.lsp4e.tests.mock.MockLanguageServer;
35+
import org.eclipse.lsp4j.DocumentHighlight;
36+
import org.eclipse.lsp4j.DocumentHighlightKind;
37+
import org.eclipse.lsp4j.Position;
38+
import org.eclipse.lsp4j.Range;
39+
import org.eclipse.lsp4j.TextEdit;
40+
import org.eclipse.lsp4j.WorkspaceEdit;
41+
import org.eclipse.lsp4j.jsonrpc.messages.Either;
42+
import org.junit.jupiter.api.Test;
43+
44+
/**
45+
* Tests the behavior of {@link LSPInlineRenameLinkedMode} in isolation from the
46+
* UI layer to keep the tests deterministic and avoid brittle dependencies on
47+
* SWT widgets or key-event timing.
48+
* <p>
49+
* The scenarios focus on:
50+
* </p>
51+
* <ul>
52+
* <li>Collecting same-file occurrences from
53+
* {@code textDocument/documentHighlight} and mapping them to {@link IRegion}
54+
* instances.</li>
55+
* <li>Driving the non-UI inline rename pipeline, including
56+
* {@code scheduleRenameJob()}, revert of the in-buffer edit, and application of
57+
* the {@link WorkspaceEdit} returned by the language server.</li>
58+
* </ul>
59+
*/
60+
class LSPInlineRenameLinkedModeTest extends AbstractTestWithProject {
61+
62+
/**
63+
* Unit-style test for
64+
* {@code LSPInlineRenameLinkedMode.collectSameFileOccurrences(...)}.
65+
* <p>
66+
* Verifies that document highlights for the caret position are converted into a
67+
* de-duplicated list of same-file regions and that the primary region is always
68+
* present as the first element.
69+
* </p>
70+
*/
71+
@Test
72+
void testInlineLinkedEditingSameFile() throws Exception {
73+
// Ensure inline rename is enabled for this test
74+
InstanceScope.INSTANCE.getNode(LanguageServerPlugin.PLUGIN_ID).putBoolean("org.eclipse.lsp4e.inlineRename", //$NON-NLS-1$
75+
true);
76+
77+
// Prepare a simple document with two occurrences of the same identifier
78+
var content = "compute();\ncompute();";
79+
IFile file = TestUtils.createUniqueTestFile(project, content);
80+
IDocument document = LSPEclipseUtils.getDocument(file);
81+
assertNotNull(document);
82+
83+
int idLength = "compute".length();
84+
int firstOffset = content.indexOf("compute");
85+
int secondOffset = content.indexOf("compute", firstOffset + 1);
86+
var primaryRegion = new Region(firstOffset, idLength);
87+
88+
// Configure mock document highlights:
89+
// - two valid occurrences
90+
// - a duplicate of the primary occurrence (should be de-duplicated)
91+
var caretPosition = LSPEclipseUtils.toPosition(firstOffset, document);
92+
var highlights = new HashMap<Position, List<? extends DocumentHighlight>>();
93+
highlights.put(caretPosition, List.of( //
94+
new DocumentHighlight(new Range(new Position(0, 0), new Position(0, idLength)),
95+
DocumentHighlightKind.Read),
96+
new DocumentHighlight(new Range(new Position(1, 0), new Position(1, idLength)),
97+
DocumentHighlightKind.Read),
98+
new DocumentHighlight(new Range(new Position(0, 0), new Position(0, idLength)),
99+
DocumentHighlightKind.Text)));
100+
MockLanguageServer.INSTANCE.setDocumentHighlights(highlights);
101+
102+
// Call the internal helper directly via reflection to avoid depending on
103+
// UI/command wiring
104+
Method collectSameFileOccurrences = LSPInlineRenameLinkedMode.class
105+
.getDeclaredMethod("collectSameFileOccurrences", IDocument.class, int.class, IRegion.class); //$NON-NLS-1$
106+
collectSameFileOccurrences.setAccessible(true);
107+
108+
@SuppressWarnings("unchecked") //
109+
var regions = (List<IRegion>) collectSameFileOccurrences.invoke(null, document, firstOffset, primaryRegion);
110+
111+
// Primary region is always present and first
112+
assertEquals(firstOffset, regions.get(0).getOffset());
113+
assertEquals(idLength, regions.get(0).getLength());
114+
115+
// Duplicates must be filtered out
116+
assertEquals(2, regions.size());
117+
assertEquals(secondOffset, regions.get(1).getOffset());
118+
assertEquals(idLength, regions.get(1).getLength());
119+
}
120+
121+
/**
122+
* Drives the inline-rename pipeline without relying on SWT key events.
123+
* <p>
124+
* The test simulates a user-typed identifier in the primary linked position,
125+
* then calls {@code scheduleRenameJob()} and asserts that the
126+
* {@link WorkspaceEdit} returned by the language server is applied back to the
127+
* document.
128+
* </p>
129+
*/
130+
@Test
131+
void testInlineRenameEndToEndSameFile() throws Exception {
132+
// Prepare a simple document with two occurrences of the same identifier
133+
var content = "compute();\ncompute();";
134+
IFile file = TestUtils.createUniqueTestFile(project, content);
135+
IDocument document = LSPEclipseUtils.getDocument(file);
136+
assertNotNull(document);
137+
138+
int idLength = "compute".length();
139+
int firstOffset = content.indexOf("compute");
140+
int secondOffset = content.indexOf("compute", firstOffset + 1);
141+
142+
// Simulated name typed by the user in linked mode
143+
String typedName = "inlineName";
144+
145+
// Prepare rename result for realism (not strictly required for this test)
146+
Position firstPos = LSPEclipseUtils.toPosition(firstOffset, document);
147+
Position secondPos = LSPEclipseUtils.toPosition(secondOffset, document);
148+
149+
var prepareRange = new Range(firstPos, new Position(firstPos.getLine(), firstPos.getCharacter() + idLength));
150+
MockLanguageServer.INSTANCE.getTextDocumentService().setPrepareRenameResult(Either.forLeft(prepareRange));
151+
152+
// WorkspaceEdit returned by textDocument/rename: both occurrences updated to
153+
// typedName.
154+
// This verifies that scheduleRenameJob():
155+
// - restores the original name before calling textDocument/rename
156+
// - applies the WorkspaceEdit from the language server.
157+
var edits = new HashMap<String, List<TextEdit>>();
158+
String uri = LSPEclipseUtils.toUri(file).toString();
159+
edits.put(uri, List.of(
160+
new TextEdit(new Range(firstPos, new Position(firstPos.getLine(), firstPos.getCharacter() + idLength)),
161+
typedName),
162+
new TextEdit(
163+
new Range(secondPos, new Position(secondPos.getLine(), secondPos.getCharacter() + idLength)),
164+
typedName)));
165+
MockLanguageServer.INSTANCE.getTextDocumentService().setRenameEdit(new WorkspaceEdit(edits));
166+
167+
// Open a viewer so that the document is wired to a text viewer like in real
168+
// usage
169+
final ITextViewer viewer = TestUtils.openTextViewer(file);
170+
171+
// Manually construct LSPInlineRenameLinkedMode to avoid brittle UI key-event
172+
// simulation
173+
var constructor = LSPInlineRenameLinkedMode.class.getDeclaredConstructor(IDocument.class, ITextViewer.class,
174+
int.class, IRegion.class, String.class, List.class, LanguageServerWrapper.class);
175+
constructor.setAccessible(true);
176+
177+
var renameRegion = new Region(firstOffset, idLength);
178+
List<IRegion> occurrences = List.of(renameRegion, new Region(secondOffset, idLength));
179+
var mode = constructor.newInstance(document, viewer, Integer.valueOf(firstOffset), renameRegion, "compute",
180+
occurrences, null);
181+
182+
// Simulate that the user has typed a new name in the primary linked position.
183+
// We update the document and the internal LinkedPosition that scheduleRenameJob
184+
// reads.
185+
int delta = typedName.length() - idLength;
186+
document.replace(firstOffset, idLength, typedName);
187+
document.replace(secondOffset + delta, idLength, typedName);
188+
189+
var linkedPosField = LSPInlineRenameLinkedMode.class.getDeclaredField("linkedPosition");
190+
linkedPosField.setAccessible(true);
191+
linkedPosField.set(mode, new LinkedPosition(document, firstOffset, typedName.length()));
192+
193+
// Invoke scheduleRenameJob() directly; it will:
194+
// - restore the original identifier
195+
// - call textDocument/rename(typedName)
196+
// - apply the WorkspaceEdit we configured above.
197+
Method schedule = LSPInlineRenameLinkedMode.class.getDeclaredMethod("scheduleRenameJob");
198+
schedule.setAccessible(true);
199+
schedule.invoke(mode);
200+
201+
waitForAndAssertCondition("Inline rename workspace edit not applied", 5_000, () -> {
202+
assertEquals(typedName + "();\n" + typedName + "();", document.get());
203+
return true;
204+
});
205+
}
206+
207+
}

‎org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/rename/RenameTest.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@
3737
import org.eclipse.core.resources.IFile;
3838
import org.eclipse.core.runtime.CoreException;
3939
import org.eclipse.core.runtime.NullProgressMonitor;
40+
import org.eclipse.core.runtime.preferences.InstanceScope;
4041
import org.eclipse.jface.dialogs.Dialog;
4142
import org.eclipse.jface.text.IDocument;
4243
import org.eclipse.lsp4e.LSPEclipseUtils;
44+
import org.eclipse.lsp4e.LanguageServerPlugin;
4345
import org.eclipse.lsp4e.operations.rename.LSPRenameProcessor;
4446
import org.eclipse.lsp4e.test.utils.AbstractTestWithProject;
4547
import org.eclipse.lsp4e.test.utils.TestUtils;
@@ -211,6 +213,9 @@ public void testRenameChangeAlsoExternalFile() throws Exception {
211213

212214
@Test
213215
public void testRenameHandlerExecution() throws Exception {
216+
// This test expects the classic dialog-based rename, so disable inline mode
217+
InstanceScope.INSTANCE.getNode(LanguageServerPlugin.PLUGIN_ID)
218+
.putBoolean("org.eclipse.lsp4e.inlineRename", false); //$NON-NLS-1$
214219
IFile file = TestUtils.createUniqueTestFile(project, "old");
215220
MockLanguageServer.INSTANCE.getTextDocumentService().setRenameEdit(createSimpleMockRenameEdit(LSPEclipseUtils.toUri(file)));
216221
final var editor = (ITextEditor) TestUtils.openEditor(file);

‎org.eclipse.lsp4e/src/org/eclipse/lsp4e/internal/IdentifierUtil.java‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2025 Vegard IT GmbH and others.
3+
* This program and the accompanying materials are made
4+
* available under the terms of the Eclipse Public License 2.0
5+
* which is available at https://www.eclipse.org/legal/epl-2.0/
6+
*
7+
* SPDX-License-Identifier: EPL-2.0
8+
*
9+
* Contributors:
10+
* Sebastian Thomschke (Vegard IT GmbH) - initial implementation.
11+
*******************************************************************************/
12+
package org.eclipse.lsp4e.internal;
13+
14+
import org.eclipse.jface.text.BadLocationException;
15+
import org.eclipse.jface.text.IDocument;
16+
import org.eclipse.jface.text.Region;
17+
import org.eclipse.lsp4e.LSPEclipseUtils;
18+
import org.eclipse.lsp4j.Range;
19+
20+
/**
21+
* Utility methods for computing identifier regions around a given offset.
22+
* <p>
23+
* In this context an identifier is a contiguous sequence of characters for
24+
* which {@link Character#isUnicodeIdentifierPart(char)} returns {@code true}.
25+
* </p>
26+
*/
27+
public final class IdentifierUtil {
28+
29+
/**
30+
* Computes the identifier range (LSP {@link Range}) around the given offset.
31+
*
32+
* @param document
33+
* the document
34+
* @param offset
35+
* the offset inside the document
36+
* @return the identifier range
37+
* @throws BadLocationException
38+
* if the offset is outside the document
39+
*/
40+
public static Range computeIdentifierRange(final IDocument document, int offset) throws BadLocationException {
41+
final Region region = computeIdentifierRegion(document, offset);
42+
final int start = region.getOffset();
43+
final int end = start + region.getLength();
44+
return new Range(LSPEclipseUtils.toPosition(start, document), LSPEclipseUtils.toPosition(end, document));
45+
}
46+
47+
/**
48+
* Computes the identifier region (Eclipse {@link Region}) around the given
49+
* offset, expanding to include all adjacent identifier characters.
50+
*
51+
* @param document
52+
* the document
53+
* @param offset
54+
* the offset inside the document
55+
* @return the identifier region
56+
* @throws BadLocationException
57+
* if the offset is outside the document
58+
*/
59+
public static Region computeIdentifierRegion(final IDocument document, int offset) throws BadLocationException {
60+
final int docLength = document.getLength();
61+
if (offset < 0 || offset > docLength) {
62+
throw new BadLocationException();
63+
}
64+
int start = offset;
65+
while (start > 0 && isIdentifierPart(document, start - 1)) {
66+
start--;
67+
}
68+
int end = offset;
69+
while (end < docLength && isIdentifierPart(document, end)) {
70+
end++;
71+
}
72+
return new Region(start, end - start);
73+
}
74+
75+
/**
76+
* Returns whether the given character is considered part of an identifier.
77+
* Delegates to {@link Character#isUnicodeIdentifierPart}.
78+
*/
79+
public static boolean isIdentifierPart(final char ch) {
80+
return Character.isUnicodeIdentifierPart(ch);
81+
}
82+
83+
/**
84+
* Returns whether the character at the given document offset is considered part
85+
* of an identifier.
86+
*
87+
* @param document
88+
* the document
89+
* @param offset
90+
* the character offset
91+
* @return {@code true} if the character is an identifier part
92+
* @throws BadLocationException
93+
* if the offset is outside the document
94+
*/
95+
public static boolean isIdentifierPart(final IDocument document, final int offset) throws BadLocationException {
96+
return isIdentifierPart(document.getChar(offset));
97+
}
98+
99+
private IdentifierUtil() {
100+
}
101+
}

‎org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/LSPTextHover.java‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import org.eclipse.lsp4e.LanguageServerPlugin;
5151
import org.eclipse.lsp4e.LanguageServers;
5252
import org.eclipse.lsp4e.internal.CancellationUtil;
53+
import org.eclipse.lsp4e.internal.IdentifierUtil;
5354
import org.eclipse.lsp4j.Hover;
5455
import org.eclipse.lsp4j.HoverParams;
5556
import org.eclipse.lsp4j.MarkedString;
@@ -191,28 +192,13 @@ public class LSPTextHover implements ITextHover, ITextHoverExtension, ITextHover
191192

192193
private static Region computeHeuristicRegion(final IDocument document, final int offset) {
193194
try {
194-
final int length = document.getLength();
195-
if (offset < 0 || offset > length) {
196-
return new Region(Math.max(0, Math.min(offset, length)), 0);
197-
}
198-
int start = offset;
199-
int end = offset;
200-
while (start > 0 && isWordPart(document.getChar(start - 1))) {
201-
start--;
202-
}
203-
while (end < length && isWordPart(document.getChar(end))) {
204-
end++;
205-
}
206-
return new Region(start, Math.max(0, end - start));
195+
return IdentifierUtil.computeIdentifierRegion(document, offset);
207196
} catch (final BadLocationException ex) {
208-
return new Region(offset, 0);
197+
final int safeOffset = Math.max(0, Math.min(offset, document.getLength()));
198+
return new Region(safeOffset, 0);
209199
}
210200
}
211201

212-
private static boolean isWordPart(char c) {
213-
return Character.isLetterOrDigit(c) || c == '_' || c == '$';
214-
}
215-
216202
/**
217203
* Cancel the last call of 'hover'.
218204
*/

0 commit comments

Comments
 (0)