Skip to content

Commit

Permalink
Allow multiple removal of unused suppress warnings tokens
Browse files Browse the repository at this point in the history 
- add new UnusedSuppressWarningsCleanUp and
  UnusedSuppressWarningsFixCore to create FixCorrectionProposals to
  remove a single unused warning token, remove all unused warning
  tokens with a particular name, and remove all unused warning
  tokens
- modify SuppressWarningsBaseSubProcessor to create up to 3 of the
  above fix correction proposals (assuming there are more than 1
  problem flagged for a particular warning token and more than 1
  warning token flagged in problems
- add new test to CleanUpTest1d8
- fixes #1668
  • Loading branch information
@jjohnstn
jjohnstn committed Sep 25, 2024
1 parent 363acce commit a30b50d
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 22 deletions.
20 changes: 11 additions & 9 deletions .../fix/UnneededSuppressWarningsCleanUp.java → ...ui/fix/UnusedSuppressWarningsCleanUp.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,23 @@
import org.eclipse.jdt.core.dom.StringLiteral;

import org.eclipse.jdt.internal.corext.fix.CleanUpConstants;
import org.eclipse.jdt.internal.corext.fix.UnneededSuppressWarningsFixCore;
import org.eclipse.jdt.internal.corext.fix.UnusedSuppressWarningsFixCore;

import org.eclipse.jdt.ui.cleanup.CleanUpRequirements;
import org.eclipse.jdt.ui.cleanup.ICleanUpFix;
import org.eclipse.jdt.ui.text.java.IProblemLocation;

/**
* Create fix to remove unnecessary SuppressWarnings
* @see org.eclipse.jdt.internal.corext.fix.UnneededSuppressWarningsFixCore
* @see org.eclipse.jdt.internal.corext.fix.UnusedSuppressWarningsFixCore
*/
public class UnneededSuppressWarningsCleanUp extends AbstractMultiFix {
public class UnusedSuppressWarningsCleanUp extends AbstractMultiFix {

public UnneededSuppressWarningsCleanUp(Map<String, String> options) {
public UnusedSuppressWarningsCleanUp(Map<String, String> options) {
super(options);
}

public UnneededSuppressWarningsCleanUp() {
public UnusedSuppressWarningsCleanUp() {
super();
}

Expand All @@ -69,7 +69,7 @@ protected ICleanUpFix createFix(CompilationUnit compilationUnit) throws CoreExce
if (compilationUnit == null)
return null;

ICleanUpFix coreFix= UnneededSuppressWarningsFixCore.createFix(fSavedCompilationUnit == null ? compilationUnit : fSavedCompilationUnit,
ICleanUpFix coreFix= UnusedSuppressWarningsFixCore.createAllFix(fSavedCompilationUnit == null ? compilationUnit : fSavedCompilationUnit,
fLiteral);
return coreFix;
}
Expand All @@ -79,15 +79,17 @@ protected ICleanUpFix createFix(CompilationUnit compilationUnit, IProblemLocatio
if (compilationUnit == null)
return null;

ICleanUpFix coreFix= UnneededSuppressWarningsFixCore.createFix(compilationUnit, fLiteral, problems);
ICleanUpFix coreFix= UnusedSuppressWarningsFixCore.createAllFix(compilationUnit, fLiteral);
return coreFix;
}

private Map<String, String> getRequiredOptions() {
Map<String, String> result= new Hashtable<>();

if (isEnabled(CleanUpConstants.REMOVE_UNNECESSARY_SUPPRESS_WARNINGS))
result.put(JavaCore.COMPILER_PB_SUPPRESS_WARNINGS, JavaCore.WARNING);
if (isEnabled(CleanUpConstants.REMOVE_UNNECESSARY_SUPPRESS_WARNINGS)) {
result.put(JavaCore.COMPILER_PB_SUPPRESS_WARNINGS, JavaCore.ENABLED);
result.put(JavaCore.COMPILER_PB_UNUSED_WARNING_TOKEN, JavaCore.WARNING);
}

return result;
}
Expand Down
2 changes: 2 additions & 0 deletions ...e.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,8 @@ private CorrectionMessages() {
public static String LocalCorrectionsSubProcessor_declareSealedAsDirectSuperClass_description;
public static String LocalCorrectionsSubProcessor_declareSubClassAsPermitsSealedClass_description;
public static String SuppressWarningsSubProcessor_fix_suppress_token_label;
public static String SuppressWarningsSubProcessor_remove_all_annotations_label;
public static String SuppressWarningsSubProcessor_remove_any_unused_annotations_label;
public static String SuppressWarningsSubProcessor_remove_annotation_label;
public static String VarargsWarningsSubProcessor_add_safevarargs_label;
public static String VarargsWarningsSubProcessor_add_safevarargs_to_method_label;
Expand Down
2 changes: 2 additions & 0 deletions ...pulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,8 @@ FixCorrectionProposal_WarningAdditionalProposalInfo=Warning:

SuppressWarningsSubProcessor_fix_suppress_token_label=Change to ''{0}''
SuppressWarningsSubProcessor_remove_annotation_label=Remove ''{0}'' token
SuppressWarningsSubProcessor_remove_all_annotations_label=Remove all unnecessary ''{0}'' tokens
SuppressWarningsSubProcessor_remove_any_unused_annotations_label=Remove all unnecessary warning tokens
ModifierCorrectionSubProcessor_changefieldmodifiertononstatic_description=Remove ''static'' modifier of ''{0}''
GetterSetterCorrectionSubProcessor_creategetterunsingencapsulatefield_description=Create getter and setter for ''{0}''...
AddGetterSetter_creategetterssettersfortype_description=Create getters and setters for ''{0}''
Expand Down
28 changes: 19 additions & 9 deletions .../fix/UnneededSuppressWarningsFixCore.java → ...xt/fix/UnusedSuppressWarningsFixCore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
package org.eclipse.jdt.internal.corext.fix;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import org.eclipse.core.runtime.CoreException;

Expand All @@ -41,45 +43,53 @@
/**
* Remove unneeded SuppressWarnings annotations.
*/
public class UnneededSuppressWarningsFixCore extends CompilationUnitRewriteOperationsFixCore {
public class UnusedSuppressWarningsFixCore extends CompilationUnitRewriteOperationsFixCore {

public UnneededSuppressWarningsFixCore(String name, CompilationUnit compilationUnit, CompilationUnitRewriteOperation operation) {
public UnusedSuppressWarningsFixCore(String name, CompilationUnit compilationUnit, CompilationUnitRewriteOperation operation) {
super(name, compilationUnit, operation);
}

public static IProposableFix createFix(CompilationUnit compilationUnit, StringLiteral origLiteral) {
public static IProposableFix createAllFix(CompilationUnit compilationUnit, StringLiteral origLiteral) {
IProblem[] problems= compilationUnit.getProblems();
List<IProblemLocation> locationsList= new ArrayList<>();
Set<String> tokens= new HashSet<>();
for (int i= 0; i < problems.length; i++) {
IProblemLocation location= new ProblemLocation(problems[i]);
if (location.getProblemId() == IProblem.UnusedWarningToken) {
ASTNode node= location.getCoveringNode(compilationUnit);
if (node instanceof StringLiteral literal) {
if (literal.getLiteralValue() == origLiteral.getLiteralValue()) {
if (origLiteral == null || literal.getLiteralValue().equals(origLiteral.getLiteralValue())) {
locationsList.add(location);
tokens.add(literal.getLiteralValue());
}
}
}
}
IProblemLocation[] locations= locationsList.toArray(new IProblemLocation[0]);
return createFix(compilationUnit, origLiteral, locations);
if (locations.length > 1 && (origLiteral != null || tokens.size() > 1)) {
String label= origLiteral == null
? CorrectionMessages.SuppressWarningsSubProcessor_remove_any_unused_annotations_label
: Messages.format(CorrectionMessages.SuppressWarningsSubProcessor_remove_all_annotations_label, origLiteral.getLiteralValue());
return createFix(label, compilationUnit, locations);
}
return null;
}

public static IProposableFix createFix(CompilationUnit compilationUnit, IProblemLocation problem) {
StringLiteral literal= (StringLiteral)problem.getCoveringNode(compilationUnit);
return createFix(compilationUnit, literal, new IProblemLocation[] { problem });
String label= Messages.format(CorrectionMessages.SuppressWarningsSubProcessor_remove_annotation_label, literal.getLiteralValue());
return createFix(label, compilationUnit, new IProblemLocation[] { problem });
}

public static IProposableFix createFix(CompilationUnit compilationUnit, StringLiteral literal, IProblemLocation[] problems) {
private static IProposableFix createFix(String label, CompilationUnit compilationUnit, IProblemLocation[] problems) {
ICompilationUnit cu= (ICompilationUnit)compilationUnit.getJavaElement();
try {
if (!cu.isStructureKnown())
return null;
} catch (JavaModelException e) {
return null;
}
String label= Messages.format(CorrectionMessages.SuppressWarningsSubProcessor_remove_annotation_label, literal.getLiteralValue());
return new UnneededSuppressWarningsFixCore(label, compilationUnit, new RemoveUnneededSuppressWarningsOperation(problems));
return new UnusedSuppressWarningsFixCore(label, compilationUnit, new RemoveUnneededSuppressWarningsOperation(problems));
}

private static class RemoveUnneededSuppressWarningsOperation extends CompilationUnitRewriteOperation {
Expand Down
26 changes: 22 additions & 4 deletions ...oposals/org/eclipse/jdt/internal/ui/text/correction/SuppressWarningsBaseSubProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
import org.eclipse.jdt.internal.corext.dom.ASTNodes;
import org.eclipse.jdt.internal.corext.fix.CleanUpConstants;
import org.eclipse.jdt.internal.corext.fix.IProposableFix;
import org.eclipse.jdt.internal.corext.fix.UnneededSuppressWarningsFixCore;
import org.eclipse.jdt.internal.corext.fix.UnusedSuppressWarningsFixCore;
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
import org.eclipse.jdt.internal.corext.util.Messages;

Expand All @@ -61,7 +61,7 @@
import org.eclipse.jdt.ui.text.java.IInvocationContext;
import org.eclipse.jdt.ui.text.java.IProblemLocation;

import org.eclipse.jdt.internal.ui.fix.UnneededSuppressWarningsCleanUp;
import org.eclipse.jdt.internal.ui.fix.UnusedSuppressWarningsCleanUp;

public abstract class SuppressWarningsBaseSubProcessor<T> {

Expand Down Expand Up @@ -254,11 +254,29 @@ public void getRemoveUnusedSuppressWarningProposals(IInvocationContext context,
if (!(parent instanceof SingleMemberAnnotation) && !(parent instanceof NormalAnnotation) && !(parent instanceof ArrayInitializer)) {
return;
}
IProposableFix fix= UnneededSuppressWarningsFixCore.createFix(context.getASTRoot(), problem);
IProposableFix fix= UnusedSuppressWarningsFixCore.createFix(context.getASTRoot(), problem);
if (fix != null) {
Map<String, String> options= new Hashtable<>();
options.put(CleanUpConstants.REMOVE_UNNECESSARY_SUPPRESS_WARNINGS, CleanUpOptions.TRUE);
UnneededSuppressWarningsCleanUp cleanUp= new UnneededSuppressWarningsCleanUp(options);
UnusedSuppressWarningsCleanUp cleanUp= new UnusedSuppressWarningsCleanUp(options);
cleanUp.setLiteral(literal);
T proposal= createFixCorrectionProposal(fix, cleanUp, IProposalRelevance.REMOVE_ANNOTATION, context);
proposals.add(proposal);
}
fix= UnusedSuppressWarningsFixCore.createAllFix(context.getASTRoot(), literal);
if (fix != null) {
Map<String, String> options= new Hashtable<>();
options.put(CleanUpConstants.REMOVE_UNNECESSARY_SUPPRESS_WARNINGS, CleanUpOptions.TRUE);
UnusedSuppressWarningsCleanUp cleanUp= new UnusedSuppressWarningsCleanUp(options);
cleanUp.setLiteral(literal);
T proposal= createFixCorrectionProposal(fix, cleanUp, IProposalRelevance.REMOVE_ANNOTATION, context);
proposals.add(proposal);
}
fix= UnusedSuppressWarningsFixCore.createAllFix(context.getASTRoot(), null);
if (fix != null) {
Map<String, String> options= new Hashtable<>();
options.put(CleanUpConstants.REMOVE_UNNECESSARY_SUPPRESS_WARNINGS, CleanUpOptions.TRUE);
UnusedSuppressWarningsCleanUp cleanUp= new UnusedSuppressWarningsCleanUp(options);
cleanUp.setLiteral(literal);
T proposal= createFixCorrectionProposal(fix, cleanUp, IProposalRelevance.REMOVE_ANNOTATION, context);
proposals.add(proposal);
Expand Down
107 changes: 107 additions & 0 deletions org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/QuickFixTest1d8.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2976,4 +2976,111 @@ static Object[] foo() {
assertExpectedExistInProposals(proposals, new String[] {expected1, expected2});
}

@Test
public void testIssue1668() throws Exception {
Hashtable<String, String> options = JavaCore.getOptions();
options.put(JavaCore.COMPILER_PB_SUPPRESS_WARNINGS, CompilerOptions.ENABLED);
options.put(JavaCore.COMPILER_PB_UNUSED_WARNING_TOKEN, CompilerOptions.WARNING);
JavaCore.setOptions(options);
IPackageFragment pack2= fSourceFolder.createPackageFragment("test1", false, null);

String str= """
package test1;
public class E {
@SuppressWarnings("restriction")
public boolean simplifyNormal(int x) {
return true;
}
@SuppressWarnings("restriction")
public boolean simplifyCompoundIf(int x) {
return false;
}
@SuppressWarnings("unchecked")
public boolean simplifyNLS(String x) {
return true;
}
public boolean simplifyAll() {
return false;
}
}
""";

ICompilationUnit cu= pack2.createCompilationUnit("E.java", str, false, null);

CompilationUnit astRoot= getASTRoot(cu);
IProblem[] problems= astRoot.getProblems();
assertNumberOfProblems(3, problems);
List<IJavaCompletionProposal> proposals= collectCorrectionsNoCheck(cu, problems[0], null);
assertCorrectLabels(proposals);

String expected1 = """
package test1;
public class E {
public boolean simplifyNormal(int x) {
return true;
}
@SuppressWarnings("restriction")
public boolean simplifyCompoundIf(int x) {
return false;
}
@SuppressWarnings("unchecked")
public boolean simplifyNLS(String x) {
return true;
}
public boolean simplifyAll() {
return false;
}
}
""";

String expected2= """
package test1;
public class E {
public boolean simplifyNormal(int x) {
return true;
}
public boolean simplifyCompoundIf(int x) {
return false;
}
@SuppressWarnings("unchecked")
public boolean simplifyNLS(String x) {
return true;
}
public boolean simplifyAll() {
return false;
}
}
""";

String expected3= """
package test1;
public class E {
public boolean simplifyNormal(int x) {
return true;
}
public boolean simplifyCompoundIf(int x) {
return false;
}
public boolean simplifyNLS(String x) {
return true;
}
public boolean simplifyAll() {
return false;
}
}
""";
assertExpectedExistInProposals(proposals, new String[] {expected1, expected2, expected3});
}
}

0 comments on commit a30b50d

Please sign in to comment.