Bug 404477: [move method] Wrong detection of duplicate methods (can result in compile errors)
cleaned up createArgumentList(..) and dependencies
diff --git a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/MoveInstanceMethodProcessor.java b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/MoveInstanceMethodProcessor.java
index b97559e..2cf23c2 100644
--- a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/MoveInstanceMethodProcessor.java
+++ b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/MoveInstanceMethodProcessor.java
@@ -268,8 +268,6 @@
private Map<IMember, IncomingMemberVisibilityAdjustment> fAdjustments;
- private boolean fNeededInsertion;
-
private Map<ICompilationUnit, CompilationUnitRewrite> fRewrites;
public DelegateInstanceMethodCreator(Map<IMember, IncomingMemberVisibilityAdjustment> adjustments, Map<ICompilationUnit, CompilationUnitRewrite> rewrites) {
@@ -284,7 +282,7 @@
final MethodInvocation invocation= getAst().newMethodInvocation();
invocation.setName(getAst().newSimpleName(getNewElementName()));
invocation.setExpression(createSimpleTargetAccessExpression(methodDeclaration));
- fNeededInsertion= createArgumentList(methodDeclaration, invocation.arguments(), new VisibilityAdjustingArgumentFactory(getAst(), fRewrites, fAdjustments));
+ createArgumentList(methodDeclaration, invocation.arguments(), new VisibilityAdjustingArgumentFactory(getAst(), fRewrites, fAdjustments));
final Block block= getAst().newBlock();
block.statements().add(createMethodInvocation(methodDeclaration, invocation));
if (!fSourceRewrite.getCu().equals(fTargetType.getCompilationUnit()))
@@ -296,10 +294,6 @@
protected ASTNode createDocReference(final BodyDeclaration declaration) throws JavaModelException {
return MoveInstanceMethodProcessor.this.createMethodReference((MethodDeclaration) declaration, getAst());
}
-
- protected boolean getNeededInsertion() {
- return fNeededInsertion;
- }
}
/**
@@ -1699,37 +1693,32 @@
* the argument list to create
* @param factory
* the argument factory to use
- * @return <code>true</code> if a target node had to be inserted as first
- * argument, <code>false</code> otherwise
* @throws JavaModelException
* if an error occurs
*/
- protected boolean createArgumentList(final MethodDeclaration declaration, final List<ASTNode> arguments, final IArgumentFactory factory) throws JavaModelException {
+ protected void createArgumentList(MethodDeclaration declaration, List<ASTNode> arguments, IArgumentFactory factory) throws JavaModelException {
Assert.isNotNull(declaration);
Assert.isNotNull(arguments);
Assert.isNotNull(factory);
- IVariableBinding binding= null;
- VariableDeclaration variable= null;
- boolean added= false;
- final int size= declaration.parameters().size();
- for (int index= 0; index < size; index++) {
- variable= (VariableDeclaration) declaration.parameters().get(index);
- binding= variable.resolveBinding();
- if (binding != null) {
- if (!Bindings.equals(binding, fTarget))
- arguments.add(factory.getArgumentNode(binding, index == size - 1));
- else if (needsTargetNode()) {
+ List<SingleVariableDeclaration> parameters= declaration.parameters();
+ int size= parameters.size();
+ for (int i= 0; i < size; i++) {
+ IVariableBinding binding= parameters.get(i).resolveBinding();
+ if (binding != null && Bindings.equals(binding, fTarget)) {
+ if (needsTargetNode()) {
+ // replace move target parameter with new target
arguments.add(factory.getTargetNode());
- added= true;
+ } else {
+ // drop unused move target parameter
}
- } else
- arguments.add(factory.getArgumentNode(binding, index == size - 1));
+ } else {
+ arguments.add(factory.getArgumentNode(binding, i == size - 1));
+ }
}
- if (needsTargetNode() && !added) {
+ if (needsTargetNode() && fTarget.isField()) {
+ // prepend new target when moving to a field
arguments.add(0, factory.getTargetNode());
- added= true;
}
- return added;
}
/*
@@ -1820,15 +1809,15 @@
adjustor.setRewrite(sourceRewrite, fSourceRewrite.getRoot());
adjustor.adjustVisibility(new SubProgressMonitor(monitor, 1));
final IDocument document= new Document(fMethod.getCompilationUnit().getBuffer().getContents());
- final boolean target= createMethodCopy(document, declaration, sourceRewrite, rewrites, adjustor.getAdjustments(), status, new SubProgressMonitor(monitor, 1));
- createMethodJavadocReferences(rewrites, declaration, references, target, status, new SubProgressMonitor(monitor, 1));
+ createMethodCopy(document, declaration, sourceRewrite, rewrites, adjustor.getAdjustments(), status, new SubProgressMonitor(monitor, 1));
+ createMethodJavadocReferences(rewrites, declaration, references, status, new SubProgressMonitor(monitor, 1));
if (!fSourceRewrite.getCu().equals(targetRewrite.getCu()))
createMethodImports(targetRewrite, declaration, new SubProgressMonitor(monitor, 1), status);
boolean removable= false;
if (fInline) {
String binaryRefsDescription= Messages.format(RefactoringCoreMessages.ReferencesInBinaryContext_ref_in_binaries_description , BasicElementLabels.getJavaElementName(getMethod().getElementName()));
ReferencesInBinaryContext binaryRefs= new ReferencesInBinaryContext(binaryRefsDescription);
- removable= createMethodDelegator(rewrites, declaration, references, adjustor.getAdjustments(), target, binaryRefs, status, new SubProgressMonitor(monitor, 1));
+ removable= createMethodDelegator(rewrites, declaration, references, adjustor.getAdjustments(), binaryRefs, status, new SubProgressMonitor(monitor, 1));
binaryRefs.addErrorIfNecessary(status);
if (fRemove && removable) {
fSourceRewrite.getASTRewrite().remove(declaration, fSourceRewrite.createGroupDescription(RefactoringCoreMessages.MoveInstanceMethodProcessor_remove_original_method));
@@ -1876,9 +1865,6 @@
* the search match representing the method invocation
* @param adjustments
* the map of elements to visibility adjustments
- * @param target
- * <code>true</code> if a target node had to be inserted as
- * first argument, <code>false</code> otherwise
* @param status
* the refactoring status
* @return <code>true</code> if the inline change could be performed,
@@ -1887,8 +1873,8 @@
* if a problem occurred while creating the inlined target
* expression for field targets
*/
- protected boolean createInlinedMethodInvocation(final CompilationUnitRewrite rewriter, final MethodDeclaration declaration, final SearchMatch match,
- final Map<IMember, IncomingMemberVisibilityAdjustment> adjustments, final boolean target, final RefactoringStatus status) throws JavaModelException {
+ protected boolean createInlinedMethodInvocation(CompilationUnitRewrite rewriter, MethodDeclaration declaration, SearchMatch match,
+ Map<IMember, IncomingMemberVisibilityAdjustment> adjustments, RefactoringStatus status) throws JavaModelException {
Assert.isNotNull(rewriter);
Assert.isNotNull(declaration);
Assert.isNotNull(match);
@@ -1904,7 +1890,7 @@
newMethodInvocation.setName(rewrite.getAST().newSimpleName(fMethodName));
if (fTarget.isField()) {
newMethodInvocation.setStructuralProperty(MethodInvocation.EXPRESSION_PROPERTY, rewrite.getAST().newSimpleName(fTarget.getName()));
- if (target) {
+ if (needsTargetNode()) {
newMethodInvocation.arguments().add(rewrite.getAST().newThisExpression());
}
for (ASTNode astNode : (List<ASTNode>) invocation.arguments()) {
@@ -1926,7 +1912,7 @@
if (arg.getNodeType() != ASTNode.THIS_EXPRESSION) {
newMethodInvocation.setStructuralProperty(MethodInvocation.EXPRESSION_PROPERTY, rewrite.createCopyTarget(arg));
}
- if (target) {
+ if (needsTargetNode()) {
newMethodInvocation.arguments().add(rewrite.getAST().newThisExpression());
}
}
@@ -1948,7 +1934,7 @@
rewrite.set(invocation, MethodInvocation.EXPRESSION_PROPERTY, access, group);
} else
rewrite.set(invocation, MethodInvocation.EXPRESSION_PROPERTY, rewrite.getAST().newSimpleName(fTarget.getName()), group);
- if (target) {
+ if (needsTargetNode()) {
if (access == null || !(access instanceof FieldAccess))
list.insertFirst(rewrite.getAST().newThisExpression(), null);
else
@@ -1971,7 +1957,7 @@
rewrite.remove(invocation.getExpression(), null);
else
rewrite.set(invocation, MethodInvocation.EXPRESSION_PROPERTY, rewrite.createCopyTarget(argument), group);
- if (target) {
+ if (needsTargetNode()) {
if (invocation.getExpression() != null)
list.replace(argument, rewrite.createCopyTarget(invocation.getExpression()), group);
else {
@@ -2052,12 +2038,10 @@
* the map of elements to visibility adjustments
* @param status
* the refactoring status
- * @return <code>true</code> if a target node had to be inserted as method
- * argument, <code>false</code> otherwise
* @throws JavaModelException
* if an error occurs while accessing the types of the arguments
*/
- protected boolean createMethodArguments(final Map<ICompilationUnit, CompilationUnitRewrite> rewrites, final ASTRewrite rewrite, final MethodDeclaration declaration, final Map<IMember, IncomingMemberVisibilityAdjustment> adjustments, final RefactoringStatus status) throws JavaModelException {
+ protected void createMethodArguments(Map<ICompilationUnit, CompilationUnitRewrite> rewrites, ASTRewrite rewrite, final MethodDeclaration declaration, Map<IMember, IncomingMemberVisibilityAdjustment> adjustments, RefactoringStatus status) throws JavaModelException {
Assert.isNotNull(rewrites);
Assert.isNotNull(declaration);
Assert.isNotNull(rewrite);
@@ -2068,7 +2052,7 @@
final AstNodeFinder finder= new AnonymousClassReferenceFinder(declaration);
declaration.accept(finder);
final List<ASTNode> arguments= new ArrayList<ASTNode>(declaration.parameters().size() + 1);
- final boolean result= createArgumentList(declaration, arguments, new VisibilityAdjustingArgumentFactory(ast, rewrites, adjustments) {
+ createArgumentList(declaration, arguments, new VisibilityAdjustingArgumentFactory(ast, rewrites, adjustments) {
@Override
public final ASTNode getArgumentNode(final IVariableBinding binding, final boolean last) throws JavaModelException {
@@ -2127,7 +2111,6 @@
node= iterator.next();
list.insertLast(node, null);
}
- return result;
}
/**
@@ -2279,10 +2262,8 @@
* the progress monitor to display progress
* @throws CoreException
* if an error occurs
- * @return <code>true</code> if a target node had to be inserted as first
- * argument, <code>false</code> otherwise
*/
- protected boolean createMethodCopy(final IDocument document, final MethodDeclaration declaration, final ASTRewrite rewrite, final Map<ICompilationUnit, CompilationUnitRewrite> rewrites, final Map<IMember, IncomingMemberVisibilityAdjustment> adjustments, final RefactoringStatus status, final IProgressMonitor monitor) throws CoreException {
+ protected void createMethodCopy(IDocument document, MethodDeclaration declaration, ASTRewrite rewrite, Map<ICompilationUnit, CompilationUnitRewrite> rewrites, Map<IMember, IncomingMemberVisibilityAdjustment> adjustments, RefactoringStatus status, IProgressMonitor monitor) throws CoreException {
Assert.isNotNull(document);
Assert.isNotNull(declaration);
Assert.isNotNull(rewrite);
@@ -2290,7 +2271,6 @@
Assert.isNotNull(adjustments);
Assert.isNotNull(status);
Assert.isNotNull(monitor);
- boolean target= false;
final CompilationUnitRewrite rewriter= getCompilationUnitRewrite(rewrites, getTargetType().getCompilationUnit());
try {
rewrite.set(declaration, MethodDeclaration.NAME_PROPERTY, rewrite.getAST().newSimpleName(fMethodName), null);
@@ -2317,7 +2297,7 @@
}
}
}
- target= createMethodArguments(rewrites, rewrite, declaration, adjustments, status);
+ createMethodArguments(rewrites, rewrite, declaration, adjustments, status);
createMethodTypeParameters(rewrite, declaration, status);
createMethodComment(rewrite, declaration);
createMethodBody(rewriter, rewrite, declaration);
@@ -2325,7 +2305,6 @@
if (fMethod.getCompilationUnit().equals(getTargetType().getCompilationUnit()))
rewriter.clearImportRewrites();
}
- return target;
}
/**
@@ -2344,10 +2323,8 @@
* the progress monitor to display progress
* @throws CoreException
* if the change could not be generated
- * @return <code>true</code> if a target node had to be inserted as first
- * argument, <code>false</code> otherwise
*/
- protected boolean createMethodDelegation(final MethodDeclaration declaration, final Map<ICompilationUnit, CompilationUnitRewrite> rewrites, final Map<IMember, IncomingMemberVisibilityAdjustment> adjustments, final RefactoringStatus status, final IProgressMonitor monitor) throws CoreException {
+ protected void createMethodDelegation(final MethodDeclaration declaration, final Map<ICompilationUnit, CompilationUnitRewrite> rewrites, final Map<IMember, IncomingMemberVisibilityAdjustment> adjustments, final RefactoringStatus status, final IProgressMonitor monitor) throws CoreException {
Assert.isNotNull(declaration);
Assert.isNotNull(monitor);
@@ -2359,8 +2336,6 @@
creator.setNewElementName(fMethodName);
creator.prepareDelegate();
creator.createEdit();
-
- return creator.getNeededInsertion();
}
/**
@@ -2376,9 +2351,6 @@
* moved method, including references in comments
* @param adjustments
* the map of elements to visibility adjustments
- * @param target
- * <code>true</code> if a target node must be inserted as first
- * argument, <code>false</code> otherwise
* @param binaryRefs
* the binary references context
* @param status
@@ -2389,7 +2361,7 @@
* method declaration could be inlined, <code>false</code>
* otherwise
*/
- protected boolean createMethodDelegator(final Map<ICompilationUnit, CompilationUnitRewrite> rewrites, final MethodDeclaration declaration, final SearchResultGroup[] groups, final Map<IMember, IncomingMemberVisibilityAdjustment> adjustments, final boolean target, ReferencesInBinaryContext binaryRefs, final RefactoringStatus status, final IProgressMonitor monitor) {
+ protected boolean createMethodDelegator(Map<ICompilationUnit, CompilationUnitRewrite> rewrites, MethodDeclaration declaration, SearchResultGroup[] groups, Map<IMember, IncomingMemberVisibilityAdjustment> adjustments, ReferencesInBinaryContext binaryRefs, RefactoringStatus status, IProgressMonitor monitor) {
Assert.isNotNull(rewrites);
Assert.isNotNull(declaration);
Assert.isNotNull(groups);
@@ -2442,7 +2414,7 @@
if (match.getAccuracy() == SearchMatch.A_INACCURATE) {
status.merge(RefactoringStatus.createWarningStatus(Messages.format(RefactoringCoreMessages.MoveInstanceMethodProcessor_inline_inaccurate, BasicElementLabels.getFileName(unit)), JavaStatusContext.create(unit, new SourceRange(match.getOffset(), match.getLength()))));
result= false;
- } else if (!createInlinedMethodInvocation(rewrite, declaration, match, adjustments, target, status))
+ } else if (!createInlinedMethodInvocation(rewrite, declaration, match, adjustments, status))
result= false;
}
} else {
@@ -2500,13 +2472,10 @@
* the source method declaration
* @param match
* the search match representing the method reference
- * @param targetNode
- * <code>true</code> if a target node had to be inserted as
- * first argument, <code>false</code> otherwise
* @param status
* the refactoring status
*/
- protected void createMethodJavadocReference(final CompilationUnitRewrite rewrite, final MethodDeclaration declaration, final SearchMatch match, final boolean targetNode, final RefactoringStatus status) {
+ protected void createMethodJavadocReference(CompilationUnitRewrite rewrite, MethodDeclaration declaration, SearchMatch match, RefactoringStatus status) {
Assert.isNotNull(rewrite);
Assert.isNotNull(declaration);
Assert.isNotNull(match);
@@ -2531,15 +2500,12 @@
* @param groups
* the search result groups representing all references to the
* moved method, including references in comments
- * @param target
- * <code>true</code> if a target node must be inserted as first
- * argument, <code>false</code> otherwise
* @param status
* the refactoring status
* @param monitor
* the progress monitor to use
*/
- protected void createMethodJavadocReferences(final Map<ICompilationUnit, CompilationUnitRewrite> rewrites, final MethodDeclaration declaration, final SearchResultGroup[] groups, final boolean target, final RefactoringStatus status, final IProgressMonitor monitor) {
+ protected void createMethodJavadocReferences(Map<ICompilationUnit, CompilationUnitRewrite> rewrites, MethodDeclaration declaration, SearchResultGroup[] groups, RefactoringStatus status, IProgressMonitor monitor) {
Assert.isNotNull(rewrites);
Assert.isNotNull(declaration);
Assert.isNotNull(status);
@@ -2566,7 +2532,7 @@
if (match.getAccuracy() == SearchMatch.A_INACCURATE) {
status.merge(RefactoringStatus.createWarningStatus(Messages.format(RefactoringCoreMessages.MoveInstanceMethodProcessor_inline_inaccurate, BasicElementLabels.getFileName(unit)), JavaStatusContext.create(unit, new SourceRange(match.getOffset(), match.getLength()))));
} else
- createMethodJavadocReference(rewrite, declaration, match, target, status);
+ createMethodJavadocReference(rewrite, declaration, match, status);
}
}
monitor.worked(1);
