Miscellaneous cleanups.

This commit is contained in:
Archie L. Cobbs 2025-08-20 12:10:34 -05:00
parent 5c2e51877c
commit 07b0cd2d3d
3 changed files with 68 additions and 77 deletions

View File

@ -230,7 +230,7 @@ public class Lint {
* <p>
* This category is not supported by {@code @SuppressWarnings}.
*/
CLASSFILE("classfile", false),
CLASSFILE("classfile", false, true),
/**
* Warn about "dangling" documentation comments,
@ -285,7 +285,7 @@ public class Lint {
* <p>
* This category is not supported by {@code @SuppressWarnings}.
*/
INCUBATING("incubating", false),
INCUBATING("incubating", false, true),
/**
* Warn about compiler possible lossy conversions.
@ -322,7 +322,7 @@ public class Lint {
* <p>
* This category is not supported by {@code @SuppressWarnings}.
*/
OUTPUT_FILE_CLASH("output-file-clash", false),
OUTPUT_FILE_CLASH("output-file-clash", false, true),
/**
* Warn about issues regarding method overloads.
@ -427,11 +427,7 @@ public class Lint {
RESTRICTED("restricted");
LintCategory(String option) {
this(option, true);
}
LintCategory(String option, boolean annotationSuppression) {
this(option, annotationSuppression, true);
this(option, true, true);
}
LintCategory(String option, boolean annotationSuppression, boolean suppressionTracking, String... aliases) {
@ -480,8 +476,8 @@ public class Lint {
}
/**
* Determine whether warnings in the given category should be calculated, because either
* (a) the category is enabled, or (b) lint category {@code "suppression"} is enabled.
* Determine whether warnings in the given category should be calculated, because
* the category is either (a) enabled or (b) being tracked for unnecessary suppression.
*
* <p>
* Use of this method is never required; it simply helps avoid potentially useless work.
@ -497,7 +493,7 @@ public class Lint {
* the SuppressWarnings annotation.
*
* <p>
* This method also optionally validates any warning suppressions currently in scope.
* This method also optionally validates any warning suppression currently in scope.
* If you just want to know the configuration of this instance, set {@code validate} to false.
* If you are using the result of this method to control whether a warning is actually
* generated, then set {@code validate} to true to ensure that any suppression of the
@ -520,7 +516,7 @@ public class Lint {
* current entity being itself deprecated.
*
* <p>
* This method also optionally validates any warning suppressions currently in scope.
* This method also optionally validates any warning suppression currently in scope.
* If you just want to know the configuration of this instance, set {@code validate} to false.
* If you are using the result of this method to control whether a warning is actually
* generated, then set {@code validate} to true to ensure that any suppression of the
@ -610,13 +606,13 @@ public class Lint {
}
/**
* Determine whether we should bother tracking suppression validation for the given lint category.
* Determine whether we should be tracking suppression validation for the given lint category.
*
* <p>
* We need to track validation of suppression of a lint category if:
* We need to track validation of the suppression of a lint category if:
* <ul>
* <li>It's supported by {@code "suppression"} suppression tracking
* <li>Category {@code "suppression"} is currently enabled
* <li>Lint category {@code "suppression"} is currently enabled
* </ul>
*/
private boolean needsSuppressionTracking(LintCategory lc) {

View File

@ -36,7 +36,6 @@ import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@ -45,7 +44,6 @@ import javax.tools.JavaFileObject;
import com.sun.tools.javac.code.Lint.LintCategory;
import com.sun.tools.javac.code.Symbol.VarSymbol;
import com.sun.tools.javac.main.Option;
import com.sun.tools.javac.resources.CompilerProperties.LintWarnings;
import com.sun.tools.javac.tree.EndPosTable;
import com.sun.tools.javac.tree.JCTree;
@ -56,12 +54,6 @@ import com.sun.tools.javac.util.Assert;
import com.sun.tools.javac.util.Context;
import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
import com.sun.tools.javac.util.Log;
import com.sun.tools.javac.util.Names;
import com.sun.tools.javac.util.Options;
import static com.sun.tools.javac.code.Lint.LintCategory.DEPRECATION;
import static com.sun.tools.javac.code.Lint.LintCategory.OPTIONS;
import static com.sun.tools.javac.code.Lint.LintCategory.SUPPRESSION;
/**
* Maps source code positions to the applicable {@link Lint} instance.
@ -77,35 +69,39 @@ import static com.sun.tools.javac.code.Lint.LintCategory.SUPPRESSION;
* if it can't be determined yet, an empty {@link Optional} is returned.
*
* <p>
* This class also tracks which {@code @SuppressWarnings} suppressions actually suppress something.
* Those that don't are unnecessary and trigger warnings in the {@code "suppression"} lint category.
* For this to work, this class must be notified any time a warning that is currently suppressed would
* have been reported; this is termed the "validation" of the suppression. That notification happens
* via {@link #validateSuppression}.
* This class also tracks which {@code @SuppressWarnings} suppressions actually suppress something;
* any that don't are unnecessary and will generate warnings in the {@code "suppression"} category.
* For this to work, this class must be notified any time a warning that is currently suppressed
* <i>would have</i> been reported; this is termed the "validation" of the suppression. That notification
* happens by invoking {@link #validateSuppression}.
*
* <p>
* Validation events "bubble up" the source tree until they are "caught" by a {@code @SuppressWarnings}
* annotation or they escape the file entirely. Being "caught" validates that suppression.
* A suppression that is never validated is unnecessary.
* Validation events "bubble up" the source tree until either they are "caught" by a {@code @SuppressWarnings}
* annotation, or they escape the file entirely. Being "caught" validates the corresponding suppression.
* A suppression that is never caught (i.e., never validated) is unnecessary.
*
* <p>
* Additional observations and corner cases:
* Some additional nuances:
* <ul>
* <li>Lint warnings can be suppressed at a module, package, class, method, or variable declaration
* (via {@code @SuppressWarnings}), or globally (via {@code -Xlint:-key}).
* <li>Consequently, an unnecessary suppression warning can only be emitted at one of those declarations.
* <li>Some categories (e.g., {@code classfile}) don't support suppression via {@code @SuppressWarnings}.
* These can only generate warnings at the global level (and therefore any {@code @SuppressWarnings}
* annotation is always unnecessary).
* <li>Some categories are never tracked for suppression, e.g., {@code options}, {@code path}, and the
* suppression category {@code "suppression"} itself.
* <li>{@code @SuppressWarnings("suppression")} is perfectly valid: it means unnecessary suppression
* warnings will never be reported for any lint category suppressed by that annotation or by any
* {@code @SuppressWarnings} annotation nested within the scope of its declaration.
* (via {@code @SuppressWarnings}), or globally on the command line via {@code -Xlint:-key}.
* <li>Consequently, unnecessary suppression warnings can only be emitted at {@code @SuppressWarnings}
* annotations. Currently, we don't report unnecessary suppression via {@code -Xlint:-key} flags.
* <li>Some lint categories don't support suppression via the {@code @SuppressWarnings} annotations
* (e.g., {@code "classfile"}). Specifying such a category in a {@code @SuppressWarnings} annotation
* is always unnecessary and will trigger an unnecessary suppression warning (if enabled).
* <li>Some categories are not tracked for unnecessary suppression, e.g., {@code "options"}, {@code "path"},
* and {@code "suppression"} itself. Specifying such a category in a {@code @SuppressWarnings} annotation
* will never generate an unnecessary suppression warning.
* <li>{@code @SuppressWarnings("suppression")} is perfectly valid and reasonable: it means unnecessary
* suppression warnings will never be reported for any lint category suppressed by that annotation
* or by any {@code @SuppressWarnings} annotation nested within the scope of its declaration.
* </ul>
*
* <p><b>This is NOT part of any supported API. If you write code that depends on this, you do so at your
* own risk. This code and its internal interfaces are subject to change or deletion without notice.</b>
* <p><b>This is NOT part of any supported API.
* If you write code that depends on this, you do so at your own risk.
* This code and its internal interfaces are subject to change or
* deletion without notice.</b>
*/
public class LintMapper {
@ -119,9 +115,9 @@ public class LintMapper {
private final Context context;
// These are initialized lazily; see initializeIfNeeded()
private Log log;
private Lint rootLint;
private Symtab syms;
private Log log;
/**
* Obtain the {@link LintMapper} context singleton.
@ -145,9 +141,9 @@ public class LintMapper {
// Lazy initialization to avoid dependency loops
private void initializeIfNeeded() {
if (rootLint == null) {
log = Log.instance(context);
rootLint = Lint.instance(context);
syms = Symtab.instance(context);
log = Log.instance(context);
}
}
@ -185,7 +181,7 @@ public class LintMapper {
*/
public void calculateLints(JavaFileObject sourceFile, JCTree tree, EndPosTable endPositions) {
Assert.check(rootLint != null);
fileInfoMap.get(sourceFile).afterAttr(syms, tree, endPositions);
fileInfoMap.get(sourceFile).afterAttr(tree, endPositions, syms);
}
/**
@ -216,25 +212,24 @@ public class LintMapper {
// Suppression Tracking
/**
* Validate the given lint category within the scope of the given symbol's declaration (or globally if symbol is null).
* Validate the given lint category within the scope of the given symbol's declaration.
*
* <p>
* This is to indicate that, if the category is being suppressed, a warning would have otherwise been generated.
* This indicates that any suppression of {@code category} currently in scope is necesssary.
*
* @param symbol innermost {@code @SuppressWarnings}-annotated symbol in scope, or null for global scope
* @param category lint category to validate
*/
public void validateSuppression(Symbol symbol, LintCategory category) {
if (symbol != null) {
if (symbol != null)
fileInfoMap.get(log.currentSourceFile()).validationsFor(symbol).add(category);
}
}
/**
* Warn about unnecessary {@code @SuppressWarnings} suppressions within the given top-level declaration.
*
* <p>
* This step must be done after the given source file has been warned about.
* All warnings within {@code tree} must have already been calculated when this method is invoked.
*
* @param sourceFile source file
* @param tree top level declaration
@ -243,28 +238,27 @@ public class LintMapper {
// Anything to do here?
initializeIfNeeded();
if (!rootLint.isEnabled(SUPPRESSION, false)) {
if (!rootLint.isEnabled(LintCategory.SUPPRESSION, false))
return;
}
// Find the LintRange corresponding to "tree"
FileInfo fileInfo = fileInfoMap.get(sourceFile);
LintRange lintRange = fileInfo.rootRange.findChild(tree.pos());
// Propagate validations within the top-level declaration to determine which suppressions never got validated
// Propagate validations within "tree" to determine which suppressions therein never got validated
fileInfo.propagateValidations(lintRange);
// Report unvalidated suppresions
// Report unvalidated suppresions, except where SUPPRESSION is itself suppressed
lintRange.stream()
.filter(node -> node.lint.isEnabled(SUPPRESSION, false))
.filter(node -> node.lint.isEnabled(LintCategory.SUPPRESSION, false))
.forEach(node -> {
String unvalidatedNames = node.unvalidated.stream()
String unnecessaryCategoryNames = node.unvalidated.stream()
.filter(lc -> lc.suppressionTracking)
.map(category -> category.option)
.map(name -> "\"" + name + "\"")
.collect(Collectors.joining(", "));
if (!unvalidatedNames.isEmpty())
log.warning(node.annotation.pos(), LintWarnings.UnnecessaryWarningSuppression(unvalidatedNames));
if (!unnecessaryCategoryNames.isEmpty())
log.warning(node.annotation.pos(), LintWarnings.UnnecessaryWarningSuppression(unnecessaryCategoryNames));
});
}
@ -295,10 +289,10 @@ public class LintMapper {
}
// After attribution: Discard the span from "unmappedDecls" and populate the declaration's subtree under "rootRange"
void afterAttr(Symtab syms, JCTree tree, EndPosTable endPositions) {
void afterAttr(JCTree tree, EndPosTable endPositions, Symtab syms) {
for (Iterator<Span> i = unmappedDecls.iterator(); i.hasNext(); ) {
if (i.next().contains(tree.pos())) {
rootRange.populateSubtree(this, syms, tree, endPositions);
rootRange.populateSubtree(this, tree, endPositions, syms);
i.remove();
return;
}
@ -317,7 +311,8 @@ public class LintMapper {
return validationsMap.computeIfAbsent(symbol, s -> LintCategory.newEmptySet());
}
// Combine the validation sets for two variable symbols that are declared together
// Merge the validation sets for two variables declared together, thereby sharing any @SuppressWarnings annotation.
// See "annotationRepresentativeSymbolMap" below for more detail on why this is needed.
void mergeValidations(VarSymbol symbol1, VarSymbol symbol2) {
EnumSet<LintCategory> validations1 = validationsFor(symbol1);
EnumSet<LintCategory> validations2 = validationsFor(symbol2);
@ -369,22 +364,22 @@ public class LintMapper {
Span span, // declaration's lexical range
Lint lint, // the Lint configuration that applies at this declaration
Symbol symbol, // declaration symbol (null for root range)
List<LintRange> children, // the nested declarations one level below this node
JCAnnotation annotation, // the @SuppressWarnings on this declaration, if any
EnumSet<LintCategory> suppressions, // categories suppressed by @SuppressWarnings, if any
EnumSet<LintCategory> unvalidated // categories in "suppressions" that were never validated
EnumSet<LintCategory> suppressions, // categories suppressed by @SuppressWarnings
EnumSet<LintCategory> unvalidated, // categories suppressed by @SuppressWarnings that were never validated
List<LintRange> children // the nested declarations one level below this node
) {
// Create a node representing the entire file, using the root lint configuration
LintRange(Lint rootLint) {
this(Span.MAXIMAL, rootLint, null, new ArrayList<>(), null, LintCategory.newEmptySet(), LintCategory.newEmptySet());
this(Span.MAXIMAL, rootLint, null, null, LintCategory.newEmptySet(), LintCategory.newEmptySet(), new ArrayList<>());
}
// Create a node representing the given declaration and its corresponding Lint configuration
LintRange(JCTree tree, EndPosTable endPositions, Lint lint, Symbol symbol,
JCAnnotation annotation, EnumSet<LintCategory> suppressions) {
this(new Span(tree, endPositions), lint, symbol, new ArrayList<>(),
annotation, suppressions, EnumSet.copyOf(suppressions));
this(new Span(tree, endPositions), lint, symbol,
annotation, suppressions, EnumSet.copyOf(suppressions), new ArrayList<>());
}
// Find the most specific node in this tree (including me) that contains the given position, if any
@ -411,7 +406,7 @@ public class LintMapper {
// Calculate the unvalidated suppressions in the subtree rooted at this node. We do this by recursively
// propagating validations upward until they are "caught" by some matching suppression; this validates
// the suppression. Validations that are never caught and "escape" are returned to the caller.
// the suppression. Validations that are never caught "escape" and are returned to the caller.
public EnumSet<LintCategory> propagateValidations(Map<Symbol, EnumSet<LintCategory>> validationsMap) {
// Recurse on subtrees first and gather their uncaught validations
@ -434,20 +429,20 @@ public class LintMapper {
return false;
});
// Propagate the remaining validations that weren't caught upward
// Any remaining validations "escape" and propagate upward
return validations;
}
// Populate a sparse subtree corresponding to the given nested declaration.
// Only "interesting" declarations are included:
// - Declarations that have a different Lint configuration than their parent
// - Declarations that have a different Lint configuration from their parent
// - Declarations with a @SuppressWarnings annotation
void populateSubtree(FileInfo fileInfo, Symtab syms, JCTree tree, EndPosTable endPositions) {
void populateSubtree(FileInfo fileInfo, JCTree tree, EndPosTable endPositions, Symtab syms) {
new TreeScanner() {
// Variables declared together (separated by commas) share their @SuppressWarnings annotation, so they must also
// share the set of validated suppressions: the suppression of a category is valid if *any* of the variables
// validates it. We detect that situation using this map and, when found, invoke FileInfo.mergeValidations().
// Variables declared together (separated by commas) share any @SuppressWarnings annotation, so they must also
// share the set of validated suppressions. That is, the suppression of a lint category is valid if a warning
// would have been generated by *any* of the variables. We detect this particular scenario using this map.
private final Map<JCAnnotation, VarSymbol> annotationRepresentativeSymbolMap = new HashMap<>();
private LintRange currentNode = LintRange.this;

View File

@ -2670,7 +2670,7 @@ public class Check {
Predicate<MethodSymbol> methodSuppresses = m -> m.owner == site.tsym &&
m.attribute(syms.suppressWarningsType.tsym) != null &&
lint.augment(m).isSuppressed(LintCategory.OVERLOADS, true);
if (methodSuppresses.test(m1) | methodSuppresses.test(m2)) // use "|" to avoid an artificial preference
if (methodSuppresses.test(m1) | methodSuppresses.test(m2)) // use "|" to validate @SuppressWarnings on BOTH methods
return FIRST | SECOND;
// Locate the warning at one of the methods, if possible