diff --git a/src/main/java/spoon/refactoring/CtParameterRemoveRefactoring.java b/src/main/java/spoon/refactoring/CtParameterRemoveRefactoring.java new file mode 100644 index 00000000000..72d807915b9 --- /dev/null +++ b/src/main/java/spoon/refactoring/CtParameterRemoveRefactoring.java @@ -0,0 +1,328 @@ +/** + * Copyright (C) 2006-2017 INRIA and contributors + * Spoon - http://spoon.gforge.inria.fr/ + * + * This software is governed by the CeCILL-C License under French law and + * abiding by the rules of distribution of free software. You can use, modify + * and/or redistribute the software under the terms of the CeCILL-C license as + * circulated by CEA, CNRS and INRIA at http://www.cecill.info. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. + * + * The fact that you are presently reading this means that you have had + * knowledge of the CeCILL-C license and that you accept its terms. + */ +package spoon.refactoring; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import spoon.SpoonException; +import spoon.reflect.code.CtAnnotationFieldAccess; +import spoon.reflect.code.CtArrayRead; +import spoon.reflect.code.CtExpression; +import spoon.reflect.code.CtFieldRead; +import spoon.reflect.code.CtInvocation; +import spoon.reflect.code.CtLiteral; +import spoon.reflect.code.CtNewArray; +import spoon.reflect.code.CtThisAccess; +import spoon.reflect.code.CtVariableRead; +import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtExecutable; +import spoon.reflect.declaration.CtParameter; +import spoon.reflect.reference.CtExecutableReference; +import spoon.reflect.reference.CtParameterReference; +import spoon.reflect.visitor.CtAbstractVisitor; +import spoon.reflect.visitor.chain.CtConsumer; +import spoon.reflect.visitor.filter.AllMethodsSameSignatureFunction; +import spoon.reflect.visitor.filter.ExecutableReferenceFilter; +import spoon.reflect.visitor.filter.ParameterReferenceFunction; + +/** + * Removes target {@link CtParameter} from the parent target {@link CtExecutable} + * and from all overriding/overridden methods of related type hierarchies + * and from all lambda expressions (if any) implementing the modified interface. + * It removes arguments from all invocations of refactored executables too.
+ * + * Before the refactoring is started it checks that: + * + * If one of the validation constraints fails, then {@link RefactoringException} is thrown and nothing is changed. + * You can override `#create*Issue(...)` methods to handle such exceptions individually. + *
+ */ +public class CtParameterRemoveRefactoring implements CtRefactoring { + + private CtParameter target; + private int parameterIndex; + /** + * List of all {@link CtExecutable}s whose parameter has to be removed + */ + private List> targetExecutables; + /** + * List of all {@link CtInvocation}s whose argument has to be removed + */ + private List> targetInvocations; + + public CtParameterRemoveRefactoring() { + } + + /** + * @return the {@link CtParameter} which has to be removed by this refactoring function + */ + public CtParameter getTarget() { + return target; + } + + /** + * @param target the {@link CtParameter} which has to be removed by this refactoring function + * @return this to support fluent API + */ + public CtParameterRemoveRefactoring setTarget(CtParameter target) { + if (this.target == target) { + return this; + } + this.target = target; + this.parameterIndex = target.getParent().getParameters().indexOf(target); + targetExecutables = null; + targetInvocations = null; + return this; + } + + /** + * @return computes and returns all executables, which will be modified by this refactoring + */ + public List> getTargetExecutables() { + if (targetExecutables == null) { + computeAllExecutables(); + } + return targetExecutables; + } + + /** + * @return computes and returns all invocations, which will be modified by this refactoring + */ + public List> getTargetInvocations() { + if (targetInvocations == null) { + computeAllInvocations(); + } + return targetInvocations; + } + + @Override + public void refactor() { + if (getTarget() == null) { + throw new SpoonException("The target of refactoring is not defined"); + } + detectIssues(); + refactorNoCheck(); + } + + /** + * validates whether this refactoring can be done without changing behavior of the refactored code. + */ + protected void detectIssues() { + checkAllExecutables(); + checkAllInvocations(); + } + + /** + * search for all methods and lambdas which has to be refactored together with target method + */ + private void computeAllExecutables() { + if (getTarget() == null) { + throw new SpoonException("The target of refactoring is not defined"); + } + final List> executables = new ArrayList<>(); + CtExecutable targetExecutable = target.getParent(); + //all the executables, which belongs to same inheritance tree + executables.add(targetExecutable); + targetExecutable.map(new AllMethodsSameSignatureFunction()).forEach(new CtConsumer>() { + @Override + public void accept(CtExecutable executable) { + executables.add(executable); + } + }); + targetExecutables = Collections.unmodifiableList(executables); + } + + /** + * search for all methods and lambdas which has to be refactored together with target method + */ + private void computeAllInvocations() { + ExecutableReferenceFilter execRefFilter = new ExecutableReferenceFilter(); + for (CtExecutable exec : getTargetExecutables()) { + execRefFilter.addExecutable(exec); + } + //all the invocations, which belongs to same inheritance tree + final List> invocations = new ArrayList<>(); + target.getFactory().getModel().getRootPackage().filterChildren(execRefFilter).forEach(new CtConsumer>() { + @Override + public void accept(CtExecutableReference t) { + CtElement parent = t.getParent(); + if (parent instanceof CtInvocation) { + invocations.add((CtInvocation) parent); + } //else ignore other hits, which are not in context of invocation + } + }); + targetInvocations = Collections.unmodifiableList(invocations); + } + + private void checkAllExecutables() { + for (CtExecutable executable : getTargetExecutables()) { + checkExecutable(executable); + } + } + + private void checkExecutable(CtExecutable executable) { + final CtParameter toBeRemovedParam = executable.getParameters().get(this.parameterIndex); + toBeRemovedParam.map(new ParameterReferenceFunction()).forEach(new CtConsumer>() { + @Override + public void accept(CtParameterReference paramRef) { + //some parameter uses are acceptable + //e.g. parameter in invocation of super of method, which is going to be removed too. + if (isAllowedParameterUsage(paramRef)) { + return; + } + createParameterUsedIssue(toBeRemovedParam, paramRef); + } + }); + } + + private void checkAllInvocations() { + for (CtInvocation invocation : getTargetInvocations()) { + checkInvocation(invocation); + } + } + + private void checkInvocation(CtInvocation invocation) { + final CtExpression toBeRemovedExpression = invocation.getArguments().get(this.parameterIndex); + if (canRemoveExpression(toBeRemovedExpression) == false) { + createExpressionCannotBeRemovedIssue(invocation, toBeRemovedExpression); + } + } + + /** + * Detects whether found usage of removed parameter is acceptable + * @param paramRef the found reference to + * @return true if it is allowed parameter use + */ + protected boolean isAllowedParameterUsage(CtParameterReference paramRef) { + if (isRemovedParamOfRefactoredInvocation(paramRef)) { + return true; + } + return false; + } + + /** + * Detects whether `toBeRemovedExpression` can be safely removed during the refactoring + * + * @param toBeRemovedExpression the {@link CtExpression}, which will be removed by this refactoring + * @return true if the expression used to deliver argument of removed parameter can be removed + * false if cannot be removed and this refactoring has to be avoided. + */ + protected boolean canRemoveExpression(CtExpression toBeRemovedExpression) { + class Context { + boolean canBeRemoved = false; + } + final Context context = new Context(); + toBeRemovedExpression.accept(new CtAbstractVisitor() { + @Override + public void visitCtVariableRead(CtVariableRead variableRead) { + context.canBeRemoved = true; + } + @Override + public void visitCtArrayRead(CtArrayRead arrayRead) { + context.canBeRemoved = true; + } + @Override + public void visitCtFieldRead(CtFieldRead fieldRead) { + context.canBeRemoved = true; + } + @Override + public void visitCtParameterReference(CtParameterReference reference) { + context.canBeRemoved = true; + } + @Override + public void visitCtLiteral(CtLiteral literal) { + context.canBeRemoved = true; + } + @Override + public void visitCtNewArray(CtNewArray newArray) { + context.canBeRemoved = true; + } + @Override + public void visitCtAnnotationFieldAccess(CtAnnotationFieldAccess annotationFieldAccess) { + context.canBeRemoved = true; + } + @Override + public void visitCtThisAccess(CtThisAccess thisAccess) { + context.canBeRemoved = true; + } + //There are more expression which is save to remove. Including tree of unary/binary operators, conditional, etc. + //It would be good to have a Filter, which matches read only expressions + }); + return context.canBeRemoved; + } + + protected boolean isRemovedParamOfRefactoredInvocation(CtParameterReference paramRef) { + CtInvocation invocation = paramRef.getParent(CtInvocation.class); + if (invocation == null) { + return false; + } + return getTargetInvocations().contains(invocation); + } + + /** + * Override this method to get access to details about this refactoring issue + * @param usedParameter to be removed parameter, which is used by `parameterUsage` + * @param parameterUsage the usage of parameter, which avoids it's remove + */ + protected void createParameterUsedIssue(CtParameter usedParameter, CtParameterReference parameterUsage) { + throw new RefactoringException("The parameter " + usedParameter.getSimpleName() + + " of method: " + parameterUsage.getDeclaringExecutable() + " cannot be removed because it is used (" + parameterUsage.getPosition() + ")"); + } + + /** + * Override this method to get access to details about this refactoring issue. + * @param toBeRemovedExpression is the expression which delivers value for the argument of the removed parameter, + * where {@link #canRemoveExpression(CtExpression)} returned false. + */ + protected void createExpressionCannotBeRemovedIssue(CtInvocation invocation, CtExpression toBeRemovedExpression) { + throw new RefactoringException("The expression " + toBeRemovedExpression + + ", which creates argument of the to be removed parameter in invocation " + invocation + " cannot be removed." + + " Override method `canRemoveExpression` to customize this behavior."); + } + + protected void refactorNoCheck() { + removeInvocationArguments(); + removeMethodParameters(); + } + + protected void removeInvocationArguments() { + List> invocations = getTargetInvocations(); + for (CtInvocation invocation : invocations) { + removeInvocationArgument(invocation); + } + } + + protected void removeInvocationArgument(CtInvocation invocation) { + invocation.removeArgument(invocation.getArguments().get(this.parameterIndex)); + } + + protected void removeMethodParameters() { + List> executables = getTargetExecutables(); + for (CtExecutable executable : executables) { + removeParameter(executable); + } + } + + protected void removeParameter(CtExecutable executable) { + executable.removeParameter(executable.getParameters().get(this.parameterIndex)); + } +} diff --git a/src/test/java/spoon/test/refactoring/MethodsRefactoringTest.java b/src/test/java/spoon/test/refactoring/MethodsRefactoringTest.java index a8b8f227839..9e011efd45b 100644 --- a/src/test/java/spoon/test/refactoring/MethodsRefactoringTest.java +++ b/src/test/java/spoon/test/refactoring/MethodsRefactoringTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.*; import java.io.File; +import java.io.FileNotFoundException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -12,9 +13,14 @@ import org.junit.Test; +import spoon.Launcher; +import spoon.OutputType; +import spoon.SpoonModelBuilder; +import spoon.compiler.SpoonResourceHelper; +import spoon.refactoring.CtParameterRemoveRefactoring; +import spoon.refactoring.RefactoringException; import spoon.reflect.code.CtLambda; import spoon.reflect.code.CtStatement; -import spoon.reflect.declaration.CtClass; import spoon.reflect.declaration.CtConstructor; import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtExecutable; @@ -24,7 +30,6 @@ import spoon.reflect.reference.CtExecutableReference; import spoon.reflect.visitor.filter.AllMethodsSameSignatureFunction; import spoon.reflect.visitor.filter.ExecutableReferenceFilter; -import spoon.reflect.visitor.filter.NameFilter; import spoon.reflect.visitor.filter.SubInheritanceHierarchyFunction; import spoon.reflect.visitor.filter.TypeFilter; import spoon.test.refactoring.parameter.testclasses.IFaceB; @@ -34,9 +39,7 @@ import spoon.test.refactoring.parameter.testclasses.TypeA; import spoon.test.refactoring.parameter.testclasses.TypeB; import spoon.test.refactoring.parameter.testclasses.TypeC; -import spoon.test.refactoring.parameter.testclasses.TypeL; import spoon.test.refactoring.parameter.testclasses.TypeR; -import spoon.test.refactoring.parameter.testclasses.TypeS; import spoon.testing.utils.ModelUtils; public class MethodsRefactoringTest { @@ -64,6 +67,7 @@ public void testSubInheritanceHierarchyFunction() { "spoon.test.refactoring.parameter.testclasses.TypeB", "spoon.test.refactoring.parameter.testclasses.TypeB$1", "spoon.test.refactoring.parameter.testclasses.TypeB$1Local", + "spoon.test.refactoring.parameter.testclasses.TypeB$2", "spoon.test.refactoring.parameter.testclasses.TypeC", "spoon.test.refactoring.parameter.testclasses.IFaceL", "spoon.test.refactoring.parameter.testclasses.TypeL", @@ -300,4 +304,64 @@ private boolean removeSame(Collection list, Object item) { } return false; } -} \ No newline at end of file + + @Test + public void testCtParameterRemoveRefactoring() throws FileNotFoundException { + String testPackagePath = "spoon/test/refactoring/parameter/testclasses"; + final Launcher launcher = new Launcher(); + launcher.getEnvironment().setNoClasspath(true); + SpoonModelBuilder comp = launcher.createCompiler(); + comp.addInputSource(SpoonResourceHelper.createResource(new File("./src/test/java/"+testPackagePath))); + comp.build(); + Factory factory = comp.getFactory(); + + CtType typeA = factory.Class().get(TypeA.class); + + CtMethod methodTypeA_method1 = typeA.getMethodsByName("method1").get(0); + CtParameterRemoveRefactoring refactor = new CtParameterRemoveRefactoring(); + refactor.setTarget(methodTypeA_method1.getParameters().get(0)); + //check that expected methods are targets of refactoring + List> execs = refactor.getTargetExecutables(); + execs.forEach(exec->{ + //check that each to be modified method has one parameter + assertEquals(1, exec.getParameters().size()); + }); + refactor.refactor(); + execs.forEach(exec->{ + //check that each to be modified method has no parameter after refactoring + assertEquals(0, exec.getParameters().size()); + }); + launcher.setSourceOutputDirectory(new File("./target/spooned/")); + launcher.getModelBuilder().generateProcessedSourceFiles(OutputType.CLASSES); + ModelUtils.canBeBuilt("./target/spooned/"+testPackagePath, 8); + } + @Test + public void testCtParameterRemoveRefactoringValidationCheck() throws FileNotFoundException { + String testPackagePath = "spoon/test/refactoring/parameter/testclasses"; + final Launcher launcher = new Launcher(); + launcher.getEnvironment().setNoClasspath(true); + SpoonModelBuilder comp = launcher.createCompiler(); + comp.addInputSource(SpoonResourceHelper.createResource(new File("./src/test/java/"+testPackagePath))); + comp.build(); + Factory factory = comp.getFactory(); + + CtType typeR = factory.Class().get(TypeR.class); + + CtMethod methodTypeR_method1 = typeR.getMethodsByName("method1").get(0); + CtParameterRemoveRefactoring refactor = new CtParameterRemoveRefactoring().setTarget(methodTypeR_method1.getParameters().get(0)); + refactor.setTarget(methodTypeR_method1.getParameters().get(0)); + //check that each to be refactored method has one parameter + List> execs = refactor.getTargetExecutables(); + execs.forEach(exec->{ + //check that each to be modified method has one parameter + assertEquals(1, exec.getParameters().size()); + }); + //try refactor + try { + refactor.refactor(); + fail(); + } catch (RefactoringException e) { + this.getClass(); + } + } +} diff --git a/src/test/java/spoon/test/refactoring/parameter/testclasses/TypeB.java b/src/test/java/spoon/test/refactoring/parameter/testclasses/TypeB.java index c6650cc779a..9ce22799e09 100644 --- a/src/test/java/spoon/test/refactoring/parameter/testclasses/TypeB.java +++ b/src/test/java/spoon/test/refactoring/parameter/testclasses/TypeB.java @@ -39,4 +39,13 @@ public void method1(Double p1) { } } } + private void anMethodWithNullParameterValue() { + IFaceB ifaceB = new IFaceB() { + @Override + @TestHierarchy("A_method1") + public void method1(String p1) { + } + }; + ifaceB.method1(null); + } } diff --git a/src/test/java/spoon/test/refactoring/parameter/testclasses/TypeS.java b/src/test/java/spoon/test/refactoring/parameter/testclasses/TypeS.java index d0bd3321156..ab225c9c900 100644 --- a/src/test/java/spoon/test/refactoring/parameter/testclasses/TypeS.java +++ b/src/test/java/spoon/test/refactoring/parameter/testclasses/TypeS.java @@ -21,4 +21,16 @@ private void methodWithLambdaOf_R() { }; ifaceT.method1(1.0); } + + private void methodWithComplexExpression_R() { + IFaceT ifaceT = p->{ + @TestHierarchy("R_method1") + int x; + }; + /* + * the refactoring should by default report calling of method as problem, because it cannot know, + * whether that method call has side effects. Such method call remove might cause malfunction. + */ + ifaceT.method1(Math.abs(1.0)); + } }