Skip to content

Commit

Permalink
Allow multiple remove of unused suppress warnings tokens (#1673)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
jjohnstn authored Sep 25, 2024
1 parent 4f73b95 commit 6be9114
Show file tree
Hide file tree
Showing 8 changed files with 445 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
/*******************************************************************************
* Copyright (c) 2024 Red Hat Inc. and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat Inc. - initial API and implementation
*******************************************************************************/
package org.eclipse.jdt.internal.ui.fix;

import java.util.Hashtable;
import java.util.Map;

import org.eclipse.core.runtime.CoreException;

import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.core.compiler.IProblem;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.StringLiteral;

import org.eclipse.jdt.internal.corext.fix.CleanUpConstants;
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.UnusedSuppressWarningsFixCore
*/
public class UnusedSuppressWarningsCleanUp extends AbstractMultiFix {

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

public UnusedSuppressWarningsCleanUp() {
super();
}

private StringLiteral fLiteral;
private CompilationUnit fSavedCompilationUnit= null;

public void setLiteral(StringLiteral literal) {
fLiteral= literal;
}

@Override
public CleanUpRequirements getRequirements() {
boolean requireAST= requireAST();
Map<String, String> requiredOptions= requireAST ? getRequiredOptions() : null;
return new CleanUpRequirements(requireAST, false, false, requiredOptions);
}

private boolean requireAST() {
return isEnabled(CleanUpConstants.REMOVE_UNNECESSARY_SUPPRESS_WARNINGS);
}

@Override
protected ICleanUpFix createFix(CompilationUnit compilationUnit) throws CoreException {
if (compilationUnit == null)
return null;

ICleanUpFix coreFix= UnusedSuppressWarningsFixCore.createAllFix(fSavedCompilationUnit == null ? compilationUnit : fSavedCompilationUnit,
fLiteral);
return coreFix;
}

@Override
protected ICleanUpFix createFix(CompilationUnit compilationUnit, IProblemLocation[] problems) throws CoreException {
if (compilationUnit == null)
return null;

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.ENABLED);
result.put(JavaCore.COMPILER_PB_UNUSED_WARNING_TOKEN, JavaCore.WARNING);
}

return result;
}

@Override
public String[] getStepDescriptions() {
return new String[0];
}

@Override
public String getPreview() {
// not used as traditional cleanup
return ""; //$NON-NLS-1$
}

@Override
public boolean canFix(ICompilationUnit compilationUnit, IProblemLocation problem) {
if (problem.getProblemId() == IProblem.UnusedWarningToken)
return isEnabled(CleanUpConstants.REMOVE_UNNECESSARY_SUPPRESS_WARNINGS);

return false;
}

@Override
public int computeNumberOfFixes(CompilationUnit compilationUnit) {
try {
ICompilationUnit cu= (ICompilationUnit)compilationUnit.getJavaElement();
if (!cu.isStructureKnown())
return 0; //[clean up] 'Remove unnecessary $NLS-TAGS$' removes necessary ones in case of syntax errors: https://bugs.eclipse.org/bugs/show_bug.cgi?id=285814 :
} catch (JavaModelException e) {
return 0;
}

fSavedCompilationUnit= compilationUnit;
int result= 0;
IProblem[] problems= compilationUnit.getProblems();
if (isEnabled(CleanUpConstants.REMOVE_UNNECESSARY_SUPPRESS_WARNINGS))
result+= getNumberOfProblems(problems, IProblem.UnusedWarningToken);

return result;
}
}
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -1465,6 +1465,18 @@ public class CleanUpConstants {
*/
public static final String REMOVE_UNNECESSARY_NLS_TAGS= "cleanup.remove_unnecessary_nls_tags"; //$NON-NLS-1$

/**
* Remove unnecessary SuppressWarnings specifiers.
* <br>
* Possible values: {TRUE, FALSE}<br>
* <br>
*
* @see CleanUpOptions#TRUE
* @see CleanUpOptions#FALSE
* @since 4.34
*/
public static final String REMOVE_UNNECESSARY_SUPPRESS_WARNINGS= "cleanup.remove_unnecessary_suppress_warnings"; //$NON-NLS-1$

/**
* Insert inferred type arguments for diamonds.<br>
* <br>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
/*******************************************************************************
* Copyright (c) 2024 Red Hat Inc. and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat Inc. - initial API and implementation
*******************************************************************************/
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;

import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.core.compiler.IProblem;
import org.eclipse.jdt.core.dom.ASTNode;
import org.eclipse.jdt.core.dom.ArrayInitializer;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.MemberValuePair;
import org.eclipse.jdt.core.dom.NormalAnnotation;
import org.eclipse.jdt.core.dom.SingleMemberAnnotation;
import org.eclipse.jdt.core.dom.StringLiteral;
import org.eclipse.jdt.core.dom.rewrite.ASTRewrite;

import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite;
import org.eclipse.jdt.internal.corext.util.Messages;

import org.eclipse.jdt.ui.text.java.IProblemLocation;

import org.eclipse.jdt.internal.ui.text.correction.CorrectionMessages;
import org.eclipse.jdt.internal.ui.text.correction.ProblemLocation;

/**
* Remove unneeded SuppressWarnings annotations.
*/
public class UnusedSuppressWarningsFixCore extends CompilationUnitRewriteOperationsFixCore {

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

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 (origLiteral == null || literal.getLiteralValue().equals(origLiteral.getLiteralValue())) {
locationsList.add(location);
tokens.add(literal.getLiteralValue());
}
}
}
}
IProblemLocation[] locations= locationsList.toArray(new IProblemLocation[0]);
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);
String label= Messages.format(CorrectionMessages.SuppressWarningsSubProcessor_remove_annotation_label, literal.getLiteralValue());
return createFix(label, compilationUnit, new IProblemLocation[] { problem });
}

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;
}
return new UnusedSuppressWarningsFixCore(label, compilationUnit, new RemoveUnneededSuppressWarningsOperation(problems));
}

