Skip to content

Commit 752d721

Browse files
datho7561Rob Stryker
datho7561
authored and
Rob Stryker
committed
Stabilize testDuplicateLocal
- Fix the number part of IVariableBinding's key - This means that the bindings for different variables with the same name in the same method can be distinguished eg. `rob` and `i` can be distinguished: ```java public class Main { public void myMethod() { for (int i = 0; i < 10; i++) { int rob = 12; } for (String i = "asdf"; "hjkl".equals(i); i = "hjkl") { String rob = "rob"; } } } ``` - As a result, completion works deterministically in these cases (eg. before you just had to hope that the correct binding was cached in order to get the right results) Signed-off-by: David Thompson <[email protected]>
1 parent 8753006 commit 752d721

File tree

4 files changed

+100
-26
lines changed

4 files changed

+100
-26
lines changed

‎org.eclipse.jdt.core.javac/src/org/eclipse/jdt/core/dom/JavacBindingResolver.java‎

Lines changed: 75 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.eclipse.core.runtime.ILog;
2828
import org.eclipse.jdt.core.IJavaProject;
2929
import org.eclipse.jdt.core.WorkingCopyOwner;
30+
import org.eclipse.jdt.internal.codeassist.DOMCompletionUtil;
3031
import org.eclipse.jdt.internal.javac.dom.JavacAnnotationBinding;
3132
import org.eclipse.jdt.internal.javac.dom.JavacErrorMethodBinding;
3233
import org.eclipse.jdt.internal.javac.dom.JavacLambdaBinding;
@@ -46,11 +47,8 @@
4647
import com.sun.tools.javac.api.JavacTaskImpl;
4748
import com.sun.tools.javac.api.JavacTrees;
4849
import com.sun.tools.javac.code.Attribute;
49-
import com.sun.tools.javac.code.Symbol;
50-
import com.sun.tools.javac.code.Symtab;
51-
import com.sun.tools.javac.code.TypeTag;
52-
import com.sun.tools.javac.code.Types;
5350
import com.sun.tools.javac.code.Attribute.Compound;
51+
import com.sun.tools.javac.code.Symbol;
5452
import com.sun.tools.javac.code.Symbol.ClassSymbol;
5553
import com.sun.tools.javac.code.Symbol.MethodSymbol;
5654
import com.sun.tools.javac.code.Symbol.ModuleSymbol;
@@ -59,6 +57,7 @@
5957
import com.sun.tools.javac.code.Symbol.TypeSymbol;
6058
import com.sun.tools.javac.code.Symbol.TypeVariableSymbol;
6159
import com.sun.tools.javac.code.Symbol.VarSymbol;
60+
import com.sun.tools.javac.code.Symtab;
6261
import com.sun.tools.javac.code.Type.ArrayType;
6362
import com.sun.tools.javac.code.Type.ClassType;
6463
import com.sun.tools.javac.code.Type.ErrorType;
@@ -70,8 +69,9 @@
7069
import com.sun.tools.javac.code.Type.ModuleType;
7170
import com.sun.tools.javac.code.Type.PackageType;
7271
import com.sun.tools.javac.code.Type.TypeVar;
72+
import com.sun.tools.javac.code.TypeTag;
73+
import com.sun.tools.javac.code.Types;
7374
import com.sun.tools.javac.tree.JCTree;
74-
import com.sun.tools.javac.tree.TreeInfo;
7575
import com.sun.tools.javac.tree.JCTree.JCAnnotatedType;
7676
import com.sun.tools.javac.tree.JCTree.JCAnnotation;
7777
import com.sun.tools.javac.tree.JCTree.JCArrayTypeTree;
@@ -96,6 +96,7 @@
9696
import com.sun.tools.javac.tree.JCTree.JCTypeParameter;
9797
import com.sun.tools.javac.tree.JCTree.JCVariableDecl;
9898
import com.sun.tools.javac.tree.JCTree.JCWildcard;
99+
import com.sun.tools.javac.tree.TreeInfo;
99100
import com.sun.tools.javac.util.Context;
100101
import com.sun.tools.javac.util.Names;
101102

