dotnet / roslyn-sdk

Roslyn-SDK templates and Syntax Visualizer
MIT License
517 stars 256 forks source link

Inject/Forge Diagnostics or run "foreign" Analyzers in CodeFix-Test #1028

Open LukasGelke opened 2 years ago

LukasGelke commented 2 years ago

I'm currently writing a CodeFixProvider which will provide possible fixes to existing Diagnostics based on pre-existing company guidelines/codebase. But I cannot test it, as the original Diagnostic (e.g. CA1062) is never triggered, even when adding the Package Microsoft.CodeAnalysis.NetAnalyzers to the TestState.ReferenceAssemblies and adding an .editorconfig (with that Analyzer on Warning) as an AdditionalFile.

My Test

using ValidatePublicParameterWithKomsaAssertVerifier = Komsa.CodeAnalyzers.Test.Internal.KomsaCodeFixVerifier<
  Komsa.CodeAnalyzers.CodeFixProviders.ValidatePublicParameterWithKomsaAssertCodeFixProvider>;

public async Task FindAndAddAssertsAsyncTest(string folder, string code, string fix, string key)
  {
    ReferenceAssemblies assemblies = ReferenceAssemblies.Net.Net60
      .AddPackages(ImmutableArray.Create<PackageIdentity>(item: new("Microsoft.CodeAnalysis.NetAnalyzers", "6.0.0")));
    var addtional = (".editorconfig", @"
[*.cs]

# CA1062: Validate arguments of public methods
dotnet_diagnostic.CA1062.severity = warning
");
    ValidatePublicParameterWithKomsaAssertVerifier.Test test = new()
    {
      TestState =
      {
        Sources = { CodeAnalyzerTestHelper.GetSourceCode(Rule, folder, code) },
        AdditionalReferences = { typeof(KomsaAssert).Assembly },
        AdditionalFiles = { addtional },
        ReferenceAssemblies = assemblies,
      },
      FixedState =
      {
        Sources = { CodeAnalyzerTestHelper.GetSourceCode(Rule, folder, fix) },
        AdditionalReferences = { typeof(KomsaAssert).Assembly },
        AdditionalFiles = { addtional },
        ReferenceAssemblies = assemblies,
      },
      CodeActionEquivalenceKey = "ValidatePublicParameterWithKomsaAssertCodeFixProvider:CA1062:" + key,
    };
    await test.RunAsync().ConfigureAwait(false);
  }

TestState.Source

public sealed class MyClass
{
  public void DoStuff(object o)
  {
    _ = {|CA1062:o|}.ToString();
  }
}
internal sealed class KomsaCodeFixVerifier<TCodeFix>
  : KomsaCodeFixVerifier<EmptyDiagnosticAnalyzer, TCodeFix> {...}
  //... and corresponding classes exist, to add company-defaults

ValidatePublicParameterWithKomsaAssertCodeFixProvider.FixableDiagnosticIds = { "CA1062" }

Assert.AreEqual failed. Expected:<1>. Actual:<0>. Context: Diagnostics of test state Mismatch between number of diagnostics returned, expected "1" actual "0"

Removing the markup doesn't raise this Exception, but I obviously have no Diagnostic for my CodeFix.

How can I produce a "usable" Diagnostic? For this test, i would only need the Span, so the markup would be sufficient. But to "future-proof" the CodeFix (and others), it would be "more stable" to run the original Analyzer. But one can't reference an Analyzer from a NuGet-Package, and use it in the Verifier.

LukasGelke commented 2 years ago

oh, and it might also be used for testing Suppressors

sharwell commented 1 year ago

You'll want to change EmptyDiagnosticAnalyzer to Microsoft.CodeQuality.Analyzers.QualityGuidelines.ValidateArgumentsOfPublicMethods, which is the analyzer responsible for reporting CA1062.

LukasGelke commented 1 year ago

this is what i thought of first too, but: a) which NuGet/Reference do i need? and more importantly b) how would I get to that in code? Assuming I can't consume an Analyzer Package normally and access it's containing Analyzers, the way it's done with 'lib' Packages

