Skip to content

Commit 08a8f21

Browse files
sebthomsebthom
committed
fix: address PR review feedback
1 parent 0961cee commit 08a8f21

7 files changed

Lines changed: 91 additions & 90 deletions

File tree

‎org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/operations/rename/FileOperationParticipantsTest.java‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,9 @@ void testFilterGlobMatching() throws Exception {
7575
IFile file = TestUtils.createUniqueTestFile(project, "content");
7676
TestUtils.openTextViewer(file);
7777
TestUtils.waitForAndAssertCondition(5_000, () -> LanguageServers.forProject(project).anyMatching());
78-
var servers = LSPFileOperationParticipantSupport.getServersWithFileOperation(file,
78+
var executor = LSPFileOperationParticipantSupport.createFileOperationExecutor(file,
7979
FileOperationsServerCapabilities::getWillRename);
80-
assertTrue(!servers.isEmpty());
80+
assertTrue(executor.anyMatching());
8181
}
8282

8383
@Test
@@ -90,16 +90,16 @@ void testWillRename() throws Exception {
9090
// Ensure an LS is available
9191
assertTrue(LanguageServers.forProject(project).anyMatching());
9292

93-
var servers = LSPFileOperationParticipantSupport.getServersWithFileOperation(file,
93+
var executor = LSPFileOperationParticipantSupport.createFileOperationExecutor(file,
9494
FileOperationsServerCapabilities::getWillRename);
95-
assertTrue(!servers.isEmpty());
95+
assertTrue(executor.anyMatching());
9696

9797
var params = new RenameFilesParams();
9898
URI newUri = LSPEclipseUtils.toUri(project.getFile("renamed-" + file.getName()));
9999
params.getFiles().add(new FileRename(uri.toString(), newUri.toString()));
100100

101101
// Exercise helper to trigger server call
102-
LSPFileOperationParticipantSupport.computePreChange("rename", params, servers,
102+
LSPFileOperationParticipantSupport.computePreChange("rename", params, executor,
103103
(ws, p) -> ws.willRenameFiles(p));
104104

105105
MockWorkspaceService ws = MockLanguageServer.INSTANCE.getWorkspaceService();
@@ -116,14 +116,14 @@ void testWillCreate() throws Exception {
116116
URI uri = LSPEclipseUtils.toUri(file);
117117
assertNotNull(uri);
118118

119-
var servers = LSPFileOperationParticipantSupport.getServersWithFileOperation(file,
119+
var executor = LSPFileOperationParticipantSupport.createFileOperationExecutor(file,
120120
FileOperationsServerCapabilities::getWillCreate);
121-
assertTrue(!servers.isEmpty());
121+
assertTrue(executor.anyMatching());
122122

123123
var params = new CreateFilesParams();
124124
params.getFiles().add(new FileCreate(uri.toString()));
125125

126-
LSPFileOperationParticipantSupport.computePreChange("create", params, servers,
126+
LSPFileOperationParticipantSupport.computePreChange("create", params, executor,
127127
(ws, p) -> ws.willCreateFiles(p));
128128

129129
MockWorkspaceService ws = MockLanguageServer.INSTANCE.getWorkspaceService();
@@ -139,14 +139,14 @@ void testWillDelete() throws Exception {
139139
URI uri = LSPEclipseUtils.toUri(file);
140140
assertNotNull(uri);
141141

142-
var servers = LSPFileOperationParticipantSupport.getServersWithFileOperation(file,
142+
var executor = LSPFileOperationParticipantSupport.createFileOperationExecutor(file,
143143
FileOperationsServerCapabilities::getWillDelete);
144-
assertTrue(!servers.isEmpty());
144+
assertTrue(executor.anyMatching());
145145

146146
var params = new DeleteFilesParams();
147147
params.getFiles().add(new FileDelete(uri.toString()));
148148

149-
LSPFileOperationParticipantSupport.computePreChange("delete", params, servers,
149+
LSPFileOperationParticipantSupport.computePreChange("delete", params, executor,
150150
(ws, p) -> ws.willDeleteFiles(p));
151151

152152
MockWorkspaceService ws = MockLanguageServer.INSTANCE.getWorkspaceService();

‎org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/operations/rename/FolderOperationParticipantsTest.java‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ void testFolderFilterGlobMatching() throws Exception {
7070
folder.create(true, true, null);
7171
}
7272

73-
var servers = LSPFileOperationParticipantSupport.getServersWithFileOperation(folder,
73+
var executor = LSPFileOperationParticipantSupport.createFileOperationExecutor(folder,
7474
FileOperationsServerCapabilities::getWillRename);
75-
assertTrue(!servers.isEmpty());
75+
assertTrue(executor.anyMatching());
7676
}
7777

7878
@Test
@@ -100,15 +100,15 @@ void testFolderWillRename() throws Exception {
100100
URI oldUri = LSPEclipseUtils.toUri(folder);
101101
assertNotNull(oldUri);
102102

103-
var servers = LSPFileOperationParticipantSupport.getServersWithFileOperation(folder,
103+
var executor = LSPFileOperationParticipantSupport.createFileOperationExecutor(folder,
104104
FileOperationsServerCapabilities::getWillRename);
105-
assertTrue(!servers.isEmpty());
105+
assertTrue(executor.anyMatching());
106106

107107
var params = new RenameFilesParams();
108108
URI newUri = LSPEclipseUtils.toUri(project.getFolder("newDir"));
109109
params.getFiles().add(new FileRename(oldUri.toString(), newUri.toString()));
110110

111-
LSPFileOperationParticipantSupport.computePreChange("rename-folder", params, servers,
111+
LSPFileOperationParticipantSupport.computePreChange("rename-folder", params, executor,
112112
(ws, p) -> ws.willRenameFiles(p));
113113

114114
MockWorkspaceService ws = MockLanguageServer.INSTANCE.getWorkspaceService();
@@ -138,14 +138,14 @@ void testFolderWillCreate() throws Exception {
138138
URI uri = LSPEclipseUtils.toUri(folder);
139139
assertNotNull(uri);
140140

141-
var servers = LSPFileOperationParticipantSupport.getServersWithFileOperation(folder,
141+
var executor = LSPFileOperationParticipantSupport.createFileOperationExecutor(folder,
142142
FileOperationsServerCapabilities::getWillCreate);
143-
assertTrue(!servers.isEmpty());
143+
assertTrue(executor.anyMatching());
144144

145145
var params = new CreateFilesParams();
146146
params.getFiles().add(new FileCreate(uri.toString()));
147147

148-
LSPFileOperationParticipantSupport.computePreChange("create-folder", params, servers,
148+
LSPFileOperationParticipantSupport.computePreChange("create-folder", params, executor,
149149
(ws, p) -> ws.willCreateFiles(p));
150150

151151
MockWorkspaceService ws = MockLanguageServer.INSTANCE.getWorkspaceService();
@@ -177,14 +177,13 @@ void testFolderWillDelete() throws Exception {
177177
URI uri = LSPEclipseUtils.toUri(folder);
178178
assertNotNull(uri);
179179

180-
var servers = LSPFileOperationParticipantSupport.getServersWithFileOperation(folder,
180+
var executor = LSPFileOperationParticipantSupport.createFileOperationExecutor(folder,
181181
FileOperationsServerCapabilities::getWillDelete);
182-
assertTrue(!servers.isEmpty());
183182

184183
var params = new DeleteFilesParams();
185184
params.getFiles().add(new FileDelete(uri.toString()));
186185

187-
LSPFileOperationParticipantSupport.computePreChange("delete-folder", params, servers,
186+
LSPFileOperationParticipantSupport.computePreChange("delete-folder", params, executor,
188187
(ws, p) -> ws.willDeleteFiles(p));
189188

190189
MockWorkspaceService ws = MockLanguageServer.INSTANCE.getWorkspaceService();

‎org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/rename/LSPCreateParticipant.java‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,9 @@
1111
*******************************************************************************/
1212
package org.eclipse.lsp4e.operations.rename;
1313

14-
import static org.eclipse.lsp4e.internal.NullSafetyHelper.*;
14+
import static org.eclipse.lsp4e.internal.NullSafetyHelper.lateNonNull;
1515

1616
import java.net.URI;
17-
import java.util.List;
1817

1918
import org.eclipse.core.resources.IFile;
2019
import org.eclipse.core.resources.IFolder;
@@ -24,7 +23,6 @@
2423
import org.eclipse.core.runtime.OperationCanceledException;
2524
import org.eclipse.jdt.annotation.Nullable;
2625
import org.eclipse.lsp4e.LSPEclipseUtils;
27-
import org.eclipse.lsp4e.LanguageServerWrapper;
2826
import org.eclipse.lsp4j.CreateFilesParams;
2927
import org.eclipse.lsp4j.FileCreate;
3028
import org.eclipse.lsp4j.FileOperationsServerCapabilities;
@@ -36,7 +34,7 @@
3634
public class LSPCreateParticipant extends CreateParticipant {
3735

3836
private URI newURI = lateNonNull();
39-
private List<LanguageServerWrapper> servers = lateNonNull();
37+
private IResource resource = lateNonNull();
4038

4139
@Override
4240
public String getName() {
@@ -45,16 +43,15 @@ public String getName() {
4543

4644
@Override
4745
protected boolean initialize(final Object element) {
48-
if (element instanceof IFile || element instanceof IFolder) {
49-
final var res = (IResource) element;
46+
if (element instanceof final IResource res && (res instanceof IFile || res instanceof IFolder)) {
47+
resource = res;
5048
final URI uri = LSPEclipseUtils.toUri(res);
5149
if (uri == null)
5250
return false;
5351
newURI = uri;
5452

55-
servers = LSPFileOperationParticipantSupport.getServersWithFileOperation(res,
56-
FileOperationsServerCapabilities::getWillCreate);
57-
return !servers.isEmpty();
53+
return LSPFileOperationParticipantSupport
54+
.createFileOperationExecutor(res, FileOperationsServerCapabilities::getWillCreate).anyMatching();
5855
}
5956

6057
return false;
@@ -77,7 +74,7 @@ public RefactoringStatus checkConditions(final IProgressMonitor monitor, final C
7774
throws CoreException, OperationCanceledException {
7875
final var params = new CreateFilesParams();
7976
params.getFiles().add(new FileCreate(newURI.toString()));
80-
return LSPFileOperationParticipantSupport.computePreChange(getName(), params, servers,
81-
(ws, p) -> ws.willCreateFiles(p));
77+
return LSPFileOperationParticipantSupport.computePreChange(getName(), params, resource,
78+
FileOperationsServerCapabilities::getWillCreate, (ws, p) -> ws.willCreateFiles(p));
8279
}
8380
}

‎org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/rename/LSPDeleteParticipant.java‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,9 @@
1111
*******************************************************************************/
1212
package org.eclipse.lsp4e.operations.rename;
1313

14-
import static org.eclipse.lsp4e.internal.NullSafetyHelper.*;
14+
import static org.eclipse.lsp4e.internal.NullSafetyHelper.lateNonNull;
1515

1616
import java.net.URI;
17-
import java.util.List;
1817

1918
import org.eclipse.core.resources.IFile;
2019
import org.eclipse.core.resources.IFolder;
@@ -24,7 +23,6 @@
2423
import org.eclipse.core.runtime.OperationCanceledException;
2524
import org.eclipse.jdt.annotation.Nullable;
2625
import org.eclipse.lsp4e.LSPEclipseUtils;
27-
import org.eclipse.lsp4e.LanguageServerWrapper;
2826
import org.eclipse.lsp4j.DeleteFilesParams;
2927
import org.eclipse.lsp4j.FileDelete;
3028
import org.eclipse.lsp4j.FileOperationsServerCapabilities;
@@ -36,7 +34,7 @@
3634
public class LSPDeleteParticipant extends DeleteParticipant {
3735

3836
private URI oldURI = lateNonNull();
39-
private List<LanguageServerWrapper> servers = lateNonNull();
37+
private IResource resource = lateNonNull();
4038

4139
@Override
4240
public String getName() {
@@ -45,15 +43,14 @@ public String getName() {
4543

4644
@Override
4745
protected boolean initialize(final Object element) {
48-
if (element instanceof IFile || element instanceof IFolder) {
49-
final var res = (IResource) element;
46+
if (element instanceof final IResource res && (res instanceof IFile || res instanceof IFolder)) {
47+
resource = res;
5048
final URI uri = LSPEclipseUtils.toUri(res);
5149
if (uri == null)
5250
return false;
5351
oldURI = uri;
54-
servers = LSPFileOperationParticipantSupport.getServersWithFileOperation(res,
55-
FileOperationsServerCapabilities::getWillDelete);
56-
return !servers.isEmpty();
52+
return LSPFileOperationParticipantSupport
53+
.createFileOperationExecutor(res, FileOperationsServerCapabilities::getWillDelete).anyMatching();
5754
}
5855
return false;
5956
}
@@ -75,7 +72,7 @@ public RefactoringStatus checkConditions(final IProgressMonitor monitor, final C
7572
throws CoreException, OperationCanceledException {
7673
final var params = new DeleteFilesParams();
7774
params.getFiles().add(new FileDelete(oldURI.toString()));
78-
return LSPFileOperationParticipantSupport.computePreChange(getName(), params, servers,
79-
(ws, p) -> ws.willDeleteFiles(p));
75+
return LSPFileOperationParticipantSupport.computePreChange(getName(), params, resource,
76+
FileOperationsServerCapabilities::getWillDelete, (ws, p) -> ws.willDeleteFiles(p));
8077
}
8178
}

‎org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/rename/LSPFileOperationParticipantSupport.java‎

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
package org.eclipse.lsp4e.operations.rename;
1313

1414
import java.nio.file.Path;
15-
import java.util.List;
1615
import java.util.Objects;
1716
import java.util.concurrent.CompletableFuture;
1817
import java.util.concurrent.TimeUnit;
@@ -23,8 +22,8 @@
2322
import org.eclipse.jdt.annotation.Nullable;
2423
import org.eclipse.lsp4e.LSPEclipseUtils;
2524
import org.eclipse.lsp4e.LanguageServerPlugin;
26-
import org.eclipse.lsp4e.LanguageServerWrapper;
2725
import org.eclipse.lsp4e.LanguageServers;
26+
import org.eclipse.lsp4e.LanguageServers.LanguageServerProjectExecutor;
2827
import org.eclipse.lsp4e.internal.files.PathPatternMatcher;
2928
import org.eclipse.lsp4j.FileOperationFilter;
3029
import org.eclipse.lsp4j.FileOperationOptions;
@@ -35,6 +34,11 @@
3534
import org.eclipse.ltk.core.refactoring.Change;
3635
import org.eclipse.ltk.core.refactoring.CompositeChange;
3736

37+
/**
38+
* Internal class, only public to be accessible by test cases.
39+
*
40+
* @noreference
41+
*/
3842
public final class LSPFileOperationParticipantSupport {
3943

4044
/**
@@ -44,22 +48,28 @@ public final class LSPFileOperationParticipantSupport {
4448
private static final long FILE_OP_TIMEOUT_SECONDS = 10;
4549

4650
public static <P> @Nullable Change computePreChange(final String changeName, final P params,
47-
final List<LanguageServerWrapper> servers,
51+
final IResource resource,
52+
final Function<FileOperationsServerCapabilities, @Nullable FileOperationOptions> optionsProvider,
4853
final BiFunction<WorkspaceService, P, CompletableFuture<@Nullable WorkspaceEdit>> request) {
54+
return computePreChange(changeName, params, createFileOperationExecutor(resource, optionsProvider), request);
55+
}
4956

50-
final CompositeChange[] changes = servers.stream() //
51-
.map(wrapper -> wrapper.execute(ls -> request //
52-
.apply(ls.getWorkspaceService(), params)) //
53-
.orTimeout(FILE_OP_TIMEOUT_SECONDS, TimeUnit.SECONDS).exceptionally(ex -> {
54-
LanguageServerPlugin.logWarning(
55-
"File operation pre-change '" + changeName + "' failed or timed out for server: " //$NON-NLS-1$ //$NON-NLS-2$
56-
+ wrapper.serverDefinition.label,
57-
ex);
58-
return null;
59-
}).thenApply(edits -> edits == null || isEmptyEdit(edits) //
60-
? (@Nullable CompositeChange) null
61-
: LSPEclipseUtils.toCompositeChange(edits, wrapper.serverDefinition.label))) //
62-
.map(CompletableFuture::join) //
57+
public static <P> @Nullable Change computePreChange(final String changeName, final P params,
58+
final LanguageServerProjectExecutor executor,
59+
final BiFunction<WorkspaceService, P, CompletableFuture<@Nullable WorkspaceEdit>> request) {
60+
final CompositeChange[] changes = executor.collectAll((wrapper, ls) -> request //
61+
.apply(ls.getWorkspaceService(), params) //
62+
.orTimeout(FILE_OP_TIMEOUT_SECONDS, TimeUnit.SECONDS).exceptionally(ex -> {
63+
LanguageServerPlugin.logWarning(
64+
"File operation pre-change '" + changeName + "' failed or timed out for server: " //$NON-NLS-1$ //$NON-NLS-2$
65+
+ wrapper.serverDefinition.label,
66+
ex);
67+
return null;
68+
}).thenApply(edits -> edits == null || isEmptyEdit(edits) //
69+
? (@Nullable CompositeChange) null
70+
: LSPEclipseUtils.toCompositeChange(edits, wrapper.serverDefinition.label))) //
71+
.join() //
72+
.stream() //
6373
.filter(Objects::nonNull) //
6474
.toArray(CompositeChange[]::new);
6575

@@ -70,24 +80,28 @@ public final class LSPFileOperationParticipantSupport {
7080
};
7181
}
7282

73-
public static List<LanguageServerWrapper> getServersWithFileOperation(final IResource res,
83+
public static LanguageServerProjectExecutor createFileOperationExecutor(final IResource res,
7484
final Function<FileOperationsServerCapabilities, @Nullable FileOperationOptions> optionsProvider) {
7585
final var uri = LSPEclipseUtils.toUri(res);
76-
if (uri == null)
77-
return List.of();
86+
final var project = res.getProject();
87+
if (uri == null) {
88+
// No URI means we cannot match any file operation filters; return an executor
89+
// that will not match any servers.
90+
return LanguageServers.forProject(project).withFilter(capabilities -> false);
91+
}
7892

7993
final var path = Path.of(uri);
80-
return LanguageServers.forProject(res.getProject()).withFilter(caps -> {
81-
final var wks = caps.getWorkspace();
82-
if (wks == null)
94+
return LanguageServers.forProject(project).withFilter(capabilities -> {
95+
final var workspace = capabilities.getWorkspace();
96+
if (workspace == null)
8397
return false;
84-
final var fileOps = wks.getFileOperations();
98+
final var fileOps = workspace.getFileOperations();
8599
if (fileOps == null)
86100
return false;
87101

88102
final var options = optionsProvider.apply(fileOps);
89103
return matches(options, path, res.getType() == IResource.FOLDER);
90-
}).collectAll((wrapper, ls) -> CompletableFuture.completedFuture(wrapper)).join();
104+
});
91105
}
92106

93107
private static boolean isEmptyEdit(final WorkspaceEdit edits) {

0 commit comments

Comments
 (0)