Community
Participate
Working Groups
3.2RC6 In presence of duplicate local variables, duplicate fields or methods; the compiler discards all instances of the offending construct. This means that more secondary errors may arise and be quite misleading outside of context, and other tools based on the AST (DOM AST for instance) may behave suboptimally as lacking some of the binding information. What if: - it still created the bindings, and attached them to the AST node - it would leave at least one entry in binding arrays to reduce amount of secondary errors. e.g. we currently complain about missing both #foo() and #baz() public class X { void foo() { } void foo() { } void bar() { foo(); baz(); } }
Other related scenario with local variables: public class X { void foo(){ int x = 0; String x = ""; x.toString(); } } Currently complains about applying #toString() to primitive type int. Considering the last definition to override previous one, we would tolerate the message send (note that variable lookup already does walk the scope backward, we simply need to insert the offending variable in scope).
Other example where variable collision is altering catch block diagnostics (i.e. unable to spot conflicting catch block until renaming local var) public class X { public static void main(String[] args) { int x = 2; try { } catch(Exception x) { } catch(Exception e) { } } }
Other example (variation on bug ) shows that we could also spot assignment with no effect in presence of dup variable: public class X { public static void main(String[] args) { int x = x = 0; if (true) { int x = x = 1; } } } "int x = x = 1;" could be blamed for assignment with no effect.
Previous comment should have mentionned bug 144426
One more interesting outcome if fixing this: Consider the following scenario, where user pasted some code which happen to have a local var name conflict (x). Let say, he wants to rename the catch x variable into x2. Move cursor on it, perform "ctrl-2 r" to perform rename in file, observe that only the catch var is renamed, user has to manually go and fix the references which is error prone as it may still bind to the outer one and be valid (so hard to spot, like in case of nested loops). import java.io.*; public class X { public static void main(String[] args) { int x = 2; try { } catch(Exception x) { if (x instanceof IOException) { } } catch(Exception e) { } } }
More compelling example (maybe) on the same case where renaming all instances at once is interesting: import java.io.*; public class X { public static void main(String[] args) { for (int i = 0; i < 10; i++) { for (int i = 0; i < 5; i++) { // do something } } } } This may occur when pasting some code, which uses same indice variable 'i'. If being more resilient, rename in file on the dup variable will fix all instances which are in scope.
Need DOM AST tests showing difference, and codeassist/select ones as well. Like here: import java.io.*; public class X { public static void main(String[] args) { for (int i = 0; i < 10; i++) { for (int i = 0; i < 5; i++) { if (i == 0) { // select // do something } } } } } selecting 'i' in "if (i == 0)" statement, and pressing F3 will take to the inner local variable.
David/Olivier - once we fix this, pls provide regression tests for codeselect and DOM AST.
Released for 3.3. Added LocalVariableTest#test013-014 (and tuned a few existing ones)
Changed title to only mention duplicate locals. Entered bug 146768 for duplicate field/method issues.
Released for 3.3 M1
Verified for 3.3 M1 using build I20060807-2000.
David and Olivier, please indicate where did you add Code select and DOM/AST tests, thanks
In the example from comment 1, variable bindings for both x have the same key. I would expect them to have a different key since they are different bindings.
This can be checked using the ASTView.
Comment 14 and comment 15 are bug 149590 and bug 155858.
Added regression tests org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2#test0660/0663
Added regression tests CompletionTests#testDuplicateLocals1() -> testDuplicateLocals5() ResolveTests#testDuplicateLocals1() -> testDuplicateLocals5()