LukasGelke commented 1 year ago

ok. So some Analyzers can be used like this

  <ItemGroup>
    <PackageReference Update="Microsoft.CodeAnalysis.NetAnalyzers" GeneratePathProperty="true" />
    <Reference Include="Microsoft.CodeAnalysis.NetAnalyzers">
      <HintPath>$(PkgMicrosoft_CodeAnalysis_NetAnalyzers)\analyzers\dotnet\cs\Microsoft.CodeAnalysis.NetAnalyzers.dll</HintPath>
      <CopyLocal>true</CopyLocal>
      <Private>True</Private>
    </Reference>
  </ItemGroup>

it seams hacky, but it works.

Now though, we would like to test stuff regarding some 'IDE'-Diagnostics. For instance IDE0002. But that's in dotnet/roslyn CSharpSimplifyTypeNamesDiagnosticAnalyzer which is internal and inaccessible through normal means. But I've hit a roadblock: https://github.com/dotnet/roslyn/blob/0b24f6bf0bc243a102248ca501e35f2d85326212/src/Analyzers/Core/Analyzers/EnforceOnBuildValues.cs#L123 suggests that IDE0002 cannot be used during build in the first place.

But how/where would one enable these Analyzers in the Testing Environment? Because https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/overview?tabs=net-7#enable-on-build says to first "Set the MSBuild property EnforceCodeStyleInBuild to true". Now I'm not sure if a Test would "qualify as a build" at all, and how to set it

sharwell commented 1 year ago

If you want to manually add diagnostics to a test, it's pretty trivial. Here's a test analyzer that reports a diagnostic for literal integers less than 5: https://github.com/dotnet/roslyn-sdk/blob/f9932c818a30eedd0715df2971c340a695d393af/tests/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Utilities/TestAnalyzers/LiteralUnderFiveAnalyzer.cs

LukasGelke commented 1 year ago

Hm. Yes, that would be possible, but I'd like the "real analyzer" better.

I need to refer to my previous comment. Our test works with Microsoft.CodeAnalysis.CSharp.Workspaces Version 4.4.0 but in trying to upgrade to version 4.5.0 there are no more diagnostics reported by Microsoft.CodeQuality.Analyzers.QualityGuidelines.ValidateArgumentsOfPublicMethods

dellis1972 commented 2 weeks ago

@LukasGelke @sharwell did you manage to solve the IDE0002 issue?

I'm am trying to test a IDE0002 suppressor for .NET Android and I'm hitting similar issues. The workaround I have for loading the test is as follows

Reference the Microsoft.CodeAnalysis.CSharp.Features Nuget

 <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Features" Version="4.11.0" GeneratePathProperty="true" />

Make sure its copied to output. This is probably not the best way.

 <ItemGroup>
<None Include="$(PkgMicrosoft_CodeAnalysis_CSharp_Features)\*\*.dll">
      <Visible>False</Visible>
      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
    </None>  
  </ItemGroup>

Then I have this Test class which loads the Analyzers and filters it to IDE0002

public class Test : CSharpAnalyzerTest<TAnalyzer, DefaultVerifier>
    {
        List<DiagnosticAnalyzer> analyzers = new List<DiagnosticAnalyzer>();

        public List<DiagnosticAnalyzer> Analyzers => analyzers;
        public Test()
        {
            SolutionTransforms.Add((solution, projectId) =>
            {
                var project = solution.GetProject(projectId);
                var compilationOptions = project.CompilationOptions;
                compilationOptions = compilationOptions.WithSpecificDiagnosticOptions(
                    compilationOptions.SpecificDiagnosticOptions.SetItems(CSharpVerifierHelper.NullableWarnings));
                solution = solution.WithProjectCompilationOptions(projectId, compilationOptions);
                return solution;
            });

            var a = new AnalyzerFileReference (Path.GetFullPath("Microsoft.CodeAnalysis.CSharp.Features.dll"), assemblyLoader: AssemblyLoader.Instance);
            foreach (var a1 in a.GetAnalyzers (LanguageNames.CSharp)) {
                if (a1.SupportedDiagnostics.Any( x => x.Id == "IDE0002"))
                    analyzers.Add (a1);
            }
        }

        protected override IEnumerable<DiagnosticAnalyzer> GetDiagnosticAnalyzers ()
        {
            return Analyzers;
        }
    }

