Skip to content

Commit f0ca579

Browse files
jjohnstnjjohnstn
authored
Add new cleanup to remove unnecessary suppress warnings annotations (eclipse-jdt#2115)
- modify CleanUpRequirements constructor to allow specifying COMPILER_PR_SUPPRESS_WARNINGS enabled if the setting of COMPILER_PB_UNUSED_WARNING_TOKEN is set to error - modify current UnusedSuppressWarningsCleanUp to properly set compiler options and to demand a fresh AST - modify UnusedSuppressWarningsFixCore to handle being used both for quick fixes and a cleanup - add UnusedSuppressWarningsCleanUp to jdt.ui plugin.xml as last clean up and also make it visible via the UnnecessaryCodeTabPage - add new test to CleanUpTest1d8 - fixes eclipse-jdt#2114
1 parent 3a2614a commit f0ca579

File tree

11 files changed

+120
-16
lines changed

11 files changed

+120
-16
lines changed

‎org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ private MultiFixMessages() {
186186
public static String RedundantSemicolonsCleanup_description;
187187
public static String RedundantComparatorCleanUp_description;
188188
public static String UnnecessaryArrayCreationCleanup_description;
189+
public static String UnusedSuppressWarningsCleanup_description;
189190
public static String ArrayWithCurlyCleanup_description;
190191
public static String ReturnExpressionCleanUp_description;
191192
public static String UselessReturnCleanUp_description;

‎org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ OverriddenAssignmentCleanUp_description=Remove overridden assignment
167167
RedundantSemicolonsCleanup_description=Remove redundant semicolons
168168
RedundantComparatorCleanUp_description=Implicit comparator
169169
UnnecessaryArrayCreationCleanup_description=Remove unnecessary array creation for varargs
170+
UnusedSuppressWarningsCleanup_description=Remove unnecessary suppress warning tokens
170171
ArrayWithCurlyCleanup_description=Create array with curly
171172
ReturnExpressionCleanUp_description=Remove variable assignment before return
172173
UselessReturnCleanUp_description=Remove useless return

‎org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/UnusedSuppressWarningsCleanUp.java‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
*******************************************************************************/
1414
package org.eclipse.jdt.internal.ui.fix;
1515

16-
import java.util.Hashtable;
1716
import java.util.Map;
1817

1918
import org.eclipse.core.runtime.CoreException;
@@ -60,7 +59,8 @@ public void setLiteral(StringLiteral literal) {
6059
public CleanUpRequirements getRequirements() {
6160
boolean requireAST= requireAST();
6261
Map<String, String> requiredOptions= requireAST ? getRequiredOptions() : null;
63-
return new CleanUpRequirements(requireAST, false, false, requiredOptions);
62+
// ask for fresh AST as we are setting all default options on and we run as last cleanup
63+
return new CleanUpRequirements(requireAST, requireAST, false, requiredOptions);
6464
}
6565

6666
private boolean requireAST() {
@@ -72,8 +72,8 @@ protected ICleanUpFix createFix(CompilationUnit compilationUnit) throws CoreExce
7272
if (compilationUnit == null)
7373
return null;
7474

75-
ICleanUpFix coreFix= UnusedSuppressWarningsFixCore.createAllFix(fSavedCompilationUnit == null ? compilationUnit : fSavedCompilationUnit,
76-
fLiteral);
75+
ICleanUpFix coreFix= fLiteral != null ? UnusedSuppressWarningsFixCore.createAllFix(fSavedCompilationUnit == null ? compilationUnit : fSavedCompilationUnit,
76+
fLiteral) : UnusedSuppressWarningsFixCore.createAllFix(compilationUnit);
7777
return coreFix;
7878
}
7979

@@ -87,25 +87,44 @@ protected ICleanUpFix createFix(CompilationUnit compilationUnit, IProblemLocatio
8787
}
8888

8989
private Map<String, String> getRequiredOptions() {
90-
Map<String, String> result= new Hashtable<>();
90+
// This cleanup is run last and can require using all default options to maximize
91+
// chance of finding an unused suppress warnings annotation.
92+
// We ask for a fresh AST so all other cleanups are complete before we do this
93+
Map<String, String> result= JavaCore.getOptions();
9194

9295
if (isEnabled(CleanUpConstants.REMOVE_UNNECESSARY_SUPPRESS_WARNINGS)) {
9396
result.put(JavaCore.COMPILER_PB_SUPPRESS_WARNINGS, JavaCore.ENABLED);
94-
result.put(JavaCore.COMPILER_PB_UNUSED_WARNING_TOKEN, JavaCore.WARNING);
97+
result.put(JavaCore.COMPILER_PB_SUPPRESS_OPTIONAL_ERRORS, JavaCore.ENABLED);
98+
result.put(JavaCore.COMPILER_PB_UNUSED_WARNING_TOKEN, JavaCore.ERROR); // do not change to WARNING
9599
}
96100

97101
return result;
98102
}
99103

100104
@Override
101105
public String[] getStepDescriptions() {
106+
if (isEnabled(CleanUpConstants.REMOVE_UNNECESSARY_SUPPRESS_WARNINGS)) {
107+
return new String[] { MultiFixMessages.UnusedSuppressWarningsCleanup_description };
108+
}
109+
102110
return new String[0];
103111
}
104112

105113
@Override
106114
public String getPreview() {
107-
// not used as traditional cleanup
108-
return ""; //$NON-NLS-1$
115+
if (isEnabled(CleanUpConstants.REMOVE_UNNECESSARY_SUPPRESS_WARNINGS)) {
116+
return """
117+
int x = 3;
118+
System.out.println(x);
119+
120+
"""; //$NON-NLS-1$
121+
}
122+
123+
return """
124+
@SuppressWarnings("unused")
125+
int x = 3;
126+
System.out.println(x);
127+
"""; //$NON-NLS-1$
109128
}
110129

111130
@Override

‎org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/ui/cleanup/CleanUpRequirements.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2008, 2019 IBM Corporation and others.
2+
* Copyright (c) 2008, 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
@@ -55,8 +55,12 @@ public CleanUpRequirements(boolean requiresAST, boolean requiresFreshAST, boolea
5555

5656
fCompilerOptions= compilerOptions;
5757
// Make sure that compile warnings are not suppressed since some clean ups work on reported warnings
58-
if (fCompilerOptions != null)
59-
fCompilerOptions.put(JavaCore.COMPILER_PB_SUPPRESS_WARNINGS, JavaCore.DISABLED);
58+
if (fCompilerOptions != null) {
59+
// This should not occur when we are performing a unused suppress warnings cleanup so check
60+
// for an option which indicates we are doing this.
61+
if (!JavaCore.ERROR.equals(fCompilerOptions.get(JavaCore.COMPILER_PB_UNUSED_WARNING_TOKEN)))
62+
fCompilerOptions.put(JavaCore.COMPILER_PB_SUPPRESS_WARNINGS, JavaCore.DISABLED);
63+
}
6064
}
6165

6266
/**

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,14 @@ public UnusedSuppressWarningsFixCore(String name, CompilationUnit compilationUni
5050
}
5151

5252
public static IProposableFix createAllFix(CompilationUnit compilationUnit, StringLiteral origLiteral) {
53+
return createAllFix(compilationUnit, origLiteral, true);
54+
}
55+
56+
public static IProposableFix createAllFix(CompilationUnit compilationUnit) {
57+
return createAllFix(compilationUnit, null, false);
58+
}
59+
60+
private static IProposableFix createAllFix(CompilationUnit compilationUnit, StringLiteral origLiteral, boolean isQuickFix) {
5361
IProblem[] problems= compilationUnit.getProblems();
5462
List<IProblemLocation> locationsList= new ArrayList<>();
5563
Set<String> tokens= new HashSet<>();
@@ -66,10 +74,15 @@ public static IProposableFix createAllFix(CompilationUnit compilationUnit, Strin
6674
}
6775
}
6876
IProblemLocation[] locations= locationsList.toArray(new IProblemLocation[0]);
69-
if (locations.length > 1 && (origLiteral != null || tokens.size() > 1)) {
70-
String label= origLiteral == null
71-
? CorrectionMessages.SuppressWarningsSubProcessor_remove_any_unused_annotations_label
72-
: Messages.format(CorrectionMessages.SuppressWarningsSubProcessor_remove_all_annotations_label, origLiteral.getLiteralValue());
77+
if (isQuickFix) {
78+
if (locations.length > 1 && (origLiteral != null || tokens.size() > 1)) {
79+
String label= origLiteral == null
80+
? CorrectionMessages.SuppressWarningsSubProcessor_remove_any_unused_annotations_label
81+
: Messages.format(CorrectionMessages.SuppressWarningsSubProcessor_remove_all_annotations_label, origLiteral.getLiteralValue());
82+
return createFix(label, compilationUnit, locations);
83+
}
84+
} else if (locations.length > 0){
85+
String label= CorrectionMessages.SuppressWarningsSubProcessor_remove_any_unused_annotations_label;
7386
return createFix(label, compilationUnit, locations);
7487
}
7588
return null;

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7101,4 +7101,56 @@ void test1() {
71017101
assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1 }, new String[] { expected }, null);
71027102

71037103
}
7104+
7105+
@Test
7106+
public void testRemoveSuppressWarnings() throws Exception {
7107+
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
7108+
String original= """
7109+
package test1;
7110+
7111+
public class E {
7112+
@SuppressWarnings({ "unused" })
7113+
private int foo(Object x, Object y, boolean b) {
7114+
if (b || !(x instanceof String)) {
7115+
if (!(y instanceof Double)) {
7116+
return 6;
7117+
}
7118+
@SuppressWarnings("unchecked")
7119+
Double d = (Double)y;
7120+
System.out.println(d.isNaN());
7121+
return 7;
7122+
}
7123+
@SuppressWarnings("unchecked")
7124+
String s = (String)x;
7125+
return s.length();
7126+
}
7127+
}
7128+
""";
7129+
ICompilationUnit cu1= pack1.createCompilationUnit("E.java", original, false, null);
7130+
7131+
enable(CleanUpConstants.REMOVE_UNNECESSARY_SUPPRESS_WARNINGS);
7132+
7133+
String expected= """
7134+
package test1;
7135+
7136+
public class E {
7137+
@SuppressWarnings({ "unused" })
7138+
private int foo(Object x, Object y, boolean b) {
7139+
if (b || !(x instanceof String)) {
7140+
if (!(y instanceof Double)) {
7141+
return 6;
7142+
}
7143+
Double d = (Double)y;
7144+
System.out.println(d.isNaN());
7145+
return 7;
7146+
}
7147+
String s = (String)x;
7148+
return s.length();
7149+
}
7150+
}
7151+
""";
7152+
7153+
assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1 }, new String[] { expected }, null);
7154+
7155+
}
71047156
}

‎org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstantsOptions.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ private static void setEclipseDefaultSettings(CleanUpOptions options) {
105105
// Unnecessary Code
106106
options.setOption(REMOVE_UNNECESSARY_CASTS, CleanUpOptions.TRUE);
107107
options.setOption(REMOVE_UNNECESSARY_NLS_TAGS, CleanUpOptions.TRUE);
108+
options.setOption(REMOVE_UNNECESSARY_SUPPRESS_WARNINGS, CleanUpOptions.FALSE);
108109
options.setOption(INSERT_INFERRED_TYPE_ARGUMENTS, CleanUpOptions.FALSE);
109110
options.setOption(SUBSTRING, CleanUpOptions.FALSE);
110111
options.setOption(ARRAYS_FILL, CleanUpOptions.FALSE);
@@ -292,6 +293,7 @@ private static void setSaveParticipantSettings(CleanUpOptions options) {
292293
// Unnecessary Code
293294
options.setOption(REMOVE_UNNECESSARY_CASTS, CleanUpOptions.TRUE);
294295
options.setOption(REMOVE_UNNECESSARY_NLS_TAGS, CleanUpOptions.FALSE);
296+
options.setOption(REMOVE_UNNECESSARY_SUPPRESS_WARNINGS, CleanUpOptions.FALSE);
295297
options.setOption(INSERT_INFERRED_TYPE_ARGUMENTS, CleanUpOptions.FALSE);
296298
options.setOption(SUBSTRING, CleanUpOptions.FALSE);
297299
options.setOption(ARRAYS_FILL, CleanUpOptions.FALSE);

‎org.eclipse.jdt.ui/plugin.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7561,6 +7561,11 @@
75617561
id="org.eclipse.jdt.ui.cleanup.rename_unused"
75627562
runAfter="org.eclipse.jdt.ui.cleanup.unboxing">
75637563
</cleanUp>
7564+
<cleanUp
7565+
class="org.eclipse.jdt.internal.ui.fix.UnusedSuppressWarningsCleanUp"
7566+
id="org.eclipse.jdt.ui.cleanup.remove_unused_suppress_warnings"
7567+
runAfter="org.eclipse.jdt.ui.cleanup.rename_unused">
7568+
</cleanUp>
75647569
</extension>
75657570

75667571
<extension

‎org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ public class CleanUpMessages extends NLS {
134134

135135
public static String UnnecessaryCodeTabPage_CheckboxName_UnnecessaryCasts;
136136
public static String UnnecessaryCodeTabPage_CheckboxName_UnnecessaryNLSTags;
137+
public static String UnnecessaryCodeTabPage_CheckboxName_UnnecessarySuppressWarnings;
137138
public static String UnnecessaryCodeTabPage_CheckboxName_Substring;
138139
public static String UnnecessaryCodeTabPage_CheckboxName_ArraysFill;
139140
public static String UnnecessaryCodeTabPage_CheckboxName_EvaluateNullable;

‎org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ UnnecessaryCodeTabPage_CheckboxName_UnusedLocalVariables=Remove unused &local va
121121
UnnecessaryCodeTabPage_GroupName_UnnecessaryCode=Unnecessary code
122122
UnnecessaryCodeTabPage_CheckboxName_UnnecessaryCasts=Remove unnecessar&y casts
123123
UnnecessaryCodeTabPage_CheckboxName_UnnecessaryNLSTags=Remove unnecessary '$NON-NLS$' ta&gs
124+
UnnecessaryCodeTabPage_CheckboxName_UnnecessarySuppressWarnings=Remove unnecessary suppress warning tokens
124125
UnnecessaryCodeTabPage_CheckboxName_Substring=Remove redundant String.substring() parameter
125126
UnnecessaryCodeTabPage_CheckboxName_ArraysFill=Use Arrays.&fill() when possible
126127
UnnecessaryCodeTabPage_CheckboxName_EvaluateNullable=Evaluate without null check

0 commit comments

Comments
 (0)