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

Add lost qualifier to viewpointtest type checker #827

Merged
merged 39 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
13e1697
Add lost qualifier
Ao-senXiong Aug 3, 2024
0364e47
Add error message for Top receiver method invocation
Ao-senXiong Aug 3, 2024
7ec7125
Merge branch 'master' into lost-qualifier
wmdietl Aug 10, 2024
c859999
Implement lost is not reflexive by overriding the subtype rules
Ao-senXiong Aug 12, 2024
76a9f40
Merge branch 'master' into lost-qualifier
Ao-senXiong Aug 12, 2024
36905fb
Empty commit for CI
Ao-senXiong Aug 12, 2024
7ea05ed
Merge branch 'master' into lost-qualifier
Ao-senXiong Aug 17, 2024
c31815d
Merge branch 'master' into lost-qualifier
Ao-senXiong Aug 18, 2024
3789eab
Merge branch 'master' into lost-qualifier
Ao-senXiong Aug 18, 2024
270c933
Correct format for Javadoc
Ao-senXiong Aug 20, 2024
4151fce
Add link to QualifierHierarchy class
Ao-senXiong Aug 20, 2024
c21d26f
Merge branch 'master' into lost-qualifier
Ao-senXiong Aug 22, 2024
744acc6
Fix javadoc link
Ao-senXiong Aug 22, 2024
92ce1c5
Merge branch 'master' into lost-qualifier
Ao-senXiong Aug 27, 2024
5252e54
Merge branch 'master' into lost-qualifier
Ao-senXiong Sep 7, 2024
c879d46
Add missing error message
Ao-senXiong Sep 8, 2024
5e8f6ff
Merge branch 'master' into lost-qualifier
Ao-senXiong Sep 8, 2024
2a1f9a2
Add javadoc
Ao-senXiong Sep 8, 2024
1be71d0
Fix Javadoc
Ao-senXiong Sep 8, 2024
ed98bf0
Wording
Ao-senXiong Oct 3, 2024
b9cf1db
Comment for viewpoint adapta super call
Ao-senXiong Oct 3, 2024
a070489
Format
Ao-senXiong Oct 3, 2024
bf37ee9
Move `new.class.type.invalid` to basetype's error message
Ao-senXiong Oct 3, 2024
c27c02d
Forbid create `@top` Object
Ao-senXiong Oct 3, 2024
696c160
Merge branch 'master' into lost-qualifier
Ao-senXiong Oct 3, 2024
c56f67f
Javadoc
Ao-senXiong Oct 3, 2024
c541606
Merge branch 'master' into lost-qualifier
Ao-senXiong Oct 9, 2024
8fd3433
Apply spotless
Ao-senXiong Oct 9, 2024
b0db749
Adapt test to new changes
Ao-senXiong Oct 9, 2024
7ba1549
Merge branch 'master' into lost-qualifier
wmdietl Nov 8, 2024
71069b3
Merge branch 'master' into lost-qualifier
wmdietl Nov 13, 2024
3970bbc
Consistent Javadoc
Ao-senXiong Nov 13, 2024
0fd471e
Address comment and update with more test cases
Ao-senXiong Nov 14, 2024
060894b
Merge branch 'master' into lost-qualifier
Ao-senXiong Nov 14, 2024
7b12191
Merge branch 'master' into lost-qualifier
Ao-senXiong Nov 23, 2024
1d1f938
Merge branch 'master' into lost-qualifier
Ao-senXiong Dec 3, 2024
249272b
Merge branch 'master' into lost-qualifier
wmdietl Dec 4, 2024
4326a19
Don't recreate the same qualifiers
wmdietl Dec 4, 2024
bbc07c1
Add downcasts
wmdietl Dec 6, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ nulltest.redundant=redundant test against null; "%s" is non-null
instanceof.nullable=instanceof is only true for a non-null expression
instanceof.nonnull.redundant=redundant @NonNull annotation on instanceof
new.array.type.invalid=annotations %s may not be applied as component type for array "%s"
new.class.type.invalid=the annotations %s do not need be applied in object creations
nullness.on.constructor=do not write nullness annotations on a constructor, whose result is always non-null
nullness.on.enum=do not write nullness annotations on an enum constant, which is always non-null
nullness.on.exception.parameter=do not write nullness annotations on an exception parameter, which is always non-null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
receiver.invalid=incompatible types.%nfound : %s%nrequired: %s
type.anno.before.modifier=write type annotation %s immediately before type, after modifiers %s
type.anno.before.decl.anno=write type annotations %s immediately before type, after declaration annotation %s
new.class.type.invalid=the annotations %s do not need be applied in object creations
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this further down to other type.invalid error messages.
While we're at it, can you improve the grammar?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!


