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

[Enhanced Switch][Null] Missing Null pointer access warning with total/unconditional patterns in case labels #3382

Merged
merged 2 commits into from
Dec 3, 2024

Conversation

@srikanth-sankaran srikanth-sankaran self-assigned this Dec 3, 2024
@srikanth-sankaran srikanth-sankaran added this to the 4.35 M1 milestone Dec 3, 2024
@srikanth-sankaran
Copy link
Contributor Author

Expecting some tests to fail

org.eclipse.jdt.core.tests.compiler.regression.NullAnnotationTests21.test_totalTypePatternDoesNotAdmitNull()
org.eclipse.jdt.core.tests.compiler.regression.NullAnnotationTests21.test_totalTypePatternNullableExpression()

but these need to be studied to see if expectation is valid

@srikanth-sankaran
Copy link
Contributor Author

Looking at org.eclipse.jdt.core.tests.compiler.regression.NullAnnotationTests21.test_totalTypePatternNullableExpression() it seems the understanding is that an unconditional pattern would match null - but that is not so.

So when case Number n0 is matched. n0 can never be null, so the warning Null type mismatch: required \'@NonNull Number\' but the provided value is inferred as @Nullable expected by the test looks wrong.

@stephan-herrmann - The proposed patch is still WIP, comments are welcome

@srikanth-sankaran
Copy link
Contributor Author

I think I have remastered the "failing" tests with right expectations. @stephan-herrmann thanks for your review.

total/unconditional patterns in case labels

* Fixes eclipse-jdt#3381
@stephan-herrmann
Copy link
Contributor

So when case Number n0 is matched. n0 can never be null

looks like this changed between Java 18 and 19.

I think I have remastered the "failing" tests with right expectations. @stephan-herrmann thanks for your review.

looking into it now.

Comment on lines +1269 to +1279
----------
1. ERROR in X.java (at line 6)
switch (i) {
^
Null pointer access: The variable i can only be null at this location
----------
2. ERROR in X.java (at line 12)
switch (i) {
^
Null pointer access: This expression of type Integer is null but requires auto-unboxing
----------
Copy link
Contributor

Choose a reason for hiding this comment

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

@srikanth-sankaran do you have an idea why these are different error messages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@srikanth-sankaran do you have an idea why these are different error messages?

Yes, I had looked into this. In the former case its is a type switch which doesn't call for unboxing and in the latter switch on the unboxed integer. This results in a special message in the latter case.

total/unconditional patterns in case labels

+ code simplification

Fixes eclipse-jdt#3381
@stephan-herrmann
Copy link
Contributor

@srikanth-sankaran your turn again :)

@srikanth-sankaran
Copy link
Contributor Author

Your additional changes look good, Thanks @stephan-herrmann!

@srikanth-sankaran srikanth-sankaran merged commit c003779 into eclipse-jdt:master Dec 3, 2024
7 of 10 checks passed
@srikanth-sankaran srikanth-sankaran deleted the Issue3381 branch December 3, 2024 23:50
srikanth-sankaran added a commit to srikanth-sankaran/eclipse.jdt.core that referenced this pull request Dec 4, 2024
@srikanth-sankaran
Copy link
Contributor Author

The first commit from here also fixes #3319 - basically the isNullHostile method's wrong result in the presence of total patterns was inhibiting a diagnostic from being reported.

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 this pull request may close these issues.

[Enhanced Switch][Null] Missing Null pointer access warning with total/unconditional patterns
2 participants