diff --git a/astra-core/src/main/java/org/alfasoftware/astra/core/refactoring/operations/methods/ChainedMethodInvocationRefactor.java b/astra-core/src/main/java/org/alfasoftware/astra/core/refactoring/operations/methods/ChainedMethodInvocationRefactor.java index 290e0b9..53d15a4 100644 --- a/astra-core/src/main/java/org/alfasoftware/astra/core/refactoring/operations/methods/ChainedMethodInvocationRefactor.java +++ b/astra-core/src/main/java/org/alfasoftware/astra/core/refactoring/operations/methods/ChainedMethodInvocationRefactor.java @@ -8,6 +8,7 @@ import org.alfasoftware.astra.core.utils.ASTOperation; import org.eclipse.jdt.core.dom.ASTNode; import org.eclipse.jdt.core.dom.CompilationUnit; +import org.eclipse.jdt.core.dom.Expression; import org.eclipse.jdt.core.dom.MethodInvocation; import org.eclipse.jdt.core.dom.rewrite.ASTRewrite; import org.eclipse.jface.text.BadLocationException; @@ -17,19 +18,25 @@ * Similar to a method invocation refactor, this swaps two chained method invocations to a different method. * * For example, + * *
  * # getCurrentFoo().doFooThing()
  * 
+ * * can be swapped to + * *
  * # doBarThing()
  * 
* * In that case, the chain to refactor is: + * *
  * ["org.example.ThingProvider getCurrentFoo", "org.example.Foo doFooThing"]
  * 
+ * * and the method name should be swapped to: + * *
  * ["doBarThing"]
  * 
