Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Aug 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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