AdditionalLocations for CS8618 in DiagnosticSuppressor

[roji]

In EF Core, dotnet/efcore#21608 introduced a DiagnosticsSuppressor to suppress CS8618 (non-nullable property is uninitialized) in a specific safe scenario. In classes without a constructor, the CS8618 diagnostic is reported on the property itself, so the suppressor can look at that property and function correctly. If a constructor is defined, however, the diagnostic is reported on the constructor declaration instead of the property, and the suppressor needs some way to resolve the uninitialized property.

Diagnostic seems to have an AdditionalLocations property which looks right for this, but it's empty (because not populated for CS8618? Because it's a DiagnosticsSuppressor rather than a regular analyzer?).

Am I missing something? What would be the correct, safe way to figure out which non-nullable property is uninitialized from CS8618 in a DiagnosticsSuppressor?

Tracking issue on the EF Core side: dotnet/efcore#26879

[Youssef1313]

A workaround that I can think of is to get the message using Diagnostic.GetMessage, find the property name (it's enclosed between single quotes in the message), then call constructorSymbol.ContainingType.GetMembers(propertyName).

The compiler can make this easier if it used AdditionalLocations for reporting CS8618. It currently uses exitLocation ?? symbol.Locations.FirstOrNone()

roslyn/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

Line 623 in 8a92d8a

Diagnostics.Add(ErrorCode.WRN_UninitializedNonNullableField, exitLocation ?? symbol.Locations.FirstOrNone(), symbol.Kind.Localize(), symbol.Name);

[roji]

@Youssef1313 thanks for your response.

A workaround that I can know of is to get the message using Diagnostic.GetMessage, find the property name (it's enclosed between single quotes in the message), then call constructorSymbol.ContainingType.GetMembers(propertyName).

Would you consider this a safe, robust workaround? For example, isn't it possible that this would fail in a different locale, since single quotes are used in the localized message in some way?

Another workaround approach may be to use reflection to access CSDiagnostic.Arguments and get the property name from there - though that's also problematic.

If this info can be added to AdditionalLocations (or any other way that allows this important context to be accessed), then we should be fine (this is a 7.0 release feature so we can wait for this to happen on the Roslyn side).

[Youssef1313]

@roji I think localization shouldn't be an issue. GetMessage takes a CultureInfo. But since this is for 7.0 release, you can wait until the symbol is added to AdditionalLocations which is absolutely better than my suggested workaround.

[roji]

Thanks @Youssef1313! I'll wait for this to be done for 7.0 then and react on the EF Core side.

[Youssef1313]

@roji I got some time and fixed this. #58096

The new behavior is that the main location (where the squiggle appears) is the same as before, but always have the symbol locations in AdditionalLocations. Let me know if you want any adjustments to this behavior.

[roji]

@Youssef1313 thanks for the super quick turnaround! This indeed looks like what I need, once it's merged I'll make the necessary changes on the EF Core side and report how it goes.

[roji]

Thanks @Youssef1313 and @RikkiGibson!