array.initializer.type.incompatible=incompatible types in array initializer.%nfound : %s%nrequired: %s
assignment.type.incompatible=incompatible types in assignment.%nfound : %s%nrequired: %s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,33 @@
import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory;
import org.checkerframework.common.basetype.BaseTypeChecker;
import org.checkerframework.framework.type.AbstractViewpointAdapter;
import org.checkerframework.framework.type.QualifierHierarchy;
import org.checkerframework.javacutil.AnnotationBuilder;

import java.lang.annotation.Annotation;
import java.util.Set;

import javax.lang.model.element.AnnotationMirror;

import viewpointtest.quals.A;
import viewpointtest.quals.B;
import viewpointtest.quals.Bottom;
import viewpointtest.quals.Lost;
import viewpointtest.quals.PolyVP;
import viewpointtest.quals.ReceiverDependentQual;
import viewpointtest.quals.Top;

/** The annotated type factory for the viewpointtest type-system. */
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved
public class ViewpointTestAnnotatedTypeFactory extends BaseAnnotatedTypeFactory {

/** The {@link Top} annotation. */
public final AnnotationMirror TOP = AnnotationBuilder.fromClass(elements, Top.class);

/**
* Create a new ViewpointTestAnnotatedTypeFactory.
*
* @param checker the checker to which this annotated type factory belongs
*/
public ViewpointTestAnnotatedTypeFactory(BaseTypeChecker checker) {
super(checker);
this.postInit();
Expand All @@ -29,11 +43,18 @@ protected Set<Class<? extends Annotation>> createSupportedTypeQualifiers() {
Bottom.class,
PolyVP.class,
ReceiverDependentQual.class,
Lost.class,
Top.class);
}

@Override
protected AbstractViewpointAdapter createViewpointAdapter() {
return new ViewpointTestViewpointAdapter(this);
}

@Override
public QualifierHierarchy createQualifierHierarchy() {
return new ViewpointTestQualifierHierarchy(
this.getSupportedTypeQualifiers(), elements, this);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package viewpointtest;

import org.checkerframework.framework.type.GenericAnnotatedTypeFactory;
import org.checkerframework.framework.type.NoElementQualifierHierarchy;
import org.checkerframework.framework.type.QualifierHierarchy;

import java.lang.annotation.Annotation;
import java.util.Collection;

import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.util.Elements;

import viewpointtest.quals.Bottom;
import viewpointtest.quals.Lost;

/** The {@link QualifierHierarchy} for the Viewpoint Test Checker. */
public class ViewpointTestQualifierHierarchy extends NoElementQualifierHierarchy {
/**
* Creates a ViewpointTestQualifierHierarchy from the given classes.
*
* @param qualifierClasses classes of annotations that are the qualifiers
* @param elements element utils
* @param atypeFactory the associated type factory
*/
public ViewpointTestQualifierHierarchy(
Collection<Class<? extends Annotation>> qualifierClasses,
Elements elements,
GenericAnnotatedTypeFactory<?, ?, ?, ?> atypeFactory) {
super(qualifierClasses, elements, atypeFactory);
}

@Override
public boolean isSubtypeQualifiers(AnnotationMirror subAnno, AnnotationMirror superAnno) {
// Lost is not reflexive and the only subtype is Bottom
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved
if (atypeFactory.areSameByClass(superAnno, Lost.class)
&& !atypeFactory.areSameByClass(subAnno, Bottom.class)) {
return false;
}
return super.isSubtypeQualifiers(subAnno, superAnno);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@

import javax.lang.model.element.AnnotationMirror;

import viewpointtest.quals.Lost;
import viewpointtest.quals.ReceiverDependentQual;
import viewpointtest.quals.Top;

/** The viewpoint adapter for the ViewpointTest checker. */
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved
public class ViewpointTestViewpointAdapter extends AbstractViewpointAdapter {

private final AnnotationMirror TOP, RECEIVERDEPENDENTQUAL;
/** The {@link Top}, {@link ReceiverDependentQual} and {@link Lost} annotation. */
private final AnnotationMirror TOP, RECEIVERDEPENDENTQUAL, LOST;

/**
* The class constructor.
Expand All @@ -26,6 +29,7 @@ public ViewpointTestViewpointAdapter(AnnotatedTypeFactory atypeFactory) {
RECEIVERDEPENDENTQUAL =
AnnotationBuilder.fromClass(
atypeFactory.getElementUtils(), ReceiverDependentQual.class);
LOST = AnnotationBuilder.fromClass(atypeFactory.getElementUtils(), Lost.class);
}

@Override
Expand All @@ -38,7 +42,11 @@ protected AnnotationMirror combineAnnotationWithAnnotation(
AnnotationMirror receiverAnnotation, AnnotationMirror declaredAnnotation) {

if (AnnotationUtils.areSame(declaredAnnotation, RECEIVERDEPENDENTQUAL)) {
return receiverAnnotation;
if (AnnotationUtils.areSame(receiverAnnotation, TOP)) {
return LOST;
} else {
return receiverAnnotation;
}
}
return declaredAnnotation;
}
Expand Down
28 changes: 28 additions & 0 deletions framework/src/test/java/viewpointtest/ViewpointTestVisitor.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package viewpointtest;

import com.sun.source.tree.NewClassTree;

import org.checkerframework.common.basetype.BaseTypeChecker;
import org.checkerframework.common.basetype.BaseTypeVisitor;
import org.checkerframework.framework.type.AnnotatedTypeMirror;

/** The visitor for the viewpointtest type-system. */
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved
public class ViewpointTestVisitor extends BaseTypeVisitor<ViewpointTestAnnotatedTypeFactory> {
/**
* Create a new ViewpointTestVisitor.
*
* @param checker the checker to which this visitor belongs
*/
public ViewpointTestVisitor(BaseTypeChecker checker) {
super(checker);
}

@Override
public Void visitNewClass(NewClassTree tree, Void p) {
AnnotatedTypeMirror Type = atypeFactory.getAnnotatedType(tree);
if (Type.hasAnnotation(atypeFactory.TOP)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't LOST also be forbidden?

Copy link
Member Author

@Ao-senXiong Ao-senXiong Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, how about poly and RD qualifier? I left out those two as I feel like we need more discussion.

checker.reportError(tree, "new.class.type.invalid", Type.getAnnotations());
}
return super.visitNewClass(tree, p);
}
}
2 changes: 1 addition & 1 deletion framework/src/test/java/viewpointtest/quals/Bottom.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@
@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER})
@SubtypeOf({A.class, B.class, ReceiverDependentQual.class})
@SubtypeOf({A.class, B.class, ReceiverDependentQual.class, Top.class, PolyVP.class, Lost.class})
@DefaultFor(TypeUseLocation.LOWER_BOUND)
public @interface Bottom {}
22 changes: 22 additions & 0 deletions framework/src/test/java/viewpointtest/quals/Lost.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package viewpointtest.quals;

import org.checkerframework.framework.qual.SubtypeOf;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* The {@link Lost} qualifier indicates that a relationship cannot be expressed. It is the result of
* viewpoint adaptation that combines {@link Top} and {@link ReceiverDependentQual}.
*
* <p>It is not reflexive in the subtyping relationship and the only subtype for {@link Lost} is
* {@link Bottom}.
*/
@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER})
@SubtypeOf({Top.class})
public @interface Lost {}
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved
35 changes: 35 additions & 0 deletions framework/tests/viewpointtest/LostNonReflexive.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import viewpointtest.quals.*;

public class LostNonReflexive {
@ReceiverDependentQual Object f;

@SuppressWarnings({"inconsistent.constructor.type", "super.invocation.invalid"})
@ReceiverDependentQual LostNonReflexive(@ReceiverDependentQual Object args) {}

@ReceiverDependentQual Object get() {
return null;
}

void set(@ReceiverDependentQual Object o) {}

void test(@Top LostNonReflexive obj, @Bottom Object bottomObj) {
// :: error: (assignment.type.incompatible)
this.f = obj.f;
this.f = bottomObj;

// :: error: (assignment.type.incompatible)
@A Object aObj = obj.get();
// :: error: (assignment.type.incompatible)
@B Object bObj = obj.get();
// :: error: (assignment.type.incompatible)
@Bottom Object botObj = obj.get();

// :: error: (argument.type.incompatible)
new LostNonReflexive(obj.f);
new LostNonReflexive(bottomObj);

// :: error: (argument.type.incompatible)
this.set(obj.f);
this.set(bottomObj);
}
}
5 changes: 3 additions & 2 deletions framework/tests/viewpointtest/PolyConstructor.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
public class PolyConstructor {

static class MyClass {
@SuppressWarnings({"inconsistent.constructor.type", "super.invocation.invalid"})
// :: error: (super.invocation.invalid) :: warning: (inconsistent.constructor.type)
@PolyVP MyClass(@PolyVP Object o) {
throw new RuntimeException(" * You are filled with DETERMINATION."); // stub
// :: warning: (cast.unsafe.constructor.invocation)
throw new @A RuntimeException(" * You are filled with DETERMINATION."); // stub
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of this change? Should there be multiple versions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expand the test suite in here.

}
}

Expand Down
4 changes: 4 additions & 0 deletions framework/tests/viewpointtest/SuperConstructorCalls.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ public Inner() {
super();
}

// The constructor's return type is implicitly @Top by default.
// When calling the super constructor, @Top becomes @Lost in the super constructor's
// signature, causing a type mismatch with the expected @ReceiverDependentQual parameter.
public Inner(@Top Object objTop) {
// :: error: (argument.type.incompatible)
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved
super(objTop);
}

Expand Down
6 changes: 4 additions & 2 deletions framework/tests/viewpointtest/TestGetAnnotatedLhs.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
@SuppressWarnings({"cast.unsafe.constructor.invocation"})
void topWithRefinement() {
TestGetAnnotatedLhs a = new @A TestGetAnnotatedLhs();
// :: error: (new.class.type.invalid)
TestGetAnnotatedLhs top = new @Top TestGetAnnotatedLhs();
top = a;
// When checking the below assignment, GenericAnnotatedTypeFactory#getAnnotatedTypeLhs()
Expand All @@ -33,10 +34,11 @@ void topWithRefinement() {

@SuppressWarnings({"cast.unsafe.constructor.invocation"})
void topWithoutRefinement() {
// :: error: (new.class.type.invalid)
TestGetAnnotatedLhs top = new @Top TestGetAnnotatedLhs();
// See #576.
// :TODO: error: (assignment.type.incompatible)
// :: error: (assignment.type.incompatible)
top.f = new @B Object();
// :: error: (assignment.type.incompatible)
top.f = new @A Object();
}
}
2 changes: 2 additions & 0 deletions framework/tests/viewpointtest/VPAExamples.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ void tests(@A RDContainer a, @B RDContainer b, @Top RDContainer top) {
// :: error: (argument.type.incompatible)
b.set(aObj);
b.set(bObj);
// :: error: (argument.type.incompatible)
top.set(aObj);
// :: error: (argument.type.incompatible)
top.set(bObj);
}
}
21 changes: 16 additions & 5 deletions framework/tests/viewpointtest/VarargsConstructor.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ public class VarargsConstructor {
@ReceiverDependentQual VarargsConstructor(@ReceiverDependentQual Object... args) {}

void foo() {
VarargsConstructor a = new VarargsConstructor("testStr", new Object());
// :: warning: (cast.unsafe.constructor.invocation)
VarargsConstructor a = new @A VarargsConstructor("testStr", new @A Object());
}

void invokeConstructor(@A Object aObj, @B Object bObj, @Top Object topObj) {
@A Object a = new @A VarargsConstructor(aObj);
@B Object b = new @B VarargsConstructor(bObj);
// :: error: (argument.type.incompatible) :: error: (new.class.type.invalid)
@Top Object top = new @Top VarargsConstructor(topObj);
Ao-senXiong marked this conversation as resolved.
Show resolved Hide resolved
// :: error: (argument.type.incompatible)
new @A VarargsConstructor(bObj);
Expand All @@ -29,14 +31,19 @@ class Inner {

void foo() {
Inner a = new Inner();
Inner b = new Inner(new Object());
// :: error: (argument.type.incompatible) :: warning:
// (cast.unsafe.constructor.invocation)
Inner b = new Inner(new @A Object());
Inner c = VarargsConstructor.this.new Inner();
Inner d = VarargsConstructor.this.new Inner(new Object());
// :: error: (argument.type.incompatible) :: warning:
// (cast.unsafe.constructor.invocation)
Inner d = VarargsConstructor.this.new Inner(new @A Object());
}

void invokeConstructor(@A Object aObj, @B Object bObj, @Top Object topObj) {
@A Object a = new @A Inner(aObj);
@B Object b = new @B Inner(bObj);
// :: error: (argument.type.incompatible) :: error: (new.class.type.invalid)
@Top Object top = new @Top Inner(topObj);
// :: error: (argument.type.incompatible)
new @A Inner(bObj);
Expand All @@ -47,13 +54,17 @@ void invokeConstructor(@A Object aObj, @B Object bObj, @Top Object topObj) {

void testAnonymousClass(@A Object aObj, @B Object bObj, @Top Object topObj) {
Object o =
new VarargsConstructor("testStr", new Object()) {
// :: warning: (cast.unsafe.constructor.invocation)
new @A VarargsConstructor("testStr", new @A Object()) {
void foo() {
VarargsConstructor a = new VarargsConstructor("testStr", new Object());
VarargsConstructor a =
// :: warning: (cast.unsafe.constructor.invocation)
new @A VarargsConstructor("testStr", new @A Object());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change illustrating something about Lost? Should there be versions of this test without an annotation? A comment would help understand this test.

Copy link
Member Author

@Ao-senXiong Ao-senXiong Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not about lost, I update this test case because I want those invalid invocation because previous we allow @Top constructor invocation. More test cases for lost is in LostNonReflexive.java

}
};
@A Object a = new @A VarargsConstructor(aObj) {};
@B Object b = new @B VarargsConstructor(bObj) {};
// :: error: (argument.type.incompatible) :: error: (new.class.type.invalid)
@Top Object top = new @Top VarargsConstructor(topObj) {};
// :: error: (argument.type.incompatible)
new @A VarargsConstructor(bObj) {};
Expand Down
Loading