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

Correctly handle declaration annotations on type variable arrays #658

Merged
merged 4 commits into from
Dec 25, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions checker/jtreg/nullness/issue6374/Lib.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import org.checkerframework.checker.nullness.qual.Nullable;
import org.jmlspecs.annotation.NonNull;

@SuppressWarnings("unchecked") // ignore heap pollution
class Lib {
// element type inferred, array non-null
static <T> void none(T... o) {}

// element type inferred, array non-null
static <T> void decl(@NonNull T... o) {}

// element type nullable, array non-null
static <T> void type(@Nullable T... o) {}

// element type nullable, array nullable
static <T> void typenn(@Nullable T @Nullable ... o) {}
}
20 changes: 20 additions & 0 deletions checker/jtreg/nullness/issue6374/User.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* @test
* @summary Test case for typetools issue #6374: https://github.com/typetools/checker-framework/issues/6374
*
* @compile Lib.java
* @compile/fail/ref=User.out -XDrawDiagnostics -Anomsgtext -processor org.checkerframework.checker.nullness.NullnessChecker User.java
*/
// Also see checker/tests/nullness/generics/Issue6374.java
class User {
void go() {
Lib.decl("", null);
// :: error: (argument.type.incompatible)
Lib.decl((Object[]) null);
Lib.type("", null);
// :: error: (argument.type.incompatible)
Lib.type((Object[]) null);
Lib.typenn("", null);
Lib.typenn((Object[]) null);
}
}
3 changes: 3 additions & 0 deletions checker/jtreg/nullness/issue6374/User.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
User.java:13:18: compiler.err.proc.messager: (argument.type.incompatible)
User.java:16:18: compiler.err.proc.messager: (argument.type.incompatible)
2 errors
37 changes: 37 additions & 0 deletions checker/tests/nullness/generics/Issue6374.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Test case for typetools issue #6374:
// https://github.com/typetools/checker-framework/issues/6374
// Also see checker/jtreg/nullness/issue6374/

import org.checkerframework.checker.nullness.qual.Nullable;
import org.jmlspecs.annotation.NonNull;

public class Issue6374 {

@SuppressWarnings("unchecked") // ignore heap pollution
static class Lib {
// element type inferred, array non-null
static <T> void none(T... o) {}

// element type inferred, array non-null
static <T> void decl(@NonNull T... o) {}

// element type nullable, array non-null
static <T> void type(@Nullable T... o) {}

// element type nullable, array nullable
static <T> void typenn(@Nullable T @Nullable ... o) {}
}

class User {
void go() {
Lib.decl("", null);
// :: error: (argument.type.incompatible)
Lib.decl((Object[]) null);
Lib.type("", null);
// :: error: (argument.type.incompatible)
Lib.type((Object[]) null);
Lib.typenn("", null);
Lib.typenn((Object[]) null);
}
}
}
2 changes: 2 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ Version 3.41.0-eisop2 (December ?, 2023)

**Closed issues:**

typetools#6374.


