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.

Sign up for GitHub

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

Jump to bottom

Only clean when the annotated type mirror is type variable #787

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Only clean when the annotated type mirror is type variable #787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
49a799d
Only clean when the annotated type mirror is type variable
Ao-senXiong Jun 19, 2024
280e8aa
Add error message on NoExplicitAnnotations test case
Ao-senXiong Jun 20, 2024
b5fc573
Empty commit for CI
Ao-senXiong Jun 20, 2024
1d7beaa
Merge branch 'master' into clearAnnotation-only-typevar
Ao-senXiong Sep 4, 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
1 change: 1 addition & 0 deletions checker/tests/stubparser-nullness/NoExplicitAnnotations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ void use(
NoExplicitAnnotationsSub2 sub2,
NoExplicitAnnotationsSub3 sub3) {
nble = sub1.method1();
// :: error: (assignment.type.incompatible)
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 provide a reason why this new error is desirable? Why is this change in behavior an improvement?

Copy link
Member

Choose a reason for hiding this comment

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

Also note that a wilcard jtreg test fails.

Copy link
Member Author

@Ao-senXiong Ao-senXiong Jun 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can you provide a reason why this new error is desirable? Why is this change in behavior an improvement?

Because nn is @Nonnull and method1() is @Nullable. So there should be an assignment incompatiable error.

Copy link
Member

Choose a reason for hiding this comment

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

But method1 is in the stub file without annotations.
Shouldn't the logic be that, if something appears in the stub-file, those annotations are used. If there are no annotations in the stub-file, the usual defaults are used.
That's at least what the current behavior seems to be. This makes the stub-files readable and understandable without looking at what the corresponding source says.

nn = sub1.method1();
nble = sub2.method2();
nn = sub2.method2();
Expand Down
4 changes: 3 additions & 1 deletion framework/src/main/java/org/checkerframework/framework/stub/AnnotationFileParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1521,7 +1521,9 @@ private void clearAnnotations(AnnotatedTypeMirror atype, Type typeDef) {
*/
// Clear existing annotations, which only makes a difference for
// type variables, but doesn't hurt in other cases.
atype.clearAnnotations();
if (atype.getKind() == TypeKind.TYPEVAR) {
atype.clearAnnotations();
}
}

/**
Expand Down
Loading