`IDE0300` is reported twice

[DoctorKrolic]

Version Used: VS 17.9 Preview 2 and roslyn main (was finally able to run roslyn deployment from the source)

Steps to Reproduce:

int[] integers = new int[] { 1, 2, 3 };

Hover over new to see diagnostics

Diagnostic Id: IDE0300

Expected Behavior: I see a single diagnostic message

Actual Behavior: When hovering over new there are 2 IDE0300 diagnostics: However there is only 1 IDE0300 in the error list:

[DoctorKrolic]

@CyrusNajmabadi I believe collection expressions IDE features are on you

[CyrusNajmabadi]

@mavasani could you tell me what i did wrong here? I was trying to follow what we do elsewhere. Where there is one main diagnostic (for teh ...) and another for the fading. But taht seems to have caused two things to appear here. The code in question is here:

https://github.com/dotnet/roslyn/blob/4405f0ede8ec376df6bd28ac25f75b94fdb7deb0/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs#L163-L186

[mavasani]

@CyrusNajmabadi DiagnosticHelper.CreateWithLocationTags API was added specifically to allow us to report diagnostic whose primary location maybe different from the location(s) required to be faded without needing to report separate diagnostics just for fading purposes. We shouldn't even need a separate UnnecessaryCodeDescriptor. Analyzers needing fading should create a single diagnostic with DiagnosticHelper.CreateWithLocationTags such that all locations needing fading are passed as argument for additionalUnnecessaryLocations

I'll create a PR to clean this up.

[CyrusNajmabadi]

@mavasani i was just copying this from a preexisting analyzer (not sure which one) :)

[mavasani]

@mavasani i was just copying this from a preexisting analyzer (not sure which one) :)

Sure, I think we might have left behind some usages of old pattern when we added the CreateWithLocationTags API. My PR should fix up all of them now.

[mavasani]

@CyrusNajmabadi Question for you - do you expect code fix to be offered in the entire span of fading locations, even if the primary location is much smaller or completely at a different place in the file? If so, I think we may have to fix up the CodeFix service to not filter out diagnostics whose primary location might not intersect with current line span but at least one fading location does - will need to file a separate tracking issue for it.

[CyrusNajmabadi]

Question for you - do you expect code fix to be offered in the entire span of fading locations, even if the primary location is much smaller or completely at a different place in the file?

Yes :-)

[DoctorKrolic]

The fix was reverted, probably need to reopen this issue to not loose it

[mavasani]

@CyrusNajmabadi So, I have encountered a tricky issue while attempting to implement this functionality, and wanted to hear your thoughts. If we switch over the analyzers to report a single diagnostic with the fading locations encoded in diagnostic's property bag instead of separate hidden, unnecessary diagnostics, everything works fine when computing full document diagnostics. However, for computing diagnostics for a line span (for lightbulb), the core analyzer driver will filter out reported analyzer diagnostics whose fading location overlaps with the requested span, but not the primary diagnostic location, which means we will not see a code fix when pressing Ctrl + Dot in fading location.

My second attempt #71277 seems to workaround this issue for non-LSP pull diagnostics mode, as diagnostics computed from full document background analysis are cached and used when asked by the lightbulb, but that only seems like a workaround. The behavior of missing lightbulb still persists in LSP pull diagnostics mode after #71277 as in that mode we don't cache diagnostics from full document background analysis.

I believe we have the following options here:

1. Change the core analyzer driver in the compiler layer to understand fading locations encoded in diagnostic's property bag, and don't filter out these diagnostics when a fading location overlaps the requested input span.
2. Retain the current implementation of reporting separate hidden, unnecessary diagnostics for fading purposes, but mark them as non-configurable (and probably choose a separate diagnostic ID) and special case these diagnostics in places such as quick info to avoid duplicates. I am not sure if the implementation will be in Roslyn or VS Platform code for LSP pull diagnostics
3. Take #71277 and also update LSP pull diagnostics to use cache the computed full document diagnostics from background analysis.
4. Do nothing - I don't think the current user experience described in this bug is that big an issue. We definitely don't want to regress functionality or make complex code changes just for this fit and finish bug.

[Cosifne]

Manish no longer works in the IDE team. So reassigned to Cyrus and Joey (shown as analyzer maintainer) to investigate.