Skip to content

Commit

Permalink
Merge pull request #6531 from sharwell/no-fsa
Browse files Browse the repository at this point in the history 
Convert FixerWithFixAllAnalyzer to a symbol start/stop analyzer
  • Loading branch information
@sharwell
sharwell authored Mar 14, 2023
2 parents 5212c6e + 4320a30 commit cc75004
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 89 deletions.
141 changes: 55 additions & 86 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/FixAnalyzers/FixerWithFixAllAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public sealed class FixerWithFixAllAnalyzer : DiagnosticAnalyzer
DiagnosticSeverity.Warning,
description: s_localizableCodeActionNeedsEquivalenceKeyDescription,
isEnabledByDefault: true,
customTags: WellKnownDiagnosticTagsExtensions.CompilationEndAndTelemetry);
customTags: WellKnownDiagnosticTagsExtensions.Telemetry);

internal static readonly DiagnosticDescriptor OverrideCodeActionEquivalenceKeyRule = new(
DiagnosticIds.OverrideCodeActionEquivalenceKeyRuleId,
Expand All @@ -51,7 +51,7 @@ public sealed class FixerWithFixAllAnalyzer : DiagnosticAnalyzer
DiagnosticSeverity.Warning,
description: s_localizableCodeActionNeedsEquivalenceKeyDescription,
isEnabledByDefault: true,
customTags: WellKnownDiagnosticTagsExtensions.CompilationEndAndTelemetry);
customTags: WellKnownDiagnosticTagsExtensions.Telemetry);

internal static readonly DiagnosticDescriptor OverrideGetFixAllProviderRule = new(
DiagnosticIds.OverrideGetFixAllProviderRuleId,
Expand All @@ -61,7 +61,7 @@ public sealed class FixerWithFixAllAnalyzer : DiagnosticAnalyzer
DiagnosticSeverity.Warning,
description: CreateLocalizableResourceString(nameof(OverrideGetFixAllProviderDescription)),
isEnabledByDefault: true,
customTags: WellKnownDiagnosticTagsExtensions.CompilationEndAndTelemetry);
customTags: WellKnownDiagnosticTagsExtensions.Telemetry);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
ImmutableArray.Create(CreateCodeActionEquivalenceKeyRule, OverrideCodeActionEquivalenceKeyRule, OverrideGetFixAllProviderRule);
Expand All @@ -73,10 +73,10 @@ public override void Initialize(AnalysisContext context)
// We need to analyze generated code, but don't intend to report diagnostics on generated code.
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze);

context.RegisterCompilationStartAction(CreateAnalyzerWithinCompilation);
context.RegisterCompilationStartAction(AnalyzeCompilation);
}

