Skip to content

Commit

Permalink
Restore ability to use unannotated stub file to override annotations …
Browse files Browse the repository at this point in the history
…in bytecode (#3599)
  • Loading branch information
kelloggm committed Aug 19, 2020
1 parent 3dc6457 commit 2680ed4
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 41 deletions.
5 changes: 4 additions & 1 deletion checker/tests/nullness-extra/Makefile
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -53,5 +53,8 @@ shorthand:
issue607:
$(MAKE) -C issue607

issue3597:
$(MAKE) -C issue3597

# All tests: passing and failing
.PHONY: all skipped ${PASSING_TESTS}
8 changes: 8 additions & 0 deletions checker/tests/nullness-extra/issue3597/Makefile
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions checker/tests/nullness-extra/issue3597/issue3597.astub
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package testpkg;

import org.checkerframework.checker.nullness.qual.Nullable;

class Issue3597B {
Object f();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package testpkg;

class Issue3597A {
void f() {
System.err.println(new Issue3597B().f().toString());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package testpkg;

import org.checkerframework.checker.nullness.qual.Nullable;

class Issue3597B {
@Nullable Object f() {
return new Object();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}.
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 2680ed4

Please sign in to comment.