From 69b5996f1c66ac5e647039098cf25f055c2f67e3 Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Tue, 21 Feb 2017 20:05:14 +0100 Subject: [PATCH 1/6] feature: change LocalVariable name --- .../refactoring/AbstractRenameRefactor.java | 111 ++++++++ .../refactoring/ChangeLocalVariableName.java | 238 +++++++++++++++++ src/main/java/spoon/refactoring/Issue.java | 21 ++ .../java/spoon/refactoring/IssueImpl.java | 36 +++ src/main/java/spoon/refactoring/Refactor.java | 27 ++ .../refactoring/ChangeVariableNameTest.java | 244 ++++++++++++++++++ .../refactoring/testclasses/TryRename.java | 16 ++ .../testclasses/VariableRename.java | 244 ++++++++++++++++++ 8 files changed, 937 insertions(+) create mode 100644 src/main/java/spoon/refactoring/AbstractRenameRefactor.java create mode 100644 src/main/java/spoon/refactoring/ChangeLocalVariableName.java create mode 100644 src/main/java/spoon/refactoring/Issue.java create mode 100644 src/main/java/spoon/refactoring/IssueImpl.java create mode 100644 src/main/java/spoon/refactoring/Refactor.java create mode 100644 src/test/java/spoon/test/refactoring/ChangeVariableNameTest.java create mode 100644 src/test/java/spoon/test/refactoring/testclasses/TryRename.java create mode 100644 src/test/java/spoon/test/refactoring/testclasses/VariableRename.java diff --git a/src/main/java/spoon/refactoring/AbstractRenameRefactor.java b/src/main/java/spoon/refactoring/AbstractRenameRefactor.java new file mode 100644 index 00000000000..a43f238297d --- /dev/null +++ b/src/main/java/spoon/refactoring/AbstractRenameRefactor.java @@ -0,0 +1,111 @@ +/** + * 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.List; +import java.util.regex.Pattern; + +import spoon.SpoonException; +import spoon.reflect.declaration.CtNamedElement; +import spoon.reflect.reference.CtReference; +import spoon.reflect.visitor.chain.CtConsumer; + +public abstract class AbstractRenameRefactor implements Refactor { + public static final Pattern javaIdentifierRE = Pattern.compile("\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*"); + + protected T target; + protected String newName; + protected Pattern newNameValidationRE; + + protected AbstractRenameRefactor(Pattern newNameValidationRE) { + this.newNameValidationRE = newNameValidationRE; + } + + @Override + public void refactor() { + if (getTarget() == null) { + throw new SpoonException("The target of refactoring is not defined"); + } + if (getNewName() == null) { + throw new SpoonException("The new name of refactoring is not defined"); + } + List issues = getIssues(); + if (issues.isEmpty() == false) { + throw new SpoonException("Refactoring cannot be processed. There are issues: " + issues.toString()); + } + refactorNoCheck(); + } + + protected void refactorNoCheck() { + forEachReference(new CtConsumer() { + @Override + public void accept(CtReference t) { + t.setSimpleName(AbstractRenameRefactor.this.newName); + } + }); + target.setSimpleName(newName); + } + + protected abstract void forEachReference(CtConsumer consumer); + + @Override + public List getIssues() { + List issues = new ArrayList<>(); + detectIssues(issues); + return issues; + } + + protected void detectIssues(List issues) { + checkNewNameIsValid(issues); + detectNameConflicts(issues); + } + + /** + * checks whether {@link #newName} is valid java identifier + * @param issues + */ + protected void checkNewNameIsValid(List issues) { + } + + protected void detectNameConflicts(List issues) { + } + + + protected boolean isJavaIdentifier(String name) { + return javaIdentifierRE.matcher(name).matches(); + } + + public T getTarget() { + return target; + } + + public void setTarget(T target) { + this.target = target; + } + + public String getNewName() { + return newName; + } + + public void setNewName(String newName) { + if (newNameValidationRE != null && newNameValidationRE.matcher(newName).matches() == false) { + throw new SpoonException("New name \"" + newName + "\" is not valid name"); + } + this.newName = newName; + } +} diff --git a/src/main/java/spoon/refactoring/ChangeLocalVariableName.java b/src/main/java/spoon/refactoring/ChangeLocalVariableName.java new file mode 100644 index 00000000000..47bb5045123 --- /dev/null +++ b/src/main/java/spoon/refactoring/ChangeLocalVariableName.java @@ -0,0 +1,238 @@ +/** + * 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.Collection; +import java.util.List; +import java.util.regex.Pattern; + +import spoon.SpoonException; +import spoon.reflect.code.CtCatchVariable; +import spoon.reflect.code.CtLocalVariable; +import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtField; +import spoon.reflect.declaration.CtParameter; +import spoon.reflect.declaration.CtType; +import spoon.reflect.declaration.CtVariable; +import spoon.reflect.reference.CtFieldReference; +import spoon.reflect.reference.CtLocalVariableReference; +import spoon.reflect.reference.CtReference; +import spoon.reflect.reference.CtVariableReference; +import spoon.reflect.visitor.Filter; +import spoon.reflect.visitor.chain.CtConsumer; +import spoon.reflect.visitor.chain.CtQueryable; +import spoon.reflect.visitor.chain.CtScannerListener; +import spoon.reflect.visitor.chain.ScanningMode; +import spoon.reflect.visitor.filter.LocalVariableReferenceFunction; +import spoon.reflect.visitor.filter.LocalVariableScopeFunction; +import spoon.reflect.visitor.filter.PotentialVariableDeclarationFunction; +import spoon.reflect.visitor.filter.SiblingsFunction; +import spoon.reflect.visitor.filter.SiblingsFunction.Mode; +import spoon.reflect.visitor.filter.VariableReferenceFunction; + +public class ChangeLocalVariableName extends AbstractRenameRefactor> { + + public static Pattern validVariableNameRE = javaIdentifierRE; + + public ChangeLocalVariableName() { + super(validVariableNameRE); + } + + @Override + protected void forEachReference(CtConsumer consumer) { + getTarget().map(new VariableReferenceFunction()).forEach(consumer); + } + + private static class QueryDriver implements CtScannerListener { + int nrOfNestedLocalClasses = 0; + CtElement ignoredParent; + + @Override + public ScanningMode enter(CtElement element) { + if (ignoredParent != null && element instanceof CtElement) { + CtElement ele = (CtElement) element; + if (ele.hasParent(ignoredParent)) { + return ScanningMode.SKIP_ALL; + } + } + if (element instanceof CtType) { + nrOfNestedLocalClasses++; + } + return ScanningMode.NORMAL; + } + + @Override + public void exit(CtElement element) { + if (ignoredParent == element) { + //we are living scope of ignored parent. We can stop checking it + ignoredParent = null; + } + if (element instanceof CtType) { + nrOfNestedLocalClasses--; + } + } + + public void ignoreChildrenOf(CtElement element) { + if (ignoredParent != null) { + throw new SpoonException("Unexpected state. The ignoredParent is already set"); + } + ignoredParent = element; + } + + public boolean isInContextOfLocalClass() { + return nrOfNestedLocalClasses > 0; + } + } + + @Override + protected void detectNameConflicts(final List issues) { + /* + * There can be these conflicts + * 1) target variable would shadow before declared variable (parameter, localVariable, catchVariable) + * -------------------------------------------------------------------------------------------------- + */ + PotentialVariableDeclarationFunction potentialDeclarationFnc = new PotentialVariableDeclarationFunction(newName); + CtVariable var = getTarget().map(potentialDeclarationFnc).first(); + if (var != null) { + if (var instanceof CtField) { + /* + * we have found a field of same name. + * It is not problem, because variables can hide field declaration. + * Do nothing - OK + */ + } else if (potentialDeclarationFnc.isTypeOnTheWay()) { + /* + * There is a local class declaration between future variable reference and variable declaration `var`. + * The found variable declaration `var` can be hidden by target variable with newName + * as long as there is no reference to `var` in visibility scope of the target variable. + * So search for such `var` reference now + */ + CtVariableReference shadowedVar = target + .map(new SiblingsFunction().includingSelf(true).mode(Mode.NEXT)) + .map(new VariableReferenceFunction(var)).first(); + if (shadowedVar != null) { + //found variable reference, which would be shadowed by variable after rename. + createNameConflictIssue(issues, var, shadowedVar); + } else { + /* + * there is no local variable reference, which would be shadowed by variable after rename. + * OK + */ + } + } else { + /* + * the found variable is in conflict with target variable with newName + */ + createNameConflictIssue(issues, var); + } + } + /* + * 2) target variable is shadowed by later declared variable + * --------------------------------------------------------- + */ + final QueryDriver queryDriver = new QueryDriver(); + getTarget().map(new LocalVariableScopeFunction(queryDriver)).select(new Filter() { + /** + * return true for all CtVariables, which are in conflict + */ + @Override + public boolean matches(CtElement element) { + if (element instanceof CtType) { + CtType localClass = (CtType) element; + //TODO use faster hasField, implemented using map(new AllFieldsFunction()).select(new NameFilter(newName)).first()!=null + Collection> fields = localClass.getAllFields(); + for (CtFieldReference fieldRef : fields) { + if (newName.equals(fieldRef.getSimpleName())) { + /* + * we have found a local class field, which will shadow input local variable if it's reference is in visibility scope of that field. + * Search for target variable reference in visibility scope of this field. + * If found than we cannot rename target variable to newName, because that reference would be shadowed + */ + queryDriver.ignoreChildrenOf(element); + CtLocalVariableReference shadowedVar = element.map(new LocalVariableReferenceFunction(target)).first(); + if (shadowedVar != null) { + createNameConflictIssue(issues, fieldRef.getFieldDeclaration(), shadowedVar); + return true; + } + return false; + } + } + return false; + } + if (element instanceof CtVariable) { + CtVariable variable = (CtVariable) element; + if (newName.equals(variable.getSimpleName()) == false) { + //the variable with different name. Ignore it + return false; + } + //we have found a variable with new name + if (variable instanceof CtField) { + throw new SpoonException("This should not happen. The children of local class which contains a field with new name should be skipped!"); + } + if (variable instanceof CtCatchVariable || variable instanceof CtLocalVariable || variable instanceof CtParameter) { + /* + * we have found a catch variable or local variable or parameter with new name. + */ + if (queryDriver.isInContextOfLocalClass()) { + /* + * We are in context of local class. + * This variable would shadow input local variable after rename + * so we cannot rename if there exist a local variable reference in variable visibility scope. + */ + queryDriver.ignoreChildrenOf(variable.getParent()); + CtQueryable searchScope; + if (variable instanceof CtLocalVariable) { + searchScope = variable.map(new SiblingsFunction().includingSelf(true).mode(Mode.NEXT)); + } else { + searchScope = variable.getParent(); + } + + CtLocalVariableReference shadowedVar = searchScope.map(new LocalVariableReferenceFunction(target)).first(); + if (shadowedVar != null) { + //found local variable reference, which would be shadowed by variable after rename. + createNameConflictIssue(issues, variable, shadowedVar); + return true; + } + //there is no local variable reference, which would be shadowed by variable after rename. + return false; + } else { + /* + * We are not in context of local class. + * So this variable is in conflict. Return it + */ + createNameConflictIssue(issues, variable); + return true; + } + } else { + //CtField should not be there, because the children of local class which contains a field with new name should be skipped! + //Any new variable type??? + throw new SpoonException("Unexpected variable " + variable.getClass().getName()); + } + } + return false; + } + }).first(); + } + + protected void createNameConflictIssue(List issues, CtVariable conflictVar) { + issues.add(new IssueImpl(conflictVar.getClass().getSimpleName() + " with name " + conflictVar.getSimpleName() + " is in conflict.")); + } + protected void createNameConflictIssue(List issues, CtVariable conflictVar, CtVariableReference shadowedVarRef) { + issues.add(new IssueImpl(conflictVar.getClass().getSimpleName() + " with name " + conflictVar.getSimpleName() + " would shadow local variable reference.")); + } + +} diff --git a/src/main/java/spoon/refactoring/Issue.java b/src/main/java/spoon/refactoring/Issue.java new file mode 100644 index 00000000000..011278c9805 --- /dev/null +++ b/src/main/java/spoon/refactoring/Issue.java @@ -0,0 +1,21 @@ +/** + * 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; + +public interface Issue { + String getMessage(); +} diff --git a/src/main/java/spoon/refactoring/IssueImpl.java b/src/main/java/spoon/refactoring/IssueImpl.java new file mode 100644 index 00000000000..9161fc0575a --- /dev/null +++ b/src/main/java/spoon/refactoring/IssueImpl.java @@ -0,0 +1,36 @@ +/** + * 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; + +public class IssueImpl implements Issue { + + private String message; + + public IssueImpl(String message) { + this.message = message; + } + + @Override + public String getMessage() { + return message; + } + + @Override + public String toString() { + return getMessage(); + } +} diff --git a/src/main/java/spoon/refactoring/Refactor.java b/src/main/java/spoon/refactoring/Refactor.java new file mode 100644 index 00000000000..b6ab969d09c --- /dev/null +++ b/src/main/java/spoon/refactoring/Refactor.java @@ -0,0 +1,27 @@ +/** + * 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.List; + +/** + * + */ +public interface Refactor { + List getIssues(); + void refactor(); +} diff --git a/src/test/java/spoon/test/refactoring/ChangeVariableNameTest.java b/src/test/java/spoon/test/refactoring/ChangeVariableNameTest.java new file mode 100644 index 00000000000..5730c937b94 --- /dev/null +++ b/src/test/java/spoon/test/refactoring/ChangeVariableNameTest.java @@ -0,0 +1,244 @@ +package spoon.test.refactoring; + +import static org.junit.Assert.*; + +import java.io.File; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLClassLoader; + +import org.junit.Before; +import org.junit.Test; + +import spoon.Launcher; +import spoon.OutputType; +import spoon.SpoonException; +import spoon.SpoonModelBuilder; +import spoon.refactoring.ChangeLocalVariableName; +import spoon.reflect.code.CtLocalVariable; +import spoon.reflect.declaration.CtAnnotation; +import spoon.reflect.declaration.CtClass; +import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtMethod; +import spoon.reflect.declaration.CtType; +import spoon.reflect.declaration.CtVariable; +import spoon.reflect.factory.Factory; +import spoon.reflect.reference.CtTypeReference; +import spoon.test.refactoring.testclasses.TryRename; +import spoon.test.refactoring.testclasses.VariableRename; +import spoon.testing.utils.ModelUtils; + +public class ChangeVariableNameTest +{ + Launcher launcher; + Factory factory; + CtClass varRenameClass; + CtTypeReference tryRename; + CtLocalVariable local1Var; + + @Before + public void setup() throws Exception { + varRenameClass = (CtClass)ModelUtils.buildClass(VariableRename.class); + launcher = new Launcher(); + launcher.addInputResource("./src/test/java/spoon/test/refactoring/testclasses/VariableRename.java"); + File outputBinDirectory = new File("./target/spooned-refactoring-test"); + if (!outputBinDirectory.exists()) { + outputBinDirectory.mkdirs(); + } + launcher.setBinaryOutputDirectory(outputBinDirectory); + launcher.setSourceOutputDirectory(outputBinDirectory); + launcher.buildModel(); + factory = launcher.getFactory(); + varRenameClass = factory.Class().get(VariableRename.class); + tryRename = factory.createCtTypeReference(TryRename.class); + local1Var = varRenameClass.filterChildren((CtLocalVariable var)->var.getSimpleName().equals("local1")).first(); + } + + @Test + public void testModelConsistency() throws Throwable { + new VariableRename(); + } + + String[] DEBUG = new String[]{/*"nestedClassMethodWithoutRefs", "var3", "var1"*/}; + + @Test + public void testRenameLocalVariableToUsedName() throws Exception { + + varRenameClass.getMethods().forEach(method->{ + //debugging support + if(DEBUG.length==3 && DEBUG[0].equals(method.getSimpleName())==false) return; + method.filterChildren((CtVariable var)->true) + .map((CtVariable var)->var.getAnnotation(tryRename)) + .forEach((CtAnnotation annotation)->{ + String[] newNames = annotation.getActualAnnotation().value(); + CtVariable targetVariable = (CtVariable)annotation.getAnnotatedElement(); + for (String newName : newNames) { + boolean renameShouldPass = newName.startsWith("-")==false; + if (!renameShouldPass) { + newName = newName.substring(1); + } + if (targetVariable instanceof CtLocalVariable) { + //debugging support + if(DEBUG.length==3 && DEBUG[1].equals(targetVariable.getSimpleName()) && DEBUG[2].equals(newName)) { + //put breakpoint here and continue debugging of the buggy case + this.getClass(); + } + checkLocalVariableRename((CtLocalVariable) targetVariable, newName, renameShouldPass); + } else { + //TODO rename of other variables + } + } + }); + }); + } + + protected void checkLocalVariableRename(CtLocalVariable targetVariable, String newName, boolean renameShouldPass) { + + String originName = targetVariable.getSimpleName(); + ChangeLocalVariableName refactor = new ChangeLocalVariableName(); + refactor.setTarget(targetVariable); + refactor.setNewName(newName); + if(renameShouldPass) { + try { + refactor.refactor(); + } catch(SpoonException e) { + throw new AssertionError(getParentMethodName(targetVariable)+" Rename of \""+originName+"\" should NOT fail when trying rename to \""+newName+"\"\n"+targetVariable.toString(), e); + } + assertEquals(getParentMethodName(targetVariable)+" Rename of \""+originName+"\" to \""+newName+"\" passed, but the name of variable was not changed", newName, targetVariable.getSimpleName()); + printModelAndTestConsistency(getParentMethodName(targetVariable)+" Rename of \""+originName+"\" to \""+newName+"\""); + } else { + try { + refactor.refactor(); + fail(getParentMethodName(targetVariable)+" Rename of \""+originName+"\" should fail when trying rename to \""+newName+"\""); + } catch(SpoonException e) { + } + assertEquals(getParentMethodName(targetVariable)+" Rename of \""+originName+"\" failed when trying rename to \""+newName+"\" but the name of variable should not be changed", originName, targetVariable.getSimpleName()); + } + if(renameShouldPass) { + rollback(targetVariable, originName); + } + assertEquals(originName, targetVariable.getSimpleName()); + } + + private void rollback(CtLocalVariable targetVariable, String originName) { + String newName = targetVariable.getSimpleName(); + ChangeLocalVariableName refactor = new ChangeLocalVariableName(); + refactor.setTarget(targetVariable); + //rollback changes + refactor.setNewName(originName); + try { + refactor.refactor(); + } catch(SpoonException e) { + throw new AssertionError(getParentMethodName(targetVariable)+" Rename of \""+originName+"\" to \""+newName+"\" passed, but rename back to \""+originName+"\" failed", e); + } + } + + private void printModelAndTestConsistency(String refactoringDescription) { +// 1) print modified model, + try { + launcher.getModelBuilder().generateProcessedSourceFiles(OutputType.CLASSES); + } catch (Throwable e) { + new AssertionError("The printing of java sources failed after: "+refactoringDescription, e); + } + +// 2) build it + try { +// launcher.getModelBuilder().compile(SpoonModelBuilder.InputType.FILES); + launcher.getModelBuilder().compile(SpoonModelBuilder.InputType.CTTYPES); + } catch (Throwable e) { + new AssertionError("The compilation of java sources in "+launcher.getEnvironment().getBinaryOutputDirectory()+" failed after: "+refactoringDescription, e); + } +// 3) create instance using that new model and test consistency + try { +// varRenameClass.newInstance(); + TestClassloader classLoader = new TestClassloader(); + Class testModelClass = classLoader.loadClass(VariableRename.class.getName()); + testModelClass.newInstance(); + } catch (Throwable e) { + throw new AssertionError("The model validation of code in "+launcher.getEnvironment().getBinaryOutputDirectory()+" failed after: "+refactoringDescription, e); + } + } + + private class TestClassloader extends URLClassLoader { + TestClassloader() throws MalformedURLException { + super(new URL[] { new File(launcher.getEnvironment().getBinaryOutputDirectory()).toURL()}, ChangeVariableNameTest.class.getClassLoader()); + } + + @Override + public Class loadClass(String s) throws ClassNotFoundException { + try { + return findClass(s); + } catch (Exception e) { + return super.loadClass(s); + } + } + } + + + private String getParentMethodName(CtElement ele) { + CtMethod parentMethod = ele.getParent(CtMethod.class); + CtMethod m; + while(parentMethod!=null && (m=parentMethod.getParent(CtMethod.class))!=null) { + parentMethod = m; + } + if(parentMethod!=null) { + return parentMethod.getParent(CtType.class).getSimpleName()+"#"+parentMethod.getSimpleName(); + } else { + return ele.getParent(CtType.class).getSimpleName()+"#annonymous block"; + } + } + + + @Test + public void testRefactorWithoutTarget() throws Exception { + + ChangeLocalVariableName refactor = new ChangeLocalVariableName(); + refactor.setNewName("local1"); + try { + refactor.refactor(); + fail(); + } catch(SpoonException e) { + + } + } + + @Test + public void testRenameLocalVariableToSameName() throws Exception { + + ChangeLocalVariableName refactor = new ChangeLocalVariableName(); + refactor.setTarget(local1Var); + refactor.setNewName("local1"); + refactor.refactor(); + assertEquals("local1", local1Var.getSimpleName()); + } + + @Test + public void testRenameLocalVariableToInvalidName() throws Exception { + + ChangeLocalVariableName refactor = new ChangeLocalVariableName(); + refactor.setTarget(local1Var); + try { + refactor.setNewName(""); + fail(); + } catch(SpoonException e) { + } + + try { + refactor.setNewName("x "); + fail(); + } catch(SpoonException e) { + } + + try { + refactor.setNewName("x y"); + fail(); + } catch(SpoonException e) { + } + + try { + refactor.setNewName("x("); + fail(); + } catch(SpoonException e) { + } + } +} diff --git a/src/test/java/spoon/test/refactoring/testclasses/TryRename.java b/src/test/java/spoon/test/refactoring/testclasses/TryRename.java new file mode 100644 index 00000000000..60f550614ac --- /dev/null +++ b/src/test/java/spoon/test/refactoring/testclasses/TryRename.java @@ -0,0 +1,16 @@ +package spoon.test.refactoring.testclasses; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target({ ElementType.LOCAL_VARIABLE, ElementType.PARAMETER, ElementType.FIELD}) +public @interface TryRename { + /** + * @return the list of names which should be tried by refactoring. + * If the name starts with prefix "-", then this refactoring should fail on some validation issue + */ + String[] value(); +} diff --git a/src/test/java/spoon/test/refactoring/testclasses/VariableRename.java b/src/test/java/spoon/test/refactoring/testclasses/VariableRename.java new file mode 100644 index 00000000000..25278e13f7e --- /dev/null +++ b/src/test/java/spoon/test/refactoring/testclasses/VariableRename.java @@ -0,0 +1,244 @@ +package spoon.test.refactoring.testclasses; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.function.Consumer; +import java.util.function.Function; + +import static org.junit.Assert.*; + +public class VariableRename +{ + public VariableRename() throws Throwable + { + int local1 = 0; + //call all not private methods of this class automatically, to check assertions + Method[] methods = getClass().getDeclaredMethods(); + for (Method method : methods) { + try { + if(Modifier.isPrivate(method.getModifiers())) { + continue; + } + method.invoke(this); + } catch (InvocationTargetException e) { + throw e.getTargetException(); + } catch (IllegalAccessException | IllegalArgumentException e) { + throw new RuntimeException("Invocation of method "+method.getName()+" failed", e); + } + } + } + + void callConflictWithParam() { + conflictWithParam(2); + } + + /** + * tests conflict of local variable with parameter + */ + private void conflictWithParam(@TryRename("-var1") int var2) { + @TryRename("-var2") + int var1 = 1; + assertTrue(var1 == 1); + assertTrue(var2 == 2); + } + + /** + * tests conflict of local variable with CtCatchVariable + */ + private void conflictWithCatchVariable() { + @TryRename({"-var2", "-var3"}) + int var1 = 1; + try { + assertTrue(var1 == 1); + @TryRename({"-var1", "var3"}) + int var2 = 2; + assertTrue(var2 == 2); + throw new NumberFormatException(); + } catch (@TryRename({"-var1", "var2"}) NumberFormatException var3) { + assertTrue(var1 == 1); + } + assertTrue(var1 == 1); + } + + /** + * Tests nested class and conflict with field, + * and nested local variable references, which must would be shadowed + */ + void nestedClassMethodWithRefs() { + @TryRename({"-var2", "-var3", "-var4", "-var5", "-var6"}) + int var1 = 1; + new Consumer() { + //must not rename to var1, because it would shadow var1 reference below + @TryRename({"-var1", "-var3", "-var4", "-var5", "-var6"}) + int var2 = 2; + @Override + public void accept( + //must not rename to var1, because reference to var1 below would be shadowed + @TryRename({"-var1", "var2", "-var3", "-var5", "-var6"}) Integer var4 + ) { + //cannot rename to var1, because reference to var1 below would be shadowed + @TryRename({"-var1", "var2", "-var4", "-var5", "-var6"}) + int var3 = 3; + try { + //cannot rename to var1, because reference to var1 below would be shadowed + @TryRename({"-var1", "var2", "-var3", "-var4", "var6"}) + int var5 = 5; + assertTrue(var1 == 1); + assertTrue(var2 == 2); + assertTrue(var3 == 3); + assertTrue(var4 == 4); + assertTrue(var5 == 5); + throw new NumberFormatException(); + } catch ( + //cannot rename to var1, because reference to var1 below would be shadowed + @TryRename({"-var1", "var2", "-var3", "-var4", "var5"}) NumberFormatException var6) { + assertTrue(var1 == 1); + assertTrue(var2 == 2); + assertTrue(var3 == 3); + assertTrue(var4 == 4); + } + } + }.accept(4); + assertTrue(var1 == 1); + } + /** + * Tests nested class and conflict with field, + * and no nested local variable references so rename is possible + */ + void nestedClassMethodWithoutRefs() { + @TryRename({"var2", "var3", "var4", "var5", "var6"}) + int var1 = 1; + new Consumer() { + //must not rename to var1, because it would shadow var1 reference below + @TryRename({"-var1", "var3", "var4", "var5", "var6"}) + int var2 = 2; + @Override + public void accept( + //must not rename to var1, because reference to var1 below would be shadowed + @TryRename({"-var1", "var2", "-var3", "-var5", "-var6"}) Integer var4 + ) { + //can rename to var1, because reference to var1 below is not shadowed + @TryRename({"var1", "var2", "-var4", "-var5", "-var6"}) + int var3 = 3; + try { + //can rename to var1, because reference to var1 below is not shadowed + @TryRename({"var1", "var2", "-var3", "-var4", "var6"}) + int var5 = 5; +// assertTrue(var1 == 1);//do not reference it in scope of other vars, so it can be renamed + assertTrue(var2 == 2); + assertTrue(var3 == 3); + assertTrue(var4 == 4); + assertTrue(var5 == 5); + throw new NumberFormatException(); + } catch ( + //can rename to var1, because reference to var1 below is not shadowed + @TryRename({"var1", "var2", "-var3", "-var4", "var5"}) NumberFormatException var6) { +// assertTrue(var1 == 1);//do not reference it in scope of other vars, so it can be renamed + assertTrue(var2 == 2); + assertTrue(var3 == 3); + assertTrue(var4 == 4); + } + } + }.accept(4); + assertTrue(var1 == 1); + } + + void nestedClassMethodWithShadowVarWithRefs() { + @TryRename({"-var2", "var3"}) + int var1 = 2; + new Runnable() { + @TryRename({"var1", "var3"}) + int var2 = 3; + @Override + public void run() { + assertTrue(var1 == 2); + @TryRename({"-var1", "var2"}) + int var3 = 1; + //this var1 shadows above defined var1. It can be renamed + @TryRename({"var2", "-var3"}) + int var1 = 4; + assertTrue(var1 == 4); + assertTrue(var2 == 3); + assertTrue(var3 == 1); + } + }.run(); + assertTrue(var1 == 2); + } + void nestedClassMethodWithShadowVarWithoutRefs() { + @TryRename({"var2", "var3"}) + int var1 = 2; + new Runnable() { + @TryRename({"var1", "var3"}) + int var2 = 3; + @Override + public void run() { + @TryRename({"-var1", "var2"}) +// assertTrue(var1 == 2); //the var1 is not referenced so it can be renamed + int var3 = 1; + //this var1 shadows above defined var1. It can be renamed + @TryRename({"var2", "-var3"}) + int var1 = 4; + assertTrue(var1 == 4); + assertTrue(var2 == 3); + assertTrue(var3 == 1); + } + }.run(); + assertTrue(var1 == 2); + } + + void nestedClassMethodWithShadowVarAndField() { + @TryRename({"var2", "var3"}) + int var1 = 2; + new Runnable() { + @TryRename({"var2", "var3"}) + //this var1 shadows above defined var1. + int var1 = 3; + @Override + public void run() { + @TryRename({"-var1", "var2"}) + int var2 = 1; + assertTrue(var1 == 3); + @TryRename({"-var2", "var3"}) + int var1 = 4; + assertTrue(var1 == 4); + assertTrue(this.var1 == 3); + assertTrue(var2 == 1); + } + }.run(); + assertTrue(var1 == 2); + } + + void lambda() { + @TryRename({"-var2", "-var3"}) + int var1 = 1; + assertTrue(var1 == 1); + Function fnc = (@TryRename({"-var1", "-var3"}) Integer var2)->{ + @TryRename({"-var1", "-var2"}) + int var3 = 3; + assertTrue(var1 == 1); + assertTrue(var2 == 2); + assertTrue(var3 == 3); + return var2; + }; + assertTrue(fnc.apply(2) == 2); + } + + void tryCatch() { + @TryRename({"-var2", "-var3", "-var4"}) + int var1 = 1; + assertTrue(var1 == 1); + try { + @TryRename({"-var1","var3","var4"}) + int var2 = 2; + assertTrue(var1 == 1); + assertTrue(var2 == 2); + throw new Exception("ex2"); + } catch (@TryRename({"-var1", "var2", "-var4"}) Exception var3) { + @TryRename({"-var1", "var2", "-var3"}) + int var4 = 4; + assertTrue(var1 == 1); + assertTrue(var4 == 4); + } + } +} From 85135df1aeacc4af33357bba1a70e25b387ca11b Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Sun, 19 Mar 2017 09:45:04 +0100 Subject: [PATCH 2/6] feature: RenameLocalVariableRefactor --- .../refactoring/AbstractRenameRefactor.java | 38 +---- src/main/java/spoon/refactoring/Issue.java | 21 --- .../java/spoon/refactoring/IssueImpl.java | 36 ----- src/main/java/spoon/refactoring/Refactor.java | 14 +- .../refactoring/RefactoringException.java | 23 +++ ....java => RenameLocalVariableRefactor.java} | 62 +++++--- ...a => RenameLocalVariableRefactorTest.java} | 135 ++++++++++-------- ...nameLocalVariableRefactorTestSubject.java} | 84 ++++++----- .../{TryRename.java => TestTryRename.java} | 2 +- 9 files changed, 204 insertions(+), 211 deletions(-) delete mode 100644 src/main/java/spoon/refactoring/Issue.java delete mode 100644 src/main/java/spoon/refactoring/IssueImpl.java create mode 100644 src/main/java/spoon/refactoring/RefactoringException.java rename src/main/java/spoon/refactoring/{ChangeLocalVariableName.java => RenameLocalVariableRefactor.java} (76%) rename src/test/java/spoon/test/refactoring/{ChangeVariableNameTest.java => RenameLocalVariableRefactorTest.java} (60%) rename src/test/java/spoon/test/refactoring/testclasses/{VariableRename.java => RenameLocalVariableRefactorTestSubject.java} (68%) rename src/test/java/spoon/test/refactoring/testclasses/{TryRename.java => TestTryRename.java} (91%) diff --git a/src/main/java/spoon/refactoring/AbstractRenameRefactor.java b/src/main/java/spoon/refactoring/AbstractRenameRefactor.java index a43f238297d..2510906ca6c 100644 --- a/src/main/java/spoon/refactoring/AbstractRenameRefactor.java +++ b/src/main/java/spoon/refactoring/AbstractRenameRefactor.java @@ -16,14 +16,10 @@ */ package spoon.refactoring; -import java.util.ArrayList; -import java.util.List; import java.util.regex.Pattern; import spoon.SpoonException; import spoon.reflect.declaration.CtNamedElement; -import spoon.reflect.reference.CtReference; -import spoon.reflect.visitor.chain.CtConsumer; public abstract class AbstractRenameRefactor implements Refactor { public static final Pattern javaIdentifierRE = Pattern.compile("\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*"); @@ -44,45 +40,25 @@ public void refactor() { if (getNewName() == null) { throw new SpoonException("The new name of refactoring is not defined"); } - List issues = getIssues(); - if (issues.isEmpty() == false) { - throw new SpoonException("Refactoring cannot be processed. There are issues: " + issues.toString()); - } + detectIssues(); refactorNoCheck(); } - protected void refactorNoCheck() { - forEachReference(new CtConsumer() { - @Override - public void accept(CtReference t) { - t.setSimpleName(AbstractRenameRefactor.this.newName); - } - }); - target.setSimpleName(newName); - } - - protected abstract void forEachReference(CtConsumer consumer); - - @Override - public List getIssues() { - List issues = new ArrayList<>(); - detectIssues(issues); - return issues; - } + protected abstract void refactorNoCheck(); - protected void detectIssues(List issues) { - checkNewNameIsValid(issues); - detectNameConflicts(issues); + protected void detectIssues() { + checkNewNameIsValid(); + detectNameConflicts(); } /** * checks whether {@link #newName} is valid java identifier * @param issues */ - protected void checkNewNameIsValid(List issues) { + protected void checkNewNameIsValid() { } - protected void detectNameConflicts(List issues) { + protected void detectNameConflicts() { } diff --git a/src/main/java/spoon/refactoring/Issue.java b/src/main/java/spoon/refactoring/Issue.java deleted file mode 100644 index 011278c9805..00000000000 --- a/src/main/java/spoon/refactoring/Issue.java +++ /dev/null @@ -1,21 +0,0 @@ -/** - * 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; - -public interface Issue { - String getMessage(); -} diff --git a/src/main/java/spoon/refactoring/IssueImpl.java b/src/main/java/spoon/refactoring/IssueImpl.java deleted file mode 100644 index 9161fc0575a..00000000000 --- a/src/main/java/spoon/refactoring/IssueImpl.java +++ /dev/null @@ -1,36 +0,0 @@ -/** - * 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; - -public class IssueImpl implements Issue { - - private String message; - - public IssueImpl(String message) { - this.message = message; - } - - @Override - public String getMessage() { - return message; - } - - @Override - public String toString() { - return getMessage(); - } -} diff --git a/src/main/java/spoon/refactoring/Refactor.java b/src/main/java/spoon/refactoring/Refactor.java index b6ab969d09c..2423510b45a 100644 --- a/src/main/java/spoon/refactoring/Refactor.java +++ b/src/main/java/spoon/refactoring/Refactor.java @@ -16,12 +16,18 @@ */ package spoon.refactoring; -import java.util.List; - /** - * + * Defines basic contract of all refactoring implementations.
+ * Usage:
+ * 
+ * SomeRefactoring r = new SomeRefactoring();
+ * //configure refactoring by calling setters on `r`
+ * r.refactor();
+ *
*/ public interface Refactor { - List getIssues(); + /** + * Process refactoring operation + */ void refactor(); } diff --git a/src/main/java/spoon/refactoring/RefactoringException.java b/src/main/java/spoon/refactoring/RefactoringException.java new file mode 100644 index 00000000000..31eb333a2cb --- /dev/null +++ b/src/main/java/spoon/refactoring/RefactoringException.java @@ -0,0 +1,23 @@ +package spoon.refactoring; + +import spoon.SpoonException; + +public class RefactoringException extends SpoonException { + private static final long serialVersionUID = 1L; + + public RefactoringException() { + } + + public RefactoringException(String msg) { + super(msg); + } + + public RefactoringException(Throwable e) { + super(e); + } + + public RefactoringException(String msg, Throwable e) { + super(msg, e); + } + +} diff --git a/src/main/java/spoon/refactoring/ChangeLocalVariableName.java b/src/main/java/spoon/refactoring/RenameLocalVariableRefactor.java similarity index 76% rename from src/main/java/spoon/refactoring/ChangeLocalVariableName.java rename to src/main/java/spoon/refactoring/RenameLocalVariableRefactor.java index 47bb5045123..6926a009efd 100644 --- a/src/main/java/spoon/refactoring/ChangeLocalVariableName.java +++ b/src/main/java/spoon/refactoring/RenameLocalVariableRefactor.java @@ -17,7 +17,6 @@ package spoon.refactoring; import java.util.Collection; -import java.util.List; import java.util.regex.Pattern; import spoon.SpoonException; @@ -44,17 +43,38 @@ import spoon.reflect.visitor.filter.SiblingsFunction.Mode; import spoon.reflect.visitor.filter.VariableReferenceFunction; -public class ChangeLocalVariableName extends AbstractRenameRefactor> { +/** + * Spoon model refactoring function which renames `target` local variable to `newName`
+ * This refactoring will throw {@link RefactoringException} if the model would be not consistent after rename to new name. + * The exception is thrown before the model modificatons are started. + * 
+ * CtLocalVariable anLocalVariable = ...
+ * RenameLocalVariableRefactor refactor = new RenameLocalVariableRefactor();
+ * refactor.setTarget(anLocalVariable);
+ * refactor.setNewName("someNewName");
+ * try {
+ *   refactor.refactor();
+ * } catch (RefactoringException e) {
+ *   //handle name conflict or name shadowing problem
+ * }
+ *
+ */ +public class RenameLocalVariableRefactor extends AbstractRenameRefactor> { public static Pattern validVariableNameRE = javaIdentifierRE; - public ChangeLocalVariableName() { + public RenameLocalVariableRefactor() { super(validVariableNameRE); } - @Override - protected void forEachReference(CtConsumer consumer) { - getTarget().map(new VariableReferenceFunction()).forEach(consumer); + protected void refactorNoCheck() { + getTarget().map(new VariableReferenceFunction()).forEach(new CtConsumer() { + @Override + public void accept(CtReference t) { + t.setSimpleName(newName); + } + }); + target.setSimpleName(newName); } private static class QueryDriver implements CtScannerListener { @@ -99,7 +119,7 @@ public boolean isInContextOfLocalClass() { } @Override - protected void detectNameConflicts(final List issues) { + protected void detectNameConflicts() { /* * There can be these conflicts * 1) target variable would shadow before declared variable (parameter, localVariable, catchVariable) @@ -126,7 +146,7 @@ protected void detectNameConflicts(final List issues) { .map(new VariableReferenceFunction(var)).first(); if (shadowedVar != null) { //found variable reference, which would be shadowed by variable after rename. - createNameConflictIssue(issues, var, shadowedVar); + createNameConflictIssue(var, shadowedVar); } else { /* * there is no local variable reference, which would be shadowed by variable after rename. @@ -137,7 +157,7 @@ protected void detectNameConflicts(final List issues) { /* * the found variable is in conflict with target variable with newName */ - createNameConflictIssue(issues, var); + createNameConflictIssue(var); } } /* @@ -165,7 +185,7 @@ public boolean matches(CtElement element) { queryDriver.ignoreChildrenOf(element); CtLocalVariableReference shadowedVar = element.map(new LocalVariableReferenceFunction(target)).first(); if (shadowedVar != null) { - createNameConflictIssue(issues, fieldRef.getFieldDeclaration(), shadowedVar); + createNameConflictIssue(fieldRef.getFieldDeclaration(), shadowedVar); return true; } return false; @@ -204,7 +224,7 @@ public boolean matches(CtElement element) { CtLocalVariableReference shadowedVar = searchScope.map(new LocalVariableReferenceFunction(target)).first(); if (shadowedVar != null) { //found local variable reference, which would be shadowed by variable after rename. - createNameConflictIssue(issues, variable, shadowedVar); + createNameConflictIssue(variable, shadowedVar); return true; } //there is no local variable reference, which would be shadowed by variable after rename. @@ -214,7 +234,7 @@ public boolean matches(CtElement element) { * We are not in context of local class. * So this variable is in conflict. Return it */ - createNameConflictIssue(issues, variable); + createNameConflictIssue(variable); return true; } } else { @@ -228,11 +248,19 @@ public boolean matches(CtElement element) { }).first(); } - protected void createNameConflictIssue(List issues, CtVariable conflictVar) { - issues.add(new IssueImpl(conflictVar.getClass().getSimpleName() + " with name " + conflictVar.getSimpleName() + " is in conflict.")); + /** + * Override this method to get access to details about this refactoring issue + * @param conflictVar - variable which would be in conflict with the `targetVariable` after it's rename to new name + */ + protected void createNameConflictIssue(CtVariable conflictVar) { + throw new RefactoringException(conflictVar.getClass().getSimpleName() + " with name " + conflictVar.getSimpleName() + " is in conflict."); } - protected void createNameConflictIssue(List issues, CtVariable conflictVar, CtVariableReference shadowedVarRef) { - issues.add(new IssueImpl(conflictVar.getClass().getSimpleName() + " with name " + conflictVar.getSimpleName() + " would shadow local variable reference.")); + /** + * Override this method to get access to details about this refactoring issue + * @param conflictVar - variable which would shadow reference to `targetVariable` after it's rename to new name + * @param shadowedVarRef - the reference to `targetVariable`, which would be shadowed by `conflictVar` + */ + protected void createNameConflictIssue(CtVariable conflictVar, CtVariableReference shadowedVarRef) { + throw new RefactoringException(conflictVar.getClass().getSimpleName() + " with name " + conflictVar.getSimpleName() + " would shadow local variable reference."); } - } diff --git a/src/test/java/spoon/test/refactoring/ChangeVariableNameTest.java b/src/test/java/spoon/test/refactoring/RenameLocalVariableRefactorTest.java similarity index 60% rename from src/test/java/spoon/test/refactoring/ChangeVariableNameTest.java rename to src/test/java/spoon/test/refactoring/RenameLocalVariableRefactorTest.java index 5730c937b94..f336d7d73e7 100644 --- a/src/test/java/spoon/test/refactoring/ChangeVariableNameTest.java +++ b/src/test/java/spoon/test/refactoring/RenameLocalVariableRefactorTest.java @@ -3,18 +3,18 @@ import static org.junit.Assert.*; import java.io.File; +import java.lang.reflect.InvocationTargetException; import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; -import org.junit.Before; import org.junit.Test; import spoon.Launcher; import spoon.OutputType; import spoon.SpoonException; import spoon.SpoonModelBuilder; -import spoon.refactoring.ChangeLocalVariableName; +import spoon.refactoring.RenameLocalVariableRefactor; import spoon.reflect.code.CtLocalVariable; import spoon.reflect.declaration.CtAnnotation; import spoon.reflect.declaration.CtClass; @@ -22,54 +22,48 @@ import spoon.reflect.declaration.CtMethod; import spoon.reflect.declaration.CtType; import spoon.reflect.declaration.CtVariable; -import spoon.reflect.factory.Factory; import spoon.reflect.reference.CtTypeReference; -import spoon.test.refactoring.testclasses.TryRename; -import spoon.test.refactoring.testclasses.VariableRename; +import spoon.test.refactoring.testclasses.TestTryRename; +import spoon.test.refactoring.testclasses.RenameLocalVariableRefactorTestSubject; import spoon.testing.utils.ModelUtils; -public class ChangeVariableNameTest +public class RenameLocalVariableRefactorTest { - Launcher launcher; - Factory factory; - CtClass varRenameClass; - CtTypeReference tryRename; - CtLocalVariable local1Var; - - @Before - public void setup() throws Exception { - varRenameClass = (CtClass)ModelUtils.buildClass(VariableRename.class); - launcher = new Launcher(); - launcher.addInputResource("./src/test/java/spoon/test/refactoring/testclasses/VariableRename.java"); - File outputBinDirectory = new File("./target/spooned-refactoring-test"); - if (!outputBinDirectory.exists()) { - outputBinDirectory.mkdirs(); - } - launcher.setBinaryOutputDirectory(outputBinDirectory); - launcher.setSourceOutputDirectory(outputBinDirectory); - launcher.buildModel(); - factory = launcher.getFactory(); - varRenameClass = factory.Class().get(VariableRename.class); - tryRename = factory.createCtTypeReference(TryRename.class); - local1Var = varRenameClass.filterChildren((CtLocalVariable var)->var.getSimpleName().equals("local1")).first(); - } - @Test public void testModelConsistency() throws Throwable { - new VariableRename(); + //contract: check that all assertions in all methods of the RenameLocalVariableRefactorTestSubject are correct + new RenameLocalVariableRefactorTestSubject().checkModelConsistency(); } - String[] DEBUG = new String[]{/*"nestedClassMethodWithoutRefs", "var3", "var1"*/}; + /** + * If you need to debug behavior of refactoring on the exact method and variable in the {@link RenameLocalVariableRefactorTestSubject} model, + * then provide + * 1) name of method of {@link RenameLocalVariableRefactorTestSubject} + * 2) original name variable in the method + * 3) new name of variable in the method + * then put breakpoint on the line `this.getClass();` below and the debugger stops just before + * the to be inspected refactoring starts + */ + private String[] DEBUG = new String[]{/*"nestedClassMethodWithoutRefs", "var3", "var1"*/}; + /** + * The {@link RenameLocalVariableRefactorTestSubject} class is loaded as spoon model. Then: + * - It looks for each CtVariable and it's CtAnnotation and tries to rename that variable to the name defined by annotation. + * - If the annotation name is prefixed with "-", then that refactoring should fail. + * - If the annotation name is not prefixed, then that refactoring should pass. + * If it behaves different then expected, then this test fails + */ @Test - public void testRenameLocalVariableToUsedName() throws Exception { + public void testRenameAllLocalVariablesOfRenameTestSubject() throws Exception { + CtClass varRenameClass = (CtClass)ModelUtils.buildClass(RenameLocalVariableRefactorTestSubject.class); + CtTypeReference tryRename = varRenameClass.getFactory().createCtTypeReference(TestTryRename.class); varRenameClass.getMethods().forEach(method->{ //debugging support if(DEBUG.length==3 && DEBUG[0].equals(method.getSimpleName())==false) return; method.filterChildren((CtVariable var)->true) .map((CtVariable var)->var.getAnnotation(tryRename)) - .forEach((CtAnnotation annotation)->{ + .forEach((CtAnnotation annotation)->{ String[] newNames = annotation.getActualAnnotation().value(); CtVariable targetVariable = (CtVariable)annotation.getAnnotatedElement(); for (String newName : newNames) { @@ -85,7 +79,7 @@ public void testRenameLocalVariableToUsedName() throws Exception { } checkLocalVariableRename((CtLocalVariable) targetVariable, newName, renameShouldPass); } else { - //TODO rename of other variables + //TODO test rename of other variables, e.g. parameters and catch... later } } }); @@ -95,7 +89,7 @@ public void testRenameLocalVariableToUsedName() throws Exception { protected void checkLocalVariableRename(CtLocalVariable targetVariable, String newName, boolean renameShouldPass) { String originName = targetVariable.getSimpleName(); - ChangeLocalVariableName refactor = new ChangeLocalVariableName(); + RenameLocalVariableRefactor refactor = new RenameLocalVariableRefactor(); refactor.setTarget(targetVariable); refactor.setNewName(newName); if(renameShouldPass) { @@ -122,7 +116,7 @@ protected void checkLocalVariableRename(CtLocalVariable targetVariable, Strin private void rollback(CtLocalVariable targetVariable, String originName) { String newName = targetVariable.getSimpleName(); - ChangeLocalVariableName refactor = new ChangeLocalVariableName(); + RenameLocalVariableRefactor refactor = new RenameLocalVariableRefactor(); refactor.setTarget(targetVariable); //rollback changes refactor.setNewName(originName); @@ -134,6 +128,14 @@ private void rollback(CtLocalVariable targetVariable, String originName) { } private void printModelAndTestConsistency(String refactoringDescription) { + Launcher launcher = new Launcher(); + File outputBinDirectory = new File("./target/spooned-refactoring-test"); + if (!outputBinDirectory.exists()) { + outputBinDirectory.mkdirs(); + } + launcher.setBinaryOutputDirectory(outputBinDirectory); + launcher.setSourceOutputDirectory(outputBinDirectory); + // 1) print modified model, try { launcher.getModelBuilder().generateProcessedSourceFiles(OutputType.CLASSES); @@ -151,17 +153,19 @@ private void printModelAndTestConsistency(String refactoringDescription) { // 3) create instance using that new model and test consistency try { // varRenameClass.newInstance(); - TestClassloader classLoader = new TestClassloader(); - Class testModelClass = classLoader.loadClass(VariableRename.class.getName()); - testModelClass.newInstance(); + TestClassloader classLoader = new TestClassloader(launcher); + Class testModelClass = classLoader.loadClass(RenameLocalVariableRefactorTestSubject.class.getName()); + testModelClass.getMethod("checkModelConsistency").invoke(testModelClass.newInstance()); + } catch (InvocationTargetException e) { + throw new AssertionError("The model validation of code in "+launcher.getEnvironment().getBinaryOutputDirectory()+" failed after: "+refactoringDescription, e.getTargetException()); } catch (Throwable e) { throw new AssertionError("The model validation of code in "+launcher.getEnvironment().getBinaryOutputDirectory()+" failed after: "+refactoringDescription, e); } } private class TestClassloader extends URLClassLoader { - TestClassloader() throws MalformedURLException { - super(new URL[] { new File(launcher.getEnvironment().getBinaryOutputDirectory()).toURL()}, ChangeVariableNameTest.class.getClassLoader()); + TestClassloader(Launcher launcher) throws MalformedURLException { + super(new URL[] { new File(launcher.getEnvironment().getBinaryOutputDirectory()).toURL()}, RenameLocalVariableRefactorTest.class.getClassLoader()); } @Override @@ -190,55 +194,62 @@ private String getParentMethodName(CtElement ele) { @Test - public void testRefactorWithoutTarget() throws Exception { + public void testRefactorWrongUsage() throws Exception { + CtType varRenameClass = ModelUtils.buildClass(RenameLocalVariableRefactorTestSubject.class); + CtLocalVariable local1Var = varRenameClass.filterChildren((CtLocalVariable var)->var.getSimpleName().equals("local1")).first(); - ChangeLocalVariableName refactor = new ChangeLocalVariableName(); + //contract: a target variable is not defined. Throw SpoonException + RenameLocalVariableRefactor refactor = new RenameLocalVariableRefactor(); refactor.setNewName("local1"); try { refactor.refactor(); fail(); } catch(SpoonException e) { - + //should fail - OK } - } - - @Test - public void testRenameLocalVariableToSameName() throws Exception { - - ChangeLocalVariableName refactor = new ChangeLocalVariableName(); - refactor.setTarget(local1Var); - refactor.setNewName("local1"); - refactor.refactor(); - assertEquals("local1", local1Var.getSimpleName()); - } - - @Test - public void testRenameLocalVariableToInvalidName() throws Exception { - - ChangeLocalVariableName refactor = new ChangeLocalVariableName(); + //contract: invalid rename request to empty string. Throw SpoonException refactor.setTarget(local1Var); try { refactor.setNewName(""); fail(); } catch(SpoonException e) { + //should fail - OK } + //contract: invalid rename request to variable name which contains space. Throw SpoonException try { refactor.setNewName("x "); fail(); } catch(SpoonException e) { + //should fail - OK } + //contract: invalid rename request to variable name which contains space. Throw SpoonException try { refactor.setNewName("x y"); fail(); } catch(SpoonException e) { + //should fail - OK } + //contract: invalid rename request to variable name which contains character which is not allowed in variable name. Throw SpoonException try { refactor.setNewName("x("); fail(); } catch(SpoonException e) { + //should fail - OK } - } + } + + @Test + public void testRenameLocalVariableToSameName() throws Exception { + CtType varRenameClass = ModelUtils.buildClass(RenameLocalVariableRefactorTestSubject.class); + CtLocalVariable local1Var = varRenameClass.filterChildren((CtLocalVariable var)->var.getSimpleName().equals("local1")).first(); + + RenameLocalVariableRefactor refactor = new RenameLocalVariableRefactor(); + refactor.setTarget(local1Var); + refactor.setNewName("local1"); + refactor.refactor(); + assertEquals("local1", local1Var.getSimpleName()); + } } diff --git a/src/test/java/spoon/test/refactoring/testclasses/VariableRename.java b/src/test/java/spoon/test/refactoring/testclasses/RenameLocalVariableRefactorTestSubject.java similarity index 68% rename from src/test/java/spoon/test/refactoring/testclasses/VariableRename.java rename to src/test/java/spoon/test/refactoring/testclasses/RenameLocalVariableRefactorTestSubject.java index 25278e13f7e..1750568ce6e 100644 --- a/src/test/java/spoon/test/refactoring/testclasses/VariableRename.java +++ b/src/test/java/spoon/test/refactoring/testclasses/RenameLocalVariableRefactorTestSubject.java @@ -8,14 +8,20 @@ import static org.junit.Assert.*; -public class VariableRename +public class RenameLocalVariableRefactorTestSubject { - public VariableRename() throws Throwable + public RenameLocalVariableRefactorTestSubject() { int local1 = 0; - //call all not private methods of this class automatically, to check assertions + } + + public void checkModelConsistency() throws Throwable { + //call all not private methods of this class automatically, to check assertions, which are there Method[] methods = getClass().getDeclaredMethods(); for (Method method : methods) { + if("checkModelConsistency".equals(method.getName())) { + continue; + } try { if(Modifier.isPrivate(method.getModifiers())) { continue; @@ -36,8 +42,8 @@ void callConflictWithParam() { /** * tests conflict of local variable with parameter */ - private void conflictWithParam(@TryRename("-var1") int var2) { - @TryRename("-var2") + private void conflictWithParam(@TestTryRename("-var1") int var2) { + @TestTryRename("-var2") int var1 = 1; assertTrue(var1 == 1); assertTrue(var2 == 2); @@ -47,15 +53,15 @@ private void conflictWithParam(@TryRename("-var1") int var2) { * tests conflict of local variable with CtCatchVariable */ private void conflictWithCatchVariable() { - @TryRename({"-var2", "-var3"}) + @TestTryRename({"-var2", "-var3"}) int var1 = 1; try { assertTrue(var1 == 1); - @TryRename({"-var1", "var3"}) + @TestTryRename({"-var1", "var3"}) int var2 = 2; assertTrue(var2 == 2); throw new NumberFormatException(); - } catch (@TryRename({"-var1", "var2"}) NumberFormatException var3) { + } catch (@TestTryRename({"-var1", "var2"}) NumberFormatException var3) { assertTrue(var1 == 1); } assertTrue(var1 == 1); @@ -66,23 +72,23 @@ private void conflictWithCatchVariable() { * and nested local variable references, which must would be shadowed */ void nestedClassMethodWithRefs() { - @TryRename({"-var2", "-var3", "-var4", "-var5", "-var6"}) + @TestTryRename({"-var2", "-var3", "-var4", "-var5", "-var6"}) int var1 = 1; new Consumer() { //must not rename to var1, because it would shadow var1 reference below - @TryRename({"-var1", "-var3", "-var4", "-var5", "-var6"}) + @TestTryRename({"-var1", "-var3", "-var4", "-var5", "-var6"}) int var2 = 2; @Override public void accept( //must not rename to var1, because reference to var1 below would be shadowed - @TryRename({"-var1", "var2", "-var3", "-var5", "-var6"}) Integer var4 + @TestTryRename({"-var1", "var2", "-var3", "-var5", "-var6"}) Integer var4 ) { //cannot rename to var1, because reference to var1 below would be shadowed - @TryRename({"-var1", "var2", "-var4", "-var5", "-var6"}) + @TestTryRename({"-var1", "var2", "-var4", "-var5", "-var6"}) int var3 = 3; try { //cannot rename to var1, because reference to var1 below would be shadowed - @TryRename({"-var1", "var2", "-var3", "-var4", "var6"}) + @TestTryRename({"-var1", "var2", "-var3", "-var4", "var6"}) int var5 = 5; assertTrue(var1 == 1); assertTrue(var2 == 2); @@ -92,7 +98,7 @@ public void accept( throw new NumberFormatException(); } catch ( //cannot rename to var1, because reference to var1 below would be shadowed - @TryRename({"-var1", "var2", "-var3", "-var4", "var5"}) NumberFormatException var6) { + @TestTryRename({"-var1", "var2", "-var3", "-var4", "var5"}) NumberFormatException var6) { assertTrue(var1 == 1); assertTrue(var2 == 2); assertTrue(var3 == 3); @@ -107,23 +113,23 @@ public void accept( * and no nested local variable references so rename is possible */ void nestedClassMethodWithoutRefs() { - @TryRename({"var2", "var3", "var4", "var5", "var6"}) + @TestTryRename({"var2", "var3", "var4", "var5", "var6"}) int var1 = 1; new Consumer() { //must not rename to var1, because it would shadow var1 reference below - @TryRename({"-var1", "var3", "var4", "var5", "var6"}) + @TestTryRename({"-var1", "var3", "var4", "var5", "var6"}) int var2 = 2; @Override public void accept( //must not rename to var1, because reference to var1 below would be shadowed - @TryRename({"-var1", "var2", "-var3", "-var5", "-var6"}) Integer var4 + @TestTryRename({"-var1", "var2", "-var3", "-var5", "-var6"}) Integer var4 ) { //can rename to var1, because reference to var1 below is not shadowed - @TryRename({"var1", "var2", "-var4", "-var5", "-var6"}) + @TestTryRename({"var1", "var2", "-var4", "-var5", "-var6"}) int var3 = 3; try { //can rename to var1, because reference to var1 below is not shadowed - @TryRename({"var1", "var2", "-var3", "-var4", "var6"}) + @TestTryRename({"var1", "var2", "-var3", "-var4", "var6"}) int var5 = 5; // assertTrue(var1 == 1);//do not reference it in scope of other vars, so it can be renamed assertTrue(var2 == 2); @@ -133,7 +139,7 @@ public void accept( throw new NumberFormatException(); } catch ( //can rename to var1, because reference to var1 below is not shadowed - @TryRename({"var1", "var2", "-var3", "-var4", "var5"}) NumberFormatException var6) { + @TestTryRename({"var1", "var2", "-var3", "-var4", "var5"}) NumberFormatException var6) { // assertTrue(var1 == 1);//do not reference it in scope of other vars, so it can be renamed assertTrue(var2 == 2); assertTrue(var3 == 3); @@ -145,18 +151,18 @@ public void accept( } void nestedClassMethodWithShadowVarWithRefs() { - @TryRename({"-var2", "var3"}) + @TestTryRename({"-var2", "var3"}) int var1 = 2; new Runnable() { - @TryRename({"var1", "var3"}) + @TestTryRename({"var1", "var3"}) int var2 = 3; @Override public void run() { assertTrue(var1 == 2); - @TryRename({"-var1", "var2"}) + @TestTryRename({"-var1", "var2"}) int var3 = 1; //this var1 shadows above defined var1. It can be renamed - @TryRename({"var2", "-var3"}) + @TestTryRename({"var2", "-var3"}) int var1 = 4; assertTrue(var1 == 4); assertTrue(var2 == 3); @@ -166,18 +172,18 @@ public void run() { assertTrue(var1 == 2); } void nestedClassMethodWithShadowVarWithoutRefs() { - @TryRename({"var2", "var3"}) + @TestTryRename({"var2", "var3"}) int var1 = 2; new Runnable() { - @TryRename({"var1", "var3"}) + @TestTryRename({"var1", "var3"}) int var2 = 3; @Override public void run() { - @TryRename({"-var1", "var2"}) + @TestTryRename({"-var1", "var2"}) // assertTrue(var1 == 2); //the var1 is not referenced so it can be renamed int var3 = 1; //this var1 shadows above defined var1. It can be renamed - @TryRename({"var2", "-var3"}) + @TestTryRename({"var2", "-var3"}) int var1 = 4; assertTrue(var1 == 4); assertTrue(var2 == 3); @@ -188,18 +194,18 @@ public void run() { } void nestedClassMethodWithShadowVarAndField() { - @TryRename({"var2", "var3"}) + @TestTryRename({"var2", "var3"}) int var1 = 2; new Runnable() { - @TryRename({"var2", "var3"}) + @TestTryRename({"var2", "var3"}) //this var1 shadows above defined var1. int var1 = 3; @Override public void run() { - @TryRename({"-var1", "var2"}) + @TestTryRename({"-var1", "var2"}) int var2 = 1; assertTrue(var1 == 3); - @TryRename({"-var2", "var3"}) + @TestTryRename({"-var2", "var3"}) int var1 = 4; assertTrue(var1 == 4); assertTrue(this.var1 == 3); @@ -210,11 +216,11 @@ public void run() { } void lambda() { - @TryRename({"-var2", "-var3"}) + @TestTryRename({"-var2", "-var3"}) int var1 = 1; assertTrue(var1 == 1); - Function fnc = (@TryRename({"-var1", "-var3"}) Integer var2)->{ - @TryRename({"-var1", "-var2"}) + Function fnc = (@TestTryRename({"-var1", "-var3"}) Integer var2)->{ + @TestTryRename({"-var1", "-var2"}) int var3 = 3; assertTrue(var1 == 1); assertTrue(var2 == 2); @@ -225,17 +231,17 @@ void lambda() { } void tryCatch() { - @TryRename({"-var2", "-var3", "-var4"}) + @TestTryRename({"-var2", "-var3", "-var4"}) int var1 = 1; assertTrue(var1 == 1); try { - @TryRename({"-var1","var3","var4"}) + @TestTryRename({"-var1","var3","var4"}) int var2 = 2; assertTrue(var1 == 1); assertTrue(var2 == 2); throw new Exception("ex2"); - } catch (@TryRename({"-var1", "var2", "-var4"}) Exception var3) { - @TryRename({"-var1", "var2", "-var3"}) + } catch (@TestTryRename({"-var1", "var2", "-var4"}) Exception var3) { + @TestTryRename({"-var1", "var2", "-var3"}) int var4 = 4; assertTrue(var1 == 1); assertTrue(var4 == 4); diff --git a/src/test/java/spoon/test/refactoring/testclasses/TryRename.java b/src/test/java/spoon/test/refactoring/testclasses/TestTryRename.java similarity index 91% rename from src/test/java/spoon/test/refactoring/testclasses/TryRename.java rename to src/test/java/spoon/test/refactoring/testclasses/TestTryRename.java index 60f550614ac..ba1bd732d6c 100644 --- a/src/test/java/spoon/test/refactoring/testclasses/TryRename.java +++ b/src/test/java/spoon/test/refactoring/testclasses/TestTryRename.java @@ -7,7 +7,7 @@ @Retention(RetentionPolicy.RUNTIME) @Target({ ElementType.LOCAL_VARIABLE, ElementType.PARAMETER, ElementType.FIELD}) -public @interface TryRename { +public @interface TestTryRename { /** * @return the list of names which should be tried by refactoring. * If the name starts with prefix "-", then this refactoring should fail on some validation issue From 89b613a8aa51b668353472ca3de224e990672277 Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Sun, 19 Mar 2017 17:51:03 +0100 Subject: [PATCH 3/6] added license header and comment to the new RefactoringException --- .../refactoring/RefactoringException.java | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/main/java/spoon/refactoring/RefactoringException.java b/src/main/java/spoon/refactoring/RefactoringException.java index 31eb333a2cb..ddeef9e58a4 100644 --- a/src/main/java/spoon/refactoring/RefactoringException.java +++ b/src/main/java/spoon/refactoring/RefactoringException.java @@ -1,7 +1,27 @@ +/** + * 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 spoon.SpoonException; +/** + * Thrown when required refactoring would cause model inconsistency + */ public class RefactoringException extends SpoonException { private static final long serialVersionUID = 1L; @@ -19,5 +39,4 @@ public RefactoringException(Throwable e) { public RefactoringException(String msg, Throwable e) { super(msg, e); } - } From d31543a85ae65507dc97a124d87eb2c9be8827c3 Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Tue, 21 Mar 2017 19:13:49 +0100 Subject: [PATCH 4/6] new interface CtRenameRefactoring, class and method renaming --- .../refactoring/AbstractRenameRefactor.java | 8 ++-- .../{Refactor.java => CtRefactoring.java} | 4 +- .../refactoring/CtRenameRefactoring.java | 42 +++++++++++++++++++ .../java/spoon/refactoring/Refactoring.java | 14 +++++++ .../RenameLocalVariableRefactorTest.java | 4 +- 5 files changed, 66 insertions(+), 6 deletions(-) rename src/main/java/spoon/refactoring/{Refactor.java => CtRefactoring.java} (84%) create mode 100644 src/main/java/spoon/refactoring/CtRenameRefactoring.java diff --git a/src/main/java/spoon/refactoring/AbstractRenameRefactor.java b/src/main/java/spoon/refactoring/AbstractRenameRefactor.java index 2510906ca6c..de3715fffbb 100644 --- a/src/main/java/spoon/refactoring/AbstractRenameRefactor.java +++ b/src/main/java/spoon/refactoring/AbstractRenameRefactor.java @@ -21,7 +21,7 @@ import spoon.SpoonException; import spoon.reflect.declaration.CtNamedElement; -public abstract class AbstractRenameRefactor implements Refactor { +public abstract class AbstractRenameRefactor implements CtRenameRefactoring { public static final Pattern javaIdentifierRE = Pattern.compile("\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*"); protected T target; @@ -70,18 +70,20 @@ public T getTarget() { return target; } - public void setTarget(T target) { + public AbstractRenameRefactor setTarget(T target) { this.target = target; + return this; } public String getNewName() { return newName; } - public void setNewName(String newName) { + public AbstractRenameRefactor setNewName(String newName) { if (newNameValidationRE != null && newNameValidationRE.matcher(newName).matches() == false) { throw new SpoonException("New name \"" + newName + "\" is not valid name"); } this.newName = newName; + return this; } } diff --git a/src/main/java/spoon/refactoring/Refactor.java b/src/main/java/spoon/refactoring/CtRefactoring.java similarity index 84% rename from src/main/java/spoon/refactoring/Refactor.java rename to src/main/java/spoon/refactoring/CtRefactoring.java index 2423510b45a..31dfdc97dc4 100644 --- a/src/main/java/spoon/refactoring/Refactor.java +++ b/src/main/java/spoon/refactoring/CtRefactoring.java @@ -18,14 +18,16 @@ /** * Defines basic contract of all refactoring implementations.
+ * Contract: to process a required refactoring.
* Usage:
* 
  * SomeRefactoring r = new SomeRefactoring();
  * //configure refactoring by calling setters on `r`
  * r.refactor();
  *
+ * See child interfaces, which implements other supported refactoring methods */ -public interface Refactor { +public interface CtRefactoring { /** * Process refactoring operation */ diff --git a/src/main/java/spoon/refactoring/CtRenameRefactoring.java b/src/main/java/spoon/refactoring/CtRenameRefactoring.java new file mode 100644 index 00000000000..e84198847a8 --- /dev/null +++ b/src/main/java/spoon/refactoring/CtRenameRefactoring.java @@ -0,0 +1,42 @@ +/** + * 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 spoon.reflect.declaration.CtNamedElement; + +/** + * The kind of refactoring, which renames a `target` element + * to the `newName`
+ * Usage:
+ * 
+ * CtVariable someVariable = ...
+ * new SomeRenameRefactoring().setTarget(someVariable).setNewName("mutchBetterName").refactor();
+ *
+ */ +public interface CtRenameRefactoring extends CtRefactoring { + /** + * @param target the model element, which has to be refactored. + * @return this to support fluent API + */ + CtRenameRefactoring setTarget(T target); + + /** + * @param newName the required name of the `target` model element + * @return this to support fluent API + */ + CtRenameRefactoring setNewName(String newName); +} diff --git a/src/main/java/spoon/refactoring/Refactoring.java b/src/main/java/spoon/refactoring/Refactoring.java index fce0c5fd249..aa003208b95 100644 --- a/src/main/java/spoon/refactoring/Refactoring.java +++ b/src/main/java/spoon/refactoring/Refactoring.java @@ -16,6 +16,7 @@ */ package spoon.refactoring; +import spoon.reflect.code.CtLocalVariable; import spoon.reflect.declaration.CtType; import spoon.reflect.reference.CtTypeReference; import spoon.reflect.visitor.Query; @@ -52,4 +53,17 @@ public boolean matches(CtTypeReference reference) { reference.setSimpleName(name); } } + + /** + * Changes name of a {@link CtLocalVariable}. + * + * @param localVariable + * to be renamed {@link CtLocalVariable} in the AST. + * @param name + * New name of the element. + * @throws RefactoringException when rename to newName would cause model inconsistency, like ambiguity, shadowing of other variables, etc. + */ + public static void changeLocalVariableName(CtLocalVariable localVariable, String name) throws RefactoringException { + new RenameLocalVariableRefactor().setTarget(localVariable).setNewName(name).refactor(); + } } diff --git a/src/test/java/spoon/test/refactoring/RenameLocalVariableRefactorTest.java b/src/test/java/spoon/test/refactoring/RenameLocalVariableRefactorTest.java index f336d7d73e7..9644fc37963 100644 --- a/src/test/java/spoon/test/refactoring/RenameLocalVariableRefactorTest.java +++ b/src/test/java/spoon/test/refactoring/RenameLocalVariableRefactorTest.java @@ -99,7 +99,7 @@ protected void checkLocalVariableRename(CtLocalVariable targetVariable, Strin throw new AssertionError(getParentMethodName(targetVariable)+" Rename of \""+originName+"\" should NOT fail when trying rename to \""+newName+"\"\n"+targetVariable.toString(), e); } assertEquals(getParentMethodName(targetVariable)+" Rename of \""+originName+"\" to \""+newName+"\" passed, but the name of variable was not changed", newName, targetVariable.getSimpleName()); - printModelAndTestConsistency(getParentMethodName(targetVariable)+" Rename of \""+originName+"\" to \""+newName+"\""); + assertCorrectModel(getParentMethodName(targetVariable)+" Rename of \""+originName+"\" to \""+newName+"\""); } else { try { refactor.refactor(); @@ -127,7 +127,7 @@ private void rollback(CtLocalVariable targetVariable, String originName) { } } - private void printModelAndTestConsistency(String refactoringDescription) { + private void assertCorrectModel(String refactoringDescription) { Launcher launcher = new Launcher(); File outputBinDirectory = new File("./target/spooned-refactoring-test"); if (!outputBinDirectory.exists()) { From 9888ff44671942fc4f76c57478d9c9e2c3596741 Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Tue, 21 Mar 2017 20:19:52 +0100 Subject: [PATCH 5/6] improve javadoc --- .../refactoring/AbstractRenameRefactor.java | 22 ++++++++++++++++--- .../refactoring/CtRenameRefactoring.java | 8 +++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/main/java/spoon/refactoring/AbstractRenameRefactor.java b/src/main/java/spoon/refactoring/AbstractRenameRefactor.java index de3715fffbb..c916f4e2348 100644 --- a/src/main/java/spoon/refactoring/AbstractRenameRefactor.java +++ b/src/main/java/spoon/refactoring/AbstractRenameRefactor.java @@ -21,6 +21,11 @@ import spoon.SpoonException; import spoon.reflect.declaration.CtNamedElement; +/** + * abstract implementation of rename element refactoring + * + * @param the type of target renamed element + */ public abstract class AbstractRenameRefactor implements CtRenameRefactoring { public static final Pattern javaIdentifierRE = Pattern.compile("\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*"); @@ -52,33 +57,44 @@ protected void detectIssues() { } /** - * checks whether {@link #newName} is valid java identifier - * @param issues + * client may implement this method to check whether {@link #newName} is valid */ protected void checkNewNameIsValid() { } + /** + * client may implement this method to check whether {@link #newName} + * is in conflict with names of other model elements + */ protected void detectNameConflicts() { } - + /** + * Helper method, which can be used by the child classes to check if name is an java identifier + * @param name the to be checked name + * @return true if name is valid java identifier + */ protected boolean isJavaIdentifier(String name) { return javaIdentifierRE.matcher(name).matches(); } + @Override public T getTarget() { return target; } + @Override public AbstractRenameRefactor setTarget(T target) { this.target = target; return this; } + @Override public String getNewName() { return newName; } + @Override public AbstractRenameRefactor setNewName(String newName) { if (newNameValidationRE != null && newNameValidationRE.matcher(newName).matches() == false) { throw new SpoonException("New name \"" + newName + "\" is not valid name"); diff --git a/src/main/java/spoon/refactoring/CtRenameRefactoring.java b/src/main/java/spoon/refactoring/CtRenameRefactoring.java index e84198847a8..e68ebdbc291 100644 --- a/src/main/java/spoon/refactoring/CtRenameRefactoring.java +++ b/src/main/java/spoon/refactoring/CtRenameRefactoring.java @@ -28,12 +28,20 @@ * */ public interface CtRenameRefactoring extends CtRefactoring { + /** + * @return target model element, which has to be refactored. + */ + T getTarget(); /** * @param target the model element, which has to be refactored. * @return this to support fluent API */ CtRenameRefactoring setTarget(T target); + /** + * @return the required name of the `target` model element + */ + String getNewName(); /** * @param newName the required name of the `target` model element * @return this to support fluent API From 32587f36bdced84ad6e6b0b9b5700c6fe0f7670b Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Wed, 22 Mar 2017 18:28:05 +0100 Subject: [PATCH 6/6] rename RenameLocalVariableRefactor*->CtRenameLocalVariableRefactoring* --- ...or.java => AbstractRenameRefactoring.java} | 8 ++--- ... => CtRenameLocalVariableRefactoring.java} | 4 +-- .../java/spoon/refactoring/Refactoring.java | 6 ++-- ...CtRenameLocalVariableRefactoringTest.java} | 32 +++++++++---------- ...eLocalVariableRefactoringTestSubject.java} | 4 +-- 5 files changed, 27 insertions(+), 27 deletions(-) rename src/main/java/spoon/refactoring/{AbstractRenameRefactor.java => AbstractRenameRefactoring.java} (87%) rename src/main/java/spoon/refactoring/{RenameLocalVariableRefactor.java => CtRenameLocalVariableRefactoring.java} (96%) rename src/test/java/spoon/test/refactoring/{RenameLocalVariableRefactorTest.java => CtRenameLocalVariableRefactoringTest.java} (85%) rename src/test/java/spoon/test/refactoring/testclasses/{RenameLocalVariableRefactorTestSubject.java => CtRenameLocalVariableRefactoringTestSubject.java} (95%) diff --git a/src/main/java/spoon/refactoring/AbstractRenameRefactor.java b/src/main/java/spoon/refactoring/AbstractRenameRefactoring.java similarity index 87% rename from src/main/java/spoon/refactoring/AbstractRenameRefactor.java rename to src/main/java/spoon/refactoring/AbstractRenameRefactoring.java index c916f4e2348..d0231265174 100644 --- a/src/main/java/spoon/refactoring/AbstractRenameRefactor.java +++ b/src/main/java/spoon/refactoring/AbstractRenameRefactoring.java @@ -26,14 +26,14 @@ * * @param the type of target renamed element */ -public abstract class AbstractRenameRefactor implements CtRenameRefactoring { +public abstract class AbstractRenameRefactoring implements CtRenameRefactoring { public static final Pattern javaIdentifierRE = Pattern.compile("\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*"); protected T target; protected String newName; protected Pattern newNameValidationRE; - protected AbstractRenameRefactor(Pattern newNameValidationRE) { + protected AbstractRenameRefactoring(Pattern newNameValidationRE) { this.newNameValidationRE = newNameValidationRE; } @@ -84,7 +84,7 @@ public T getTarget() { } @Override - public AbstractRenameRefactor setTarget(T target) { + public AbstractRenameRefactoring setTarget(T target) { this.target = target; return this; } @@ -95,7 +95,7 @@ public String getNewName() { } @Override - public AbstractRenameRefactor setNewName(String newName) { + public AbstractRenameRefactoring setNewName(String newName) { if (newNameValidationRE != null && newNameValidationRE.matcher(newName).matches() == false) { throw new SpoonException("New name \"" + newName + "\" is not valid name"); } diff --git a/src/main/java/spoon/refactoring/RenameLocalVariableRefactor.java b/src/main/java/spoon/refactoring/CtRenameLocalVariableRefactoring.java similarity index 96% rename from src/main/java/spoon/refactoring/RenameLocalVariableRefactor.java rename to src/main/java/spoon/refactoring/CtRenameLocalVariableRefactoring.java index 6926a009efd..2e211ebcfe1 100644 --- a/src/main/java/spoon/refactoring/RenameLocalVariableRefactor.java +++ b/src/main/java/spoon/refactoring/CtRenameLocalVariableRefactoring.java @@ -59,11 +59,11 @@ * } * */ -public class RenameLocalVariableRefactor extends AbstractRenameRefactor> { +public class CtRenameLocalVariableRefactoring extends AbstractRenameRefactoring> { public static Pattern validVariableNameRE = javaIdentifierRE; - public RenameLocalVariableRefactor() { + public CtRenameLocalVariableRefactoring() { super(validVariableNameRE); } diff --git a/src/main/java/spoon/refactoring/Refactoring.java b/src/main/java/spoon/refactoring/Refactoring.java index aa003208b95..bc000fdab47 100644 --- a/src/main/java/spoon/refactoring/Refactoring.java +++ b/src/main/java/spoon/refactoring/Refactoring.java @@ -59,11 +59,11 @@ public boolean matches(CtTypeReference reference) { * * @param localVariable * to be renamed {@link CtLocalVariable} in the AST. - * @param name + * @param newName * New name of the element. * @throws RefactoringException when rename to newName would cause model inconsistency, like ambiguity, shadowing of other variables, etc. */ - public static void changeLocalVariableName(CtLocalVariable localVariable, String name) throws RefactoringException { - new RenameLocalVariableRefactor().setTarget(localVariable).setNewName(name).refactor(); + public static void changeLocalVariableName(CtLocalVariable localVariable, String newName) throws RefactoringException { + new CtRenameLocalVariableRefactoring().setTarget(localVariable).setNewName(newName).refactor(); } } diff --git a/src/test/java/spoon/test/refactoring/RenameLocalVariableRefactorTest.java b/src/test/java/spoon/test/refactoring/CtRenameLocalVariableRefactoringTest.java similarity index 85% rename from src/test/java/spoon/test/refactoring/RenameLocalVariableRefactorTest.java rename to src/test/java/spoon/test/refactoring/CtRenameLocalVariableRefactoringTest.java index 9644fc37963..a9a150f2173 100644 --- a/src/test/java/spoon/test/refactoring/RenameLocalVariableRefactorTest.java +++ b/src/test/java/spoon/test/refactoring/CtRenameLocalVariableRefactoringTest.java @@ -14,7 +14,7 @@ import spoon.OutputType; import spoon.SpoonException; import spoon.SpoonModelBuilder; -import spoon.refactoring.RenameLocalVariableRefactor; +import spoon.refactoring.CtRenameLocalVariableRefactoring; import spoon.reflect.code.CtLocalVariable; import spoon.reflect.declaration.CtAnnotation; import spoon.reflect.declaration.CtClass; @@ -24,21 +24,21 @@ import spoon.reflect.declaration.CtVariable; import spoon.reflect.reference.CtTypeReference; import spoon.test.refactoring.testclasses.TestTryRename; -import spoon.test.refactoring.testclasses.RenameLocalVariableRefactorTestSubject; +import spoon.test.refactoring.testclasses.CtRenameLocalVariableRefactoringTestSubject; import spoon.testing.utils.ModelUtils; -public class RenameLocalVariableRefactorTest +public class CtRenameLocalVariableRefactoringTest { @Test public void testModelConsistency() throws Throwable { //contract: check that all assertions in all methods of the RenameLocalVariableRefactorTestSubject are correct - new RenameLocalVariableRefactorTestSubject().checkModelConsistency(); + new CtRenameLocalVariableRefactoringTestSubject().checkModelConsistency(); } /** - * If you need to debug behavior of refactoring on the exact method and variable in the {@link RenameLocalVariableRefactorTestSubject} model, + * If you need to debug behavior of refactoring on the exact method and variable in the {@link CtRenameLocalVariableRefactoringTestSubject} model, * then provide - * 1) name of method of {@link RenameLocalVariableRefactorTestSubject} + * 1) name of method of {@link CtRenameLocalVariableRefactoringTestSubject} * 2) original name variable in the method * 3) new name of variable in the method * then put breakpoint on the line `this.getClass();` below and the debugger stops just before @@ -47,7 +47,7 @@ public void testModelConsistency() throws Throwable { private String[] DEBUG = new String[]{/*"nestedClassMethodWithoutRefs", "var3", "var1"*/}; /** - * The {@link RenameLocalVariableRefactorTestSubject} class is loaded as spoon model. Then: + * The {@link CtRenameLocalVariableRefactoringTestSubject} class is loaded as spoon model. Then: * - It looks for each CtVariable and it's CtAnnotation and tries to rename that variable to the name defined by annotation. * - If the annotation name is prefixed with "-", then that refactoring should fail. * - If the annotation name is not prefixed, then that refactoring should pass. @@ -55,7 +55,7 @@ public void testModelConsistency() throws Throwable { */ @Test public void testRenameAllLocalVariablesOfRenameTestSubject() throws Exception { - CtClass varRenameClass = (CtClass)ModelUtils.buildClass(RenameLocalVariableRefactorTestSubject.class); + CtClass varRenameClass = (CtClass)ModelUtils.buildClass(CtRenameLocalVariableRefactoringTestSubject.class); CtTypeReference tryRename = varRenameClass.getFactory().createCtTypeReference(TestTryRename.class); varRenameClass.getMethods().forEach(method->{ @@ -89,7 +89,7 @@ public void testRenameAllLocalVariablesOfRenameTestSubject() throws Exception { protected void checkLocalVariableRename(CtLocalVariable targetVariable, String newName, boolean renameShouldPass) { String originName = targetVariable.getSimpleName(); - RenameLocalVariableRefactor refactor = new RenameLocalVariableRefactor(); + CtRenameLocalVariableRefactoring refactor = new CtRenameLocalVariableRefactoring(); refactor.setTarget(targetVariable); refactor.setNewName(newName); if(renameShouldPass) { @@ -116,7 +116,7 @@ protected void checkLocalVariableRename(CtLocalVariable targetVariable, Strin private void rollback(CtLocalVariable targetVariable, String originName) { String newName = targetVariable.getSimpleName(); - RenameLocalVariableRefactor refactor = new RenameLocalVariableRefactor(); + CtRenameLocalVariableRefactoring refactor = new CtRenameLocalVariableRefactoring(); refactor.setTarget(targetVariable); //rollback changes refactor.setNewName(originName); @@ -154,7 +154,7 @@ private void assertCorrectModel(String refactoringDescription) { try { // varRenameClass.newInstance(); TestClassloader classLoader = new TestClassloader(launcher); - Class testModelClass = classLoader.loadClass(RenameLocalVariableRefactorTestSubject.class.getName()); + Class testModelClass = classLoader.loadClass(CtRenameLocalVariableRefactoringTestSubject.class.getName()); testModelClass.getMethod("checkModelConsistency").invoke(testModelClass.newInstance()); } catch (InvocationTargetException e) { throw new AssertionError("The model validation of code in "+launcher.getEnvironment().getBinaryOutputDirectory()+" failed after: "+refactoringDescription, e.getTargetException()); @@ -165,7 +165,7 @@ private void assertCorrectModel(String refactoringDescription) { private class TestClassloader extends URLClassLoader { TestClassloader(Launcher launcher) throws MalformedURLException { - super(new URL[] { new File(launcher.getEnvironment().getBinaryOutputDirectory()).toURL()}, RenameLocalVariableRefactorTest.class.getClassLoader()); + super(new URL[] { new File(launcher.getEnvironment().getBinaryOutputDirectory()).toURL()}, CtRenameLocalVariableRefactoringTest.class.getClassLoader()); } @Override @@ -195,11 +195,11 @@ private String getParentMethodName(CtElement ele) { @Test public void testRefactorWrongUsage() throws Exception { - CtType varRenameClass = ModelUtils.buildClass(RenameLocalVariableRefactorTestSubject.class); + CtType varRenameClass = ModelUtils.buildClass(CtRenameLocalVariableRefactoringTestSubject.class); CtLocalVariable local1Var = varRenameClass.filterChildren((CtLocalVariable var)->var.getSimpleName().equals("local1")).first(); //contract: a target variable is not defined. Throw SpoonException - RenameLocalVariableRefactor refactor = new RenameLocalVariableRefactor(); + CtRenameLocalVariableRefactoring refactor = new CtRenameLocalVariableRefactoring(); refactor.setNewName("local1"); try { refactor.refactor(); @@ -243,10 +243,10 @@ public void testRefactorWrongUsage() throws Exception { @Test public void testRenameLocalVariableToSameName() throws Exception { - CtType varRenameClass = ModelUtils.buildClass(RenameLocalVariableRefactorTestSubject.class); + CtType varRenameClass = ModelUtils.buildClass(CtRenameLocalVariableRefactoringTestSubject.class); CtLocalVariable local1Var = varRenameClass.filterChildren((CtLocalVariable var)->var.getSimpleName().equals("local1")).first(); - RenameLocalVariableRefactor refactor = new RenameLocalVariableRefactor(); + CtRenameLocalVariableRefactoring refactor = new CtRenameLocalVariableRefactoring(); refactor.setTarget(local1Var); refactor.setNewName("local1"); refactor.refactor(); diff --git a/src/test/java/spoon/test/refactoring/testclasses/RenameLocalVariableRefactorTestSubject.java b/src/test/java/spoon/test/refactoring/testclasses/CtRenameLocalVariableRefactoringTestSubject.java similarity index 95% rename from src/test/java/spoon/test/refactoring/testclasses/RenameLocalVariableRefactorTestSubject.java rename to src/test/java/spoon/test/refactoring/testclasses/CtRenameLocalVariableRefactoringTestSubject.java index 1750568ce6e..de67ecc36f2 100644 --- a/src/test/java/spoon/test/refactoring/testclasses/RenameLocalVariableRefactorTestSubject.java +++ b/src/test/java/spoon/test/refactoring/testclasses/CtRenameLocalVariableRefactoringTestSubject.java @@ -8,9 +8,9 @@ import static org.junit.Assert.*; -public class RenameLocalVariableRefactorTestSubject +public class CtRenameLocalVariableRefactoringTestSubject { - public RenameLocalVariableRefactorTestSubject() + public CtRenameLocalVariableRefactoringTestSubject() { int local1 = 0; }