private void CreateAnalyzerWithinCompilation(CompilationStartAnalysisContext context)
private static void AnalyzeCompilation(CompilationStartAnalysisContext context)
{
context.CancellationToken.ThrowIfCancellationRequested();

Expand Down Expand Up @@ -106,29 +106,38 @@ private void CreateAnalyzerWithinCompilation(CompilationStartAnalysisContext con

var createMethods = codeActionSymbol.GetMembers(CreateMethodName).OfType<IMethodSymbol>().ToImmutableHashSet();

CompilationAnalyzer compilationAnalyzer = new CompilationAnalyzer(codeFixProviderSymbol, codeActionSymbol, context.Compilation.Assembly, createMethods);
var analysisTypes = new AnalysisTypes(
CodeFixProviderType: codeFixProviderSymbol,
CodeActionType: codeActionSymbol,
GetFixAllProviderMethod: getFixAllProviderMethod,
EquivalenceKeyProperty: equivalenceKeyProperty,
CreateMethods: createMethods);

context.RegisterSymbolAction(compilationAnalyzer.AnalyzeNamedTypeSymbol, SymbolKind.NamedType);
context.RegisterOperationBlockStartAction(compilationAnalyzer.OperationBlockStart);
context.RegisterCompilationEndAction(compilationAnalyzer.CompilationEnd);
context.RegisterSymbolStartAction(context => AnalyzeNamedType(context, analysisTypes), SymbolKind.NamedType);
}

private sealed class CompilationAnalyzer
private static void AnalyzeNamedType(SymbolStartAnalysisContext context, AnalysisTypes analysisTypes)
{
private readonly INamedTypeSymbol _codeFixProviderSymbol;
private readonly INamedTypeSymbol _codeActionSymbol;
private readonly ImmutableHashSet<IMethodSymbol> _createMethods;
private readonly IAssemblySymbol _sourceAssembly;
var symbol = (INamedTypeSymbol)context.Symbol;
if (!symbol.DerivesFrom(analysisTypes.CodeFixProviderType))
return;

/// <summary>
/// Set of all non-abstract sub-types of <see cref="CodeFixProvider"/> in this compilation.
/// </summary>
private readonly HashSet<INamedTypeSymbol> _codeFixProviders = new();
var namedTypeAnalyzer = new NamedTypeAnalyzer(analysisTypes);

/// <summary>
/// Set of all non-abstract sub-types of <see cref="CodeAction"/> which override <see cref="CodeAction.EquivalenceKey"/> in this compilation.
/// </summary>
private readonly HashSet<INamedTypeSymbol> _codeActionsWithEquivalenceKey = new();
context.RegisterOperationBlockStartAction(namedTypeAnalyzer.OperationBlockStart);
context.RegisterSymbolEndAction(namedTypeAnalyzer.SymbolEnd);
}

private record AnalysisTypes(
INamedTypeSymbol CodeFixProviderType,
INamedTypeSymbol CodeActionType,
IMethodSymbol GetFixAllProviderMethod,
IPropertySymbol EquivalenceKeyProperty,
ImmutableHashSet<IMethodSymbol> CreateMethods);

private sealed class NamedTypeAnalyzer
{
private readonly AnalysisTypes _analysisTypes;

/// <summary>
/// Map of invocations from code fix providers to invocations that create a code action using the static "Create" methods on <see cref="CodeAction"/>.
Expand All @@ -140,35 +149,9 @@ private sealed class CompilationAnalyzer
/// </summary>
private readonly Dictionary<INamedTypeSymbol, HashSet<IObjectCreationOperation>> _codeActionObjectCreations = new();

public CompilationAnalyzer(
INamedTypeSymbol codeFixProviderSymbol,
INamedTypeSymbol codeActionSymbol,
IAssemblySymbol sourceAssembly,
ImmutableHashSet<IMethodSymbol> createMethods)
public NamedTypeAnalyzer(AnalysisTypes analysisTypes)
{
_codeFixProviderSymbol = codeFixProviderSymbol;
_codeActionSymbol = codeActionSymbol;
_sourceAssembly = sourceAssembly;
_createMethods = createMethods;
}

internal void AnalyzeNamedTypeSymbol(SymbolAnalysisContext context)
{
var namedType = (INamedTypeSymbol)context.Symbol;
if (namedType.DerivesFrom(_codeFixProviderSymbol))
{
lock (_codeFixProviders)
{
_codeFixProviders.Add(namedType);
}
}
else if (IsCodeActionWithOverriddenEquivlanceKeyCore(namedType))
{
lock (_codeActionsWithEquivalenceKey)
{
_codeActionsWithEquivalenceKey.Add(namedType);
}
}
_analysisTypes = analysisTypes;
}

internal void OperationBlockStart(OperationBlockStartAnalysisContext context)
Expand All @@ -179,26 +162,26 @@ internal void OperationBlockStart(OperationBlockStartAnalysisContext context)
}

INamedTypeSymbol namedType = method.ContainingType;
if (!namedType.DerivesFrom(_codeFixProviderSymbol))
if (!namedType.DerivesFrom(_analysisTypes.CodeFixProviderType))
{
return;
}

context.RegisterOperationAction(operationContext =>
context.RegisterOperationAction(context =>
{
var invocation = (IInvocationOperation)operationContext.Operation;
if (invocation.TargetMethod is IMethodSymbol invocationSym && _createMethods.Contains(invocationSym))
var invocation = (IInvocationOperation)context.Operation;
if (invocation.TargetMethod is IMethodSymbol invocationSym && _analysisTypes.CreateMethods.Contains(invocationSym))
{
AddOperation(namedType, invocation, _codeActionCreateInvocations);
}
},
OperationKind.Invocation);

context.RegisterOperationAction(operationContext =>
context.RegisterOperationAction(context =>
{
var objectCreation = (IObjectCreationOperation)operationContext.Operation;
var objectCreation = (IObjectCreationOperation)context.Operation;
IMethodSymbol constructor = objectCreation.Constructor;
if (constructor != null && constructor.ContainingType.DerivesFrom(_codeActionSymbol))
if (constructor != null && constructor.ContainingType.DerivesFrom(_analysisTypes.CodeActionType))
{
AddOperation(namedType, objectCreation, _codeActionObjectCreations);
}
Expand All @@ -221,33 +204,25 @@ private static void AddOperation<T>(INamedTypeSymbol namedType, T operation, Dic
}
}

internal void CompilationEnd(CompilationAnalysisContext context)
internal void SymbolEnd(SymbolAnalysisContext context)
{
if (_codeFixProviders == null)
{
// No fixers.
return;
}

if (_codeActionCreateInvocations == null && _codeActionObjectCreations == null)
if (_codeActionCreateInvocations.Count == 0 && _codeActionObjectCreations.Count == 0)
{
// No registered fixes.
return;
}

// Analyze all fixers that have FixAll support.
// Analyze the fixer if it has FixAll support.
// Otherwise, report RS1016 (OverrideGetFixAllProviderRule) to recommend adding FixAll support.
foreach (INamedTypeSymbol fixer in _codeFixProviders)
var fixer = (INamedTypeSymbol)context.Symbol;
if (OverridesGetFixAllProvider(fixer))
{
if (OverridesGetFixAllProvider(fixer))
{
AnalyzeFixerWithFixAll(fixer, context);
}
else if (SymbolEqualityComparer.Default.Equals(fixer.BaseType, _codeFixProviderSymbol))
{
Diagnostic diagnostic = fixer.CreateDiagnostic(OverrideGetFixAllProviderRule, fixer.Name);
context.ReportDiagnostic(diagnostic);
}
AnalyzeFixerWithFixAll(fixer, context);
}
else if (SymbolEqualityComparer.Default.Equals(fixer.BaseType, _analysisTypes.CodeFixProviderType))
{
Diagnostic diagnostic = fixer.CreateDiagnostic(OverrideGetFixAllProviderRule, fixer.Name);
context.ReportDiagnostic(diagnostic);
}

return;
Expand All @@ -257,7 +232,7 @@ bool OverridesGetFixAllProvider(INamedTypeSymbol fixer)
{
foreach (INamedTypeSymbol type in fixer.GetBaseTypesAndThis())
{
if (!SymbolEqualityComparer.Default.Equals(type, _codeFixProviderSymbol))
if (!SymbolEqualityComparer.Default.Equals(type, _analysisTypes.CodeFixProviderType))
{
IMethodSymbol getFixAllProviderMethod = type.GetMembers(GetFixAllProviderMethodName).OfType<IMethodSymbol>().FirstOrDefault();
if (getFixAllProviderMethod != null && getFixAllProviderMethod.IsOverride)
Expand Down Expand Up @@ -290,7 +265,7 @@ static bool IsViolatingCodeActionCreateInvocation(IInvocationOperation invocatio
return true;
}

void AnalyzeFixerWithFixAll(INamedTypeSymbol fixer, CompilationAnalysisContext context)
void AnalyzeFixerWithFixAll(INamedTypeSymbol fixer, SymbolAnalysisContext context)
{
if (_codeActionCreateInvocations != null
&& _codeActionCreateInvocations.TryGetValue(fixer, out HashSet<IInvocationOperation> invocations))
Expand Down Expand Up @@ -326,17 +301,11 @@ bool IsViolatingCodeActionObjectCreation(IObjectCreationOperation objectCreation
// Local functions
bool IsCodeActionWithOverriddenEquivalenceKey(INamedTypeSymbol namedType)
{
if (SymbolEqualityComparer.Default.Equals(namedType, _codeActionSymbol))
if (SymbolEqualityComparer.Default.Equals(namedType, _analysisTypes.CodeActionType))
{
return false;
}

// We are already tracking CodeActions with equivalence key in this compilation.
if (SymbolEqualityComparer.Default.Equals(namedType.ContainingAssembly, _sourceAssembly))
{
return _codeActionsWithEquivalenceKey != null && _codeActionsWithEquivalenceKey.Contains(namedType);
}

// For types in different compilation, perform the check.
return IsCodeActionWithOverriddenEquivlanceKeyCore(namedType);
}
Expand All @@ -345,7 +314,7 @@ bool IsCodeActionWithOverriddenEquivalenceKey(INamedTypeSymbol namedType)

private bool IsCodeActionWithOverriddenEquivlanceKeyCore(INamedTypeSymbol namedType)
{
if (!namedType.DerivesFrom(_codeActionSymbol))
if (!namedType.DerivesFrom(_analysisTypes.CodeActionType))
{
// Not a CodeAction.
return false;
Expand Down
3 changes: 0 additions & 3 deletions src/Microsoft.CodeAnalysis.Analyzers/Microsoft.CodeAnalysis.Analyzers.sarif
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@
"Visual Basic"
],
"tags": [
"CompilationEnd",
"Telemetry"
]
}
Expand All @@ -111,7 +110,6 @@
"Visual Basic"
],
"tags": [
"CompilationEnd",
"Telemetry"
]
}
Expand Down Expand Up @@ -166,7 +164,6 @@
"Visual Basic"
],
"tags": [
"CompilationEnd",
"Telemetry"
]
}
Expand Down

0 comments on commit cc75004

Please sign in to comment.