Incorrect report of RS1008 (storing per-compilation data)

[sharwell]

I believe the following will reproduce the issue:

private static readonly Action<SyntaxTreeAnalysisContext, Compilation, StyleCopSettings> SyntaxTreeAction = HandleSyntaxTree;

private static void HandleSyntaxTree(SyntaxTreeAnalysisContext context, Compilation compilation, StyleCopSettings settings)
{
}

The warning is reported in the declaration of SyntaxTreeAction, on the token Compilation.

[Evangelink]

The code currently looks for outer and inner types of all fields. So fields of type Action or Func with any inner type being Compilation will be reporting an issue.

We could ignore inner types for those cases. I am wondering if this exclusion should be more restrictive and only be applicable for readonly fields. Because it could be possible to capture some compilation through a local lambda declaration in which case the message would still make sense.

It could be interesting to have 2 messages, one for confident cases and one where we would want the user to review the actual usage.

WDYT @sharwell?

Full test:

[Fact]
[WorkItem(782, "https://github.com/dotnet/roslyn-analyzers/issues/782")]
public void CSharp_ActionWithCompilation_NoDiagnostic()
{
	var source = @"
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Diagnostics;

class StyleCopSettings { }

[DiagnosticAnalyzer(LanguageNames.CSharp)]
class MyAnalyzer : DiagnosticAnalyzer
{
	private static readonly Action<SyntaxTreeAnalysisContext, Compilation, StyleCopSettings> SyntaxTreeAction = HandleSyntaxTree;

	private static void HandleSyntaxTree(SyntaxTreeAnalysisContext context, Compilation compilation, StyleCopSettings settings)
	{
	}

	public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
	{
		get
		{
			throw new NotImplementedException();
		}
	}

	public override void Initialize(AnalysisContext context)
	{
	}
}";

	VerifyCSharp(source);
}

[themightyzek]

This warning is also reported on nested types that that contain a variable of type INamedTypeSymbol.
I believe that this code resembles correct usage, but the warning is thrown nonetheless.
Moving the struct out of the class stops the warning from triggering.

namespace MyNamespace
{
    [DiagnosticAnalyzer(LanguageNames.CSharp)]
    public class AnyInstanceInjectionAnalyzer : APCodingRulesAnalyzerBase
    {
        public struct DependencyAccess
        {
            public IMethodSymbol method;
            public string expectedName;
        }

        public override void Initialize(AnalysisContext context)
        {
            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
            context.EnableConcurrentExecution();
            context.RegisterCompilationStartAction(OnCompilationStart);
        }

        public void OnCompilationStart(CompilationStartAnalysisContext context)
        {
            var accessors = new ConcurrentBag<DependencyAccess>();

            context.RegisterSymbolAction(
                symbolContext => AnalyzeSymbol(symbolContext, accessors),
                SymbolKind.Property,
                SymbolKind.Field
            );

            context.RegisterSemanticModelAction(
                semanticModelContext => AnalyzeSemanticModel(semanticModelContext, accessors)
            );
        }

        public void AnalyzeSymbol(SymbolAnalysisContext context, ConcurrentBag<DependencyAccess> accessors)
        {
            // collect symbols for analysis
        }

        public void AnalyzeSemanticModel(SemanticModelAnalysisContext context, ConcurrentBag<DependencyAccess> accessors)
        {
            foreach (var access in accessors)
            {
                // analyze
            }
        }
    }
}

[themightyzek]

Also, the warning no longer triggers when I change the IMethodSymbol in the code to a ISymbol type. Is this intended behaviour?

[Rekkonnect]

Also, the warning no longer triggers when I change the IMethodSymbol in the code to a ISymbol type. Is this intended behaviour?

It should not be, and I reported this bug here #7196