Skip to content

Commit

Permalink
Add FL0019 and FL0020.
Browse files Browse the repository at this point in the history
  • Loading branch information
StephenCleary committed Feb 13, 2024
1 parent adc0cf3 commit 2588996
Show file tree
Hide file tree
Showing 4 changed files with 170 additions and 2 deletions.
4 changes: 2 additions & 2 deletions Directory.Build.props
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<Project>

<PropertyGroup>
<VersionPrefix>1.4.0</VersionPrefix>
<LangVersion>10.0</LangVersion>
<VersionPrefix>1.5.0</VersionPrefix>
<LangVersion>11.0</LangVersion>
<Nullable>enable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
Expand Down
5 changes: 5 additions & 0 deletions ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
57 changes: 57 additions & 0 deletions src/Faithlife.Analyzers/LocalFunctionEventHandler.cs
Original file line number Diff line number Diff line change
@@ -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<DiagnosticDescriptor> 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}"
);
}
106 changes: 106 additions & 0 deletions tests/Faithlife.Analyzers.Tests/LocalFunctionEventHandlerTests.cs
Original file line number Diff line number Diff line change
@@ -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();
}

0 comments on commit 2588996

Please sign in to comment.