Add ECS0008 use the null conditional operator for event invocations

docs/RULE-REFERENCE-TEMPLATE.md

@@ -10,6 +10,23 @@ This rule is described in detail in [Effective C#: 50 Specific Ways to Improve y
 
 ## When to suppress warnings
 
+### Suppress a warning
+
+If you just want to suppress a single violation, add preprocessor directives to your source file to disable and then re-enable the rule.
+
+```csharp
+#pragma warning disable RULEID
+// The code that's violating the rule
+#pragma warning restore RULEID
+```
+
+To disable the rule for a file, folder, or project, set its severity to none in the [configuration file](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-files).
+
+```ini
+[*.cs]
+dotnet_diagnostic.RULEID.severity = none
+```
+
 ## Example of a violation
 
 ### Description

docs/rules/ECS0008.md

@@ -0,0 +1,81 @@
+# ECS0008: Use the Null-Conditional Operator for Event Invocations
+
+This rule is described in detail in [Effective C#: 50 Specific Ways to Improve your C#](https://www.oreilly.com/library/view/effective-c-50/9780134579290/).
+
+## Cause
+
+This rule is triggered when an event handler is invoked without using the null-conditional operator (`?.`), which can potentially lead to a `NullReferenceException` if there are no subscribers to the event.
+
+
+## Rule description
+
+When invoking events in C#, it is recommended to use the null-conditional operator to ensure that the event is only invoked if it has subscribers. This prevents potential runtime errors and makes the code more robust. This rule checks for patterns where the event handler is invoked directly or after a null check and suggests replacing them with the null-conditional operator.
+
+## How to fix violations
+
+Replace any `if` statement that checks if an event handler is `null` and then invokes the handler with the null-conditional operator.
+
+If the code uses an intermediate variable (e.g., `var handler = Updated;`), remove the variable and replace the `if` statement with the null-conditional operator directly on the event.
+
+## When to suppress warnings
+
+You can suppress warnings from this rule if you're confident that the event will always have subscribers at the time of invocation, or if you have special logic that must be executed before the event is invoked.
+
+## Example of a violation
+
+### Description
+
+Directly invoking the event handler or checking for null before invoking without using the null-conditional operator.
+
+### Code
+
+```csharp
+public class EventSource
+{
+    private EventHandler<int> Updated;
+    private int counter;
+
+    public void RaiseUpdates()
+    {
+        counter++;
+        if (Updated != null)
+            Updated(this, counter);
+    }
+}
+
+public class EventSource
+{
+    private EventHandler<int> Updated;
+    private int counter;
+
+    public void RaiseUpdates()
+    {
+        counter++;
+        var handler = Updated;
+        if (handler != null)
+            handler(this, counter);
+    }
+}
+```
+
+## Example of how to fix
+
+### Description
+
+Replace the direct invocation or the null check with the null-conditional operator.
+
+### Code
+
+```csharp
+public class EventSource
+{
+    private EventHandler<int> Updated;
+    private int counter;
+
+    public void RaiseUpdates()
+    {
+        counter++;
+        Updated?.Invoke(this, counter);
+    }
+}
+```

src/EffectiveCSharp.Analyzers/AnalyzerReleases.Unshipped.md

@@ -4,10 +4,11 @@
 
 Rule ID | Category | Severity | Notes
 --------|----------|----------|-------
-ECS0001 | Style | Info | PreferImplicitlyTypedLocalVariablesAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/e7c151c721c3039011356d6012838f46e4b60a21/docs/ECS0001.md)
-ECS0002 | Maintainability | Info | PreferReadonlyOverConstAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/10c2d53afd688efe5a59097f76cb4edf33f6a474/docs/ECS0002.md)
-ECS0004 | Style | Info | ReplaceStringFormatAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers5da647e447fad4eb0a9e3db287e1d16cce316114/docs/ECS0004.md)
-ECS0006 | Refactoring | Info | AvoidStringlyTypedApisAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers6213cba8473dac61d6132e205550884eae1c94bf/docs/ECS0006.md)
-ECS0007 | Design | Info | ExpressCallbacksWithDelegatesAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/dc194bbde030e9c40d8d9cdb1e0b5ff8919fe5a8/docs/ECS0007.md)
-ECS0009 | Performance | Info | MinimizeBoxingUnboxingAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/6213cba8473dac61d6132e205550884eae1c94bf/docs/ECS0009.md)
-ECS1000 | Performance | Info | SpanAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/d00a4cc9f61e7d5b392894aad859e46c43a5611c/docs/ECS1000.md)
+ECS0001 | Style | Info | PreferImplicitlyTypedLocalVariablesAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/e7c151c721c3039011356d6012838f46e4b60a21/docs/rules/ECS0001.md)
+ECS0002 | Maintainability | Info | PreferReadonlyOverConstAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/10c2d53afd688efe5a59097f76cb4edf33f6a474/docs/rules/ECS0002.md)
+ECS0004 | Style | Info | ReplaceStringFormatAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers5da647e447fad4eb0a9e3db287e1d16cce316114/docs/rules/ECS0004.md)
+ECS0006 | Refactoring | Info | AvoidStringlyTypedApisAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers6213cba8473dac61d6132e205550884eae1c94bf/docs/rules/ECS0006.md)
+ECS0007 | Design | Info | ExpressCallbacksWithDelegatesAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/dc194bbde030e9c40d8d9cdb1e0b5ff8919fe5a8/docs/rules/ECS0007.md)
+ECS0008 | Usage | Info | EventInvocationAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/cc7d91eb81f6781851c09732db1268be7dab402b/docs/rules/ECS0008.md)
+ECS0009 | Performance | Info | MinimizeBoxingUnboxingAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/6213cba8473dac61d6132e205550884eae1c94bf/docs/rules/ECS0009.md)
+ECS1000 | Performance | Info | SpanAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/d00a4cc9f61e7d5b392894aad859e46c43a5611c/docs/rules/ECS1000.md)

