Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stabilize testDuplicateLocal #1228

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.eclipse.core.runtime.ILog;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.WorkingCopyOwner;
import org.eclipse.jdt.internal.codeassist.DOMCompletionUtil;
import org.eclipse.jdt.internal.javac.dom.JavacAnnotationBinding;
import org.eclipse.jdt.internal.javac.dom.JavacErrorMethodBinding;
import org.eclipse.jdt.internal.javac.dom.JavacLambdaBinding;
Expand All @@ -46,11 +47,8 @@
import com.sun.tools.javac.api.JavacTaskImpl;
import com.sun.tools.javac.api.JavacTrees;
import com.sun.tools.javac.code.Attribute;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.TypeTag;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.code.Attribute.Compound;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Symbol.ModuleSymbol;
Expand All @@ -59,6 +57,7 @@
import com.sun.tools.javac.code.Symbol.TypeSymbol;
import com.sun.tools.javac.code.Symbol.TypeVariableSymbol;
import com.sun.tools.javac.code.Symbol.VarSymbol;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.Type.ArrayType;
import com.sun.tools.javac.code.Type.ClassType;
import com.sun.tools.javac.code.Type.ErrorType;
Expand All @@ -70,8 +69,9 @@
import com.sun.tools.javac.code.Type.ModuleType;
import com.sun.tools.javac.code.Type.PackageType;
import com.sun.tools.javac.code.Type.TypeVar;
import com.sun.tools.javac.code.TypeTag;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.tree.JCTree;
import com.sun.tools.javac.tree.TreeInfo;
import com.sun.tools.javac.tree.JCTree.JCAnnotatedType;
import com.sun.tools.javac.tree.JCTree.JCAnnotation;
import com.sun.tools.javac.tree.JCTree.JCArrayTypeTree;
Expand All @@ -96,6 +96,7 @@
import com.sun.tools.javac.tree.JCTree.JCTypeParameter;
import com.sun.tools.javac.tree.JCTree.JCVariableDecl;
import com.sun.tools.javac.tree.JCTree.JCWildcard;
import com.sun.tools.javac.tree.TreeInfo;
import com.sun.tools.javac.util.Context;
import com.sun.tools.javac.util.Names;

