From 258899659d1e12a6328340ea77f9b10ad3767f6d Mon Sep 17 00:00:00 2001 From: StephenCleary Date: Tue, 13 Feb 2024 13:14:01 -0500 Subject: [PATCH] Add FL0019 and FL0020. --- Directory.Build.props | 4 +- ReleaseNotes.md | 5 + .../LocalFunctionEventHandler.cs | 57 ++++++++++ .../LocalFunctionEventHandlerTests.cs | 106 ++++++++++++++++++ 4 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 src/Faithlife.Analyzers/LocalFunctionEventHandler.cs create mode 100644 tests/Faithlife.Analyzers.Tests/LocalFunctionEventHandlerTests.cs diff --git a/Directory.Build.props b/Directory.Build.props index 9745ad5..ec2d9aa 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -1,8 +1,8 @@ - 1.4.0 - 10.0 + 1.5.0 + 11.0 enable enable true diff --git a/ReleaseNotes.md b/ReleaseNotes.md index a6c96d1..2acc228 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -9,6 +9,11 @@ Prefix the description of the change with `[major]`, `[minor]`, or `[patch]` in New analyzers are considered "minor" changes (even though adding a new analyzer is likely to generate warnings or errors for existing code when the package is upgraded). +## 1.5.0 + +* Add FL0019: Local functions used as event handlers (unless they are static). +* Add FL0020: Lambda expressions used as event handlers. + ## 1.4.0 * Add FL0017: Do not switch on a constant value. diff --git a/src/Faithlife.Analyzers/LocalFunctionEventHandler.cs b/src/Faithlife.Analyzers/LocalFunctionEventHandler.cs new file mode 100644 index 0000000..62fdd7c --- /dev/null +++ b/src/Faithlife.Analyzers/LocalFunctionEventHandler.cs @@ -0,0 +1,57 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Faithlife.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class LocalFunctionEventHandler : DiagnosticAnalyzer +{ + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.SubtractAssignmentExpression); + } + + private void Analyze(SyntaxNodeAnalysisContext context) + { + var assignment = (AssignmentExpressionSyntax) context.Node; + if (context.SemanticModel.GetSymbolInfo(assignment.Left).Symbol is not IEventSymbol) + return; + if (context.SemanticModel.GetSymbolInfo(assignment.Right).Symbol is not IMethodSymbol methodGroup) + return; + if (methodGroup is { MethodKind: MethodKind.LocalFunction, IsStatic: false }) + context.ReportDiagnostic(Diagnostic.Create(s_localFunctionRule, assignment.GetLocation())); + else if (methodGroup.MethodKind == MethodKind.AnonymousFunction) + context.ReportDiagnostic(Diagnostic.Create(s_lambdaRule, assignment.GetLocation())); + } + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(s_localFunctionRule, s_lambdaRule); + + public const string LocalFunctionDiagnosticId = "FL0019"; + public const string LambdaDiagnosticId = "FL0020"; + + private static readonly DiagnosticDescriptor s_localFunctionRule = new( + id: LocalFunctionDiagnosticId, + title: "Local Functions as Event Handlers", + messageFormat: "Local functions should probably not be used as event handlers (unless they are static).", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: $"https://github.com/Faithlife/FaithlifeAnalyzers/wiki/{LocalFunctionDiagnosticId}" + ); + + private static readonly DiagnosticDescriptor s_lambdaRule = new( + id: LambdaDiagnosticId, + title: "Lambda Expressions as Event Handlers", + messageFormat: "Lambda expressions may not be used as event handlers.", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + helpLinkUri: $"https://github.com/Faithlife/FaithlifeAnalyzers/wiki/{LambdaDiagnosticId}" + ); +} diff --git a/tests/Faithlife.Analyzers.Tests/LocalFunctionEventHandlerTests.cs b/tests/Faithlife.Analyzers.Tests/LocalFunctionEventHandlerTests.cs new file mode 100644 index 0000000..3683cf6 --- /dev/null +++ b/tests/Faithlife.Analyzers.Tests/LocalFunctionEventHandlerTests.cs @@ -0,0 +1,106 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using NUnit.Framework; + +namespace Faithlife.Analyzers.Tests; + +[TestFixture] +public class LocalFunctionEventHandlerTests : CodeFixVerifier +{ + [Test] + public void LocalFunction() + { + const string program = + """ + using System; + class Test + { + public event EventHandler OnFrob; + public void Hook() + { + OnFrob += Local; + OnFrob -= Local; + void Local(object a, EventArgs b) { } + OnFrob?.Invoke(this, EventArgs.Empty); + } + } + """; + VerifyCSharpDiagnostic(program, new DiagnosticResult + { + Id = LocalFunctionEventHandler.LocalFunctionDiagnosticId, + Severity = DiagnosticSeverity.Warning, + Message = "Local functions should probably not be used as event handlers (unless they are static).", + Locations = new[] { new DiagnosticResultLocation("Test0.cs", 8, 3) }, + }); + } + + [Test] + public void StaticLocalFunction() + { + const string program = + """ + using System; + class Test + { + public event EventHandler OnFrob; + public void Hook() + { + OnFrob += Local; + OnFrob -= Local; + static void Local(object a, EventArgs b) { } + OnFrob?.Invoke(this, EventArgs.Empty); + } + } + """; + VerifyCSharpDiagnostic(program); + } + + [Test] + public void Lambda() + { + const string program = + """ + using System; + class Test + { + public event EventHandler OnFrob; + public void Hook() + { + OnFrob += (_, __) => { }; + OnFrob -= (_, __) => { }; + OnFrob?.Invoke(this, EventArgs.Empty); + } + } + """; + VerifyCSharpDiagnostic(program, new DiagnosticResult + { + Id = LocalFunctionEventHandler.LambdaDiagnosticId, + Severity = DiagnosticSeverity.Error, + Message = "Lambda expressions may not be used as event handlers.", + Locations = new[] { new DiagnosticResultLocation("Test0.cs", 8, 3) }, + }); + } + + [Test] + public void NonStaticInstanceMethod() + { + const string program = + """ + using System; + class Test + { + public event EventHandler OnFrob; + public void Hook() + { + OnFrob += Frobbed; + OnFrob -= Frobbed; + OnFrob?.Invoke(this, EventArgs.Empty); + } + private void Frobbed(object a, EventArgs b) { } + } + """; + VerifyCSharpDiagnostic(program); + } + + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() => new LocalFunctionEventHandler(); +}