@@ -39,11 +46,13 @@ public class ChainedMethodInvocationRefactor implements ASTOperation { private List before = new ArrayList<>(); private List after = new ArrayList<>(); + public ChainedMethodInvocationRefactor(List before, List after) { this.before = before; this.after = after; } + @Override public void run(CompilationUnit compilationUnit, ASTNode node, ASTRewrite rewriter) throws IOException, MalformedTreeException, BadLocationException { @@ -56,19 +65,50 @@ public void run(CompilationUnit compilationUnit, ASTNode node, ASTRewrite rewrit private void handleMethodInvocation(MethodInvocation node, ASTRewrite rewriter) { // second if (before.get(before.size() - 1).getMethodName().filter(name -> name.test(node.getName().toString())).isPresent() && - // wrappedA.get().first() - node.getExpression() != null && node.getExpression() instanceof MethodInvocation) { + // wrappedA.get().first() + node.getExpression() != null && node.getExpression() instanceof MethodInvocation) { // first - MethodInvocation nextMethodInvocation = (MethodInvocation) node.getExpression(); - if (before.get(before.size() - 2).getMethodName().filter(name -> name.test(nextMethodInvocation.getName().toString())).isPresent()) { - // TODO #36 make this looped, so we can handle chains of arbitrary length + MethodInvocation methodInvocation = (MethodInvocation) node.getExpression(); + int methodIterator = 2; + while (methodIterator <= before.size()) { + MethodInvocation nextMethodInvocation = methodInvocation; + if (before.get(before.size() - methodIterator).getMethodName() + .filter(name -> name.test(nextMethodInvocation.getName().toString())) + .isPresent()) { + methodIterator += 1; + if (before.size() == 2) { + rewriter.set(node, MethodInvocation.EXPRESSION_PROPERTY, methodInvocation.getExpression(), null); + rewriter.set(node, MethodInvocation.NAME_PROPERTY, node.getAST().newSimpleName(after.get(after.size() - 1)), null); + break; + } else if (methodIterator == before.size()) { + methodInvocation = (MethodInvocation) methodInvocation.getExpression(); + MethodInvocation newMethodInvocation = node.getAST().newMethodInvocation(); + + for (int i = 0; i < after.size(); i++) { + if (i == 0) { + Expression methodInvocationExpression = methodInvocation.getExpression(); + methodInvocation.setExpression(null); + newMethodInvocation.setName(newMethodInvocation.getAST().newSimpleName(after.get(i))); + newMethodInvocation.setExpression(methodInvocationExpression); + } else { + MethodInvocation expression = node.getAST().newMethodInvocation(); + expression.setName(expression.getAST().newSimpleName(after.get(i))); + expression.setExpression(newMethodInvocation); + newMethodInvocation = expression; + } + } - // change the chain to the "after" - // TODO #36 handle arbitrary "after" lengths - rewriter.set(node, MethodInvocation.EXPRESSION_PROPERTY, nextMethodInvocation.getExpression(), null); - rewriter.set(node, MethodInvocation.NAME_PROPERTY, node.getAST().newSimpleName(after.get(0)), null); + rewriter.set(node, MethodInvocation.EXPRESSION_PROPERTY, newMethodInvocation.getExpression(), null); + rewriter.set(node, MethodInvocation.NAME_PROPERTY, node.getAST().newSimpleName(after.get(after.size() - 1)), null); + + break; + } + } else { + break; + } + + methodInvocation = (MethodInvocation) methodInvocation.getExpression(); } } } } - diff --git a/astra-core/src/test/java/org/alfasoftware/astra/core/refactoring/methods/methodInvocation/TestMethodInvocationRefactor.java b/astra-core/src/test/java/org/alfasoftware/astra/core/refactoring/methods/methodInvocation/TestMethodInvocationRefactor.java index 7a2ceed..8c861f3 100644 --- a/astra-core/src/test/java/org/alfasoftware/astra/core/refactoring/methods/methodInvocation/TestMethodInvocationRefactor.java +++ b/astra-core/src/test/java/org/alfasoftware/astra/core/refactoring/methods/methodInvocation/TestMethodInvocationRefactor.java @@ -68,8 +68,8 @@ public void testInvocationChangeOutsideClassFullyQualifiedUsingMatcher() throws ) ); } - - + + /** * Changes an invocation of a method inherited from a class with a specific name, to another method. * Method invocations with the same name, but declared on a different class to the one specified, @@ -88,6 +88,7 @@ public void testInvocationChangeInheritedFromSuperclass() { .toNewMethodName("getClass"))))); } + /* * Uses the JavaPattern framework to change an invocation of a method inherited from a class with a specific name, to another method. */ @@ -103,7 +104,7 @@ public void testInvocationChangeInheritedFromSuperclassMatcher() throws IOExcept ); } - + /** * Changes an invocation of a method inherited from an interface with a specific name, to another method. * Method invocations with the same name, but declared on a different class to the one specified, @@ -121,8 +122,8 @@ public void testInvocationChangeInheritedFromInterface() { .to(new Changes() .toNewMethodName("getClass"))))); } - - + + /** * Changes an invocation of a method inherited from an interface which extends another interface, to another method. * Method invocations with the same name, but declared on a different class to the one specified, @@ -140,7 +141,7 @@ public void testInvocationChangeInheritedFromExtensionOfInterface() { .to(new Changes() .toNewMethodName("getClass"))))); } - + /** * Changes a static method invocation target. @@ -268,16 +269,13 @@ public void testInvocationChained() { )) ); } - - + + /** * ChainedMethodInvocationRefactor should be able to handle chains of arbitrary length. * This means we should be able to match on arbitrarily large chains of method calls, * and replace them with chains of method calls. - * - * Both the matching and replacement of method calls currently stops after two. */ - @Ignore("Illustrates issue https://github.com/alfasoftware/astra/issues/36") @Test public void testInvocationChainedWithLargeMatchingAndReplacementChains() { assertRefactor(InvocationChainedLargeChainsExample.class, @@ -319,7 +317,7 @@ public void testInvocationChainedWithLargeMatchingAndReplacementChains() { )) ); } - + @Test public void testInvocationChainedWrapped() { @@ -388,19 +386,19 @@ public void testInvocationChangeWithTransformMatcher() throws IOException { @Test - @Ignore("In this case, we find that we can't resolve bindings inside a lambda, where the lambda has any prior unknown types." + - " Here, that means that even though we have the class files for a method invocation, because we're missing the class" + - " files for a type referenced in the lambda, we cannot resolve the method invocation." + - " We need this information for type inference - https://docs.oracle.com/javase/specs/jls/se8/html/jls-18.html" + - " Because we don't have that type information, we don't know the return type, which means we are also missing" + - " the type which could be used as a type parameter, or an argument to a later function - so we can't determine which" + - " of a number of overloaded methods are being called, or if we have met the specific type criteria for a method." + - " The best we could do is match a method which uses 'Object'. In a later release, it's possible that Eclipse JDT Core" + - " may attempt a 'best guess' - but this sounds like it's not planned, and is considered a difficult problem to solve." + - " In this test, the unknown type is 'ASTNode'. If we provided the classpath for org.eclipse.jdt.core, the ASTNode type" + - " would be resolved, the rest of the lambda would be evaluated, the method identified and refactored, and the test would pass." + - " The 'proper' solution for this tool is to run with all the class files provided to the AST - but this makes the tool" + - " take significantly longer to run, due to the extra time taken to generate each file's AST." + + @Ignore("In this case, we find that we can't resolve bindings inside a lambda, where the lambda has any prior unknown types." + + " Here, that means that even though we have the class files for a method invocation, because we're missing the class" + + " files for a type referenced in the lambda, we cannot resolve the method invocation." + + " We need this information for type inference - https://docs.oracle.com/javase/specs/jls/se8/html/jls-18.html" + + " Because we don't have that type information, we don't know the return type, which means we are also missing" + + " the type which could be used as a type parameter, or an argument to a later function - so we can't determine which" + + " of a number of overloaded methods are being called, or if we have met the specific type criteria for a method." + + " The best we could do is match a method which uses 'Object'. In a later release, it's possible that Eclipse JDT Core" + + " may attempt a 'best guess' - but this sounds like it's not planned, and is considered a difficult problem to solve." + + " In this test, the unknown type is 'ASTNode'. If we provided the classpath for org.eclipse.jdt.core, the ASTNode type" + + " would be resolved, the rest of the lambda would be evaluated, the method identified and refactored, and the test would pass." + + " The 'proper' solution for this tool is to run with all the class files provided to the AST - but this makes the tool" + + " take significantly longer to run, due to the extra time taken to generate each file's AST." + " If 100% accuracy is required, provide all the class files and accept the slow run.") public void testInvocationInLambdaWithUnknownType() { assertRefactor(