Bug 560310 - [null] Bogus error message in loop
Summary: [null] Bogus error message in loop
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.15   Edit
Hardware: PC Mac OS X
: P3 normal (vote)
Target Milestone: 4.15 RC1   Edit
Assignee: Stephan Herrmann CLA
QA Contact:
URL:
Whiteboard:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2020-02-19 05:05 EST by Till Brychcy CLA
Modified: 2020-02-24 03:40 EST (History)
4 users (show)

See Also:
manoj.palat: review+
register.eclipse: review+

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Till Brychcy CLA 2020-02-19 05:05:49 EST 
In the following example:

package confusing;

import java.util.ArrayList;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;

public abstract class Confusing {
    abstract int unannotated(ArrayList<String> list);

    @NonNullByDefault
    public void f(boolean x) {
        ArrayList<String> list = x ? null : new ArrayList<>();

        while (true) {
            unannotated(list);
        }
    }
}


A bogus message is reported for the list argument in the unannotated(list)-invocation:
Null type mismatch: required '@NonNull ArrayList<String>' but the provided value is inferred as @Nullable

If "while" is replace by "if" the correct problem is reported:
Unsafe null type conversion (type annotations): The value of type 'ArrayList<@NonNull String>' is made accessible using the less-annotated type 'ArrayList<String>'
Comment 1 Stephan Herrmann CLA 2020-02-20 09:12:40 EST 
This is a recent regression, scheduling for RC1.
Comment 2 Eclipse Genie CLA 2020-02-22 08:40:33 EST 
New Gerrit change created: https://git.eclipse.org/r/158118
Comment 3 Stephan Herrmann CLA 2020-02-22 09:39:32 EST 
(In reply to Eclipse Genie from comment #2)
> New Gerrit change created: https://git.eclipse.org/r/158118

Some explanations to facilitate reviewing:

1. Deferred checking via LoopingFlowContext did not yet handle the new NullAnnotationMatching.Severity.UNCHECKED_TO_UNANNOTATED. To do so, the NullAnnotationMatching status is now propagated through this chain:
  FlowContext.internalRecordNullityMismatch()
  FlowContext.recordNullReference()
  LoopingFlowContext#nullAnnotationStatuses
  LoopingFlowContext#complainOnDeferredNullChecks()
  ProblemReporter.nullityMismatchingTypeAnnotation()
This fairly mechanical change fixes the regression.

1.a: to make rerouted problems conform to old behavior, the *FlowContext needs to update NAM.nullStatus, which is known only during complainOnDeferredNullChecks().

2. The same was needed for FinallyFlowContext, as demonstrated by the 2nd added test.

3. The above caused an undesired change of error message in NTAT.testBug466556Loops(): since we now route error reporting through nullityMismatchingTypeAnnotation() we lost the message about free type variable. Fixed in the ProblemReporter.

4. Item (3) caused a "collateral improvement": a better message is now also given for NTAT.testBug466562, test expectation has been adjusted.
Comment 4 Stephan Herrmann CLA 2020-02-22 12:36:55 EST 
Manoj, Jay, Till:
This regression fix needs a code review and +1 from a project lead.

Would be great if one of you finds the time for RC1, who will it be? :)

See comment 3 for some explanations.
Comment 5 Stephan Herrmann CLA 2020-02-23 08:55:40 EST 
Thanks, Till for the review.

@Manoj, @Jay, all we need now is +1 from a project lead :)
Comment 7 Manoj N Palat CLA 2020-02-23 22:20:02 EST 
(In reply to Stephan Herrmann from comment #5)
> Thanks, Till for the review.
> 
> @Manoj, @Jay, all we need now is +1 from a project lead :)

Done and submitted. Thanks Stephan!
Comment 8 Stephan Herrmann CLA 2020-02-24 03:40:40 EST 
(In reply to Manoj Palat from comment #7)
> (In reply to Stephan Herrmann from comment #5)
> > Thanks, Till for the review.
> > 
> > @Manoj, @Jay, all we need now is +1 from a project lead :)
> 
> Done and submitted. Thanks Stephan!

thanks