Handle new extension foreach nullability #79319
Conversation
AlekseyTs
added
the
Feature - Extension Everything
|
Done with review pass (commit 5) #Closed |
nit: consider leaving assertions that Refers to: src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs:11685 in e09ff76. [](commit_id = e09ff76, deletion_comment = False) |
| s = null; | ||
|
|
||
| var c = M(s); // 'C<string?>' | ||
| foreach (var x in c) |
There was a problem hiding this comment.
nit: Consider also covering the happy case where IEnumerator<string> is inferred/returned
| } | ||
| }"; | ||
| var verifier = CompileAndVerify(source, expectedOutput: "23", parseOptions: TestOptions.RegularPreview.WithFeature("run-nullable-analysis", "never")); // Tracked by https://github.com/dotnet/roslyn/issues/78828: Nullable analysis asserts | ||
| var verifier = CompileAndVerify(source, expectedOutput: "23", parseOptions: TestOptions.RegularPreview); |
There was a problem hiding this comment.
nit: could remove RegularPreview since it's the default
I think this will be covered by a future change which adds an assertion to |
…ethodAndVisitArguments
| """; | ||
| var comp = CreateCompilation(src); | ||
| CompileAndVerify(comp, expectedOutput: "42").VerifyDiagnostics(); | ||
| } |
There was a problem hiding this comment.
nit: Consider adding a test like foreach (var x in M(null)) where the receiver of GetEnumerator produces a warning (only once).
There was a problem hiding this comment.
I didn't follow what you meant by M(null). But, I added a version of the GetEnumerator test with a null receiver and classic extension method.
There was a problem hiding this comment.
I meant a test where we have a nullability warning in the receiver of GetEnumerator, showing that we only report that warning once.
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
3 participants
Related to #78828