Then I needed to generated a "wrapper" Analyzer for the existing rule .

[DiagnosticAnalyzer (LanguageNames.CSharp)]
#pragma warning disable RS1036 // Specify analyzer banned API enforcement setting
    public class IDE0002AnalyserWrapper : DiagnosticAnalyzer
    {
        DiagnosticAnalyzer analyzer;
        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => GetDiagnostics ();

        private ImmutableArray<DiagnosticDescriptor> GetDiagnostics ()
        {
            return ImmutableArray.Create (analyzer.SupportedDiagnostics.ToArray ());
        }

        public IDE0002AnalyserWrapper ()
        {
            var a = new AnalyzerFileReference (Path.GetFullPath ("Microsoft.CodeAnalysis.CSharp.Features.dll"), assemblyLoader: AssemblyLoader.Instance);
            foreach (var a1 in a.GetAnalyzers (LanguageNames.CSharp)) {
                if (a1.SupportedDiagnostics.Any (x => x.Id == "IDE0002")) {
                    analyzer = a1;
                    break;
                }
            }
        }

        public override void Initialize (AnalysisContext context)
        {
            analyzer.Initialize (context);
        }
    }
#pragma warning restore RS1036 // Specify analyzer banned API enforcement settingX

I can then test via

using VerifyCSAnalyser = CSharpAnalyzerVerifier<DNAS0001Tests.IDE0002AnalyserWrapper>;
...
[Test]
    public async Task IDE0001IsNotSuppressed ()
    {
        var expected = VerifyCSAnalyser.Diagnostic (new DiagnosticDescriptor ("IDE0002", "", "Name can be simplified", "", DiagnosticSeverity.Hidden, isEnabledByDefault: true)).WithSpan (11, 23, 11, 31);
        await VerifyCSAnalyser.VerifyAnalyzerAsync (brokenCode, expected);
    }

I then wrote the following Verifier

public static partial class CSharpSuppressorVerifier<TAnalyzer, TSuppressor>
    where TAnalyzer : DiagnosticAnalyzer, new()
    where TSuppressor: DiagnosticSuppressor, new ()
{
    /// <inheritdoc cref="AnalyzerVerifier{TAnalyzer, TTest, TVerifier}.Diagnostic()"/>
    public static DiagnosticResult Diagnostic()
        => CSharpAnalyzerVerifier<TAnalyzer, DefaultVerifier>.Diagnostic();

    /// <inheritdoc cref="AnalyzerVerifier{TAnalyzer, TTest, TVerifier}.Diagnostic(string)"/>
    public static DiagnosticResult Diagnostic(string diagnosticId)
        => CSharpAnalyzerVerifier<TAnalyzer, DefaultVerifier>.Diagnostic(diagnosticId);

    /// <inheritdoc cref="AnalyzerVerifier{TAnalyzer, TTest, TVerifier}.Diagnostic(DiagnosticDescriptor)"/>
    public static DiagnosticResult Diagnostic(DiagnosticDescriptor descriptor)
        => CSharpAnalyzerVerifier<TAnalyzer, DefaultVerifier>.Diagnostic(descriptor);
    public static async Task VerifySuppressorAsync(string source, params DiagnosticResult[] expected)
    {
        var test = new Test
        {
            TestCode = source,
        };
        test.ExpectedDiagnostics.AddRange(expected);
        await test.RunAsync(CancellationToken.None);
    }

    public class Test : CSharpAnalyzerTest<TAnalyzer, DefaultVerifier>
    {
        protected override IEnumerable<DiagnosticAnalyzer> GetDiagnosticAnalyzers()
            => base.GetDiagnosticAnalyzers().Concat(new[] { new TSuppressor() });
    }
}

