Restore ability to use unannotated stub file to override annotations in bytecode

checker/tests/nullness-extra/Makefile

@@ -1,5 +1,5 @@
 # Tests that are currently passing
-PASSING_TESTS = Bug109 compat issue265 issue594 issue607 multiple-errors package-anno shorthand
+PASSING_TESTS = Bug109 compat issue265 issue594 issue607 multiple-errors package-anno shorthand issue3597
 ifeq (,$(findstring 1.8,$(shell javac -version)))
   # issue309 and issue502 fail with Java 11 because of differences between Java 8 and Java 11 bytecode.
   # TODO: issue559 should work with an annotated jdk11.
@@ -53,5 +53,8 @@ shorthand:
 issue607:
 	$(MAKE) -C issue607
 
+issue3597:
+	$(MAKE) -C issue3597
+
 # All tests: passing and failing
 .PHONY: all skipped ${PASSING_TESTS}

checker/tests/nullness-extra/issue3597/Makefile

@@ -0,0 +1,8 @@
+.PHONY: all
+
+all: clean
+	$(JAVAC) testpkg/Issue3597B.java
+	$(JAVAC) -Astubs=issue3597.astub -processor Nullness -sourcepath : -cp . testpkg/Issue3597A.java
+
+clean:
+	rm -f testpkg/*.class

checker/tests/nullness-extra/issue3597/issue3597.astub

@@ -0,0 +1,7 @@
+package testpkg;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+class Issue3597B {
+  Object f();
+}

checker/tests/nullness-extra/issue3597/testpkg/Issue3597A.java

@@ -0,0 +1,7 @@
+package testpkg;
+
+class Issue3597A {
+    void f() {
+        System.err.println(new Issue3597B().f().toString());
+    }
+}

checker/tests/nullness-extra/issue3597/testpkg/Issue3597B.java

@@ -0,0 +1,9 @@
+package testpkg;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+class Issue3597B {
+    @Nullable Object f() {
+        return new Object();
+    }
+}

framework/src/main/java/org/checkerframework/common/wholeprograminference/SceneToStubWriter.java

@@ -173,79 +173,29 @@ private static String formatAnnotations(Collection<? extends Annotation> annos)
      * @return the type formatted to be written to Java source code, followed by a space character
      */
     private static String formatArrayType(ATypeElement scenelibRep, ArrayType javacRep) {
-        int levels =
-                2; // 1 for the final non-array component type, another for the original array type.
         TypeMirror componentType = javacRep.getComponentType();
+        ATypeElement scenelibComponent = getNextArrayLevel(scenelibRep);
         while (componentType.getKind() == TypeKind.ARRAY) {
             componentType = ((ArrayType) componentType).getComponentType();
-            levels++;
+            scenelibComponent = getNextArrayLevel(scenelibComponent);
         }
-
-        List<ATypeElement> scenelibRepInJavacOrder =
-                getSceneLibRepInJavacOrder(scenelibRep, levels);
-        return formatArrayTypeImpl(scenelibRepInJavacOrder, javacRep);
-    }
-
-    /**
-     * This method returns each array level in scenelib's representation of an array in a list in
-     * the same order used by javac. This is necessary because javac's TypeMirror and its
-     * derivatives represent arrays differently than scene-lib does.
-     *
-     * <p>If we label an array as such: (0) int (1) [] (2) [] (3) [], then level 0 is the component
-     * type, level 1 is the "outermost" type, and 3 is the "innermost" array type. Scene-lib's
-     * representation of this type is a nested ATypeElement, with this structure: (1) - (2) - (3) -
-     * (0). The TypeMirror, on the other hand, represents the type like this: (3) - (2) - (1) - (0),
-     * for ease of printing. This method therefore descends through the scenelib structure until it
-     * finds the component, adding each item to a list. It then reverses the list, and then adds the
-     * component type to the end.
-     *
-     * <p><a
-     * href="https://checkerframework.org/jsr308/specification/java-annotation-design.html#array-syntax">The
-     * JSR 308 specification</a> explains the reasoning for scenelib's representation.
-     *
-     * @param scenelibRep scenelib's representation of an array type
-     * @param levels the number of component types the type should have, derived from the javac
-     *     representation
-     * @return a list of the array levels in scenelib's representation, but in the order used by
-     *     javac. Guaranteed to have exactly {@code levels} entries. Entries may be null, if the
-     *     corresponding parts of {@code scenelibRep} are null. See <a
-     *     href="https://github.com/typetools/checker-framework/issues/3422">issue 3422</a> for an
-     *     example of code that causes a null ATypeElement, because the component type is unknown,
-     *     but the primary type of the array is known.
-     */
-    private static List<@Nullable ATypeElement> getSceneLibRepInJavacOrder(
-            ATypeElement scenelibRep, int levels) {
-        List<ATypeElement> result = new ArrayList<>();
-        ATypeElement array = scenelibRep;
-        ATypeElement component = getNextArrayLevel(scenelibRep);
-        for (int i = 0; i < levels - 1; i++) {
-            result.add(array);
-            array = component;
-            component = getNextArrayLevel(array);
-        }
-        Collections.reverse(result);
-        // at this point, array has become the actual base component
-        result.add(array);
-        return result;
+        return formatType(scenelibComponent, componentType)
+                + formatArrayTypeImpl(scenelibRep, javacRep);
     }
 
     /**
      * Formats the type of an array to be printable in Java source code, with the annotations from
-     * the scenelib representation added. This method formats a single level of the array, and then
-     * either calls itself recursively (if the component is an array) or formats the component type
-     * using {@link #formatType(ATypeElement, TypeMirror)}.
+     * the scenelib representation added. This method formats only the "array" parts of an array
+     * type; it does not format (or attempt to format) the ultimate component type (that is, the
+     * non-array part of the array type).
      *
-     * @param scenelibRepInJavacOrder the scenelib representation, reordered to match javac's order.
-     *     See {@link #getSceneLibRepInJavacOrder} for an explanation of why this is necessary and
-     *     why the elements may be null.
+     * @param scenelibRep the scene-lib representation
      * @param javacRep the javac representation of the array type
      * @return the type formatted to be written to Java source code, followed by a space character
      */