Version 3.41.0-eisop1 (December 5, 2023)
----------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,16 @@ static void addDeclarationAnnotationsFromElement(
// org.checkerframework.framework.type.TypeFromMemberVisitor.visitVariable
AnnotatedTypeMirror innerType = AnnotatedTypes.innerMostType(type);
if (innerType != type) {
for (AnnotationMirror annotation : annotations) {
if (AnnotationUtils.annotationName(annotation).startsWith("org.checkerframework")) {
innerType.addAnnotation(annotation);
for (AnnotationMirror anno : annotations) {
if (AnnotationUtils.isDeclarationAnnotation(anno)
// Always treat Checker Framework annotations as type annotations.
&& !AnnotationUtils.annotationName(anno)
.startsWith("org.checkerframework")) {
// Declaration annotations apply to the outer type.
type.addAnnotation(anno);
} else {
type.addAnnotation(annotation);
// Type annotations apply to the innermost type.
innerType.addAnnotation(anno);
}
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedArrayType;
import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedTypeVariable;
import org.checkerframework.framework.type.ElementAnnotationApplier;
import org.checkerframework.framework.util.AnnotatedTypes;
import org.checkerframework.framework.util.element.ElementAnnotationUtil.UnexpectedAnnotationLocationException;
import org.checkerframework.javacutil.BugInCF;

Expand Down Expand Up @@ -47,6 +48,8 @@ public static void apply(
* Returns true if type is an AnnotatedTypeVariable, or an AnnotatedArrayType with a type
* variable component, and the element is not a TYPE_PARAMETER.
*
* @param type the type to test
* @param element the corresponding element
* @return true if type is an AnnotatedTypeVariable, or an AnnotatedArrayType with a type
* variable component, and the element is not a TYPE_PARAMETER
*/
Expand All @@ -55,19 +58,15 @@ public static boolean accepts(AnnotatedTypeMirror type, Element element) {
&& ElementAnnotationUtil.contains(element.getKind(), acceptedKinds);
}

/**
* Returns true if type is an array type whose innermost component type is a type variable.
*
* @param type the type to test
* @return true if type is an array type whose innermost component type is a type variable
*/
private static boolean isGenericArrayType(AnnotatedTypeMirror type) {
return type instanceof AnnotatedArrayType
&& getNestedComponentType(type) instanceof AnnotatedTypeVariable;
}

private static AnnotatedTypeMirror getNestedComponentType(AnnotatedTypeMirror type) {

AnnotatedTypeMirror componentType = type;
while (componentType instanceof AnnotatedArrayType) {
componentType = ((AnnotatedArrayType) componentType).getComponentType();
}

return componentType;
&& AnnotatedTypes.innerMostType(type) instanceof AnnotatedTypeVariable;
}

/** The generic array type, if any. */
Expand Down Expand Up @@ -108,7 +107,7 @@ private static AnnotatedTypeMirror getNestedComponentType(AnnotatedTypeMirror ty

if (isGenericArrayType(type)) {
this.arrayType = (AnnotatedArrayType) type;
this.typeVariable = (AnnotatedTypeVariable) getNestedComponentType(type);
this.typeVariable = (AnnotatedTypeVariable) AnnotatedTypes.innerMostType(type);
this.declarationElem =
(TypeParameterElement) typeVariable.getUnderlyingType().asElement();
this.useElem = element;
Expand All @@ -131,10 +130,15 @@ private static AnnotatedTypeMirror getNestedComponentType(AnnotatedTypeMirror ty
* @throws UnexpectedAnnotationLocationException if invalid location for an annotation was found
*/
public void extractAndApply() throws UnexpectedAnnotationLocationException {
ElementAnnotationUtil.addDeclarationAnnotationsFromElement(
typeVariable, useElem.getAnnotationMirrors());
if (arrayType != null) {
ElementAnnotationUtil.addDeclarationAnnotationsFromElement(
arrayType, useElem.getAnnotationMirrors());
} else {
ElementAnnotationUtil.addDeclarationAnnotationsFromElement(
typeVariable, useElem.getAnnotationMirrors());
}

// apply declaration annotations
// apply annotations from the type parameter declaration
ElementAnnotationApplier.apply(typeVariable, declarationElem, atypeFactory);

List<Attribute.TypeCompound> annotations = getAnnotations(useElem, declarationElem);
Expand All @@ -145,7 +149,6 @@ public void extractAndApply() throws UnexpectedAnnotationLocationException {
// are not applied as the type variables primary annotation
typeVarAnnotations = removeComponentAnnotations(arrayType, annotations);
ElementAnnotationUtil.annotateViaTypeAnnoPosition(arrayType, annotations);

} else {
typeVarAnnotations = annotations;
}
Expand All @@ -155,27 +158,40 @@ public void extractAndApply() throws UnexpectedAnnotationLocationException {
}
}

private List<Attribute.TypeCompound> removeComponentAnnotations(
/**
* Return the annotations that apply to the base component of the array and remove these
* annotations from the parameter.
*
* @param arrayType the array type
* @param annotations the annotations to inspect and modify
* @return the annotations that apply to the base component of the array
*/
private static List<Attribute.TypeCompound> removeComponentAnnotations(
AnnotatedArrayType arrayType, List<Attribute.TypeCompound> annotations) {

List<Attribute.TypeCompound> componentAnnotations = new ArrayList<>();

if (arrayType != null) {
for (int i = 0; i < annotations.size(); ) {
Attribute.TypeCompound anno = annotations.get(i);
if (isBaseComponent(arrayType, anno)) {
componentAnnotations.add(anno);
annotations.remove(anno);
} else {
i++;
}
for (int i = 0; i < annotations.size(); ) {
Attribute.TypeCompound anno = annotations.get(i);
if (isBaseComponent(arrayType, anno)) {
componentAnnotations.add(anno);
annotations.remove(anno);
} else {
i++;
}
}

return componentAnnotations;
}

private boolean isBaseComponent(AnnotatedArrayType arrayType, Attribute.TypeCompound anno) {
/**
* Return true if anno applies to the base component of arrayType.
*
* @param arrayType the array type
* @param anno the annotation to inspect
* @return true if anno applies to the base component of arrayType
*/
private static boolean isBaseComponent(
AnnotatedArrayType arrayType, Attribute.TypeCompound anno) {
try {
return ElementAnnotationUtil.getTypeAtLocation(arrayType, anno.getPosition().location)
.getKind()
Expand Down Expand Up @@ -235,7 +251,6 @@ private static List<Attribute.TypeCompound> getVariableAnnos(Element variableEle
List<Attribute.TypeCompound> annotations = new ArrayList<>();

for (Attribute.TypeCompound anno : varSymbol.getRawTypeAttributes()) {

TypeAnnotationPosition pos = anno.position;
switch (pos.type) {
case FIELD:
Expand Down
Loading