private static class RemoveUnneededSuppressWarningsOperation extends CompilationUnitRewriteOperation {

private IProblemLocation[] fLocations;
public RemoveUnneededSuppressWarningsOperation(IProblemLocation[] locations) {
this.fLocations= locations;
}

@Override
public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore linkedModel) throws CoreException {

for (IProblemLocation location : fLocations) {
ASTNode coveringNode= location.getCoveringNode(cuRewrite.getRoot());
if (!(coveringNode instanceof StringLiteral))
continue;

if (coveringNode.getParent() instanceof MemberValuePair) {
coveringNode= coveringNode.getParent();
}

ASTNode parent= coveringNode.getParent();
ASTRewrite rewrite= cuRewrite.getASTRewrite();
if (parent instanceof SingleMemberAnnotation) {
rewrite.remove(parent, null);
} else if (parent instanceof NormalAnnotation) {
NormalAnnotation annot= (NormalAnnotation) parent;
if (annot.values().size() == 1) {
rewrite.remove(annot, null);
} else {
rewrite.remove(coveringNode, null);
}
} else if (parent instanceof ArrayInitializer) {
rewrite.remove(coveringNode, null);
}
}

}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
package org.eclipse.jdt.internal.ui.text.correction;

import java.util.Collection;
import java.util.Hashtable;
import java.util.List;
import java.util.Map;

import org.eclipse.jdt.core.CorrectionEngine;
import org.eclipse.jdt.core.ICompilationUnit;
Expand Down Expand Up @@ -48,12 +50,19 @@

import org.eclipse.jdt.internal.core.manipulation.util.BasicElementLabels;
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.UnusedSuppressWarningsFixCore;
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
import org.eclipse.jdt.internal.corext.util.Messages;

import org.eclipse.jdt.ui.cleanup.CleanUpOptions;
import org.eclipse.jdt.ui.cleanup.ICleanUp;
import org.eclipse.jdt.ui.text.java.IInvocationContext;
import org.eclipse.jdt.ui.text.java.IProblemLocation;

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

public abstract class SuppressWarningsBaseSubProcessor<T> {

static final String ADD_SUPPRESSWARNINGS_ID= "org.eclipse.jdt.ui.correction.addSuppressWarnings"; //$NON-NLS-1$
Expand Down Expand Up @@ -224,30 +233,41 @@ public void getRemoveUnusedSuppressWarningProposals(IInvocationContext context,
}

ASTNode parent= coveringNode.getParent();

ASTRewrite rewrite= ASTRewrite.create(coveringNode.getAST());
if (parent instanceof SingleMemberAnnotation) {
rewrite.remove(parent, null);
} else if (parent instanceof NormalAnnotation) {
NormalAnnotation annot= (NormalAnnotation) parent;
if (annot.values().size() == 1) {
rewrite.remove(annot, null);
} else {
rewrite.remove(coveringNode, null);
}
} else if (parent instanceof ArrayInitializer) {
rewrite.remove(coveringNode, null);
} else {
if (!(parent instanceof SingleMemberAnnotation) && !(parent instanceof NormalAnnotation) && !(parent instanceof ArrayInitializer)) {
return;
}
String label= Messages.format(CorrectionMessages.SuppressWarningsSubProcessor_remove_annotation_label, literal.getLiteralValue());
T proposal= createASTRewriteCorrectionProposal(label, context.getCompilationUnit(), rewrite, IProposalRelevance.REMOVE_ANNOTATION);
proposals.add(proposal);
return;
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);
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);
}
}

protected abstract T createSuppressWarningsProposal(String warningToken, String label, ICompilationUnit cu, ASTNode node, ChildListPropertyDescriptor property, int relevance);

protected abstract T createASTRewriteCorrectionProposal(String name, ICompilationUnit cu, ASTRewrite rewrite, int relevance);

protected abstract T createFixCorrectionProposal(IProposableFix fix, ICleanUp cleanUp, int relevance, IInvocationContext context);
}
Loading

0 comments on commit 6be9114

Please sign in to comment.