From d2e62a03e773b9cbb78fca929a4a3f0050e6bf03 Mon Sep 17 00:00:00 2001 From: Valeriy Svydenko Date: Wed, 20 Jun 2018 10:49:06 +0200 Subject: [PATCH] Show warning dialog after refactoring operation if something isn't valid (#10080) Signed-off-by: Valeriy Svydenko --- .../move/wizard/MovePresenter.java | 8 +-- .../rename/JavaRefactoringRename.java | 29 ++++++++-- .../rename/wizard/RenamePresenter.java | 55 +++++++++++++++++-- .../JavaLanguageExtensionServiceClient.java | 10 ++-- .../JavaLanguageServerExtensionService.java | 25 +++++---- .../methods/RenameStaticMethodsTest.java | 2 +- 6 files changed, 101 insertions(+), 28 deletions(-) diff --git a/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/move/wizard/MovePresenter.java b/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/move/wizard/MovePresenter.java index fd4acf55e86..bb067ccbb1d 100644 --- a/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/move/wizard/MovePresenter.java +++ b/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/move/wizard/MovePresenter.java @@ -157,8 +157,8 @@ public void onPreviewButtonClicked() { extensionService .move(moveSettings) .then( - workspaceEdit -> { - previewPresenter.show(workspaceEdit, this); + refactoringResult -> { + previewPresenter.show(refactoringResult.getCheWorkspaceEdit(), this); }) .catchError( error -> { @@ -174,9 +174,9 @@ public void onAcceptButtonClicked() { extensionService .move(moveSettings) .then( - workspaceEdit -> { + refactoringResult -> { view.close(); - applyWorkspaceEditAction.applyWorkspaceEdit(workspaceEdit); + applyWorkspaceEditAction.applyWorkspaceEdit(refactoringResult.getCheWorkspaceEdit()); setEditorFocus(); }) .catchError( diff --git a/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/rename/JavaRefactoringRename.java b/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/rename/JavaRefactoringRename.java index d687bea3749..8242b047c65 100644 --- a/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/rename/JavaRefactoringRename.java +++ b/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/rename/JavaRefactoringRename.java @@ -14,6 +14,7 @@ import static org.eclipse.che.ide.api.editor.events.FileEvent.FileOperation.CLOSE; import static org.eclipse.che.ide.api.notification.StatusNotification.DisplayMode.FLOAT_MODE; import static org.eclipse.che.ide.api.notification.StatusNotification.Status.FAIL; +import static org.eclipse.che.jdt.ls.extension.api.RefactoringSeverity.FATAL; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -44,6 +45,8 @@ import org.eclipse.che.ide.ext.java.client.service.JavaLanguageExtensionServiceClient; import org.eclipse.che.ide.ui.dialogs.DialogFactory; import org.eclipse.che.jdt.ls.extension.api.RenameKind; +import org.eclipse.che.jdt.ls.extension.api.dto.RefactoringStatus; +import org.eclipse.che.jdt.ls.extension.api.dto.RefactoringStatusEntry; import org.eclipse.che.jdt.ls.extension.api.dto.RenameSettings; import org.eclipse.che.plugin.languageserver.ide.editor.quickassist.ApplyWorkspaceEditAction; import org.eclipse.che.plugin.languageserver.ide.service.TextDocumentServiceClient; @@ -308,12 +311,21 @@ private void performRename(String newName) { extensionServiceClient .rename(settings) .then( - edits -> { + result -> { enableAutoSave(); undoChanges(); - applyWorkspaceEditAction.applyWorkspaceEdit(edits); - clientServerEventService.sendFileTrackingResumeEvent(); - sendOpenEvent(); + RefactoringStatus refactoringStatus = result.getRefactoringStatus(); + if (!FATAL.equals(refactoringStatus.getRefactoringSeverity())) { + applyWorkspaceEditAction.applyWorkspaceEdit(result.getCheWorkspaceEdit()); + clientServerEventService.sendFileTrackingResumeEvent(); + sendOpenEvent(); + } else { + notificationManager.notify( + locale.failedToRename(), + getErrorMessage(refactoringStatus.getRefactoringStatusEntries()), + FAIL, + FLOAT_MODE); + } }) .catchError( error -> { @@ -325,6 +337,15 @@ private void performRename(String newName) { }); } + private String getErrorMessage(List entries) { + for (RefactoringStatusEntry entry : entries) { + if (FATAL.equals(entry.getRefactoringSeverity())) { + return entry.getMessage(); + } + } + return ""; + } + private void enableAutoSave() { if (linkedEditor instanceof EditorWithAutoSave) { ((EditorWithAutoSave) linkedEditor).enableAutoSave(); diff --git a/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/rename/wizard/RenamePresenter.java b/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/rename/wizard/RenamePresenter.java index cabc05b5910..090ef2dcdae 100644 --- a/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/rename/wizard/RenamePresenter.java +++ b/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/refactoring/rename/wizard/RenamePresenter.java @@ -18,6 +18,7 @@ import com.google.inject.Inject; import com.google.inject.Singleton; +import java.util.List; import org.eclipse.che.api.promises.client.Promise; import org.eclipse.che.ide.api.editor.EditorAgent; import org.eclipse.che.ide.api.editor.EditorPartPresenter; @@ -35,8 +36,13 @@ import org.eclipse.che.ide.ext.java.client.refactoring.rename.wizard.RenameView.ActionDelegate; import org.eclipse.che.ide.ext.java.client.refactoring.rename.wizard.similarnames.SimilarNamesConfigurationPresenter; import org.eclipse.che.ide.ext.java.client.service.JavaLanguageExtensionServiceClient; +import org.eclipse.che.ide.ui.dialogs.DialogFactory; +import org.eclipse.che.ide.ui.dialogs.confirm.ConfirmCallback; +import org.eclipse.che.jdt.ls.extension.api.RefactoringSeverity; import org.eclipse.che.jdt.ls.extension.api.RenameKind; import org.eclipse.che.jdt.ls.extension.api.dto.CheWorkspaceEdit; +import org.eclipse.che.jdt.ls.extension.api.dto.RefactoringResult; +import org.eclipse.che.jdt.ls.extension.api.dto.RefactoringStatusEntry; import org.eclipse.che.jdt.ls.extension.api.dto.RenameSelectionParams; import org.eclipse.che.jdt.ls.extension.api.dto.RenameSettings; import org.eclipse.che.jdt.ls.extension.api.dto.RenamingElementInfo; @@ -58,6 +64,7 @@ public class RenamePresenter implements ActionDelegate, RefactoringActionDelegat private final ApplyWorkspaceEditAction applyWorkspaceEditAction; private final JavaLocalizationConstant locale; private final DtoBuildHelper dtoBuildHelper; + private final DialogFactory dialogFactory; private final EditorAgent editorAgent; private final NotificationManager notificationManager; private final PreviewPresenter previewPresenter; @@ -73,6 +80,7 @@ public RenamePresenter( ApplyWorkspaceEditAction applyWorkspaceEditAction, JavaLocalizationConstant locale, DtoBuildHelper dtoBuildHelper, + DialogFactory dialogFactory, EditorAgent editorAgent, NotificationManager notificationManager, PreviewPresenter previewPresenter, @@ -83,6 +91,7 @@ public RenamePresenter( this.applyWorkspaceEditAction = applyWorkspaceEditAction; this.locale = locale; this.dtoBuildHelper = dtoBuildHelper; + this.dialogFactory = dialogFactory; this.editorAgent = editorAgent; this.notificationManager = notificationManager; this.view.setDelegate(this); @@ -199,7 +208,23 @@ public void onPreviewButtonClicked() { /** {@inheritDoc} */ @Override public void onAcceptButtonClicked() { - getChanges().then(this::applyRefactoring); + getChanges() + .then( + refactoringResult -> { + RefactoringSeverity severity = + refactoringResult.getRefactoringStatus().getRefactoringSeverity(); + switch (severity) { + case WARNING: + case ERROR: + showWarningDialog(refactoringResult); + break; + case FATAL: + view.showErrorMessage(refactoringResult.getRefactoringStatus()); + break; + default: + applyRefactoring(refactoringResult.getCheWorkspaceEdit()); + } + }); } /** {@inheritDoc} */ @@ -276,13 +301,17 @@ public void validateName() { private void showPreview() { getChanges() .then( - workspaceEdit -> { - previewPresenter.show(workspaceEdit, this); + refactoringResult -> { + CheWorkspaceEdit edit = refactoringResult.getCheWorkspaceEdit(); + if (edit == null) { + return; + } + previewPresenter.show(edit, this); previewPresenter.setTitle(locale.renameItemTitle()); }); } - private Promise getChanges() { + private Promise getChanges() { RenameSettings renameSettings = createRenameSettings(); RenameParams renameParams = createRenameParams(renameSettings); @@ -335,6 +364,24 @@ private void applyRefactoring(CheWorkspaceEdit workspaceEdit) { setEditorFocus(); } + private void showWarningDialog(RefactoringResult refactoringResult) { + List entries = + refactoringResult.getRefactoringStatus().getRefactoringStatusEntries(); + + ConfirmCallback confirmCallback = + () -> applyRefactoring(refactoringResult.getCheWorkspaceEdit()); + + dialogFactory + .createConfirmDialog( + locale.warningOperationTitle(), + entries.isEmpty() ? locale.warningOperationContent() : entries.get(0).getMessage(), + locale.renameRename(), + locale.buttonCancel(), + confirmCallback, + () -> {}) + .show(); + } + private RenameSettings createRenameSettings() { RenameSettings renameSettings = dtoFactory.createDto(RenameSettings.class); renameSettings.setDelegateUpdating(view.isUpdateDelegateUpdating()); diff --git a/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/service/JavaLanguageExtensionServiceClient.java b/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/service/JavaLanguageExtensionServiceClient.java index 642f38365b4..719e133a98b 100644 --- a/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/service/JavaLanguageExtensionServiceClient.java +++ b/plugins/plugin-java/che-plugin-java-ext-lang-client/src/main/java/org/eclipse/che/ide/ext/java/client/service/JavaLanguageExtensionServiceClient.java @@ -51,7 +51,6 @@ import org.eclipse.che.api.promises.client.js.RejectFunction; import org.eclipse.che.ide.api.app.AppContext; import org.eclipse.che.ide.api.workspace.WorkspaceReadyEvent; -import org.eclipse.che.jdt.ls.extension.api.dto.CheWorkspaceEdit; import org.eclipse.che.jdt.ls.extension.api.dto.ClasspathEntry; import org.eclipse.che.jdt.ls.extension.api.dto.CreateMoveParams; import org.eclipse.che.jdt.ls.extension.api.dto.ExtendedSymbolInformation; @@ -66,6 +65,7 @@ import org.eclipse.che.jdt.ls.extension.api.dto.OrganizeImportParams; import org.eclipse.che.jdt.ls.extension.api.dto.OrganizeImportsResult; import org.eclipse.che.jdt.ls.extension.api.dto.ReImportMavenProjectsCommandParameters; +import org.eclipse.che.jdt.ls.extension.api.dto.RefactoringResult; import org.eclipse.che.jdt.ls.extension.api.dto.RefactoringStatus; import org.eclipse.che.jdt.ls.extension.api.dto.RenameSelectionParams; import org.eclipse.che.jdt.ls.extension.api.dto.RenameSettings; @@ -295,7 +295,7 @@ public Promise libraryEntry(String resourceUri) { } /** Rename refactoring. */ - public Promise rename(RenameSettings renameSettings) { + public Promise rename(RenameSettings renameSettings) { return Promises.create( (resolve, reject) -> requestTransmitter @@ -303,7 +303,7 @@ public Promise rename(RenameSettings renameSettings) { .endpointId(WS_AGENT_JSON_RPC_ENDPOINT_ID) .methodName(REFACTORING_RENAME) .paramsAsDto(renameSettings) - .sendAndReceiveResultAsDto(CheWorkspaceEdit.class, REQUEST_TIMEOUT) + .sendAndReceiveResultAsDto(RefactoringResult.class, REQUEST_TIMEOUT) .onSuccess(resolve::apply) .onTimeout(() -> onTimeout(reject)) .onFailure(error -> reject.apply(ServiceUtil.getPromiseError(error)))); @@ -355,7 +355,7 @@ public Promise> getLinkedModeModel(TextDocumentPositionParams linked } /** Move refactoring. */ - public Promise move(MoveSettings moveSettings) { + public Promise move(MoveSettings moveSettings) { return Promises.create( (resolve, reject) -> requestTransmitter @@ -363,7 +363,7 @@ public Promise move(MoveSettings moveSettings) { .endpointId(WS_AGENT_JSON_RPC_ENDPOINT_ID) .methodName(REFACTORING_MOVE) .paramsAsDto(moveSettings) - .sendAndReceiveResultAsDto(CheWorkspaceEdit.class, REQUEST_TIMEOUT) + .sendAndReceiveResultAsDto(RefactoringResult.class, REQUEST_TIMEOUT) .onSuccess(resolve::apply) .onTimeout(() -> onTimeout(reject)) .onFailure(error -> reject.apply(ServiceUtil.getPromiseError(error)))); diff --git a/plugins/plugin-java/che-plugin-java-server/src/main/java/org/eclipse/che/plugin/java/languageserver/JavaLanguageServerExtensionService.java b/plugins/plugin-java/che-plugin-java-server/src/main/java/org/eclipse/che/plugin/java/languageserver/JavaLanguageServerExtensionService.java index 134afe2743b..5705ba03627 100644 --- a/plugins/plugin-java/che-plugin-java-server/src/main/java/org/eclipse/che/plugin/java/languageserver/JavaLanguageServerExtensionService.java +++ b/plugins/plugin-java/che-plugin-java-server/src/main/java/org/eclipse/che/plugin/java/languageserver/JavaLanguageServerExtensionService.java @@ -116,6 +116,7 @@ import org.eclipse.che.jdt.ls.extension.api.dto.PackageFragment; import org.eclipse.che.jdt.ls.extension.api.dto.PackageFragmentRoot; import org.eclipse.che.jdt.ls.extension.api.dto.ReImportMavenProjectsCommandParameters; +import org.eclipse.che.jdt.ls.extension.api.dto.RefactoringResult; import org.eclipse.che.jdt.ls.extension.api.dto.RefactoringStatus; import org.eclipse.che.jdt.ls.extension.api.dto.RenameSelectionParams; import org.eclipse.che.jdt.ls.extension.api.dto.RenameSettings; @@ -288,7 +289,7 @@ public void configureMethods() { .newConfiguration() .methodName(REFACTORING_RENAME) .paramsAsDto(RenameSettings.class) - .resultAsDto(CheWorkspaceEdit.class) + .resultAsDto(RefactoringResult.class) .withFunction(this::rename); requestHandler @@ -323,7 +324,7 @@ public void configureMethods() { .newConfiguration() .methodName(REFACTORING_MOVE) .paramsAsDto(MoveSettings.class) - .resultAsDto(CheWorkspaceEdit.class) + .resultAsDto(RefactoringResult.class) .withFunction(this::move); requestHandler @@ -681,17 +682,19 @@ private JarEntry getLibraryEntry(String resourceUri) { return doGetOne(GET_LIBRARY_ENTRY_COMMAND, singletonList(fixJdtUri(resourceUri)), type); } - private CheWorkspaceEdit rename(RenameSettings renameSettings) { - Type type = new TypeToken() {}.getType(); + private RefactoringResult rename(RenameSettings renameSettings) { + Type type = new TypeToken() {}.getType(); String uri = renameSettings.getRenameParams().getTextDocument().getUri(); renameSettings.getRenameParams().getTextDocument().setUri(prefixURI(uri)); - CheWorkspaceEdit cheWorkspaceEdit = + RefactoringResult refactoringResult = doGetOne(RENAME_COMMAND, singletonList(renameSettings), type); + CheWorkspaceEdit cheWorkspaceEdit = refactoringResult.getCheWorkspaceEdit(); List resourceChanges = getResourceChanges(cheWorkspaceEdit); cheWorkspaceEdit.setCheResourceChanges(resourceChanges); cheWorkspaceEdit.setChanges(getTextChanges(cheWorkspaceEdit)); - return cheWorkspaceEdit; + refactoringResult.setCheWorkspaceEdit(cheWorkspaceEdit); + return refactoringResult; } private List getResourceChanges(CheWorkspaceEdit cheWorkspaceEdit) { @@ -770,8 +773,8 @@ private void convertUrisToPath(List projectStructures) { } } - private CheWorkspaceEdit move(MoveSettings moveSettings) { - Type type = new TypeToken() {}.getType(); + private RefactoringResult move(MoveSettings moveSettings) { + Type type = new TypeToken() {}.getType(); String destinationUri = moveSettings.getDestination(); moveSettings.setDestination(prefixURI(destinationUri)); @@ -784,12 +787,14 @@ private CheWorkspaceEdit move(MoveSettings moveSettings) { moveSettings.setElements(resourceToMove); - CheWorkspaceEdit cheWorkspaceEdit = + RefactoringResult refactoringResult = doGetOne(Commands.MOVE_COMMAND, singletonList(moveSettings), type); + CheWorkspaceEdit cheWorkspaceEdit = refactoringResult.getCheWorkspaceEdit(); List resourceChanges = getResourceChanges(cheWorkspaceEdit); cheWorkspaceEdit.setCheResourceChanges(resourceChanges); cheWorkspaceEdit.setChanges(getTextChanges(cheWorkspaceEdit)); - return cheWorkspaceEdit; + refactoringResult.setCheWorkspaceEdit(cheWorkspaceEdit); + return refactoringResult; } private Boolean validateMove(CreateMoveParams moveParams) { diff --git a/selenium/che-selenium-test/src/test/java/org/eclipse/che/selenium/refactor/methods/RenameStaticMethodsTest.java b/selenium/che-selenium-test/src/test/java/org/eclipse/che/selenium/refactor/methods/RenameStaticMethodsTest.java index 84fb8c7649c..0f2230da851 100644 --- a/selenium/che-selenium-test/src/test/java/org/eclipse/che/selenium/refactor/methods/RenameStaticMethodsTest.java +++ b/selenium/che-selenium-test/src/test/java/org/eclipse/che/selenium/refactor/methods/RenameStaticMethodsTest.java @@ -47,7 +47,7 @@ public class RenameStaticMethodsTest { private static final String pathToPackageInChePrefix = NAME_OFP_ROJECT + "/src" + "/main" + "/java" + "/renameStaticMethods"; private static final String testsFail5ErrorMess = - "Related method 'm' (declared in 'renameStaticMethods.testFail5.A') is native. Renaming will cause an UnsatisfiedLinkError on runtime."; + "Renaming native methods will cause an unsatisfied link error on runtime."; private String pathToCurrentPackage; private String contentFromInA;