Improved method PolymorphismRefactoring.getFile() to search in all so…

…urce folders Added new precondition to exclude the cases where the type-check class is part of the inheritance hierarchy associated with the type-checking conditional logic
tsantalis · Nov 10, 2019 · 9374d10 · 9374d10
1 parent 8db9766
commit 9374d10
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 11 deletions.
diff --git a/META-INF/MANIFEST.MF b/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: JDeodorant Plug-in
 Bundle-SymbolicName: gr.uom.java.jdeodorant; singleton:=true
-Bundle-Version: 5.0.72
+Bundle-Version: 5.0.73
 Bundle-Activator: gr.uom.java.jdeodorant.refactoring.Activator
 Bundle-Localization: plugin
 Require-Bundle: org.eclipse.ui,

diff --git a/src/gr/uom/java/jdeodorant/refactoring/manipulators/PolymorphismRefactoring.java b/src/gr/uom/java/jdeodorant/refactoring/manipulators/PolymorphismRefactoring.java
@@ -1,5 +1,6 @@
 package gr.uom.java.jdeodorant.refactoring.manipulators;
 
+import gr.uom.java.ast.ASTReader;
 import gr.uom.java.ast.util.ExpressionExtractor;
 import gr.uom.java.ast.util.MethodDeclarationUtility;
 
@@ -18,6 +19,7 @@
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.jdt.core.ICompilationUnit;
 import org.eclipse.jdt.core.IJavaElement;
+import org.eclipse.jdt.core.IPackageFragmentRoot;
 import org.eclipse.jdt.core.JavaModelException;
 import org.eclipse.jdt.core.dom.AST;
 import org.eclipse.jdt.core.dom.Assignment;
