Skip to content

Commit f6689fe

Browse files
jjohnstnjjohnstn
authored
Do not add always add final field qualifier when compilation unit has errors (#2167)
- modify VariableDeclarationFixCore to not add final qualifier when there are recovered bindings in the file and in the case where a binding is not created, keep track of the simple name and don't add final qualifier to any variable that matches these unresolved names - add new test to CleanUpTest - fix some AssistQuickFixTest tests which reference wrong package - fixes #2166
1 parent 0bbc4ad commit f6689fe

File tree

3 files changed

+83
-40
lines changed

3 files changed

+83
-40
lines changed

‎org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/VariableDeclarationFixCore.java‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2019, 2023 IBM Corporation and others.
2+
* Copyright (c) 2019, 2025 IBM Corporation and others.
33
*
44
* This program and the accompanying materials
55
* are made available under the terms of the Eclipse Public License 2.0
@@ -66,9 +66,11 @@ public class VariableDeclarationFixCore extends CompilationUnitRewriteOperations
6666
public static class WrittenNamesFinder extends GenericVisitor {
6767

6868
private final HashMap<IBinding, List<SimpleName>> fResult;
69+
private final HashSet<String> fUnresolved;
6970

70-
public WrittenNamesFinder(HashMap<IBinding, List<SimpleName>> result) {
71+
public WrittenNamesFinder(HashMap<IBinding, List<SimpleName>> result, HashSet<String> unresolved) {
7172
fResult= result;
73+
fUnresolved= unresolved;
7274
}
7375

7476
@Override
@@ -79,6 +81,11 @@ public boolean visit(SimpleName node) {
7981
return super.visit(node);
8082

8183
IBinding binding= node.resolveBinding();
84+
if (binding == null) {
85+
fUnresolved.add(node.getFullyQualifiedName());
86+
} else if (binding.isRecovered()) {
87+
throw new AbortSearchException();
88+
}
8289
if (!(binding instanceof IVariableBinding))
8390
return super.visit(node);
8491

@@ -111,21 +118,24 @@ public static class VariableDeclarationFinder extends GenericVisitor {
111118

112119
private final List<ModifierChangeOperation> fResult;
113120
private final HashMap<IBinding, List<SimpleName>> fWrittenVariables;
121+
private final HashSet<String> fUnresolved;
114122
private final boolean fAddFinalFields;
115123
private final boolean fAddFinalParameters;
116124
private final boolean fAddFinalLocals;
117125

118126
public VariableDeclarationFinder(boolean addFinalFields,
119127
boolean addFinalParameters,
120128
boolean addFinalLocals,
121-
final List<ModifierChangeOperation> result, final HashMap<IBinding, List<SimpleName>> writtenNames) {
129+
final List<ModifierChangeOperation> result, final HashMap<IBinding, List<SimpleName>> writtenNames,
130+
final HashSet<String> unresolved) {
122131

123132
super();
124133
fAddFinalFields= addFinalFields;
125134
fAddFinalParameters= addFinalParameters;
126135
fAddFinalLocals= addFinalLocals;
127136
fResult= result;
128137
fWrittenVariables= writtenNames;
138+
fUnresolved= unresolved;
129139
}
130140

131141
@Override
@@ -212,7 +222,7 @@ private boolean fieldCanBeFinal(VariableDeclarationFragment fragment, IVariableB
212222
if (Modifier.isStatic(((FieldDeclaration)fragment.getParent()).getModifiers()))
213223
return false;
214224

215-
if (!fWrittenVariables.containsKey(binding)) {
225+
if (!fWrittenVariables.containsKey(binding) && !fUnresolved.contains(binding.getName())) {
216226
//variable is not written
217227
if (fragment.getInitializer() == null) {//variable is not initialized
218228
return false;
@@ -524,10 +534,15 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore
524534

525535
public static VariableDeclarationFixCore createChangeModifierToFinalFix(final CompilationUnit compilationUnit, ASTNode[] selectedNodes) {
526536
HashMap<IBinding, List<SimpleName>> writtenNames= new HashMap<>();
527-
WrittenNamesFinder finder= new WrittenNamesFinder(writtenNames);
528-
compilationUnit.accept(finder);
537+
HashSet<String> unresolved= new HashSet<>();
538+
WrittenNamesFinder finder= new WrittenNamesFinder(writtenNames, unresolved);
539+
try {
540+
compilationUnit.accept(finder);
541+
} catch (AbortSearchException e) {
542+
return null;
543+
}
529544
List<ModifierChangeOperation> ops= new ArrayList<>();
530-
VariableDeclarationFinder visitor= new VariableDeclarationFinder(true, true, true, ops, writtenNames);
545+
VariableDeclarationFinder visitor= new VariableDeclarationFinder(true, true, true, ops, writtenNames, unresolved);
531546
if (selectedNodes.length == 1) {
532547
selectedNodes[0].accept(visitor);
533548
} else {
@@ -555,11 +570,17 @@ public static ICleanUpFix createCleanUp(CompilationUnit compilationUnit,
555570
return null;
556571

557572
HashMap<IBinding, List<SimpleName>> writtenNames= new HashMap<>();
558-
WrittenNamesFinder finder= new WrittenNamesFinder(writtenNames);
559-
compilationUnit.accept(finder);
573+
HashSet<String> unresolved= new HashSet<>();
574+
WrittenNamesFinder finder= new WrittenNamesFinder(writtenNames, unresolved);
575+
try {
576+
compilationUnit.accept(finder);
577+
} catch (AbortSearchException e) {
578+
return null;
579+
}
560580

561581
List<ModifierChangeOperation> operations= new ArrayList<>();
562-
VariableDeclarationFinder visitor= new VariableDeclarationFinder(addFinalFields, addFinalParameters, addFinalLocals, operations, writtenNames);
582+
VariableDeclarationFinder visitor= new VariableDeclarationFinder(addFinalFields, addFinalParameters, addFinalLocals, operations,
583+
writtenNames, unresolved);
563584
compilationUnit.accept(visitor);
564585

565586
if (operations.isEmpty())

‎org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest.java‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6723,7 +6723,7 @@ private void foo() {
67236723
public void testMakeFinal02() throws Exception {
67246724
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
67256725
String str= """
6726-
package test;
6726+
package test1;
67276727
public class E {
67286728
private final int i= 0;
67296729
private void foo() {
@@ -6749,7 +6749,7 @@ private void foo() {
67496749
public void testMakeFinal03() throws Exception {
67506750
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
67516751
String str= """
6752-
package test;
6752+
package test1;
67536753
public class E {
67546754
public int i= 0;
67556755
private void foo() {
@@ -6767,7 +6767,7 @@ private void foo() {
67676767
assertProposalDoesNotExist(proposals, CHANGE_MODIFIER_TO_FINAL);
67686768

67696769
String expected1= """
6770-
package test;
6770+
package test1;
67716771
public class E {
67726772
private int i= 0;
67736773
private void foo() {
@@ -6788,7 +6788,7 @@ public void setI(int i) {
67886788
public void testMakeFinal04() throws Exception {
67896789
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
67906790
String str= """
6791-
package test;
6791+
package test1;
67926792
public class E {
67936793
private void foo() {
67946794
int i= 0, j= 0;
@@ -6808,7 +6808,7 @@ private void foo() {
68086808
assertCorrectLabels(proposals);
68096809

68106810
String str1= """
6811-
package test;
6811+
package test1;
68126812
public class E {
68136813
private void foo() {
68146814
final int i= 0;
@@ -6825,7 +6825,7 @@ private void foo() {
68256825
public void testMakeFinal05() throws Exception {
68266826
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
68276827
String str= """
6828-
package test;
6828+
package test1;
68296829
public class E {
68306830
private void foo(int i, int j) {
68316831
System.out.println(i);
@@ -6844,7 +6844,7 @@ private void foo(int i, int j) {
68446844
assertCorrectLabels(proposals);
68456845

68466846
String str1= """
6847-
package test;
6847+
package test1;
68486848
public class E {
68496849
private void foo(final int i, int j) {
68506850
System.out.println(i);
@@ -6859,7 +6859,7 @@ private void foo(final int i, int j) {
68596859
public void testMakeFinal06() throws Exception {
68606860
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
68616861
String str= """
6862-
package test;
6862+
package test1;
68636863
public class E {
68646864
public void foo() {
68656865
int i= 0;
@@ -6885,7 +6885,7 @@ public void foo() {
68856885
public void testMakeFinal07() throws Exception {
68866886
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
68876887
String str= """
6888-
package test;
6888+
package test1;
68896889
public class E {
68906890
private int i= 0;
68916891
public void foo() {
@@ -6911,7 +6911,7 @@ public void set(int i) {
69116911
public void testMakeFinal08() throws Exception {
69126912
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
69136913
String str= """
6914-
package test;
6914+
package test1;
69156915
public class E {
69166916
private int i= 0;
69176917
public void foo() {
@@ -6939,7 +6939,7 @@ public void reset() {
69396939
public void testMakeFinal09() throws Exception {
69406940
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
69416941
String str= """
6942-
package test;
6942+
package test1;
69436943
public class E {
69446944
private int i= 0;
69456945
public void foo() {
@@ -6967,7 +6967,7 @@ public void reset() {
69676967
public void testMakeFinal10() throws Exception {
69686968
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
69696969
String str= """
6970-
package test;
6970+
package test1;
69716971
public class E {
69726972
private int i= 0;
69736973
public void foo() {
@@ -6995,7 +6995,7 @@ public void reset() {
69956995
public void testMakeFinal11() throws Exception {
69966996
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
69976997
String str= """
6998-
package test;
6998+
package test1;
69996999
public class E {
70007000
public void foo() {
70017001
for (int j= 0, i= 0; j < (new int[0]).length; j++) {
@@ -7021,7 +7021,7 @@ public void foo() {
70217021
public void testMakeFinal12() throws Exception {
70227022
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
70237023
String str= """
7024-
package test;
7024+
package test1;
70257025
public class E {
70267026
public void foo() {
70277027
int i= 1, j= i + 1, h= j + 1;
@@ -7042,7 +7042,7 @@ public void foo() {
70427042
assertCorrectLabels(proposals);
70437043

70447044
String str1= """
7045-
package test;
7045+
package test1;
70467046
public class E {
70477047
public void foo() {
70487048
final int i= 1;
@@ -7060,7 +7060,7 @@ public void foo() {
70607060
public void testMakeFinal13() throws Exception {
70617061
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
70627062
String str= """
7063-
package test;
7063+
package test1;
70647064
public class E {
70657065
public void foo() {
70667066
int i= 1, j= i + 1, h= j + 1;
@@ -7081,7 +7081,7 @@ public void foo() {
70817081
assertCorrectLabels(proposals);
70827082

70837083
String ex1= """
7084-
package test;
7084+
package test1;
70857085
public class E {
70867086
public void foo() {
70877087
int i= 1;
@@ -7095,7 +7095,7 @@ public void foo() {
70957095
""";
70967096

70977097
String ex2= """
7098-
package test;
7098+
package test1;
70997099
public class E {
71007100
public void foo() {
71017101
int i= 1, j, h= j + 1;
@@ -7114,7 +7114,7 @@ public void foo() {
71147114
public void testMakeFinal14() throws Exception {
71157115
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
71167116
String str= """
7117-
package test;
7117+
package test1;
71187118
public class E {
71197119
public void foo() {
71207120
int i= 1, j= i + 1, h= j + 1;
@@ -7135,7 +7135,7 @@ public void foo() {
71357135
assertCorrectLabels(proposals);
71367136

71377137
String ex1= """
7138-
package test;
7138+
package test1;
71397139
public class E {
71407140
public void foo() {
71417141
int i= 1, j= i + 1;
@@ -7148,7 +7148,7 @@ public void foo() {
71487148
""";
71497149

71507150
String ex2= """
7151-
package test;
7151+
package test1;
71527152
public class E {
71537153
public void foo() {
71547154
int i= 1, j= i + 1, h;
@@ -7167,7 +7167,7 @@ public void foo() {
71677167
public void testMakeFinal15() throws Exception {
71687168
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
71697169
String str= """
7170-
package test;
7170+
package test1;
71717171
import java.io.Serializable;
71727172
public class E {
71737173
public void foo() {
@@ -7193,7 +7193,7 @@ public void foo() {
71937193
assertCorrectLabels(proposals);
71947194

71957195
String expected1= """
7196-
package test;
7196+
package test1;
71977197
import java.io.Serializable;
71987198
public class E {
71997199
public void foo() {
@@ -7218,7 +7218,7 @@ public void foo() {
72187218
public void testMakeFinal16() throws Exception {
72197219
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
72207220
String str= """
7221-
package test;
7221+
package test1;
72227222
public class E {
72237223
public void foo() {
72247224
int i= 0;
@@ -7241,7 +7241,7 @@ public void foo() {
72417241
public void testMakeFinal17() throws Exception {
72427242
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
72437243
String str= """
7244-
package test;
7244+
package test1;
72457245
public class E {
72467246
private int i= 0;
72477247
private void foo() {
@@ -7259,7 +7259,7 @@ private void foo() {
72597259
assertProposalDoesNotExist(proposals, CHANGE_MODIFIER_TO_FINAL);
72607260

72617261
String str1= """
7262-
package test;
7262+
package test1;
72637263
public class E {
72647264
private int i= 0;
72657265
private void foo() {
@@ -7280,7 +7280,7 @@ public void setI(int i) {
72807280
public void testMakeFinal18() throws Exception {
72817281
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
72827282
String str= """
7283-
package test;
7283+
package test1;
72847284
public class E {
72857285
private int i= 0;
72867286
private void foo() {
@@ -7305,7 +7305,7 @@ private void foo() {
73057305
public void testMakeFinal19() throws Exception {
73067306
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
73077307
String str= """
7308-
package test;
7308+
package test1;
73097309
public class E {
73107310
private void foo() {
73117311
int i= 0;
@@ -7328,7 +7328,7 @@ private void foo() {
73287328
public void testMakeFinalBug148373() throws Exception {
73297329
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
73307330
String str= """
7331-
package test;
7331+
package test1;
73327332
public class E {
73337333
public void foo(Integer i) {
73347334
}
@@ -7345,7 +7345,7 @@ public void foo(Integer i) {
73457345
assertCorrectLabels(proposals);
73467346

73477347
String str1= """
7348-
package test;
7348+
package test1;
73497349
public class E {
73507350
public void foo(final Integer i) {
73517351
}

0 commit comments

Comments
 (0)