From f0317fde51fc0c572dbce91a5a6c3efd735b79de Mon Sep 17 00:00:00 2001 From: Thomas Bruderer Date: Mon, 19 Oct 2026 17:03:04 +0200 Subject: [PATCH 1/3] =?UTF-8?q?Add=20analyzer=20=CE=BB1018:=20prefer=20Fla?= =?UTF-8?q?tten=20over=20SelectMany=20with=20the=20identity=20function?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SelectMany(x => x) and SelectMany(Identity) on a nested Option, Result, Either or IEnumerable collapse one level of nesting; Flatten() does the same and says so by name. The lambda may return its parameter through an implicit reference conversion (IEnumerable), which Flatten accepts through the same covariance. The result-selector overload, query syntax, the static call form and expression trees are not reported. The code fix replaces the call and adds `using Funcky.Monads;` or `using Funcky.Extensions;` when the extension does not bind yet. The using helper takes the file's own line ending for a file without usings. Dogfooding: Sequence.Concat, CycleMaterialized, RepeatMaterialized and a test use Flatten; the four Flatten implementations suppress the rule. Co-Authored-By: Claude Fable 5.1 --- Documentation/src/SUMMARY.md | 1 + .../src/analyzer-rules/analyzer-rules.md | 1 + .../src/analyzer-rules/\316\2731018.md" | 42 ++++ .../CodeAnalysisExtensions/UsingDirectives.cs | 33 +++ .../Flatten/PreferFlattenCodeFix.cs | 59 +++++ .../Flatten/PreferFlattenTest.cs | 208 ++++++++++++++++++ .../AnalyzerReleases.Unshipped.md | 1 + .../Flatten/PreferFlattenAnalyzer.cs | 133 +++++++++++ .../EnumerableExtensions/SplitTest.cs | 2 +- .../EnumerableExtensions/Flatten.cs | 2 + Funcky/Monads/Either/EitherExtensions.cs | 2 + Funcky/Monads/Option/OptionExtensions.cs | 2 + Funcky/Monads/Result/ResultExtensions.cs | 2 + Funcky/Sequence/Sequence.Concat.cs | 2 +- Funcky/Sequence/Sequence.CycleMaterialized.cs | 2 +- .../Sequence/Sequence.RepeatMaterialized.cs | 2 +- changelog.md | 5 + 17 files changed, 495 insertions(+), 4 deletions(-) create mode 100644 "Documentation/src/analyzer-rules/\316\2731018.md" create mode 100644 Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/UsingDirectives.cs create mode 100644 Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Flatten/PreferFlattenCodeFix.cs create mode 100644 Funcky.Analyzers/Funcky.Analyzers.Test/Flatten/PreferFlattenTest.cs create mode 100644 Funcky.Analyzers/Funcky.Analyzers/Flatten/PreferFlattenAnalyzer.cs diff --git a/Documentation/src/SUMMARY.md b/Documentation/src/SUMMARY.md index 4227261f6..8a483f45f 100644 --- a/Documentation/src/SUMMARY.md +++ b/Documentation/src/SUMMARY.md @@ -50,6 +50,7 @@ * [λ1008](./analyzer-rules/λ1008.md) * [λ1009](./analyzer-rules/λ1009.md) * [λ1010](./analyzer-rules/λ1010.md) + * [λ1018](./analyzer-rules/λ1018.md) * [λ1101](./analyzer-rules/λ1101.md) [Changelog](changelog.md) diff --git a/Documentation/src/analyzer-rules/analyzer-rules.md b/Documentation/src/analyzer-rules/analyzer-rules.md index b56571295..8ce41cf87 100644 --- a/Documentation/src/analyzer-rules/analyzer-rules.md +++ b/Documentation/src/analyzer-rules/analyzer-rules.md @@ -32,6 +32,7 @@ Rules in the `λ11xx` range concern the `FunctionalAssert` helpers from the [`Fu | [λ1008](./λ1008.md) | Warning | Prefer `ToNullable` over `Match` | | [λ1009](./λ1009.md) | Error | Do not use `default` to instantiate this type | | [λ1010](./λ1010.md) | Error | An option has either zero or one elements | +| [λ1018](./λ1018.md) | Warning | Prefer `Flatten` over `SelectMany` with the identity function | | [λ1101](./λ1101.md) | Warning | Assert can be simplified | ## Removed rules diff --git "a/Documentation/src/analyzer-rules/\316\2731018.md" "b/Documentation/src/analyzer-rules/\316\2731018.md" new file mode 100644 index 000000000..6df15ede1 --- /dev/null +++ "b/Documentation/src/analyzer-rules/\316\2731018.md" @@ -0,0 +1,42 @@ +# λ1018: Prefer `Flatten` over `SelectMany` with the identity function +`SelectMany(x => x)` collapses one level of nesting. `Flatten()` does the same and says so by name. + +## Cause +`SelectMany` is called with the identity function, `x => x` or `Identity` from `Funcky.Functional`, on a nested +`Option>`, `Result>`, `Either>` or `IEnumerable>`. + +The overload with a result selector, query syntax (`from x in xs from y in x select y`) and `SelectMany` inside an +expression tree are not reported. + +## Reason for rule +A selector that returns its argument is noise: the reader has to look at the lambda to find out that it does nothing. +`Flatten` states the intent and needs no selector. λ1017 would otherwise turn the lambda into `SelectMany(Identity)`, +which is still a roundabout way of writing `Flatten()`. + +## How to fix violations +Replace the `SelectMany` call with `Flatten()`. The code fix does this and adds the `using Funcky.Monads;` or +`using Funcky.Extensions;` when the extension method is not yet in scope. + +## Examples + +### Disallowed +```cs +static Option Example(Option> option) + => option.SelectMany(x => x); + +static IEnumerable Example(IEnumerable> values) + => values.SelectMany(Identity); +``` + +### Allowed +```cs +static Option Example(Option> option) + => option.Flatten(); + +static IEnumerable Example(IEnumerable> values) + => values.Flatten(); + +// The selector does something. +static Option Example(Option> option) + => option.SelectMany(inner => inner.Select(x => x + 1)); +``` diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/UsingDirectives.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/UsingDirectives.cs new file mode 100644 index 000000000..99334ec35 --- /dev/null +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/UsingDirectives.cs @@ -0,0 +1,33 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory; + +namespace Funcky.Analyzers.CodeAnalysisExtensions; + +internal static class UsingDirectives +{ + /// Adds using <namespace>; next to the existing usings: into the namespace declaration enclosing + /// when the usings are there, otherwise at the top of the file. The + /// must be a node of . + public static SyntaxNode AddUsing(SyntaxNode root, SyntaxNode anchor, string namespaceName) + => anchor.FirstAncestorOrSelf(namespaceDeclaration => namespaceDeclaration.Usings.Count > 0) is { } namespaceDeclaration + ? root.ReplaceNode(namespaceDeclaration, namespaceDeclaration.AddUsings(UsingDirective(root, namespaceDeclaration.Usings, namespaceName))) + : ((CompilationUnitSyntax)root).AddUsings(UsingDirective(root, ((CompilationUnitSyntax)root).Usings, namespaceName)); + + /// The directive is parsed rather than built from tokens so that it carries no elastic trivia, which the formatter would otherwise + /// normalize together with the surrounding line endings. It takes its indentation and line ending from the last existing using. + private static UsingDirectiveSyntax UsingDirective(SyntaxNode root, SyntaxList usings, string namespaceName) + { + var usingDirective = ParseCompilationUnit($"using {namespaceName};").Usings.Single(); + return usings.LastOrDefault() is { } lastUsing + ? usingDirective.WithLeadingTrivia(lastUsing.GetLeadingTrivia()).WithTrailingTrivia(lastUsing.GetTrailingTrivia()) + : usingDirective.WithTrailingTrivia(EndOfLine(root), EndOfLine(root)); + } + + /// The line ending the file uses, so that the added directive does not introduce a mixed one. + private static SyntaxTrivia EndOfLine(SyntaxNode root) + => root.DescendantTrivia().FirstOrDefault(trivia => trivia.IsKind(SyntaxKind.EndOfLineTrivia)) is { RawKind: not 0 } endOfLine + ? endOfLine + : CarriageReturnLineFeed; +} diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Flatten/PreferFlattenCodeFix.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Flatten/PreferFlattenCodeFix.cs new file mode 100644 index 000000000..d5be7ce73 --- /dev/null +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Flatten/PreferFlattenCodeFix.cs @@ -0,0 +1,59 @@ +using System.Collections.Immutable; +using System.Composition; +using Funcky.Analyzers.CodeAnalysisExtensions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory; + +namespace Funcky.Analyzers.Flatten; + +/// Replaces SelectMany(x => x) with Flatten(), adding the using for the extension's namespace when needed. +[Shared] +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(PreferFlattenCodeFix))] +public sealed class PreferFlattenCodeFix : CodeFixProvider +{ + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(PreferFlattenAnalyzer.DiagnosticId); + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + 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 { Expression: MemberAccessExpressionSyntax memberAccess } invocation + && diagnostic.Properties.TryGetValue(PreferFlattenAnalyzer.FlattenNamespaceProperty, out var flattenNamespace) + && flattenNamespace is not null) + { + context.RegisterCodeFix( + CodeAction.Create( + $"Replace with '{PreferFlattenAnalyzer.FlattenMethodName}()'", + cancellationToken => ReplaceWithFlattenAsync(context.Document, invocation, memberAccess, flattenNamespace, cancellationToken), + nameof(PreferFlattenCodeFix)), + diagnostic); + } + } + } + + private static async Task ReplaceWithFlattenAsync(Document document, InvocationExpressionSyntax invocation, MemberAccessExpressionSyntax memberAccess, string flattenNamespace, CancellationToken cancellationToken) + { + if (await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false) is not { } root + || await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false) is not { } semanticModel) + { + return document; + } + + var flatten = invocation + .WithExpression(memberAccess.WithName(IdentifierName(PreferFlattenAnalyzer.FlattenMethodName).WithTriviaFrom(memberAccess.Name))) + .WithArgumentList(ArgumentList().WithTriviaFrom(invocation.ArgumentList)); + var newRoot = root.ReplaceNode(invocation, flatten); + + return document.WithSyntaxRoot(IsImported(semanticModel, invocation, flatten) ? newRoot : UsingDirectives.AddUsing(newRoot, newRoot.FindNode(invocation.Span), flattenNamespace)); + } + + private static bool IsImported(SemanticModel semanticModel, InvocationExpressionSyntax original, InvocationExpressionSyntax flatten) + => semanticModel.GetSpeculativeSymbolInfo(original.SpanStart, flatten, SpeculativeBindingOption.BindAsExpression) is { Symbol: IMethodSymbol } or { CandidateSymbols.IsEmpty: false }; +} diff --git a/Funcky.Analyzers/Funcky.Analyzers.Test/Flatten/PreferFlattenTest.cs b/Funcky.Analyzers/Funcky.Analyzers.Test/Flatten/PreferFlattenTest.cs new file mode 100644 index 000000000..2e477a1f0 --- /dev/null +++ b/Funcky.Analyzers/Funcky.Analyzers.Test/Flatten/PreferFlattenTest.cs @@ -0,0 +1,208 @@ +using Xunit; +using VerifyCS = Funcky.Analyzers.Test.CSharpCodeFixVerifier; + +namespace Funcky.Analyzers.Test.Flatten; + +public sealed class PreferFlattenTest +{ + private const string FunckyStubs = """ + + namespace Funcky + { + public static class Functional + { + public static T Identity(T x) => x; + } + } + + namespace Funcky.Monads + { + using System; + + public readonly struct Option + { + public Option SelectMany(Func> selector) => default; + public Option SelectMany(Func> selector, Func resultSelector) => default; + public Option Select(Func selector) => default; + } + + public readonly struct Result + { + public Result SelectMany(Func> selector) => default; + } + + public readonly struct Either + { + public Either SelectMany(Func> selector) => default; + } + + public static class OptionExtensions + { + public static Option Flatten(this Option> option) => default; + } + + public static class ResultExtensions + { + public static Result Flatten(this Result> result) => default; + } + + public static class EitherExtensions + { + public static Either Flatten(this Either> either) => default; + } + } + + namespace Funcky.Extensions + { + using System.Collections.Generic; + + public static class EnumerableExtensions + { + public static IEnumerable Flatten(this IEnumerable> enumerable) => default!; + } + } + """; + + [Fact] + public async Task SelectManyWithIdentityOnOptionIsReportedAndReplaced() + { + const string inputCode = """ + using Funcky.Monads; + using static Funcky.Functional; + + public static class C + { + public static Option Lambda(Option> option) => {|λ1018:option.SelectMany(x => x)|}; + + public static Option Block(Option> option) => {|λ1018:option.SelectMany(x => { return x; })|}; + + public static Option MethodGroup(Option> option) => {|λ1018:option.SelectMany(Identity)|}; + + public static Option Chained(Option>> option) => {|λ1018:{|λ1018:option.SelectMany(x => x)|}.SelectMany(x => x)|}; + } + """; + const string fixedCode = """ + using Funcky.Monads; + using static Funcky.Functional; + + public static class C + { + public static Option Lambda(Option> option) => option.Flatten(); + + public static Option Block(Option> option) => option.Flatten(); + + public static Option MethodGroup(Option> option) => option.Flatten(); + + public static Option Chained(Option>> option) => option.Flatten().Flatten(); + } + """; + await VerifyCS.VerifyCodeFixAsync(inputCode + FunckyStubs, fixedCode + FunckyStubs); + } + + [Fact] + public async Task SelectManyWithIdentityOnResultAndEitherIsReportedAndReplaced() + { + const string inputCode = """ + using Funcky.Monads; + + public static class C + { + public static Result R(Result> result) => {|λ1018:result.SelectMany(x => x)|}; + + public static Either E(Either> either) => {|λ1018:either.SelectMany(x => x)|}; + } + """; + const string fixedCode = """ + using Funcky.Monads; + + public static class C + { + public static Result R(Result> result) => result.Flatten(); + + public static Either E(Either> either) => either.Flatten(); + } + """; + await VerifyCS.VerifyCodeFixAsync(inputCode + FunckyStubs, fixedCode + FunckyStubs); + } + + [Fact] + public async Task SelectManyWithIdentityOnEnumerableIsReplacedAndTheUsingAdded() + { + const string inputCode = """ + using System.Collections.Generic; + using System.Linq; + + public static class C + { + public static IEnumerable M(IEnumerable> values) => {|λ1018:values.SelectMany(x => x)|}; + + public static IEnumerable N(List values) => {|λ1018:values.SelectMany(x => x)|}.Where(x => x > 0); + } + """; + const string fixedCode = """ + using System.Collections.Generic; + using System.Linq; + using Funcky.Extensions; + + public static class C + { + public static IEnumerable M(IEnumerable> values) => values.Flatten(); + + public static IEnumerable N(List values) => values.Flatten().Where(x => x > 0); + } + """; + await VerifyCS.VerifyCodeFixAsync(inputCode + FunckyStubs, fixedCode + FunckyStubs); + } + + [Fact] + public async Task UsingIsAddedForTheMonadNamespaceWhenTheTypeIsFullyQualified() + { + const string inputCode = """ + public static class C + { + public static Funcky.Monads.Option M(Funcky.Monads.Option> option) => {|λ1018:option.SelectMany(x => x)|}; + } + """; + const string fixedCode = """ + using Funcky.Monads; + + public static class C + { + public static Funcky.Monads.Option M(Funcky.Monads.Option> option) => option.Flatten(); + } + """; + await VerifyCS.VerifyCodeFixAsync(inputCode + FunckyStubs, fixedCode + FunckyStubs); + } + + [Fact] + public async Task OtherSelectorsAndFormsAreNotReported() + { + const string inputCode = """ + using System; + using System.Collections.Generic; + using System.Linq; + using System.Linq.Expressions; + using Funcky.Monads; + + public static class C + { + public static Option Selector(Option> option) => option.SelectMany(x => x.Select(y => y + 1)); + + public static Option TwoSelectors(Option> option) => option.SelectMany(x => x, (x, y) => y); + + public static Option Query(Option> option) => from x in option from y in x select y; + + public static IEnumerable Static(IEnumerable> values) => Enumerable.SelectMany(values, x => x); + + public static IEnumerable Indexed(IEnumerable> values) => values.SelectMany((x, _) => x); + + public static Expression>, IEnumerable>> Tree() => values => values.SelectMany(x => x); + + public static Option NotNested(Option option) => option.SelectMany(x => Some(x)); + + private static Option Some(int x) => default; + } + """; + await VerifyCS.VerifyAnalyzerAsync(inputCode + FunckyStubs); + } +} diff --git a/Funcky.Analyzers/Funcky.Analyzers/AnalyzerReleases.Unshipped.md b/Funcky.Analyzers/Funcky.Analyzers/AnalyzerReleases.Unshipped.md index ac2a3b551..77d8d81d1 100644 --- a/Funcky.Analyzers/Funcky.Analyzers/AnalyzerReleases.Unshipped.md +++ b/Funcky.Analyzers/Funcky.Analyzers/AnalyzerReleases.Unshipped.md @@ -5,4 +5,5 @@ Rule ID | Category | Severity | Notes --------|----------|----------|------- λ1010 | Funcky | Error | OptionListPatternAnalyzer +λ1018 | Funcky | Warning | PreferFlattenAnalyzer λ1101 | Funcky | Warning | FunctionalAssertAnalyzer diff --git a/Funcky.Analyzers/Funcky.Analyzers/Flatten/PreferFlattenAnalyzer.cs b/Funcky.Analyzers/Funcky.Analyzers/Flatten/PreferFlattenAnalyzer.cs new file mode 100644 index 000000000..007cbd289 --- /dev/null +++ b/Funcky.Analyzers/Funcky.Analyzers/Flatten/PreferFlattenAnalyzer.cs @@ -0,0 +1,133 @@ +using System.Collections.Immutable; +using Funcky.Analyzers.CodeAnalysisExtensions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; +using static Funcky.Analyzers.Functions.AnonymousFunctionMatching; +using static Funcky.Analyzers.Functions.IdentityFunctionMatching; + +namespace Funcky.Analyzers.Flatten; + +/// Reports SelectMany(x => x) and SelectMany(Identity) on a nested Option, Result, +/// Either or IEnumerable: that is Flatten(). +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class PreferFlattenAnalyzer : DiagnosticAnalyzer +{ + public const string DiagnosticId = $"{DiagnosticName.Prefix}{DiagnosticName.Usage}18"; + + public const string FlattenNamespaceProperty = nameof(FlattenNamespaceProperty); + + public const string FlattenMethodName = "Flatten"; + + private static readonly DiagnosticDescriptor Descriptor = new( + id: DiagnosticId, + title: "Prefer Flatten over SelectMany with the identity function", + messageFormat: "Prefer 'Flatten' over 'SelectMany' with the identity function", + category: nameof(Funcky), + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "SelectMany with the identity function collapses one level of nesting. Flatten does the same and says so by name, without a selector to read.", + helpLinkUri: DiagnosticName.HelpLink(DiagnosticId)); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Descriptor); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private static void OnCompilationStart(CompilationStartAnalysisContext context) + { + var flattenHolders = FlattenHolders.Collect(context.Compilation); + + if (!flattenHolders.IsEmpty) + { + context.RegisterOperationAction(AnalyzeInvocation(flattenHolders, context.Compilation.GetExpressionOfTType()), OperationKind.Invocation); + } + } + + private static Action AnalyzeInvocation(FlattenHolders flattenHolders, INamedTypeSymbol? expressionOfTType) + => context => + { + var invocation = (IInvocationOperation)context.Operation; + + if (invocation.TargetMethod.Name == FunckyWellKnownMemberNames.SelectManyMethodName + && MatchSelector(invocation) is { } selector + && (IsIdentityFunction(selector.Value) || IsIdentityFunctionWithReferenceConversion(selector.Value)) + && flattenHolders.FindNamespace(ReceiverType(invocation)) is { } flattenNamespace + && !invocation.Syntax.IsInExpressionTree(invocation.SemanticModel!, expressionOfTType, context.CancellationToken)) + { + context.ReportDiagnostic(Diagnostic.Create( + Descriptor, + invocation.Syntax.GetLocation(), + ImmutableDictionary.Empty.Add(FlattenNamespaceProperty, flattenNamespace))); + } + }; + + /// The single selector of a SelectMany called in member form: the instance method of the monads takes one + /// parameter, the LINQ extension method takes the receiver as its first parameter. + private static IArgumentOperation? MatchSelector(IInvocationOperation invocation) + => invocation.Syntax is InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax, ArgumentList.Arguments: [_] } + ? invocation switch + { + { TargetMethod: { IsExtensionMethod: false, Parameters.Length: 1 }, Arguments: [var selector] } => selector, + { TargetMethod: { IsExtensionMethod: true, Parameters.Length: 2 }, Arguments: [_, var selector] } => selector, + _ => null, + } + : null; + + /// x => x on an IEnumerable<int[]> returns the parameter through an implicit reference conversion + /// (array to IEnumerable<int>); Flatten accepts the receiver through the same covariance. + private static bool IsIdentityFunctionWithReferenceConversion(IOperation operation) + => operation is IDelegateCreationOperation { Target: IAnonymousFunctionOperation anonymousFunction } + && MatchAnonymousUnaryFunctionWithSingleReturn(anonymousFunction) is [var returnOperation] + && returnOperation.ReturnedValue is IConversionOperation { IsImplicit: true, Conversion.IsReference: true, Operand: IParameterReferenceOperation { Parameter.ContainingSymbol: var parameterContainingSymbol } } + && SymbolEquals(parameterContainingSymbol, anonymousFunction.Symbol); + + private static ITypeSymbol? ReceiverType(IInvocationOperation invocation) + => invocation switch + { + { Instance: { } instance } => instance.Type, + { Arguments: [{ Value: var receiver }, ..] } => receiver.Type, + _ => null, + }; + + /// The types that have a Flatten extension method, keyed by the original definition of the receiver type, + /// together with the namespace the extension lives in. + private sealed class FlattenHolders + { + private readonly ImmutableArray<(INamedTypeSymbol ReceiverType, string Namespace)> _holders; + + private FlattenHolders(ImmutableArray<(INamedTypeSymbol ReceiverType, string Namespace)> holders) + => _holders = holders; + + public bool IsEmpty => _holders.IsEmpty; + + public static FlattenHolders Collect(Compilation compilation) + => new(ImmutableArray.CreateRange( + new (INamedTypeSymbol? ReceiverType, INamedTypeSymbol? Holder)[] + { + (compilation.GetOptionOfTType(), compilation.GetOptionExtensionsType()), + (compilation.GetResultOfTType(), compilation.GetTypeByMetadataName("Funcky.Monads.ResultExtensions")), + (compilation.GetEitherOfTType(), compilation.GetTypeByMetadataName("Funcky.Monads.EitherExtensions")), + (compilation.GetSpecialType(SpecialType.System_Collections_Generic_IEnumerable_T), compilation.GetEnumerableExtensionType()), + } + .Where(pair => pair is { ReceiverType: not null, Holder: { } holder } && HasFlatten(holder)) + .Select(pair => (pair.ReceiverType!, pair.Holder!.ContainingNamespace.ToDisplayString())))); + + /// The receiver is the holder's type itself or, for IEnumerable<T>, any type implementing it. + public string? FindNamespace(ITypeSymbol? receiverType) + => receiverType is null + ? null + : _holders + .Where(holder => receiverType.AllInterfaces.Prepend(receiverType).Any(type => SymbolEquals(holder.ReceiverType, type.OriginalDefinition))) + .Select(holder => holder.Namespace) + .FirstOrDefault(); + + private static bool HasFlatten(INamedTypeSymbol holder) + => holder.GetMembers(FlattenMethodName).Any(member => member is IMethodSymbol { IsExtensionMethod: true }); + } +} diff --git a/Funcky.Test/Extensions/EnumerableExtensions/SplitTest.cs b/Funcky.Test/Extensions/EnumerableExtensions/SplitTest.cs index 8f7f113e8..d0f07084f 100644 --- a/Funcky.Test/Extensions/EnumerableExtensions/SplitTest.cs +++ b/Funcky.Test/Extensions/EnumerableExtensions/SplitTest.cs @@ -37,7 +37,7 @@ public Property JoiningThePartsWithTheSeparatorRestoresTheSource(List sourc { var parts = source.Split(separator); - return source.SequenceEqual(parts.Intersperse([separator]).SelectMany(Identity)).ToProperty(); + return source.SequenceEqual(parts.Intersperse([separator]).Flatten()).ToProperty(); } [Fact] diff --git a/Funcky/Extensions/EnumerableExtensions/Flatten.cs b/Funcky/Extensions/EnumerableExtensions/Flatten.cs index ff6f5091c..f9bbbb105 100644 --- a/Funcky/Extensions/EnumerableExtensions/Flatten.cs +++ b/Funcky/Extensions/EnumerableExtensions/Flatten.cs @@ -4,5 +4,7 @@ public static partial class EnumerableExtensions { [Pure] public static IEnumerable Flatten(this IEnumerable> enumerable) +#pragma warning disable λ1018 // This is the implementation of Flatten. => enumerable.SelectMany(Identity); +#pragma warning restore λ1018 } diff --git a/Funcky/Monads/Either/EitherExtensions.cs b/Funcky/Monads/Either/EitherExtensions.cs index b39ea20f4..43fee1445 100644 --- a/Funcky/Monads/Either/EitherExtensions.cs +++ b/Funcky/Monads/Either/EitherExtensions.cs @@ -6,7 +6,9 @@ public static partial class EitherExtensions public static Either Flatten(this Either> either) where TLeft : notnull where TRight : notnull +#pragma warning disable λ1018 // This is the implementation of Flatten. => either.SelectMany(Identity); +#pragma warning restore λ1018 /// Returns the left value or if the is a right value. [Pure] diff --git a/Funcky/Monads/Option/OptionExtensions.cs b/Funcky/Monads/Option/OptionExtensions.cs index 3de0b0492..441d8db04 100644 --- a/Funcky/Monads/Option/OptionExtensions.cs +++ b/Funcky/Monads/Option/OptionExtensions.cs @@ -7,7 +7,9 @@ public static partial class OptionExtensions [Pure] public static Option Flatten(this Option> option) where T : notnull +#pragma warning disable λ1018 // This is the implementation of Flatten. => option.SelectMany(Identity); +#pragma warning restore λ1018 [Pure] public static Either ToEither(this Option option, TLeft left) diff --git a/Funcky/Monads/Result/ResultExtensions.cs b/Funcky/Monads/Result/ResultExtensions.cs index b929c30c5..93ad1717d 100644 --- a/Funcky/Monads/Result/ResultExtensions.cs +++ b/Funcky/Monads/Result/ResultExtensions.cs @@ -4,5 +4,7 @@ public static partial class ResultExtensions { public static Result Flatten(this Result> result) where T : notnull +#pragma warning disable λ1018 // This is the implementation of Flatten. => result.SelectMany(Identity); +#pragma warning restore λ1018 } diff --git a/Funcky/Sequence/Sequence.Concat.cs b/Funcky/Sequence/Sequence.Concat.cs index f2bb1c654..fea26d1f0 100644 --- a/Funcky/Sequence/Sequence.Concat.cs +++ b/Funcky/Sequence/Sequence.Concat.cs @@ -15,5 +15,5 @@ public static IEnumerable Concat(params IEnumerable[] [Pure] public static IEnumerable Concat(IEnumerable> sources) => sources - .SelectMany(Identity); + .Flatten(); } diff --git a/Funcky/Sequence/Sequence.CycleMaterialized.cs b/Funcky/Sequence/Sequence.CycleMaterialized.cs index 691b9c55e..e899d9a1e 100644 --- a/Funcky/Sequence/Sequence.CycleMaterialized.cs +++ b/Funcky/Sequence/Sequence.CycleMaterialized.cs @@ -12,6 +12,6 @@ public static partial class Sequence [Pure] public static IEnumerable CycleMaterialized(IReadOnlyCollection source) => source.Count > 0 - ? Cycle(source).SelectMany(Identity) + ? Cycle(source).Flatten() : throw new InvalidOperationException("you cannot cycle an empty enumerable"); } diff --git a/Funcky/Sequence/Sequence.RepeatMaterialized.cs b/Funcky/Sequence/Sequence.RepeatMaterialized.cs index 80e1e4db3..dc59eb16f 100644 --- a/Funcky/Sequence/Sequence.RepeatMaterialized.cs +++ b/Funcky/Sequence/Sequence.RepeatMaterialized.cs @@ -13,5 +13,5 @@ public static partial class Sequence /// Use if you need to cycle a lazy sequence. [Pure] public static IEnumerable RepeatMaterialized(IReadOnlyCollection source, int count) - => Enumerable.Repeat(source, count).SelectMany(Identity); + => Enumerable.Repeat(source, count).Flatten(); } diff --git a/changelog.md b/changelog.md index 9add34c90..00bcb7d7f 100644 --- a/changelog.md +++ b/changelog.md @@ -2,6 +2,11 @@ All notable changes to this project will be documented in this file. Funcky adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased: Funcky 3.7.0 | Funcky.Analyzers 1.5.0 + +### Analyzers +* New: rule `λ1018` reports `SelectMany(x => x)` and `SelectMany(Identity)` on a nested `Option`, `Result`, `Either` or + `IEnumerable` and suggests `Flatten()`, which says what happens without a selector to read. The code fix replaces the call. ## Funcky 3.6.1 | Funcky.Async 1.5.0 | Funcky.Xunit 2.2.0 | Funcky.Xunit.v3 1.0.0 | Funcky.Analyzers 1.4.2 From 099bd443a759cebf1bc13f0ef1c37bb88942e5f2 Mon Sep 17 00:00:00 2001 From: Thomas Bruderer Date: Tue, 20 Oct 2026 15:49:09 +0200 Subject: [PATCH 2/3] Insert usings outside conditional regions, with formatting trivia only, and behind leading #define directives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The directive-safe insertion of the λ1017 fix moves into the shared UsingDirectives helper, which copies only whitespace and line endings from its neighbour. In a file without usings a leading `#define` or `#undef` moves in front of the new using, since those directives must precede every token. Co-Authored-By: Claude Fable 5.1 --- .../CodeAnalysisExtensions/UsingDirectives.cs | 73 ++++++++++++++++--- .../Functions/PreferNamedFunctionCodeFix.cs | 44 +---------- 2 files changed, 66 insertions(+), 51 deletions(-) diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/UsingDirectives.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/UsingDirectives.cs index 99334ec35..1fcba4f56 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/UsingDirectives.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/CodeAnalysisExtensions/UsingDirectives.cs @@ -12,22 +12,75 @@ internal static class UsingDirectives /// must be a node of . public static SyntaxNode AddUsing(SyntaxNode root, SyntaxNode anchor, string namespaceName) => anchor.FirstAncestorOrSelf(namespaceDeclaration => namespaceDeclaration.Usings.Count > 0) is { } namespaceDeclaration - ? root.ReplaceNode(namespaceDeclaration, namespaceDeclaration.AddUsings(UsingDirective(root, namespaceDeclaration.Usings, namespaceName))) - : ((CompilationUnitSyntax)root).AddUsings(UsingDirective(root, ((CompilationUnitSyntax)root).Usings, namespaceName)); + ? root.ReplaceNode(namespaceDeclaration, Insert(namespaceDeclaration, ParseUsingDirective($"using {namespaceName};"))) + : Insert((CompilationUnitSyntax)root, ParseUsingDirective($"using {namespaceName};")); + + /// Inserts the directive after the last using that is not inside a #if region, so that it is not swallowed by the + /// condition; when every using is conditional it goes in front of them all. It takes the indentation and line ending of its + /// neighbour, but none of its comments or directives: copying a leading #if would open a second region without an #endif. + public static TContainer Insert(TContainer container, UsingDirectiveSyntax usingDirective) + where TContainer : SyntaxNode + { + var usings = container switch + { + CompilationUnitSyntax compilationUnit => compilationUnit.Usings, + BaseNamespaceDeclarationSyntax namespaceDeclaration => namespaceDeclaration.Usings, + _ => default, + }; + var endOfLine = EndOfLine(container); + + return (usings.LastOrDefault(existing => !IsInsideConditionalRegion(existing)), usings.FirstOrDefault()) switch + { + ({ } anchor, _) => container.InsertNodesAfter(anchor, [usingDirective.WithLeadingTrivia(FormattingOnly(anchor.GetLeadingTrivia())).WithTrailingTrivia(FormattingOnly(anchor.GetTrailingTrivia()))]), + (null, { } first) => container.InsertNodesBefore(first, [usingDirective.WithLeadingTrivia(FormattingOnly(first.GetLeadingTrivia())).WithTrailingTrivia(endOfLine)]), + (null, null) => container switch + { + CompilationUnitSyntax compilationUnit => (TContainer)(SyntaxNode)WithFirstUsing(compilationUnit, usingDirective.WithTrailingTrivia(endOfLine, endOfLine)), + BaseNamespaceDeclarationSyntax namespaceDeclaration => (TContainer)(SyntaxNode)namespaceDeclaration.AddUsings(usingDirective.WithTrailingTrivia(endOfLine, endOfLine)), + _ => container, + }, + }; + } /// The directive is parsed rather than built from tokens so that it carries no elastic trivia, which the formatter would otherwise - /// normalize together with the surrounding line endings. It takes its indentation and line ending from the last existing using. - private static UsingDirectiveSyntax UsingDirective(SyntaxNode root, SyntaxList usings, string namespaceName) + /// normalize together with the surrounding line endings. + public static UsingDirectiveSyntax ParseUsingDirective(string directive) + => ParseCompilationUnit(directive).Usings.Single(); + + // `#define` and `#undef` must precede every token, the using included. In a file without usings they lead the first + // member, so they (and whatever stands before them) move in front of the new using; an `#if` or a comment stays where it is. + private static CompilationUnitSyntax WithFirstUsing(CompilationUnitSyntax compilationUnit, UsingDirectiveSyntax usingDirective) { - var usingDirective = ParseCompilationUnit($"using {namespaceName};").Usings.Single(); - return usings.LastOrDefault() is { } lastUsing - ? usingDirective.WithLeadingTrivia(lastUsing.GetLeadingTrivia()).WithTrailingTrivia(lastUsing.GetTrailingTrivia()) - : usingDirective.WithTrailingTrivia(EndOfLine(root), EndOfLine(root)); + var firstToken = compilationUnit.GetFirstToken(includeZeroWidth: true); + var leadingTrivia = firstToken.LeadingTrivia; + var headerLength = leadingTrivia.Select((trivia, index) => (Trivia: trivia, Index: index)).Where(pair => pair.Trivia.IsKind(SyntaxKind.DefineDirectiveTrivia) || pair.Trivia.IsKind(SyntaxKind.UndefDirectiveTrivia)).Select(pair => pair.Index + 1).LastOrDefault(); + + return headerLength == 0 + ? compilationUnit.AddUsings(usingDirective) + : compilationUnit + .ReplaceToken(firstToken, firstToken.WithLeadingTrivia(leadingTrivia.Skip(headerLength))) + .AddUsings(usingDirective.WithLeadingTrivia(leadingTrivia.Take(headerLength))); } + // Counting the directives before the using: more `#if` than `#endif` means it sits inside a region. + private static bool IsInsideConditionalRegion(UsingDirectiveSyntax usingDirective) + => (usingDirective.Ancestors().LastOrDefault() ?? usingDirective) + .DescendantNodes(descendIntoTrivia: true) + .OfType() + .Where(directive => directive.SpanStart < usingDirective.SpanStart) + .Sum(directive => directive switch + { + IfDirectiveTriviaSyntax => 1, + EndIfDirectiveTriviaSyntax => -1, + _ => 0, + }) > 0; + + private static SyntaxTriviaList FormattingOnly(SyntaxTriviaList trivia) + => TriviaList(trivia.Where(t => t.IsKind(SyntaxKind.WhitespaceTrivia) || t.IsKind(SyntaxKind.EndOfLineTrivia))); + /// The line ending the file uses, so that the added directive does not introduce a mixed one. - private static SyntaxTrivia EndOfLine(SyntaxNode root) - => root.DescendantTrivia().FirstOrDefault(trivia => trivia.IsKind(SyntaxKind.EndOfLineTrivia)) is { RawKind: not 0 } endOfLine + private static SyntaxTrivia EndOfLine(SyntaxNode node) + => (node.Ancestors().LastOrDefault() ?? node).DescendantTrivia().FirstOrDefault(trivia => trivia.IsKind(SyntaxKind.EndOfLineTrivia)) is { RawKind: not 0 } endOfLine ? endOfLine : CarriageReturnLineFeed; } diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Functions/PreferNamedFunctionCodeFix.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Functions/PreferNamedFunctionCodeFix.cs index 9a4bb7378..d9b9f3e28 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Functions/PreferNamedFunctionCodeFix.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Functions/PreferNamedFunctionCodeFix.cs @@ -1,5 +1,6 @@ using System.Collections.Immutable; using System.Composition; +using Funcky.Analyzers.CodeAnalysisExtensions; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; @@ -109,53 +110,14 @@ private static SyntaxNode Replace(SemanticModel semanticModel, SyntaxNode root, (true, null) => WithUsingStatic((CompilationUnitSyntax)root.ReplaceNode(anonymousFunction, replacement), semanticModel, position: 0), }; - /// Inserts the directive after the last using that is not inside a #if region, so that it is not swallowed by the - /// condition; when every using is conditional it goes in front of them all. It takes the indentation and line ending of its neighbour. - /// The directive is parsed rather than built from tokens so that it carries no elastic trivia, which the formatter would otherwise - /// normalize together with the surrounding line endings. + /// The using static goes after the last using outside a #if region, see . private static TContainer WithUsingStatic(TContainer container, SemanticModel semanticModel, int position) where TContainer : SyntaxNode - { - var usings = container switch - { - CompilationUnitSyntax compilationUnit => compilationUnit.Usings, - BaseNamespaceDeclarationSyntax namespaceDeclaration => namespaceDeclaration.Usings, - _ => default, - }; - var usingDirective = ParseCompilationUnit($"using static {QualifiedFunctionalName(semanticModel, position)};").Usings.Single(); - - return (usings.LastOrDefault(usingDirective => !IsInsideConditionalRegion(usingDirective)), usings.FirstOrDefault()) switch - { - ({ } anchor, _) => container.InsertNodesAfter(anchor, [usingDirective.WithLeadingTrivia(FormattingOnly(anchor.GetLeadingTrivia())).WithTrailingTrivia(FormattingOnly(anchor.GetTrailingTrivia()))]), - (null, { } first) => container.InsertNodesBefore(first, [usingDirective.WithTrailingTrivia(CarriageReturnLineFeed)]), - (null, null) => container switch - { - CompilationUnitSyntax compilationUnit => (TContainer)(SyntaxNode)compilationUnit.AddUsings(usingDirective.WithTrailingTrivia(CarriageReturnLineFeed, CarriageReturnLineFeed)), - BaseNamespaceDeclarationSyntax namespaceDeclaration => (TContainer)(SyntaxNode)namespaceDeclaration.AddUsings(usingDirective.WithTrailingTrivia(CarriageReturnLineFeed, CarriageReturnLineFeed)), - _ => container, - }, - }; - } + => UsingDirectives.Insert(container, UsingDirectives.ParseUsingDirective($"using static {QualifiedFunctionalName(semanticModel, position)};")); // Inside a namespace that contains its own `Funcky` namespace a relative `Funcky.Functional` would bind to that one. private static string QualifiedFunctionalName(SemanticModel semanticModel, int position) => semanticModel.LookupNamespacesAndTypes(position, name: FunctionalNamespace).Any(symbol => symbol is not INamespaceSymbol { ContainingNamespace.IsGlobalNamespace: true }) ? $"global::{FunctionalNamespace}.{FunctionalTypeName}" : $"{FunctionalNamespace}.{FunctionalTypeName}"; - - // Counting the directives before the using: more `#if` than `#endif` means it sits inside a region. - private static bool IsInsideConditionalRegion(UsingDirectiveSyntax usingDirective) - => (usingDirective.Ancestors().LastOrDefault() ?? usingDirective) - .DescendantNodes(descendIntoTrivia: true) - .OfType() - .Where(directive => directive.SpanStart < usingDirective.SpanStart) - .Sum(directive => directive switch - { - IfDirectiveTriviaSyntax => 1, - EndIfDirectiveTriviaSyntax => -1, - _ => 0, - }) > 0; - - private static SyntaxTriviaList FormattingOnly(SyntaxTriviaList trivia) - => TriviaList(trivia.Where(t => t.IsKind(SyntaxKind.WhitespaceTrivia) || t.IsKind(SyntaxKind.EndOfLineTrivia))); } From 1cafb1ad2b7384e6b6104bd95750cf12cb7da537 Mon Sep 17 00:00:00 2001 From: Thomas Bruderer Date: Tue, 20 Oct 2026 15:49:10 +0200 Subject: [PATCH 3/3] =?UTF-8?q?Bind=20the=20=CE=BB1018=20fix=20to=20Funcky?= =?UTF-8?q?'s=20Flatten,=20keep=20explicit=20type=20arguments=20and=20matc?= =?UTF-8?q?h=20only=20the=20owning=20SelectMany?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The analyzer only reports the SelectMany of the monads and of System.Linq.Enumerable: ParallelEnumerable and Queryable have their own, which Flatten would turn into a plain enumerable. The rule uses the Funcky.Style category. - The fix keeps the last explicit type argument as `Flatten()`, tracks the replacement with an annotation instead of the old span, and is only offered when the rewritten call (with the using added if necessary) binds to Funcky's Flatten; a Flatten member of the receiver's own type leaves the call reported without a fix. Co-Authored-By: Claude Fable 5.1 --- .../src/analyzer-rules/\316\2731018.md" | 9 +- .../Flatten/PreferFlattenCodeFix.cs | 58 ++++-- .../Flatten/PreferFlattenTest.cs | 194 ++++++++++++++++++ .../Flatten/PreferFlattenAnalyzer.cs | 34 +-- 4 files changed, 262 insertions(+), 33 deletions(-) diff --git "a/Documentation/src/analyzer-rules/\316\2731018.md" "b/Documentation/src/analyzer-rules/\316\2731018.md" index 6df15ede1..4169db22a 100644 --- "a/Documentation/src/analyzer-rules/\316\2731018.md" +++ "b/Documentation/src/analyzer-rules/\316\2731018.md" @@ -3,7 +3,9 @@ ## Cause `SelectMany` is called with the identity function, `x => x` or `Identity` from `Funcky.Functional`, on a nested -`Option>`, `Result>`, `Either>` or `IEnumerable>`. +`Option>`, `Result>`, `Either>` or `IEnumerable>`. Only the `SelectMany` +of the monads and of `System.Linq.Enumerable` counts: `ParallelEnumerable` and `Queryable` have their own, which `Flatten` +would turn into a plain enumerable. The overload with a result selector, query syntax (`from x in xs from y in x select y`) and `SelectMany` inside an expression tree are not reported. @@ -15,7 +17,10 @@ which is still a roundabout way of writing `Flatten()`. ## How to fix violations Replace the `SelectMany` call with `Flatten()`. The code fix does this and adds the `using Funcky.Monads;` or -`using Funcky.Extensions;` when the extension method is not yet in scope. +`using Funcky.Extensions;` when the extension method is not yet in scope. Explicit type arguments select the element type +and are kept: `SelectMany, object>(x => x)` becomes `Flatten()`. The fix is only offered when +the rewritten call reaches Funcky's `Flatten`; a `Flatten` member of the receiver's own type would win over the extension +method, so there the call is reported without a fix. ## Examples diff --git a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Flatten/PreferFlattenCodeFix.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Flatten/PreferFlattenCodeFix.cs index d5be7ce73..088f40dde 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Flatten/PreferFlattenCodeFix.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Flatten/PreferFlattenCodeFix.cs @@ -9,7 +9,9 @@ namespace Funcky.Analyzers.Flatten; -/// Replaces SelectMany(x => x) with Flatten(), adding the using for the extension's namespace when needed. +/// Replaces SelectMany(x => x) with Flatten(), adding the using for the extension's namespace when needed. +/// The fix is only offered when the rewritten call binds to Funcky's Flatten: a Flatten member of the receiver's own +/// type wins over any extension method and would silently redirect the call. [Shared] [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(PreferFlattenCodeFix))] public sealed class PreferFlattenCodeFix : CodeFixProvider @@ -20,40 +22,64 @@ public sealed class PreferFlattenCodeFix : CodeFixProvider public override async Task RegisterCodeFixesAsync(CodeFixContext context) { - var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root + || await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false) is not { } semanticModel) + { + return; + } foreach (var diagnostic in context.Diagnostics) { - if (root?.FindInvocationExpression(diagnostic.Location.SourceSpan) is { Expression: MemberAccessExpressionSyntax memberAccess } invocation + if (root.FindInvocationExpression(diagnostic.Location.SourceSpan) is { Expression: MemberAccessExpressionSyntax memberAccess } invocation && diagnostic.Properties.TryGetValue(PreferFlattenAnalyzer.FlattenNamespaceProperty, out var flattenNamespace) - && flattenNamespace is not null) + && flattenNamespace is not null + && await ReplaceWithFlattenAsync(context.Document, root, semanticModel, invocation, memberAccess, flattenNamespace, context.CancellationToken).ConfigureAwait(false) is { } fixedDocument) { context.RegisterCodeFix( CodeAction.Create( $"Replace with '{PreferFlattenAnalyzer.FlattenMethodName}()'", - cancellationToken => ReplaceWithFlattenAsync(context.Document, invocation, memberAccess, flattenNamespace, cancellationToken), + _ => Task.FromResult(fixedDocument), nameof(PreferFlattenCodeFix)), diagnostic); } } } - private static async Task ReplaceWithFlattenAsync(Document document, InvocationExpressionSyntax invocation, MemberAccessExpressionSyntax memberAccess, string flattenNamespace, CancellationToken cancellationToken) + /// The call is tried as is and, when that does not reach Funcky's Flatten, once more with the using added. The + /// using alone does not guarantee the binding (an instance member named Flatten still wins), so the result is checked in the + /// rewritten document; without a successful binding there is no fix. + private static async Task ReplaceWithFlattenAsync(Document document, SyntaxNode root, SemanticModel semanticModel, InvocationExpressionSyntax invocation, MemberAccessExpressionSyntax memberAccess, string flattenNamespace, CancellationToken cancellationToken) { - if (await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false) is not { } root - || await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false) is not { } semanticModel) + var annotation = new SyntaxAnnotation(); + var flatten = invocation + .WithExpression(memberAccess.WithName(FlattenName(memberAccess.Name))) + .WithArgumentList(ArgumentList().WithTriviaFrom(invocation.ArgumentList)) + .WithAdditionalAnnotations(annotation); + + if (IsFunckyFlatten(semanticModel.GetSpeculativeSymbolInfo(invocation.SpanStart, flatten, SpeculativeBindingOption.BindAsExpression).Symbol, flattenNamespace)) { - return document; + return document.WithSyntaxRoot(root.ReplaceNode(invocation, flatten)); } - var flatten = invocation - .WithExpression(memberAccess.WithName(IdentifierName(PreferFlattenAnalyzer.FlattenMethodName).WithTriviaFrom(memberAccess.Name))) - .WithArgumentList(ArgumentList().WithTriviaFrom(invocation.ArgumentList)); - var newRoot = root.ReplaceNode(invocation, flatten); + var rootWithReplacement = root.ReplaceNode(invocation, flatten); + var fixedDocument = document.WithSyntaxRoot(UsingDirectives.AddUsing(rootWithReplacement, rootWithReplacement.GetAnnotatedNodes(annotation).Single(), flattenNamespace)); - return document.WithSyntaxRoot(IsImported(semanticModel, invocation, flatten) ? newRoot : UsingDirectives.AddUsing(newRoot, newRoot.FindNode(invocation.Span), flattenNamespace)); + return await fixedDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false) is { } fixedRoot + && await fixedDocument.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false) is { } fixedSemanticModel + && IsFunckyFlatten(fixedSemanticModel.GetSymbolInfo(fixedRoot.GetAnnotatedNodes(annotation).Single(), cancellationToken).Symbol, flattenNamespace) + ? fixedDocument + : null; } - private static bool IsImported(SemanticModel semanticModel, InvocationExpressionSyntax original, InvocationExpressionSyntax flatten) - => semanticModel.GetSpeculativeSymbolInfo(original.SpanStart, flatten, SpeculativeBindingOption.BindAsExpression) is { Symbol: IMethodSymbol } or { CandidateSymbols.IsEmpty: false }; + /// Explicit type arguments select the result's element type (SelectMany<IEnumerable<string>, object>(x => x) + /// yields IEnumerable<object>), so the last one is carried over: Flatten<object>(). Where Flatten has + /// another arity the call does not bind and there is no fix. + private static SimpleNameSyntax FlattenName(SimpleNameSyntax selectManyName) + => selectManyName is GenericNameSyntax { TypeArgumentList.Arguments: [.., var resultType] } + ? GenericName(Identifier(PreferFlattenAnalyzer.FlattenMethodName), TypeArgumentList(SingletonSeparatedList(resultType.WithoutTrivia()))).WithTriviaFrom(selectManyName) + : IdentifierName(PreferFlattenAnalyzer.FlattenMethodName).WithTriviaFrom(selectManyName); + + private static bool IsFunckyFlatten(ISymbol? symbol, string flattenNamespace) + => symbol is IMethodSymbol { Name: PreferFlattenAnalyzer.FlattenMethodName, ReducedFrom: not null, ContainingNamespace: var containingNamespace } + && containingNamespace.ToDisplayString() == flattenNamespace; } diff --git a/Funcky.Analyzers/Funcky.Analyzers.Test/Flatten/PreferFlattenTest.cs b/Funcky.Analyzers/Funcky.Analyzers.Test/Flatten/PreferFlattenTest.cs index 2e477a1f0..cf97370b9 100644 --- a/Funcky.Analyzers/Funcky.Analyzers.Test/Flatten/PreferFlattenTest.cs +++ b/Funcky.Analyzers/Funcky.Analyzers.Test/Flatten/PreferFlattenTest.cs @@ -1,3 +1,4 @@ +using Microsoft.CodeAnalysis.Testing; using Xunit; using VerifyCS = Funcky.Analyzers.Test.CSharpCodeFixVerifier; @@ -205,4 +206,197 @@ public static class C """; await VerifyCS.VerifyAnalyzerAsync(inputCode + FunckyStubs); } + + [Fact] + public async Task ExplicitTypeArgumentsSelectTheElementTypeAndStay() + { + const string inputCode = """ + using System.Collections.Generic; + using System.Linq; + using Funcky.Extensions; + using Funcky.Monads; + + public static class C + { + public static List Enumerable(IEnumerable> values) => {|λ1018:values.SelectMany, object>(x => x)|}.ToList(); + + public static Option Option(Option> option) => {|λ1018:option.SelectMany(x => x)|}; + + public static Either Either(Either> either) => {|λ1018:either.SelectMany(x => x)|}; + } + """; + const string fixedCode = """ + using System.Collections.Generic; + using System.Linq; + using Funcky.Extensions; + using Funcky.Monads; + + public static class C + { + public static List Enumerable(IEnumerable> values) => values.Flatten().ToList(); + + public static Option Option(Option> option) => option.Flatten(); + + public static Either Either(Either> either) => {|λ1018:either.SelectMany(x => x)|}; + } + """; + + // The remaining diagnostic has no fix, which the default markup handling of the fixed state would hide. + await new VerifyCS.Test + { + TestCode = inputCode + FunckyStubs, + FixedCode = fixedCode + FunckyStubs, + FixedState = { MarkupHandling = MarkupMode.Allow }, + }.RunAsync(); + } + + [Fact] + public async Task UsingIsAddedWhenTheCallEndsTheFile() + { + const string inputCode = """ + public static class C + { + public static Funcky.Monads.Option M(Funcky.Monads.Option> option) => {|λ1018:option.SelectMany(x => x)|}; + } + """; + const string fixedCode = """ + using Funcky.Monads; + + public static class C + { + public static Funcky.Monads.Option M(Funcky.Monads.Option> option) => option.Flatten(); + } + """; + await new VerifyCS.Test + { + TestState = { Sources = { inputCode, FunckyStubs } }, + FixedState = { Sources = { fixedCode, FunckyStubs } }, + }.RunAsync(); + } + + [Fact] + public async Task UsingIsAddedAfterADefineDirective() + { + const string inputCode = """ + #define FEATURE + public static class C + { + public static Funcky.Monads.Option M(Funcky.Monads.Option> option) => {|λ1018:option.SelectMany(x => x)|}; + } + """; + const string fixedCode = """ + #define FEATURE + using Funcky.Monads; + + public static class C + { + public static Funcky.Monads.Option M(Funcky.Monads.Option> option) => option.Flatten(); + } + """; + await new VerifyCS.Test + { + TestState = { Sources = { inputCode, FunckyStubs } }, + FixedState = { Sources = { fixedCode, FunckyStubs } }, + }.RunAsync(); + } + + [Fact] + public async Task UsingIsAddedInsideARegionWithoutCopyingTheDirective() + { + const string inputCode = """ + #region Imports + using System.Linq; + #endregion + + public static class C + { + public static System.Collections.Generic.IEnumerable M(int[][] values) => {|λ1018:values.SelectMany(x => x)|}; + } + """; + const string fixedCode = """ + #region Imports + using System.Linq; + using Funcky.Extensions; + #endregion + + public static class C + { + public static System.Collections.Generic.IEnumerable M(int[][] values) => values.Flatten(); + } + """; + await VerifyCS.VerifyCodeFixAsync(inputCode + FunckyStubs, fixedCode + FunckyStubs); + } + + [Fact] + public async Task AFlattenMemberOfTheReceiverIsReportedWithoutFix() + { + const string inputCode = """ + using System.Collections.Generic; + using System.Linq; + + public static class C + { + public static IEnumerable Own(OwnFlatten values) => {|λ1018:values.SelectMany(x => x)|}; + + public static IEnumerable Inapplicable(InapplicableFlatten values) => {|λ1018:values.SelectMany(x => x)|}; + } + + public sealed class OwnFlatten : List + { + public IEnumerable Flatten() => new int[0]; + } + + public sealed class InapplicableFlatten : List + { + public IEnumerable Flatten(int unused) => new int[0]; + } + """; + const string fixedCode = """ + using System.Collections.Generic; + using System.Linq; + using Funcky.Extensions; + + public static class C + { + public static IEnumerable Own(OwnFlatten values) => {|λ1018:values.SelectMany(x => x)|}; + + public static IEnumerable Inapplicable(InapplicableFlatten values) => values.Flatten(); + } + + public sealed class OwnFlatten : List + { + public IEnumerable Flatten() => new int[0]; + } + + public sealed class InapplicableFlatten : List + { + public IEnumerable Flatten(int unused) => new int[0]; + } + """; + + // The remaining diagnostic has no fix, which the default markup handling of the fixed state would hide. + await new VerifyCS.Test + { + TestCode = inputCode + FunckyStubs, + FixedCode = fixedCode + FunckyStubs, + FixedState = { MarkupHandling = MarkupMode.Allow }, + }.RunAsync(); + } + + [Fact] + public async Task ParallelAndQueryableSelectManyAreNotReported() + { + const string inputCode = """ + using System.Collections.Generic; + using System.Linq; + + public static class C + { + public static void Parallel(IEnumerable> values) => values.AsParallel().SelectMany(x => x).ForAll(_ => { }); + + public static IQueryable Queryable(IQueryable> values) => values.SelectMany(x => x); + } + """; + await VerifyCS.VerifyAnalyzerAsync(inputCode + FunckyStubs); + } } diff --git a/Funcky.Analyzers/Funcky.Analyzers/Flatten/PreferFlattenAnalyzer.cs b/Funcky.Analyzers/Funcky.Analyzers/Flatten/PreferFlattenAnalyzer.cs index 007cbd289..bfaa885b5 100644 --- a/Funcky.Analyzers/Funcky.Analyzers/Flatten/PreferFlattenAnalyzer.cs +++ b/Funcky.Analyzers/Funcky.Analyzers/Flatten/PreferFlattenAnalyzer.cs @@ -24,7 +24,7 @@ public sealed class PreferFlattenAnalyzer : DiagnosticAnalyzer id: DiagnosticId, title: "Prefer Flatten over SelectMany with the identity function", messageFormat: "Prefer 'Flatten' over 'SelectMany' with the identity function", - category: nameof(Funcky), + category: DiagnosticName.Category, defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true, description: "SelectMany with the identity function collapses one level of nesting. Flatten does the same and says so by name, without a selector to read.", @@ -57,7 +57,7 @@ private static Action AnalyzeInvocation(FlattenHolders if (invocation.TargetMethod.Name == FunckyWellKnownMemberNames.SelectManyMethodName && MatchSelector(invocation) is { } selector && (IsIdentityFunction(selector.Value) || IsIdentityFunctionWithReferenceConversion(selector.Value)) - && flattenHolders.FindNamespace(ReceiverType(invocation)) is { } flattenNamespace + && flattenHolders.FindNamespace(ReceiverType(invocation), invocation.TargetMethod) is { } flattenNamespace && !invocation.Syntax.IsInExpressionTree(invocation.SemanticModel!, expressionOfTType, context.CancellationToken)) { context.ReportDiagnostic(Diagnostic.Create( @@ -95,34 +95,38 @@ private static bool IsIdentityFunctionWithReferenceConversion(IOperation operati _ => null, }; - /// The types that have a Flatten extension method, keyed by the original definition of the receiver type, - /// together with the namespace the extension lives in. + /// The types that have a Flatten extension method, keyed by the original definition of the receiver type and + /// the type whose SelectMany it replaces, together with the namespace the extension lives in. Only that SelectMany + /// counts: ParallelEnumerable and Queryable also have one over an enumerable receiver, but Flatten would + /// turn their query into a plain enumerable. private sealed class FlattenHolders { - private readonly ImmutableArray<(INamedTypeSymbol ReceiverType, string Namespace)> _holders; + private readonly ImmutableArray<(INamedTypeSymbol ReceiverType, INamedTypeSymbol SelectManyOwner, string Namespace)> _holders; - private FlattenHolders(ImmutableArray<(INamedTypeSymbol ReceiverType, string Namespace)> holders) + private FlattenHolders(ImmutableArray<(INamedTypeSymbol ReceiverType, INamedTypeSymbol SelectManyOwner, string Namespace)> holders) => _holders = holders; public bool IsEmpty => _holders.IsEmpty; public static FlattenHolders Collect(Compilation compilation) => new(ImmutableArray.CreateRange( - new (INamedTypeSymbol? ReceiverType, INamedTypeSymbol? Holder)[] + new (INamedTypeSymbol? ReceiverType, INamedTypeSymbol? SelectManyOwner, INamedTypeSymbol? Holder)[] { - (compilation.GetOptionOfTType(), compilation.GetOptionExtensionsType()), - (compilation.GetResultOfTType(), compilation.GetTypeByMetadataName("Funcky.Monads.ResultExtensions")), - (compilation.GetEitherOfTType(), compilation.GetTypeByMetadataName("Funcky.Monads.EitherExtensions")), - (compilation.GetSpecialType(SpecialType.System_Collections_Generic_IEnumerable_T), compilation.GetEnumerableExtensionType()), + (compilation.GetOptionOfTType(), compilation.GetOptionOfTType(), compilation.GetOptionExtensionsType()), + (compilation.GetResultOfTType(), compilation.GetResultOfTType(), compilation.GetTypeByMetadataName("Funcky.Monads.ResultExtensions")), + (compilation.GetEitherOfTType(), compilation.GetEitherOfTType(), compilation.GetTypeByMetadataName("Funcky.Monads.EitherExtensions")), + (compilation.GetSpecialType(SpecialType.System_Collections_Generic_IEnumerable_T), compilation.GetEnumerableType(), compilation.GetEnumerableExtensionType()), } - .Where(pair => pair is { ReceiverType: not null, Holder: { } holder } && HasFlatten(holder)) - .Select(pair => (pair.ReceiverType!, pair.Holder!.ContainingNamespace.ToDisplayString())))); + .Where(triple => triple is { ReceiverType: not null, SelectManyOwner: not null, Holder: { } holder } && HasFlatten(holder)) + .Select(triple => (triple.ReceiverType!, triple.SelectManyOwner!, triple.Holder!.ContainingNamespace.ToDisplayString())))); - /// The receiver is the holder's type itself or, for IEnumerable<T>, any type implementing it. - public string? FindNamespace(ITypeSymbol? receiverType) + /// The receiver is the holder's type itself or, for IEnumerable<T>, any type implementing it, and the + /// SelectMany is the one of the monad or of System.Linq.Enumerable. + public string? FindNamespace(ITypeSymbol? receiverType, IMethodSymbol selectMany) => receiverType is null ? null : _holders + .Where(holder => SymbolEquals(holder.SelectManyOwner, selectMany.ContainingType.OriginalDefinition)) .Where(holder => receiverType.AllInterfaces.Prepend(receiverType).Any(type => SymbolEquals(holder.ReceiverType, type.OriginalDefinition))) .Select(holder => holder.Namespace) .FirstOrDefault();