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

false positive contradictory null annotations since 2024-03 #2178

Closed
Bananeweizen opened this issue Mar 18, 2024 · 4 comments · Fixed by #2226
Closed

false positive contradictory null annotations since 2024-03 #2178

Bananeweizen opened this issue Mar 18, 2024 · 4 comments · Fixed by #2226
Assignees

Comments

@Bananeweizen
Copy link
Contributor

Bananeweizen commented Mar 18, 2024

This error did not exist in 2024-03 milestone 2, but pops up many times for us in the final 2024-03 release.

grafik

The external null annotations are such that String.valueOf() returns NonNull, while Objects.requireNonNull(...) takes a nullable argument:
grafik

The actual parameter is nonnull, which is surely assignment compatible. But instead of accepting the nonnull object for nullable usage, the compiler assumes to have both annotations at the argument.

Be aware this happens only with external null annotations. If I replace the externally annotated methods with identical private methods with the same annotations there are no errors at all.

@Bananeweizen
Copy link
Contributor Author

If someone from JDT had a look at this issue before, please be aware the previous attachment was the wrong reproduction project. This one is the actual reproducer for this issue:
reproducer_eea.zip

@stephan-herrmann
Copy link
Contributor

Thanks for the report.

I could reproduce in the IDE, but unfortunately so far my attempts to wrap this in a unit test failed. Smells like some order-dependence ...

However, I found that this is caused by the "kludge" from #1513 (comment) -- need to go back and analyse the reasons for putting in that kludge in the first place.

@Bananeweizen
Copy link
Contributor Author

Thanks, Stephan. Maybe just one more question: Of course I tried debugging into this myself, too. But when I tried to check out some commit from 2 months ago (e.g. the big one introducing resource annotation analysis), then my JDT target platform would no longer be correct for the dependency to jdt.manipulation or what it's called. Do you have an advice for working with an older git commit and getting the target platform of that point in time?

@stephan-herrmann
Copy link
Contributor

Thanks, Stephan. Maybe just one more question: Of course I tried debugging into this myself, too. But when I tried to check out some commit from 2 months ago (e.g. the big one introducing resource annotation analysis), then my JDT target platform would no longer be correct for the dependency to jdt.manipulation or what it's called. Do you have an advice for working with an older git commit and getting the target platform of that point in time?

Sorry, nothing systematic :(

In my main JDT workspace I have several related projects checked out from Git, too, so I "only" need to check out corresponding commits from different repos ... until some platform dependencies get out of sync etc.

That's actually one of the reason why I prefer to work from a JUnit test case, because then I don't need any plug-ins above JDT for executing the code :)

FYI: the fix is about ready, but in this case it's the unit test case that causes grief: the problem occurs only on JDK 21+, and since for our model tests we have our own library stubs, those must closely resemble the original to trigger the bad code path. Hopefully done in some minutes ...

stephan-herrmann added a commit to stephan-herrmann/eclipse.jdt.core that referenced this issue Mar 26, 2024
fixes eclipse-jdt#2178

includes updates to enable using jclMin21.jar + src.zip
stephan-herrmann added a commit that referenced this issue Mar 27, 2024
fixes #2178

includes updates to enable using jclMin21.jar + src.zip
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants