Support DefaultQualifiers that do not apply to subpackages

checker-qual/src/main/java/org/checkerframework/framework/qual/DefaultQualifier.java

@@ -26,6 +26,9 @@
  *   class MyClass { ... }
  * </pre>
  *
+ * <p>Defaults on a package also apply to subpackages, unless the {@code applyToSubpackages} field
+ * is set to false.
+ *
  * <p>This annotation currently has no effect in stub files.
  *
  * @see org.checkerframework.framework.qual.TypeUseLocation
@@ -63,6 +66,13 @@
    */
   TypeUseLocation[] locations() default {TypeUseLocation.ALL};
 
+  /**
+   * When used on a package, whether the defaults should also apply to subpackages.
+   *
+   * @return whether the default should be inherited by subpackages
+   */
+  boolean applyToSubpackages() default true;
+
   /**
    * A wrapper annotation that makes the {@link DefaultQualifier} annotation repeatable.
    *

docs/CHANGELOG.md

@@ -3,6 +3,10 @@ Version 3.48.2 (November 1, 2024)
 
 **User-visible changes:**
 
+`DefaultQualifier` supports the new `applyToSubpackages` annotation attribute
+to decide whether a default should also apply to subpackages.  To preserve the
+current behavior, the default is `true`.
+
 **Implementation details:**
 
 **Closed issues:**

framework/src/main/java/org/checkerframework/framework/util/defaults/Default.java

@@ -21,25 +21,40 @@ public class Default implements Comparable<Default> {
   /** The type use location. */
   public final TypeUseLocation location;
 
+  /** Whether the default should be inherited by subpackages. */
+  public final boolean applyToSubpackages;
+
   /**
    * Construct a Default object.
    *
    * @param anno the default annotation mirror
    * @param location the type use location
+   * @param applyToSubpackages whether the default should be inherited by subpackages
    */