Which I tested via

using VerifyCSSuppressor = CSharpSuppressorVerifier<DNAS0001Tests.IDE0002AnalyserWrapper, ResourceDesignerDiagnosticSuppressor>;
...
    [Test]
    public async Task IDE0001IsSuppressed ()
    {
        var expected = VerifyCSSuppressor.Diagnostic (new DiagnosticDescriptor ("IDE0002", "", "Name can be simplified", "", DiagnosticSeverity.Hidden, isEnabledByDefault: true)).WithSpan (11, 23, 11, 31).WithIsSuppressed (true);
        await VerifyCSSuppressor.VerifySuppressorAsync (brokenCode, expected);
    }

This works fine until I try to test the suppressor. Then I get the following crash, so I'm not sure where to go from here.

IDE0001IsSuppressed (42ms): Error Message: System.TypeInitializationException : The type initializer for 'Microsoft.C
      odeAnalysis.Testing.Lightup.ProgrammaticSuppressionInfoWrapper' threw an exception.
        ----> System.InvalidOperationException : Property 'System.Collections.Immutable.ImmutableArray`1[Microsoft.CodeAnal
      ysis.Diagnostics.Suppression] Suppressions' produces a value of type 'System.Collections.Immutable.ImmutableArray`1[M
      icrosoft.CodeAnalysis.Diagnostics.Suppression]', which is not assignable to type 'System.Collections.Immutable.Immuta
      bleHashSet`1[System.ValueTuple`2[System.String,Microsoft.CodeAnalysis.LocalizableString]]'
      Stack Trace:
         at Microsoft.CodeAnalysis.Testing.Lightup.ProgrammaticSuppressionInfoWrapper.FromInstance(Object instance) in /_/s
      rc/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/Lightup/ProgrammaticSuppressionInfoWrapper.
      cs:line 32
         at Microsoft.CodeAnalysis.Testing.Extensions.DiagnosticExtensions.ProgrammaticSuppressionInfo(Diagnostic diagnosti
      c) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/Extensions/DiagnosticExtensions.c
      s:line 38
         at Microsoft.CodeAnalysis.Testing.AnalyzerTest`1.<>c.<FilterDiagnostics>b__103_0(ValueTuple`2 d) in /_/src/Microso
      ft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs:line 1687
         at System.Linq.Enumerable.ArrayWhereIterator`1.ToArray(ReadOnlySpan`1 source, Func`2 predicate)
         at System.Linq.Enumerable.ArrayWhereIterator`1.ToArray()
         at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
         at System.Collections.Immutable.ImmutableArray.CreateRange[T](IEnumerable`1 items)
         at System.Collections.Immutable.ImmutableArray.ToImmutableArray[TSource](IEnumerable`1 items)
         at Microsoft.CodeAnalysis.Testing.AnalyzerTest`1.FilterDiagnostics(ImmutableArray`1 diagnostics) in /_/src/Microso
      ft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs:line 1686
         at Microsoft.CodeAnalysis.Testing.AnalyzerTest`1.GetSortedDiagnosticsAsync(Solution solution, ImmutableArray`1 ana
      lyzers, ImmutableArray`1 additionalDiagnostics, CompilerDiagnostics compilerDiagnostics, IVerifier verifier, Cancella
      tionToken cancellationToken) in /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/Analyze
      rTest`1.cs:line 1144
sharwell commented 1 week ago

This works fine until I try to test the suppressor. Then I get the following crash, so I'm not sure where to go from here.

This was reported as #1175 and has since been fixed.