@@ -331,11 +332,11 @@ public JavacTypeVariableBinding getTypeVariableBinding(TypeVar typeVar) {
331332
}
332333
//
333334
private Map<String, JavacVariableBinding> variableBindings = new HashMap<>();
334-
public JavacVariableBinding getVariableBinding(VarSymbol varSymbol) {
335+
public JavacVariableBinding getVariableBinding(VarSymbol varSymbol, boolean isUnique) {
335336
if (varSymbol == null) {
336337
return null;
337338
}
338-
JavacVariableBinding newInstance = new JavacVariableBinding(varSymbol, JavacBindingResolver.this) { };
339+
JavacVariableBinding newInstance = new JavacVariableBinding(varSymbol, JavacBindingResolver.this, isUnique) { };
339340
String k = newInstance.getKey();
340341
if( k != null ) {
341342
variableBindings.putIfAbsent(k, newInstance);
@@ -386,7 +387,15 @@ public IBinding getBinding(final Symbol owner, final com.sun.tools.javac.code.Ty
386387
return getMethodBinding(methodType, other, null, false);
387388
}
388389
} else if (owner instanceof final VarSymbol other) {
389-
return getVariableBinding(other);
390+
if (JavacBindingResolver.this.symbolToDeclaration != null) {
391+
ASTNode ownerDecl = JavacBindingResolver.this.symbolToDeclaration.get(owner.owner);
392+
MethodDeclaration methodDecl = (MethodDeclaration) DOMCompletionUtil.findParent(ownerDecl, new int[] { ASTNode.METHOD_DECLARATION });
393+
if (methodDecl != null) {
394+
boolean isUnique = calculateIsUnique(methodDecl, other.name.toString());
395+
return getVariableBinding(other, isUnique);
396+
}
397+
return getVariableBinding(other, true);
398+
}
390399
}
391400
return null;
392401
}
@@ -743,7 +752,7 @@ IVariableBinding resolveField(FieldAccess fieldAccess) {
743752
resolve();
744753
JCTree javacElement = this.converter.domToJavac.get(fieldAccess);
745754
if (javacElement instanceof JCFieldAccess javacFieldAccess && javacFieldAccess.sym instanceof VarSymbol varSymbol) {
746-
return this.bindings.getVariableBinding(varSymbol);
755+
return this.bindings.getVariableBinding(varSymbol, true);
747756
}
748757
return null;
749758
}
@@ -753,7 +762,7 @@ IVariableBinding resolveField(SuperFieldAccess fieldAccess) {
753762
resolve();
754763
JCTree javacElement = this.converter.domToJavac.get(fieldAccess);
755764
if (javacElement instanceof JCFieldAccess javacFieldAccess && javacFieldAccess.sym instanceof VarSymbol varSymbol) {
756-
return this.bindings.getVariableBinding(varSymbol);
765+
return this.bindings.getVariableBinding(varSymbol, true);
757766
}
758767
return null;
759768
}
@@ -1214,7 +1223,7 @@ IVariableBinding resolveVariable(EnumConstantDeclaration enumConstant) {
12141223
if (this.converter.domToJavac.get(enumConstant) instanceof JCVariableDecl decl) {
12151224
// the decl.type can be null when there are syntax errors
12161225
if ((decl.type != null && !decl.type.isErroneous()) || this.isRecoveringBindings()) {
1217-
return this.bindings.getVariableBinding(decl.sym);
1226+
return this.bindings.getVariableBinding(decl.sym, true);
12181227
}
12191228
}
12201229
return null;
@@ -1223,17 +1232,69 @@ IVariableBinding resolveVariable(EnumConstantDeclaration enumConstant) {
12231232
@Override
12241233
IVariableBinding resolveVariable(VariableDeclaration variable) {
12251234
resolve();
1235+
boolean isUnique = calculateIsUnique(variable);
12261236
if (this.converter.domToJavac.get(variable) instanceof JCVariableDecl decl) {
12271237
// the decl.type can be null when there are syntax errors
12281238
if ((decl.type != null && !decl.type.isErroneous()) || this.isRecoveringBindings()) {
12291239
if (decl.name != Names.instance(this.context).error) { // cannot recover if name is error
1230-
return this.bindings.getVariableBinding(decl.sym);
1240+
return this.bindings.getVariableBinding(decl.sym, isUnique);
12311241
}
12321242
}
12331243
}
12341244
return null;
12351245
}
12361246

1247+
public static boolean calculateIsUnique(VariableDeclaration variable) {
1248+
MethodDeclaration parentMethod = (MethodDeclaration)DOMCompletionUtil.findParent(variable, new int[] { ASTNode.METHOD_DECLARATION });
1249+
if (parentMethod == null) {
1250+
return true;
1251+
}
1252+
final String variableName = variable.getName().toString();
1253+
class UniquenessVisitor extends ASTVisitor {
1254+
boolean isUnique = true;
1255+
@Override
1256+
public boolean visit(VariableDeclarationFragment node) {
1257+
if (node != variable && variableName.equals(node.getName().toString())) {
1258+
isUnique = false;
1259+
}
1260+
return super.visit(node);
1261+
}
1262+
@Override
1263+
public boolean visit(SingleVariableDeclaration node) {
1264+
if (node != variable && variableName.equals(node.getName().toString())) {
1265+
isUnique = false;
1266+
}
1267+
return super.visit(node);
1268+
}
1269+
}
1270+
UniquenessVisitor uniquenessVisitor = new UniquenessVisitor();
1271+
parentMethod.accept(uniquenessVisitor);
1272+
return uniquenessVisitor.isUnique;
1273+
}
1274+
1275+
public static boolean calculateIsUnique(MethodDeclaration methodDecl, String name) {
1276+
class UniquenessVisitor extends ASTVisitor {
1277+
int count = 0;
1278+
@Override
1279+
public boolean visit(VariableDeclarationFragment node) {
1280+
if (name.equals(node.getName().toString())) {
1281+
this.count++;
1282+
}
1283+
return super.visit(node);
1284+
}
1285+
@Override
1286+
public boolean visit(SingleVariableDeclaration node) {
1287+
if (name.equals(node.getName().toString())) {
1288+
this.count++;
1289+
}
1290+
return super.visit(node);
1291+
}
1292+
}
1293+
UniquenessVisitor uniquenessVisitor = new UniquenessVisitor();
1294+
methodDecl.accept(uniquenessVisitor);
1295+
return uniquenessVisitor.count <= 1;
1296+
}
1297+
12371298
@Override
12381299
public IPackageBinding resolvePackage(PackageDeclaration decl) {
12391300
resolve();
@@ -1480,7 +1541,7 @@ public Object getValueFromAttribute(Attribute attribute) {
14801541
} else if (attribute instanceof Attribute.Class clazz) {
14811542
return this.bindings.getTypeBinding(clazz.classType);
14821543
} else if (attribute instanceof Attribute.Enum enumm) {
1483-
return this.bindings.getVariableBinding(enumm.value);
1544+
return this.bindings.getVariableBinding(enumm.value, true);
14841545
} else if (attribute instanceof Attribute.Array array) {
14851546
return Stream.of(array.values) //
14861547
.map(nestedAttr -> {
@@ -1489,7 +1550,7 @@ public Object getValueFromAttribute(Attribute attribute) {
14891550
} else if (nestedAttr instanceof Attribute.Class clazz) {
14901551
return this.bindings.getTypeBinding(clazz.classType);
14911552
} else if (nestedAttr instanceof Attribute.Enum enumerable) {
1492-
return this.bindings.getVariableBinding(enumerable.value);
1553+
return this.bindings.getVariableBinding(enumerable.value, true);
14931554
}
14941555
throw new IllegalArgumentException("Unexpected attribute type: " + nestedAttr.getClass().getCanonicalName());
14951556
}) //

‎org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/dom/JavacMethodBinding.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,12 @@
3535
import org.eclipse.jdt.core.dom.IVariableBinding;
3636
import org.eclipse.jdt.core.dom.JavacBindingResolver;
3737
import org.eclipse.jdt.core.dom.JavacBindingResolver.BindingKeyException;
38+
import org.eclipse.jdt.core.dom.LambdaExpression;
3839
import org.eclipse.jdt.core.dom.MethodDeclaration;
3940
import org.eclipse.jdt.core.dom.Modifier;
4041
import org.eclipse.jdt.core.dom.SingleVariableDeclaration;
4142
import org.eclipse.jdt.core.dom.TypeParameter;
43+
import org.eclipse.jdt.internal.codeassist.DOMCompletionUtil;
4244
import org.eclipse.jdt.internal.core.BinaryMethod;
4345
import org.eclipse.jdt.internal.core.JavaElement;
4446
import org.eclipse.jdt.internal.core.Member;
@@ -679,8 +681,16 @@ public IVariableBinding[] getSyntheticOuterLocals() {
679681
if (!this.methodSymbol.isLambdaMethod()) {
680682
return new IVariableBinding[0];
681683
}
684+
LambdaExpression lambdaExpression = (LambdaExpression) this.resolver.symbolToDeclaration.get(this.methodSymbol);
685+
MethodDeclaration methodDeclaration = (MethodDeclaration) DOMCompletionUtil.findParent(lambdaExpression, new int[] { ASTNode.METHOD_DECLARATION });
682686
return this.methodSymbol.capturedLocals.stream() //
683-
.map(this.resolver.bindings::getVariableBinding) //
687+
.map(varSymbol -> {
688+
boolean isUnique = true;
689+
if (methodDeclaration != null) {
690+
isUnique = JavacBindingResolver.calculateIsUnique(methodDeclaration, getName());
691+
}
692+
return this.resolver.bindings.getVariableBinding(varSymbol, isUnique);
693+
}) //
684694
.toArray(IVariableBinding[]::new);
685695
}
686696

‎org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/dom/JavacTypeBinding.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,11 @@
5252
import org.eclipse.jdt.core.dom.ITypeBinding;
5353
import org.eclipse.jdt.core.dom.IVariableBinding;
5454
import org.eclipse.jdt.core.dom.JavacBindingResolver;
55+
import org.eclipse.jdt.core.dom.JavacBindingResolver.BindingKeyException;
5556
import org.eclipse.jdt.core.dom.MethodDeclaration;
5657
import org.eclipse.jdt.core.dom.Modifier;
5758
import org.eclipse.jdt.core.dom.RecordDeclaration;
5859
import org.eclipse.jdt.core.dom.TypeDeclaration;
59-
import org.eclipse.jdt.core.dom.JavacBindingResolver.BindingKeyException;
6060
import org.eclipse.jdt.internal.compiler.codegen.ConstantPool;
6161
import org.eclipse.jdt.internal.core.BinaryType;
6262
import org.eclipse.jdt.internal.core.JavaElement;
@@ -67,12 +67,9 @@
6767
import com.sun.tools.javac.code.Attribute;
6868
import com.sun.tools.javac.code.Flags;
6969
import com.sun.tools.javac.code.Kinds;
70-
import com.sun.tools.javac.code.Symbol;
71-
import com.sun.tools.javac.code.Type;
72-
import com.sun.tools.javac.code.TypeTag;
73-
import com.sun.tools.javac.code.Types;
7470
import com.sun.tools.javac.code.Kinds.Kind;
7571
import com.sun.tools.javac.code.Kinds.KindSelector;
72+
import com.sun.tools.javac.code.Symbol;
7673
import com.sun.tools.javac.code.Symbol.ClassSymbol;
7774
import com.sun.tools.javac.code.Symbol.CompletionFailure;
7875
import com.sun.tools.javac.code.Symbol.MethodSymbol;
@@ -81,6 +78,7 @@
8178
import com.sun.tools.javac.code.Symbol.TypeSymbol;
8279
import com.sun.tools.javac.code.Symbol.TypeVariableSymbol;
8380
import com.sun.tools.javac.code.Symbol.VarSymbol;
81+
import com.sun.tools.javac.code.Type;
8482
import com.sun.tools.javac.code.Type.ArrayType;
8583
import com.sun.tools.javac.code.Type.ClassType;
8684
import com.sun.tools.javac.code.Type.ErrorType;
@@ -90,6 +88,8 @@
9088
import com.sun.tools.javac.code.Type.MethodType;
9189
import com.sun.tools.javac.code.Type.TypeVar;
9290
import com.sun.tools.javac.code.Type.WildcardType;
91+
import com.sun.tools.javac.code.TypeTag;
92+
import com.sun.tools.javac.code.Types;
9393
import com.sun.tools.javac.code.Types.FunctionDescriptorLookupError;
9494
import com.sun.tools.javac.util.Name;
9595
import com.sun.tools.javac.util.Names;
@@ -561,7 +561,7 @@ public IVariableBinding[] getDeclaredFields() {
561561
.filter(VarSymbol.class::isInstance)
562562
.map(VarSymbol.class::cast)
563563
.filter(sym -> sym.name != this.names.error)
564-
.map(this.resolver.bindings::getVariableBinding)
564+
.map(varSym -> this.resolver.bindings.getVariableBinding(varSym, true))
565565
.filter(Objects::nonNull)
566566
.toArray(IVariableBinding[]::new);
567567
}

‎org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/dom/JavacVariableBinding.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,12 @@ public abstract class JavacVariableBinding implements IVariableBinding {
5959

6060
public final VarSymbol variableSymbol;
6161
private final JavacBindingResolver resolver;
62+
private final boolean isUnique;
6263

63-
public JavacVariableBinding(VarSymbol sym, JavacBindingResolver resolver) {
64+
public JavacVariableBinding(VarSymbol sym, JavacBindingResolver resolver, boolean isUnique) {
6465
this.variableSymbol = sym;
6566
this.resolver = resolver;
67+
this.isUnique = isUnique;
6668
}
6769

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

0 commit comments

Comments
 (0)