From a30b50d36c9bc8f535cf298e336fe51bb6ccc559 Mon Sep 17 00:00:00 2001 From: Jeff Johnston Date: Wed, 25 Sep 2024 16:16:29 -0400 Subject: [PATCH] Allow multiple removal of unused suppress warnings tokens - 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 --- ...ava => UnusedSuppressWarningsCleanUp.java} | 20 ++-- .../text/correction/CorrectionMessages.java | 2 + .../correction/CorrectionMessages.properties | 2 + ...ava => UnusedSuppressWarningsFixCore.java} | 28 +++-- .../SuppressWarningsBaseSubProcessor.java | 26 ++++- .../ui/tests/quickfix/QuickFixTest1d8.java | 107 ++++++++++++++++++ 6 files changed, 163 insertions(+), 22 deletions(-) rename org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/{UnneededSuppressWarningsCleanUp.java => UnusedSuppressWarningsCleanUp.java} (82%) rename org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/{UnneededSuppressWarningsFixCore.java => UnusedSuppressWarningsFixCore.java} (75%) diff --git a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/UnneededSuppressWarningsCleanUp.java b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/UnusedSuppressWarningsCleanUp.java similarity index 82% rename from org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/UnneededSuppressWarningsCleanUp.java rename to org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/UnusedSuppressWarningsCleanUp.java index 11d2cc9472d..5c5b04f7dae 100644 --- a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/UnneededSuppressWarningsCleanUp.java +++ b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/UnusedSuppressWarningsCleanUp.java @@ -26,7 +26,7 @@ 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; @@ -34,15 +34,15 @@ /** * 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 options) { + public UnusedSuppressWarningsCleanUp(Map options) { super(options); } - public UnneededSuppressWarningsCleanUp() { + public UnusedSuppressWarningsCleanUp() { super(); } @@ -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; } @@ -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 getRequiredOptions() { Map 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; } diff --git a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.java b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.java index 99933273cb1..61e00426fb7 100644 --- a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.java +++ b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.java @@ -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; diff --git a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties index 05482decaef..a614b2f6014 100644 --- a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties +++ b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties @@ -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}'' diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/UnneededSuppressWarningsFixCore.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/UnusedSuppressWarningsFixCore.java similarity index 75% rename from org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/UnneededSuppressWarningsFixCore.java rename to org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/UnusedSuppressWarningsFixCore.java index 7664055bc8c..542d599c580 100644 --- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/UnneededSuppressWarningsFixCore.java +++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/UnusedSuppressWarningsFixCore.java @@ -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; @@ -41,36 +43,45 @@ /** * 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 locationsList= new ArrayList<>(); + Set 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()) @@ -78,8 +89,7 @@ public static IProposableFix createFix(CompilationUnit compilationUnit, StringLi } 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 { diff --git a/org.eclipse.jdt.core.manipulation/proposals/org/eclipse/jdt/internal/ui/text/correction/SuppressWarningsBaseSubProcessor.java b/org.eclipse.jdt.core.manipulation/proposals/org/eclipse/jdt/internal/ui/text/correction/SuppressWarningsBaseSubProcessor.java index be07dd24304..382d0417da7 100644 --- a/org.eclipse.jdt.core.manipulation/proposals/org/eclipse/jdt/internal/ui/text/correction/SuppressWarningsBaseSubProcessor.java +++ b/org.eclipse.jdt.core.manipulation/proposals/org/eclipse/jdt/internal/ui/text/correction/SuppressWarningsBaseSubProcessor.java @@ -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; @@ -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 { @@ -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 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 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 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); diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/QuickFixTest1d8.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/QuickFixTest1d8.java index d8182ed33a1..59a1e1f0fa9 100644 --- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/QuickFixTest1d8.java +++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/QuickFixTest1d8.java @@ -2976,4 +2976,111 @@ static Object[] foo() { assertExpectedExistInProposals(proposals, new String[] {expected1, expected2}); } + @Test + public void testIssue1668() throws Exception { + Hashtable 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 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}); + } }