Skip to content

Commit

Permalink
Enable quick fix to create non existing package when package declarat…
Browse files Browse the repository at this point in the history 
…ion mismatch

Signed-off-by: Jinbo Wang <jinbwan@microsoft.com>
  • Loading branch information
@testforstephen @fbricon
testforstephen authored and fbricon committed Sep 5, 2019
1 parent 98cdee5 commit 258729f
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 34 deletions.
13 changes: 10 additions & 3 deletions org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/ChangeUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.core.runtime.Path;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaElement;
import org.eclipse.jdt.core.IPackageDeclaration;
import org.eclipse.jdt.core.IPackageFragment;
import org.eclipse.jdt.core.IType;
import org.eclipse.jdt.core.JavaCore;
Expand Down Expand Up @@ -152,9 +154,14 @@ private static void convertMoveCompilationUnitChange(WorkspaceEdit edit, MoveCom
ICompilationUnit unit = change.getCu();
CompilationUnit astCU = RefactoringASTParser.parseWithASTProvider(unit, true, new NullProgressMonitor());
ASTRewrite rewrite = ASTRewrite.create(astCU.getAST());
// update the package declaration
updatePackageStatement(astCU, newPackage.getElementName(), rewrite, unit);
convertTextEdit(edit, unit, rewrite.rewriteAST());

IPackageDeclaration[] packDecls = unit.getPackageDeclarations();
String oldPackageName = packDecls.length > 0 ? packDecls[0].getElementName() : "";
if (!Objects.equals(oldPackageName, newPackage.getElementName())) {
// update the package declaration
updatePackageStatement(astCU, newPackage.getElementName(), rewrite, unit);
convertTextEdit(edit, unit, rewrite.rewriteAST());
}

RenameFile cuResourceChange = new RenameFile();
cuResourceChange.setOldUri(JDTUtils.toURI(unit));
Expand Down
25 changes: 25 additions & 0 deletions .../org/eclipse/jdt/ls/core/internal/corrections/proposals/ReorgCorrectionsSubProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
import org.eclipse.core.runtime.IStatus;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.IPackageDeclaration;
import org.eclipse.jdt.core.IPackageFragment;
import org.eclipse.jdt.core.IPackageFragmentRoot;
import org.eclipse.jdt.core.JavaConventions;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.dom.ASTNode;
Expand All @@ -37,9 +39,11 @@
import org.eclipse.jdt.internal.corext.util.Messages;
import org.eclipse.jdt.internal.ui.text.correction.IProblemLocationCore;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
import org.eclipse.jdt.ls.core.internal.corext.refactoring.changes.MoveCompilationUnitChange;
import org.eclipse.jdt.ls.core.internal.corext.refactoring.changes.RenameCompilationUnitChange;
import org.eclipse.jdt.ls.core.internal.corrections.CorrectionMessages;
import org.eclipse.jdt.ls.core.internal.corrections.IInvocationContext;
import org.eclipse.jdt.ls.core.internal.hover.JavaElementLabels;
import org.eclipse.jface.text.IDocument;
import org.eclipse.lsp4j.CodeActionKind;
import org.eclipse.text.edits.TextEdit;
Expand Down Expand Up @@ -115,6 +119,27 @@ public static void getWrongPackageDeclNameProposals(IInvocationContext context,
// correct package declaration
int relevance= cu.getPackageDeclarations().length == 0 ? IProposalRelevance.MISSING_PACKAGE_DECLARATION : IProposalRelevance.CORRECT_PACKAGE_DECLARATION; // bug 38357
proposals.add(new CorrectPackageDeclarationProposal(cu, problem, relevance));

// move to package
IPackageDeclaration[] packDecls= cu.getPackageDeclarations();
String newPackName= packDecls.length > 0 ? packDecls[0].getElementName() : ""; //$NON-NLS-1$

IPackageFragmentRoot root= JavaModelUtil.getPackageFragmentRoot(cu);
IPackageFragment newPack= root.getPackageFragment(newPackName);

ICompilationUnit newCU= newPack.getCompilationUnit(cu.getElementName());
boolean isLinked= cu.getResource().isLinked();
if (!newCU.exists() && !isLinked) {
String label;
if (newPack.isDefaultPackage()) {
label= Messages.format(CorrectionMessages.ReorgCorrectionsSubProcessor_movecu_default_description, BasicElementLabels.getFileName(cu));
} else {
String packageLabel= JavaElementLabels.getElementLabel(newPack, JavaElementLabels.ALL_DEFAULT);
label= Messages.format(CorrectionMessages.ReorgCorrectionsSubProcessor_movecu_description, new Object[] { BasicElementLabels.getFileName(cu), packageLabel });
}

proposals.add(new ChangeCorrectionProposal(label, CodeActionKind.QuickFix, new MoveCompilationUnitChange(cu, newPack), IProposalRelevance.MOVE_CU_TO_PACKAGE));
}
}

public static void removeImportStatementProposals(IInvocationContext context, IProblemLocationCore problem,
Expand Down
2 changes: 1 addition & 1 deletion ...se.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/correction/AbstractQuickFixTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ protected List<Either<Command, CodeAction>> evaluateCodeActions(ICompilationUnit
return codeActions;
}

private String evaluateCodeActionCommand(Either<Command, CodeAction> codeAction)
protected String evaluateCodeActionCommand(Either<Command, CodeAction> codeAction)
throws BadLocationException, JavaModelException {

Command c = codeAction.isLeft() ? codeAction.getLeft() : codeAction.getRight().getCommand();
Expand Down
99 changes: 69 additions & 30 deletions ...lipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/correction/ReorgQuickFixTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,29 @@
*******************************************************************************/
package org.eclipse.jdt.ls.core.internal.correction;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.when;

import java.util.List;
import java.util.Objects;
import java.util.Optional;

import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.IPackageFragment;
import org.eclipse.jdt.core.IPackageFragmentRoot;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
import org.eclipse.jdt.ls.core.internal.ResourceUtils;
import org.eclipse.jdt.ls.core.internal.handlers.CodeActionHandler;
import org.eclipse.lsp4j.CodeAction;
import org.eclipse.lsp4j.Command;
import org.eclipse.lsp4j.RenameFile;
import org.eclipse.lsp4j.ResourceOperation;
import org.eclipse.lsp4j.TextDocumentEdit;
import org.eclipse.lsp4j.WorkspaceEdit;
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -37,7 +53,7 @@ public void setup() throws Exception {
fJProject1.setOptions(TestOptions.getDefaultOptions());

JavaLanguageServerPlugin.setPreferencesManager(preferenceManager);
when(preferenceManager.getClientPreferences().isResourceOperationSupported()).thenReturn(false);
when(preferenceManager.getClientPreferences().isResourceOperationSupported()).thenReturn(true);

fSourceFolder = fJProject1.getPackageFragmentRoot(fJProject1.getProject().getFolder("src"));
}
Expand Down Expand Up @@ -158,15 +174,44 @@ public void testWrongPackageStatement() throws Exception {
buf.append("public class E {\n");
buf.append("}\n");
ICompilationUnit cu = pack1.createCompilationUnit("E.java", buf.toString(), false, null);
List<Either<Command, CodeAction>> codeActions = evaluateCodeActions(cu);

Either<Command, CodeAction> codeAction = findAction(codeActions, "Change package declaration to 'test1'");
assertNotNull(codeAction);
buf = new StringBuilder();
buf.append("package test1;\n");
buf.append("\n");
buf.append("public class E {\n");
buf.append("}\n");
assertEquals(buf.toString(), evaluateCodeActionCommand(codeAction));

Expected e1 = new Expected("Change package declaration to 'test1'", buf.toString());
assertCodeActions(cu, e1);
codeAction = findAction(codeActions, "Move 'E.java' to package 'test2'");
assertNotNull(codeAction);
assertRenameFileOperation(codeAction, ResourceUtils.fixURI(cu.getResource().getRawLocationURI()).replace("test1", "test2"));
}

private Either<Command, CodeAction> findAction(List<Either<Command, CodeAction>> codeActions, String title) {
Optional<Either<Command, CodeAction>> any = codeActions.stream().filter((action) -> Objects.equals(title, action.getLeft() == null ? action.getRight().getTitle() : action.getLeft().getTitle())).findFirst();
return any.isPresent() ? any.get() : null;
}

private WorkspaceEdit getWorkspaceEdit(Either<Command, CodeAction> codeAction) {
Command c = codeAction.isLeft() ? codeAction.getLeft() : codeAction.getRight().getCommand();
assertEquals(CodeActionHandler.COMMAND_ID_APPLY_EDIT, c.getCommand());
assertNotNull(c.getArguments());
assertTrue(c.getArguments().get(0) instanceof WorkspaceEdit);
return (WorkspaceEdit) c.getArguments().get(0);
}

private void assertRenameFileOperation(Either<Command, CodeAction> codeAction, String newUri) {
WorkspaceEdit edit = getWorkspaceEdit(codeAction);
List<Either<TextDocumentEdit, ResourceOperation>> documentChanges = edit.getDocumentChanges();
assertNotNull(documentChanges);
assertEquals(1, documentChanges.size());
ResourceOperation resourceOperation = documentChanges.get(0).getRight();
assertNotNull(resourceOperation);
assertTrue(resourceOperation instanceof RenameFile);
assertEquals(newUri, ((RenameFile) resourceOperation).getNewUri());
}

@Test
Expand All @@ -178,15 +223,20 @@ public void testWrongPackageStatementInEnum() throws Exception {
buf.append("public enum E {\n");
buf.append("}\n");
ICompilationUnit cu = pack1.createCompilationUnit("E.java", buf.toString(), false, null);
List<Either<Command, CodeAction>> codeActions = evaluateCodeActions(cu);

Either<Command, CodeAction> codeAction = findAction(codeActions, "Change package declaration to 'test1'");
assertNotNull(codeAction);
buf = new StringBuilder();
buf.append("package test1;\n");
buf.append("\n");
buf.append("public enum E {\n");
buf.append("}\n");
assertEquals(buf.toString(), evaluateCodeActionCommand(codeAction));

Expected e1 = new Expected("Change package declaration to 'test1'", buf.toString());
assertCodeActions(cu, e1);
codeAction = findAction(codeActions, "Move 'E.java' to package 'test2'");
assertNotNull(codeAction);
assertRenameFileOperation(codeAction, ResourceUtils.fixURI(cu.getResource().getRawLocationURI()).replace("test1", "test2"));
}

@Test
Expand All @@ -198,15 +248,20 @@ public void testWrongPackageStatementFromDefault() throws Exception {
buf.append("public class E {\n");
buf.append("}\n");
ICompilationUnit cu = pack1.createCompilationUnit("E.java", buf.toString(), false, null);
List<Either<Command, CodeAction>> codeActions = evaluateCodeActions(cu);

Either<Command, CodeAction> codeAction = findAction(codeActions, "Remove package declaration 'package test2'");
assertNotNull(codeAction);
buf = new StringBuilder();
buf.append("\n");
buf.append("\n");
buf.append("public class E {\n");
buf.append("}\n");
assertEquals(buf.toString(), evaluateCodeActionCommand(codeAction));

Expected e1 = new Expected("Remove package declaration 'package test2'", buf.toString());
assertCodeActions(cu, e1);
codeAction = findAction(codeActions, "Move 'E.java' to package 'test2'");
assertNotNull(codeAction);
assertRenameFileOperation(codeAction, ResourceUtils.fixURI(pack1.getResource().getRawLocation().append("test2/E.java").toFile().toURI()));
}

@Test
Expand All @@ -216,35 +271,20 @@ public void testWrongDefaultPackageStatement() throws Exception {
buf.append("public class E {\n");
buf.append("}\n");
ICompilationUnit cu = pack1.createCompilationUnit("E.java", buf.toString(), false, null);
List<Either<Command, CodeAction>> codeActions = evaluateCodeActions(cu);

Either<Command, CodeAction> codeAction = findAction(codeActions, "Add package declaration 'test2;'");
assertNotNull(codeAction);
buf = new StringBuilder();
buf.append("package test2;\n");
buf.append("\n");
buf.append("public class E {\n");
buf.append("}\n");
assertEquals(buf.toString(), evaluateCodeActionCommand(codeAction));

Expected e1 = new Expected("Add package declaration 'test2;'", buf.toString());
assertCodeActions(cu, e1);
}

@Test
public void testWrongPackageStatementButColliding() throws Exception {
IPackageFragment pack1 = fSourceFolder.createPackageFragment("test1", false, null);
StringBuilder buf = new StringBuilder();
buf.append("package test2;\n");
buf.append("\n");
buf.append("public class E {\n");
buf.append("}\n");
ICompilationUnit cu = pack1.createCompilationUnit("E.java", buf.toString(), false, null);

buf = new StringBuilder();
buf.append("package test1;\n");
buf.append("\n");
buf.append("public class E {\n");
buf.append("}\n");

Expected e1 = new Expected("Change package declaration to 'test1'", buf.toString());
assertCodeActions(cu, e1);
codeAction = findAction(codeActions, "Move 'E.java' to the default package");
assertNotNull(codeAction);
assertRenameFileOperation(codeAction, ResourceUtils.fixURI(pack1.getResource().getRawLocation().append("../E.java").toFile().toURI()));
}

@Test
Expand Down Expand Up @@ -397,5 +437,4 @@ public void testWrongTypeNameInAnnot() throws Exception {
Expected e1 = new Expected("Rename type to 'E'", buf.toString());
assertCodeActions(cu, e1);
}

}

0 comments on commit 258729f

Please sign in to comment.