@@ -701,11 +703,21 @@ else if(!(newSimpleName.getParent() instanceof QualifiedName)) {
 							subclassRewriter.set(methodInvocation, MethodInvocation.EXPRESSION_PROPERTY, subclassAST.newSimpleName(invokerName), null);
 							if(newSimpleName.getParent() instanceof FieldAccess) {
 								FieldAccess fieldAccess = (FieldAccess)newSimpleName.getParent();
-								subclassRewriter.replace(fieldAccess, methodInvocation, null);
+								if(newSimpleName.equals(fieldAccess.getName())) {
+									subclassRewriter.replace(fieldAccess, methodInvocation, null);
+								}
+								else {
+									subclassRewriter.set(fieldAccess, FieldAccess.EXPRESSION_PROPERTY, methodInvocation, null);
+								}
 							}
 							else if(newSimpleName.getParent() instanceof QualifiedName) {
 								QualifiedName qualifiedName = (QualifiedName)newSimpleName.getParent();
-								subclassRewriter.replace(qualifiedName, methodInvocation, null);
+								if(newSimpleName.equals(qualifiedName.getName())) {
+									subclassRewriter.replace(qualifiedName, methodInvocation, null);
+								}
+								else {
+									subclassRewriter.set(qualifiedName, QualifiedName.QUALIFIER_PROPERTY, methodInvocation, null);
+								}
 							}
 							else {
 								subclassRewriter.replace(newSimpleName, methodInvocation, null);
@@ -804,7 +816,24 @@ protected IMethodBinding findGetterMethodInContext(IVariableBinding fieldBinding
 		return null;
 	}
 
-	protected IFile getFile(IContainer rootContainer, String fullyQualifiedClassName) {
+	protected IFile getFile(String fullyQualifiedClassName) {
+		try {
+			IPackageFragmentRoot[] rootContainers = ASTReader.getExaminedProject().getAllPackageFragmentRoots();
+			for(IPackageFragmentRoot fragmentRoot : rootContainers) {
+				if(fragmentRoot.getKind() == IPackageFragmentRoot.K_SOURCE) {
+					IFile file = getFile((IContainer)fragmentRoot.getResource(), fullyQualifiedClassName);
+					if(file != null) {
+						return file;
+					}
+				}
+			}
+		} catch (JavaModelException e) {
+			e.printStackTrace();
+		}
+		return null;
+	}
+
+	private IFile getFile(IContainer rootContainer, String fullyQualifiedClassName) {
 		String[] subPackages = fullyQualifiedClassName.split("\\.");
 		IContainer classContainer = rootContainer;
 		IFile classFile = null;

diff --git a/src/gr/uom/java/jdeodorant/refactoring/manipulators/ReplaceConditionalWithPolymorphism.java b/src/gr/uom/java/jdeodorant/refactoring/manipulators/ReplaceConditionalWithPolymorphism.java
@@ -263,7 +263,7 @@ private void modifyInheritanceHierarchy() {
 				rootContainer = (IContainer)rootContainer.getParent();
 		}
 		String abstractClassFullyQualifiedName = typeCheckElimination.getAbstractClassName();
-		IFile abstractClassFile = getFile(rootContainer, abstractClassFullyQualifiedName);
+		IFile abstractClassFile = getFile(abstractClassFullyQualifiedName);
 
 		ICompilationUnit abstractICompilationUnit = null;
 		CompilationUnit abstractCompilationUnit = null;
@@ -495,7 +495,7 @@ else if(variableDeclarationFragment.getParent() instanceof FieldDeclaration) {
 			else {
 				statements = typeCheckElimination.getDefaultCaseStatements();
 			}
-			IFile subclassFile = getFile(rootContainer, subclassNames.get(i));
+			IFile subclassFile = getFile(subclassNames.get(i));
 			ICompilationUnit subclassICompilationUnit = null;
 			CompilationUnit subclassCompilationUnit = null;
 			AST subclassAST = null;

diff --git a/src/gr/uom/java/jdeodorant/refactoring/manipulators/ReplaceTypeCodeWithStateStrategy.java b/src/gr/uom/java/jdeodorant/refactoring/manipulators/ReplaceTypeCodeWithStateStrategy.java
@@ -947,7 +947,7 @@ private void createStateStrategyHierarchy() {
 		IFile stateStrategyFile = null;
 		if(tree != null) {
 			DefaultMutableTreeNode rootNode = tree.getRootNode();
-			stateStrategyFile = getFile(rootContainer, (String)rootNode.getUserObject());
+			stateStrategyFile = getFile((String)rootNode.getUserObject());
 		}
 		else {
 			if(contextContainer instanceof IProject) {
@@ -1258,7 +1258,7 @@ else if(variableDeclarationFragment.getParent() instanceof FieldDeclaration) {
 				while(leaf != null) {
 					String qualifiedSubclassName = (String)leaf.getUserObject();
 					if((qualifiedSubclassName.contains(".") && qualifiedSubclassName.endsWith("." + subclassNames.get(i))) || qualifiedSubclassName.equals(subclassNames.get(i))) {
-						subclassFile = getFile(rootContainer, qualifiedSubclassName);
+						subclassFile = getFile(qualifiedSubclassName);
 						break;
 					}
 					leaf = leaf.getNextLeaf();
@@ -1649,7 +1649,7 @@ private void createIntermediateClassAndItsSubclasses(List<SimpleName> staticFiel
 			while(leaf != null) {
 				String qualifiedSubclassName = (String)leaf.getUserObject();
 				if((qualifiedSubclassName.contains(".") && qualifiedSubclassName.endsWith("." + intermediateClassName)) || qualifiedSubclassName.equals(intermediateClassName)) {
-					intermediateClassFile = getFile(rootContainer, qualifiedSubclassName);
+					intermediateClassFile = getFile(qualifiedSubclassName);
 					break;
 				}
 				leaf = leaf.getNextLeaf();
@@ -1942,7 +1942,7 @@ else if(variableDeclarationFragment.getParent() instanceof FieldDeclaration) {
 				while(leaf != null) {
 					String qualifiedSubclassName = (String)leaf.getUserObject();
 					if((qualifiedSubclassName.contains(".") && qualifiedSubclassName.endsWith("." + subclassNames.get(i))) || qualifiedSubclassName.equals(subclassNames.get(i))) {
-						subclassFile = getFile(rootContainer, qualifiedSubclassName);
+						subclassFile = getFile(qualifiedSubclassName);
 						break;
 					}
 					leaf = leaf.getNextLeaf();

diff --git a/src/gr/uom/java/jdeodorant/refactoring/manipulators/TypeCheckElimination.java b/src/gr/uom/java/jdeodorant/refactoring/manipulators/TypeCheckElimination.java
@@ -7,6 +7,7 @@
 import gr.uom.java.ast.util.StatementExtractor;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -426,12 +427,24 @@ public boolean allTypeCheckingsContainStaticFieldOrSubclassType() {
 
 	public boolean isApplicable() {
 		if(!containsLocalVariableAssignment() && !containsBranchingStatement() && !containsSuperMethodInvocation() && !containsSuperFieldAccess() &&
-				!isSubclassTypeAnInterface() && !returnStatementAfterTypeCheckCodeFragment())
+				!isSubclassTypeAnInterface() && !returnStatementAfterTypeCheckCodeFragment() && !typeCheckClassPartOfExistingInheritanceTree())
 			return true;
 		else
 			return false;
 	}
 
+	private boolean typeCheckClassPartOfExistingInheritanceTree() {
+		Collection<List<Type>> subTypeCollection = subclassTypeMap.values();
+		for(List<Type> subTypes : subTypeCollection) {
+			for(Type subType : subTypes) {
+				if(subType.resolveBinding().isEqualTo(typeCheckClass.resolveBinding())) {
+					return true;
+				}
+			}
+		}
+		return false;
+	}
+
 	private boolean isSubclassTypeAnInterface() {
 		for(List<Type> subTypes : subclassTypeMap.values()) {
 			for(Type subType : subTypes) {