diff --git a/checker-qual/src/main/java/org/checkerframework/common/util/report/qual/ReportUse.java b/checker-qual/src/main/java/org/checkerframework/common/util/report/qual/ReportUse.java index 91b7f6465297..b9077e2a0058 100644 --- a/checker-qual/src/main/java/org/checkerframework/common/util/report/qual/ReportUse.java +++ b/checker-qual/src/main/java/org/checkerframework/common/util/report/qual/ReportUse.java @@ -6,8 +6,22 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -/** Report all uses of a type that has this annotation. Can also be used on a package. */ +/** + * Report all uses of a type that has this annotation. Can also be used on a package. + * + *

When written on a package, {@code @ReportUse} applies to that package and its subpackages by + * default. Set {@link #applyToSubpackages()} to false to limit it to the package itself; doing so + * does not block an applicable {@code @ReportUse} on an enclosing package. + */ @Documented @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.PACKAGE, ElementType.TYPE}) -public @interface ReportUse {} +public @interface ReportUse { + + /** + * When used on a package, whether this annotation should also apply to subpackages. + * + * @return whether this annotation should be inherited by subpackages + */ + boolean applyToSubpackages() default true; +} diff --git a/checker-qual/src/main/java/org/checkerframework/framework/qual/AnnotatedFor.java b/checker-qual/src/main/java/org/checkerframework/framework/qual/AnnotatedFor.java index 48c4f7ab47f7..05da2f4138ba 100644 --- a/checker-qual/src/main/java/org/checkerframework/framework/qual/AnnotatedFor.java +++ b/checker-qual/src/main/java/org/checkerframework/framework/qual/AnnotatedFor.java @@ -23,6 +23,10 @@ * warnings. However, a class with a relevant {@code @AnnotatedFor} annotation is always defaulted * normally (typically using the CLIMB-to-top rule), and typechecking warnings are issued. * + *

An {@code @AnnotatedFor} on a package also applies to subpackages, unless the {@code + * applyToSubpackages} field is set to false. Setting it to false does not block an applicable + * {@code @AnnotatedFor} on an enclosing package. + * * @checker_framework.manual #compiling-libraries Compiling partially-annotated libraries */ @Documented @@ -39,4 +43,11 @@ * @checker_framework.manual #shorthand-for-checkers Short names for built-in checkers */ String[] value(); + + /** + * When used on a package, whether this annotation should also apply to subpackages. + * + * @return whether this annotation should be inherited by subpackages + */ + boolean applyToSubpackages() default true; } diff --git a/checker-qual/src/main/java/org/checkerframework/framework/qual/HasQualifierParameter.java b/checker-qual/src/main/java/org/checkerframework/framework/qual/HasQualifierParameter.java index cd59052d2f50..7f1cb3c6eed5 100644 --- a/checker-qual/src/main/java/org/checkerframework/framework/qual/HasQualifierParameter.java +++ b/checker-qual/src/main/java/org/checkerframework/framework/qual/HasQualifierParameter.java @@ -63,9 +63,11 @@ *

Written on a package

* *

