NullableContextAttribute for property parameters is from accessor rather than containing type#47223
NullableContextAttribute for property parameters is from accessor rather than containing type#47223cston merged 1 commit intodotnet:masterfrom
Conversation
| { | ||
| var a = A.F(x, y); | ||
| var b = a[x, y]; | ||
| b.ToString(); // 1 |
There was a problem hiding this comment.
Previously, a warning was reported for y in a[x, y].
| { | ||
| var a = new A(); | ||
| var b = a[x, y]; // 1 | ||
| b.ToString(); |
There was a problem hiding this comment.
Previously, no warning was reported for y in a[x, y].
|
Will we need to take a new compiler in |
I believe the ref assemblies are already emitted correctly. The issue was in decoding the attributes when loading the methods from metadata. |
| public class A | ||
| { | ||
| public object this[object? x, object y] => new object(); | ||
| public static A? F0; |
There was a problem hiding this comment.
Are F0-4 necessary for this test?
There was a problem hiding this comment.
Are F0-4 necessary for this test?
Yes, we need more annotated reference types than unannotated reference types in the type definition to generate a [NullableContext(2)] on type, and that attribute is needed to force a [NullableContext(1)] on the property accessor.
There was a problem hiding this comment.
Yes, we need more annotated reference types than unannotated reference types in the type definition
that's probably why I couldn't repro this standalone myself....
333fred
left a comment
There was a problem hiding this comment.
LGTM (commit 1), other than the question about unused test elements.
* upstream/master: (43 commits) Fix typos (dotnet#47264) Disable CS0660 where Full Solution Analysis produces a different result Pass in correct arguments to TypeScript handler Add skipped failing test for IDE0044 (dotnet#45288) Avoid first chance exception Code review feedback part 3 Update stale URLs in the readme Update IntegrationTestSourceGenerator.cs Fix comment Update IntegrationTestSourceGenerator.cs Remove unused reference to obsolete class Update nullable annotations in Classification folder Update generator public API names: (dotnet#47222) NullableContextAttribute for property parameters is from accessor (dotnet#47223) Modify global analyzer config precedence: (dotnet#45871) Skip the C# source generator integration test running for now Additional XAML LSP handlers (dotnet#47217) Remap diagnostics using IWorkspaceVenusSpanMappingService in-proc Warn on type named "record" (dotnet#47094) Bail out early before getting SyntaxTree ...
Properties from metadata have parameters defined by the accessor including any
[Nullable]attributes, so the[NullableContext]attribute should also be taken from the accessor as well.(The
[NullableContext]attribute for a method parameter, including accessors, is the nearest attribute on the method, or the containing types.)Fixes #47221.