diff --git a/Documentation/src/SUMMARY.md b/Documentation/src/SUMMARY.md index 3a07c8a02..bc9fa29c1 100644 --- a/Documentation/src/SUMMARY.md +++ b/Documentation/src/SUMMARY.md @@ -57,6 +57,7 @@ * [λ1015](./analyzer-rules/λ1015.md) * [λ1016](./analyzer-rules/λ1016.md) * [λ1017](./analyzer-rules/λ1017.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 85426d419..821ab97d7 100644 --- a/Documentation/src/analyzer-rules/analyzer-rules.md +++ b/Documentation/src/analyzer-rules/analyzer-rules.md @@ -53,6 +53,7 @@ contains a rule. | [λ1015](./λ1015.md) | Info | Fallback is computed eagerly | | [λ1016](./λ1016.md) | Warning | Pass the fallback value directly | | [λ1017](./λ1017.md) | Warning | Prefer the named function over the lambda | +| [λ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..4169db22a --- /dev/null +++ "b/Documentation/src/analyzer-rules/\316\2731018.md" @@ -0,0 +1,47 @@ +# λ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>`. 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. + +## 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. 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 + +### 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/Flatten/PreferFlattenCodeFix.cs b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Flatten/PreferFlattenCodeFix.cs new file mode 100644 index 000000000..088f40dde --- /dev/null +++ b/Funcky.Analyzers/Funcky.Analyzers.CodeFixes/Flatten/PreferFlattenCodeFix.cs @@ -0,0 +1,85 @@ +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. +/// 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 +{ + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(PreferFlattenAnalyzer.DiagnosticId); + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + if (await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root + || await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false) is not { } semanticModel) + { + return; + } + + 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 + && await ReplaceWithFlattenAsync(context.Document, root, semanticModel, invocation, memberAccess, flattenNamespace, context.CancellationToken).ConfigureAwait(false) is { } fixedDocument) + { + context.RegisterCodeFix( + CodeAction.Create( + $"Replace with '{PreferFlattenAnalyzer.FlattenMethodName}()'", + _ => Task.FromResult(fixedDocument), + nameof(PreferFlattenCodeFix)), + diagnostic); + } + } + } + + /// 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) + { + 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.WithSyntaxRoot(root.ReplaceNode(invocation, flatten)); + } + + var rootWithReplacement = root.ReplaceNode(invocation, flatten); + var fixedDocument = document.WithSyntaxRoot(UsingDirectives.AddUsing(rootWithReplacement, rootWithReplacement.GetAnnotatedNodes(annotation).Single(), 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; + } + + /// 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 new file mode 100644 index 000000000..cf97370b9 --- /dev/null +++ b/Funcky.Analyzers/Funcky.Analyzers.Test/Flatten/PreferFlattenTest.cs @@ -0,0 +1,402 @@ +using Microsoft.CodeAnalysis.Testing; +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); + } + + [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/AnalyzerReleases.Unshipped.md b/Funcky.Analyzers/Funcky.Analyzers/AnalyzerReleases.Unshipped.md index 732aee942..fec50314c 100644 --- a/Funcky.Analyzers/Funcky.Analyzers/AnalyzerReleases.Unshipped.md +++ b/Funcky.Analyzers/Funcky.Analyzers/AnalyzerReleases.Unshipped.md @@ -11,6 +11,7 @@ Rule ID | Category | Severity | Notes λ1015 | Funcky.Style | Info | EagerlyComputedFallbackAnalyzer λ1016 | Funcky.Style | Warning | TrivialLazyFallbackAnalyzer λ1017 | Funcky.Style | Warning | PreferNamedFunctionAnalyzer +λ1018 | Funcky.Style | Warning | PreferFlattenAnalyzer λ1101 | Funcky.Style | Warning | FunctionalAssertAnalyzer ### Removed Rules diff --git a/Funcky.Analyzers/Funcky.Analyzers/Flatten/PreferFlattenAnalyzer.cs b/Funcky.Analyzers/Funcky.Analyzers/Flatten/PreferFlattenAnalyzer.cs new file mode 100644 index 000000000..bfaa885b5 --- /dev/null +++ b/Funcky.Analyzers/Funcky.Analyzers/Flatten/PreferFlattenAnalyzer.cs @@ -0,0 +1,137 @@ +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: 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.", + 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), invocation.TargetMethod) 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 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, INamedTypeSymbol SelectManyOwner, 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? SelectManyOwner, INamedTypeSymbol? Holder)[] + { + (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(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, 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(); + + 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 686314761..a789508a5 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 1b74b7075..4fa5fe9ef 100644 --- a/changelog.md +++ b/changelog.md @@ -60,6 +60,8 @@ warnings (an info for pure performance hints), come with a code fix and carry th `null` with an `ArgumentNullException`, so `Option option = null;` and even `option == null` throw at run time; with nullable reference types disabled the compiler is silent about it. The code fix replaces the literal with `Option.None`. +* 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. * New: rule `λ1012` reports `FirstOrDefault`, `LastOrDefault`, `SingleOrDefault` and `ElementAtOrDefault` whose result is immediately tested for `null` (`?? fallback`, `?.`, `is null`, `is {} x`, `== null`, …) and suggests the `…OrNone` variant, which makes the missing element a `None` instead of a `null`. The code fix rewrites `xs.FirstOrDefault() ?? fallback`