From 625628846d5ee8a2ae80e89df893edcf561757a6 Mon Sep 17 00:00:00 2001 From: Thomas Bruderer Date: Tue, 27 Oct 2026 17:03:33 +0100 Subject: [PATCH 1/2] Rewrite all extension-form calls of a document in one Fix-All pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code fixes for λ1201, λ1202, λ1203 and the Option.FromBoolean obsoletion all add `using Funcky.Extensions;` when needed. The batch fixer treats three or more identical directive insertions in a document as colliding edits and drops those actions, call rewrite included, so a second Fix-All pass was needed. The new ExtensionMemberCodeFix base owns a DocumentBasedFixAllProvider that applies every rewrite of a document at once through the plural FunckyExtensionsImport.ReplaceWithExtensionMemberCalls and adds the using once per using container. BooleanThen keeps its post-check that every Then call binds to Funcky's extension and leaves out the calls that do not. The single-diagnostic actions keep their behaviour. The "adds the using" tests of all four fixes now have three or more diagnostics per document. Closes #1009 Co-Authored-By: Claude Fable 5.1 --- .../CodeAnalysisExtensions/BooleanThen.cs | 42 +++++---- .../ExtensionMemberRewrite.cs | 6 ++ .../FunckyExtensionsImport.cs | 58 +++++++++++-- ...erEnumerableSequenceConstructorsCodeFix.cs | 42 ++------- .../PreferStaticParseOrNoneCodeFix.cs | 64 ++++---------- .../PreferXunitAssertCodeFix.cs | 42 ++------- .../ExtensionMemberCodeFix.cs | 87 +++++++++++++++++++ .../OptionFromBooleanCodeFix.cs | 53 ++++++----- .../OptionFromBooleanCodeFixTest.cs | 11 ++- ...referEnumerableSequenceConstructorsTest.cs | 13 ++- .../PreferStaticParseOrNoneTest.cs | 8 +- .../PreferXunitAssertTest.cs | 17 +++- changelog.md | 4 + 13 files changed, 271 insertions(+), 176 deletions(-) create mode 100644 Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/ExtensionMemberRewrite.cs create mode 100644 Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionMemberCodeFix.cs diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/BooleanThen.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/BooleanThen.cs index 46dd53e3..32bf53a5 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/BooleanThen.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/BooleanThen.cs @@ -1,3 +1,4 @@ +using System.Collections.Immutable; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -11,10 +12,6 @@ namespace Funcky.Analyzers.CodeAnalysisExtensions; /// extension type nested in BooleanExtensions (or directly in it when declared as a classic extension method). internal static class BooleanThen { - public const string BooleanExtensionsNamespace = "Funcky.Extensions"; - - private static readonly SyntaxAnnotation ReplacementAnnotation = new(); - public static InvocationExpressionSyntax Invocation(ExpressionSyntax condition, SimpleNameSyntax thenName, ArgumentSyntax argument) => InvocationExpression( MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, Receiver(condition), thenName), @@ -46,31 +43,40 @@ public static bool IsThen(ISymbol? symbol, INamedTypeSymbol booleanExtensionsTyp /// Replaces with the Then call, adding using Funcky.Extensions; when the call does not /// bind without it. Returns when the call still does not bind to Funcky's Then afterwards, because /// another Then extension in scope wins or makes the call ambiguous. - public static async Task TryReplaceAsync(Document document, SyntaxNode root, SemanticModel semanticModel, ExpressionSyntax target, InvocationExpressionSyntax replacement, INamedTypeSymbol booleanExtensionsType, CancellationToken cancellationToken) - { - var rootWithReplacement = root.ReplaceNode(target, replacement.WithAdditionalAnnotations(ReplacementAnnotation)); + public static Task TryReplaceAsync(Document document, SyntaxNode root, SemanticModel semanticModel, ExpressionSyntax target, InvocationExpressionSyntax replacement, INamedTypeSymbol booleanExtensionsType, CancellationToken cancellationToken) + => TryReplaceAllAsync(document, root, semanticModel, ImmutableArray.Create(new ExtensionMemberRewrite(target, replacement)), booleanExtensionsType, cancellationToken); - if (IsThen(semanticModel.GetSpeculativeSymbolInfo(target.SpanStart, replacement, SpeculativeBindingOption.BindAsExpression).Symbol, booleanExtensionsType)) + /// Applies all rewrites at once, adding using Funcky.Extensions; once when a Then call does not bind without it. + /// A call that still does not bind to Funcky's Then afterwards, because another Then extension in scope wins or makes + /// it ambiguous, is left as it is; the result is when that holds for every call. + public static async Task TryReplaceAllAsync(Document document, SyntaxNode root, SemanticModel semanticModel, ImmutableArray rewrites, INamedTypeSymbol booleanExtensionsType, CancellationToken cancellationToken) + { + if (rewrites.IsEmpty) { - return document.WithSyntaxRoot(rootWithReplacement); + return null; } - var documentWithUsing = document.WithSyntaxRoot(UsingDirectives.AddUsing(rootWithReplacement, rootWithReplacement.GetAnnotatedNodes(ReplacementAnnotation).Single(), BooleanExtensionsNamespace)); + var changedDocument = FunckyExtensionsImport.ReplaceWithExtensionMemberCalls(document, root, semanticModel, rewrites, symbol => IsThen(symbol, booleanExtensionsType)); + var unboundRewrites = await UnboundRewritesAsync(changedDocument, rewrites.Length, cancellationToken).ConfigureAwait(false); - return await BindsToThenAsync(documentWithUsing, cancellationToken).ConfigureAwait(false) - ? documentWithUsing - : null; + return unboundRewrites.IsEmpty + ? changedDocument + : await TryReplaceAllAsync(document, root, semanticModel, rewrites.Where((_, index) => !unboundRewrites.Contains(index)).ToImmutableArray(), booleanExtensionsType, cancellationToken).ConfigureAwait(false); } - private static async Task BindsToThenAsync(Document document, CancellationToken cancellationToken) + /// The indices of the rewrites whose Then call does not bind to Funcky's extension in the changed document; all of + /// them when the document cannot be bound at all. + private static async Task> UnboundRewritesAsync(Document document, int rewriteCount, CancellationToken cancellationToken) { var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - return root is not null - && semanticModel is not null - && semanticModel.Compilation.GetBooleanExtensionsType() is { } booleanExtensionsType - && IsThen(semanticModel.GetSymbolInfo(root.GetAnnotatedNodes(ReplacementAnnotation).Single(), cancellationToken).Symbol, booleanExtensionsType); + return root is not null && semanticModel is not null && semanticModel.Compilation.GetBooleanExtensionsType() is { } booleanExtensionsType + ? FunckyExtensionsImport.Replacements(root) + .Where(replacement => !IsThen(semanticModel.GetSymbolInfo(replacement.Replacement, cancellationToken).Symbol, booleanExtensionsType)) + .Select(replacement => replacement.Index) + .ToImmutableArray() + : ImmutableArray.CreateRange(Enumerable.Range(0, rewriteCount)); } // A member access binds tighter than every operator, so anything but a primary expression needs parentheses. diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/ExtensionMemberRewrite.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/ExtensionMemberRewrite.cs new file mode 100644 index 00000000..c634aa45 --- /dev/null +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/ExtensionMemberRewrite.cs @@ -0,0 +1,6 @@ +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Funcky.Analyzers.CodeAnalysisExtensions; + +/// An expression to replace with a call of a static extension member of a class in Funcky.Extensions. +public sealed record ExtensionMemberRewrite(ExpressionSyntax Original, ExpressionSyntax Replacement); diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/FunckyExtensionsImport.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/FunckyExtensionsImport.cs index 8084a160..39e24f5d 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/FunckyExtensionsImport.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/FunckyExtensionsImport.cs @@ -1,22 +1,64 @@ +using System.Collections.Immutable; +using System.Globalization; using Funcky.Analyzers.ExtensionForms; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; namespace Funcky.Analyzers.CodeAnalysisExtensions; -/// Replaces an expression with one that calls a static extension member of a class in Funcky.Extensions, and -/// adds the using for that namespace when the replacement does not bind to such a member without it. +/// Replaces expressions with ones that call a static extension member of a class in Funcky.Extensions, and +/// adds the using for that namespace when a replacement does not bind to such a member without it. internal static class FunckyExtensionsImport { - private static readonly SyntaxAnnotation ReplacementAnnotation = new(); + private const string ReplacementAnnotationKind = "Funcky.Analyzers.FunckyExtensionsImport.Replacement"; public static Document ReplaceWithExtensionMemberCall(Document document, SyntaxNode root, SemanticModel semanticModel, ExpressionSyntax original, ExpressionSyntax replacement) + => ReplaceWithExtensionMemberCalls(document, root, semanticModel, ImmutableArray.Create(new ExtensionMemberRewrite(original, replacement)), ExtensionMemberSupport.IsDeclaredInFunckyExtensions); + + /// Applies all rewrites to the document at once. The using is added once per group of usings (the enclosing + /// namespace declaration or the file) that has a replacement in need of it, so that any number of rewrites work in one pass. + /// A rewrite nested in another one is skipped, the outer replacement is built from the original and would discard it anyway; + /// its diagnostic stays and is fixed by a subsequent pass. tells whether the symbol a + /// replacement binds to without the using is the intended extension member. + public static Document ReplaceWithExtensionMemberCalls(Document document, SyntaxNode root, SemanticModel semanticModel, ImmutableArray rewrites, Func bindsToExtensionMember) { - var annotatedReplacement = replacement.WithAdditionalAnnotations(ReplacementAnnotation); - var rootWithReplacement = root.ReplaceNode(original, annotatedReplacement); + var originals = rewrites.Select(rewrite => rewrite.Original).ToImmutableHashSet(); + var outermostRewrites = rewrites + .Select((rewrite, index) => (Rewrite: rewrite, Index: index)) + .Where(pair => !pair.Rewrite.Original.Ancestors().Any(originals.Contains)) + .GroupBy(pair => pair.Rewrite.Original) + .ToDictionary(group => group.Key, group => group.First()); + var rootWithReplacements = root.ReplaceNodes(outermostRewrites.Keys, (original, _) => outermostRewrites[original].Rewrite.Replacement.WithAdditionalAnnotations(Annotation(outermostRewrites[original].Index))); + var replacementsInNeedOfImport = outermostRewrites.Values + .Where(pair => !bindsToExtensionMember(semanticModel.GetSpeculativeSymbolInfo(pair.Rewrite.Original.SpanStart, pair.Rewrite.Replacement, SpeculativeBindingOption.BindAsExpression).Symbol)) + .Select(pair => pair.Index) + .ToImmutableArray(); - return ExtensionMemberSupport.IsDeclaredInFunckyExtensions(semanticModel.GetSpeculativeSymbolInfo(original.SpanStart, replacement, SpeculativeBindingOption.BindAsExpression).Symbol) - ? document.WithSyntaxRoot(rootWithReplacement) - : document.WithSyntaxRoot(UsingDirectives.AddUsing(rootWithReplacement, rootWithReplacement.GetAnnotatedNodes(ReplacementAnnotation).Single(), ExtensionMemberSupport.ExtensionsNamespace)); + return document.WithSyntaxRoot(AddUsings(rootWithReplacements, replacementsInNeedOfImport)); } + + /// The replacements in a root produced by , each with the index of the rewrite it stems from. + public static IEnumerable<(int Index, SyntaxNode Replacement)> Replacements(SyntaxNode root) + => root.GetAnnotatedNodes(ReplacementAnnotationKind).Select(node => (Index(node), node)); + + private static SyntaxNode AddUsings(SyntaxNode root, ImmutableArray replacementsInNeedOfImport) + { + var anchors = Replacements(root) + .Where(replacement => replacementsInNeedOfImport.Contains(replacement.Index)) + .GroupBy(replacement => UsingContainer(replacement.Replacement)?.Span ?? default) + .Select(group => group.First().Index) + .ToImmutableArray(); + + return anchors.Aggregate(root, (current, anchor) => UsingDirectives.AddUsing(current, Replacements(current).Single(replacement => replacement.Index == anchor).Replacement, ExtensionMemberSupport.ExtensionsNamespace)); + } + + // Where UsingDirectives.AddUsing puts the directive: the enclosing namespace declaration with usings, or the file. + private static BaseNamespaceDeclarationSyntax? UsingContainer(SyntaxNode node) + => node.FirstAncestorOrSelf(namespaceDeclaration => namespaceDeclaration.Usings.Count > 0); + + private static SyntaxAnnotation Annotation(int index) + => new(ReplacementAnnotationKind, index.ToString(CultureInfo.InvariantCulture)); + + private static int Index(SyntaxNode replacement) + => int.Parse(replacement.GetAnnotations(ReplacementAnnotationKind).Single().Data!, CultureInfo.InvariantCulture); } diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionForms/PreferEnumerableSequenceConstructorsCodeFix.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionForms/PreferEnumerableSequenceConstructorsCodeFix.cs index 867d21bd..0bc2ba17 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionForms/PreferEnumerableSequenceConstructorsCodeFix.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionForms/PreferEnumerableSequenceConstructorsCodeFix.cs @@ -3,7 +3,6 @@ using Funcky.Analyzers.CodeAnalysisExtensions; using Funcky.Analyzers.ExtensionForms; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -14,28 +13,20 @@ namespace Funcky.Analyzers; [Shared] [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(PreferEnumerableSequenceConstructorsCodeFix))] -public sealed class PreferEnumerableSequenceConstructorsCodeFix : CodeFixProvider +public sealed class PreferEnumerableSequenceConstructorsCodeFix : ExtensionMemberCodeFix { private const string EnumerableTypeName = "global::System.Linq.Enumerable"; public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(PreferEnumerableSequenceConstructorsAnalyzer.PreferEnumerable.Id); - public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + protected override string GetTitle(ExtensionMemberRewrite rewrite) => "Replace with the Enumerable form"; - public override async Task RegisterCodeFixesAsync(CodeFixContext context) - { - var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); - - foreach (var diagnostic in context.Diagnostics) - { - if (root?.FindInvocationExpression(diagnostic.Location.SourceSpan) is { } invocation - && invocation.Span == diagnostic.Location.SourceSpan - && WithEnumerableReceiver(invocation.Expression) is { } replacement) - { - context.RegisterCodeFix(new UseEnumerableCodeFixAction(context.Document, invocation, replacement), diagnostic); - } - } - } + protected override ExtensionMemberRewrite? TryCreateRewrite(SyntaxNode root, SemanticModel semanticModel, Diagnostic diagnostic, CancellationToken cancellationToken) + => root.FindInvocationExpression(diagnostic.Location.SourceSpan) is { } invocation + && invocation.Span == diagnostic.Location.SourceSpan + && WithEnumerableReceiver(invocation.Expression) is { } replacement + ? new ExtensionMemberRewrite(invocation, replacement) + : null; // `Sequence.Return(x)` and `Funcky.Sequence.Return(x)` keep their name (and type arguments); `Return(x)` through a // `using static` gets the Enumerable receiver in front. @@ -49,21 +40,4 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) private static ExpressionSyntax Enumerable() => ParseExpression(EnumerableTypeName).WithAdditionalAnnotations(Simplifier.Annotation); - - private sealed class UseEnumerableCodeFixAction(Document document, InvocationExpressionSyntax invocation, InvocationExpressionSyntax replacement) : CodeAction - { - public override string Title => "Replace with the Enumerable form"; - - public override string EquivalenceKey => nameof(PreferEnumerableSequenceConstructorsCodeFix); - - protected override async Task GetChangedDocumentAsync(CancellationToken cancellationToken) - { - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - - return root is null || semanticModel is null - ? document - : FunckyExtensionsImport.ReplaceWithExtensionMemberCall(document, root, semanticModel, invocation, replacement); - } - } } diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionForms/PreferStaticParseOrNoneCodeFix.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionForms/PreferStaticParseOrNoneCodeFix.cs index fed65aee..3ee70995 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionForms/PreferStaticParseOrNoneCodeFix.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionForms/PreferStaticParseOrNoneCodeFix.cs @@ -3,7 +3,6 @@ using Funcky.Analyzers.CodeAnalysisExtensions; using Funcky.Analyzers.ExtensionForms; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Operations; @@ -13,52 +12,25 @@ namespace Funcky.Analyzers; [Shared] [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(PreferStaticParseOrNoneCodeFix))] -public sealed class PreferStaticParseOrNoneCodeFix : CodeFixProvider +public sealed class PreferStaticParseOrNoneCodeFix : ExtensionMemberCodeFix { - public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(PreferStaticParseOrNoneAnalyzer.PreferStaticParseOrNone.Id); - - public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; - - public override async Task RegisterCodeFixesAsync(CodeFixContext context) - { - var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); - var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); - - foreach (var diagnostic in context.Diagnostics) - { - if (root?.FindInvocationExpression(diagnostic.Location.SourceSpan) is { } invocation - && invocation.Span == diagnostic.Location.SourceSpan - && diagnostic.Properties.TryGetValue(PreferStaticParseOrNoneAnalyzer.TargetTypeProperty, out var targetType) - && diagnostic.Properties.TryGetValue(PreferStaticParseOrNoneAnalyzer.StaticMemberNameProperty, out var staticMemberName) - && targetType is not null - && staticMemberName is not null - && semanticModel?.GetOperation(invocation, context.CancellationToken) is IInvocationOperation operation - && PreferStaticParseOrNoneAnalyzer.StaticInvocation(operation, targetType, staticMemberName) is { } replacement) - { - context.RegisterCodeFix(new UseStaticMemberCodeFixAction(context.Document, invocation, replacement, targetType, staticMemberName), diagnostic); - } - } - } + private const string GlobalAliasPrefix = "global::"; - private sealed class UseStaticMemberCodeFixAction(Document document, InvocationExpressionSyntax invocation, InvocationExpressionSyntax replacement, string targetType, string staticMemberName) : CodeAction - { - public override string Title => $"Replace with {targetType.Replace("global::", string.Empty)}.{staticMemberName}"; - - public override string EquivalenceKey => nameof(PreferStaticParseOrNoneCodeFix); - - protected override async Task GetChangedDocumentAsync(CancellationToken cancellationToken) - { - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(PreferStaticParseOrNoneAnalyzer.PreferStaticParseOrNone.Id); - return root is null || semanticModel is null - ? document - : FunckyExtensionsImport.ReplaceWithExtensionMemberCall( - document, - root, - semanticModel, - invocation, - replacement.WithExpression(replacement.Expression.WithAdditionalAnnotations(Simplifier.Annotation)).WithTriviaFrom(invocation)); - } - } + // The replacement calls `global::TargetType.StaticMember`. + protected override string GetTitle(ExtensionMemberRewrite rewrite) + => $"Replace with {((InvocationExpressionSyntax)rewrite.Replacement).Expression.ToString().Replace(GlobalAliasPrefix, string.Empty)}"; + + protected override ExtensionMemberRewrite? TryCreateRewrite(SyntaxNode root, SemanticModel semanticModel, Diagnostic diagnostic, CancellationToken cancellationToken) + => root.FindInvocationExpression(diagnostic.Location.SourceSpan) is { } invocation + && invocation.Span == diagnostic.Location.SourceSpan + && diagnostic.Properties.TryGetValue(PreferStaticParseOrNoneAnalyzer.TargetTypeProperty, out var targetType) + && diagnostic.Properties.TryGetValue(PreferStaticParseOrNoneAnalyzer.StaticMemberNameProperty, out var staticMemberName) + && targetType is not null + && staticMemberName is not null + && semanticModel.GetOperation(invocation, cancellationToken) is IInvocationOperation operation + && PreferStaticParseOrNoneAnalyzer.StaticInvocation(operation, targetType, staticMemberName) is { } replacement + ? new ExtensionMemberRewrite(invocation, replacement.WithExpression(replacement.Expression.WithAdditionalAnnotations(Simplifier.Annotation)).WithTriviaFrom(invocation)) + : null; } diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionForms/PreferXunitAssertCodeFix.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionForms/PreferXunitAssertCodeFix.cs index 3e251263..51410a5d 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionForms/PreferXunitAssertCodeFix.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionForms/PreferXunitAssertCodeFix.cs @@ -3,7 +3,6 @@ using Funcky.Analyzers.CodeAnalysisExtensions; using Funcky.Analyzers.ExtensionForms; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -14,28 +13,20 @@ namespace Funcky.Analyzers; [Shared] [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(PreferXunitAssertCodeFix))] -public sealed class PreferXunitAssertCodeFix : CodeFixProvider +public sealed class PreferXunitAssertCodeFix : ExtensionMemberCodeFix { private const string AssertTypeName = "global::Xunit.Assert"; public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(PreferXunitAssertAnalyzer.PreferAssert.Id); - public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + protected override string GetTitle(ExtensionMemberRewrite rewrite) => "Replace with the assertion on Assert"; - public override async Task RegisterCodeFixesAsync(CodeFixContext context) - { - var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); - - foreach (var diagnostic in context.Diagnostics) - { - if (root?.FindInvocationExpression(diagnostic.Location.SourceSpan) is { } invocation - && invocation.Span == diagnostic.Location.SourceSpan - && WithAssertReceiver(invocation) is { } replacement) - { - context.RegisterCodeFix(new UseAssertCodeFixAction(context.Document, invocation, replacement), diagnostic); - } - } - } + protected override ExtensionMemberRewrite? TryCreateRewrite(SyntaxNode root, SemanticModel semanticModel, Diagnostic diagnostic, CancellationToken cancellationToken) + => root.FindInvocationExpression(diagnostic.Location.SourceSpan) is { } invocation + && invocation.Span == diagnostic.Location.SourceSpan + && WithAssertReceiver(invocation) is { } replacement + ? new ExtensionMemberRewrite(invocation, replacement) + : null; // `FunctionalAssert.Some(x)` and `Funcky.FunctionalAssert.Some(x)` keep their name (and type arguments); `Some(x)` through // a `using static` gets the Assert receiver in front. @@ -49,21 +40,4 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) private static ExpressionSyntax Assert() => ParseExpression(AssertTypeName).WithAdditionalAnnotations(Simplifier.Annotation); - - private sealed class UseAssertCodeFixAction(Document document, InvocationExpressionSyntax invocation, InvocationExpressionSyntax replacement) : CodeAction - { - public override string Title => "Replace with the assertion on Assert"; - - public override string EquivalenceKey => nameof(PreferXunitAssertCodeFix); - - protected override async Task GetChangedDocumentAsync(CancellationToken cancellationToken) - { - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - - return root is null || semanticModel is null - ? document - : FunckyExtensionsImport.ReplaceWithExtensionMemberCall(document, root, semanticModel, invocation, replacement); - } - } } diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionMemberCodeFix.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionMemberCodeFix.cs new file mode 100644 index 00000000..51fbbc57 --- /dev/null +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/ExtensionMemberCodeFix.cs @@ -0,0 +1,87 @@ +using System.Collections.Immutable; +using Funcky.Analyzers.CodeAnalysisExtensions; +using Funcky.Analyzers.ExtensionForms; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; + +namespace Funcky.Analyzers; + +/// Base of the code fixes that replace a call with one of a static extension member in Funcky.Extensions and add +/// the using for the namespace when needed. Every fix adds the same directive, which the batch fixer treats as colliding +/// edits: with three or more fixes in a document it drops some of them, call rewrite included. The Fix-All provider of this +/// base therefore rewrites all flagged calls of a document at once and adds the directive once. +public abstract class ExtensionMemberCodeFix : CodeFixProvider +{ + public sealed override FixAllProvider GetFixAllProvider() => ExtensionMemberFixAllProvider.Instance; + + /// Registers one action per fixable diagnostic, which applies its rewrite when invoked. + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + + if (root is null || semanticModel is null) + { + return; + } + + foreach (var diagnostic in context.Diagnostics) + { + if (TryCreateRewrite(root, semanticModel, diagnostic, context.CancellationToken) is { } rewrite) + { + context.RegisterCodeFix(CodeAction.Create(GetTitle(rewrite), cancellationToken => ApplyAsync(context.Document, rewrite, cancellationToken), GetType().Name), diagnostic); + } + } + } + + /// The title of the action that applies the rewrite. + protected abstract string GetTitle(ExtensionMemberRewrite rewrite); + + /// The rewrite that fixes the diagnostic, or when there is no fix for it. + protected abstract ExtensionMemberRewrite? TryCreateRewrite(SyntaxNode root, SemanticModel semanticModel, Diagnostic diagnostic, CancellationToken cancellationToken); + + /// Applies the rewrites to the document, adding the using when needed, or returns when none of them can be applied. + protected virtual Task TryApplyAsync(Document document, SyntaxNode root, SemanticModel semanticModel, ImmutableArray rewrites, CancellationToken cancellationToken) + => Task.FromResult(FunckyExtensionsImport.ReplaceWithExtensionMemberCalls(document, root, semanticModel, rewrites, ExtensionMemberSupport.IsDeclaredInFunckyExtensions)); + + private async Task ApplyAsync(Document document, ExtensionMemberRewrite rewrite, CancellationToken cancellationToken) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + + return root is null || semanticModel is null + ? document + : await TryApplyAsync(document, root, semanticModel, ImmutableArray.Create(rewrite), cancellationToken).ConfigureAwait(false) ?? document; + } + + private async Task FixAllAsync(Document document, ImmutableArray diagnostics, CancellationToken cancellationToken) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + + if (root is null || semanticModel is null) + { + return null; + } + + var rewrites = diagnostics + .Select(diagnostic => TryCreateRewrite(root, semanticModel, diagnostic, cancellationToken)) + .OfType() + .ToImmutableArray(); + + return rewrites.IsEmpty + ? null + : await TryApplyAsync(document, root, semanticModel, rewrites, cancellationToken).ConfigureAwait(false); + } + + private sealed class ExtensionMemberFixAllProvider : DocumentBasedFixAllProvider + { + public static readonly ExtensionMemberFixAllProvider Instance = new(); + + protected override Task FixAllAsync(FixAllContext fixAllContext, Document document, ImmutableArray diagnostics) + => fixAllContext.CodeFixProvider is ExtensionMemberCodeFix codeFix + ? codeFix.FixAllAsync(document, diagnostics, fixAllContext.CancellationToken) + : Task.FromResult(null); + } +} diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/OptionFromBooleanCodeFix.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/OptionFromBooleanCodeFix.cs index 72402a43..a788508b 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/OptionFromBooleanCodeFix.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/OptionFromBooleanCodeFix.cs @@ -21,46 +21,52 @@ namespace Funcky.Analyzers; /// [Shared] [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(OptionFromBooleanCodeFix))] -public sealed class OptionFromBooleanCodeFix : CodeFixProvider +public sealed class OptionFromBooleanCodeFix : ExtensionMemberCodeFix { /// The compiler diagnostic reported on the use of a member marked [Obsolete] with a message. private const string ObsoleteMemberDiagnosticId = "CS0618"; public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(ObsoleteMemberDiagnosticId); - public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; - + // The document is rewritten up front: the fix is withheld when the Then call would not bind to Funcky's extension. public override async Task RegisterCodeFixesAsync(CodeFixContext context) { var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); - if (root is null || semanticModel is null || GetSymbols(semanticModel.Compilation) is not { } symbols) + if (root is null || semanticModel is null) { return; } foreach (var diagnostic in context.Diagnostics) { - // The warning spans the whole call; one with a smaller span belongs to an obsolete member inside an argument. - // The document is rewritten up front: the fix is withheld when the Then call would not bind to Funcky's extension. - if (root.FindInvocationExpression(diagnostic.Location.SourceSpan) is { } invocation - && invocation.Span == diagnostic.Location.SourceSpan - && semanticModel.GetOperation(invocation, context.CancellationToken) is IInvocationOperation { TargetMethod: { Name: OptionFromBooleanMethodName, IsStatic: true } method } operation - && SymbolEqualityComparer.Default.Equals(method.ContainingType, symbols.OptionType) - && Rewrite.TryCreate(operation, invocation, semanticModel, symbols, context.CancellationToken) is { } rewrite - && await rewrite.TryApplyAsync(context.Document, root, semanticModel, symbols, context.CancellationToken).ConfigureAwait(false) is { } changedDocument) + if (TryCreateRewrite(root, semanticModel, diagnostic, context.CancellationToken) is { } rewrite + && await TryApplyAsync(context.Document, root, semanticModel, ImmutableArray.Create(rewrite), context.CancellationToken).ConfigureAwait(false) is { } changedDocument) { - context.RegisterCodeFix( - CodeAction.Create( - $"Replace Option.{OptionFromBooleanMethodName} with {BooleanThenMethodName}", - _ => Task.FromResult(changedDocument), - nameof(OptionFromBooleanCodeFix)), - diagnostic); + context.RegisterCodeFix(CodeAction.Create(GetTitle(rewrite), _ => Task.FromResult(changedDocument), nameof(OptionFromBooleanCodeFix)), diagnostic); } } } + protected override string GetTitle(ExtensionMemberRewrite rewrite) => $"Replace Option.{OptionFromBooleanMethodName} with {BooleanThenMethodName}"; + + // The warning spans the whole call; one with a smaller span belongs to an obsolete member inside an argument. + protected override ExtensionMemberRewrite? TryCreateRewrite(SyntaxNode root, SemanticModel semanticModel, Diagnostic diagnostic, CancellationToken cancellationToken) + => GetSymbols(semanticModel.Compilation) is { } symbols + && root.FindInvocationExpression(diagnostic.Location.SourceSpan) is { } invocation + && invocation.Span == diagnostic.Location.SourceSpan + && semanticModel.GetOperation(invocation, cancellationToken) is IInvocationOperation { TargetMethod: { Name: OptionFromBooleanMethodName, IsStatic: true } method } operation + && SymbolEqualityComparer.Default.Equals(method.ContainingType, symbols.OptionType) + && Rewrite.TryCreate(operation, invocation, semanticModel, symbols, cancellationToken) is { } rewrite + ? rewrite.ToExtensionMemberRewrite() + : null; + + protected override Task TryApplyAsync(Document document, SyntaxNode root, SemanticModel semanticModel, ImmutableArray rewrites, CancellationToken cancellationToken) + => semanticModel.Compilation.GetBooleanExtensionsType() is { } booleanExtensionsType + ? BooleanThen.TryReplaceAllAsync(document, root, semanticModel, rewrites, booleanExtensionsType, cancellationToken) + : Task.FromResult(null); + private static Symbols? GetSymbols(Compilation compilation) => compilation.GetOptionType() is { } optionType && compilation.GetOptionOfTType() is { } optionOfTType @@ -91,15 +97,8 @@ private sealed record Rewrite(InvocationExpressionSyntax Target, ExpressionSynta }; } - public Task TryApplyAsync(Document document, SyntaxNode root, SemanticModel semanticModel, Symbols symbols, CancellationToken cancellationToken) - => BooleanThen.TryReplaceAsync( - document, - root, - semanticModel, - Target, - BooleanThen.Invocation(Condition, ThenName, ThenArgument).WithTriviaFrom(Target), - symbols.BooleanExtensionsType, - cancellationToken); + public ExtensionMemberRewrite ToExtensionMemberRewrite() + => new(Target, BooleanThen.Invocation(Condition, ThenName, ThenArgument).WithTriviaFrom(Target)); private static Rewrite? TryCreate(IInvocationOperation operation, InvocationExpressionSyntax invocation, SimpleNameSyntax name, ExpressionSyntax condition, ExpressionSyntax argument, IOperation value, SemanticModel semanticModel, Symbols symbols, CancellationToken cancellationToken) { diff --git a/Funcky.Analyzers/Funcky.Analyzers.Test/OptionFromBooleanCodeFixTest.cs b/Funcky.Analyzers/Funcky.Analyzers.Test/OptionFromBooleanCodeFixTest.cs index 731e9696..46e70b45 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.Test/OptionFromBooleanCodeFixTest.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.Test/OptionFromBooleanCodeFixTest.cs @@ -175,6 +175,7 @@ private static void Use(Option option) where T : notnull { } await test.RunAsync(CancellationToken.None); } + // Three or more diagnostics: Fix-All has to rewrite every call in one pass although each fix on its own adds the same using (#1009). [Fact] public async Task AddsTheUsingForTheExtensionWhenMissing() { @@ -186,7 +187,9 @@ public static class C { public static void M(bool flag, int number) { - Use(Option.FromBoolean(flag, number)); + Use({|CS0618:Option.FromBoolean(flag, number)|}); + Use({|CS0618:Option.FromBoolean(flag)|}); + Use({|CS0618:Option.FromBoolean(flag, () => number)|}); } private static void Use(Option option) where T : notnull { } @@ -202,15 +205,15 @@ public static class C public static void M(bool flag, int number) { Use(flag.Then(number)); + Use(flag.Then(Funcky.Unit.Value)); + Use(flag.Then(() => number)); } private static void Use(Option option) where T : notnull { } } """; - var test = CreateTest(inputCode, fixedCode); - test.ExpectedDiagnostics.Add(DiagnosticResult.CompilerWarning(ObsoleteMember).WithSpan(7, 13, 7, 45)); - await test.RunAsync(CancellationToken.None); + await CreateTest(inputCode, fixedCode).RunAsync(CancellationToken.None); } [Fact] diff --git a/Funcky.Analyzers/Funcky.Analyzers.Test/PreferEnumerableSequenceConstructorsTest.cs b/Funcky.Analyzers/Funcky.Analyzers.Test/PreferEnumerableSequenceConstructorsTest.cs index 72f8125f..7fcaa26b 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.Test/PreferEnumerableSequenceConstructorsTest.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.Test/PreferEnumerableSequenceConstructorsTest.cs @@ -97,6 +97,7 @@ public static void M() await VerifyCS.VerifyCodeFixAsync(inputCode + Environment.NewLine + StubCode, fixedCode + Environment.NewLine + StubCode); } + // Three or more diagnostics: Fix-All has to rewrite every call in one pass although each fix on its own adds the same using (#1009). [Fact] public async Task AddsTheUsingForTheExtensionWhenMissing() { @@ -107,7 +108,11 @@ public async Task AddsTheUsingForTheExtensionWhenMissing() public static class C { - public static IReadOnlyList M() => {|λ1202:Sequence.Return(1)|}; + public static IReadOnlyList Single() => {|λ1202:Sequence.Return(1)|}; + + public static IReadOnlyList Many() => {|λ1202:Sequence.Return(1, 2)|}; + + public static IEnumerable Successors() => {|λ1202:Funcky.Sequence.Successors(1, n => n + 1)|}; } """; const string fixedCode = @@ -118,7 +123,11 @@ public static class C public static class C { - public static IReadOnlyList M() => System.Linq.Enumerable.Return(1); + public static IReadOnlyList Single() => System.Linq.Enumerable.Return(1); + + public static IReadOnlyList Many() => System.Linq.Enumerable.Return(1, 2); + + public static IEnumerable Successors() => System.Linq.Enumerable.Successors(1, n => n + 1); } """; diff --git a/Funcky.Analyzers/Funcky.Analyzers.Test/PreferStaticParseOrNoneTest.cs b/Funcky.Analyzers/Funcky.Analyzers.Test/PreferStaticParseOrNoneTest.cs index d6b6c926..050b29ed 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.Test/PreferStaticParseOrNoneTest.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.Test/PreferStaticParseOrNoneTest.cs @@ -197,7 +197,7 @@ public static void M(string text, IFormatProvider provider) await VerifyCS.VerifyCodeFixAsync(inputCode + Environment.NewLine + StubCode, fixedCode + Environment.NewLine + StubCode); } - // Two diagnostics per test: with three or more fixes that all add the using, the batch fixer needs a second pass. + // Three or more diagnostics per test: Fix-All has to rewrite every call in one pass although each fix on its own adds the same using (#1009). [Fact] public async Task AddsTheUsingWhenTheExtensionIsCalledFullyQualified() { @@ -213,6 +213,8 @@ public static void M(string text) { Option a = {|λ1201:Funcky.Extensions.ParseExtensions.ParseInt32OrNone(text)|}; Option b = {|λ1201:Funcky.Extensions.ParseExtensions.ParseInt32OrNone(candidate: text)|}; + Option c = {|λ1201:Funcky.Extensions.ParseExtensions.ParseExactDateTimeOrNone(text, "O", null)|}; + Option d = {|λ1201:Funcky.Extensions.ParseExtensions.ParseInt32OrNone(text.Trim())|}; } } """; @@ -229,6 +231,8 @@ public static void M(string text) { Option a = int.ParseOrNone(text); Option b = int.ParseOrNone(text); + Option c = DateTime.ParseExactOrNone(text, "O", null); + Option d = int.ParseOrNone(text.Trim()); } } """; @@ -254,6 +258,7 @@ public static void M(string text) { Option a = {|λ1201:text.ParseEnumOrNone(ignoreCase: true)|}; Option b = {|λ1201:text.ParseExactDateTimeOrNone("O", null)|}; + Option c = {|λ1201:text.ParseInt32OrNone()|}; } } """; @@ -273,6 +278,7 @@ public static void M(string text) { Option a = Color.ParseOrNone(text, ignoreCase: true); Option b = DateTime.ParseExactOrNone(text, "O", null); + Option c = int.ParseOrNone(text); } } """; diff --git a/Funcky.Analyzers/Funcky.Analyzers.Test/PreferXunitAssertTest.cs b/Funcky.Analyzers/Funcky.Analyzers.Test/PreferXunitAssertTest.cs index 29c89a5b..1a8ee8bb 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.Test/PreferXunitAssertTest.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.Test/PreferXunitAssertTest.cs @@ -110,6 +110,7 @@ public static void M(Option option) await VerifyCS.VerifyCodeFixAsync(inputCode + Environment.NewLine + StubCode, fixedCode + Environment.NewLine + StubCode); } + // Three or more diagnostics: Fix-All has to rewrite every call in one pass although each fix on its own adds the same using (#1009). [Fact] public async Task AddsTheUsingForTheExtensionWhenMissing() { @@ -121,7 +122,13 @@ public async Task AddsTheUsingForTheExtensionWhenMissing() public static class C { - public static void M(Option option) => {|λ1203:FunctionalAssert.None(option)|}; + public static void M(Option option) + { + {|λ1203:FunctionalAssert.None(option)|}; + {|λ1203:FunctionalAssert.Some(10, option)|}; + var value = {|λ1203:FunctionalAssert.Some(option)|}; + Assert.Equal(10, value); + } } """; const string fixedCode = @@ -133,7 +140,13 @@ public static class C public static class C { - public static void M(Option option) => Assert.None(option); + public static void M(Option option) + { + Assert.None(option); + Assert.Some(10, option); + var value = Assert.Some(option); + Assert.Equal(10, value); + } } """; diff --git a/changelog.md b/changelog.md index afbc8a3f..520f92fc 100644 --- a/changelog.md +++ b/changelog.md @@ -97,6 +97,10 @@ 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)). ## Funcky 3.7.0 | Funcky.Analyzers 2.0.0 From 567a03dcddb295f8bf6373d54832afd0cd861b70 Mon Sep 17 00:00:00 2001 From: Thomas Bruderer Date: Tue, 27 Oct 2026 18:41:41 +0100 Subject: [PATCH 2/2] Strip the replacement annotations from the fixed document MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-rewrite annotations that FunckyExtensionsImport uses to find the replacements stayed in the returned document. A host that keeps the syntax trees between two fixes (the test harness re-parses them) then handed them to the next fix on the same document: BooleanThen took a leftover parse replacement for one of its own Then calls, saw that it does not bind to Then and withheld the valid fix; a later fix needing the import could hit duplicate indices in AddUsings. ReplaceWithExtensionMemberCalls now strips the annotations before the document is returned; BooleanThen binds the annotated root and strips them afterwards. ExtensionMemberCodeFixTest applies a λ1201 fix and then the Option.FromBoolean fix to the changed solution directly, and PreferStaticParseOrNoneTest covers two namespace declarations that each get their own using. Co-Authored-By: Claude Fable 5.1 --- .../CodeAnalysisExtensions/BooleanThen.cs | 6 +- .../FunckyExtensionsImport.cs | 14 +- .../ExtensionMemberCodeFixTest.cs | 132 ++++++++++++++++++ .../PreferStaticParseOrNoneTest.cs | 61 ++++++++ 4 files changed, 208 insertions(+), 5 deletions(-) create mode 100644 Funcky.Analyzers/Funcky.Analyzers.Test/ExtensionMemberCodeFixTest.cs diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/BooleanThen.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/BooleanThen.cs index 32bf53a5..d5c08c40 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/BooleanThen.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/BooleanThen.cs @@ -56,11 +56,11 @@ public static bool IsThen(ISymbol? symbol, INamedTypeSymbol booleanExtensionsTyp return null; } - var changedDocument = FunckyExtensionsImport.ReplaceWithExtensionMemberCalls(document, root, semanticModel, rewrites, symbol => IsThen(symbol, booleanExtensionsType)); - var unboundRewrites = await UnboundRewritesAsync(changedDocument, rewrites.Length, cancellationToken).ConfigureAwait(false); + var annotatedRoot = FunckyExtensionsImport.ReplaceWithAnnotatedExtensionMemberCalls(root, semanticModel, rewrites, symbol => IsThen(symbol, booleanExtensionsType)); + var unboundRewrites = await UnboundRewritesAsync(document.WithSyntaxRoot(annotatedRoot), rewrites.Length, cancellationToken).ConfigureAwait(false); return unboundRewrites.IsEmpty - ? changedDocument + ? document.WithSyntaxRoot(FunckyExtensionsImport.WithoutReplacementAnnotations(annotatedRoot)) : await TryReplaceAllAsync(document, root, semanticModel, rewrites.Where((_, index) => !unboundRewrites.Contains(index)).ToImmutableArray(), booleanExtensionsType, cancellationToken).ConfigureAwait(false); } diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/FunckyExtensionsImport.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/FunckyExtensionsImport.cs index 39e24f5d..3b3975f6 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/FunckyExtensionsImport.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/FunckyExtensionsImport.cs @@ -21,6 +21,13 @@ public static Document ReplaceWithExtensionMemberCall(Document document, SyntaxN /// its diagnostic stays and is fixed by a subsequent pass. tells whether the symbol a /// replacement binds to without the using is the intended extension member. public static Document ReplaceWithExtensionMemberCalls(Document document, SyntaxNode root, SemanticModel semanticModel, ImmutableArray rewrites, Func bindsToExtensionMember) + => document.WithSyntaxRoot(WithoutReplacementAnnotations(ReplaceWithAnnotatedExtensionMemberCalls(root, semanticModel, rewrites, bindsToExtensionMember))); + + /// Like , but the replacements in the returned root still carry the annotations + /// that finds, so that a caller can bind them in the changed document before it strips them with + /// . A document handed to the host must not keep them: a later fix on the same document + /// would take them for its own. + public static SyntaxNode ReplaceWithAnnotatedExtensionMemberCalls(SyntaxNode root, SemanticModel semanticModel, ImmutableArray rewrites, Func bindsToExtensionMember) { var originals = rewrites.Select(rewrite => rewrite.Original).ToImmutableHashSet(); var outermostRewrites = rewrites @@ -34,13 +41,16 @@ public static Document ReplaceWithExtensionMemberCalls(Document document, Syntax .Select(pair => pair.Index) .ToImmutableArray(); - return document.WithSyntaxRoot(AddUsings(rootWithReplacements, replacementsInNeedOfImport)); + return AddUsings(rootWithReplacements, replacementsInNeedOfImport); } - /// The replacements in a root produced by , each with the index of the rewrite it stems from. + /// The replacements in a root produced by , each with the index of the rewrite it stems from. public static IEnumerable<(int Index, SyntaxNode Replacement)> Replacements(SyntaxNode root) => root.GetAnnotatedNodes(ReplacementAnnotationKind).Select(node => (Index(node), node)); + public static SyntaxNode WithoutReplacementAnnotations(SyntaxNode root) + => root.ReplaceNodes(root.GetAnnotatedNodes(ReplacementAnnotationKind), (_, replacement) => replacement.WithoutAnnotations(ReplacementAnnotationKind)); + private static SyntaxNode AddUsings(SyntaxNode root, ImmutableArray replacementsInNeedOfImport) { var anchors = Replacements(root) diff --git a/Funcky.Analyzers/Funcky.Analyzers.Test/ExtensionMemberCodeFixTest.cs b/Funcky.Analyzers/Funcky.Analyzers.Test/ExtensionMemberCodeFixTest.cs new file mode 100644 index 00000000..f6dabe83 --- /dev/null +++ b/Funcky.Analyzers/Funcky.Analyzers.Test/ExtensionMemberCodeFixTest.cs @@ -0,0 +1,132 @@ +using System.Collections.Immutable; +using Funcky.Analyzers.ExtensionForms; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Testing; +using Xunit; + +namespace Funcky.Analyzers.Test; + +/// Applies the fixes to the changed solution directly, the way a host that keeps the syntax trees does, where the test +/// harness re-parses every document between two fixes. +public sealed class ExtensionMemberCodeFixTest +{ + private const string ObsoleteMember = "CS0618"; + + // language=csharp + private const string InputCode = + """ + #nullable enable + using Funcky.Monads; + + public static class C + { + public static Option Parse(string text) => Funcky.Extensions.ParseExtensions.ParseInt32OrNone(text); + + public static Option Then(bool flag, int number) => Option.FromBoolean(flag, number); + } + + namespace Funcky + { + public readonly struct Unit + { + public static Unit Value => default; + } + } + + namespace Funcky.Monads + { + public readonly struct Option + where TItem : notnull + { + } + + public static class Option + { + [System.Obsolete("Use condition.Then(item) instead.")] + public static Option FromBoolean(bool boolean, TItem item) where TItem : notnull => default; + } + } + + namespace Funcky.Extensions + { + using Funcky.Monads; + + public static class ParseExtensions + { + public static Option ParseInt32OrNone(this string candidate) => default; + } + + public static class Int32ParseExtensions + { + extension(int) + { + public static Option ParseOrNone(string candidate) => default; + } + } + + public static class BooleanExtensions + { + public static Option Then(this bool condition, TItem value) where TItem : notnull => default; + } + } + """; + + [Fact] + public async Task AFixLeavesNoTraceThatConfusesALaterFixOnTheSameDocument() + { + using var workspace = new AdhocWorkspace(); + var document = await CreateDocumentAsync(workspace); + + var parseDiagnostic = Assert.Single(await GetAnalyzerDiagnosticsAsync(document, new PreferStaticParseOrNoneAnalyzer())); + var documentAfterParseFix = await ApplyFixAsync(document, new PreferStaticParseOrNoneCodeFix(), parseDiagnostic); + + var thenDiagnostic = Assert.Single(await GetCompilerDiagnosticsAsync(documentAfterParseFix, ObsoleteMember)); + var documentAfterThenFix = await ApplyFixAsync(documentAfterParseFix, new OptionFromBooleanCodeFix(), thenDiagnostic); + + var text = (await documentAfterThenFix.GetTextAsync()).ToString(); + Assert.Contains("int.ParseOrNone(text)", text, StringComparison.Ordinal); + Assert.Contains("flag.Then(number)", text, StringComparison.Ordinal); + Assert.Equal(1, text.Split("using Funcky.Extensions;").Length - 1); + } + + private static async Task CreateDocumentAsync(AdhocWorkspace workspace) + { + var references = await ReferenceAssemblies.Default.ResolveAsync(LanguageNames.CSharp, CancellationToken.None); + var project = workspace + .AddProject("Test", LanguageNames.CSharp) + .WithCompilationOptions(new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)) + .WithParseOptions(new CSharpParseOptions(LanguageVersion.Preview)) + .AddMetadataReferences(references); + + return project.AddDocument("Test.cs", InputCode); + } + + private static async Task> GetAnalyzerDiagnosticsAsync(Document document, DiagnosticAnalyzer analyzer) + { + var compilation = await document.Project.GetCompilationAsync() ?? throw new InvalidOperationException("Missing compilation"); + var syntaxTree = await document.GetSyntaxTreeAsync(); + var diagnostics = await compilation.WithAnalyzers([analyzer]).GetAnalyzerDiagnosticsAsync(); + return diagnostics.Where(diagnostic => diagnostic.Location.SourceTree == syntaxTree).ToImmutableArray(); + } + + private static async Task> GetCompilerDiagnosticsAsync(Document document, string id) + { + var semanticModel = await document.GetSemanticModelAsync() ?? throw new InvalidOperationException("Missing semantic model"); + return semanticModel.GetDiagnostics().Where(diagnostic => diagnostic.Id == id).ToImmutableArray(); + } + + private static async Task ApplyFixAsync(Document document, CodeFixProvider codeFix, Diagnostic diagnostic) + { + var actions = new List(); + await codeFix.RegisterCodeFixesAsync(new CodeFixContext(document, diagnostic, (action, _) => actions.Add(action), CancellationToken.None)); + var action = Assert.Single(actions); + var operations = await action.GetOperationsAsync(CancellationToken.None); + var changedSolution = Assert.Single(operations.OfType()).ChangedSolution; + + return changedSolution.GetDocument(document.Id) ?? throw new InvalidOperationException("Missing document"); + } +} diff --git a/Funcky.Analyzers/Funcky.Analyzers.Test/PreferStaticParseOrNoneTest.cs b/Funcky.Analyzers/Funcky.Analyzers.Test/PreferStaticParseOrNoneTest.cs index 050b29ed..4e5d3cb0 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.Test/PreferStaticParseOrNoneTest.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.Test/PreferStaticParseOrNoneTest.cs @@ -286,6 +286,67 @@ public static void M(string text) await VerifyCS.VerifyCodeFixAsync(inputCode + Environment.NewLine + StubCode, fixedCode + Environment.NewLine + StubCode); } + // Each namespace gets its own using: in one Fix-All pass, and when the fixes are applied one after the other to the + // same document, which must not be confused by traces of an earlier fix. + [Fact] + public async Task AddsTheUsingToEveryNamespaceDeclarationWithUsings() + { + const string inputCode = + """ + #nullable enable + namespace A + { + using System; + using Funcky.Monads; + + public static class C + { + public static Option M(string text) => {|λ1201:Funcky.Extensions.ParseExtensions.ParseInt32OrNone(text)|}; + } + } + + namespace B + { + using System; + using Funcky.Monads; + + public static class C + { + public static Option M(string text) => {|λ1201:Funcky.Extensions.ParseExtensions.ParseExactDateTimeOrNone(text, "O", null)|}; + } + } + """; + const string fixedCode = + """ + #nullable enable + namespace A + { + using System; + using Funcky.Monads; + using Funcky.Extensions; + + public static class C + { + public static Option M(string text) => int.ParseOrNone(text); + } + } + + namespace B + { + using System; + using Funcky.Monads; + using Funcky.Extensions; + + public static class C + { + public static Option M(string text) => DateTime.ParseExactOrNone(text, "O", null); + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(inputCode + Environment.NewLine + StubCode, fixedCode + Environment.NewLine + StubCode); + } + [Fact] public async Task DoesNotReportWithoutAStaticCounterpartWhenTheNamespaceIsNotImported() {