"IDE0305 Use collection expression for fluent" inconsistent between .NET 8.0.204 and 8.0.300 despite explicit analyzer package dependency

[Piedone]

Analyzer

Diagnostic ID: IDE0305: Use collection expression for fluent

Analyzer source

NuGet Package: Microsoft.CodeAnalysis.CSharp.CodeStyle

Version: 4.9.2

Describe the bug

Steps To Reproduce

1. Create a project that explicitly references Microsoft.CodeAnalysis.CSharp.CodeStyle , and enforces the usage of the analyzer from the package (instead of the SDK) with:

<_SkipUpgradeNetAnalyzersNuGetWarning>true</_SkipUpgradeNetAnalyzersNuGetWarning>
<EnforceCodeStyleInBuild>false</EnforceCodeStyleInBuild>

2. Add the following code:

using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.Primitives;

namespace Lombiq.AnalyzersTest
{
    public static class ApplicationBuilderExtensions
    {
        public static IApplicationBuilder UseStrictAndSecureCookies(this IApplicationBuilder app)
        {
            return app.Use((context, next) =>
            {
                const string setCookieHeader = "Set-Cookie";
                context.Response.OnStarting(() =>
                {
                    var setCookie = context.Response.Headers[setCookieHeader];
                    var newCookies = new List<string>(capacity: setCookie.Count);
                    context.Response.Headers[setCookieHeader] = new StringValues(newCookies.ToArray());

                    return Task.CompletedTask;
                });

                return next();
            });
        }
    }
}

3. Build it with .NET 8.0.204 (I've run dotnet build Lombiq.AnalyzersTest.sln --no-incremental -warnaserror /p:TreatWarningsAsErrors=true /p:RunAnalyzersDuringBuild=true -nologo -consoleLoggerParameters:NoSummary -verbosity:quiet).
4. Observe that the build passes and there are no analyzer violations.
5. Build it with .NET 8.0.300.
6. Observe that the build correctly fails with "IDE0305: Collection initialization can be simplified" analyzer violation on the context.Response.Headers[setCookieHeader] = new StringValues(newCookies.ToArray()); line.

Expected behavior

The analyzer violation is surfaced with both SDK versions.

Actual behavior

Despite the project taking an explicit dependency on the analyzer package and setting _SkipUpgradeNetAnalyzersNuGetWarning to true, the analyzer's behavior is different between .NET versions.

Additional context

Full source for the passing build available here and the failing here. The offending code is in ApplicationBuilderExtensions in both cases. Just changed dotnet-version in .github/workflows/build-and-test.yml between the two.

We've seen similar inconsistencies with IDE rules previously too. This is an issue because our CI workflows take a dependency on .NET 8.0.x, i.e. auto-updating with patch versions, since we consider patch updates safe. However, still, patch version updates bring new analyzer violations like this, breaking our builds.

While an alternative would be to take dependency on an exact .NET version, we'd prefer to keep patch updates coming. However, we still expect analyzer behavior to be consistent across patch versions due to us using the analyzer from the NuGet package.

#73638 might be related.

[mpidash]

AFAIK all IDExxxx rules are part of roslyn (in this case contained in Microsoft.CodeAnalysis.CSharp.CodeStyle) and not part of roslyn-analyzer (Microsoft.CodeAnalysis.NetAnalyzers), so I am not sure if setting _SkipUpgradeNetAnalyzersNuGetWarning has any effect.

[CyrusNajmabadi]

@sharwell @mavasani is it intentional that the experience is different here? I don't know what is actually expected in this case.

[sharwell]

@CyrusNajmabadi I verified that the same code style DLL is being used in both cases. I cannot find any indication that this scenario is impacted by the severity configuration changes that occurred in 8.0.300. I am guessing that the difference is one of:

1. The .NET reference assemblies changed from v8.0.4 to v8.0.5 (but couldn't find any reason why this would matter)
2. The compiler changed from 4.9.2-3.24129.6 (9934fb9) to 4.10.0-3.24216.12 (3af0081)

I suspect that a change within the speculative model in the newer version made the analyzer start working.

It sounds like this is a true positive diagnostic where the previous false negative has since been fixed. I don't believe further work would be necessary unless either 1) using the default code style package on 8.0.300+ causes the diagnostic to disappear again or 2) we plan to back port the fix for this to servicing branch 8.0.2xx.

[Piedone]

What would you recommend us avoiding our code suddenly failing static code analysis and thus CI builds with .NET patch updates? Anything better than pinning the .NET version to a concrete patch version?

[sharwell]

Pinning the .NET version in your CI configuration is a great way to ensure it stays stable. You just have to have a process in place to manually update it, e.g. a bot that submits a PR to increment the version used for CI builds when it becomes available. That way, anything in that update that would destabilize the build is constrained to a pull request, and by the time it merges you'll have resolved it.

[CyrusNajmabadi]

Seems reasonable to me that if versions are changing that behavior changes. We don't maintain behavior consistency between analyzer releases. Bug fixes, feature requests, and whatnot all lead to changes.

[Piedone]

OK, thanks!