diff --git a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ConstantChecks.java b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ConstantChecks.java index 95fff9a..191ff71 100644 --- a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ConstantChecks.java +++ b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ConstantChecks.java @@ -7,25 +7,35 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Jerome Cambon - [extract constant] blocked for anonymous and lambdas that declare variables - https://bugs.eclipse.org/271526 *******************************************************************************/ package org.eclipse.jdt.internal.corext.refactoring.code; +import java.util.ArrayList; +import java.util.List; + import org.eclipse.core.runtime.Assert; import org.eclipse.jdt.core.dom.ASTVisitor; +import org.eclipse.jdt.core.dom.AnonymousClassDeclaration; import org.eclipse.jdt.core.dom.FieldAccess; +import org.eclipse.jdt.core.dom.FieldDeclaration; 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.LambdaExpression; +import org.eclipse.jdt.core.dom.MethodDeclaration; import org.eclipse.jdt.core.dom.MethodInvocation; import org.eclipse.jdt.core.dom.Modifier; import org.eclipse.jdt.core.dom.Name; import org.eclipse.jdt.core.dom.QualifiedName; import org.eclipse.jdt.core.dom.SimpleName; +import org.eclipse.jdt.core.dom.SingleVariableDeclaration; import org.eclipse.jdt.core.dom.SuperFieldAccess; import org.eclipse.jdt.core.dom.SuperMethodInvocation; import org.eclipse.jdt.core.dom.ThisExpression; +import org.eclipse.jdt.core.dom.VariableDeclarationFragment; import org.eclipse.jdt.internal.corext.dom.fragments.ASTFragmentFactory; import org.eclipse.jdt.internal.corext.dom.fragments.IExpressionFragment; @@ -35,10 +45,18 @@ class ConstantChecks { private final IExpressionFragment fExpression; protected boolean fResult= true; - + protected boolean fAnonymous= false; + protected boolean fLambda= false; + protected List fLambdaAnonymousNames= new ArrayList(); + public ExpressionChecker(IExpressionFragment ex) { fExpression= ex; } + + public ExpressionChecker(IExpressionFragment ex, List lambdaAnonymousNames) { + fExpression= ex; + fLambdaAnonymousNames= lambdaAnonymousNames; + } public boolean check() { fResult= true; fExpression.getAssociatedNode().accept(this); @@ -50,6 +68,9 @@ class ConstantChecks { public LoadTimeConstantChecker(IExpressionFragment ex) { super(ex); } + public LoadTimeConstantChecker(IExpressionFragment ex, List lambdaAnonymousNames) { + super(ex, lambdaAnonymousNames); + } @Override public boolean visit(SuperFieldAccess node) { @@ -67,18 +88,54 @@ class ConstantChecks { return false; } @Override + public boolean visit(FieldDeclaration node) { + // Is it really needed ? Seems one never jump into this for anonymous/lambda + if (isAnonymousOrLambda()) { + for (Object fragment : node.fragments()) { + Assert.isTrue(fragment instanceof VariableDeclarationFragment); + fLambdaAnonymousNames.add(((VariableDeclarationFragment) fragment).getName().toString()); + } + return true; + } + return true; + } + + @Override public boolean visit(FieldAccess node) { - fResult&= new LoadTimeConstantChecker((IExpressionFragment) ASTFragmentFactory.createFragmentForFullSubtree(node.getExpression())).check(); + fResult&= new LoadTimeConstantChecker((IExpressionFragment) ASTFragmentFactory.createFragmentForFullSubtree(node.getExpression()), fLambdaAnonymousNames).check(); return false; } @Override + public boolean visit(VariableDeclarationFragment node) { + if (isAnonymousOrLambda()) { + fLambdaAnonymousNames.add(node.getName().toString()); + } + return true; + } + @Override + public boolean visit(MethodDeclaration node) { + if (isAnonymousOrLambda()) { + fLambdaAnonymousNames.add(node.getName().toString()); + for (Object param : node.parameters()) { + Assert.isTrue(param instanceof SingleVariableDeclaration); + SingleVariableDeclaration varDeclaration= (SingleVariableDeclaration) param; + fLambdaAnonymousNames.add(varDeclaration.getName().toString()); + } + } + return true; + } + @Override public boolean visit(MethodInvocation node) { - if(node.getExpression() == null) { + if (node.getExpression() == null) { visitName(node.getName()); } else { - fResult&= new LoadTimeConstantChecker((IExpressionFragment) ASTFragmentFactory.createFragmentForFullSubtree(node.getExpression())).check(); + if (node.getExpression().resolveTypeBinding().isAnonymous()) { + // If the expression is anonymous, this method invocation cannot be a load time constant + fResult= false; + } else { + fResult&= new LoadTimeConstantChecker((IExpressionFragment) ASTFragmentFactory.createFragmentForFullSubtree(node.getExpression()), fLambdaAnonymousNames).check(); + } } - return false; } @Override @@ -89,6 +146,23 @@ class ConstantChecks { public boolean visit(SimpleName node) { return visitName(node); } + @Override + public boolean visit(AnonymousClassDeclaration node) { + fAnonymous= true; + return true; + } + @Override + public boolean visit(LambdaExpression node) { + fLambda= true; + return true; + } + + private boolean isAnonymousOrLambda() { + if (fAnonymous || fLambda) { + return true; + } + return false; + } private boolean visitName(Name name) { fResult&= checkName(name); @@ -102,11 +176,15 @@ class ConstantChecks { scenarios which may have been deemed unacceptable in the presence of semantic information will be admitted. */ + // Check if the name is declared in anonymous/lambda + if (fLambdaAnonymousNames.contains(name.toString())) + return true; + // If name represents a member: if (binding instanceof IVariableBinding || binding instanceof IMethodBinding) return isMemberReferenceValidInClassInitialization(name); else if (binding instanceof ITypeBinding) - return ! ((ITypeBinding) binding).isTypeVariable(); + return !((ITypeBinding) binding).isTypeVariable(); else { return true; // e.g. a NameQualifiedType's qualifier, which can be a package binding }