When {@code @HasQualifierParameter} is written on a package, it is equivalent to writing that - * annotation on each class in the package or in a sub-package. It can be disabled on a specific - * class and its subclasses by writing {@code @NoQualifierParameter} on that class. This annotation - * may not be written on the same class as {@code NoQualifierParameter} for the same hierarchy. + * annotation on each class in the package or in a sub-package. Set the {@code applyToSubpackages} + * field to false to limit it to the package itself; doing so does not block an applicable + * {@code @HasQualifierParameter} on an enclosing package. It can be disabled on a specific class + * and its subclasses by writing {@code @NoQualifierParameter} on that class. This annotation may + * not be written on the same class as {@code @NoQualifierParameter} for the same hierarchy. * * @see NoQualifierParameter */ @@ -81,4 +83,11 @@ * @return the value */ Class[] value(); + + /** + * When used on a package, whether this annotation should also apply to subpackages. + * + * @return whether this annotation should be inherited by subpackages + */ + boolean applyToSubpackages() default true; } diff --git a/checker/jtreg/subpackages/AnnotatedForNested.java b/checker/jtreg/subpackages/AnnotatedForNested.java new file mode 100644 index 000000000000..c70bbf840f9b --- /dev/null +++ b/checker/jtreg/subpackages/AnnotatedForNested.java @@ -0,0 +1,9 @@ +/* + * @test + * @summary An AnnotatedFor with applyToSubpackages=false limits only its own annotation. An + * enclosing package whose annotation applies to subpackages still reaches through it, so code in + * the nested subpackage is checked rather than given conservative defaults. + * + * @compile/fail/ref=AnnotatedForNested.out -XDrawDiagnostics -processor org.checkerframework.checker.nullness.NullnessChecker -AuseConservativeDefaultsForUncheckedCode=source,bytecode af/package-info.java af/sub/package-info.java af/sub/deep/Deep.java + */ +public class AnnotatedForNested {} diff --git a/checker/jtreg/subpackages/AnnotatedForNested.out b/checker/jtreg/subpackages/AnnotatedForNested.out new file mode 100644 index 000000000000..36a9fdd43786 --- /dev/null +++ b/checker/jtreg/subpackages/AnnotatedForNested.out @@ -0,0 +1,4 @@ +Deep.java:15:14: compiler.err.proc.messager: [argument.type.incompatible] incompatible argument for parameter nn of Deep.take. +found : @Nullable Object +required: @NonNull Object +1 error diff --git a/checker/jtreg/subpackages/HasQualifierParameterNested.java b/checker/jtreg/subpackages/HasQualifierParameterNested.java new file mode 100644 index 000000000000..aee4a3b916c8 --- /dev/null +++ b/checker/jtreg/subpackages/HasQualifierParameterNested.java @@ -0,0 +1,8 @@ +/* + * @test + * @summary A HasQualifierParameter with applyToSubpackages=false limits only its own annotation. + * An enclosing package whose annotation applies to subpackages still reaches through it. + * + * @compile -processor org.checkerframework.checker.tainting.TaintingChecker -Werror hqp/package-info.java hqp/sub/package-info.java hqp/sub/deep/Deep.java + */ +public class HasQualifierParameterNested {} diff --git a/checker/jtreg/subpackages/af/package-info.java b/checker/jtreg/subpackages/af/package-info.java new file mode 100644 index 000000000000..db01de56626e --- /dev/null +++ b/checker/jtreg/subpackages/af/package-info.java @@ -0,0 +1,4 @@ +@AnnotatedFor("nullness") +package af; + +import org.checkerframework.framework.qual.AnnotatedFor; diff --git a/checker/jtreg/subpackages/af/sub/deep/Deep.java b/checker/jtreg/subpackages/af/sub/deep/Deep.java new file mode 100644 index 000000000000..bafb339edfa5 --- /dev/null +++ b/checker/jtreg/subpackages/af/sub/deep/Deep.java @@ -0,0 +1,17 @@ +package af.sub.deep; + +import org.checkerframework.checker.nullness.qual.Nullable; + +/** + * Package af.sub sets applyToSubpackages=false, which limits its own annotation to af.sub. It does + * not block package af, whose annotation applies to subpackages and so still reaches here. This + * code is therefore inside an AnnotatedFor scope and its warnings are issued; if the walk up the + * package chain stopped at af.sub, conservative defaults would suppress them. + */ +public class Deep { + void take(Object nn) {} + + void m(@Nullable Object nble) { + take(nble); + } +} diff --git a/checker/jtreg/subpackages/af/sub/package-info.java b/checker/jtreg/subpackages/af/sub/package-info.java new file mode 100644 index 000000000000..0498d465e285 --- /dev/null +++ b/checker/jtreg/subpackages/af/sub/package-info.java @@ -0,0 +1,4 @@ +@AnnotatedFor(value = "nullness", applyToSubpackages = false) +package af.sub; + +import org.checkerframework.framework.qual.AnnotatedFor; diff --git a/checker/jtreg/subpackages/hqp/package-info.java b/checker/jtreg/subpackages/hqp/package-info.java new file mode 100644 index 000000000000..951717241071 --- /dev/null +++ b/checker/jtreg/subpackages/hqp/package-info.java @@ -0,0 +1,5 @@ +@HasQualifierParameter(Tainted.class) +package hqp; + +import org.checkerframework.checker.tainting.qual.Tainted; +import org.checkerframework.framework.qual.HasQualifierParameter; diff --git a/checker/jtreg/subpackages/hqp/sub/deep/Deep.java b/checker/jtreg/subpackages/hqp/sub/deep/Deep.java new file mode 100644 index 000000000000..838cd551ab61 --- /dev/null +++ b/checker/jtreg/subpackages/hqp/sub/deep/Deep.java @@ -0,0 +1,13 @@ +package hqp.sub.deep; + +import org.checkerframework.checker.tainting.qual.PolyTainted; + +/** + * Package hqp.sub sets applyToSubpackages=false, which limits its own annotation to hqp.sub. It + * does not block package hqp, whose annotation applies to subpackages and so still reaches here. + * The class therefore has a qualifier parameter and the polymorphic qualifier is allowed; if the + * walk up the package chain stopped at hqp.sub, this would be invalid.polymorphic.qualifier.use. + */ +public class Deep { + @PolyTainted int field; +} diff --git a/checker/jtreg/subpackages/hqp/sub/package-info.java b/checker/jtreg/subpackages/hqp/sub/package-info.java new file mode 100644 index 000000000000..02fbc2aee00d --- /dev/null +++ b/checker/jtreg/subpackages/hqp/sub/package-info.java @@ -0,0 +1,5 @@ +@HasQualifierParameter(value = Tainted.class, applyToSubpackages = false) +package hqp.sub; + +import org.checkerframework.checker.tainting.qual.Tainted; +import org.checkerframework.framework.qual.HasQualifierParameter; diff --git a/checker/tests/tainting/hqpoptin/InNestedSubpackage.java b/checker/tests/tainting/hqpoptin/InNestedSubpackage.java new file mode 100644 index 000000000000..163ab18c4e5f --- /dev/null +++ b/checker/tests/tainting/hqpoptin/InNestedSubpackage.java @@ -0,0 +1,9 @@ +package hqpoptin.sub.nested; + +import org.checkerframework.checker.tainting.qual.PolyTainted; + +// applyToSubpackages defaults to true, so the qualifier parameter reaches transitively nested +// subpackages. +public class InNestedSubpackage { + @PolyTainted int field; +} diff --git a/checker/tests/tainting/hqpoptin/InSubpackage.java b/checker/tests/tainting/hqpoptin/InSubpackage.java new file mode 100644 index 000000000000..79218845208f --- /dev/null +++ b/checker/tests/tainting/hqpoptin/InSubpackage.java @@ -0,0 +1,9 @@ +package hqpoptin.sub; + +import org.checkerframework.checker.tainting.qual.PolyTainted; + +// applyToSubpackages defaults to true, so this class inherits the qualifier parameter from package +// hqpoptin and the polymorphic qualifier is allowed. +public class InSubpackage { + @PolyTainted int field; +} diff --git a/checker/tests/tainting/hqpoptin/package-info.java b/checker/tests/tainting/hqpoptin/package-info.java new file mode 100644 index 000000000000..1f825895f366 --- /dev/null +++ b/checker/tests/tainting/hqpoptin/package-info.java @@ -0,0 +1,7 @@ +// This file and the class beside it are deliberately in one directory so that the package +// annotation and the subpackage class are compiled together. +@HasQualifierParameter(Tainted.class) +package hqpoptin; + +import org.checkerframework.checker.tainting.qual.Tainted; +import org.checkerframework.framework.qual.HasQualifierParameter; diff --git a/checker/tests/tainting/hqpoptout/InNestedSubpackage.java b/checker/tests/tainting/hqpoptout/InNestedSubpackage.java new file mode 100644 index 000000000000..677c77af5c2c --- /dev/null +++ b/checker/tests/tainting/hqpoptout/InNestedSubpackage.java @@ -0,0 +1,9 @@ +package hqpoptout.sub.nested; + +import org.checkerframework.checker.tainting.qual.PolyTainted; + +// The enclosing package opts out of all subpackages, including transitively nested ones. +public class InNestedSubpackage { + // :: error: (invalid.polymorphic.qualifier.use) + @PolyTainted int field; +} diff --git a/checker/tests/tainting/hqpoptout/InPackage.java b/checker/tests/tainting/hqpoptout/InPackage.java new file mode 100644 index 000000000000..71bf437a24e0 --- /dev/null +++ b/checker/tests/tainting/hqpoptout/InPackage.java @@ -0,0 +1,9 @@ +package hqpoptout; + +import org.checkerframework.checker.tainting.qual.PolyTainted; + +// The package's @HasQualifierParameter still covers the package itself, so a polymorphic +// qualifier may be written on this field. +public class InPackage { + @PolyTainted int field; +} diff --git a/checker/tests/tainting/hqpoptout/InSubpackage.java b/checker/tests/tainting/hqpoptout/InSubpackage.java new file mode 100644 index 000000000000..c30b98a2cf79 --- /dev/null +++ b/checker/tests/tainting/hqpoptout/InSubpackage.java @@ -0,0 +1,10 @@ +package hqpoptout.sub; + +import org.checkerframework.checker.tainting.qual.PolyTainted; + +// Package hqpoptout sets applyToSubpackages=false, so this class has no qualifier parameter and +// the polymorphic qualifier is rejected. +public class InSubpackage { + // :: error: (invalid.polymorphic.qualifier.use) + @PolyTainted int field; +} diff --git a/checker/tests/tainting/hqpoptout/package-info.java b/checker/tests/tainting/hqpoptout/package-info.java new file mode 100644 index 000000000000..0344f9494897 --- /dev/null +++ b/checker/tests/tainting/hqpoptout/package-info.java @@ -0,0 +1,7 @@ +// This file and the classes beside it are deliberately in one directory so that the package +// annotation and the subpackage class are compiled together. +@HasQualifierParameter(value = Tainted.class, applyToSubpackages = false) +package hqpoptout; + +import org.checkerframework.checker.tainting.qual.Tainted; +import org.checkerframework.framework.qual.HasQualifierParameter; diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 3ceff2562c1f..9496c9063f6b 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -52,6 +52,13 @@ alternative to running it as a standalone annotation processor. It is published `io.github.eisop:framework-errorprone` and requires JDK 21 or later. See the manual's "Error Prone" section. +`AnnotatedFor`, `HasQualifierParameter`, and `ReportUse` gain the +`applyToSubpackages` element that `DefaultQualifier` already had. It says whether +an annotation written on a package also applies to that package's subpackages, +and defaults to `true`, so existing code is unaffected. Setting it to false limits +only that annotation; an applicable annotation on an enclosing package still +applies. + The Checker Framework now issues an `annotation.on.supertype` error when an annotation supported by the checker is written as a main annotation on the superclass or interface in an `extends` or `implements` clause. Annotations on the supertype's type arguments remain permitted. A checker @@ -408,6 +415,15 @@ through fix-carrying overloads, which are `private`. Host-side interception of diagnostics is done by installing a `DiagnosticSink`, not by overriding `printOrStoreMessage`. +Code that walks up the package chain looking for a package annotation must now gate +each step to an enclosing package on that annotation's `applyToSubpackages` element; +the annotated package itself is always in scope. There are two new methods for this: +`AnnotationUtils.appliesToSubpackages(AnnotationMirror, ExecutableElement)`, and +`AnnotatedTypeFactory.doesAnnotatedForApplyToSubpackages(AnnotationMirror)` for +`@AnnotatedFor`. A null element, as in a `checker-qual` that predates it, is treated +as true, so a package annotation from such an artifact applies to subpackages as it +always did. + `AnnotatedIntersectionType.summarizeBounds` computes the summary described above, reading each bound's qualifier, explicit or defaulted, uniformly, and folding @@ -765,7 +781,8 @@ eisop#104, eisop#386, eisop#433, eisop#622, eisop#737, eisop#778, eisop#786, eisop#792, eisop#863, eisop#949, eisop#1015, eisop#1059, eisop#1074, eisop#1244, eisop#1315, eisop#1564, eisop#1592, eisop#1642, eisop#1653, eisop#1735, eisop#1801, eisop#1818, eisop#1819, eisop#1861, eisop#1862, eisop#1863, -eisop#1865, eisop#1887, eisop#1965, eisop#1987, typetools#399, typetools#3203. +eisop#1865, eisop#1887, eisop#1965, eisop#1987, eisop#1990, typetools#399, +typetools#3203. Version 3.49.5-eisop1 (April 26, 2026) diff --git a/docs/manual/advanced-features.tex b/docs/manual/advanced-features.tex index 6cefdfb98fb9..e197375a0714 100644 --- a/docs/manual/advanced-features.tex +++ b/docs/manual/advanced-features.tex @@ -322,7 +322,9 @@ If \code{@DefaultQualifier}[\code{s}] is placed on a package (via the \ file), then it applies to the given package \emph{and} -all subpackages. +all subpackages. To limit a default to the package it is written on, set +the \ element to false, as in +\<@DefaultQualifier(value = NonNull.class, applyToSubpackages = false)>. % This is slightly at odds with Java's treatment of packages of different % names as essentially unrelated, but is more intuitive and useful. diff --git a/docs/manual/annotating-libraries.tex b/docs/manual/annotating-libraries.tex index b6ce89dc8fd8..e6c84bc03334 100644 --- a/docs/manual/annotating-libraries.tex +++ b/docs/manual/annotating-libraries.tex @@ -436,6 +436,17 @@ any annotations, but that you examined the source code and verified that all appropriate annotations are present. +\<@AnnotatedFor> may also be written on a package, via the +\ file, in which case it applies to the given package +\emph{and} all subpackages. To limit it to the package it is written on, +set the \ element to false, as in +\<@AnnotatedFor(value = "nullness", applyToSubpackages = false)>. +This limits only that annotation; it does not block an applicable +\<@AnnotatedFor> on an enclosing package. +Because \<@AnnotatedFor> has source retention, a package annotation only +affects compilation units that are compiled together with its +\ file. + \begin{sloppypar} Whenever you compile a class using the Checker Framework, including when using the \<-AuseConservativeDefaultsForUncheckedCode=source,bytecode> command-line diff --git a/docs/manual/generics.tex b/docs/manual/generics.tex index e517bd34224b..cc54a4a8a37e 100644 --- a/docs/manual/generics.tex +++ b/docs/manual/generics.tex @@ -1051,6 +1051,12 @@ package mypackage; \end{Verbatim} +To limit \<@HasQualifierParameter> to the package it is written on, set the +\ element to false, as in +\<@HasQualifierParameter(value = Tainted.class, applyToSubpackages = false)>. +This limits only that annotation; it does not block an applicable +\<@HasQualifierParameter> on an enclosing package. + When using \<@HasQualifierParameter> on a package, it's possible to disable it for a specific class using \refqualclass{framework/qual}{NoQualifierParameter}. Writing this on a class indicates it has no class qualifier parameter and diff --git a/framework/jtreg/subpackages/ReportUseNested.java b/framework/jtreg/subpackages/ReportUseNested.java new file mode 100644 index 000000000000..f8f701afa9e9 --- /dev/null +++ b/framework/jtreg/subpackages/ReportUseNested.java @@ -0,0 +1,8 @@ +/* + * @test + * @summary A ReportUse with applyToSubpackages=false limits only its own annotation. An enclosing + * package whose annotation applies to subpackages still reaches through it. + * + * @compile/fail/ref=ReportUseNested.out -XDrawDiagnostics -processor org.checkerframework.common.util.report.ReportChecker ru/package-info.java ru/sub/package-info.java ru/sub/deep/Deep.java + */ +public class ReportUseNested {} diff --git a/framework/jtreg/subpackages/ReportUseNested.out b/framework/jtreg/subpackages/ReportUseNested.out new file mode 100644 index 000000000000..72306ac32c6e --- /dev/null +++ b/framework/jtreg/subpackages/ReportUseNested.out @@ -0,0 +1,3 @@ +Deep.java:8:8: compiler.err.proc.messager: [usage] Usage of ru [PACKAGE] by ru.sub.deep.Deep [CLASS] +Deep.java:8:8: compiler.err.proc.messager: [usage] Usage of ru [PACKAGE] by ru.sub.deep.Deep [CLASS] +2 errors diff --git a/framework/jtreg/subpackages/ru/package-info.java b/framework/jtreg/subpackages/ru/package-info.java new file mode 100644 index 000000000000..5967008992df --- /dev/null +++ b/framework/jtreg/subpackages/ru/package-info.java @@ -0,0 +1,4 @@ +@ReportUse +package ru; + +import org.checkerframework.common.util.report.qual.ReportUse; diff --git a/framework/jtreg/subpackages/ru/sub/deep/Deep.java b/framework/jtreg/subpackages/ru/sub/deep/Deep.java new file mode 100644 index 000000000000..ce85c36b2b9b --- /dev/null +++ b/framework/jtreg/subpackages/ru/sub/deep/Deep.java @@ -0,0 +1,8 @@ +package ru.sub.deep; + +/** + * Package ru.sub sets applyToSubpackages=false, which limits its own annotation to ru.sub. It does + * not block package ru, whose annotation applies to subpackages and so still reaches here, so this + * class is reported. + */ +public class Deep {} diff --git a/framework/jtreg/subpackages/ru/sub/package-info.java b/framework/jtreg/subpackages/ru/sub/package-info.java new file mode 100644 index 000000000000..baaa3969e497 --- /dev/null +++ b/framework/jtreg/subpackages/ru/sub/package-info.java @@ -0,0 +1,4 @@ +@ReportUse(applyToSubpackages = false) +package ru.sub; + +import org.checkerframework.common.util.report.qual.ReportUse; diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java index ae058151e952..e7e55c7a964b 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java @@ -81,6 +81,16 @@ public abstract class BaseTypeChecker extends SourceChecker { private final IdentityHashMap elementAnnotatedForThisCheckerOrUpstreamCache = new IdentityHashMap<>(); + /** + * A mapping from a package to whether that package's subpackages are covered by an + * {@code @AnnotatedFor} for this checker or an upstream checker, written on it or on an + * enclosing package. Separate from {@link #elementAnnotatedForThisCheckerOrUpstreamCache} + * because an {@code @AnnotatedFor} that opts out of subpackages still covers its own package, + * so the two answers differ for the same package. + */ + private final IdentityHashMap annotatedForReachesSubpackagesCache = + new IdentityHashMap<>(); + /** An array containing just {@code BaseTypeChecker.class}. */ protected static Class[] baseTypeCheckerClassArray = new Class[] {BaseTypeChecker.class}; @@ -345,21 +355,55 @@ public boolean isElementAnnotatedForThisCheckerOrUpstreamChecker(@Nullable Eleme && atypeFactory.doesAnnotatedForApplyToThisChecker(annotatedFor); if (!elementAnnotatedForThisChecker) { - Element parent; if (elt.getKind() == ElementKind.PACKAGE) { - parent = - ElementUtils.parentPackage( - (PackageElement) elt, atypeFactory.getElementUtils()); + // A package is covered by an enclosing package only if that package's + // @AnnotatedFor applies to subpackages. + elementAnnotatedForThisChecker = + doesAnnotatedForReachSubpackages( + ElementUtils.parentPackage( + (PackageElement) elt, atypeFactory.getElementUtils())); } else { - parent = elt.getEnclosingElement(); - } - - if (parent != null && isElementAnnotatedForThisCheckerOrUpstreamChecker(parent)) { - elementAnnotatedForThisChecker = true; + // A non-package element is inside its enclosing element rather than in a + // subpackage of it, so applyToSubpackages does not apply to this step. + Element parent = elt.getEnclosingElement(); + elementAnnotatedForThisChecker = + parent != null && isElementAnnotatedForThisCheckerOrUpstreamChecker(parent); } } elementAnnotatedForThisCheckerOrUpstreamCache.put(elt, elementAnnotatedForThisChecker); return elementAnnotatedForThisChecker; } + + /** + * Returns true if the subpackages of {@code pkg} are covered by an {@code @AnnotatedFor} for + * this checker or an upstream checker. Such an annotation may be written on {@code pkg} itself + * or on any enclosing package: a package that opts out of subpackages does not shield its own + * subpackages from an enclosing package that opts in. + * + * @param pkg a package, or null for no package + * @return true if an {@code @AnnotatedFor} covers the subpackages of {@code pkg} + */ + private boolean doesAnnotatedForReachSubpackages(@Nullable PackageElement pkg) { + if (pkg == null) { + return false; + } + + Boolean cached = annotatedForReachesSubpackagesCache.get(pkg); + if (cached != null) { + return cached; + } + + AnnotatedTypeFactory atypeFactory = getTypeFactory(); + AnnotationMirror annotatedFor = atypeFactory.getDeclAnnotation(pkg, AnnotatedFor.class); + boolean result = + (annotatedFor != null + && atypeFactory.doesAnnotatedForApplyToThisChecker(annotatedFor) + && atypeFactory.doesAnnotatedForApplyToSubpackages(annotatedFor)) + || doesAnnotatedForReachSubpackages( + ElementUtils.parentPackage(pkg, atypeFactory.getElementUtils())); + + annotatedForReachesSubpackagesCache.put(pkg, result); + return result; + } } diff --git a/framework/src/main/java/org/checkerframework/common/util/report/ReportVisitor.java b/framework/src/main/java/org/checkerframework/common/util/report/ReportVisitor.java index 0b34c8acb5b4..3249d29be8a7 100644 --- a/framework/src/main/java/org/checkerframework/common/util/report/ReportVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/util/report/ReportVisitor.java @@ -29,6 +29,7 @@ import org.checkerframework.framework.type.AnnotatedTypeFactory; import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedDeclaredType; import org.checkerframework.framework.util.AnnotatedTypes; +import org.checkerframework.javacutil.AnnotationUtils; import org.checkerframework.javacutil.ElementUtils; import org.checkerframework.javacutil.TreeUtils; @@ -37,6 +38,7 @@ import java.util.Locale; import java.util.Map; +import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.ExecutableElement; @@ -53,9 +55,26 @@ public class ReportVisitor extends BaseTypeVisitor { /** The modifiers that should be reported; may be null. */ private final @Nullable EnumSet modifiers; + /** + * The {@link ReportUse#applyToSubpackages()} element, or null if the checker-qual version on + * the classpath predates that element. + */ + private final @Nullable ExecutableElement reportUseApplyToSubpackagesElement; + + /** + * Creates a ReportVisitor. + * + * @param checker the checker + */ public ReportVisitor(BaseTypeChecker checker) { super(checker); + reportUseApplyToSubpackagesElement = + TreeUtils.getMethodOrNull( + ReportUse.class, + "applyToSubpackages", + 0, + checker.getProcessingEnvironment()); EnumSet treeKindsTmp = EnumSet.noneOf(Tree.Kind.class); for (String treeKind : checker.getStringsOption("reportTreeKinds", ',')) { treeKindsTmp.add(Tree.Kind.valueOf(treeKind.toUpperCase(Locale.ROOT))); @@ -87,10 +106,16 @@ public Void scan(Tree tree, Void p) { * @param member the element from which to start looking */ private void checkReportUse(Tree tree, Element member) { - Element loop = member; - while (loop != null) { - boolean report = this.atypeFactory.getDeclAnnotation(loop, ReportUse.class) != null; - if (report) { + // Once the walk moves from a package to its parent, an annotation applies only if it + // applies to subpackages. Everything before that -- the member, its enclosing types, and + // its own package -- is covered by an annotation written on it. + boolean inEnclosingPackage = false; + for (Element loop = member; loop != null; ) { + AnnotationMirror reportUse = this.atypeFactory.getDeclAnnotation(loop, ReportUse.class); + if (reportUse != null + && (!inEnclosingPackage + || AnnotationUtils.appliesToSubpackages( + reportUse, reportUseApplyToSubpackagesElement))) { checker.reportError( tree, "usage", @@ -99,15 +124,16 @@ private void checkReportUse(Tree tree, Element member) { loop.getKind(), ElementUtils.getQualifiedName(member), member.getKind()); - break; + return; + } + if (loop.getKind() == ElementKind.PACKAGE) { + loop = ElementUtils.parentPackage((PackageElement) loop, elements); + inEnclosingPackage = true; } else { - if (loop.getKind() == ElementKind.PACKAGE) { - loop = ElementUtils.parentPackage((PackageElement) loop, elements); - continue; - } + // The enclosing element of a top-level type is its package, so once the walk + // reaches a package it stays in packages until it runs out. + loop = loop.getEnclosingElement(); } - // Package will always be the last iteration. - loop = loop.getEnclosingElement(); } } diff --git a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java index 92992a67cd0d..959e188605a3 100644 --- a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java +++ b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java @@ -206,6 +206,13 @@ public class AnnotatedTypeFactory implements AnnotationProvider { /** The AnnotatedFor.value argument/element. */ protected final ExecutableElement annotatedForValueElement; + /** + * The AnnotatedFor.applyToSubpackages() field/element. Null if the version of + * {@code @AnnotatedFor} on the classpath predates this element, in which case an + * {@code @AnnotatedFor} on a package always applies to subpackages. + */ + protected final @Nullable ExecutableElement annotatedForApplyToSubpackagesElement; + /** The EnsuresQualifier.expression field/element. */ protected final ExecutableElement ensuresQualifierExpressionElement; @@ -230,6 +237,13 @@ public class AnnotatedTypeFactory implements AnnotationProvider { /** The HasQualifierParameter.value field/element. */ protected final ExecutableElement hasQualifierParameterValueElement; + /** + * The HasQualifierParameter.applyToSubpackages() field/element. Null if the version of + * {@code @HasQualifierParameter} on the classpath predates this element, in which case a + * {@code @HasQualifierParameter} on a package always applies to subpackages. + */ + protected final @Nullable ExecutableElement hasQualifierParameterApplyToSubpackagesElement; + /** The MethodVal.className argument/element. */ public final ExecutableElement methodValClassNameElement; @@ -800,6 +814,9 @@ public AnnotatedTypeFactory(BaseTypeChecker checker) { annotatedForValueElement = TreeUtils.getMethod(AnnotatedFor.class, "value", 0, processingEnv); + annotatedForApplyToSubpackagesElement = + TreeUtils.getMethodOrNull( + AnnotatedFor.class, "applyToSubpackages", 0, processingEnv); ensuresQualifierExpressionElement = TreeUtils.getMethod(EnsuresQualifier.class, "expression", 0, processingEnv); ensuresQualifierListValueElement = @@ -816,6 +833,9 @@ public AnnotatedTypeFactory(BaseTypeChecker checker) { TreeUtils.getMethod(FieldInvariant.class, "qualifier", 0, processingEnv); hasQualifierParameterValueElement = TreeUtils.getMethod(HasQualifierParameter.class, "value", 0, processingEnv); + hasQualifierParameterApplyToSubpackagesElement = + TreeUtils.getMethodOrNull( + HasQualifierParameter.class, "applyToSubpackages", 0, processingEnv); methodValClassNameElement = TreeUtils.getMethod(MethodVal.class, "className", 0, processingEnv); methodValMethodNameElement = @@ -5481,17 +5501,23 @@ public AnnotationMirrorSet getQualifierParameterHierarchies(Element element) { element, HasQualifierParameter.class, hasQualifierParameterValueElement)); AnnotationMirrorSet hasQualifierParameterTops = new AnnotationMirrorSet(); PackageElement packageElement = ElementUtils.enclosingPackage(element); - - // Traverse all packages containing this element. + // Traverse all packages containing this element. The element's own package always + // applies; an enclosing package applies only if its annotation applies to subpackages. + boolean isOwnPackage = true; while (packageElement != null) { - AnnotationMirrorSet packageDefaultTops = - getSupportedAnnotationsInElementAnnotation( - packageElement, - HasQualifierParameter.class, - hasQualifierParameterValueElement); - hasQualifierParameterTops.addAll(packageDefaultTops); - + AnnotationMirror hasQualifierParameter = + getDeclAnnotation(packageElement, HasQualifierParameter.class); + if (hasQualifierParameter != null + && (isOwnPackage + || AnnotationUtils.appliesToSubpackages( + hasQualifierParameter, + hasQualifierParameterApplyToSubpackagesElement))) { + hasQualifierParameterTops.addAll( + getSupportedAnnotationsInAnnotation( + hasQualifierParameter, hasQualifierParameterValueElement)); + } packageElement = ElementUtils.parentPackage(packageElement, elements); + isOwnPackage = false; } AnnotationMirrorSet noQualifierParamClasses = @@ -5532,7 +5558,20 @@ private AnnotationMirrorSet getSupportedAnnotationsInElementAnnotation( if (annotation == null) { return AnnotationMirrorSet.emptySet(); } + return getSupportedAnnotationsInAnnotation(annotation, valueElement); + } + /** + * Returns the supported annotation mirrors named by {@code valueElement} of {@code annotation}. + * The same as {@link #getSupportedAnnotationsInElementAnnotation}, for a caller that already + * holds the annotation. + * + * @param annotation an annotation whose {@code valueElement} names annotation classes + * @param valueElement the element of {@code annotation} whose value is a list of classes + * @return the supported annotations named by {@code valueElement} + */ + private AnnotationMirrorSet getSupportedAnnotationsInAnnotation( + AnnotationMirror annotation, ExecutableElement valueElement) { AnnotationMirrorSet found = new AnnotationMirrorSet(); List<@CanonicalName Name> qualClasses = AnnotationUtils.getElementValueClassNames(annotation, valueElement); @@ -6852,6 +6891,19 @@ protected void makeConditionConsistentWithOtherMethod( } */ + /** + * Does {@code annotatedForAnno}, which is an {@link + * org.checkerframework.framework.qual.AnnotatedFor} annotation written on a package, also apply + * to subpackages of that package? + * + * @param annotatedForAnno an {@link AnnotatedFor} annotation written on a package + * @return whether {@code annotatedForAnno} applies to subpackages + */ + public boolean doesAnnotatedForApplyToSubpackages(AnnotationMirror annotatedForAnno) { + return AnnotationUtils.appliesToSubpackages( + annotatedForAnno, annotatedForApplyToSubpackagesElement); + } + /** * Does {@code annotatedForAnno}, which is an {@link * org.checkerframework.framework.qual.AnnotatedFor} annotation, apply to this checker? diff --git a/framework/tests/conservative-defaults/annotatedfor/subpkgoptin/InNestedSubpackage.java b/framework/tests/conservative-defaults/annotatedfor/subpkgoptin/InNestedSubpackage.java new file mode 100644 index 000000000000..a7c93a2db4f5 --- /dev/null +++ b/framework/tests/conservative-defaults/annotatedfor/subpkgoptin/InNestedSubpackage.java @@ -0,0 +1,13 @@ +package afoptin.sub.nested; + +import org.checkerframework.framework.testchecker.util.SubQual; +import org.checkerframework.framework.testchecker.util.SuperQual; + +// applyToSubpackages defaults to true, so package afoptin's @AnnotatedFor reaches transitively +// nested subpackages. +public class InNestedSubpackage { + void m() { + // :: error: (assignment.type.incompatible) + @SubQual Object o = new @SuperQual Object(); + } +} diff --git a/framework/tests/conservative-defaults/annotatedfor/subpkgoptin/InPackage.java b/framework/tests/conservative-defaults/annotatedfor/subpkgoptin/InPackage.java new file mode 100644 index 000000000000..c917ca9431c8 --- /dev/null +++ b/framework/tests/conservative-defaults/annotatedfor/subpkgoptin/InPackage.java @@ -0,0 +1,11 @@ +package afoptin; + +import org.checkerframework.framework.testchecker.util.SubQual; +import org.checkerframework.framework.testchecker.util.SuperQual; + +public class InPackage { + void m() { + // :: error: (assignment.type.incompatible) + @SubQual Object o = new @SuperQual Object(); + } +} diff --git a/framework/tests/conservative-defaults/annotatedfor/subpkgoptin/InSubpackage.java b/framework/tests/conservative-defaults/annotatedfor/subpkgoptin/InSubpackage.java new file mode 100644 index 000000000000..d39320f29a62 --- /dev/null +++ b/framework/tests/conservative-defaults/annotatedfor/subpkgoptin/InSubpackage.java @@ -0,0 +1,13 @@ +package afoptin.sub; + +import org.checkerframework.framework.testchecker.util.SubQual; +import org.checkerframework.framework.testchecker.util.SuperQual; + +// applyToSubpackages defaults to true, so package afoptin's @AnnotatedFor reaches this subpackage +// and its subtyping warnings are issued. +public class InSubpackage { + void m() { + // :: error: (assignment.type.incompatible) + @SubQual Object o = new @SuperQual Object(); + } +} diff --git a/framework/tests/conservative-defaults/annotatedfor/subpkgoptin/package-info.java b/framework/tests/conservative-defaults/annotatedfor/subpkgoptin/package-info.java new file mode 100644 index 000000000000..26873e01fba4 --- /dev/null +++ b/framework/tests/conservative-defaults/annotatedfor/subpkgoptin/package-info.java @@ -0,0 +1,7 @@ +// This file and the classes beside it are deliberately in one directory: @AnnotatedFor is +// source-retention, so a package annotation only reaches other compilation units when its +// package-info is compiled in the same run. +@AnnotatedFor("subtyping") +package afoptin; + +import org.checkerframework.framework.qual.AnnotatedFor; diff --git a/framework/tests/conservative-defaults/annotatedfor/subpkgoptout/InNestedSubpackage.java b/framework/tests/conservative-defaults/annotatedfor/subpkgoptout/InNestedSubpackage.java new file mode 100644 index 000000000000..6782bbc5fbab --- /dev/null +++ b/framework/tests/conservative-defaults/annotatedfor/subpkgoptout/InNestedSubpackage.java @@ -0,0 +1,11 @@ +package afoptout.sub.nested; + +import org.checkerframework.framework.testchecker.util.SubQual; +import org.checkerframework.framework.testchecker.util.SuperQual; + +// The enclosing package opts out of all subpackages, including transitively nested ones. +public class InNestedSubpackage { + void m() { + @SubQual Object o = new @SuperQual Object(); + } +} diff --git a/framework/tests/conservative-defaults/annotatedfor/subpkgoptout/InPackage.java b/framework/tests/conservative-defaults/annotatedfor/subpkgoptout/InPackage.java new file mode 100644 index 000000000000..d53dccf3fc8b --- /dev/null +++ b/framework/tests/conservative-defaults/annotatedfor/subpkgoptout/InPackage.java @@ -0,0 +1,13 @@ +package afoptout; + +import org.checkerframework.framework.testchecker.util.SubQual; +import org.checkerframework.framework.testchecker.util.SuperQual; + +// Opting out of subpackages does not opt the annotated package itself out, so this code is in an +// @AnnotatedFor scope and its subtyping warnings are issued. +public class InPackage { + void m() { + // :: error: (assignment.type.incompatible) + @SubQual Object o = new @SuperQual Object(); + } +} diff --git a/framework/tests/conservative-defaults/annotatedfor/subpkgoptout/InSubpackage.java b/framework/tests/conservative-defaults/annotatedfor/subpkgoptout/InSubpackage.java new file mode 100644 index 000000000000..d5e36dbf249d --- /dev/null +++ b/framework/tests/conservative-defaults/annotatedfor/subpkgoptout/InSubpackage.java @@ -0,0 +1,12 @@ +package afoptout.sub; + +import org.checkerframework.framework.testchecker.util.SubQual; +import org.checkerframework.framework.testchecker.util.SuperQual; + +// Package afoptout sets applyToSubpackages=false, so this code is outside any @AnnotatedFor scope +// and conservative defaults suppress its warnings. No error is expected below. +public class InSubpackage { + void m() { + @SubQual Object o = new @SuperQual Object(); + } +} diff --git a/framework/tests/conservative-defaults/annotatedfor/subpkgoptout/package-info.java b/framework/tests/conservative-defaults/annotatedfor/subpkgoptout/package-info.java new file mode 100644 index 000000000000..60679e76ec01 --- /dev/null +++ b/framework/tests/conservative-defaults/annotatedfor/subpkgoptout/package-info.java @@ -0,0 +1,7 @@ +// This file and the classes beside it are deliberately in one directory: @AnnotatedFor is +// source-retention, so a package annotation only reaches other compilation units when its +// package-info is compiled in the same run. +@AnnotatedFor(value = "subtyping", applyToSubpackages = false) +package afoptout; + +import org.checkerframework.framework.qual.AnnotatedFor; diff --git a/framework/tests/report/reportuseoptin/InInnerPackage.java b/framework/tests/report/reportuseoptin/InInnerPackage.java new file mode 100644 index 000000000000..5f70a5af9a13 --- /dev/null +++ b/framework/tests/report/reportuseoptin/InInnerPackage.java @@ -0,0 +1,4 @@ +package reportuseoptin.inner; + +// :: error: (usage) +public class InInnerPackage {} diff --git a/framework/tests/report/reportuseoptin/InNestedSubpackage.java b/framework/tests/report/reportuseoptin/InNestedSubpackage.java new file mode 100644 index 000000000000..76bd897d808a --- /dev/null +++ b/framework/tests/report/reportuseoptin/InNestedSubpackage.java @@ -0,0 +1,4 @@ +package reportuseoptin.inner.nested; + +// :: error: (usage) +public class InNestedSubpackage {} diff --git a/framework/tests/report/reportuseoptin/Uses.java b/framework/tests/report/reportuseoptin/Uses.java new file mode 100644 index 000000000000..ddb59bcb8c04 --- /dev/null +++ b/framework/tests/report/reportuseoptin/Uses.java @@ -0,0 +1,9 @@ +public class Uses { + // The default true reaches a direct subpackage. + // :: error: (usage) + reportuseoptin.inner.InInnerPackage inInnerPackage; + + // The default true also reaches transitively nested subpackages. + // :: error: (usage) + reportuseoptin.inner.nested.InNestedSubpackage inNestedSubpackage; +} diff --git a/framework/tests/report/reportuseoptin/package-info.java b/framework/tests/report/reportuseoptin/package-info.java new file mode 100644 index 000000000000..a0df72eb4043 --- /dev/null +++ b/framework/tests/report/reportuseoptin/package-info.java @@ -0,0 +1,4 @@ +@ReportUse +package reportuseoptin; + +import org.checkerframework.common.util.report.qual.ReportUse; diff --git a/framework/tests/report/reportuseoptout/InNestedSubpackage.java b/framework/tests/report/reportuseoptout/InNestedSubpackage.java new file mode 100644 index 000000000000..5611760761d2 --- /dev/null +++ b/framework/tests/report/reportuseoptout/InNestedSubpackage.java @@ -0,0 +1,3 @@ +package reportuseoptout.sub.nested; + +public class InNestedSubpackage {} diff --git a/framework/tests/report/reportuseoptout/InPackage.java b/framework/tests/report/reportuseoptout/InPackage.java new file mode 100644 index 000000000000..ad9e2b80f4a3 --- /dev/null +++ b/framework/tests/report/reportuseoptout/InPackage.java @@ -0,0 +1,4 @@ +package reportuseoptout; + +// :: error: (usage) +public class InPackage {} diff --git a/framework/tests/report/reportuseoptout/Uses.java b/framework/tests/report/reportuseoptout/Uses.java new file mode 100644 index 000000000000..b6b318e12900 --- /dev/null +++ b/framework/tests/report/reportuseoptout/Uses.java @@ -0,0 +1,8 @@ +public class Uses { + // The annotation always covers the package on which it is written. + // :: error: (usage) + reportuseoptout.InPackage inPackage; + + // applyToSubpackages=false also excludes transitively nested packages. + reportuseoptout.sub.nested.InNestedSubpackage inNestedSubpackage; +} diff --git a/framework/tests/report/reportuseoptout/package-info.java b/framework/tests/report/reportuseoptout/package-info.java new file mode 100644 index 000000000000..6decbad4bf5e --- /dev/null +++ b/framework/tests/report/reportuseoptout/package-info.java @@ -0,0 +1,4 @@ +@ReportUse(applyToSubpackages = false) +package reportuseoptout; + +import org.checkerframework.common.util.report.qual.ReportUse; diff --git a/javacutil/src/main/java/org/checkerframework/javacutil/AnnotationUtils.java b/javacutil/src/main/java/org/checkerframework/javacutil/AnnotationUtils.java index 20e4a18b076a..40d559c30b45 100644 --- a/javacutil/src/main/java/org/checkerframework/javacutil/AnnotationUtils.java +++ b/javacutil/src/main/java/org/checkerframework/javacutil/AnnotationUtils.java @@ -968,6 +968,23 @@ public static T getElementValue( } } + /** + * Returns whether an annotation written on a package also applies to that package's + * subpackages. + * + * @param anno an annotation written on a package + * @param applyToSubpackagesElement {@code anno}'s own {@code applyToSubpackages} element, or + * null if the {@code checker-qual} on the classpath predates that element + * @return true if {@code anno} applies to subpackages + */ + public static boolean appliesToSubpackages( + AnnotationMirror anno, @Nullable ExecutableElement applyToSubpackagesElement) { + // A checker-qual without the element gives no way to opt out, so an annotation from it + // applies to subpackages, as it always did. + return applyToSubpackagesElement == null + || getElementValue(anno, applyToSubpackagesElement, Boolean.class, true); + } + /** * Get the given boolean element of the annotation {@code anno}. *