Skip to content

Commit c6a9bab

Browse files
viceroypenguinviceroypenguin
authored
Improve static handler management (#329)
* `IHR0019` disabled by default when no dependencies * Suppress CA1822 when relevant
1 parent a02a16a commit c6a9bab

20 files changed

Lines changed: 617 additions & 24 deletions

‎Directory.Packages.props‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,27 +27,31 @@
2727

2828
<ItemGroup Condition=" '$(TargetFramework)' == 'net11.0' ">
2929
<PackageVersion Include="Basic.Reference.Assemblies.Net110" Version="1.8.6" />
30+
<PackageVersion Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="11.0.100-preview.3.26207.106" />
3031
<PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="11.0.0-preview.3.26207.106" />
3132
<PackageVersion Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="11.0.0-preview.3.26207.106" />
3233
<PackageVersion Include="Microsoft.Extensions.Diagnostics.Testing" Version="10.5.0" />
3334
</ItemGroup>
3435

3536
<ItemGroup Condition=" '$(TargetFramework)' == 'net10.0' ">
3637
<PackageVersion Include="Basic.Reference.Assemblies.Net100" Version="1.8.6" />
38+
<PackageVersion Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="10.0.203" />
3739
<PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="10.0.7" />
3840
<PackageVersion Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="10.0.7" />
3941
<PackageVersion Include="Microsoft.Extensions.Diagnostics.Testing" Version="10.5.0" />
4042
</ItemGroup>
4143

4244
<ItemGroup Condition=" '$(TargetFramework)' == 'net9.0' ">
4345
<PackageVersion Include="Basic.Reference.Assemblies.Net90" Version="1.8.6" />
46+
<PackageVersion Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="9.0.0" />
4447
<PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="9.0.14" />
4548
<PackageVersion Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="9.0.14" />
4649
<PackageVersion Include="Microsoft.Extensions.Diagnostics.Testing" Version="9.10.0" />
4750
</ItemGroup>
4851

4952
<ItemGroup Condition=" '$(TargetFramework)' == 'net8.0' ">
5053
<PackageVersion Include="Basic.Reference.Assemblies.Net80" Version="1.8.6" />
54+
<PackageVersion Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="8.0.0" />
5155
<PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="8.0.1" />
5256
<PackageVersion Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="8.0.2" />
5357
<PackageVersion Include="Microsoft.Extensions.Diagnostics.Testing" Version="8.10.0" />

‎src/Immediate.Handlers.Analyzers/AnalyzerOptionsExtensions.cs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright @meziantou aka Gérald Barré
2+
// Copied from https://github.com/meziantou/Meziantou.Analyzer/blob/main/src/Meziantou.Analyzer/Configurations/AnalyzerOptionsExtensions.cs
3+
4+
using System.Diagnostics.CodeAnalysis;
5+
using Microsoft.CodeAnalysis;
6+
using Microsoft.CodeAnalysis.Diagnostics;
7+
8+
namespace Immediate.Handlers.Analyzers;
9+
10+
internal static class AnalyzerOptionsExtensions
11+
{
12+
public static bool GetConfigurationValue(this AnalyzerOptions options, ISymbol symbol, string key, bool defaultValue)
13+
{
14+
foreach (var location in symbol.Locations)
15+
{
16+
var syntaxTree = location.SourceTree;
17+
if (syntaxTree is not null && options.TryGetConfigurationValue(syntaxTree, key, out var str))
18+
return ChangeType(str, defaultValue);
19+
}
20+
21+
return defaultValue;
22+
}
23+
24+
public static bool TryGetConfigurationValue(this AnalyzerOptions options, SyntaxTree syntaxTree, string key, [NotNullWhen(true)] out string? value)
25+
{
26+
var configuration = options.AnalyzerConfigOptionsProvider.GetOptions(syntaxTree);
27+
return configuration.TryGetValue(key, out value);
28+
}
29+
30+
private static bool ChangeType(string value, bool defaultValue)
31+
{
32+
if (value is not null && bool.TryParse(value, out var result))
33+
return result;
34+
35+
return defaultValue;
36+
}
37+
}

‎src/Immediate.Handlers.Analyzers/HandlerClassAnalyzer.cs‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ namespace Immediate.Handlers.Analyzers;
88
[DiagnosticAnalyzer(LanguageNames.CSharp)]
99
public sealed class HandlerClassAnalyzer : DiagnosticAnalyzer
1010
{
11+
public const string DiagnosticOptionIhr0019EnableNoDependencies = "dotnet_diagnostic.IHR0019.enable_when_handler_has_no_dependencies";
12+
1113
public static readonly DiagnosticDescriptor HandlerMethodMustExist =
1214
new(
1315
id: DiagnosticIds.IHR0001HandlerMethodMustExist,
@@ -322,13 +324,20 @@ or INamedTypeSymbol
322324

323325
private static void AnalyzeStaticHandler(SymbolAnalysisContext context, INamedTypeSymbol containerSymbol, IMethodSymbol method)
324326
{
325-
context.ReportDiagnostic(
326-
Diagnostic.Create(
327-
StaticHandlerCouldBeSealed,
328-
containerSymbol.Locations[0],
329-
containerSymbol.Name
330-
)
331-
);
327+
if (
328+
method.Parameters.Length > 2
329+
|| (method.Parameters.Length > 1 && !method.Parameters[^1].Type.IsCancellationToken)
330+
|| context.Options.GetConfigurationValue(method, DiagnosticOptionIhr0019EnableNoDependencies, defaultValue: false)
331+
)
332+
{
333+
context.ReportDiagnostic(
334+
Diagnostic.Create(
335+
StaticHandlerCouldBeSealed,
336+
containerSymbol.Locations[0],
337+
containerSymbol.Name
338+
)
339+
);
340+
}
332341

333342
if (method.Parameters is [] or [{ Type.IsCancellationToken: true }])
334343
{

‎src/Immediate.Handlers.Analyzers/MarkHandleMethodAsStaticSuppressor.cs‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
using System.Collections.Immutable;
2+
using Microsoft.CodeAnalysis;
3+
using Microsoft.CodeAnalysis.CSharp.Syntax;
4+
using Microsoft.CodeAnalysis.Diagnostics;
5+
6+
namespace Immediate.Handlers.Analyzers;
7+
8+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
9+
public sealed class MarkHandleMethodAsStaticSuppressor : DiagnosticSuppressor
10+
{
11+
public override ImmutableArray<SuppressionDescriptor> SupportedSuppressions =>
12+
ImmutableArray.Create<SuppressionDescriptor>([
13+
new(
14+
id: "MarkMembersAsStaticSuppression",
15+
suppressedDiagnosticId: "CA1822",
16+
justification: "Handler methods should not be triggered to be static."
17+
),
18+
]);
19+
20+
public override void ReportSuppressions(SuppressionAnalysisContext context)
21+
{
22+
foreach (var diagnostic in context.ReportedDiagnostics)
23+
{
24+
context.CancellationToken.ThrowIfCancellationRequested();
25+
26+
if (diagnostic.Location.SourceTree?.GetRoot().FindNode(diagnostic.Location.SourceSpan) is not MethodDeclarationSyntax node)
27+
continue;
28+
29+
var model = context.GetSemanticModel(diagnostic.Location.SourceTree);
30+
var method = (IMethodSymbol?)model.GetDeclaredSymbol(node, context.CancellationToken);
31+
32+
if (
33+
method is not
34+
{
35+
MethodKind: MethodKind.Ordinary,
36+
Name: "Handle" or "HandleAsync",
37+
ReturnType.IsValidHandlerReturn: true,
38+
}
39+
|| !method.ContainingSymbol.GetAttributes().Any(ad => ad.AttributeClass.IsHandlerAttribute)
40+
)
41+
{
42+
continue;
43+
}
44+
45+
if (!context.Options.GetConfigurationValue(
46+
method,
47+
HandlerClassAnalyzer.DiagnosticOptionIhr0019EnableNoDependencies,
48+
defaultValue: false
49+
))
50+
{
51+
continue;
52+
}
53+
54+
context.ReportSuppression(
55+
Suppression.Create(
56+
SupportedSuppressions[0],
57+
diagnostic
58+
)
59+
);
60+
}
61+
}
62+
}

‎src/Immediate.Handlers.CodeFixes/StaticToSealedHandlerCodeFixProvider.cs‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ CancellationToken token
8080
{
8181
var methodParameters = methodDeclarationSyntax.ParameterList.Parameters;
8282

83-
var isLastParamCancellationToken = model.IsCancellationToken(methodParameters[^1].Type, token);
83+
var isLastParamCancellationToken = methodParameters.Count > 0 && model.IsCancellationToken(methodParameters[^1].Type, token);
8484

8585
var classParameters = methodParameters
8686
.Skip(1)
@@ -104,12 +104,7 @@ CancellationToken token
104104
.ReplaceNode(methodDeclarationSyntax, newMethodDeclarationSyntax)
105105
.WithModifiers(
106106
classDeclarationSyntax.Modifiers
107-
.RemoveStaticModifier()
108-
.Insert(
109-
// valid case will have `partial` as final element; insert `sealed` before `partial`
110-
classDeclarationSyntax.Modifiers.Count - 2,
111-
Token(SyntaxKind.SealedKeyword).WithTrailingTrivia(ElasticSpace)
112-
)
107+
.CorrectClassModifiers()
113108
);
114109

115110
if (classParameters.Count > 0)
@@ -140,4 +135,20 @@ int count
140135
public static SyntaxTokenList RemoveStaticModifier(
141136
this SyntaxTokenList list
142137
) => new(list.Where(static token => !token.IsKind(SyntaxKind.StaticKeyword)));
138+
139+
public static SyntaxTokenList CorrectClassModifiers(
140+
this SyntaxTokenList list
141+
)
142+
{
143+
list = list
144+
.Replace(
145+
list.First(static token => token.IsKind(SyntaxKind.StaticKeyword)),
146+
Token(SyntaxKind.SealedKeyword).WithTrailingTrivia(ElasticSpace)
147+
);
148+
149+
if (!list.Any(static token => token.IsKind(SyntaxKind.PartialKeyword)))
150+
list = list.Add(Token(SyntaxKind.PartialKeyword).WithTrailingTrivia(ElasticSpace));
151+
152+
return list;
153+
}
143154
}

‎tests/Immediate.Handlers.Tests/AnalyzerTests/AnalyzerTestHelpers.cs‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1+
using System.Collections.Immutable;
2+
using System.ComponentModel;
13
using System.Diagnostics.CodeAnalysis;
24
using Immediate.Handlers.Generators;
5+
using Microsoft.CodeAnalysis;
36
using Microsoft.CodeAnalysis.CSharp.Testing;
47
using Microsoft.CodeAnalysis.Diagnostics;
58
using Microsoft.CodeAnalysis.Testing;
@@ -35,4 +38,118 @@ private sealed class ImmediateHandlersGeneratorAnalyzerTest<TAnalyzer> : CSharpA
3538
protected override IEnumerable<Type> GetSourceGenerators() =>
3639
[typeof(ImmediateHandlersGenerator)];
3740
}
41+
42+
public sealed class CSharpSuppressorTest<TSuppressor, TVerifier> : CSharpAnalyzerTest<TSuppressor, TVerifier>
43+
where TSuppressor : DiagnosticSuppressor, new()
44+
where TVerifier : IVerifier, new()
45+
{
46+
private readonly List<DiagnosticAnalyzer> _analyzers = [];
47+
48+
protected override IEnumerable<DiagnosticAnalyzer> GetDiagnosticAnalyzers() =>
49+
base.GetDiagnosticAnalyzers().Concat(_analyzers);
50+
51+
public CSharpSuppressorTest<TSuppressor, TVerifier> WithAnalyzer<TAnalyzer>(bool enableDiagnostics = false)
52+
where TAnalyzer : DiagnosticAnalyzer, new()
53+
{
54+
var analyzer = new TAnalyzer();
55+
_analyzers.Add(analyzer);
56+
57+
if (enableDiagnostics)
58+
{
59+
var diagnosticOptions = analyzer.SupportedDiagnostics
60+
.ToImmutableDictionary(
61+
descriptor => descriptor.Id,
62+
descriptor => descriptor.DefaultSeverity.ToReportDiagnostic()
63+
);
64+
65+
SolutionTransforms.Clear();
66+
SolutionTransforms.Add(EnableDiagnostics(diagnosticOptions));
67+
}
68+
69+
return this;
70+
}
71+
72+
public CSharpSuppressorTest<TSuppressor, TVerifier> WithSpecificDiagnostics(
73+
params DiagnosticResult[] diagnostics
74+
)
75+
{
76+
var diagnosticOptions = diagnostics
77+
.ToImmutableDictionary(
78+
descriptor => descriptor.Id,
79+
descriptor => descriptor.Severity.ToReportDiagnostic()
80+
);
81+
82+
SolutionTransforms.Clear();
83+
SolutionTransforms.Add(EnableDiagnostics(diagnosticOptions));
84+
return this;
85+
}
86+
87+
private static Func<Solution, ProjectId, Solution> EnableDiagnostics(
88+
ImmutableDictionary<string, ReportDiagnostic> diagnostics
89+
) =>
90+
(solution, id) =>
91+
{
92+
var options = solution.GetProject(id)?.CompilationOptions
93+
?? throw new InvalidOperationException("Compilation options missing.");
94+
95+
return solution
96+
.WithProjectCompilationOptions(
97+
id,
98+
options
99+
.WithSpecificDiagnosticOptions(diagnostics)
100+
);
101+
};
102+
103+
public CSharpSuppressorTest<TSuppressor, TVerifier> WithExpectedDiagnosticsResults(
104+
params DiagnosticResult[] diagnostics
105+
)
106+
{
107+
ExpectedDiagnostics.AddRange(diagnostics);
108+
return this;
109+
}
110+
111+
public CSharpSuppressorTest<TSuppressor, TVerifier> WithEditorConfig(
112+
string content
113+
)
114+
{
115+
TestState.AnalyzerConfigFiles.Add(("/.editorconfig", content));
116+
return this;
117+
}
118+
}
119+
120+
public static CSharpSuppressorTest<TSuppressor, DefaultVerifier> CreateSuppressorTest<TSuppressor>(
121+
[StringSyntax("c#-test")] string inputSource
122+
)
123+
where TSuppressor : DiagnosticSuppressor, new()
124+
{
125+
var test = new CSharpSuppressorTest<TSuppressor, DefaultVerifier>
126+
{
127+
TestCode = inputSource,
128+
ReferenceAssemblies = Utility.ReferenceAssemblies,
129+
CompilerDiagnostics = CompilerDiagnostics.Warnings,
130+
DisabledDiagnostics =
131+
{
132+
"CS1591",
133+
"CS8767",
134+
},
135+
};
136+
137+
test.TestState.AdditionalReferences
138+
.AddRange(DriverReferenceAssemblies.Normal.GetAdditionalReferences());
139+
140+
return test;
141+
}
142+
}
143+
144+
file static class DiagnosticSeverityExtensions
145+
{
146+
public static ReportDiagnostic ToReportDiagnostic(this DiagnosticSeverity severity)
147+
=> severity switch
148+
{
149+
DiagnosticSeverity.Hidden => ReportDiagnostic.Hidden,
150+
DiagnosticSeverity.Info => ReportDiagnostic.Info,
151+
DiagnosticSeverity.Warning => ReportDiagnostic.Warn,
152+
DiagnosticSeverity.Error => ReportDiagnostic.Error,
153+
_ => throw new InvalidEnumArgumentException(nameof(severity), (int)severity, typeof(DiagnosticSeverity)),
154+
};
38155
}

‎tests/Immediate.Handlers.Tests/AnalyzerTests/HandlerClassAnalyzerTests/Tests.HandleMethodDoesNotReturnTask.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ await AnalyzerTestHelpers.CreateAnalyzerTest<HandlerClassAnalyzer>(
1818
using Immediate.Handlers.Shared;
1919
2020
[Handler]
21-
public static partial class {|IHR0019:GetUsersQuery|}
21+
public static partial class GetUsersQuery
2222
{
2323
public record Query;
2424

‎tests/Immediate.Handlers.Tests/AnalyzerTests/HandlerClassAnalyzerTests/Tests.HandleMethodIsCorrectWithIntReturn.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ await AnalyzerTestHelpers.CreateAnalyzerTest<HandlerClassAnalyzer>(
1818
using Immediate.Handlers.Shared;
1919
2020
[Handler]
21-
public static partial class {|IHR0019:GetUsersQuery|}
21+
public static partial class GetUsersQuery
2222
{
2323
public record Query;
2424

‎tests/Immediate.Handlers.Tests/AnalyzerTests/HandlerClassAnalyzerTests/Tests.HandleMethodIsCorrectWithVoidReturn.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ await AnalyzerTestHelpers.CreateAnalyzerTest<HandlerClassAnalyzer>(
1818
using Immediate.Handlers.Shared;
1919
2020
[Handler]
21-
public static partial class {|IHR0019:GetUsersQuery|}
21+
public static partial class GetUsersQuery
2222
{
2323
public record Query;
2424

‎tests/Immediate.Handlers.Tests/AnalyzerTests/HandlerClassAnalyzerTests/Tests.HandleMethodIsNotPrivate.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ await AnalyzerTestHelpers.CreateAnalyzerTest<HandlerClassAnalyzer>(
1818
using Immediate.Handlers.Shared;
1919
2020
[Handler]
21-
public static partial class {|IHR0019:GetUsersQuery|}
21+
public static partial class GetUsersQuery
2222
{
2323
public record Query;
2424

0 commit comments

Comments
 (0)