-    private static String formatArrayTypeImpl(
-            List<@Nullable ATypeElement> scenelibRepInJavacOrder, ArrayType javacRep) {
+    private static String formatArrayTypeImpl(ATypeElement scenelibRep, ArrayType javacRep) {
         TypeMirror javacComponent = javacRep.getComponentType();
-        ATypeElement scenelibRep = scenelibRepInJavacOrder.get(0);
-        ATypeElement scenelibComponent = scenelibRepInJavacOrder.get(1);
+        ATypeElement scenelibComponent = getNextArrayLevel(scenelibRep);
         String result = "";
         List<? extends AnnotationMirror> explicitAnnos = javacRep.getAnnotationMirrors();
         for (AnnotationMirror explicitAnno : explicitAnnos) {
@@ -257,12 +207,10 @@ private static String formatArrayTypeImpl(
         }
         result += "[] ";
         if (javacComponent.getKind() == TypeKind.ARRAY) {
-            return formatArrayTypeImpl(
-                            scenelibRepInJavacOrder.subList(1, scenelibRepInJavacOrder.size()),
-                            (ArrayType) javacComponent)
-                    + result;
+            return result + formatArrayTypeImpl(scenelibComponent, (ArrayType) javacComponent);
+        } else {
+            return result;
         }
-        return formatType(scenelibComponent, javacComponent) + result;
     }
 
     /**

framework/src/main/java/org/checkerframework/framework/stub/StubParser.java

@@ -142,9 +142,9 @@ public class StubParser {
     private final Elements elements;
 
     /**
-     * The set of annotations found in the stub file. Keys are simple (unqualified) names. (This may
-     * be a problem in the unlikely occurrence that a type-checker supports two annotations with the
-     * same simple name.)
+     * The set of annotations found in the stub file. Keys are both fully-qualified and simple
+     * names: there are two entries for each annotation: the annotation's simple name and its
+     * fully-qualified name.
      *
      * @see #getAllStubAnnotations
      */
@@ -237,38 +237,39 @@ private StubParser(
 
     /**
      * All annotations defined in the package (but not those nested within classes in the package).
-     * Keys are simple names.
+     * Keys are both fully-qualified and simple names.
      *
      * @param packageElement a package
      * @return a map from annotation name to TypeElement
      */
     private Map<String, TypeElement> annosInPackage(PackageElement packageElement) {
-        return createImportedAnnotationsMap(
+        return createNameToAnnotationMap(
                 ElementFilter.typesIn(packageElement.getEnclosedElements()));
     }
 
     /**
-     * All annotations declared (directly) within a class. Keys are simple names.
+     * All annotations declared (directly) within a class. Keys are both fully-qualified and simple
+     * names.
      *
      * @param typeElement a type
      * @return a map from annotation name to TypeElement
      */
     private Map<String, TypeElement> annosInType(TypeElement typeElement) {
-        return createImportedAnnotationsMap(
-                ElementFilter.typesIn(typeElement.getEnclosedElements()));
+        return createNameToAnnotationMap(ElementFilter.typesIn(typeElement.getEnclosedElements()));
     }
 
     /**
      * All annotations declared within any of the given elements.
      *
      * @param typeElements the elements whose annotations to retrieve
-     * @return a map from annotation name to TypeElement
+     * @return a map from annotation names (both fully-qualified and simple names) to TypeElement
      */
-    private Map<String, TypeElement> createImportedAnnotationsMap(List<TypeElement> typeElements) {
+    private Map<String, TypeElement> createNameToAnnotationMap(List<TypeElement> typeElements) {
         Map<String, TypeElement> result = new HashMap<>();
         for (TypeElement typeElm : typeElements) {
             if (typeElm.getKind() == ElementKind.ANNOTATION_TYPE) {
                 putNoOverride(result, typeElm.getSimpleName().toString(), typeElm);
+                putNoOverride(result, typeElm.getQualifiedName().toString(), typeElm);
             }
         }
         return result;
@@ -297,14 +298,18 @@ private static List<String> getImportableMembers(TypeElement typeElement) {
         return result;
     }
 
-    // TODO: I'm not sure what "found" means.  This method seems to collect only those that are
-    // imported, so it will miss ones whose fully-qualified name is used in the stub file.
+    //  TODO: This method collects only those that are imported, so it will miss ones whose
+    //   fully-qualified name is used in the stub file. The #getAnnotation method in this class
+    //   compensates for this deficiency by attempting to add any fully-qualified annotation
+    //   that it encounters.
     /**
-     * Returns all annotations found in the stub file, as a value for {@link #allStubAnnotations}.
-     * Note that this also modifies {@link #importedConstants} and {@link #importedTypes}.
+     * Returns all annotations imported by the stub file, as a value for {@link
+     * #allStubAnnotations}. Note that this also modifies {@link #importedConstants} and {@link
+     * #importedTypes}.
      *
-     * @return a map from simple (unqualified) name to TypeElement, for all annotations found in the
-     *     stub file
+     * @return a map from names to TypeElement, for all annotations imported by the stub file. Two
+     *     entries for each annotation: one for the simple name and another for the fully-qualified
+     *     name, with the same value.
      * @see #allStubAnnotations
      */
     private Map<String, TypeElement> getAllStubAnnotations() {
@@ -1003,9 +1008,7 @@ private void annotate(
             return;
         }
 
-        if (mightHaveTypeArguments(atype)) {
-            clearAnnotations(atype, typeDef);
-        }
+        clearAnnotations(atype, typeDef);
 
         // Primary annotations for the type of a variable declaration are not stored in typeDef, but
         // rather as declaration annotations (passed as declAnnos to this method).  But, if typeDef
@@ -1102,26 +1105,6 @@ private void annotate(
         }
     }
 
-    /**
-     * Returns true if atype might have type arguments that {@link
-     * #clearAnnotations(AnnotatedTypeMirror, Type)} might need to remove.
-     *
-     * @param atype the type to check
-     * @return a conservative approximation of whether atype might have type arguments
-     */
-    private boolean mightHaveTypeArguments(AnnotatedTypeMirror atype) {
-        switch (atype.getKind()) {
-            case DECLARED:
-                AnnotatedDeclaredType adtype = (AnnotatedDeclaredType) atype;
-                return !adtype.getTypeArguments().isEmpty();
-            case WILDCARD:
-            case TYPEVAR:
-                return true;
-            default:
-                return false;
-        }
-    }
-
     /**
      * Process the field declaration in decl, and attach any type qualifiers to the type of elt in
      * {@link #atypes}.
@@ -1644,7 +1627,19 @@ private AnnotationMirror getAnnotation(
 
         TypeElement annoTypeElm = allStubAnnotations.get(annoName);
         if (annoTypeElm == null) {
-            // Not a supported qualifier -> ignore
+            // If the annotation was not imported, then #getAllStubAnnotations does
+            // not add it to the allStubAnnotations field. This code compensates for
+            // that deficiency by adding the annotation when it is encountered (i.e. here).
+            // Note that this goes not call #getTypeElement to avoid a spurious diagnostic
+            // if the annotation is actually unknown.
+            TypeElement annoTypeElt = elements.getTypeElement(annotation.getNameAsString());
+            if (annoTypeElt != null) {
+                putAllNew(
+                        allStubAnnotations,
+                        createNameToAnnotationMap(Collections.singletonList(annoTypeElt)));
+                return getAnnotation(annotation, allStubAnnotations);
+            }
+            // Not a supported annotation -> ignore
             return null;
         }
         annoName = annoTypeElm.getQualifiedName().toString();