Expand Down Expand Up @@ -331,11 +332,11 @@ public JavacTypeVariableBinding getTypeVariableBinding(TypeVar typeVar) {
}
//
private Map<String, JavacVariableBinding> variableBindings = new HashMap<>();
public JavacVariableBinding getVariableBinding(VarSymbol varSymbol) {
public JavacVariableBinding getVariableBinding(VarSymbol varSymbol, boolean isUnique) {
if (varSymbol == null) {
return null;
}
JavacVariableBinding newInstance = new JavacVariableBinding(varSymbol, JavacBindingResolver.this) { };
JavacVariableBinding newInstance = new JavacVariableBinding(varSymbol, JavacBindingResolver.this, isUnique) { };
String k = newInstance.getKey();
if( k != null ) {
variableBindings.putIfAbsent(k, newInstance);
Expand Down Expand Up @@ -386,7 +387,15 @@ public IBinding getBinding(final Symbol owner, final com.sun.tools.javac.code.Ty
return getMethodBinding(methodType, other, null, false);
}
} else if (owner instanceof final VarSymbol other) {
return getVariableBinding(other);
if (JavacBindingResolver.this.symbolToDeclaration != null) {
ASTNode ownerDecl = JavacBindingResolver.this.symbolToDeclaration.get(owner.owner);
MethodDeclaration methodDecl = (MethodDeclaration) DOMCompletionUtil.findParent(ownerDecl, new int[] { ASTNode.METHOD_DECLARATION });
if (methodDecl != null) {
boolean isUnique = calculateIsUnique(methodDecl, other.name.toString());
return getVariableBinding(other, isUnique);
}
return getVariableBinding(other, true);
}
}
return null;
}
Expand Down Expand Up @@ -743,7 +752,7 @@ IVariableBinding resolveField(FieldAccess fieldAccess) {
resolve();
JCTree javacElement = this.converter.domToJavac.get(fieldAccess);
if (javacElement instanceof JCFieldAccess javacFieldAccess && javacFieldAccess.sym instanceof VarSymbol varSymbol) {
return this.bindings.getVariableBinding(varSymbol);
return this.bindings.getVariableBinding(varSymbol, true);
}
return null;
}
Expand All @@ -753,7 +762,7 @@ IVariableBinding resolveField(SuperFieldAccess fieldAccess) {
resolve();
JCTree javacElement = this.converter.domToJavac.get(fieldAccess);
if (javacElement instanceof JCFieldAccess javacFieldAccess && javacFieldAccess.sym instanceof VarSymbol varSymbol) {
return this.bindings.getVariableBinding(varSymbol);
return this.bindings.getVariableBinding(varSymbol, true);
}
return null;
}
Expand Down Expand Up @@ -1214,7 +1223,7 @@ IVariableBinding resolveVariable(EnumConstantDeclaration enumConstant) {
if (this.converter.domToJavac.get(enumConstant) instanceof JCVariableDecl decl) {
// the decl.type can be null when there are syntax errors
if ((decl.type != null && !decl.type.isErroneous()) || this.isRecoveringBindings()) {
return this.bindings.getVariableBinding(decl.sym);
return this.bindings.getVariableBinding(decl.sym, true);
}
}
return null;
Expand All @@ -1223,17 +1232,69 @@ IVariableBinding resolveVariable(EnumConstantDeclaration enumConstant) {
@Override
IVariableBinding resolveVariable(VariableDeclaration variable) {
resolve();
boolean isUnique = calculateIsUnique(variable);
if (this.converter.domToJavac.get(variable) instanceof JCVariableDecl decl) {
// the decl.type can be null when there are syntax errors
if ((decl.type != null && !decl.type.isErroneous()) || this.isRecoveringBindings()) {
if (decl.name != Names.instance(this.context).error) { // cannot recover if name is error
return this.bindings.getVariableBinding(decl.sym);
return this.bindings.getVariableBinding(decl.sym, isUnique);
}
}
}
return null;
}

public static boolean calculateIsUnique(VariableDeclaration variable) {
MethodDeclaration parentMethod = (MethodDeclaration)DOMCompletionUtil.findParent(variable, new int[] { ASTNode.METHOD_DECLARATION });
if (parentMethod == null) {
return true;
}
final String variableName = variable.getName().toString();
class UniquenessVisitor extends ASTVisitor {
boolean isUnique = true;
@Override
public boolean visit(VariableDeclarationFragment node) {
if (node != variable && variableName.equals(node.getName().toString())) {
isUnique = false;
}
return super.visit(node);
}
@Override
public boolean visit(SingleVariableDeclaration node) {
if (node != variable && variableName.equals(node.getName().toString())) {
isUnique = false;
}
return super.visit(node);
}
}
UniquenessVisitor uniquenessVisitor = new UniquenessVisitor();
parentMethod.accept(uniquenessVisitor);
return uniquenessVisitor.isUnique;
}

public static boolean calculateIsUnique(MethodDeclaration methodDecl, String name) {
class UniquenessVisitor extends ASTVisitor {
int count = 0;
@Override
public boolean visit(VariableDeclarationFragment node) {
if (name.equals(node.getName().toString())) {
this.count++;
}
return super.visit(node);
}
@Override
public boolean visit(SingleVariableDeclaration node) {
if (name.equals(node.getName().toString())) {
this.count++;
}
return super.visit(node);
}
}
UniquenessVisitor uniquenessVisitor = new UniquenessVisitor();
methodDecl.accept(uniquenessVisitor);
return uniquenessVisitor.count <= 1;
}

@Override
public IPackageBinding resolvePackage(PackageDeclaration decl) {
resolve();
Expand Down Expand Up @@ -1480,7 +1541,7 @@ public Object getValueFromAttribute(Attribute attribute) {
} else if (attribute instanceof Attribute.Class clazz) {
return this.bindings.getTypeBinding(clazz.classType);
} else if (attribute instanceof Attribute.Enum enumm) {
return this.bindings.getVariableBinding(enumm.value);
return this.bindings.getVariableBinding(enumm.value, true);
} else if (attribute instanceof Attribute.Array array) {
return Stream.of(array.values) //
.map(nestedAttr -> {
Expand All @@ -1489,7 +1550,7 @@ public Object getValueFromAttribute(Attribute attribute) {
} else if (nestedAttr instanceof Attribute.Class clazz) {
return this.bindings.getTypeBinding(clazz.classType);
} else if (nestedAttr instanceof Attribute.Enum enumerable) {
return this.bindings.getVariableBinding(enumerable.value);
return this.bindings.getVariableBinding(enumerable.value, true);
}
throw new IllegalArgumentException("Unexpected attribute type: " + nestedAttr.getClass().getCanonicalName());
}) //
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@
import org.eclipse.jdt.core.dom.IVariableBinding;
import org.eclipse.jdt.core.dom.JavacBindingResolver;
import org.eclipse.jdt.core.dom.JavacBindingResolver.BindingKeyException;
import org.eclipse.jdt.core.dom.LambdaExpression;
import org.eclipse.jdt.core.dom.MethodDeclaration;
import org.eclipse.jdt.core.dom.Modifier;
import org.eclipse.jdt.core.dom.SingleVariableDeclaration;
import org.eclipse.jdt.core.dom.TypeParameter;
import org.eclipse.jdt.internal.codeassist.DOMCompletionUtil;
import org.eclipse.jdt.internal.core.BinaryMethod;
import org.eclipse.jdt.internal.core.JavaElement;
import org.eclipse.jdt.internal.core.Member;
Expand Down Expand Up @@ -679,8 +681,16 @@ public IVariableBinding[] getSyntheticOuterLocals() {
if (!this.methodSymbol.isLambdaMethod()) {
return new IVariableBinding[0];
}
LambdaExpression lambdaExpression = (LambdaExpression) this.resolver.symbolToDeclaration.get(this.methodSymbol);
MethodDeclaration methodDeclaration = (MethodDeclaration) DOMCompletionUtil.findParent(lambdaExpression, new int[] { ASTNode.METHOD_DECLARATION });
return this.methodSymbol.capturedLocals.stream() //
.map(this.resolver.bindings::getVariableBinding) //
.map(varSymbol -> {
boolean isUnique = true;
if (methodDeclaration != null) {
isUnique = JavacBindingResolver.calculateIsUnique(methodDeclaration, getName());
}
return this.resolver.bindings.getVariableBinding(varSymbol, isUnique);
}) //
.toArray(IVariableBinding[]::new);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@
import org.eclipse.jdt.core.dom.ITypeBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;
import org.eclipse.jdt.core.dom.JavacBindingResolver;
import org.eclipse.jdt.core.dom.JavacBindingResolver.BindingKeyException;
import org.eclipse.jdt.core.dom.MethodDeclaration;
import org.eclipse.jdt.core.dom.Modifier;
import org.eclipse.jdt.core.dom.RecordDeclaration;
import org.eclipse.jdt.core.dom.TypeDeclaration;
import org.eclipse.jdt.core.dom.JavacBindingResolver.BindingKeyException;
import org.eclipse.jdt.internal.compiler.codegen.ConstantPool;
import org.eclipse.jdt.internal.core.BinaryType;
import org.eclipse.jdt.internal.core.JavaElement;
Expand All @@ -67,12 +67,9 @@
import com.sun.tools.javac.code.Attribute;
import com.sun.tools.javac.code.Flags;
import com.sun.tools.javac.code.Kinds;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.TypeTag;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.code.Kinds.Kind;
import com.sun.tools.javac.code.Kinds.KindSelector;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
import com.sun.tools.javac.code.Symbol.CompletionFailure;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
Expand All @@ -81,6 +78,7 @@
import com.sun.tools.javac.code.Symbol.TypeSymbol;
import com.sun.tools.javac.code.Symbol.TypeVariableSymbol;
import com.sun.tools.javac.code.Symbol.VarSymbol;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Type.ArrayType;
import com.sun.tools.javac.code.Type.ClassType;
import com.sun.tools.javac.code.Type.ErrorType;
Expand All @@ -90,6 +88,8 @@
import com.sun.tools.javac.code.Type.MethodType;
import com.sun.tools.javac.code.Type.TypeVar;
import com.sun.tools.javac.code.Type.WildcardType;
import com.sun.tools.javac.code.TypeTag;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.code.Types.FunctionDescriptorLookupError;
import com.sun.tools.javac.util.Name;
import com.sun.tools.javac.util.Names;
Expand Down Expand Up @@ -561,7 +561,7 @@ public IVariableBinding[] getDeclaredFields() {
.filter(VarSymbol.class::isInstance)
.map(VarSymbol.class::cast)
.filter(sym -> sym.name != this.names.error)
.map(this.resolver.bindings::getVariableBinding)
.map(varSym -> this.resolver.bindings.getVariableBinding(varSym, true))
.filter(Objects::nonNull)
.toArray(IVariableBinding[]::new);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,12 @@ public abstract class JavacVariableBinding implements IVariableBinding {

public final VarSymbol variableSymbol;
private final JavacBindingResolver resolver;
private final boolean isUnique;

public JavacVariableBinding(VarSymbol sym, JavacBindingResolver resolver) {
public JavacVariableBinding(VarSymbol sym, JavacBindingResolver resolver, boolean isUnique) {
this.variableSymbol = sym;
this.resolver = resolver;
this.isUnique = isUnique;
}

@Override
Expand Down Expand Up @@ -194,10 +196,11 @@ private String getKeyImpl() throws BindingKeyException {
return builder.toString();
} else if (this.variableSymbol.owner instanceof MethodSymbol methodSymbol) {
JavacMethodBinding.getKey(builder, methodSymbol, methodSymbol.type instanceof Type.MethodType methodType ? methodType : null, null, this.resolver);
// TODO, need to replace the '0' with an integer representing location in the method. Unclear how to do so.
// It needs to trace upwards through the blocks, with a # for each parent block
// and a number representing something like what statement in the block it is??
builder.append("#"); //0#";
// no need to get it right, just get it right enough
if (!isUnique) {
builder.append(this.variableSymbol.pos);
}
builder.append("#");
builder.append(this.variableSymbol.name);
// FIXME: is it possible for the javac AST to contain multiple definitions of the same variable?
// If so, we will need to distinguish them (@see org.eclipse.jdt.internal.compiler.lookup.LocalVariableBinding)
Expand Down
Loading