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