| [jdt-ui-dev] [PATCH] fix for inline refactoring bugs #24941 and #35905 |
|
Hello all,
In our company
we use Eclipse as a main tool for Java programming and are
very happy with it. I'd like to participate in its development. For a
start, to get familiar with the codebase, I decided to fix several
bugs. Attached is a patch fixing two refactoring bugs (24941, 35905). Could
somebody review it and either apply to CVS or make some comments and
suggestions how I can improve the fix?
Thank you in
advance,
Dmitry
|
Index: CallInliner.java
===================================================================
RCS file: /home/eclipse/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/code/CallInliner.java,v
retrieving revision 1.27
diff -u -r1.27 CallInliner.java
--- CallInliner.java 12 Jun 2003 10:41:58 -0000 1.27
+++ CallInliner.java 13 Jun 2003 22:00:13 -0000
@@ -14,40 +14,46 @@
import java.util.Iterator;
import java.util.List;
-import org.eclipse.core.runtime.CoreException;
-
import org.eclipse.core.resources.IFile;
-
+import org.eclipse.core.runtime.CoreException;
import org.eclipse.jdt.core.ICompilationUnit;
+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.ASTVisitor;
import org.eclipse.jdt.core.dom.Block;
import org.eclipse.jdt.core.dom.BodyDeclaration;
+import org.eclipse.jdt.core.dom.CastExpression;
+import org.eclipse.jdt.core.dom.ClassInstanceCreation;
import org.eclipse.jdt.core.dom.DoStatement;
import org.eclipse.jdt.core.dom.Expression;
import org.eclipse.jdt.core.dom.FieldAccess;
import org.eclipse.jdt.core.dom.ForStatement;
import org.eclipse.jdt.core.dom.IBinding;
+import org.eclipse.jdt.core.dom.IMethodBinding;
import org.eclipse.jdt.core.dom.ITypeBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;
import org.eclipse.jdt.core.dom.IfStatement;
import org.eclipse.jdt.core.dom.Initializer;
import org.eclipse.jdt.core.dom.MethodDeclaration;
+import org.eclipse.jdt.core.dom.MethodInvocation;
import org.eclipse.jdt.core.dom.Name;
import org.eclipse.jdt.core.dom.ParenthesizedExpression;
import org.eclipse.jdt.core.dom.SimpleName;
import org.eclipse.jdt.core.dom.Statement;
import org.eclipse.jdt.core.dom.SuperFieldAccess;
import org.eclipse.jdt.core.dom.ThisExpression;
+import org.eclipse.jdt.core.dom.TypeDeclaration;
import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
import org.eclipse.jdt.core.dom.VariableDeclarationStatement;
import org.eclipse.jdt.core.dom.WhileStatement;
-
import org.eclipse.jdt.internal.corext.Assert;
import org.eclipse.jdt.internal.corext.codemanipulation.CodeGenerationSettings;
import org.eclipse.jdt.internal.corext.codemanipulation.ImportEdit;
import org.eclipse.jdt.internal.corext.dom.ASTNodeFactory;
import org.eclipse.jdt.internal.corext.dom.ASTNodes;
import org.eclipse.jdt.internal.corext.dom.ASTRewrite;
+import org.eclipse.jdt.internal.corext.dom.Bindings;
import org.eclipse.jdt.internal.corext.dom.CodeScopeBuilder;
import org.eclipse.jdt.internal.corext.dom.HierarchicalASTVisitor;
import org.eclipse.jdt.internal.corext.dom.LocalVariableIndex;
@@ -60,6 +66,7 @@
import org.eclipse.jdt.internal.corext.textmanipulation.TextBuffer;
import org.eclipse.jdt.internal.corext.textmanipulation.TextEdit;
import org.eclipse.jdt.internal.corext.util.WorkingCopyUtil;
+import org.eclipse.jdt.internal.ui.text.correction.ASTResolving;
public class CallInliner {
@@ -230,7 +237,11 @@
Expression expression= (Expression)arguments.get(i);
ParameterData parameter= fSourceProvider.getParameterData(i);
if (canInline(expression, parameter)) {
- realArguments[i]= getContent(expression);
+ realArguments[i] = getContent(expression);
+ // fixes bug #35905
+ if(expression instanceof CastExpression) {
+ realArguments[i] = "(" + realArguments[i] + ")"; //$NON-NLS-1$//$NON-NLS-2$
+ }
} else {
String name= fInvocationScope.createName(parameter.getName(), true);
realArguments[i]= name;
@@ -284,7 +295,19 @@
}
}
- private void replaceCall(int callType, String[] blocks) {
+ private static class ImportAnalyzer extends ASTVisitor {
+ private ImportEdit fImportEdit;
+ public ImportAnalyzer(ImportEdit importEdit) {
+ fImportEdit = importEdit;
+ }
+ public boolean visit(ClassInstanceCreation node) {
+ fImportEdit.addImport(node.resolveTypeBinding());
+ return false;
+ }
+ }
+
+ private void replaceCall(int callType, String[] blocks)
+ throws JavaModelException {
// Inline empty body
if (blocks.length == 0) {
if (fNeedsStatement) {
@@ -293,6 +316,9 @@
fRewriter.markAsRemoved(fTargetNode);
}
} else {
+ ImportAnalyzer analyzer = new ImportAnalyzer(fImportEdit);
+ fSourceProvider.getDeclaration().getBody().accept(analyzer);
+
ASTNode node= null;
for (int i= 0; i < blocks.length - 1; i++) {
node= fRewriter.createPlaceholder(blocks[i], ASTRewrite.STATEMENT);
@@ -318,7 +344,20 @@
node= null;
}
} else if (fTargetNode instanceof Expression) {
+
node= fRewriter.createPlaceholder(block, ASTRewrite.EXPRESSION);
+
+ // fixes bug #24941
+ if(needsExplicitCast()) {
+ AST ast = node.getAST();
+ CastExpression castExpression = ast.newCastExpression();
+ ITypeBinding returnType = fSourceProvider.getReturnType();
+ fImportEdit.addImport(returnType);
+ castExpression.setType(ASTNodeFactory.newType(ast, returnType, false));
+ castExpression.setExpression((Expression)node);
+ node = castExpression;
+ }
+
if (needsParenthesis()) {
ParenthesizedExpression pExp= fTargetNode.getAST().newParenthesizedExpression();
pExp.setExpression((Expression)node);
@@ -342,6 +381,107 @@
}
}
}
+ }
+
+ /**
+ * @return <code>true</code> if explicit cast is needed otherwise <code>false</code>
+ * @throws JavaModelException
+ */
+ private boolean needsExplicitCast()
+ throws JavaModelException {
+
+ ASTNode parent = fTargetNode.getParent();
+ int nodeType = parent.getNodeType();
+ if(nodeType == ASTNode.METHOD_INVOCATION) {
+ MethodInvocation methodInvocation = (MethodInvocation)parent;
+ IMethodBinding method = methodInvocation.resolveMethodBinding();
+ ITypeBinding[] parameters = method.getParameterTypes();
+ ITypeBinding type = resolveTypeBinding(methodInvocation);
+
+ // finds all overloaded methods with the same name in hierarchy
+ IMethodBinding[] methods = findMethodsInHierarchy(type, method.getName());
+ for(int i = 0; i < methods.length; i++) {
+ // skip the method itself
+ if(!Bindings.equals(methods[i], method)) {
+ // access modifier of the method can restrict the call but
+ // still it is better to have explicit cast in the place
+ if(canImplicitlyCall(methods[i], parameters)) {
+ return true;
+ }
+ }
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Utility method that returns type used in the method invocation.
+ *
+ * @param methodInvocation invocation to resolve type of
+ * @return type used in the method invocation. NB! not the type declaring
+ * the method, it can be declared in a super class.
+ * @throws JavaModelException
+ */
+ private ITypeBinding resolveTypeBinding(MethodInvocation methodInvocation)
+ throws JavaModelException {
+
+ ITypeBinding typeBinding = null;
+ Expression exp = methodInvocation.getExpression();
+ if(exp != null) {
+ typeBinding = exp.resolveTypeBinding();
+ }
+ else {
+ TypeDeclaration type = (TypeDeclaration)ASTResolving.findParentType(methodInvocation);
+ typeBinding = type.resolveBinding();
+ }
+ return typeBinding;
+ }
+
+ /**
+ * The utility method returns all method bindings with the name specified
+ * in type hierarchy.
+ *
+ * @param type type to start from
+ * @param methodName name of the method to search for in the hierarchy
+ * @return array of methods with the name specified
+ */
+ private IMethodBinding[] findMethodsInHierarchy(ITypeBinding type, String methodName) {
+
+ ArrayList result = new ArrayList();
+
+ while(type != null) {
+ IMethodBinding[] methods = type.getDeclaredMethods();
+ for(int i = 0; i < methods.length; i++) {
+ if(methodName.equals(methods[i].getName())) {
+ result.add(methods[i]);
+ }
+ }
+ type = type.getSuperclass();
+ }
+
+ return (IMethodBinding[])result.toArray(new IMethodBinding[result.size()]);
+ }
+
+ /**
+ * @param method method binding to query for
+ * @param types method parameters
+ * @return <code>true</code> if the method can be called with specified parameters
+ * without explicit casts, otherwise <code>false</code>.
+ * @throws JavaModelException
+ */
+ private boolean canImplicitlyCall(IMethodBinding method, ITypeBinding[] types)
+ throws JavaModelException {
+
+ ITypeBinding[] parameters = method.getParameterTypes();
+ if(types.length != parameters.length) {
+ return false;
+ }
+ for(int i = 0; i < parameters.length; i++) {
+ if(!ASTResolving.canAssign(parameters[i], types[i])) {
+ return false;
+ }
+ }
+ return true;
}
private boolean needsParenthesis() {