[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [jdt-ui-dev] [PATCH] fix for inline refactoring bugs #24941 and #35905
|
Hello Dimitry,
thanks for providing the patch for the two bugs. I will look at the patch
the next couple of days and will let you know
if there is something missing besides test cases for the new functionality
;--).
Are you aware of the org.eclipse.jdt.ui.tests.refactoring projects. It
contains all our refactoring tests. So for the new
functionality we should have test cases as well. The interesting test case
class is InlineMethodTests and the
test resources reside in resources/InlineMethodWorkspace/TestCases.
Dirk
"Stalnov, Dmitry"
<dstalnov@fusionO
ne.com> To
Sent by: "'jdt-ui-dev@xxxxxxxxxxx'"
jdt-ui-dev-admin@ <jdt-ui-dev@xxxxxxxxxxx>
eclipse.org cc
Subject
06/14/2003 12:13 [jdt-ui-dev] [PATCH] fix for inline
AM refactoring bugs #24941 and #35905
Please respond to
jdt-ui-dev@eclips
e.org
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(See attached file: patch.txt)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() {
