Skip to content

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

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

Closed
aosen-xiong wants to merge 4 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 3 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
aosen-xiong Jun 19, 2024
280e8aa
Add error message on NoExplicitAnnotations test case
aosen-xiong Jun 20, 2024
b5fc573
Empty commit for CI
aosen-xiong Jun 20, 2024
1d7beaa
Merge branch 'master' into clearAnnotation-only-typevar
aosen-xiong 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)

@wmdietl wmdietl Jun 20, 2024

Copy link
Copy Markdown
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?

@wmdietl wmdietl Jun 20, 2024

Copy link
Copy Markdown
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.

@aosen-xiong aosen-xiong Jun 21, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

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.

@wmdietl wmdietl Jun 21, 2024

Copy link
Copy Markdown
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