Skip to content

Commit

Permalink
-The update of access modifiers is propagated
Browse files Browse the repository at this point in the history 
to the subclass methods overriding the superclass methods
whose access modifier has been updated.

-Adding import statement to target class
for nested type members in the source class
that are instantiated within the method to be moved.

-Extended Move Method refactoring implementation
to update the access modifier of methods declared in the superclass
of the source class containing the method to be moved.

-Fixed the computation of thrown exception types for individual method invocations.

This is a fix for case found in project JFreeChart-1.0.10
org.jfree.chart.renderer.category.LineAndShapeRenderer.drawItem(Graphics2D, CategoryItemRendererState, Rectangle2D, CategoryPlot, CategoryAxis, ValueAxis, CategoryDataset, int, int, int)  lines 924-958
org.jfree.chart.renderer.category.StatisticalLineAndShapeRenderer.drawItem(Graphics2D, CategoryItemRendererState, Rectangle2D, CategoryPlot, CategoryAxis, ValueAxis, CategoryDataset, int, int, int)  lines 230-266

Method getItemMargin() is not supposed to throw any exceptions.

-Added new precondition for Move Method Refactoring
The target class should not contain a method with the same signature as the moved method.
  • Loading branch information
@tsantalis
tsantalis committed Sep 8, 2016
1 parent e1c90f3 commit adfdc82
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 55 deletions.
5 changes: 4 additions & 1 deletion src/gr/uom/java/ast/decomposition/cfg/MethodCallAnalyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public void processArgumentsOfInternalMethodInvocation(ClassObject classObject,
else {
Set<AbstractVariable> usedVariablesBefore = new LinkedHashSet<AbstractVariable>(this.usedVariables);
Set<AbstractVariable> definedVariablesBefore = new LinkedHashSet<AbstractVariable>(this.definedVariables);
Set<String> thrownExceptionTypesBefore = new LinkedHashSet<String>(this.thrownExceptionTypes);
processInternalMethodInvocation(classObject, methodObject, variable, new LinkedHashSet<String>());
//save in cache
Set<AbstractVariable> usedVariablesAfter = new LinkedHashSet<AbstractVariable>(this.usedVariables);
Expand Down Expand Up @@ -148,7 +149,9 @@ else if(definedField instanceof CompositeVariable) {
if(definedFieldCount == 0) {
cache.setEmptyDefinedFieldsForMethodExpression(methodObject);
}
cache.setThrownExceptionTypesForMethodExpression(methodObject, new LinkedHashSet<String>(this.thrownExceptionTypes));
LinkedHashSet<String> thrownExceptionTypesAfter = new LinkedHashSet<String>(this.thrownExceptionTypes);
thrownExceptionTypesAfter.removeAll(thrownExceptionTypesBefore);
cache.setThrownExceptionTypesForMethodExpression(methodObject, thrownExceptionTypesAfter);
}
int argumentPosition = 0;
for(Expression argument : arguments) {
Expand Down
14 changes: 13 additions & 1 deletion src/gr/uom/java/distance/MoveMethodCandidateRefactoring.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void apply() {
public boolean isApplicable() {
if(!isSynchronized() && !containsSuperMethodInvocation() && !overridesMethod() && !containsFieldAssignment() && !isTargetClassAnInterface() &&
validTargetObject() && !oneToManyRelationshipWithTargetClass() && !containsAssignmentToTargetClassVariable() &&
!containsMethodCallWithThisExpressionAsArgument() && !isTargetClassAnEnum() && !isSourceClassATestClass())
!containsMethodCallWithThisExpressionAsArgument() && !isTargetClassAnEnum() && !isSourceClassATestClass() && !targetClassContainsMethodWithSourceMethodSignature())
return true;
else
return false;
Expand All @@ -108,6 +108,18 @@ public boolean leaveDelegate() {
system.getSystemObject().containsSuperMethodInvocation(getSourceMethod().getMethodObject().generateSuperMethodInvocation());
}

private boolean targetClassContainsMethodWithSourceMethodSignature() {
MethodObject sourceMethod = this.sourceMethod.getMethodObject();
for(MethodObject targetMethod : targetClass.getClassObject().getMethodList()) {
if(targetMethod.getName().equals(sourceMethod.getName()) &&
targetMethod.getReturnType().equals(sourceMethod.getReturnType()) &&
targetMethod.getParameterTypeList().equals(sourceMethod.getParameterTypeList())) {
return true;
}
}
return false;
}

private boolean isSourceClassATestClass() {
return sourceClass.getClassObject().containsMethodWithTestAnnotation() || sourceClass.getClassObject().extendsTestCase();
}
Expand Down
44 changes: 11 additions & 33 deletions src/gr/uom/java/jdeodorant/refactoring/manipulators/ExtractCloneRefactoring.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ else if(package1 == null && package2 == null) {
compilationUnit = sourceCompilationUnits.get(0);
}
else if(commonSuperType != null) {
compilationUnit = findCompilationUnit(commonSuperType.getAbstractTypeDeclaration());
compilationUnit = RefactoringUtility.findCompilationUnit(commonSuperType.getAbstractTypeDeclaration());
}
else {
compilationUnit = sourceCompilationUnits.get(0);
Expand Down Expand Up @@ -906,8 +906,8 @@ else if(typeName.equals("Class")) {
Set<ITypeBinding> typeBindings = new LinkedHashSet<ITypeBinding>();
boolean clones = type2Clones(methodDeclaration1, methodDeclaration2);
Type returnType = methodDeclaration1.getReturnType2();
TypeDeclaration typeDeclaration1 = findTypeDeclaration(methodDeclaration1);
TypeDeclaration typeDeclaration2 = findTypeDeclaration(methodDeclaration2);
TypeDeclaration typeDeclaration1 = RefactoringUtility.findTypeDeclaration(methodDeclaration1);
TypeDeclaration typeDeclaration2 = RefactoringUtility.findTypeDeclaration(methodDeclaration2);
Set<VariableDeclaration> fieldsAccessedInMethod1 = getFieldsAccessedInMethod(indirectlyAccessedLocalFieldsG1, methodDeclaration1);
Set<VariableDeclaration> fieldsAccessedInMethod2 = getFieldsAccessedInMethod(indirectlyAccessedLocalFieldsG2, methodDeclaration2);
Set<VariableDeclaration> fieldsModifiedInMethod1 = getFieldsAccessedInMethod(indirectlyModifiedLocalFieldsG1, methodDeclaration1);
Expand Down Expand Up @@ -1596,7 +1596,7 @@ else if(bodyDeclaration instanceof FieldDeclaration) {
}
AST ast = bodyDeclaration.getAST();
ASTRewrite rewriter = ASTRewrite.create(ast);
CompilationUnit compilationUnit = findCompilationUnit(bodyDeclaration);
CompilationUnit compilationUnit = RefactoringUtility.findCompilationUnit(bodyDeclaration);
ListRewrite modifiersRewrite = rewriter.getListRewrite(bodyDeclaration, modifiersChildListPropertyDescriptor);
List<IExtendedModifier> originalModifiers = bodyDeclaration.modifiers();
boolean accessModifierFound = false;
Expand Down Expand Up @@ -1685,8 +1685,8 @@ private boolean type2Clones(MethodDeclaration methodDeclaration1, MethodDeclarat
StatementCollector collector2 = new StatementCollector();
methodDeclaration2.getBody().accept(collector2);
List<ASTNode> statements2 = collector2.getStatementList();
ITypeRoot typeRoot1 = findCompilationUnit(methodDeclaration1).getTypeRoot();
ITypeRoot typeRoot2 = findCompilationUnit(methodDeclaration2).getTypeRoot();
ITypeRoot typeRoot1 = RefactoringUtility.findCompilationUnit(methodDeclaration1).getTypeRoot();
ITypeRoot typeRoot2 = RefactoringUtility.findCompilationUnit(methodDeclaration2).getTypeRoot();
if(statements1.size() != statements2.size()) {
return false;
}
Expand Down Expand Up @@ -4054,18 +4054,18 @@ private void modifySourceClass(CompilationUnit compilationUnit, TypeDeclaration
}
for(MethodDeclaration methodDeclaration : methodDeclarationsToBePulledUp) {
if(methodDeclaration.getRoot().equals(compilationUnit)) {
removeMethodDeclaration(methodDeclaration, findTypeDeclaration(methodDeclaration), findCompilationUnit(methodDeclaration));
removeMethodDeclaration(methodDeclaration, RefactoringUtility.findTypeDeclaration(methodDeclaration), RefactoringUtility.findCompilationUnit(methodDeclaration));
}
}
removeFieldDeclarations(fieldDeclarationsToBePulledUp);
for(VariableDeclaration variableDeclaration : accessedFieldDeclarations) {
if(variableDeclaration.getRoot().equals(compilationUnit)) {
createGetterMethodDeclaration(variableDeclaration, findTypeDeclaration(variableDeclaration), findCompilationUnit(variableDeclaration));
createGetterMethodDeclaration(variableDeclaration, RefactoringUtility.findTypeDeclaration(variableDeclaration), RefactoringUtility.findCompilationUnit(variableDeclaration));
}
}
for(VariableDeclaration variableDeclaration : assignedFieldDeclarations) {
if(variableDeclaration.getRoot().equals(compilationUnit)) {
createSetterMethodDeclaration(variableDeclaration, findTypeDeclaration(variableDeclaration), findCompilationUnit(variableDeclaration));
createSetterMethodDeclaration(variableDeclaration, RefactoringUtility.findTypeDeclaration(variableDeclaration), RefactoringUtility.findCompilationUnit(variableDeclaration));
}
}
}
Expand Down Expand Up @@ -4270,8 +4270,8 @@ private void removeFieldDeclarations(Set<VariableDeclaration> variableDeclaratio
orderedFields.addAll(variableDeclarations);
List<TextEdit> textEdits = new ArrayList<TextEdit>();
for(VariableDeclaration variableDeclaration : orderedFields) {
TypeDeclaration typeDeclaration = findTypeDeclaration(variableDeclaration);
CompilationUnit compilationUnit = findCompilationUnit(variableDeclaration);
TypeDeclaration typeDeclaration = RefactoringUtility.findTypeDeclaration(variableDeclaration);
CompilationUnit compilationUnit = RefactoringUtility.findCompilationUnit(variableDeclaration);
if(variableDeclaration.getRoot().equals(compilationUnit)) {
boolean found = false;
AST ast = typeDeclaration.getAST();
Expand Down Expand Up @@ -4328,28 +4328,6 @@ private void removeFieldDeclarations(Set<VariableDeclaration> variableDeclaratio
}
}

private TypeDeclaration findTypeDeclaration(ASTNode node) {
ASTNode parent = node.getParent();
while(parent != null) {
if(parent instanceof TypeDeclaration) {
return (TypeDeclaration)parent;
}
parent = parent.getParent();
}
return null;
}

private CompilationUnit findCompilationUnit(ASTNode node) {
ASTNode parent = node.getParent();
while(parent != null) {
if(parent instanceof CompilationUnit) {
return (CompilationUnit)parent;
}
parent = parent.getParent();
}
return null;
}

private void removeMethodDeclaration(MethodDeclaration methodDeclaration, TypeDeclaration typeDeclaration, CompilationUnit compilationUnit) {
AST ast = typeDeclaration.getAST();
ASTRewrite rewriter = ASTRewrite.create(ast);
Expand Down
84 changes: 64 additions & 20 deletions src/gr/uom/java/jdeodorant/refactoring/manipulators/MoveMethodRefactoring.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package gr.uom.java.jdeodorant.refactoring.manipulators;

import gr.uom.java.ast.ASTReader;
import gr.uom.java.ast.ClassObject;
import gr.uom.java.ast.CompilationUnitCache;
import gr.uom.java.ast.decomposition.cfg.MethodCallAnalyzer;
import gr.uom.java.ast.util.ExpressionExtractor;
import gr.uom.java.ast.util.MethodDeclarationUtility;
import gr.uom.java.ast.util.StatementExtractor;
Expand All @@ -18,9 +22,11 @@
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.OperationCanceledException;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IType;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.core.dom.AST;
import org.eclipse.jdt.core.dom.ASTNode;
import org.eclipse.jdt.core.dom.AbstractTypeDeclaration;
import org.eclipse.jdt.core.dom.Assignment;
import org.eclipse.jdt.core.dom.Block;
import org.eclipse.jdt.core.dom.BodyDeclaration;
Expand Down Expand Up @@ -174,7 +180,7 @@ private void addRequiredTargetImportDeclarations() {
ImportRewrite targetImportRewrite = ImportRewrite.create(targetCompilationUnit, true);

for(ITypeBinding typeBinding : typeVisitor.getTypeBindings()) {
if(!typeBinding.isNested())
if(!typeBinding.isNested() || (typeBinding.isNested() && sourceTypeDeclaration.resolveBinding().isEqualTo(typeBinding.getDeclaringClass())))
targetImportRewrite.addImport(typeBinding);
}
for(ITypeBinding typeBinding : additionalTypeBindingsToBeImportedInTargetClass) {
Expand Down Expand Up @@ -1078,8 +1084,12 @@ else if(!expressionName.getIdentifier().equals(targetClassVariableName) && !addi
AST ast = newMethodDeclaration.getAST();
targetRewriter.set(newMethodInvocation, MethodInvocation.EXPRESSION_PROPERTY, ast.newSimpleName(sourceTypeDeclaration.getName().getIdentifier()), null);
if(!sourceMethodsWithPublicModifier.contains(methodInvocation.resolveMethodBinding().getKey())) {
setPublicModifierToSourceMethod(methodInvocation);
setPublicModifierToSourceMethod(methodInvocation.resolveMethodBinding(), sourceTypeDeclaration);
sourceMethodsWithPublicModifier.add(methodInvocation.resolveMethodBinding().getKey());
Map<IMethodBinding, TypeDeclaration> subclassTypeDeclarationMap = findSubclassesOverridingMethod(sourceTypeDeclaration, methodInvocation.resolveMethodBinding());
for(IMethodBinding methodBindingKey : subclassTypeDeclarationMap.keySet()) {
setPublicModifierToSourceMethod(methodBindingKey, subclassTypeDeclarationMap.get(methodBindingKey));
}
}
}
else if(fieldName != null) {
Expand All @@ -1094,8 +1104,12 @@ else if(fieldName != null) {
}
targetRewriter.set(newMethodInvocation, MethodInvocation.EXPRESSION_PROPERTY, parameterName, null);
if(!sourceMethodsWithPublicModifier.contains(methodInvocation.resolveMethodBinding().getKey())) {
setPublicModifierToSourceMethod(methodInvocation);
setPublicModifierToSourceMethod(methodInvocation.resolveMethodBinding(), sourceTypeDeclaration);
sourceMethodsWithPublicModifier.add(methodInvocation.resolveMethodBinding().getKey());
Map<IMethodBinding, TypeDeclaration> subclassTypeDeclarationMap = findSubclassesOverridingMethod(sourceTypeDeclaration, methodInvocation.resolveMethodBinding());
for(IMethodBinding methodBindingKey : subclassTypeDeclarationMap.keySet()) {
setPublicModifierToSourceMethod(methodBindingKey, subclassTypeDeclarationMap.get(methodBindingKey));
}
}
}
}
Expand Down Expand Up @@ -1124,6 +1138,13 @@ else if(fieldName != null) {
parameterName = addSourceClassParameterToMovedMethod(newMethodDeclaration, targetRewriter);
}
targetRewriter.set(newMethodInvocation, MethodInvocation.EXPRESSION_PROPERTY, parameterName, null);
if(!sourceMethodsWithPublicModifier.contains(methodBinding.getKey())) {
TypeDeclaration superclassTypeDeclaration = RefactoringUtility.findDeclaringTypeDeclaration(superclassMethodBinding, sourceTypeDeclaration);
if(superclassTypeDeclaration != null) {
setPublicModifierToSourceMethod(methodInvocation.resolveMethodBinding(), superclassTypeDeclaration);
}
sourceMethodsWithPublicModifier.add(methodBinding.getKey());
}
}
}
}
Expand Down Expand Up @@ -1153,6 +1174,28 @@ private SimpleName addSourceClassParameterToMovedMethod(MethodDeclaration newMet
return parameterName;
}

private Map<IMethodBinding, TypeDeclaration> findSubclassesOverridingMethod(TypeDeclaration typeDeclaration, IMethodBinding methodBinding) {
Map<IMethodBinding, TypeDeclaration> subclassTypeDeclarationMap = new LinkedHashMap<IMethodBinding, TypeDeclaration>();
CompilationUnitCache cache = CompilationUnitCache.getInstance();
Set<IType> subTypes = cache.getSubTypes((IType)typeDeclaration.resolveBinding().getJavaElement());
for(IType iType : subTypes) {
String fullyQualifiedTypeName = iType.getFullyQualifiedName();
ClassObject classObject = ASTReader.getSystemObject().getClassObject(fullyQualifiedTypeName);
if(classObject != null) {
AbstractTypeDeclaration abstractTypeDeclaration = classObject.getAbstractTypeDeclaration();
if(abstractTypeDeclaration instanceof TypeDeclaration) {
TypeDeclaration subclassTypeDeclaration = (TypeDeclaration)abstractTypeDeclaration;
for(MethodDeclaration subclassMethodDeclaration : subclassTypeDeclaration.getMethods()) {
if(MethodCallAnalyzer.equalSignature(subclassMethodDeclaration.resolveBinding(), methodBinding)) {
subclassTypeDeclarationMap.put(subclassMethodDeclaration.resolveBinding(), subclassTypeDeclaration);
}
}
}
}
}
return subclassTypeDeclarationMap;
}

private void setPublicModifierToSourceTypeDeclaration() {
PackageDeclaration sourcePackageDeclaration = sourceCompilationUnit.getPackage();
PackageDeclaration targetPackageDeclaration = targetCompilationUnit.getPackage();
Expand Down Expand Up @@ -1238,10 +1281,11 @@ private void addParameterToMovedMethod(MethodDeclaration newMethodDeclaration, I
addParamTagElementToJavadoc(newMethodDeclaration, targetRewriter, variableBinding.getName());
}

private void setPublicModifierToSourceMethod(MethodInvocation methodInvocation) {
private void setPublicModifierToSourceMethod(IMethodBinding methodBinding, TypeDeclaration sourceTypeDeclaration) {
MethodDeclaration[] methodDeclarations = sourceTypeDeclaration.getMethods();
for(MethodDeclaration methodDeclaration : methodDeclarations) {
if(methodDeclaration.resolveBinding().isEqualTo(methodInvocation.resolveMethodBinding())) {
if(methodDeclaration.resolveBinding().isEqualTo(methodBinding)) {
CompilationUnit sourceCompilationUnit = RefactoringUtility.findCompilationUnit(methodDeclaration);
ASTRewrite sourceRewriter = ASTRewrite.create(sourceCompilationUnit.getAST());
ListRewrite modifierRewrite = sourceRewriter.getListRewrite(methodDeclaration, MethodDeclaration.MODIFIERS2_PROPERTY);
Modifier publicModifier = methodDeclaration.getAST().newModifier(Modifier.ModifierKeyword.PUBLIC_KEYWORD);
Expand All @@ -1257,27 +1301,27 @@ else if(modifier.getKeyword().equals(Modifier.ModifierKeyword.PRIVATE_KEYWORD) |
modifier.getKeyword().equals(Modifier.ModifierKeyword.PROTECTED_KEYWORD)) {
modifierFound = true;
modifierRewrite.replace(modifier, publicModifier, null);
try {
TextEdit sourceEdit = sourceRewriter.rewriteAST();
sourceMultiTextEdit.addChild(sourceEdit);
sourceCompilationUnitChange.addTextEditGroup(new TextEditGroup("Change access level to public", new TextEdit[] {sourceEdit}));
}
catch(JavaModelException javaModelException) {
javaModelException.printStackTrace();
}
}
}
}
if(!modifierFound) {
modifierRewrite.insertFirst(publicModifier, null);
try {
TextEdit sourceEdit = sourceRewriter.rewriteAST();
sourceMultiTextEdit.addChild(sourceEdit);
sourceCompilationUnitChange.addTextEditGroup(new TextEditGroup("Set access level to public", new TextEdit[] {sourceEdit}));
}
catch(JavaModelException javaModelException) {
javaModelException.printStackTrace();
}
try {
TextEdit sourceEdit = sourceRewriter.rewriteAST();
ICompilationUnit sourceICompilationUnit = (ICompilationUnit)sourceCompilationUnit.getJavaElement();
CompilationUnitChange change = fChanges.get(sourceICompilationUnit);
if(change == null) {
MultiTextEdit sourceMultiTextEdit = new MultiTextEdit();
change = new CompilationUnitChange("", sourceICompilationUnit);
change.setEdit(sourceMultiTextEdit);
fChanges.put(sourceICompilationUnit, change);
}
change.getEdit().addChild(sourceEdit);
change.addTextEditGroup(new TextEditGroup("Update access modifier to public", new TextEdit[] {sourceEdit}));
}
catch(JavaModelException javaModelException) {
javaModelException.printStackTrace();
}
}
}
Expand Down
Loading

0 comments on commit adfdc82

Please sign in to comment.