From 513d7a4858d6c71bc107246acffc0882efdabb36 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 22:33:55 +0000 Subject: [PATCH] Extract ErrorCodeFacts helper and flag discarded ToException() result (b) Factor the ErrorCode.Create("literal") recognition and the Optional.ConstantValue handling shared by FCE001, FCE002, FCE004 and FCE005 into a single ErrorCodeFacts helper. FCE004/FCE005 previously relied on Optional.Value defaulting to null when HasValue is false; the semantics are now centralized and made explicit, and each analyzer no longer repeats the static-Create / arity / constant checks. Behavior is unchanged for all four rules. (a) FCE016 now also reports `_ = error.ToException();`. The explicit discard wraps the invocation in an ISimpleAssignmentOperation whose target is an IDiscardOperation, so the previous "bare expression statement" guard missed it even though the result is thrown away just the same. ToException() is a pure builder, so an explicit discard is as pointless as a standalone call. Added a unit test and kept the FCE016 documentation (EN + FR) in sync. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019NBsUT3XFzH86c8L9iSDmn --- .../Fce016UnusedToExceptionResultTests.cs | 18 ++++++ .../DuplicateErrorCodeAnalyzer.cs | 18 ++---- .../EmptyErrorCodeAnalyzer.cs | 17 ++--- FirstClassErrors.Analyzers/ErrorCodeFacts.cs | 63 +++++++++++++++++++ .../InvalidErrorCodeFormatAnalyzer.cs | 16 ++--- .../TooGenericErrorCodeAnalyzer.cs | 14 ++--- .../UnusedToExceptionResultAnalyzer.cs | 13 +++- doc/analyzers/FCE016.en.md | 8 ++- doc/analyzers/FCE016.fr.md | 8 ++- doc/analyzers/README.fr.md | 2 +- doc/analyzers/README.md | 2 +- 11 files changed, 126 insertions(+), 53 deletions(-) create mode 100644 FirstClassErrors.Analyzers/ErrorCodeFacts.cs diff --git a/FirstClassErrors.Analyzers.UnitTests/Fce016UnusedToExceptionResultTests.cs b/FirstClassErrors.Analyzers.UnitTests/Fce016UnusedToExceptionResultTests.cs index 679a706..131462a 100644 --- a/FirstClassErrors.Analyzers.UnitTests/Fce016UnusedToExceptionResultTests.cs +++ b/FirstClassErrors.Analyzers.UnitTests/Fce016UnusedToExceptionResultTests.cs @@ -28,6 +28,24 @@ public static void M(DomainError error) { Check.That(diagnostics[0].Id).IsEqualTo("FCE016"); } + [Fact] + public async Task Reports_when_result_is_assigned_to_a_discard() { + const string source = """ + using FirstClassErrors; + + public static class Sample { + public static void M(DomainError error) { + _ = error.ToException(); + } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new UnusedToExceptionResultAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(1); + Check.That(diagnostics[0].Id).IsEqualTo("FCE016"); + } + [Fact] public async Task Does_not_report_when_thrown() { const string source = """ diff --git a/FirstClassErrors.Analyzers/DuplicateErrorCodeAnalyzer.cs b/FirstClassErrors.Analyzers/DuplicateErrorCodeAnalyzer.cs index c344b00..bc33f4d 100644 --- a/FirstClassErrors.Analyzers/DuplicateErrorCodeAnalyzer.cs +++ b/FirstClassErrors.Analyzers/DuplicateErrorCodeAnalyzer.cs @@ -17,9 +17,6 @@ namespace FirstClassErrors.Analyzers; [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class DuplicateErrorCodeAnalyzer : DiagnosticAnalyzer { - private const string ErrorCodeMetadataName = "FirstClassErrors.ErrorCode"; - private const string CreateMethodName = "Create"; - /// public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Descriptors.DuplicateErrorCode); @@ -32,7 +29,7 @@ public override void Initialize(AnalysisContext context) { } private static void OnCompilationStart(CompilationStartAnalysisContext context) { - INamedTypeSymbol? errorCodeType = context.Compilation.GetTypeByMetadataName(ErrorCodeMetadataName); + INamedTypeSymbol? errorCodeType = context.Compilation.GetTypeByMetadataName(ErrorCodeFacts.ErrorCodeMetadataName); if (errorCodeType is null) { return; } // Per-compilation state; ErrorCode registers with ordinal comparison, so duplicates are case-sensitive. @@ -48,17 +45,12 @@ private static void Collect( ConcurrentDictionary> occurrences) { IInvocationOperation invocation = (IInvocationOperation)context.Operation; - IMethodSymbol method = invocation.TargetMethod; - - if (!method.IsStatic || method.Name != CreateMethodName) { return; } - if (!SymbolEqualityComparer.Default.Equals(method.ContainingType, errorCodeType)) { return; } - if (invocation.Arguments.Length != 1) { return; } - IOperation argument = invocation.Arguments[0].Value; - Optional constant = argument.ConstantValue; + IOperation? argument = ErrorCodeFacts.GetCreateArgument(invocation, errorCodeType); + if (argument is null) { return; } - if (!constant.HasValue) { return; } // non-literal → FCE003 - if (constant.Value is not string code || string.IsNullOrWhiteSpace(code)) { return; } // empty → FCE002 + // Non-literal codes are FCE003's concern, empty ones FCE002's; only real codes can collide. + if (!ErrorCodeFacts.TryGetNonEmptyLiteralCode(argument, out string code)) { return; } occurrences.GetOrAdd(code, _ => new ConcurrentBag()).Add(argument.Syntax.GetLocation()); } diff --git a/FirstClassErrors.Analyzers/EmptyErrorCodeAnalyzer.cs b/FirstClassErrors.Analyzers/EmptyErrorCodeAnalyzer.cs index d90d3e9..780ee10 100644 --- a/FirstClassErrors.Analyzers/EmptyErrorCodeAnalyzer.cs +++ b/FirstClassErrors.Analyzers/EmptyErrorCodeAnalyzer.cs @@ -14,9 +14,6 @@ namespace FirstClassErrors.Analyzers; [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class EmptyErrorCodeAnalyzer : DiagnosticAnalyzer { - private const string ErrorCodeMetadataName = "FirstClassErrors.ErrorCode"; - private const string CreateMethodName = "Create"; - /// public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Descriptors.EmptyErrorCode); @@ -29,7 +26,7 @@ public override void Initialize(AnalysisContext context) { } private static void OnCompilationStart(CompilationStartAnalysisContext context) { - INamedTypeSymbol? errorCodeType = context.Compilation.GetTypeByMetadataName(ErrorCodeMetadataName); + INamedTypeSymbol? errorCodeType = context.Compilation.GetTypeByMetadataName(ErrorCodeFacts.ErrorCodeMetadataName); if (errorCodeType is null) { return; } context.RegisterOperationAction(operationContext => Analyze(operationContext, errorCodeType), OperationKind.Invocation); @@ -37,18 +34,12 @@ private static void OnCompilationStart(CompilationStartAnalysisContext context) private static void Analyze(OperationAnalysisContext context, INamedTypeSymbol errorCodeType) { IInvocationOperation invocation = (IInvocationOperation)context.Operation; - IMethodSymbol method = invocation.TargetMethod; - - if (!method.IsStatic || method.Name != CreateMethodName) { return; } - if (!SymbolEqualityComparer.Default.Equals(method.ContainingType, errorCodeType)) { return; } - if (invocation.Arguments.Length != 1) { return; } - IOperation argument = invocation.Arguments[0].Value; - Optional constant = argument.ConstantValue; + IOperation? argument = ErrorCodeFacts.GetCreateArgument(invocation, errorCodeType); + if (argument is null) { return; } // A non-constant argument cannot be judged statically; FCE003 flags that separately. - if (!constant.HasValue) { return; } - if (constant.Value is string value && !string.IsNullOrWhiteSpace(value)) { return; } + if (!ErrorCodeFacts.IsBlankLiteralCode(argument)) { return; } context.ReportDiagnostic(Diagnostic.Create(Descriptors.EmptyErrorCode, argument.Syntax.GetLocation())); } diff --git a/FirstClassErrors.Analyzers/ErrorCodeFacts.cs b/FirstClassErrors.Analyzers/ErrorCodeFacts.cs new file mode 100644 index 0000000..eb1bca9 --- /dev/null +++ b/FirstClassErrors.Analyzers/ErrorCodeFacts.cs @@ -0,0 +1,63 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Operations; + +namespace FirstClassErrors.Analyzers; + +/// +/// Recognizes ErrorCode.Create("...") invocations and extracts the literal code, centralizing the +/// handling shared by the error-code analyzers (FCE001, FCE002, FCE004, FCE005). +/// +internal static class ErrorCodeFacts { + + /// + /// Metadata name of the ErrorCode type the analyzers key off of. + /// + public const string ErrorCodeMetadataName = "FirstClassErrors.ErrorCode"; + + private const string CreateMethodName = "Create"; + + /// + /// Returns the single argument operation of a static ErrorCode.Create(x) call, or null when the + /// invocation is not such a call. + /// + public static IOperation? GetCreateArgument(IInvocationOperation invocation, INamedTypeSymbol errorCodeType) { + IMethodSymbol method = invocation.TargetMethod; + + if (!method.IsStatic || method.Name != CreateMethodName) { return null; } + if (!SymbolEqualityComparer.Default.Equals(method.ContainingType, errorCodeType)) { return null; } + if (invocation.Arguments.Length != 1) { return null; } + + return invocation.Arguments[0].Value; + } + + /// + /// Extracts the code when is a non-empty literal string constant. A non-literal + /// argument ( is false) is FCE003's concern; a null, empty, or + /// whitespace literal is FCE002's concern. Both yield false here. + /// + public static bool TryGetNonEmptyLiteralCode(IOperation argument, out string code) { + Optional constant = argument.ConstantValue; + + if (constant.HasValue && constant.Value is string value && !string.IsNullOrWhiteSpace(value)) { + code = value; + return true; + } + + code = string.Empty; + return false; + } + + /// + /// True when is a literal constant whose string value is missing (null) or + /// blank. A non-literal argument ( is false) yields false; that + /// case is FCE003's concern. + /// + public static bool IsBlankLiteralCode(IOperation argument) { + Optional constant = argument.ConstantValue; + + if (!constant.HasValue) { return false; } + + return constant.Value is not string value || string.IsNullOrWhiteSpace(value); + } + +} diff --git a/FirstClassErrors.Analyzers/InvalidErrorCodeFormatAnalyzer.cs b/FirstClassErrors.Analyzers/InvalidErrorCodeFormatAnalyzer.cs index 9d33073..53995ed 100644 --- a/FirstClassErrors.Analyzers/InvalidErrorCodeFormatAnalyzer.cs +++ b/FirstClassErrors.Analyzers/InvalidErrorCodeFormatAnalyzer.cs @@ -14,9 +14,6 @@ namespace FirstClassErrors.Analyzers; [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class InvalidErrorCodeFormatAnalyzer : DiagnosticAnalyzer { - private const string ErrorCodeMetadataName = "FirstClassErrors.ErrorCode"; - private const string CreateMethodName = "Create"; - private static readonly Regex UpperSnakeCase = new("^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$", RegexOptions.CultureInvariant); /// @@ -31,7 +28,7 @@ public override void Initialize(AnalysisContext context) { } private static void OnCompilationStart(CompilationStartAnalysisContext context) { - INamedTypeSymbol? errorCodeType = context.Compilation.GetTypeByMetadataName(ErrorCodeMetadataName); + INamedTypeSymbol? errorCodeType = context.Compilation.GetTypeByMetadataName(ErrorCodeFacts.ErrorCodeMetadataName); if (errorCodeType is null) { return; } context.RegisterOperationAction(operationContext => Analyze(operationContext, errorCodeType), OperationKind.Invocation); @@ -39,15 +36,12 @@ private static void OnCompilationStart(CompilationStartAnalysisContext context) private static void Analyze(OperationAnalysisContext context, INamedTypeSymbol errorCodeType) { IInvocationOperation invocation = (IInvocationOperation)context.Operation; - IMethodSymbol method = invocation.TargetMethod; - if (!method.IsStatic || method.Name != CreateMethodName) { return; } - if (!SymbolEqualityComparer.Default.Equals(method.ContainingType, errorCodeType)) { return; } - if (invocation.Arguments.Length != 1) { return; } + IOperation? argument = ErrorCodeFacts.GetCreateArgument(invocation, errorCodeType); + if (argument is null) { return; } - IOperation argument = invocation.Arguments[0].Value; - if (argument.ConstantValue.Value is not string code) { return; } // non-literal → FCE003 - if (string.IsNullOrWhiteSpace(code)) { return; } // empty → FCE002 + // Non-literal codes are FCE003's concern, empty ones FCE002's. + if (!ErrorCodeFacts.TryGetNonEmptyLiteralCode(argument, out string code)) { return; } if (UpperSnakeCase.IsMatch(code)) { return; } context.ReportDiagnostic(Diagnostic.Create(Descriptors.InvalidErrorCodeFormat, argument.Syntax.GetLocation(), code)); diff --git a/FirstClassErrors.Analyzers/TooGenericErrorCodeAnalyzer.cs b/FirstClassErrors.Analyzers/TooGenericErrorCodeAnalyzer.cs index 7c1ff87..95ebe92 100644 --- a/FirstClassErrors.Analyzers/TooGenericErrorCodeAnalyzer.cs +++ b/FirstClassErrors.Analyzers/TooGenericErrorCodeAnalyzer.cs @@ -13,9 +13,6 @@ namespace FirstClassErrors.Analyzers; [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class TooGenericErrorCodeAnalyzer : DiagnosticAnalyzer { - private const string ErrorCodeMetadataName = "FirstClassErrors.ErrorCode"; - private const string CreateMethodName = "Create"; - private static readonly ImmutableHashSet GenericCodes = ImmutableHashSet.Create( StringComparer.OrdinalIgnoreCase, "ERROR", "INVALID", "FAILED", "FAILURE", "UNKNOWN", "EXCEPTION", "BAD", "WRONG", "GENERIC", "PROBLEM"); @@ -32,7 +29,7 @@ public override void Initialize(AnalysisContext context) { } private static void OnCompilationStart(CompilationStartAnalysisContext context) { - INamedTypeSymbol? errorCodeType = context.Compilation.GetTypeByMetadataName(ErrorCodeMetadataName); + INamedTypeSymbol? errorCodeType = context.Compilation.GetTypeByMetadataName(ErrorCodeFacts.ErrorCodeMetadataName); if (errorCodeType is null) { return; } context.RegisterOperationAction(operationContext => Analyze(operationContext, errorCodeType), OperationKind.Invocation); @@ -40,14 +37,11 @@ private static void OnCompilationStart(CompilationStartAnalysisContext context) private static void Analyze(OperationAnalysisContext context, INamedTypeSymbol errorCodeType) { IInvocationOperation invocation = (IInvocationOperation)context.Operation; - IMethodSymbol method = invocation.TargetMethod; - if (!method.IsStatic || method.Name != CreateMethodName) { return; } - if (!SymbolEqualityComparer.Default.Equals(method.ContainingType, errorCodeType)) { return; } - if (invocation.Arguments.Length != 1) { return; } + IOperation? argument = ErrorCodeFacts.GetCreateArgument(invocation, errorCodeType); + if (argument is null) { return; } - IOperation argument = invocation.Arguments[0].Value; - if (argument.ConstantValue.Value is not string code) { return; } + if (!ErrorCodeFacts.TryGetNonEmptyLiteralCode(argument, out string code)) { return; } if (!GenericCodes.Contains(code.Trim())) { return; } context.ReportDiagnostic(Diagnostic.Create(Descriptors.TooGenericErrorCode, argument.Syntax.GetLocation(), code)); diff --git a/FirstClassErrors.Analyzers/UnusedToExceptionResultAnalyzer.cs b/FirstClassErrors.Analyzers/UnusedToExceptionResultAnalyzer.cs index 49d20d6..e86d4a1 100644 --- a/FirstClassErrors.Analyzers/UnusedToExceptionResultAnalyzer.cs +++ b/FirstClassErrors.Analyzers/UnusedToExceptionResultAnalyzer.cs @@ -42,10 +42,19 @@ private static void Analyze(OperationAnalysisContext context, INamedTypeSymbol e if (method.Name != ToExceptionMethodName || method.Parameters.Length != 0) { return; } if (!SymbolFacts.IsOrInheritsFrom(method.ContainingType, errorType)) { return; } - // The result is used unless the invocation stands alone as an expression statement. - if (invocation.Parent is not IExpressionStatementOperation) { return; } + if (!IsResultDiscarded(invocation)) { return; } context.ReportDiagnostic(Diagnostic.Create(Descriptors.UnusedToExceptionResult, invocation.Syntax.GetLocation())); } + // The result is thrown away either when the call stands alone as a statement or when it is explicitly discarded + // (`_ = error.ToException();`). ToException() is a pure builder, so an explicit discard is just as pointless. + private static bool IsResultDiscarded(IInvocationOperation invocation) { + return invocation.Parent switch { + IExpressionStatementOperation => true, + ISimpleAssignmentOperation { Target: IDiscardOperation } => true, + _ => false, + }; + } + } diff --git a/doc/analyzers/FCE016.en.md b/doc/analyzers/FCE016.en.md index 9ff4797..2c0169e 100644 --- a/doc/analyzers/FCE016.en.md +++ b/doc/analyzers/FCE016.en.md @@ -9,7 +9,7 @@ | **Severity** | 🟠 Warning | | **Enabled by default** | Yes | -`Error.ToException()` is called as a standalone statement and its result is discarded. `ToException()` only builds the exception; without a `throw` (or capturing the result) nothing happens and the error is lost. +`Error.ToException()` is called as a standalone statement, or its result is explicitly discarded with `_ =`. Either way the result is thrown away. `ToException()` only builds the exception; without a `throw` (or capturing the result) nothing happens and the error is lost. ## Noncompliant @@ -19,6 +19,12 @@ public void Guard(DomainError error) { } ``` +```csharp +public void Guard(DomainError error) { + _ = error.ToException(); // FCE016: result discarded +} +``` + ## Compliant ```csharp diff --git a/doc/analyzers/FCE016.fr.md b/doc/analyzers/FCE016.fr.md index 8a18a8c..f57a811 100644 --- a/doc/analyzers/FCE016.fr.md +++ b/doc/analyzers/FCE016.fr.md @@ -9,7 +9,7 @@ | **Sévérité** | 🟠 Warning | | **Activée par défaut** | Oui | -`Error.ToException()` est appelé comme instruction isolée et son résultat est ignoré. `ToException()` ne fait que construire l'exception : sans `throw` (ou sans capturer le résultat), rien ne se passe et l'erreur est perdue. +`Error.ToException()` est appelé comme instruction isolée, ou son résultat est explicitement ignoré avec `_ =`. Dans les deux cas le résultat est jeté. `ToException()` ne fait que construire l'exception : sans `throw` (ou sans capturer le résultat), rien ne se passe et l'erreur est perdue. ## Non conforme @@ -19,6 +19,12 @@ public void Guard(DomainError error) { } ``` +```csharp +public void Guard(DomainError error) { + _ = error.ToException(); // FCE016: result discarded +} +``` + ## Conforme ```csharp diff --git a/doc/analyzers/README.fr.md b/doc/analyzers/README.fr.md index e7a208a..8ed0f12 100644 --- a/doc/analyzers/README.fr.md +++ b/doc/analyzers/README.fr.md @@ -41,7 +41,7 @@ Chaque règle a un identifiant stable `FCExxx`. Les erreurs sont des défauts du | Règle | Sévérité | Défaut | Description | |------|----------|---------|-------------| -| [FCE016 UnusedToExceptionResult](FCE016.fr.md) | 🟠 Warning | activée | Error.ToException() est appelé comme instruction isolée et son résultat est ignoré. | +| [FCE016 UnusedToExceptionResult](FCE016.fr.md) | 🟠 Warning | activée | Error.ToException() est appelé comme instruction isolée, ou son résultat est explicitement ignoré avec `_ =`. | ## Configuration diff --git a/doc/analyzers/README.md b/doc/analyzers/README.md index e240976..5d733ac 100644 --- a/doc/analyzers/README.md +++ b/doc/analyzers/README.md @@ -41,7 +41,7 @@ Each rule has a stable id `FCExxx`. Errors are hard defects; warnings flag likel | Rule | Severity | Default | Description | |------|----------|---------|-------------| -| [FCE016 UnusedToExceptionResult](FCE016.en.md) | 🟠 Warning | on | Error.ToException() is called as a standalone statement and its result is discarded. | +| [FCE016 UnusedToExceptionResult](FCE016.en.md) | 🟠 Warning | on | Error.ToException() is called as a standalone statement, or its result is explicitly discarded with `_ =`. | ## Configuring