From c6573a269a3c4c73d010ea31f6282732ef7dc6f8 Mon Sep 17 00:00:00 2001 From: Thomas Bruderer Date: Tue, 27 Oct 2026 18:51:28 +0100 Subject: [PATCH] =?UTF-8?q?Derive=20the=20=CE=BB1012=20code=20fix=20from?= =?UTF-8?q?=20ExtensionMemberCodeFix=20for=20a=20one-pass=20Fix-All?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix inserted `using Funcky.Extensions;` through its own path with the default BatchFixer, so three or more diagnostics in one document that all needed the directive collided and some were only rewritten by a second Fix-All pass, the same defect as #1009. It now derives from the base of #1013, which rewrites all flagged calls of a document at once and adds the directive once. The rewrite replaces the whole `?? fallback` coalesce, which the base covers unchanged; only the binding probe differs, since the replacement binds to `Option.GetOrElse` in `Funcky.Monads` rather than to a member of `Funcky.Extensions`, so the fix overrides `TryApplyAsync` with a predicate that takes any bound method as proof the directive is in place. Two tests with three and four fixes per document, with the usings at file level and inside the namespace, failed before with "Expected '1' iterations but found '2' iterations". Closes #1020 Co-Authored-By: Claude Fable 5.1 --- .../OrNone/PreferOrNoneCodeFix.cs | 73 ++++++------------ .../OrNone/PreferOrNoneTest.cs | 76 +++++++++++++++++++ changelog.md | 9 ++- 3 files changed, 106 insertions(+), 52 deletions(-) diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/OrNone/PreferOrNoneCodeFix.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/OrNone/PreferOrNoneCodeFix.cs index cc02d2093..2b7d375f6 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/OrNone/PreferOrNoneCodeFix.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/OrNone/PreferOrNoneCodeFix.cs @@ -3,7 +3,6 @@ using Funcky.Analyzers.CodeAnalysisExtensions; using Funcky.Analyzers.Fallback; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -16,10 +15,8 @@ namespace Funcky.Analyzers.OrNone; /// The other null checks (?., is null, …) have no single idiomatic shape, so they are reported without a fix. [Shared] [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(PreferOrNoneCodeFix))] -public sealed class PreferOrNoneCodeFix : CodeFixProvider +public sealed class PreferOrNoneCodeFix : ExtensionMemberCodeFix { - private const string ExtensionsNamespace = "Funcky.Extensions"; - private const string GetOrElseMethodName = "GetOrElse"; private enum FallbackKind @@ -30,34 +27,27 @@ private enum FallbackKind public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(PreferOrNoneAnalyzer.DiagnosticId); - public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + // The replacement is `xs.FirstOrNone().GetOrElse(fallback)`. + protected override string GetTitle(ExtensionMemberRewrite rewrite) + => $"Replace with '{OrNoneMemberAccess(rewrite).Name.Identifier.Text}().{GetOrElseMethodName}(…)'"; + + protected override ExtensionMemberRewrite? TryCreateRewrite(SyntaxNode root, SemanticModel semanticModel, Diagnostic diagnostic, CancellationToken cancellationToken) + => root.FindInvocationExpression(diagnostic.Location.SourceSpan) is { Expression: MemberAccessExpressionSyntax memberAccess } invocation + && diagnostic.Properties.TryGetValue(PreferOrNoneAnalyzer.OrNoneMethodNameProperty, out var orNoneMethodName) + && orNoneMethodName is not null + && FindCoalesceExpression(invocation) is { } coalesce + && KeepsTheResultType(semanticModel, invocation, coalesce, cancellationToken) + && ClassifyFallback(semanticModel, Unparenthesized(coalesce.Right), cancellationToken) is { } fallbackKind + ? new ExtensionMemberRewrite(coalesce, GetOrElseInvocation(coalesce, invocation, memberAccess, orNoneMethodName, fallbackKind)) + : null; - public override async Task RegisterCodeFixesAsync(CodeFixContext context) - { - if (await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root - || await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false) is not { } semanticModel) - { - return; - } + /// The replacement binds to Option.GetOrElse, a member of Funcky.Monads, when its …OrNone receiver binds, + /// which it does only with the using for Funcky.Extensions in place. Any bound method therefore shows the directive is there. + protected override Task TryApplyAsync(Document document, SyntaxNode root, SemanticModel semanticModel, ImmutableArray rewrites, CancellationToken cancellationToken) + => Task.FromResult(FunckyExtensionsImport.ReplaceWithExtensionMemberCalls(document, root, semanticModel, rewrites, static symbol => symbol is IMethodSymbol)); - foreach (var diagnostic in context.Diagnostics) - { - if (root.FindInvocationExpression(diagnostic.Location.SourceSpan) is { Expression: MemberAccessExpressionSyntax memberAccess } invocation - && diagnostic.Properties.TryGetValue(PreferOrNoneAnalyzer.OrNoneMethodNameProperty, out var orNoneMethodName) - && orNoneMethodName is not null - && FindCoalesceExpression(invocation) is { } coalesce - && KeepsTheResultType(semanticModel, invocation, coalesce, context.CancellationToken) - && ClassifyFallback(semanticModel, Unparenthesized(coalesce.Right), context.CancellationToken) is { } fallbackKind) - { - context.RegisterCodeFix( - CodeAction.Create( - $"Replace with '{orNoneMethodName}().{GetOrElseMethodName}(…)'", - cancellationToken => ReplaceWithGetOrElseAsync(context.Document, new Rewrite(coalesce, invocation, memberAccess, orNoneMethodName, fallbackKind), cancellationToken), - nameof(PreferOrNoneCodeFix)), - diagnostic); - } - } - } + private static MemberAccessExpressionSyntax OrNoneMemberAccess(ExtensionMemberRewrite rewrite) + => (MemberAccessExpressionSyntax)((InvocationExpressionSyntax)((MemberAccessExpressionSyntax)((InvocationExpressionSyntax)rewrite.Replacement).Expression).Expression).Expression; private static BinaryExpressionSyntax? FindCoalesceExpression(InvocationExpressionSyntax invocation) => SkipParentheses(invocation) is { Parent: BinaryExpressionSyntax { RawKind: (int)SyntaxKind.CoalesceExpression } coalesce } left && coalesce.Left == left @@ -102,22 +92,14 @@ private static IEnumerable DescendantsAndSelfOutsideFunctions(IOpera ? [] : [operation, .. operation.ChildOperations.SelectMany(DescendantsAndSelfOutsideFunctions)]; - private static async Task ReplaceWithGetOrElseAsync(Document document, Rewrite rewrite, CancellationToken cancellationToken) + private static InvocationExpressionSyntax GetOrElseInvocation(BinaryExpressionSyntax coalesce, InvocationExpressionSyntax invocation, MemberAccessExpressionSyntax memberAccess, string orNoneMethodName, FallbackKind fallbackKind) { - if (await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false) is not { } root - || await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false) is not { } semanticModel) - { - return document; - } + var orNoneInvocation = invocation.WithExpression(memberAccess.WithName(RenamedTo(memberAccess.Name, orNoneMethodName))); - var orNoneInvocation = rewrite.Invocation.WithExpression(rewrite.MemberAccess.WithName(RenamedTo(rewrite.MemberAccess.Name, rewrite.OrNoneMethodName))); - var getOrElse = InvocationExpression( + return InvocationExpression( MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, orNoneInvocation.WithoutTrailingTrivia(), IdentifierName(GetOrElseMethodName)), - ArgumentList(SingletonSeparatedList(Argument(Fallback(Unparenthesized(rewrite.Coalesce.Right), rewrite.FallbackKind))))) - .WithTriviaFrom(rewrite.Coalesce); - - var newRoot = root.ReplaceNode(rewrite.Coalesce, getOrElse); - return document.WithSyntaxRoot(IsExtensionsNamespaceImported(semanticModel, rewrite.Invocation, orNoneInvocation) ? newRoot : UsingDirectives.AddUsing(newRoot, newRoot.FindNode(rewrite.Coalesce.Span), ExtensionsNamespace)); + ArgumentList(SingletonSeparatedList(Argument(Fallback(Unparenthesized(coalesce.Right), fallbackKind))))) + .WithTriviaFrom(coalesce); } /// Only the identifier changes: explicit type arguments (FirstOrDefault<object>()) select the element type and stay. @@ -128,9 +110,4 @@ private static ExpressionSyntax Fallback(ExpressionSyntax fallback, FallbackKind => fallbackKind == FallbackKind.Lazy ? ParenthesizedLambdaExpression(fallback.WithoutTrivia()) : fallback.WithoutTrivia(); - - private static bool IsExtensionsNamespaceImported(SemanticModel semanticModel, InvocationExpressionSyntax original, InvocationExpressionSyntax orNoneInvocation) - => semanticModel.GetSpeculativeSymbolInfo(original.SpanStart, orNoneInvocation, SpeculativeBindingOption.BindAsExpression) is { Symbol: IMethodSymbol } or { CandidateSymbols.IsEmpty: false }; - - private sealed record Rewrite(BinaryExpressionSyntax Coalesce, InvocationExpressionSyntax Invocation, MemberAccessExpressionSyntax MemberAccess, string OrNoneMethodName, FallbackKind FallbackKind); } diff --git a/Funcky.Analyzers/Funcky.Analyzers.Test/OrNone/PreferOrNoneTest.cs b/Funcky.Analyzers/Funcky.Analyzers.Test/OrNone/PreferOrNoneTest.cs index c653caa4f..598c1710a 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.Test/OrNone/PreferOrNoneTest.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.Test/OrNone/PreferOrNoneTest.cs @@ -140,6 +140,82 @@ public static class C await VerifyCS.VerifyCodeFixAsync(inputCode + FunckyStubs, fixedCode + FunckyStubs); } + [Fact] + public async Task ThreeFixesThatAllNeedTheUsingAreAppliedInOnePass() + { + const string inputCode = """ + using System.Collections.Generic; + using System.Linq; + + public static class C + { + public static string First(IEnumerable values) => {|λ1012:values.FirstOrDefault()|} ?? "fallback"; + + public static string Last(IEnumerable values) => {|λ1012:values.LastOrDefault()|} ?? "fallback"; + + public static string Single(IEnumerable values) => {|λ1012:values.SingleOrDefault()|} ?? "fallback"; + + public static string ElementAt(IEnumerable values) => {|λ1012:values.ElementAtOrDefault(2)|} ?? "fallback"; + } + """; + const string fixedCode = """ + using System.Collections.Generic; + using System.Linq; + using Funcky.Extensions; + + public static class C + { + public static string First(IEnumerable values) => values.FirstOrNone().GetOrElse("fallback"); + + public static string Last(IEnumerable values) => values.LastOrNone().GetOrElse("fallback"); + + public static string Single(IEnumerable values) => values.SingleOrNone().GetOrElse("fallback"); + + public static string ElementAt(IEnumerable values) => values.ElementAtOrNone(2).GetOrElse("fallback"); + } + """; + await VerifyCS.VerifyCodeFixAsync(inputCode + FunckyStubs, fixedCode + FunckyStubs); + } + + [Fact] + public async Task ThreeFixesInANamespaceThatAllNeedTheUsingAreAppliedInOnePass() + { + const string inputCode = """ + namespace Example + { + using System.Collections.Generic; + using System.Linq; + + public static class C + { + public static string First(IEnumerable values) => {|λ1012:values.FirstOrDefault()|} ?? "fallback"; + + public static string Last(IEnumerable values) => {|λ1012:values.LastOrDefault()|} ?? "fallback"; + + public static string Single(IEnumerable values) => {|λ1012:values.SingleOrDefault()|} ?? "fallback"; + } + } + """; + const string fixedCode = """ + namespace Example + { + using System.Collections.Generic; + using System.Linq; + using Funcky.Extensions; + + public static class C + { + public static string First(IEnumerable values) => values.FirstOrNone().GetOrElse("fallback"); + + public static string Last(IEnumerable values) => values.LastOrNone().GetOrElse("fallback"); + + public static string Single(IEnumerable values) => values.SingleOrNone().GetOrElse("fallback"); + } + } + """; + await VerifyCS.VerifyCodeFixAsync(inputCode + FunckyStubs, fixedCode + FunckyStubs); + } + [Fact] public async Task OtherNullChecksAreReportedWithoutFix() { diff --git a/changelog.md b/changelog.md index 520f92fc5..e0a01a065 100644 --- a/changelog.md +++ b/changelog.md @@ -97,10 +97,11 @@ Funcky adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * [λ1203](https://polyadic.github.io/funcky/analyzer-rules/λ1203.html) `FunctionalAssert.Some(…)` → `Assert.Some(…)`. * Fix: λ1101 names the receiver as written in its message (`Assert.Some`); for a static extension member it used to print the unnamed extension block (`.Some`). -* Fix: the code fixes that add `using Funcky.Extensions;` (λ1201, λ1202, λ1203 and the `Option.FromBoolean` fix) share a - Fix-All provider that rewrites every flagged call of a document in one pass and adds the directive once. The batch - fixer treated three or more identical directive insertions as colliding edits and dropped those fixes, call rewrite - included, so a second Fix-All pass was needed ([#1009](https://github.com/polyadic/funcky/issues/1009)). +* Fix: the code fixes that add `using Funcky.Extensions;` (λ1012, λ1201, λ1202, λ1203 and the `Option.FromBoolean` fix) + share a Fix-All provider that rewrites every flagged call of a document in one pass and adds the directive once. The + batch fixer treated three or more identical directive insertions as colliding edits and dropped those fixes, call + rewrite included, so a second Fix-All pass was needed ([#1009](https://github.com/polyadic/funcky/issues/1009), + [#1020](https://github.com/polyadic/funcky/issues/1020)). ## Funcky 3.7.0 | Funcky.Analyzers 2.0.0