Support Rename refactoring #9636

svor · 2018-05-07T16:14:18Z

What does this PR do?

Make it possible to do Rename operation by using jdt.ls.
The result of this PR is here: https://youtu.be/HkCKpGSP05o

Depends on

What issues does this PR fix or reference?

tolusha · 2018-05-08T08:37:44Z

.../src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/preview/PreviewPresenter.java

-            });
+    applyWorkspaceEditAction.applyWorkspaceEdit(edit);
+    view.close();
+    refactoringActionDelegate.closeWizard();


It invokes onCancelButtonClicked();
I think view.close() is enough.

view.close() closes just Preview window, but also we need to close the other one (Rename or Move) window which was opened previously

tsmaeder · 2018-05-09T11:27:31Z

...t/src/main/java/org/eclipse/che/ide/ext/java/client/diagnostics/PomDiagnosticsRequestor.java

      final JavaLanguageExtensionServiceClient service) {
    eventBus.addHandler(
        EditorOpenedEvent.TYPE,
        new EditorOpenedEventHandler() {
          @Override
          public void onEditorOpened(EditorOpenedEvent event) {
            String uri = buildHelper.getUri(event.getFile());
-            if (uri.endsWith(POM_FILE)) {
+            if (!uri.endsWith(POM_FILE)) {


Drive-by fix?

that's right

tsmaeder · 2018-05-09T11:36:13Z

.../src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/preview/PreviewPresenter.java

+          if (!arg.isPresent()) {
+            return;
+          }
+          File existedFile = arg.get();


"existingFile"

tsmaeder · 2018-05-09T11:38:56Z

.../src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/preview/PreviewPresenter.java

+    }
+    PreviewNode node = nodes.get(selectedNode.getUri());
+    List<TextEdit> edits = new ArrayList<>();
+    if (node.getId().equals(selectedNode.getId())) {


Not quite sure what we are doing here. Refactor so it's obvious or add comments.

refactored and added some comments

tsmaeder · 2018-05-09T11:41:29Z

.../src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/preview/PreviewPresenter.java


-  private RefactorInfo refactorInfo;
-  private RefactoringSession session;
+  private Map<String, PreviewNode> nodes;


Are all nodes in this map? If not, this needs a different name. fileNodes?

renamed to fileNodes

tsmaeder · 2018-05-09T11:47:07Z

.../src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/preview/PreviewPresenter.java

+    this.refactoringActionDelegate = refactoringActionDelegate;
+    nodes = new LinkedHashMap<>();
+
+    prepareNodes(workspaceEdit);


As I see it, prepareNodes just fills in the nodes map. why not do
nodes= prepareNodes(workspaceEdit);
If you make prepareNodes static, you can show that there are no side-effects in the method. Making stuff side-effect free simplifies understanding the code.

tsmaeder · 2018-05-09T11:59:56Z

.../main/java/org/eclipse/che/ide/ext/java/client/refactoring/rename/JavaRefactoringRename.java

              }
+              undoChanges();


Could you clarify please?
If you're thinking about why I undo changes here. It is because before applying all TextEdit changes we need to have initial state of the content where linked mode was activated.

yes, that's what I was thinking of.

tsmaeder · 2018-05-09T12:05:15Z

...main/java/org/eclipse/che/ide/ext/java/client/refactoring/rename/wizard/RenamePresenter.java

+    edit.setChanges(workspaceEdit.getChanges());
+    edit.setResourceChanges(new LinkedList<>());
+    for (ResourceChange resourceChange : workspaceEdit.getResourceChanges()) {
+      edit.getResourceChanges().add(Either.forLeft(resourceChange));


Out of curiosity...I thought sending WorkspaceChange over the wire did not work? What has changed?

Sending List<Either<ResourceChange, TextEdit>> which is described in org.eclipse.lsp4j.WorkspaceEdit doesn't work.
So here workspaceEdit is an instance of custom object (org.eclipse.che.jdt.ls.extension.api.dto.CheWorkspaceEdit) where I already have all actual changes and I take all changes from it and put them to the edit (org.eclipse.lsp4j.WorkspaceEdit) to sent it to class which applies org.eclipse.lsp4j.WorkspaceEdit (org.eclipse.che.plugin.languageserver.ide.editor.quickassist.ApplyWorkspaceEditAction#ApplyWorkspaceEditAction)

tsmaeder · 2018-05-09T12:11:48Z

...a/org/eclipse/che/plugin/languageserver/ide/editor/quickassist/ApplyWorkspaceEditAction.java

+      String newUri = change.getNewUri();
+      String current = change.getCurrent();
+
+      Path path = toPath(newUri).removeFirstSegments(1).makeAbsolute();


Transformation of file uri's to paths is usually done in WSMaster, so results from lsp servers contain either workspace paths of non-file URI's. See LanguageServiceUtils.removePrefixURI.

GWT doesn't support java.net.* so we can't use LanguageServiceUtils.java on the client. I've created utility class org.eclipse.che.api.languageserver.util.URIUtil which transforms uri to path.

Not what I mean, I've tried to explain in chat.

tsmaeder · 2018-05-09T12:13:43Z

...a/org/eclipse/che/plugin/languageserver/ide/editor/quickassist/ApplyWorkspaceEditAction.java

+        .getResource(path)
+        .then(
+            resource -> {
+              if (isNullOrEmpty(path.getFileExtension())) {


I dont' think this is a valid condition. It's perfectly valid to create files without extensions and folders with.

yes, but I don't see another way to recognize which type of resource is here. Is it a folder or is a file.

I think we need to fix this in CheWorkspaceEdit (make CheResourceChange.java) and have a "resourceType" enum to be used on create.

tsmaeder · 2018-05-09T12:14:30Z

...a/org/eclipse/che/plugin/languageserver/ide/editor/quickassist/ApplyWorkspaceEditAction.java

+    resource
+        .move(path)
+        .then(
+            arg -> {


arg is not a good name.

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

svor added kind/enhancement A feature request - must adhere to the feature request template. team/languages labels May 7, 2018

svor self-assigned this May 7, 2018

svor requested review from dkulieshov, evidolob and vparfonov as code owners May 7, 2018 16:14

benoitf added target/branch Indicates that a PR will be merged into a branch other than master. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels May 7, 2018

svor changed the title ~~[WIP]Support Rename refactoring~~ Support Rename refactoring May 8, 2018

svor requested review from tolusha and tsmaeder May 8, 2018 07:29

tolusha reviewed May 8, 2018

View reviewed changes

vparfonov approved these changes May 8, 2018

View reviewed changes

tsmaeder requested changes May 9, 2018

View reviewed changes

tsmaeder added team/languages and removed team/languages labels May 11, 2018

tolusha force-pushed the 5730_java_ls_poc branch from 245c161 to 62ecb10 Compare May 12, 2018 19:47

svor requested review from ashumilova and benoitf as code owners May 12, 2018 21:37

svor requested a review from a user May 12, 2018 21:37

svor requested review from garagatyi, l0rd, riuvshin, skabashnyuk and sleshchenko as code owners May 12, 2018 21:37

svor force-pushed the rename branch from 31d6ffb to 81688be Compare May 12, 2018 21:49

svor removed request for benoitf, l0rd, ashumilova and skabashnyuk May 12, 2018 21:50

tsmaeder force-pushed the 5730_java_ls_poc branch 2 times, most recently from 6901019 to 518f5f6 Compare June 4, 2018 09:34

use jdt.ls to support Rename refactoring

73dca1a

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

svor force-pushed the rename branch from 6a742c4 to 73dca1a Compare June 4, 2018 14:19

tsmaeder mentioned this pull request Jun 5, 2018

Languages Sprint 150 #9804

Closed

22 tasks

svor merged commit 0558d31 into 5730_java_ls_poc Jun 5, 2018

svor deleted the rename branch June 5, 2018 12:36

benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jun 5, 2018

tsmaeder pushed a commit that referenced this pull request Jun 13, 2018

use jdt.ls to support Rename refactoring (#9636)

d4c04a3

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Jun 13, 2018

use jdt.ls to support Rename refactoring (#9636)

3dafece

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder mentioned this pull request Jun 18, 2018

Languages Sprint 151 #10075

Closed

26 tasks

tsmaeder pushed a commit that referenced this pull request Jun 26, 2018

use jdt.ls to support Rename refactoring (#9636)

8bde312

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Jul 13, 2018

use jdt.ls to support Rename refactoring (#9636)

67f361c

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Jul 27, 2018

use jdt.ls to support Rename refactoring (#9636)

50d54df

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tolusha pushed a commit that referenced this pull request Aug 7, 2018

use jdt.ls to support Rename refactoring (#9636)

528a188

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Aug 22, 2018

use jdt.ls to support Rename refactoring (#9636)

1003ccd

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Aug 29, 2018

use jdt.ls to support Rename refactoring (#9636)

4042dbb

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tolusha pushed a commit that referenced this pull request Sep 5, 2018

use jdt.ls to support Rename refactoring (#9636)

747143c

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Sep 13, 2018

use jdt.ls to support Rename refactoring (#9636)

aac7989

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Sep 20, 2018

use jdt.ls to support Rename refactoring (#9636)

b8ac5ef

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Sep 26, 2018

use jdt.ls to support Rename refactoring (#9636)

cf83a3d

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Oct 1, 2018

use jdt.ls to support Rename refactoring (#9636)

4088647

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Oct 5, 2018

use jdt.ls to support Rename refactoring (#9636)

6d4a58b

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Oct 12, 2018

use jdt.ls to support Rename refactoring (#9636)

9a98d20

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Oct 16, 2018

use jdt.ls to support Rename refactoring (#9636)

87f175a

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Oct 17, 2018

use jdt.ls to support Rename refactoring (#9636)

85004d5

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Oct 17, 2018

use jdt.ls to support Rename refactoring (#9636)

559294e

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Oct 17, 2018

use jdt.ls to support Rename refactoring (#9636)

a3adf16

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Oct 17, 2018

use jdt.ls to support Rename refactoring (#9636)

068336b

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

tsmaeder pushed a commit that referenced this pull request Oct 17, 2018

use jdt.ls to support Rename refactoring (#9636)

4a6be0f

Signed-off-by: Valeriy Svydenko <vsvydenk@redhat.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Rename refactoring #9636

Support Rename refactoring #9636

svor commented May 7, 2018

tolusha May 8, 2018

svor May 8, 2018

tsmaeder May 9, 2018

svor May 10, 2018

tsmaeder May 9, 2018

svor May 10, 2018

tsmaeder May 9, 2018

svor May 10, 2018

tsmaeder May 9, 2018

svor May 10, 2018

tsmaeder May 9, 2018

tsmaeder May 9, 2018

svor May 10, 2018

tsmaeder May 18, 2018

tsmaeder May 9, 2018

svor May 10, 2018

tsmaeder May 9, 2018

svor May 10, 2018

tsmaeder May 11, 2018

svor May 13, 2018

tsmaeder May 9, 2018

svor May 10, 2018

tsmaeder May 18, 2018

tsmaeder May 9, 2018

svor May 10, 2018

Support Rename refactoring #9636

Support Rename refactoring #9636

Conversation

svor commented May 7, 2018

What does this PR do?

What issues does this PR fix or reference?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment