diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java index 8c78ab16520..df91a917a2e 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java @@ -230,7 +230,7 @@ public class Lint { *

* 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 { *

* 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 { *

* 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. * *

* Use of this method is never required; it simply helps avoid potentially useless work. @@ -497,7 +493,7 @@ public class Lint { * the SuppressWarnings annotation. * *

- * 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. * *

- * 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. * *

- * 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: *

*/ private boolean needsSuppressionTracking(LintCategory lc) { diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/LintMapper.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/LintMapper.java index b67b95832be..c72a69d482a 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/LintMapper.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/LintMapper.java @@ -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. * *

- * 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 + * would have been reported; this is termed the "validation" of the suppression. That notification + * happens by invoking {@link #validateSuppression}. * *

- * 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. * *

- * Additional observations and corner cases: + * Some additional nuances: *

* - *

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. + *

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. */ 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. * *

- * 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. * *

- * 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 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 validations1 = validationsFor(symbol1); EnumSet 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 children, // the nested declarations one level below this node JCAnnotation annotation, // the @SuppressWarnings on this declaration, if any - EnumSet suppressions, // categories suppressed by @SuppressWarnings, if any - EnumSet unvalidated // categories in "suppressions" that were never validated + EnumSet suppressions, // categories suppressed by @SuppressWarnings + EnumSet unvalidated, // categories suppressed by @SuppressWarnings that were never validated + List 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 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 propagateValidations(Map> 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 annotationRepresentativeSymbolMap = new HashMap<>(); private LintRange currentNode = LintRange.this; diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java index 675045f2446..92068fa239b 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java @@ -2670,7 +2670,7 @@ public class Check { Predicate 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