src/EffectiveCSharp.Analyzers/Common/Categories.cs

@@ -5,4 +5,5 @@ internal static class Categories
     internal static readonly string Refactoring = nameof(Refactoring);
     internal static readonly string Performance = nameof(Performance);
     internal static readonly string Style = nameof(Style);
+    internal static readonly string Usage = nameof(Usage);
 }

src/EffectiveCSharp.Analyzers/DiagnosticIds.cs → src/EffectiveCSharp.Analyzers/Common/DiagnosticIds.cs

@@ -10,6 +10,7 @@ internal static class DiagnosticIds
     internal const string ReplaceStringFormatWithInterpolatedString = "ECS0004";
     internal const string AvoidStringlyTypedApis = "ECS0006";
     internal const string ExpressCallbacksWithDelegates = "ECS0007";
+    internal const string UseNullConditionalOperatorForEventInvocations = "ECS0008";
     internal const string MinimizeBoxingUnboxing = "ECS0009";
     internal const string BeAwareOfValueTypeCopyInReferenceTypes = "ECS0009";
     internal const string UseSpanInstead = "ECS1000";

src/EffectiveCSharp.Analyzers/EventInvocationAnalyzer.cs

@@ -0,0 +1,120 @@
+namespace EffectiveCSharp.Analyzers;
+
+/// <summary>
+/// A <see cref="DiagnosticAnalyzer"/> for Effective C# Item #8 - Use the Null Conditional Operator for Event Invocations.
+/// </summary>
+/// <seealso cref="Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer" />
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public class EventInvocationAnalyzer : DiagnosticAnalyzer
+{
+    private static readonly string Title = "Use the Null Conditional Operator for Event Invocations";
+    private static readonly string MessageFormat = "Use the null-conditional operator to invoke the event '{0}'";
+    private static readonly string Description = "Event invocation should use the null-conditional operator to avoid race conditions and improve readability.";
+
+    private static readonly DiagnosticDescriptor Rule = new(
+        DiagnosticIds.UseNullConditionalOperatorForEventInvocations,
+        Title,
+        MessageFormat,
+        Categories.Usage,
+        DiagnosticSeverity.Info,
+        isEnabledByDefault: true,
+        description: Description,
+        helpLinkUri: $"https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/{ThisAssembly.GitCommitId}/docs/rules/{DiagnosticIds.UseNullConditionalOperatorForEventInvocations}.md");
+
+    /// <inheritdoc />
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+    /// <inheritdoc />
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+        context.EnableConcurrentExecution();
+        context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.IfStatement, SyntaxKind.InvocationExpression);
+    }
+
+    private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
+    {
+        switch (context.Node)
+        {
+            case IfStatementSyntax ifStatement:
+                AnalyzeIfStatement(context, ifStatement);
+                break;
+            case InvocationExpressionSyntax invocationExpression:
+                AnalyzeInvocationExpression(context, invocationExpression);
+                break;
+            default:
+                Debug.Fail("Unknown node type");
+                break;
+        }
+    }
+
+    private static void AnalyzeInvocationExpression(SyntaxNodeAnalysisContext context, InvocationExpressionSyntax invocationExpression)
+    {
+        // Check if the invocation is on an event handler directly
+        if (invocationExpression.Expression is not IdentifierNameSyntax identifierName)
+        {
+            return;
+        }
+
+        ISymbol? symbol = context.SemanticModel.GetSymbolInfo(identifierName, context.CancellationToken).Symbol;
+
+        if (symbol == null || !IsEventSymbol(symbol))
+        {
+            return;
+        }
+
+        // Check if the invocation is not within an if statement or null-conditional access
+        SyntaxNode? parent = invocationExpression.Parent;
+        while (parent != null)
+        {
+            if (parent is IfStatementSyntax { Condition: BinaryExpressionSyntax binaryExpression }
+                && binaryExpression.IsKind(SyntaxKind.NotEqualsExpression)
+                && binaryExpression.Left is IdentifierNameSyntax binaryIdentifier
+                && string.Equals(binaryIdentifier.Identifier.Text, identifierName.Identifier.Text, StringComparison.Ordinal)
+                && binaryExpression.Right.IsKind(SyntaxKind.NullLiteralExpression))
+            {
+                return; // Safe pattern, exit
+            }
+
+            if (parent is ConditionalAccessExpressionSyntax)
+            {
+                return; // Safe pattern, exit
+            }
+
+            parent = parent.Parent;
+        }
+
+        // Report a diagnostic for direct event handler invocation
+        Diagnostic diagnostic = invocationExpression.GetLocation().CreateDiagnostic(Rule, identifierName.Identifier.Text);
+        context.ReportDiagnostic(diagnostic);
+    }
+
+    private static bool IsEventSymbol(ISymbol symbol)
+    {
+        return symbol is IFieldSymbol fieldSymbol && fieldSymbol.Type.Name.StartsWith("EventHandler", StringComparison.Ordinal);
+    }
+
+#pragma warning disable S125 // Sections of code should not be commented out
+    private static void AnalyzeIfStatement(SyntaxNodeAnalysisContext context, IfStatementSyntax ifStatement)
+    {
+        // Check for patterns like: if (handler != null) handler(args);
+        if (ifStatement.Condition is not BinaryExpressionSyntax binaryExpression
+            || !binaryExpression.IsKind(SyntaxKind.NotEqualsExpression)
+            || binaryExpression.Left is not IdentifierNameSyntax identifierName
+            || !binaryExpression.Right.IsKind(SyntaxKind.NullLiteralExpression))
+        {
+            return;
+        }
+
+        StatementSyntax statement = ifStatement.Statement;
+        if (statement is not ExpressionStatementSyntax { Expression: InvocationExpressionSyntax { Expression: IdentifierNameSyntax invocationIdentifier } }
+            || !string.Equals(invocationIdentifier.Identifier.Text, identifierName.Identifier.Text, StringComparison.Ordinal))
+        {
+            return;
+        }
+
+        Diagnostic diagnostic = ifStatement.GetLocation().CreateDiagnostic(Rule, identifierName.Identifier.Text);
+        context.ReportDiagnostic(diagnostic);
+    }
+#pragma warning restore S125 // Sections of code should not be commented out
+}