-  public Default(AnnotationMirror anno, TypeUseLocation location) {
+  public Default(
+      final AnnotationMirror anno,
+      final TypeUseLocation location,
+      final boolean applyToSubpackages) {
     this.anno = anno;
     this.location = location;
+    this.applyToSubpackages = applyToSubpackages;
   }
 
   @Override
   public int compareTo(Default other) {
     int locationOrder = location.compareTo(other.location);
-    if (locationOrder == 0) {
-      return AnnotationUtils.compareAnnotationMirrors(anno, other.anno);
-    } else {
+    if (locationOrder != 0) {
       return locationOrder;
     }
+    int annoOrder = AnnotationUtils.compareAnnotationMirrors(anno, other.anno);
+    if (annoOrder != 0) {
+      return annoOrder;
+    }
+    if (applyToSubpackages == other.applyToSubpackages) {
+      return 0;
+    } else {
+      return applyToSubpackages ? 1 : -1;
+    }
   }
 
   @Override
@@ -57,11 +72,16 @@ public boolean equals(@Nullable Object thatObj) {
 
   @Override
   public int hashCode() {
-    return Objects.hash(anno, location);
+    return Objects.hash(anno, location, applyToSubpackages);
   }
 
   @Override
   public String toString() {
-    return "( " + location.name() + " => " + anno + " )";
+    return "( "
+        + location.name()
+        + " => "
+        + anno
+        + (applyToSubpackages ? "applies to subpackages" : "")
+        + " )";
   }
 }

framework/src/main/java/org/checkerframework/framework/util/defaults/QualifierDefaults.java

@@ -90,6 +90,9 @@ public class QualifierDefaults {
   /** The locations() element/field of a @DefaultQualifier annotation. */
   protected final ExecutableElement defaultQualifierLocationsElement;
 
+  /** The applyToSubpackages() element/field of a @DefaultQualifier annotation. */
+  protected final ExecutableElement defaultQualifierApplyToSubpackagesElement;
+
   /** The value() element/field of a @DefaultQualifier.List annotation. */
   protected final ExecutableElement defaultQualifierListValueElement;
 
@@ -189,6 +192,8 @@ public QualifierDefaults(Elements elements, AnnotatedTypeFactory atypeFactory) {
         TreeUtils.getMethod(DefaultQualifier.class, "value", 0, processingEnv);
     this.defaultQualifierLocationsElement =
         TreeUtils.getMethod(DefaultQualifier.class, "locations", 0, processingEnv);
+    this.defaultQualifierApplyToSubpackagesElement =
+        TreeUtils.getMethod(DefaultQualifier.class, "applyToSubpackages", 0, processingEnv);
     this.defaultQualifierListValueElement =
         TreeUtils.getMethod(DefaultQualifier.List.class, "value", 0, processingEnv);
   }
@@ -277,25 +282,50 @@ public void addClimbStandardDefaults() {
    *
    * @param absoluteDefaultAnno the default annotation mirror
    * @param location the type use location
+   * @param applyToSubpackages whether the default should be inherited by subpackages
    */
   public void addCheckedCodeDefault(
-      AnnotationMirror absoluteDefaultAnno, TypeUseLocation location) {
+      AnnotationMirror absoluteDefaultAnno, TypeUseLocation location, boolean applyToSubpackages) {
     checkDuplicates(checkedCodeDefaults, absoluteDefaultAnno, location);
-    checkedCodeDefaults.add(new Default(absoluteDefaultAnno, location));
+    checkedCodeDefaults.add(new Default(absoluteDefaultAnno, location, applyToSubpackages));
+  }
+
+  /**
+   * Adds a default annotation that also applies to subpackages, if applicable. A programmer may
+   * override this by writing the @DefaultQualifier annotation on an element.
+   *
+   * @param absoluteDefaultAnno the default annotation mirror
+   * @param location the type use location
+   */
+  public void addCheckedCodeDefault(
+      AnnotationMirror absoluteDefaultAnno, TypeUseLocation location) {
+    addCheckedCodeDefault(absoluteDefaultAnno, location, true);
   }
 
   /**
    * Add a default annotation for unchecked elements.
    *
    * @param uncheckedDefaultAnno the default annotation mirror
    * @param location the type use location
+   * @param applyToSubpackages whether the default should be inherited by subpackages
    */
   public void addUncheckedCodeDefault(
-      AnnotationMirror uncheckedDefaultAnno, TypeUseLocation location) {
+      AnnotationMirror uncheckedDefaultAnno, TypeUseLocation location, boolean applyToSubpackages) {
     checkDuplicates(uncheckedCodeDefaults, uncheckedDefaultAnno, location);
     checkIsValidUncheckedCodeLocation(uncheckedDefaultAnno, location);
+    uncheckedCodeDefaults.add(new Default(uncheckedDefaultAnno, location, applyToSubpackages));
+  }
 
-    uncheckedCodeDefaults.add(new Default(uncheckedDefaultAnno, location));
+  /**
+   * Add a default annotation for unchecked elements that also applies to subpackages, if
+   * applicable.
+   *
+   * @param uncheckedDefaultAnno the default annotation mirror
+   * @param location the type use location
+   */
+  public void addUncheckedCodeDefault(
+      AnnotationMirror uncheckedDefaultAnno, TypeUseLocation location) {
+    addUncheckedCodeDefault(uncheckedDefaultAnno, location, true);
   }
 
   /** Sets the default annotation for unchecked elements, with specific locations. */
@@ -320,6 +350,16 @@ public void addCheckedCodeDefaults(
    * @param elementDefaultAnno the default to set
    * @param location the location to apply the default to
    */
+  /*
+   * TODO(cpovirk): This method looks dangerous for a type system to call early: If it "adds" a
+   * default for an Element before defaultsAt runs for that Element, that looks like it would
+   * prevent any @DefaultQualifier or similar annotation from having any effect (because
+   * defaultsAt would short-circuit after discovering that an entry already exists for the
+   * Element). Maybe this method should run defaultsAt before inserting its own entry? Or maybe
+   * it's too early to run defaultsAt? Or maybe we'd see new problems in existing code because
+   * we'd start running checkDuplicates to look for overlap between the @DefaultQualifier defaults
+   * and addElementDefault defaults?
+   */
   public void addElementDefault(
       Element elem, AnnotationMirror elementDefaultAnno, TypeUseLocation location) {
     DefaultSet prevset = elementDefaults.get(elem);
@@ -328,7 +368,8 @@ public void addElementDefault(
     } else {
       prevset = new DefaultSet();
     }
-    prevset.add(new Default(elementDefaultAnno, location));
+    // TODO: expose applyToSubpackages
+    prevset.add(new Default(elementDefaultAnno, location, true));
     elementDefaults.put(elem, prevset);
   }
 
@@ -355,7 +396,8 @@ private void checkDuplicates(
           "Only one qualifier from a hierarchy can be the default. Existing: "
               + previousDefaults
               + " and new: "
-              + new Default(newAnno, newLoc));
+              // TODO: expose applyToSubpackages
+              + new Default(newAnno, newLoc, true));
     }
   }
 
@@ -588,9 +630,13 @@ private void applyDefaults(Tree tree, AnnotatedTypeMirror type) {
               defaultQualifierLocationsElement,
               TypeUseLocation.class,
               defaultQualifierValueDefault);
+      boolean applyToSubpackages =
+          AnnotationUtils.getElementValue(
+              dq, defaultQualifierApplyToSubpackagesElement, Boolean.class, true);
+
       DefaultSet ret = new DefaultSet();
       for (TypeUseLocation loc : locations) {
-        ret.add(new Default(anno, loc));
+        ret.add(new Default(anno, loc, applyToSubpackages));
       }
       return ret;
     } else {
@@ -656,55 +702,40 @@ private DefaultSet defaultsAt(Element elt) {
       return elementDefaults.get(elt);
     }
 
-    DefaultSet qualifiers = null;
-
-    {
-      AnnotationMirror dqAnno = atypeFactory.getDeclAnnotation(elt, DefaultQualifier.class);
-
-      if (dqAnno != null) {
-        qualifiers = new DefaultSet();
-        Set<Default> p = fromDefaultQualifier(dqAnno);
-
-        if (p != null) {
-          qualifiers.addAll(p);
-        }
-      }
-    }
+    DefaultSet qualifiers = defaultsAtDirect(elt);
 
-    {
-      AnnotationMirror dqListAnno =
-          atypeFactory.getDeclAnnotation(elt, DefaultQualifier.List.class);
-      if (dqListAnno != null) {
-        if (qualifiers == null) {
-          qualifiers = new DefaultSet();
-        }
-
-        List<AnnotationMirror> values =
-            AnnotationUtils.getElementValueArray(
-                dqListAnno, defaultQualifierListValueElement, AnnotationMirror.class);
-        for (AnnotationMirror dqAnno : values) {
-          Set<Default> p = fromDefaultQualifier(dqAnno);
-          if (p != null) {
-            qualifiers.addAll(p);
-          }
+    DefaultSet parentDefaults;
+    if (elt.getKind() == ElementKind.PACKAGE) {
+      Element parent = ElementUtils.parentPackage((PackageElement) elt, elements);
+      DefaultSet origParentDefaults = defaultsAt(parent);
+      parentDefaults = new DefaultSet();
+      for (Default d : origParentDefaults) {
+        if (d.applyToSubpackages) {
+          parentDefaults.add(d);
         }
       }
-    }
-
-    Element parent;
-    if (elt.getKind() == ElementKind.PACKAGE) {
-      parent = ElementUtils.parentPackage((PackageElement) elt, elements);
     } else {
-      parent = elt.getEnclosingElement();
+      Element parent = elt.getEnclosingElement();
+      parentDefaults = defaultsAt(parent);
     }
 
-    DefaultSet parentDefaults = defaultsAt(parent);
     if (qualifiers == null || qualifiers.isEmpty()) {
       qualifiers = parentDefaults;
     } else {
+      // TODO(cpovirk): What should happen with conflicts?
       qualifiers.addAll(parentDefaults);
     }
 
+    /* TODO: it would seem more efficient to also cache null/empty as the result.
+     * However, doing so causes KeyFor tests to fail.
+           if (qualifiers == null) {
+               qualifiers = DefaultSet.EMPTY;
+           }
+
+           elementDefaults.put(elt, qualifiers);
+           return qualifiers;
+    */
+
     if (qualifiers != null && !qualifiers.isEmpty()) {
       elementDefaults.put(elt, qualifiers);
       return qualifiers;
@@ -713,6 +744,48 @@ private DefaultSet defaultsAt(Element elt) {
     }
   }
 
+  /**
+   * Returns the defaults that apply directly to the given Element, without considering enclosing
+   * Elements.
+   *
+   * @param elt the element
+   * @return the defaults
+   */
+  private DefaultSet defaultsAtDirect(final Element elt) {
+    DefaultSet qualifiers = null;
+
+    // Handle DefaultQualifier
+    AnnotationMirror dqAnno = atypeFactory.getDeclAnnotation(elt, DefaultQualifier.class);
+
+    if (dqAnno != null) {
+      Set<Default> p = fromDefaultQualifier(dqAnno);
+
+      if (p != null) {
+        qualifiers = new DefaultSet();
+        qualifiers.addAll(p);
+      }
+    }
+
+    // Handle DefaultQualifier.List
+    AnnotationMirror dqListAnno = atypeFactory.getDeclAnnotation(elt, DefaultQualifier.List.class);
+    if (dqListAnno != null) {
+      if (qualifiers == null) {
+        qualifiers = new DefaultSet();
+      }
+      @SuppressWarnings("unchecked")
+      List<AnnotationMirror> values =
+          AnnotationUtils.getElementValue(dqListAnno, defaultQualifierListValueElement, List.class);
+      for (AnnotationMirror dqlAnno : values) {
+        Set<Default> p = fromDefaultQualifier(dqlAnno);
+        if (p != null) {
+          // TODO(cpovirk): What should happen with conflicts?
+          qualifiers.addAll(p);
+        }
+      }
+    }
+    return qualifiers;
+  }
+
   /**
    * Given an element, returns whether the conservative default should be applied for it. Handles
    * elements from bytecode or source code.

framework/tests/h1h2checker/pkg1/PackageDefaulting.java

@@ -0,0 +1,23 @@
+package pkg1;
+
+import org.checkerframework.framework.testchecker.h1h2checker.quals.H1S1;
+import org.checkerframework.framework.testchecker.h1h2checker.quals.H1S2;
+import org.checkerframework.framework.testchecker.h1h2checker.quals.H2S1;
+import org.checkerframework.framework.testchecker.h1h2checker.quals.H2S2;
+
+/* Defaults from package pkg1 apply within the package. */
+public class PackageDefaulting {
+  // Test H1 hierarchy
+  void m(@H1S1 @H2S1 Object p1, @H1S2 @H2S1 Object p2) {
+    Object l1 = p1;
+    // :: error: (assignment)
+    Object l2 = p2;
+  }
+
+  // Test H2 hierarchy
+  void m2(@H1S1 @H2S1 Object p1, @H1S1 @H2S2 Object p2) {
+    Object l1 = p1;
+    // :: error: (assignment)
+    Object l2 = p2;
+  }
+}

framework/tests/h1h2checker/pkg1/package-info.java

@@ -0,0 +1,14 @@
+@DefaultQualifier(
+    value = H1S1.class,
+    locations = {TypeUseLocation.LOCAL_VARIABLE},
+    applyToSubpackages = false)
+@DefaultQualifier(
+    value = H2S1.class,
+    locations = {TypeUseLocation.LOCAL_VARIABLE},
+    applyToSubpackages = true)
+package pkg1;
+
+import org.checkerframework.framework.qual.DefaultQualifier;
+import org.checkerframework.framework.qual.TypeUseLocation;
+import org.checkerframework.framework.testchecker.h1h2checker.quals.H1S1;
+import org.checkerframework.framework.testchecker.h1h2checker.quals.H2S1;