From 338ba2a1bf85be2de4bb38850312a0f751e88d81 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Sat, 10 May 2025 12:08:49 -0500 Subject: [PATCH] Add support for the "suppression" lint category. --- make/CompileToolsJdk.gmk | 4 +- make/GenerateLinkOptData.gmk | 2 +- make/JrtfsJar.gmk | 2 +- make/modules/java.base/Java.gmk | 4 +- make/modules/java.desktop/Java.gmk | 2 +- make/modules/java.management/Java.gmk | 2 +- make/modules/java.naming/Java.gmk | 2 +- make/modules/java.prefs/Java.gmk | 2 + make/modules/java.rmi/Java.gmk | 2 +- make/modules/java.sql.rowset/Java.gmk | 2 +- make/modules/java.sql/Java.gmk | 2 +- make/modules/java.xml.crypto/Java.gmk | 2 +- make/modules/java.xml/Java.gmk | 2 +- make/modules/jdk.attach/Java.gmk | 30 + make/modules/jdk.dynalink/Java.gmk | 2 + make/modules/jdk.internal.le/Java.gmk | 2 +- make/modules/jdk.jartool/Java.gmk | 2 + make/modules/jdk.jdeps/Java.gmk | 2 + make/modules/jdk.jfr/Java.gmk | 2 +- make/modules/jdk.jlink/Java.gmk | 2 + make/modules/jdk.jpackage/Java.gmk | 2 +- make/modules/jdk.jshell/Java.gmk | 2 + make/modules/jdk.management.agent/Java.gmk | 30 + make/modules/jdk.management.jfr/Java.gmk | 30 + make/modules/jdk.management/Java.gmk | 2 +- make/modules/jdk.naming.rmi/Java.gmk | 30 + make/test/BuildFailureHandler.gmk | 2 +- make/test/BuildMicrobenchmark.gmk | 2 +- make/test/BuildTestLib.gmk | 1 + .../com/sun/tools/javac/code/Lint.java | 137 ++- .../com/sun/tools/javac/code/LintMapper.java | 248 ++++- .../com/sun/tools/javac/comp/Attr.java | 2 +- .../com/sun/tools/javac/comp/Check.java | 171 ++- .../com/sun/tools/javac/comp/Modules.java | 2 +- .../tools/javac/comp/ThisEscapeAnalyzer.java | 18 +- .../sun/tools/javac/comp/WarningAnalyzer.java | 6 + .../sun/tools/javac/file/BaseFileManager.java | 2 +- .../tools/javac/resources/compiler.properties | 5 + .../tools/javac/resources/javac.properties | 3 + .../classes/com/sun/tools/javac/util/Log.java | 56 +- .../share/classes/module-info.java | 1 + src/jdk.compiler/share/man/javac.md | 3 + .../WarnUnnecessaryLintSuppression.java | 29 + .../javac/lint/SuppressionWarningTest.java | 981 ++++++++++++++++++ .../tools/javac/warnings/DepAnn.java | 2 +- .../tools/lib/toolbox/TestRunner.java | 62 +- 46 files changed, 1708 insertions(+), 193 deletions(-) create mode 100644 make/modules/jdk.attach/Java.gmk create mode 100644 make/modules/jdk.management.agent/Java.gmk create mode 100644 make/modules/jdk.management.jfr/Java.gmk create mode 100644 make/modules/jdk.naming.rmi/Java.gmk create mode 100644 test/langtools/tools/javac/diags/examples/WarnUnnecessaryLintSuppression.java create mode 100644 test/langtools/tools/javac/lint/SuppressionWarningTest.java diff --git a/make/CompileToolsJdk.gmk b/make/CompileToolsJdk.gmk index c291dbdba0a..d825caca1c2 100644 --- a/make/CompileToolsJdk.gmk +++ b/make/CompileToolsJdk.gmk @@ -47,7 +47,7 @@ $(eval $(call SetupJavaCompilation, BUILD_TOOLS_JDK, \ build/tools/jigsaw \ build/tools/depend, \ BIN := $(BUILDTOOLS_OUTPUTDIR)/jdk_tools_classes, \ - DISABLED_WARNINGS := dangling-doc-comments options, \ + DISABLED_WARNINGS := dangling-doc-comments options suppression, \ JAVAC_FLAGS := \ --add-exports java.desktop/sun.awt=ALL-UNNAMED \ --add-exports java.base/sun.text=ALL-UNNAMED \ @@ -81,7 +81,7 @@ $(eval $(call SetupJavaCompilation, COMPILE_DEPEND, \ SRC := $(TOPDIR)/make/jdk/src/classes, \ INCLUDES := build/tools/depend, \ BIN := $(BUILDTOOLS_OUTPUTDIR)/depend, \ - DISABLED_WARNINGS := options, \ + DISABLED_WARNINGS := options suppression, \ JAVAC_FLAGS := \ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \ --add-exports jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED \ diff --git a/make/GenerateLinkOptData.gmk b/make/GenerateLinkOptData.gmk index 1f52b88e1ef..f02c4b803ac 100644 --- a/make/GenerateLinkOptData.gmk +++ b/make/GenerateLinkOptData.gmk @@ -40,7 +40,7 @@ $(eval $(call SetupJavaCompilation, CLASSLIST_JAR, \ SMALL_JAVA := false, \ SRC := $(TOPDIR)/make/jdk/src/classes, \ INCLUDES := build/tools/classlist, \ - DISABLED_WARNINGS := dangling-doc-comments, \ + DISABLED_WARNINGS := dangling-doc-comments suppression, \ BIN := $(BUILDTOOLS_OUTPUTDIR)/classlist_classes, \ JAR := $(SUPPORT_OUTPUTDIR)/classlist.jar, \ )) diff --git a/make/JrtfsJar.gmk b/make/JrtfsJar.gmk index 54e2b094318..e7b93757bcb 100644 --- a/make/JrtfsJar.gmk +++ b/make/JrtfsJar.gmk @@ -51,7 +51,7 @@ JIMAGE_PKGS := \ # ends up in the image, this will ensure reproducible classes $(eval $(call SetupJavaCompilation, BUILD_JRTFS, \ COMPILER := interim, \ - DISABLED_WARNINGS := options, \ + DISABLED_WARNINGS := options suppression, \ TARGET_RELEASE := $(TARGET_RELEASE_JDK8), \ SRC := $(TOPDIR)/src/java.base/share/classes, \ EXCLUDE_FILES := module-info.java, \ diff --git a/make/modules/java.base/Java.gmk b/make/modules/java.base/Java.gmk index fc091377456..27eec17ea25 100644 --- a/make/modules/java.base/Java.gmk +++ b/make/modules/java.base/Java.gmk @@ -28,8 +28,8 @@ # The base module should be built with all warnings enabled. When a # new warning is added to javac, it can be temporarily added to the # disabled warnings list. -# -# DISABLED_WARNINGS_java += + +DISABLED_WARNINGS_java += suppression DOCLINT += -Xdoclint:all/protected \ '-Xdoclint/package:java.*,javax.*' diff --git a/make/modules/java.desktop/Java.gmk b/make/modules/java.desktop/Java.gmk index bab6186fb0d..626571633fc 100644 --- a/make/modules/java.desktop/Java.gmk +++ b/make/modules/java.desktop/Java.gmk @@ -25,7 +25,7 @@ ################################################################################ -DISABLED_WARNINGS_java += dangling-doc-comments lossy-conversions this-escape +DISABLED_WARNINGS_java += dangling-doc-comments lossy-conversions this-escape suppression DOCLINT += -Xdoclint:all/protected \ '-Xdoclint/package:java.*,javax.*' COPY += .gif .png .wav .txt .xml .css .pf diff --git a/make/modules/java.management/Java.gmk b/make/modules/java.management/Java.gmk index 44e3f328c7f..f43c27df595 100644 --- a/make/modules/java.management/Java.gmk +++ b/make/modules/java.management/Java.gmk @@ -25,7 +25,7 @@ ################################################################################ -DISABLED_WARNINGS_java += dangling-doc-comments this-escape +DISABLED_WARNINGS_java += dangling-doc-comments this-escape suppression DOCLINT += -Xdoclint:all/protected \ '-Xdoclint/package:java.*,javax.*' diff --git a/make/modules/java.naming/Java.gmk b/make/modules/java.naming/Java.gmk index 1c7a2a1668a..13d8247a5e7 100644 --- a/make/modules/java.naming/Java.gmk +++ b/make/modules/java.naming/Java.gmk @@ -25,7 +25,7 @@ ################################################################################ -DISABLED_WARNINGS_java += dangling-doc-comments this-escape +DISABLED_WARNINGS_java += dangling-doc-comments this-escape suppression DOCLINT += -Xdoclint:all/protected \ '-Xdoclint/package:java.*,javax.*' diff --git a/make/modules/java.prefs/Java.gmk b/make/modules/java.prefs/Java.gmk index 6e5ea0e2c73..fb542fe1084 100644 --- a/make/modules/java.prefs/Java.gmk +++ b/make/modules/java.prefs/Java.gmk @@ -25,6 +25,8 @@ ################################################################################ +DISABLED_WARNINGS_java += suppression + DOCLINT += -Xdoclint:all/protected \ '-Xdoclint/package:java.*,javax.*' diff --git a/make/modules/java.rmi/Java.gmk b/make/modules/java.rmi/Java.gmk index 4f24edb6f67..3c11b0c9608 100644 --- a/make/modules/java.rmi/Java.gmk +++ b/make/modules/java.rmi/Java.gmk @@ -25,7 +25,7 @@ ################################################################################ -DISABLED_WARNINGS_java += this-escape +DISABLED_WARNINGS_java += this-escape suppression DOCLINT += -Xdoclint:all/protected \ '-Xdoclint/package:java.*,javax.*' diff --git a/make/modules/java.sql.rowset/Java.gmk b/make/modules/java.sql.rowset/Java.gmk index ecfe3e6e641..0c27ee35102 100644 --- a/make/modules/java.sql.rowset/Java.gmk +++ b/make/modules/java.sql.rowset/Java.gmk @@ -25,7 +25,7 @@ ################################################################################ -DISABLED_WARNINGS_java += dangling-doc-comments +DISABLED_WARNINGS_java += dangling-doc-comments suppression DOCLINT += -Xdoclint:all/protected \ '-Xdoclint/package:java.*,javax.*' diff --git a/make/modules/java.sql/Java.gmk b/make/modules/java.sql/Java.gmk index 44e3f328c7f..f43c27df595 100644 --- a/make/modules/java.sql/Java.gmk +++ b/make/modules/java.sql/Java.gmk @@ -25,7 +25,7 @@ ################################################################################ -DISABLED_WARNINGS_java += dangling-doc-comments this-escape +DISABLED_WARNINGS_java += dangling-doc-comments this-escape suppression DOCLINT += -Xdoclint:all/protected \ '-Xdoclint/package:java.*,javax.*' diff --git a/make/modules/java.xml.crypto/Java.gmk b/make/modules/java.xml.crypto/Java.gmk index 68db8ed817a..0940e25a362 100644 --- a/make/modules/java.xml.crypto/Java.gmk +++ b/make/modules/java.xml.crypto/Java.gmk @@ -25,7 +25,7 @@ ################################################################################ -DISABLED_WARNINGS_java += dangling-doc-comments this-escape +DISABLED_WARNINGS_java += dangling-doc-comments this-escape suppression DOCLINT += -Xdoclint:all/protected \ '-Xdoclint/package:java.*,javax.*' diff --git a/make/modules/java.xml/Java.gmk b/make/modules/java.xml/Java.gmk index 35f66238a7a..e6896dc85d7 100644 --- a/make/modules/java.xml/Java.gmk +++ b/make/modules/java.xml/Java.gmk @@ -25,7 +25,7 @@ ################################################################################ -DISABLED_WARNINGS_java += dangling-doc-comments lossy-conversions this-escape +DISABLED_WARNINGS_java += dangling-doc-comments lossy-conversions this-escape suppression DOCLINT += -Xdoclint:all/protected \ '-Xdoclint/package:$(call CommaList, javax.xml.catalog javax.xml.datatype \ javax.xml.transform javax.xml.validation javax.xml.xpath)' diff --git a/make/modules/jdk.attach/Java.gmk b/make/modules/jdk.attach/Java.gmk new file mode 100644 index 00000000000..de42fd80b6b --- /dev/null +++ b/make/modules/jdk.attach/Java.gmk @@ -0,0 +1,30 @@ +# +# Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. +# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. +# +# This code is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License version 2 only, as +# published by the Free Software Foundation. Oracle designates this +# particular file as subject to the "Classpath" exception as provided +# by Oracle in the LICENSE file that accompanied this code. +# +# This code is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +# version 2 for more details (a copy is included in the LICENSE file that +# accompanied this code). +# +# You should have received a copy of the GNU General Public License version +# 2 along with this work; if not, write to the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA +# or visit www.oracle.com if you need additional information or have any +# questions. +# + +################################################################################ + +DISABLED_WARNINGS_java += suppression + +################################################################################ diff --git a/make/modules/jdk.dynalink/Java.gmk b/make/modules/jdk.dynalink/Java.gmk index f95e59bf36e..52efb935420 100644 --- a/make/modules/jdk.dynalink/Java.gmk +++ b/make/modules/jdk.dynalink/Java.gmk @@ -25,6 +25,8 @@ ################################################################################ +DISABLED_WARNINGS_java += suppression + CLEAN += .properties ################################################################################ diff --git a/make/modules/jdk.internal.le/Java.gmk b/make/modules/jdk.internal.le/Java.gmk index 27c6eaf5f7f..77439b114d7 100644 --- a/make/modules/jdk.internal.le/Java.gmk +++ b/make/modules/jdk.internal.le/Java.gmk @@ -25,7 +25,7 @@ ################################################################################ -DISABLED_WARNINGS_java += dangling-doc-comments this-escape +DISABLED_WARNINGS_java += dangling-doc-comments this-escape suppression COPY += .properties .caps .txt diff --git a/make/modules/jdk.jartool/Java.gmk b/make/modules/jdk.jartool/Java.gmk index 806975d1e18..71e0ac9c5f1 100644 --- a/make/modules/jdk.jartool/Java.gmk +++ b/make/modules/jdk.jartool/Java.gmk @@ -25,6 +25,8 @@ ################################################################################ +DISABLED_WARNINGS_java += suppression + JAVAC_FLAGS += -XDstringConcat=inline ################################################################################ diff --git a/make/modules/jdk.jdeps/Java.gmk b/make/modules/jdk.jdeps/Java.gmk index f2da87aeade..a1bbecc389f 100644 --- a/make/modules/jdk.jdeps/Java.gmk +++ b/make/modules/jdk.jdeps/Java.gmk @@ -25,6 +25,8 @@ ################################################################################ +DISABLED_WARNINGS_java += suppression + COPY += .txt CLEAN_FILES += $(wildcard \ diff --git a/make/modules/jdk.jfr/Java.gmk b/make/modules/jdk.jfr/Java.gmk index 5fabda8a4b7..d9d7c0b56f9 100644 --- a/make/modules/jdk.jfr/Java.gmk +++ b/make/modules/jdk.jfr/Java.gmk @@ -25,7 +25,7 @@ ################################################################################ -DISABLED_WARNINGS_java += dangling-doc-comments exports +DISABLED_WARNINGS_java += dangling-doc-comments exports suppression COPY := .xsd .xml .dtd .ini JAVAC_FLAGS := -XDstringConcat=inline diff --git a/make/modules/jdk.jlink/Java.gmk b/make/modules/jdk.jlink/Java.gmk index 4ddd1eab03d..7fc7da7c5f2 100644 --- a/make/modules/jdk.jlink/Java.gmk +++ b/make/modules/jdk.jlink/Java.gmk @@ -29,4 +29,6 @@ # upgrade_files_.conf files COPY += .conf +DISABLED_WARNINGS_java += suppression + ################################################################################ diff --git a/make/modules/jdk.jpackage/Java.gmk b/make/modules/jdk.jpackage/Java.gmk index da66fc14009..f3434f2ebab 100644 --- a/make/modules/jdk.jpackage/Java.gmk +++ b/make/modules/jdk.jpackage/Java.gmk @@ -25,7 +25,7 @@ ################################################################################ -DISABLED_WARNINGS_java += dangling-doc-comments +DISABLED_WARNINGS_java += dangling-doc-comments suppression COPY += .gif .png .txt .spec .script .prerm .preinst \ .postrm .postinst .list .sh .desktop .copyright .control .plist .template \ diff --git a/make/modules/jdk.jshell/Java.gmk b/make/modules/jdk.jshell/Java.gmk index f4194b23af7..1b9bf5b36b0 100644 --- a/make/modules/jdk.jshell/Java.gmk +++ b/make/modules/jdk.jshell/Java.gmk @@ -25,6 +25,8 @@ ################################################################################ +DISABLED_WARNINGS_java += suppression + COPY += .jsh .properties ################################################################################ diff --git a/make/modules/jdk.management.agent/Java.gmk b/make/modules/jdk.management.agent/Java.gmk new file mode 100644 index 00000000000..de42fd80b6b --- /dev/null +++ b/make/modules/jdk.management.agent/Java.gmk @@ -0,0 +1,30 @@ +# +# Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. +# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. +# +# This code is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License version 2 only, as +# published by the Free Software Foundation. Oracle designates this +# particular file as subject to the "Classpath" exception as provided +# by Oracle in the LICENSE file that accompanied this code. +# +# This code is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +# version 2 for more details (a copy is included in the LICENSE file that +# accompanied this code). +# +# You should have received a copy of the GNU General Public License version +# 2 along with this work; if not, write to the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA +# or visit www.oracle.com if you need additional information or have any +# questions. +# + +################################################################################ + +DISABLED_WARNINGS_java += suppression + +################################################################################ diff --git a/make/modules/jdk.management.jfr/Java.gmk b/make/modules/jdk.management.jfr/Java.gmk new file mode 100644 index 00000000000..de42fd80b6b --- /dev/null +++ b/make/modules/jdk.management.jfr/Java.gmk @@ -0,0 +1,30 @@ +# +# Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. +# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. +# +# This code is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License version 2 only, as +# published by the Free Software Foundation. Oracle designates this +# particular file as subject to the "Classpath" exception as provided +# by Oracle in the LICENSE file that accompanied this code. +# +# This code is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +# version 2 for more details (a copy is included in the LICENSE file that +# accompanied this code). +# +# You should have received a copy of the GNU General Public License version +# 2 along with this work; if not, write to the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA +# or visit www.oracle.com if you need additional information or have any +# questions. +# + +################################################################################ + +DISABLED_WARNINGS_java += suppression + +################################################################################ diff --git a/make/modules/jdk.management/Java.gmk b/make/modules/jdk.management/Java.gmk index aca47fc97f7..640c9a10f2e 100644 --- a/make/modules/jdk.management/Java.gmk +++ b/make/modules/jdk.management/Java.gmk @@ -25,6 +25,6 @@ ################################################################################ -DISABLED_WARNINGS_java += this-escape +DISABLED_WARNINGS_java += this-escape suppression ################################################################################ diff --git a/make/modules/jdk.naming.rmi/Java.gmk b/make/modules/jdk.naming.rmi/Java.gmk new file mode 100644 index 00000000000..de42fd80b6b --- /dev/null +++ b/make/modules/jdk.naming.rmi/Java.gmk @@ -0,0 +1,30 @@ +# +# Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. +# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. +# +# This code is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License version 2 only, as +# published by the Free Software Foundation. Oracle designates this +# particular file as subject to the "Classpath" exception as provided +# by Oracle in the LICENSE file that accompanied this code. +# +# This code is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +# version 2 for more details (a copy is included in the LICENSE file that +# accompanied this code). +# +# You should have received a copy of the GNU General Public License version +# 2 along with this work; if not, write to the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA +# or visit www.oracle.com if you need additional information or have any +# questions. +# + +################################################################################ + +DISABLED_WARNINGS_java += suppression + +################################################################################ diff --git a/make/test/BuildFailureHandler.gmk b/make/test/BuildFailureHandler.gmk index b4f3d690b0d..2e039260c04 100644 --- a/make/test/BuildFailureHandler.gmk +++ b/make/test/BuildFailureHandler.gmk @@ -47,7 +47,7 @@ $(eval $(call SetupJavaCompilation, BUILD_FAILURE_HANDLER, \ TARGET_RELEASE := $(TARGET_RELEASE_BOOTJDK), \ SRC := $(FH_BASEDIR)/src/share/classes $(FH_BASEDIR)/src/share/conf, \ BIN := $(FH_SUPPORT)/classes, \ - DISABLED_WARNINGS := options serial try this-escape, \ + DISABLED_WARNINGS := options serial try this-escape suppression, \ COPY := .properties, \ CLASSPATH := $(JTREG_JAR) $(TOOLS_JAR), \ JAR := $(FH_JAR), \ diff --git a/make/test/BuildMicrobenchmark.gmk b/make/test/BuildMicrobenchmark.gmk index 347ca44d25f..84c7700258e 100644 --- a/make/test/BuildMicrobenchmark.gmk +++ b/make/test/BuildMicrobenchmark.gmk @@ -84,7 +84,7 @@ $(eval $(call SetupJavaCompilation, BUILD_JDK_MICROBENCHMARK, \ CLASSPATH := $(JMH_COMPILE_JARS), \ CREATE_API_DIGEST := true, \ DISABLED_WARNINGS := restricted this-escape processing rawtypes removal cast \ - serial preview dangling-doc-comments, \ + serial preview dangling-doc-comments suppression, \ SRC := $(MICROBENCHMARK_SRC), \ BIN := $(MICROBENCHMARK_CLASSES), \ JAVAC_FLAGS := \ diff --git a/make/test/BuildTestLib.gmk b/make/test/BuildTestLib.gmk index dc5e0a9bd64..eb25784b2b2 100644 --- a/make/test/BuildTestLib.gmk +++ b/make/test/BuildTestLib.gmk @@ -63,6 +63,7 @@ $(eval $(call SetupJavaCompilation, BUILD_TEST_LIB_JAR, \ BIN := $(TEST_LIB_SUPPORT)/test-lib_classes, \ HEADERS := $(TEST_LIB_SUPPORT)/test-lib_headers, \ JAR := $(TEST_LIB_SUPPORT)/test-lib.jar, \ + DISABLED_WARNINGS := suppression, \ JAVAC_FLAGS := --add-exports java.base/sun.security.util=ALL-UNNAMED \ --add-exports java.base/jdk.internal.classfile=ALL-UNNAMED \ --add-exports java.base/jdk.internal.classfile.attribute=ALL-UNNAMED \ 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 27df573fe4f..7698b0c03d5 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 @@ -38,7 +38,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.JCDiagnostic.LintWarning; -import com.sun.tools.javac.util.Log; import com.sun.tools.javac.util.Names; import com.sun.tools.javac.util.Options; @@ -76,8 +75,9 @@ public class Lint { */ public Lint augment(Symbol sym) { EnumSet suppressions = suppressionsFrom(sym); + suppressions.removeIf(lc -> !lc.annotationSuppression); // ignore categories that don't support @SuppressWarnings if (!suppressions.isEmpty()) { - Lint lint = new Lint(this); + Lint lint = new Lint(this, sym); lint.values.removeAll(suppressions); lint.suppressedValues.addAll(suppressions); return lint; @@ -90,7 +90,7 @@ public class Lint { * @param lc one or more categories to be enabled */ public Lint enable(LintCategory... lc) { - Lint l = new Lint(this); + Lint l = new Lint(this, symbol); l.values.addAll(Arrays.asList(lc)); l.suppressedValues.removeAll(Arrays.asList(lc)); return l; @@ -101,15 +101,14 @@ public class Lint { * @param lc one or more categories to be suppressed */ public Lint suppress(LintCategory... lc) { - Lint l = new Lint(this); + Lint l = new Lint(this, symbol); l.values.removeAll(Arrays.asList(lc)); l.suppressedValues.addAll(Arrays.asList(lc)); return l; } private final Context context; - private final Options options; - private final Log log; + private final LintMapper lintMapper; // These are initialized lazily to avoid dependency loops private Symtab syms; @@ -119,22 +118,27 @@ public class Lint { private EnumSet values; private EnumSet suppressedValues; - private static final Map map = new ConcurrentHashMap<>(20); + // The symbol corresponding to some declaration, or null for the root instance + private final Symbol symbol; + // LintCategory lookup by option string + private static final Map map = new ConcurrentHashMap<>(40); + + // Instantiate the root instance @SuppressWarnings("this-escape") protected Lint(Context context) { this.context = context; context.put(lintKey, this); - options = Options.instance(context); - log = Log.instance(context); + symbol = null; + lintMapper = LintMapper.instance(context); } // Instantiate a non-root ("symbol scoped") instance - protected Lint(Lint other) { - other.initializeRootIfNeeded(); + protected Lint(Lint other, Symbol symbol) { + Assert.check(symbol != null); + this.symbol = symbol; this.context = other.context; - this.options = other.options; - this.log = other.log; + this.lintMapper = other.lintMapper; this.syms = other.syms; this.names = other.names; this.values = other.values.clone(); @@ -149,6 +153,7 @@ public class Lint { return; // Initialize enabled categories based on "-Xlint" flags + Options options = Options.instance(context); if (options.isSet(Option.XLINT) || options.isSet(Option.XLINT_CUSTOM, "all")) { // If -Xlint or -Xlint:all is given, enable all categories by default values = EnumSet.allOf(LintCategory.class); @@ -192,7 +197,11 @@ public class Lint { @Override public String toString() { initializeRootIfNeeded(); - return "Lint:[enable" + values + ",suppress" + suppressedValues + "]"; + return "Lint[" + + (symbol != null ? "sym=" + symbol : "ROOT") + + ",enable" + values + + ",suppress" + suppressedValues + + "]"; } /** @@ -293,9 +302,10 @@ public class Lint { * Warn about issues relating to use of command line options. * *

- * This category is not supported by {@code @SuppressWarnings}. + * This category is not supported by {@code @SuppressWarnings} + * and is not tracked for unnecessary suppression. */ - OPTIONS("options", false), + OPTIONS("options", false, false), /** * Warn when any output file is written to more than once. @@ -319,9 +329,10 @@ public class Lint { * Warn about invalid path elements on the command line. * *

- * This category is not supported by {@code @SuppressWarnings}. + * This category is not supported by {@code @SuppressWarnings} + * and is not tracked for unnecessary suppression. */ - PATH("path", false), + PATH("path", false, false), /** * Warn about issues regarding annotation processing. @@ -363,6 +374,14 @@ public class Lint { */ STRICTFP("strictfp"), + /** + * Warn about recognized {@code @SuppressWarnings} lint categories that don't actually suppress any warnings. + * + *

+ * This category is not tracked for unnecessary suppression. + */ + SUPPRESSION("suppression", true, false), + /** * Warn about synchronization attempts on instances of @ValueBased classes. */ @@ -408,8 +427,13 @@ public class Lint { } LintCategory(String option, boolean annotationSuppression) { + this(option, annotationSuppression, true); + } + + LintCategory(String option, boolean annotationSuppression, boolean suppressionTracking) { this.option = option; this.annotationSuppression = annotationSuppression; + this.suppressionTracking = suppressionTracking; map.put(option, this); } @@ -432,15 +456,42 @@ public class Lint { /** Does this category support being suppressed by the {@code @SuppressWarnings} annotation? */ public final boolean annotationSuppression; + + /** Does the {@code "suppression"} category track suppressions in this category? */ + public final boolean suppressionTracking; + } + + /** + * 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. + * + *

+ * Use of this method is never required; it simply helps avoid potentially useless work. + */ + public boolean isActive(LintCategory lc) { + initializeRootIfNeeded(); + return values.contains(lc) || needsSuppressionTracking(lc); } /** * Checks if a warning category is enabled. A warning category may be enabled * on the command line, or by default, and can be temporarily disabled with * the SuppressWarnings annotation. + * + *

+ * This method also optionally validates any warning suppressions 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 + * category in scope is validated (i.e., determined to actually be suppressing something). + * + * @param lc lint category + * @param validateSuppression true to also validate any suppression of the category */ - public boolean isEnabled(LintCategory lc) { + public boolean isEnabled(LintCategory lc, boolean validateSuppression) { initializeRootIfNeeded(); + if (validateSuppression) + validateSuppression(lc); return values.contains(lc); } @@ -449,9 +500,21 @@ public class Lint { * of the SuppressWarnings annotation, or, in the case of the deprecated * category, whether it has been implicitly suppressed by virtue of the * current entity being itself deprecated. + * + *

+ * This method also optionally validates any warning suppressions 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 + * category in scope is validated (i.e., determined to actually be suppressing something). + * + * @param lc lint category + * @param validateSuppression true to also validate any suppression of the category */ - public boolean isSuppressed(LintCategory lc) { + public boolean isSuppressed(LintCategory lc, boolean validateSuppression) { initializeRootIfNeeded(); + if (validateSuppression) + validateSuppression(lc); return suppressedValues.contains(lc); } @@ -478,7 +541,7 @@ public class Lint { * @param annotation @SuppressWarnings annotation, or null * @return set of lint categories, possibly empty but never null */ - private EnumSet suppressionsFrom(JCAnnotation annotation) { + public EnumSet suppressionsFrom(JCAnnotation annotation) { initializeSymbolsIfNeeded(); if (annotation == null) return LintCategory.newEmptySet(); @@ -504,12 +567,42 @@ public class Lint { for (Attribute value : values.values) { Optional.of((String)((Attribute.Constant)value).value) .flatMap(LintCategory::get) - .filter(lc -> lc.annotationSuppression) .ifPresent(result::add); } return result; } + /** + * Validate any suppression of the given category currently in scope. + * + *

+ * Such a suppression will therefore not be declared as unnecessary by the + * {@code "suppression"} warning. + * + * @param lc the lint category to be validated + * @return this instance + */ + public Lint validateSuppression(LintCategory lc) { + if (needsSuppressionTracking(lc)) + lintMapper.validateSuppression(symbol, lc); + return this; + } + + /** + * Determine whether we should bother tracking suppression validation for the given lint category. + * + *

+ * We need to track validation of suppression of a lint category if: + *

    + *
  • It's supported by {@code "suppression"} suppression tracking + *
  • Category {@code "suppression"} is currently enabled + *
+ */ + private boolean needsSuppressionTracking(LintCategory lc) { + initializeRootIfNeeded(); + return lc.suppressionTracking && values.contains(LintCategory.SUPPRESSION); + } + private void initializeSymbolsIfNeeded() { if (syms == null) { syms = Symtab.instance(context); 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 8b35cc197e7..d92a10353a2 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 @@ -26,19 +26,26 @@ package com.sun.tools.javac.code; import java.util.ArrayList; +import java.util.Collection; import java.util.Comparator; +import java.util.EnumSet; import java.util.HashMap; import java.util.List; 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; import javax.tools.DiagnosticListener; 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; import com.sun.tools.javac.tree.JCTree.*; @@ -47,6 +54,13 @@ import com.sun.tools.javac.tree.TreeScanner; 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. @@ -61,10 +75,37 @@ import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition; * The method {@link #lintAt} returns the {@link Lint} instance applicable to source position; * if it can't be determined yet, an empty {@link Optional} is returned. * - *

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

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

+ * Additional observations and corner cases: + *

    + *
  • Lint warnings can be suppressed at a module, package, class, method, or variable declaration + * (via {@code @SuppressWarnings}), or globally (via {@code -Xlint:-key}). + *
  • Consequently, an unnecessary suppression warning can only be emitted at one of those declarations, + * or globally at the end of compilation (the latter warning is possible in the future). + *
  • 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). + *
  • Some categories are never tracked for suppression, e.g., {@code options}, {@code path}, and the + * suppression category {@code "suppression"} itself. + *
  • {@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. + *
+ * + *

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 { @@ -74,11 +115,18 @@ public class LintMapper { // Per-source file lint information private final Map fileInfoMap = new HashMap<>(); + // Validations of "-Xlint:-foo" suppressions + private final EnumSet optionFlagValidations = LintCategory.newEmptySet(); + // Compiler context private final Context context; // These are initialized lazily; see initializeIfNeeded() + private Log log; private Lint rootLint; + private Symtab syms; + private Names names; + private Options options; /** * Obtain the {@link LintMapper} context singleton. @@ -101,8 +149,13 @@ public class LintMapper { // Lazy initialization to avoid dependency loops private void initializeIfNeeded() { - if (rootLint == null) + if (rootLint == null) { + log = Log.instance(context); rootLint = Lint.instance(context); + syms = Symtab.instance(context); + names = Names.instance(context); + options = Options.instance(context); + } } // Lint Operations @@ -147,6 +200,7 @@ public class LintMapper { */ public void clear() { fileInfoMap.clear(); + optionFlagValidations.clear(); } // Parsing Notifications @@ -167,6 +221,64 @@ public class LintMapper { fileInfoMap.get(tree.sourcefile).afterParse(tree); } +// Suppression Tracking + + /** + * Validate the given lint category within the scope of the given symbol's declaration (or globally if symbol is null). + * + *

+ * This is to indicate that, if the category is being suppressed, a warning would have otherwise been generated. + * + * @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) { + EnumSet validations = symbol != null ? + fileInfoMap.get(log.currentSourceFile()).validationsFor(symbol) : optionFlagValidations; + validations.add(category); + } + + /** + * Warn about unnecessary {@code @SuppressWarnings} suppressions within the given tree. + * + *

+ * This step must be done after the given source file has been warned about. + * + * @param sourceFile source file + * @param tree top level declaration + */ + public void reportUnnecessarySuppressionAnnotations(JavaFileObject sourceFile, JCTree tree) { + initializeIfNeeded(); + FileInfo fileInfo = fileInfoMap.get(sourceFile); + DeclNode topNode = fileInfo.findTopNode(tree.pos()); + + // Propagate validations in this top-level declaration to determine which suppressions never got validated + propagateValidations(fileInfo, topNode); + + // Report them if needed + if (rootLint.isEnabled(SUPPRESSION, false)) { + topNode.stream() + .filter(node -> node.lint.isEnabled(SUPPRESSION, false)) + .forEach(node -> report(node.unvalidated, name -> "\"" + name + "\"", + names -> log.warning(node.annotation.pos(), LintWarnings.UnnecessaryWarningSuppression(names)))); + } + } + + private void report(EnumSet unvalidated, Function formatter, Consumer logger) { + String names = unvalidated.stream() + .filter(lc -> lc.suppressionTracking) + .map(category -> category.option) + .map(formatter) + .collect(Collectors.joining(", ")); + if (!names.isEmpty()) + logger.accept(names); + } + + // Propagate validations in the given top-level declaration; any that escape validate the corresponding "Xlint" suppression + private void propagateValidations(FileInfo fileInfo, DeclNode topNode) { + optionFlagValidations.addAll(fileInfo.propagateValidations(topNode)); + } + // FileInfo /** @@ -184,8 +296,10 @@ public class LintMapper { */ private class FileInfo { - List topSpans; // the spans of all top level declarations - final DeclNode rootNode = new DeclNode(rootLint); // tree of file's "interesting" declaration nodes + List topSpans; // the spans of all top level declarations + final DeclNode rootNode = new DeclNode(rootLint); // tree of file's "interesting" declaration nodes + final Map> validationsMap // maps declaration symbol to validations therein + = new HashMap<>(); // Find the Lint that applies to the given position, if known Optional lintAt(DiagnosticPosition pos) { @@ -200,6 +314,25 @@ public class LintMapper { return Optional.of(node.lint); // use its Lint } + // Obtain the validation state for the given symbol + EnumSet validationsFor(Symbol symbol) { + return validationsMap.computeIfAbsent(symbol, s -> LintCategory.newEmptySet()); + } + + // Combine the validation sets for two variable symbols that are declared together + void mergeValidations(VarSymbol symbol1, VarSymbol symbol2) { + EnumSet validations1 = validationsFor(symbol1); + EnumSet validations2 = validationsFor(symbol2); + Assert.check(validations1.equals(validations2)); + validationsMap.put(symbol2, validations1); // now the two symbols share the same validation set + } + + // Propagate validations in the given top-level node + EnumSet propagateValidations(DeclNode topNode) { + Assert.check(rootNode.children.contains(topNode)); + return topNode.propagateValidations(validationsMap); + } + void afterParse(JCCompilationUnit tree) { Assert.check(topSpans == null, "source already parsed"); topSpans = tree.defs.stream() @@ -211,7 +344,7 @@ public class LintMapper { void afterAttr(JCTree tree, EndPosTable endPositions) { Assert.check(topSpans != null, "source not parsed"); Assert.check(findTopNode(tree.pos()) == null, "duplicate call"); - new DeclNodeTreeBuilder(rootNode, endPositions).scan(tree); + new DeclNodeTreeBuilder(this, rootNode, endPositions).scan(tree); } Optional findTopSpan(DiagnosticPosition pos) { @@ -286,6 +419,9 @@ public class LintMapper { final DeclNode parent; // the immediately containing declaration (null for root) final List children = new ArrayList<>(); // the immediately next level down declarations under this node final Lint lint; // the Lint configuration that applies at this declaration + final JCAnnotation annotation; // the @SuppressWarnings on this declaration, if any + final EnumSet suppressions; // categories suppressed by @SuppressWarnings, if any + final EnumSet unvalidated; // categories in "suppressions" that were never validated // Create a root node representing the entire file DeclNode(Lint rootLint) { @@ -293,14 +429,21 @@ public class LintMapper { this.symbol = null; this.parent = null; this.lint = rootLint; + this.annotation = null; + this.suppressions = LintCategory.newEmptySet(); // you can't put @SuppressWarnings on a file + this.unvalidated = EnumSet.copyOf(suppressions); } // Create a normal declaration node - DeclNode(Symbol symbol, DeclNode parent, JCTree tree, EndPosTable endPositions, Lint lint) { + DeclNode(Symbol symbol, DeclNode parent, JCTree tree, EndPosTable endPositions, + Lint lint, JCAnnotation annotation, EnumSet suppressions) { super(tree, endPositions); this.symbol = symbol; this.parent = parent; this.lint = lint; + this.annotation = annotation; + this.suppressions = suppressions; + this.unvalidated = EnumSet.copyOf(suppressions); parent.children.add(this); } @@ -318,10 +461,39 @@ public class LintMapper { return Stream.concat(Stream.of(this), children.stream().flatMap(DeclNode::stream)); } + // 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 not caught are returned to the caller. + public EnumSet propagateValidations(Map> validationsMap) { + + // Recurse on subtrees first and gather their uncaught validations + EnumSet validations = LintCategory.newEmptySet(); + children.stream() + .map(child -> child.propagateValidations(validationsMap)) + .forEach(validations::addAll); + + // Add in the validations that occurred at this node, if any + Optional.of(symbol) + .map(validationsMap::get) + .ifPresent(validations::addAll); + + // Apply (and then discard) validations that match any of this node's suppressions + validations.removeIf(category -> { + if (suppressions.contains(category)) { + unvalidated.remove(category); + return true; + } + return false; + }); + + // Propagate the remaining validations that weren't caught upward + return validations; + } + @Override public String toString() { String label = symbol != null ? "sym=" + symbol : "ROOT"; - return String.format("DeclNode[%s,lint=%s]", label, lint); + return String.format("DeclNode[%s,lint=%s,suppressions=%s]", label, lint, suppressions); } } @@ -332,12 +504,19 @@ public class LintMapper { */ private class DeclNodeTreeBuilder extends 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(). + private final Map annotationRepresentativeSymbolMap = new HashMap<>(); + + private final FileInfo fileInfo; private final EndPosTable endPositions; private DeclNode parent; private Lint lint; - DeclNodeTreeBuilder(DeclNode rootNode, EndPosTable endPositions) { + DeclNodeTreeBuilder(FileInfo fileInfo, DeclNode rootNode, EndPosTable endPositions) { + this.fileInfo = fileInfo; this.endPositions = endPositions; this.parent = rootNode; this.lint = rootNode.lint; // i.e, rootLint @@ -345,30 +524,30 @@ public class LintMapper { @Override public void visitModuleDef(JCModuleDecl tree) { - scanDecl(tree, tree.sym, super::visitModuleDef); + scanDecl(tree, tree.sym, findAnnotation(tree.mods), super::visitModuleDef); } @Override public void visitPackageDef(JCPackageDecl tree) { - scanDecl(tree, tree.packge, super::visitPackageDef); + scanDecl(tree, tree.packge, findAnnotation(tree.annotations), super::visitPackageDef); } @Override public void visitClassDef(JCClassDecl tree) { - scanDecl(tree, tree.sym, super::visitClassDef); + scanDecl(tree, tree.sym, findAnnotation(tree.mods), super::visitClassDef); } @Override public void visitMethodDef(JCMethodDecl tree) { - scanDecl(tree, tree.sym, super::visitMethodDef); + scanDecl(tree, tree.sym, findAnnotation(tree.mods), super::visitMethodDef); } @Override public void visitVarDef(JCVariableDecl tree) { - scanDecl(tree, tree.sym, super::visitVarDef); + scanDecl(tree, tree.sym, findAnnotation(tree.mods), super::visitVarDef); } - private void scanDecl(T tree, Symbol symbol, Consumer recursion) { + private void scanDecl(T tree, Symbol symbol, JCAnnotation annotation, Consumer recursion) { // "symbol" can be null if there were earlier errors; skip this declaration if so if (symbol == null) { @@ -380,14 +559,27 @@ public class LintMapper { Lint previousLint = lint; lint = lint.augment(symbol); + // Get the lint categories explicitly suppressed at this symbol's declaration by @SuppressedWarnings + EnumSet suppressed = Optional.ofNullable(annotation) + .map(anno -> rootLint.suppressionsFrom(anno)) + .orElseGet(LintCategory::newEmptySet); + + // Merge validation sets for variables that share the same declaration (and therefore the same @SuppressedWarnings) + if (annotation != null && symbol instanceof VarSymbol varSym) { + annotationRepresentativeSymbolMap.merge(annotation, varSym, (oldSymbol, newSymbol) -> { + fileInfo.mergeValidations(oldSymbol, newSymbol); + return oldSymbol; + }); + } + // If this declaration is not "interesting", we don't need to create a DeclNode for it - if (lint == previousLint && parent.parent != null) { + if (lint == previousLint && parent.parent != null && suppressed.isEmpty()) { recursion.accept(tree); return; } // Add a DeclNode here - DeclNode node = new DeclNode(symbol, parent, tree, endPositions, lint); + DeclNode node = new DeclNode(symbol, parent, tree, endPositions, lint, annotation, suppressed); parent = node; try { recursion.accept(tree); @@ -396,5 +588,23 @@ public class LintMapper { lint = previousLint; } } + + // Retrieve the @SuppressWarnings annotation, if any, from the given modifiers + private JCAnnotation findAnnotation(JCModifiers mods) { + return Optional.ofNullable(mods) + .map(m -> m.annotations) + .map(this::findAnnotation) + .orElse(null); + } + + // Retrieve the @SuppressWarnings annotation, if any, from the given list of annotations + private JCAnnotation findAnnotation(Collection annotations) { + return Optional.ofNullable(annotations) + .stream() + .flatMap(Collection::stream) + .filter(a -> a.attribute.type.tsym == syms.suppressWarningsType.tsym) + .findFirst() + .orElse(null); + } } } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java index 8ac0e3bfb12..b5ab0fdcad6 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java @@ -5668,7 +5668,7 @@ public class Attr extends JCTree.Visitor { // Check for proper use of serialVersionUID and other // serialization-related fields and methods - if (env.info.lint.isEnabled(LintCategory.SERIAL) + if (env.info.lint.isActive(LintCategory.SERIAL) && rs.isSerializable(c.type) && !c.isAnonymous()) { chk.checkSerialStructure(tree, c); 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 0b2b981ae30..414e7307209 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 @@ -2119,7 +2119,7 @@ public class Check { private void checkClassOverrideEqualsAndHash(DiagnosticPosition pos, ClassSymbol someClass) { - if (lint.isEnabled(LintCategory.OVERRIDES)) { + if (lint.isActive(LintCategory.OVERRIDES)) { MethodSymbol equalsAtObject = (MethodSymbol)syms.objectType .tsym.members().findFirst(names.equals); MethodSymbol hashCodeAtObject = (MethodSymbol)syms.objectType @@ -2171,7 +2171,7 @@ public class Check { public void checkModuleName (JCModuleDecl tree) { Name moduleName = tree.sym.name; Assert.checkNonNull(moduleName); - if (lint.isEnabled(LintCategory.MODULE)) { + if (lint.isActive(LintCategory.MODULE)) { JCExpression qualId = tree.qualId; while (qualId != null) { Name componentName; @@ -2642,7 +2642,7 @@ public class Check { void checkPotentiallyAmbiguousOverloads(JCClassDecl tree, Type site) { // Skip if warning not enabled - if (!lint.isEnabled(LintCategory.OVERLOADS)) + if (!lint.isActive(LintCategory.OVERLOADS)) return; // Gather all of site's methods, including overridden methods, grouped by name (except Object methods) @@ -2655,10 +2655,6 @@ public class Check { // Now remove overridden methods from each group, leaving only site's actual members methodGroups.forEach(list -> removePreempted(list, (m1, m2) -> m1.overrides(m2, site.tsym, types, false))); - // Allow site's own declared methods (only) to apply @SuppressWarnings("overloads") - methodGroups.forEach(list -> list.removeIf( - m -> m.owner == site.tsym && !lint.augment(m).isEnabled(LintCategory.OVERLOADS))); - // Warn about ambiguous overload method pairs for which site is responsible methodGroups.forEach(list -> compareAndRemove(list, (m1, m2) -> { @@ -2666,6 +2662,16 @@ public class Check { if (!potentiallyAmbiguousOverload(site, m1, m2) || !responsible.test(m1, m2)) return 0; + // Allow the site's own declared methods (only) to apply @SuppressWarnings("overloads"). + // Treat both methods equally so they "share" the validation of the warning suppression, + // but also verify an annotation actually exists on a method before doing that, because + // otherwise we could incorrectly validate an outer annotation. + 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 + return FIRST | SECOND; + // Locate the warning at one of the methods, if possible DiagnosticPosition pos = m1.owner == site.tsym ? TreeInfo.diagnosticPositionFor(m1, tree) : @@ -3678,14 +3684,14 @@ public class Check { } void checkDeprecatedAnnotation(DiagnosticPosition pos, Symbol s) { - if (lint.isEnabled(LintCategory.DEP_ANN) && s.isDeprecatableViaAnnotation() && + if (lint.isActive(LintCategory.DEP_ANN) && s.isDeprecatableViaAnnotation() && (s.flags() & DEPRECATED) != 0 && !syms.deprecatedType.isErroneous() && s.attribute(syms.deprecatedType.tsym) == null) { log.warning(pos, LintWarnings.MissingDeprecatedAnnotation); } // Note: @Deprecated has no effect on local variables, parameters and package decls. - if (lint.isEnabled(LintCategory.DEPRECATION) && !s.isDeprecatableViaAnnotation()) { + if (lint.isActive(LintCategory.DEPRECATION) && !s.isDeprecatableViaAnnotation()) { if (!syms.deprecatedType.isErroneous() && s.attribute(syms.deprecatedType.tsym) != null) { log.warning(pos, LintWarnings.DeprecatedAnnotationHasNoEffect(Kinds.kindName(s))); } @@ -4213,7 +4219,7 @@ public class Check { * Check for a default constructor in an exported package. */ void checkDefaultConstructor(ClassSymbol c, DiagnosticPosition pos) { - if (lint.isEnabled(LintCategory.MISSING_EXPLICIT_CTOR) && + if (lint.isActive(LintCategory.MISSING_EXPLICIT_CTOR) && ((c.flags() & (ENUM | RECORD)) == 0) && !c.isAnonymous() && ((c.flags() & (PUBLIC | PROTECTED)) != 0) && @@ -4431,7 +4437,7 @@ public class Check { Lint prevLint = lint; try { lint = lint.augment(tree.sym); - if (lint.isEnabled(LintCategory.EXPORTS)) { + if (lint.isActive(LintCategory.EXPORTS)) { super.visitMethodDef(tree); } } finally { @@ -4445,7 +4451,7 @@ public class Check { Lint prevLint = lint; try { lint = lint.augment(tree.sym); - if (lint.isEnabled(LintCategory.EXPORTS)) { + if (lint.isActive(LintCategory.EXPORTS)) { scan(tree.mods); scan(tree.vartype); } @@ -4464,7 +4470,7 @@ public class Check { Lint prevLint = lint; try { lint = lint.augment(tree.sym); - if (lint.isEnabled(LintCategory.EXPORTS)) { + if (lint.isActive(LintCategory.EXPORTS)) { scan(tree.mods); scan(tree.typarams); try { @@ -4863,6 +4869,14 @@ public class Check { Lint lint; + // Because runUnderLint() uses "augment" to customize the current Lint instance, + // we must check if the warning category is enabled manually before logging a warning. + private void warning(DiagnosticPosition pos, LintWarning warningKey) { + if (lint.isEnabled(warningKey.getLintCategory(), true)) { + log.warning(pos, warningKey); + } + } + @Override public Void defaultAction(Element e, JCClassDecl p) { throw new IllegalArgumentException(Objects.requireNonNullElse(e.toString(), "")); @@ -4894,7 +4908,7 @@ public class Check { } if (svuidSym == null) { - log.warning(p.pos(), LintWarnings.MissingSVUID(c)); + warning(p.pos(), LintWarnings.MissingSVUID(c)); } // Check for serialPersistentFields to gate checks for @@ -4921,9 +4935,8 @@ public class Check { // Note per JLS arrays are // serializable even if the // component type is not. - log.warning( - TreeInfo.diagnosticPositionFor(enclosed, tree), - LintWarnings.NonSerializableInstanceField); + warning(TreeInfo.diagnosticPositionFor(enclosed, tree), + LintWarnings.NonSerializableInstanceField); } else if (varType.hasTag(ARRAY)) { ArrayType arrayType = (ArrayType)varType; Type elementType = arrayType.elemtype; @@ -4932,9 +4945,8 @@ public class Check { elementType = arrayType.elemtype; } if (!canBeSerialized(elementType)) { - log.warning( - TreeInfo.diagnosticPositionFor(enclosed, tree), - LintWarnings.NonSerializableInstanceFieldArray(elementType)); + warning(TreeInfo.diagnosticPositionFor(enclosed, tree), + LintWarnings.NonSerializableInstanceFieldArray(elementType)); } } } @@ -5017,8 +5029,7 @@ public class Check { } } } - log.warning(tree.pos(), - LintWarnings.ExternalizableMissingPublicNoArgCtor); + warning(tree.pos(), LintWarnings.ExternalizableMissingPublicNoArgCtor); } else { // Approximate access to the no-arg constructor up in // the superclass chain by checking that the @@ -5045,8 +5056,8 @@ public class Check { // Handle nested classes and implicit this$0 (supertype.getNestingKind() == NestingKind.MEMBER && ((supertype.flags() & STATIC) == 0))) - log.warning(tree.pos(), - LintWarnings.SerializableMissingAccessNoArgCtor(supertype.getQualifiedName())); + warning(tree.pos(), + LintWarnings.SerializableMissingAccessNoArgCtor(supertype.getQualifiedName())); } } } @@ -5064,43 +5075,31 @@ public class Check { // fields. if ((svuid.flags() & (STATIC | FINAL)) != (STATIC | FINAL)) { - log.warning( - TreeInfo.diagnosticPositionFor(svuid, tree), - LintWarnings.ImproperSVUID((Symbol)e)); + warning(TreeInfo.diagnosticPositionFor(svuid, tree), LintWarnings.ImproperSVUID((Symbol)e)); } // check svuid has type long if (!svuid.type.hasTag(LONG)) { - log.warning( - TreeInfo.diagnosticPositionFor(svuid, tree), - LintWarnings.LongSVUID((Symbol)e)); + warning(TreeInfo.diagnosticPositionFor(svuid, tree), LintWarnings.LongSVUID((Symbol)e)); } if (svuid.getConstValue() == null) - log.warning( - TreeInfo.diagnosticPositionFor(svuid, tree), - LintWarnings.ConstantSVUID((Symbol)e)); + warning(TreeInfo.diagnosticPositionFor(svuid, tree), LintWarnings.ConstantSVUID((Symbol)e)); } private void checkSerialPersistentFields(JCClassDecl tree, Element e, VarSymbol spf) { // To be effective, serialPersisentFields must be private, static, and final. if ((spf.flags() & (PRIVATE | STATIC | FINAL)) != (PRIVATE | STATIC | FINAL)) { - log.warning( - TreeInfo.diagnosticPositionFor(spf, tree), - LintWarnings.ImproperSPF); + warning(TreeInfo.diagnosticPositionFor(spf, tree), LintWarnings.ImproperSPF); } if (!types.isSameType(spf.type, OSF_TYPE)) { - log.warning( - TreeInfo.diagnosticPositionFor(spf, tree), - LintWarnings.OSFArraySPF); + warning(TreeInfo.diagnosticPositionFor(spf, tree), LintWarnings.OSFArraySPF); } if (isExternalizable((Type)(e.asType()))) { - log.warning( - TreeInfo.diagnosticPositionFor(spf, tree), - LintWarnings.IneffectualSerialFieldExternalizable); + warning(TreeInfo.diagnosticPositionFor(spf, tree), LintWarnings.IneffectualSerialFieldExternalizable); } // Warn if serialPersistentFields is initialized to a @@ -5110,8 +5109,7 @@ public class Check { JCVariableDecl variableDef = (JCVariableDecl) spfDecl; JCExpression initExpr = variableDef.init; if (initExpr != null && TreeInfo.isNull(initExpr)) { - log.warning(initExpr.pos(), - LintWarnings.SPFNullInit); + warning(initExpr.pos(), LintWarnings.SPFNullInit); } } } @@ -5189,24 +5187,19 @@ public class Check { private void checkExternMethodRecord(JCClassDecl tree, Element e, MethodSymbol method, Type argType, boolean isExtern) { if (isExtern && isExternMethod(tree, e, method, argType)) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.IneffectualExternalizableMethodRecord(method.getSimpleName().toString())); + warning(TreeInfo.diagnosticPositionFor(method, tree), + LintWarnings.IneffectualExternalizableMethodRecord(method.getSimpleName().toString())); } } void checkPrivateNonStaticMethod(JCClassDecl tree, MethodSymbol method) { var flags = method.flags(); if ((flags & PRIVATE) == 0) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.SerialMethodNotPrivate(method.getSimpleName())); + warning(TreeInfo.diagnosticPositionFor(method, tree), LintWarnings.SerialMethodNotPrivate(method.getSimpleName())); } if ((flags & STATIC) != 0) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.SerialMethodStatic(method.getSimpleName())); + warning(TreeInfo.diagnosticPositionFor(method, tree), LintWarnings.SerialMethodStatic(method.getSimpleName())); } } @@ -5229,18 +5222,14 @@ public class Check { case FIELD -> { var field = (VarSymbol)enclosed; if (serialFieldNames.contains(name)) { - log.warning( - TreeInfo.diagnosticPositionFor(field, tree), - LintWarnings.IneffectualSerialFieldEnum(name)); + warning(TreeInfo.diagnosticPositionFor(field, tree), LintWarnings.IneffectualSerialFieldEnum(name)); } } case METHOD -> { var method = (MethodSymbol)enclosed; if (serialMethodNames.contains(name)) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.IneffectualSerialMethodEnum(name)); + warning(TreeInfo.diagnosticPositionFor(method, tree), LintWarnings.IneffectualSerialMethodEnum(name)); } if (isExtern) { @@ -5278,9 +5267,8 @@ public class Check { private void checkExternMethodEnum(JCClassDecl tree, Element e, MethodSymbol method, Type argType) { if (isExternMethod(tree, e, method, argType)) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.IneffectualExternMethodEnum(method.getSimpleName().toString())); + warning(TreeInfo.diagnosticPositionFor(method, tree), + LintWarnings.IneffectualExternMethodEnum(method.getSimpleName().toString())); } } @@ -5310,9 +5298,7 @@ public class Check { name = field.getSimpleName().toString(); switch(name) { case "serialPersistentFields" -> { - log.warning( - TreeInfo.diagnosticPositionFor(field, tree), - LintWarnings.IneffectualSerialFieldInterface); + warning(TreeInfo.diagnosticPositionFor(field, tree), LintWarnings.IneffectualSerialFieldInterface); } case "serialVersionUID" -> { @@ -5350,9 +5336,7 @@ public class Check { Element e, MethodSymbol method) { if ((method.flags() & PRIVATE) == 0) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.NonPrivateMethodWeakerAccess); + warning(TreeInfo.diagnosticPositionFor(method, tree), LintWarnings.NonPrivateMethodWeakerAccess); } } @@ -5360,9 +5344,7 @@ public class Check { Element e, MethodSymbol method) { if ((method.flags() & DEFAULT) == DEFAULT) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.DefaultIneffective); + warning(TreeInfo.diagnosticPositionFor(method, tree), LintWarnings.DefaultIneffective); } } @@ -5407,9 +5389,7 @@ public class Check { var field = (VarSymbol)enclosed; switch(name) { case "serialPersistentFields" -> { - log.warning( - TreeInfo.diagnosticPositionFor(field, tree), - LintWarnings.IneffectualSerialFieldRecord); + warning(TreeInfo.diagnosticPositionFor(field, tree), LintWarnings.IneffectualSerialFieldRecord); } case "serialVersionUID" -> { @@ -5431,9 +5411,8 @@ public class Check { default -> { if (serialMethodNames.contains(name)) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.IneffectualSerialMethodRecord(name)); + warning(TreeInfo.diagnosticPositionFor(method, tree), + LintWarnings.IneffectualSerialMethodRecord(name)); } }} }}}); @@ -5445,9 +5424,8 @@ public class Check { Element enclosing, MethodSymbol method) { if ((method.flags() & (STATIC | ABSTRACT)) != 0) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.SerialConcreteInstanceMethod(method.getSimpleName())); + warning(TreeInfo.diagnosticPositionFor(method, tree), + LintWarnings.SerialConcreteInstanceMethod(method.getSimpleName())); } } @@ -5462,9 +5440,8 @@ public class Check { // checking. Type rtype = method.getReturnType(); if (!types.isSameType(expectedReturnType, rtype)) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.SerialMethodUnexpectedReturnType(method.getSimpleName(), + warning(TreeInfo.diagnosticPositionFor(method, tree), + LintWarnings.SerialMethodUnexpectedReturnType(method.getSimpleName(), rtype, expectedReturnType)); } } @@ -5478,17 +5455,15 @@ public class Check { var parameters= method.getParameters(); if (parameters.size() != 1) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.SerialMethodOneArg(method.getSimpleName(), parameters.size())); + warning(TreeInfo.diagnosticPositionFor(method, tree), + LintWarnings.SerialMethodOneArg(method.getSimpleName(), parameters.size())); return; } Type parameterType = parameters.get(0).asType(); if (!types.isSameType(parameterType, expectedType)) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.SerialMethodParameterType(method.getSimpleName(), + warning(TreeInfo.diagnosticPositionFor(method, tree), + LintWarnings.SerialMethodParameterType(method.getSimpleName(), expectedType, parameterType)); } @@ -5507,18 +5482,16 @@ public class Check { private void checkNoArgs(JCClassDecl tree, Element enclosing, MethodSymbol method) { var parameters = method.getParameters(); if (!parameters.isEmpty()) { - log.warning( - TreeInfo.diagnosticPositionFor(parameters.get(0), tree), - LintWarnings.SerialMethodNoArgs(method.getSimpleName())); + warning(TreeInfo.diagnosticPositionFor(parameters.get(0), tree), + LintWarnings.SerialMethodNoArgs(method.getSimpleName())); } } private void checkExternalizable(JCClassDecl tree, Element enclosing, MethodSymbol method) { // If the enclosing class is externalizable, warn for the method if (isExternalizable((Type)enclosing.asType())) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.IneffectualSerialMethodExternalizable(method.getSimpleName())); + warning(TreeInfo.diagnosticPositionFor(method, tree), + LintWarnings.IneffectualSerialMethodExternalizable(method.getSimpleName())); } return; } @@ -5545,10 +5518,8 @@ public class Check { } } if (!declared) { - log.warning( - TreeInfo.diagnosticPositionFor(method, tree), - LintWarnings.SerialMethodUnexpectedException(method.getSimpleName(), - thrownType)); + warning(TreeInfo.diagnosticPositionFor(method, tree), + LintWarnings.SerialMethodUnexpectedException(method.getSimpleName(), thrownType)); } } } @@ -5560,7 +5531,7 @@ public class Check { try { lint = lint.augment((Symbol) symbol); - if (lint.isEnabled(LintCategory.SERIAL)) { + if (lint.isActive(LintCategory.SERIAL)) { task.accept(symbol, p); } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java index 788b7d08a8a..1591773e6d4 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java @@ -1352,7 +1352,7 @@ public class Modules extends JCTree.Visitor { .forEach(result::add); } - if (lint.isEnabled(LintCategory.INCUBATING)) { + if (lint.isActive(LintCategory.INCUBATING)) { String incubatingModules = filterAlreadyWarnedIncubatorModules(result.stream() .filter(msym -> msym.resolutionFlags.contains(ModuleResolutionFlags.WARN_INCUBATING)) .map(msym -> msym.name.toString())) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java index 2f21dcecd94..76a55294a5f 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java @@ -263,7 +263,7 @@ public class ThisEscapeAnalyzer extends TreeScanner { Assert.check(methodMap.isEmpty()); // we are not prepared to be used more than once // Short circuit if this calculation is unnecessary - if (!lintMapper.lintAt(env.toplevel.sourcefile, env.tree.pos()).get().isEnabled(THIS_ESCAPE)) + if (!lintMapper.lintAt(env.toplevel.sourcefile, env.tree.pos()).get().isActive(THIS_ESCAPE)) return; // Determine which packages are exported by the containing module, if any. @@ -343,7 +343,7 @@ public class ThisEscapeAnalyzer extends TreeScanner { .filter(MethodInfo::analyzable) .forEach(this::analyzeConstructor); - // Manually apply any Lint suppressions + // Manually apply (and validate) any Lint suppressions filterWarnings(warning -> !warning.isSuppressed()); // Field intitializers and initialization blocks will generate a separate warning for each primary constructor. @@ -1723,7 +1723,7 @@ public class ThisEscapeAnalyzer extends TreeScanner { } boolean isSuppressed() { - return suppressible && !lint().isEnabled(THIS_ESCAPE); + return suppressible && !lint().isEnabled(THIS_ESCAPE, true); } int comparePos(StackFrame that) { @@ -1788,12 +1788,12 @@ public class ThisEscapeAnalyzer extends TreeScanner { } }; - // Determine whether this warning is suppressed. A single "this-escape" warning involves multiple source code - // positions, so we must determine suppression manually. We do this as follows: A warning is suppressed if - // "this-escape" is disabled at any position in the stack where that stack frame corresponds to a constructor - // or field initializer in the target class. That means, for example, @SuppressWarnings("this-escape") annotations - // on regular methods are ignored. We work our way back up the call stack from the point of the leak until we - // encounter a suppressible stack frame. + // Determine whether this warning is suppressed and, if so, validate that suppression. A single "this-escape" + // warning involves multiple source code positions, so we must determine and validate suppression manually. + // We do this as follows: A warning is suppressed if "this-escape" is disabled at any position in the stack + // where that stack frame corresponds to a constructor or field initializer in the target class. That means, + // for example, @SuppressWarnings("this-escape") annotations on regular methods are ignored. We work our way + // back up the call stack from the point of the leak until we encounter a suppressible stack frame. boolean isSuppressed() { for (int index = stack.size() - 1; index >= 0; index--) { if (stack.get(index).isSuppressed()) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/WarningAnalyzer.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/WarningAnalyzer.java index 00d1de386db..3dbc385682c 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/WarningAnalyzer.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/WarningAnalyzer.java @@ -25,6 +25,7 @@ package com.sun.tools.javac.comp; +import com.sun.tools.javac.code.LintMapper; import com.sun.tools.javac.util.Context; import com.sun.tools.javac.util.Log; @@ -42,6 +43,7 @@ public class WarningAnalyzer { private final Log log; private final ThisEscapeAnalyzer thisEscapeAnalyzer; + private final LintMapper lintMapper; public static WarningAnalyzer instance(Context context) { WarningAnalyzer instance = context.get(contextKey); @@ -55,9 +57,13 @@ public class WarningAnalyzer { context.put(contextKey, this); log = Log.instance(context); thisEscapeAnalyzer = ThisEscapeAnalyzer.instance(context); + lintMapper = LintMapper.instance(context); } public void analyzeTree(Env env) { thisEscapeAnalyzer.analyzeTree(env); + + // This one should go last + lintMapper.reportUnnecessarySuppressionAnnotations(env.toplevel.sourcefile, env.tree); } } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/file/BaseFileManager.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/file/BaseFileManager.java index 5964c16c151..0c96d62ce8f 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/file/BaseFileManager.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/file/BaseFileManager.java @@ -519,7 +519,7 @@ public abstract class BaseFileManager implements JavaFileManager { synchronized void newOutputToPath(Path path) throws IOException { // Is output file clash detection enabled? - if (!lint.isEnabled(LintCategory.OUTPUT_FILE_CLASH)) + if (!lint.isActive(LintCategory.OUTPUT_FILE_CLASH)) return; // Get the "canonical" version of the file's path; we are assuming diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties index 79d15a96a6e..e022f770937 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties @@ -2246,6 +2246,11 @@ compiler.warn.requires.automatic=\ compiler.warn.requires.transitive.automatic=\ requires transitive directive for an automatic module +# 0: string +# lint: suppression +compiler.warn.unnecessary.warning.suppression=\ + unnecessary warning suppression: {0} + # Warnings related to annotation processing # 0: string compiler.warn.proc.package.does.not.exist=\ diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties index 1a8be506e7f..742d8e665f9 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties @@ -210,6 +210,9 @@ javac.opt.Xlint.desc.empty=\ javac.opt.Xlint.desc.exports=\ Warn about issues regarding module exports. +javac.opt.Xlint.desc.suppression=\ + Warn about recognized @SuppressWarnings values that don''t actually suppress any warnings. + javac.opt.Xlint.desc.fallthrough=\ Warn about falling through from one case of a switch statement to the next. diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Log.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Log.java index c7627d6e45b..385fe3c6b8e 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Log.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Log.java @@ -159,21 +159,25 @@ public class Log extends AbstractLog { public final void reportWithLint(JCDiagnostic diag, Lint lint) { // Apply hackery for REQUIRES_TRANSITIVE_AUTOMATIC (see also Check.checkModuleRequires()) - if (diag.getCode().equals(RequiresTransitiveAutomatic.key()) && !lint.isEnabled(REQUIRES_TRANSITIVE_AUTOMATIC)) { + if (diag.getCode().equals(RequiresTransitiveAutomatic.key()) && !lint.isEnabled(REQUIRES_TRANSITIVE_AUTOMATIC, true)) { reportWithLint(diags.warning(diag.getDiagnosticSource(), diag.getDiagnosticPosition(), RequiresAutomatic), lint); return; } // Apply the lint configuration (if any) and discard the warning if it gets filtered out if (lint != null) { + + // Gather information LintCategory category = diag.getLintCategory(); boolean emit = !diag.isFlagSet(DEFAULT_ENABLED) ? // is the warning not enabled by default? - lint.isEnabled(category) : // then emit if the category is enabled + lint.isEnabled(category, false) : // then emit if the category is enabled category.annotationSuppression ? // else emit if the category is not suppressed, where - !lint.isSuppressed(category) : // ...suppression happens via @SuppressWarnings + !lint.isSuppressed(category, false) : // ...suppression happens via @SuppressWarnings !options.isSet(XLINT_CUSTOM, "-" + category.option); // ...suppression happens via -Xlint:-category - if (!emit) + if (!emit) { + validateSuppression(new SuppressionValidation(lint, diag)); // validate any suppression return; + } } // Proceed @@ -185,6 +189,11 @@ public class Log extends AbstractLog { */ protected abstract void reportReady(JCDiagnostic diag); + /** + * Validate a lint suppression. + */ + protected abstract void validateSuppression(SuppressionValidation validation); + protected void addLintWaiter(JavaFileObject sourceFile, JCDiagnostic diagnostic) { lintWaitersMap.computeIfAbsent(sourceFile, s -> new LinkedList<>()).add(diagnostic); } @@ -220,6 +229,12 @@ public class Log extends AbstractLog { return diagnosticList.isEmpty(); }); } + + protected record SuppressionValidation(Lint lint, JCDiagnostic diag) { + void validate() { + lint.validateSuppression(diag.getLintCategory()); + } + } } /** @@ -232,6 +247,9 @@ public class Log extends AbstractLog { @Override protected void reportReady(JCDiagnostic diag) { } + + @Override + protected void validateSuppression(SuppressionValidation validation) { } } /** @@ -243,6 +261,7 @@ public class Log extends AbstractLog { */ public class DeferredDiagnosticHandler extends DiagnosticHandler { private List deferred = new ArrayList<>(); + private List validatedSuppressions = new ArrayList<>(); private final Predicate filter; private final boolean passOnNonDeferrable; @@ -281,6 +300,15 @@ public class Log extends AbstractLog { } } + @Override + protected void validateSuppression(SuppressionValidation validation) { + if (deferrable(validation.diag)) { + validatedSuppressions.add(validation); + } else { + prev.validateSuppression(validation); + } + } + public List getDiagnostics() { return deferred; } @@ -305,6 +333,12 @@ public class Log extends AbstractLog { .filter(accepter) .forEach(diagnostic -> prev.addLintWaiter(sourceFile, diagnostic))); lintWaitersMap = null; // prevent accidental ongoing use + + // Flush matching suppression validations to the previous handler + validatedSuppressions.stream() + .filter(vs -> accepter.test(vs.diag)) + .forEach(prev::validateSuppression); + validatedSuppressions = null; // prevent accidental ongoing use } /** Report all deferred diagnostics in the specified order. */ @@ -943,9 +977,14 @@ public class Log extends AbstractLog { // Apply the appropriate mandatory warning aggregator, if needed if (diagnostic.isFlagSet(AGGREGATE)) { LintCategory category = diagnostic.getLintCategory(); - boolean verbose = lintFor(diagnostic).isEnabled(category); - if (!aggregatorFor(category).aggregate(diagnostic, verbose)) + Lint lint = lintFor(diagnostic); + boolean verbose = lint.isEnabled(category, false); + if (!aggregatorFor(category).aggregate(diagnostic, verbose)) { + + // Aggregation effectively suppresses the warning, so validate that suppression + validateSuppression(new SuppressionValidation(lint, diagnostic)); return; + } } // Emit warning unless not mandatory and warnings are disabled @@ -974,6 +1013,11 @@ public class Log extends AbstractLog { compressedOutput = true; } } + + @Override + protected void validateSuppression(SuppressionValidation validation) { + validation.validate(); + } } /** diff --git a/src/jdk.compiler/share/classes/module-info.java b/src/jdk.compiler/share/classes/module-info.java index 60a9ae0e476..7322035e83d 100644 --- a/src/jdk.compiler/share/classes/module-info.java +++ b/src/jdk.compiler/share/classes/module-info.java @@ -186,6 +186,7 @@ import javax.tools.StandardLocation; * and interfaces * {@code static} accessing a static member using an instance * {@code strictfp} unnecessary use of the {@code strictfp} modifier + * {@code suppression} unnecessary suppressions in {@code @SuppressWarnings} annotations * {@code synchronization} synchronization attempts on instances of value-based classes * {@code text-blocks} inconsistent white space characters in text block indentation * {@code this-escape} superclass constructor leaking {@code this} before subclass initialized diff --git a/src/jdk.compiler/share/man/javac.md b/src/jdk.compiler/share/man/javac.md index f951ea0def3..b96ea9c336a 100644 --- a/src/jdk.compiler/share/man/javac.md +++ b/src/jdk.compiler/share/man/javac.md @@ -645,6 +645,9 @@ file system locations may be directories, JAR files or JMOD files. - `strictfp`: Warns about unnecessary use of the `strictfp` modifier. + - `suppression`: Warns about recognized `@SuppressWarnings` values that + don't actually suppress any warnings. + - `synchronization`: Warns about synchronization attempts on instances of value-based classes. diff --git a/test/langtools/tools/javac/diags/examples/WarnUnnecessaryLintSuppression.java b/test/langtools/tools/javac/diags/examples/WarnUnnecessaryLintSuppression.java new file mode 100644 index 00000000000..fa20afe3764 --- /dev/null +++ b/test/langtools/tools/javac/diags/examples/WarnUnnecessaryLintSuppression.java @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +// key: compiler.warn.unnecessary.warning.suppression +// options: -Xlint:suppression + +@SuppressWarnings("unchecked") +class X { +} diff --git a/test/langtools/tools/javac/lint/SuppressionWarningTest.java b/test/langtools/tools/javac/lint/SuppressionWarningTest.java new file mode 100644 index 00000000000..f226db208fe --- /dev/null +++ b/test/langtools/tools/javac/lint/SuppressionWarningTest.java @@ -0,0 +1,981 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8344159 + * @summary Test "suppression" lint warnings + * @library /tools/lib + * @modules + * jdk.compiler/com.sun.tools.javac.api + * jdk.compiler/com.sun.tools.javac.code + * jdk.compiler/com.sun.tools.javac.main + * @build toolbox.ToolBox toolbox.JavacTask toolbox.JarTask + * @run main SuppressionWarningTest + */ + +import java.io.File; +import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.lang.reflect.Method; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.EnumMap; +import java.util.EnumSet; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import com.sun.tools.javac.code.Lint; +import com.sun.tools.javac.code.Lint.LintCategory; +import com.sun.tools.javac.code.Source; + +import toolbox.JarTask; +import toolbox.JavacTask; +import toolbox.Task.Mode; +import toolbox.Task; +import toolbox.TestRunner; +import toolbox.ToolBox; + +import static com.sun.tools.javac.code.Lint.LintCategory.*; + +public class SuppressionWarningTest extends TestRunner { + + // Test cases for testSuppressWarnings() + public static final List SUPPRESS_WARNINGS_TEST_CASES = Stream.of(LintCategory.values()) + .filter(category -> category.suppressionTracking) + .map(category -> switch (category) { + case AUXILIARYCLASS -> new SuppressTest(category, + "compiler.warn.auxiliary.class.accessed.from.outside.of.its.source.file", + null, + """ + public class Class1 { } + class AuxClass { } + """, + """ + @OUTER@ + public class Class2 { + @INNER@ + public Object obj = new AuxClass(); + } + """ + ); + + case CAST -> new SuppressTest(category, + "compiler.warn.redundant.cast", + null, + """ + @OUTER@ + public class Test { + @INNER@ + public Object obj = (Object)new Object(); + } + """ + ); + + case CLASSFILE -> null; // skip, too hard to simluate + + case DANGLING_DOC_COMMENTS -> new SuppressTest(category, + "compiler.warn.dangling.doc.comment", + null, + """ + @OUTER@ + public class Test { + /** Dangling comment */ + /** Javadoc comment */ + @INNER@ + public void foo() { + } + } + """ + ); + + case DEPRECATION -> new SuppressTest(category, + "compiler.warn.has.been.deprecated", + null, + """ + public class Super { + @Deprecated + public void foo() { } + } + """, + """ + @OUTER@ + public class Sub extends Super { + @INNER@ + @Override + public void foo() { } + } + """ + ); + + case DEP_ANN -> new SuppressTest(category, + "compiler.warn.missing.deprecated.annotation", + null, + """ + @OUTER@ + public class Test { + @INNER@ + public class TestSub { + /** @deprecated */ + public void method() { } + } + } + """ + ); + + case DIVZERO -> new SuppressTest(category, + "compiler.warn.div.zero", + null, + """ + @OUTER@ + public class Test { + @INNER@ + public int method() { + return 1/0; + } + } + """ + ); + + case EMPTY -> new SuppressTest(category, + "compiler.warn.empty.if", + null, + """ + @OUTER@ + public class Test { + @INNER@ + public void method(boolean x) { + if (x); + } + } + """ + ); + + case EXPORTS -> new SuppressTest(category, + "compiler.warn.leaks.not.accessible", + null, + """ + module mod { + exports pkg1; + } + """, + """ + // @MODULE@:mod + package pkg1; + @OUTER@ + public class Class1 { + @INNER@ + public pkg2.Class2 obj2; // warning here + } + """, + """ + // @MODULE@:mod + package pkg2; + public class Class2 { + } + """ + ); + + case FALLTHROUGH -> new SuppressTest(category, + "compiler.warn.possible.fall-through.into.case", + null, + """ + @OUTER@ + public class Test { + @INNER@ + public void method(int x) { + switch (x) { + case 1: + System.out.println(1); + default: + System.out.println(0); + } + } + } + """ + ); + + case FINALLY -> new SuppressTest(category, + "compiler.warn.finally.cannot.complete", + null, + """ + @OUTER@ + public class Test { + @INNER@ + public void method(int x) { + try { + System.out.println(x); + } finally { + throw new RuntimeException(); + } + } + } + """ + ); + + case INCUBATING -> null; // skip, too hard to simluate reliably over time + + case LOSSY_CONVERSIONS -> new SuppressTest(category, + "compiler.warn.possible.loss.of.precision", + null, + """ + @OUTER@ + public class Test { + @INNER@ + public void method() { + long b = 1L; + b += 0.1 * 3L; + } + } + """ + ); + + case MISSING_EXPLICIT_CTOR -> new SuppressTest(category, + "compiler.warn.missing-explicit-ctor", + null, + """ + module mod { + exports pkg1; + } + """, + """ + package pkg1; + @OUTER@ + public class Class1 { + public Class1(int x) { + } + @INNER@ + public static class Sub { + } + } + """ + ); + + case MODULE -> new SuppressTest(category, + "compiler.warn.poor.choice.for.module.name", + null, + """ + @OUTER@ + module mod0 { + } + """ + ); + + case OPENS -> new SuppressTest(category, + "compiler.warn.package.empty.or.not.found", + null, + """ + @OUTER@ + module mod { + opens pkg1; + } + """ + ); + + // This test case only works on MacOS + case OUTPUT_FILE_CLASH -> + System.getProperty("os.name").startsWith("Mac") ? + new SuppressTest(category, + "compiler.warn.output.file.clash", + null, + """ + @OUTER@ + public class Test { + interface Cafe\u0301 { // macos normalizes "e" + U0301 -> U00e9 + } + interface Caf\u00e9 { + } + } + """ + ) : null; + + case OVERLOADS -> new SuppressTest(category, + "compiler.warn.potentially.ambiguous.overload", + null, + """ + import java.util.function.*; + @OUTER@ + public class Super { + public void foo(IntConsumer c) { + } + @INNER@ + public void foo(Consumer c) { + } + } + """ + ); + + case OVERRIDES -> new SuppressTest(category, + "compiler.warn.override.equals.but.not.hashcode", + null, + """ + @OUTER@ + public class Test { + @INNER@ + public class Test2 { + public boolean equals(Object obj) { + return false; + } + } + } + """ + ); + + case PROCESSING -> null; // skip for now + + case RAW -> new SuppressTest(category, + "compiler.warn.raw.class.use", + null, + """ + @OUTER@ + public class Test { + @INNER@ + public void foo() { + Iterable i = null; + } + } + """ + ); + + case REMOVAL -> new SuppressTest(category, + "compiler.warn.has.been.deprecated.for.removal", + null, + """ + public class Super { + @Deprecated(forRemoval = true) + public void foo() { } + } + """, + """ + @OUTER@ + public class Sub extends Super { + @INNER@ + @Override + public void foo() { } + } + """ + ); + + // This test case requires special support; see testSuppressWarnings() + case REQUIRES_AUTOMATIC -> new SuppressTest(category, + "compiler.warn.requires.automatic", + null, + """ + @OUTER@ + module m1x { + requires randomjar; + } + """ + ); + + // This test case requires special support; see testSuppressWarnings() + case REQUIRES_TRANSITIVE_AUTOMATIC -> new SuppressTest(category, + "compiler.warn.requires.transitive.automatic", + null, + """ + @OUTER@ + module m1x { + requires transitive randomjar; + } + """ + ); + + case SERIAL -> new SuppressTest(category, + "compiler.warn.missing.SVUID", + null, + """ + @OUTER@ + public class Test { + @INNER@ + public static class Inner implements java.io.Serializable { + public int x; + } + } + """ + ); + + case STATIC -> new SuppressTest(category, + "compiler.warn.static.not.qualified.by.type", + null, + """ + @OUTER@ + public class Test { + public static void foo() { + } + @INNER@ + public void bar() { + this.foo(); + } + } + """ + ); + + case STRICTFP -> new SuppressTest(category, + "compiler.warn.strictfp", + null, + """ + @OUTER@ + public class Test { + @INNER@ + public strictfp void foo() { + } + } + """ + ); + + case SYNCHRONIZATION -> new SuppressTest(category, + "compiler.warn.attempt.to.synchronize.on.instance.of.value.based.class", + null, + """ + @OUTER@ + public class Outer { + @INNER@ + public void foo() { + Integer i = 42; + synchronized (i) { + } + } + } + """ + ); + + case TEXT_BLOCKS -> new SuppressTest(category, + "compiler.warn.trailing.white.space.will.be.removed", + null, + """ + @OUTER@ + public class Test { + public void foo() { + String s = + \"\"\" + add trailing spaces here: + \"\"\"; + } + } + """.replaceAll("add trailing spaces here:", "$0 ") + ); + + case THIS_ESCAPE -> new SuppressTest(category, + "compiler.warn.possible.this.escape", + null, + """ + @OUTER@ + public class Outer { + @INNER@ + public static class Inner { + public Inner() { + leak(); + } + public void leak() { } + } + } + """ + ); + + case TRY -> new SuppressTest(category, + "compiler.warn.try.explicit.close.call", + null, + """ + import java.io.*; + @OUTER@ + public class Outer { + @INNER@ + public void foo() throws IOException { + try (InputStream in = new FileInputStream("x")) { + in.close(); + } + } + } + """ + ); + + case UNCHECKED -> new SuppressTest(category, + "compiler.warn.prob.found.req: (compiler.misc.unchecked.cast.to.type)", + null, + """ + @OUTER@ + public class Test { + public void foo() { + Iterable c = null; + @INNER@ + Iterable t = (Iterable)c, s = null; + } + } + """ + ); + + case VARARGS -> new SuppressTest(category, + "compiler.warn.varargs.unsafe.use.varargs.param", + null, + """ + @OUTER@ + public class Test { + @INNER@ + @SafeVarargs + public static void bar(final T... barArgs) { + baz(barArgs); + } + public static void baz(final T[] bazArgs) { + } + } + """ + ); + + // This test case requires special support; see testSuppressWarnings() + case PREVIEW -> new SuppressTest(category, + "compiler.warn.preview.feature.use", + new String[] { + "--enable-preview", + "-XDforcePreview" + }, + """ + @OUTER@ + public class Test { + @INNER@ + public Test(Object x) { + int value = x instanceof Integer i ? i : -1; + } + } + """ + ); + + case RESTRICTED -> new SuppressTest(category, + "compiler.warn.restricted.method", + null, + """ + @OUTER@ + public class Test { + @INNER@ + public void foo() { + System.load(""); + } + } + """ + ); + + default -> throw new AssertionError("missing test case for " + category); + + }) + .filter(Objects::nonNull) // skip categories with no test case defined + .collect(Collectors.toList()); + + protected final ToolBox tb; + + public SuppressionWarningTest() { + super(System.err); + tb = new ToolBox(); + } + + public static void main(String... args) throws Exception { + SuppressionWarningTest test = new SuppressionWarningTest(); + + // Run parameterized tests + test.runTestsMulti(m -> switch (m.getName()) { + case "testSuppressWarnings" -> SUPPRESS_WARNINGS_TEST_CASES.stream() + .map(testCase -> new Object[] { testCase }); + case "testUselessAnnotation" -> Stream.of(LintCategory.values()) + .filter(category -> category.suppressionTracking) + .map(category -> new Object[] { category }); + case "testSelfSuppression" -> Stream.of(RAW, SUPPRESSION) + .map(category -> new Object[] { category }); + case "testOverloads" -> Stream.of(new Object[0]); // no parameters for this test + case "testThisEscape" -> Stream.of(new Object[0]); // no parameters for this test + default -> throw new AssertionError("missing params for " + m); + }); + } + + // We are testing all combinations of nested @SuppressWarning annotations and lint flags + @Test + public void testSuppressWarnings(SuppressTest test) throws Exception { + + // Setup directories + Path base = Paths.get("testSuppressWarnings"); + resetCompileDirectories(base); + + // Detect if any modules are being compiled; if so we need to create an extra source directory level + Pattern moduleDecl = Pattern.compile("module\\s+(\\S*).*"); + Set moduleNames = test.sources().stream() + .flatMap(source -> Stream.of(source.split("\\n"))) + .map(moduleDecl::matcher) + .filter(Matcher::matches) + .map(matcher -> matcher.group(1)) + .collect(Collectors.toSet()); + + // Special JAR file support for REQUIRES_AUTOMATIC and REQUIRES_TRANSITIVE_AUTOMATIC + Path modulePath = base.resolve("modules"); + resetDirectory(modulePath); + LintCategory category = test.category(); + switch (category) { + case REQUIRES_AUTOMATIC: + case REQUIRES_TRANSITIVE_AUTOMATIC: + + // Compile a simple automatic module (randomjar-1.0) + Path randomJarBase = base.resolve("randomjar"); + tb.writeJavaFiles(getSourcesDir(randomJarBase), "package api; public class Api {}"); + List log = compile(randomJarBase, Task.Expect.SUCCESS, "-Werror"); + if (!log.isEmpty()) { + throw new AssertionError(String.format( + "non-empty log output:%n %s", log.stream().collect(Collectors.joining("\n ")))); + } + + // JAR it up + Path automaticJar = modulePath.resolve("randomjar-1.0.jar"); + new JarTask(tb, automaticJar) + .baseDir(getClassesDir(randomJarBase)) + .files("api/Api.class") + .run(); + break; + + default: + modulePath = null; + break; + }; + + // Create a @SuppressWarnings annotation + String annotation = String.format("@SuppressWarnings(\"%s\")", category.option); + + // See which annotation substitutions this test supports + boolean hasOuterAnnotation = test.sources().stream().anyMatch(source -> source.contains("@OUTER@")); + boolean hasInnerAnnotation = test.sources().stream().anyMatch(source -> source.contains("@INNER@")); + + // Try all combinations of inner and outer @SuppressWarnings + boolean[] booleans = new boolean[] { false, true }; + for (boolean outerAnnotation : booleans) { for (boolean innerAnnotation : booleans) { + + // Skip this scenario if not supported by test case + if ((outerAnnotation && !hasOuterAnnotation) || (innerAnnotation && !hasInnerAnnotation)) + continue; + + // Insert or comment out the @SuppressWarnings annotations in the source templates + String[] sources = test.sources().stream() + .map(source -> source.replace("@OUTER@", + String.format("%s@SuppressWarnings(\"%s\")", outerAnnotation ? "" : "//", category.option))) + .map(source -> source.replace("@INNER@", + String.format("%s@SuppressWarnings(\"%s\")", innerAnnotation ? "" : "//", category.option))) + .toArray(String[]::new); + for (String source : sources) { + Path pkgRoot = getSourcesDir(base); + String moduleName = Optional.of("@MODULE@:(\\S+)") + .map(Pattern::compile) + .map(p -> p.matcher(source)) + .filter(Matcher::find) + .map(m -> m.group(1)) + .orElse(null); + if (moduleName != null) { // add an extra directory for module + if (!moduleNames.contains(moduleName)) + throw new AssertionError(String.format("unknown module \"%s\" in %s", moduleName, category)); + pkgRoot = pkgRoot.resolve(moduleName); + } + tb.writeJavaFiles(pkgRoot, source); + } + + // Try all combinations of lint flags + for (boolean enableCategory : booleans) { // [-]category + for (boolean enableSuppression : booleans) { // [-]suppression + + // Should we expect the warning to be emitted? + boolean expectCategoryWarning = category.annotationSuppression ? + enableCategory && !outerAnnotation && !innerAnnotation : enableCategory; + + // Should we expect the SUPPRESSION warning to be emitted? + boolean expectSuppressionWarning = category.annotationSuppression ? + enableSuppression && outerAnnotation && innerAnnotation : // only if both, outer is redundant + enableSuppression && (outerAnnotation || innerAnnotation); // either one is always redundant + + // Prepare command line flags + ArrayList flags = new ArrayList<>(); + if (modulePath != null) { + flags.add("--module-path"); + flags.add(modulePath.toString()); + } + flags.add("--release"); + flags.add(Source.DEFAULT.name); + flags.addAll(test.compileFlags()); + + ArrayList lints = new ArrayList<>(); + lints.add(String.format("%s%s", enableCategory ? "" : "-", category.option)); + if (enableSuppression) + lints.add(SUPPRESSION.option); + if (!lints.isEmpty()) + flags.add("-Xlint:" + lints.stream().collect(Collectors.joining(","))); + + // Test case description + String description = String.format("[%s] outer=%s inner=%s enable=%s flags=\"%s\"", + category, outerAnnotation, innerAnnotation, enableCategory, + flags.stream().collect(Collectors.joining(" "))); + + // Only print log if test case fails + StringWriter buf = new StringWriter(); + PrintWriter log = new PrintWriter(buf); + try { + + // Logging + log.println(String.format(">>> Test START: %s", description)); + Stream.of(sources).forEach(log::println); + log.println(String.format(">>> expectCategoryWarning=%s", expectCategoryWarning)); + log.println(String.format(">>> expectSuppressionWarning=%s", expectSuppressionWarning)); + + // Compile sources and get log output + List output = compile(base, Task.Expect.SUCCESS, flags.toArray(new String[0])); + + // Scrub insignificant log output + output.removeIf(line -> line.matches("[0-9]+ (error|warning)s?")); + output.removeIf(line -> line.contains("compiler.err.warnings.and.werror")); + output.removeIf(line -> line.matches("- compiler\\.note\\..*")); // mandatory warning "recompile" etc. + + // See which warnings appeared + boolean foundSuppressionWarning = output.removeIf( + line -> line.contains("compiler.warn.unnecessary.warning.suppression")); + boolean foundCategoryWarning = output.removeIf(line -> line.contains(test.warningKey())); + + // Compare that vs. expectations + if (foundCategoryWarning != expectCategoryWarning) { + throw new AssertionError(String.format("%s: category warning: found=%s but expected=%s", + description, foundCategoryWarning, expectCategoryWarning)); + } + if (foundSuppressionWarning != expectSuppressionWarning) { + throw new AssertionError(String.format("%s: \"%s\" warning: found=%s but expected=%s", + description, SUPPRESSION.option, foundSuppressionWarning, expectSuppressionWarning)); + } + + // There shouldn't be any other warnings + if (!output.isEmpty()) { + throw new AssertionError(String.format( + "%s: %d unexpected warning(s): %s", description, output.size(), output)); + } + + // Done + log.println(String.format("<<< Test PASSED: %s", description)); + } catch (AssertionError e) { + log.println(String.format("<<< Test FAILED: %s", description)); + log.flush(); + out.print(buf); + throw e; + } + } + } + } } + } + + // Test a @SuppressWarning annotation that suppresses nothing + @Test + public void testUselessAnnotation(LintCategory category) throws Exception { + compileAndExpectWarning( + "compiler.warn.unnecessary.warning.suppression", + String.format( + """ + @SuppressWarnings(\"%s\") + public class Test { } + """, + category.option), + String.format("-Xlint:%s", SUPPRESSION.option)); + } + + // Test the suppression of SUPPRESSION itself, which should always work, + // even when the same annotation uselessly suppresses some other category. + @Test + public void testSelfSuppression(LintCategory category) throws Exception { + + // Test category and SUPPRESSION in the same annotation + compileAndExpectSuccess( + String.format( + """ + @SuppressWarnings({ \"%s\", \"%s\" }) + public class Test { + } + """, + category.option, // this is actually a useless suppression + SUPPRESSION.option), // but this prevents us from reporting it + String.format("-Xlint:%s", SUPPRESSION.option)); + + // Test category and SUPPRESSION in nested annotations + compileAndExpectSuccess( + String.format( + """ + @SuppressWarnings(\"%s\") // suppress useless suppression warnings + public class Test { + @SuppressWarnings(\"%s\") // a useless suppression + public class Sub { } + } + """, + SUPPRESSION.option, // this prevents us from reporting the nested useless suppression + category.option), // this is a useless suppression + String.format("-Xlint:%s", SUPPRESSION.option)); + } + + // Test OVERLOADS which has tricky "either-or" suppression + @Test + public void testOverloads() throws Exception { + compileAndExpectSuccess( + """ + import java.util.function.*; + public class Super { + @SuppressWarnings("overloads") + public void foo(IntConsumer c) { + } + @SuppressWarnings("overloads") + public void foo(Consumer c) { + } + } + """, + String.format("-Xlint:%s", OVERLOADS.option), + String.format("-Xlint:%s", SUPPRESSION.option)); + } + + // Test THIS_ESCAPE which has tricky control-flow based suppression + @Test + public void testThisEscape() throws Exception { + compileAndExpectSuccess( + """ + public class Test { + public Test() { + this(0); + } + @SuppressWarnings("this-escape") + private Test(int x) { + this.leak(); + } + protected void leak() { } + } + """, + String.format("-Xlint:%s", THIS_ESCAPE.option), + String.format("-Xlint:%s", SUPPRESSION.option)); + } + + public void compileAndExpectWarning(String errorKey, String source, String... flags) throws Exception { + + // Setup source & destination diretories + Path base = Paths.get("compileAndExpectWarning"); + resetCompileDirectories(base); + + // Write source file + tb.writeJavaFiles(getSourcesDir(base), source); + + // Compile sources and verify we got the warning + List log = compile(base, Task.Expect.FAIL, addWerror(flags)); + if (log.stream().noneMatch(line -> line.contains(errorKey))) { + throw new AssertionError(String.format( + "did not find \"%s\" in log output:%n %s", + errorKey, log.stream().collect(Collectors.joining("\n ")))); + } + } + + public void compileAndExpectSuccess(String source, String... flags) throws Exception { + + // Setup source & destination diretories + Path base = Paths.get("compileAndExpectSuccess"); + resetCompileDirectories(base); + + // Write source file + tb.writeJavaFiles(getSourcesDir(base), source); + + // Compile sources and verify there is no log output + List log = compile(base, Task.Expect.SUCCESS, addWerror(flags)); + if (!log.isEmpty()) { + throw new AssertionError(String.format( + "non-empty log output:%n %s", log.stream().collect(Collectors.joining("\n ")))); + } + } + + private List compile(Path base, Task.Expect expectation, String... flags) throws Exception { + ArrayList options = new ArrayList<>(); + options.add("-XDrawDiagnostics"); + Stream.of(flags).forEach(options::add); + List log; + try { + log = new JavacTask(tb, Mode.CMDLINE) + .options(options.toArray(new String[0])) + .files(tb.findJavaFiles(getSourcesDir(base))) + .outdir(getClassesDir(base)) + .run(expectation) + .writeAll() + .getOutputLines(Task.OutputKind.DIRECT); + } catch (Task.TaskError e) { + throw new AssertionError(String.format( + "compile in %s failed: %s", getSourcesDir(base), e.getMessage()), e); + } + log.removeIf(line -> line.trim().isEmpty()); + return log; + } + + private Path getSourcesDir(Path base) { + return base.resolve("sources"); + } + + private Path getClassesDir(Path base) { + return base.resolve("classes"); + } + + private void resetCompileDirectories(Path base) throws IOException { + for (Path dir : List.of(getSourcesDir(base), getClassesDir(base))) + resetDirectory(dir); + } + + private void resetDirectory(Path dir) throws IOException { + if (Files.exists(dir, LinkOption.NOFOLLOW_LINKS)) + Files.walkFileTree(dir, new Deleter()); + Files.createDirectories(dir); + } + + private String[] addWerror(String[] flags) { + return Stream.concat(Stream.of(flags), Stream.of("-Werror")).toArray(String[]::new); + } + +// SuppressTest + + private record SuppressTest( + LintCategory category, // The Lint category being tested + String warningKey, // Expected warning message key in compiler.properties + List compileFlags, // Any required compilation flags + List sources // Source files with @MODULE@, @OUTER@ and @INNER@ placeholders + ) { + SuppressTest(LintCategory category, String warningKey, String[] compileFlags, String... sources) { + this(category, warningKey, List.of(compileFlags != null ? compileFlags : new String[0]), List.of(sources)); + } + } + +// Deleter + + private static class Deleter extends SimpleFileVisitor { + + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + Files.delete(file); + return FileVisitResult.CONTINUE; + } + } +} diff --git a/test/langtools/tools/javac/warnings/DepAnn.java b/test/langtools/tools/javac/warnings/DepAnn.java index d725258cf3a..42090899711 100644 --- a/test/langtools/tools/javac/warnings/DepAnn.java +++ b/test/langtools/tools/javac/warnings/DepAnn.java @@ -1,7 +1,7 @@ /* * @test /nodynamiccopyright/ * @bug 4986256 - * @compile/ref=DepAnn.out -XDrawDiagnostics -Xlint:all,-dangling-doc-comments DepAnn.java + * @compile/ref=DepAnn.out -XDrawDiagnostics -Xlint:all,-dangling-doc-comments,-suppression DepAnn.java */ // control: this class should generate warnings diff --git a/test/langtools/tools/lib/toolbox/TestRunner.java b/test/langtools/tools/lib/toolbox/TestRunner.java index e93370a6e33..2bdbe5ddaeb 100644 --- a/test/langtools/tools/lib/toolbox/TestRunner.java +++ b/test/langtools/tools/lib/toolbox/TestRunner.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -29,7 +29,12 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.Iterator; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Utility class to manage and execute sub-tests within a test. @@ -74,28 +79,59 @@ public abstract class TestRunner { } /** - * Invoke all methods annotated with @Test. - * @param f a lambda expression to specify arguments for the test method + * Invoke each @Test method once using the parameters returned by the function. + * + *

+ * If the function returns null for some method, that method is skipped. + * + *

+ * If system property {@code test.query} is set, only that method is included. + * + * @param f function mapping method name to an array of method parameters * @throws java.lang.Exception if any errors occur */ protected void runTests(Function f) throws Exception { + runTestsMulti(f.andThen(Stream::of)); + } + + /** + * Invoke each @Test method once for each array of parameters returned by the iteration. + * + *

+ * If the function returns null for some method, that method is skipped. + * + * @param f function mapping method name to an iteration of arrays of method parameters + * @throws java.lang.Exception if any errors occur + */ + protected void runTestsMulti(Function> f) throws Exception { String testQuery = System.getProperty("test.query"); for (Method m : getClass().getDeclaredMethods()) { Annotation a = m.getAnnotation(Test.class); if (a != null) { testName = m.getName(); if (testQuery == null || testQuery.equals(testName)) { - try { - testCount++; - out.println("test: " + testName); - m.invoke(this, f.apply(m)); - } catch (InvocationTargetException e) { - errorCount++; - Throwable cause = e.getCause(); - out.println("Exception running test " + testName + ": " + e.getCause()); - cause.printStackTrace(out); + Iterator iterator = Optional.of(m).map(f).map(Stream::iterator).orElse(null); + if (iterator == null) + return; + for (int testNum = 1; iterator.hasNext(); testNum++) { + Object[] params = iterator.next(); + try { + testCount++; + out.println(String.format("test: %s#%d", testName, testNum)); + m.invoke(this, params); + } catch (InvocationTargetException e) { + errorCount++; + Throwable cause = e.getCause(); + String paramsDesc = Stream.of(params) + .map(String::valueOf) + .collect(Collectors.joining(", ")); + out.println(String.format( + "Exception running test %s#%d(%s): %s", + testName, testNum, paramsDesc, e.getCause())); + cause.printStackTrace(out); + } + out.println(); } - out.println(); } } }