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