Detect more fluent cases for converting collection expressions #75267

CyrusNajmabadi · 2024-09-26T20:39:56Z

CyrusNajmabadi · 2024-09-27T20:05:53Z

...nalyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs

-        using var _ = ArrayBuilder<CollectionExpressionMatch<ArgumentSyntax>>.GetInstance(out var matchesInReverse);
-        if (!AnalyzeInvocation(text, state, invocation, addMatches ? matchesInReverse : null, out var existingInitializer, cancellationToken))
+        using var _1 = ArrayBuilder<CollectionExpressionMatch<ArgumentSyntax>>.GetInstance(out var preMatchesInReverse);
+        using var _2 = ArrayBuilder<CollectionExpressionMatch<ArgumentSyntax>>.GetInstance(out var postMatchesInReverse);


the general idea here is that when converting to a collection expr, there is often:

values that should be inserted into the collection expr prior to the values in any existing initializer.

the values from an existing initializer. (note: the initializer is also used to help determine how to format/shape the collection, so we want to pass it along, not convert it into the matches-array)

values taht should be inserted into the collection expr after any values in the xisting initializer.

This PR updates everything to follow this model.

CyrusNajmabadi · 2024-09-27T20:07:08Z

...nalyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs

+                    }
+
+                    // Add the arguments to the pre-matches.  They will execute before the initializer values are added.
+                    AddArgumentsInReverse(preMatchesInReverse, objectCreation.ArgumentList.Arguments, useSpread: true);


new logic. if the user wrote new List<...>(someValuesToCopy) we'll create a collection expr starting with [.. someValuesToCopy. Any existing initializer values will be added after this. And any more fluent additions will go after that.

CyrusNajmabadi · 2024-09-27T20:07:31Z

src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs

@@ -6,7 +6,7 @@

 namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression;

-/// <inheritdoc cref="Match{TStatementSyntax}"/>
+/// <inheritdoc cref="Match"/>
 internal readonly record struct CollectionExpressionMatch<TMatchNode>(


note: we have a few match structs that are all about the same. I want to unify them in the future.

CyrusNajmabadi · 2024-09-27T20:08:18Z

...s/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs


-    private static async Task<ExpressionSyntax> GetNewObjectCreationAsync(


inlined this method.

CyrusNajmabadi · 2024-09-27T20:08:45Z

...CollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionInitializer.cs

@@ -23,7 +23,7 @@ internal partial class CSharpUseCollectionInitializerCodeFixProvider
 {
    private static BaseObjectCreationExpressionSyntax CreateObjectInitializerExpression(
        BaseObjectCreationExpressionSyntax objectCreation,
-        ImmutableArray<Match<StatementSyntax>> matches)


made non-generic. we wanted to be able to pass along either a Statement or Expression now, so i just typed this as SyntaxNode.

CyrusNajmabadi · 2024-09-27T20:09:12Z

...nalyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs

@@ -47,13 +47,18 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer<
        TVariableDeclaratorSyntax,
        TAnalyzer>, new()
 {
+    public readonly record struct AnalysisResult(
+        ImmutableArray<Match> PreMatches,
+        ImmutableArray<Match> PostMatches);


similarly, we have a few of these floating around. i'd like to get it down to 1.

CyrusNajmabadi · 2024-09-27T20:39:26Z

...Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs

@@ -57,7 +58,7 @@ private void AnalyzeArrayCreationExpression(SyntaxNodeAnalysisContext context, I
        ReportArrayCreationDiagnostics(context, syntaxTree, option.Notification, arrayCreationExpression, changesSemantics);
    }

-    public static ImmutableArray<CollectionExpressionMatch<StatementSyntax>> TryGetMatches(
+    public static ImmutableArray<CollectionMatch<StatementSyntax>> TryGetMatches(


renamed this and replace another type with this type.

CyrusNajmabadi · 2024-09-30T17:05:34Z

@akhera99 ptal.

ToddGrun · 2024-09-30T21:23:28Z

...nalyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs

@@ -187,7 +199,7 @@ private static bool AnalyzeInvocation(
            // left hand side of the expression.  In the inner expressions we can have things like `.Concat/.Append`
            // calls as the outer expressions will realize the collection.
            if (current is InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax currentMemberAccess } currentInvocation &&
-                IsMatch(state, currentMemberAccess, currentInvocation, allowLinq: true, matchesInReverse, out _, cancellationToken))
+                IsMatch(state, currentMemberAccess, currentInvocation, allowLinq: true, postMatchesInReverse, out _, cancellationToken))


IsMatch

I think this would be easier for me to read if the while loop ended after this if block

will make that chnage.

this got much nicer. thanks.

ToddGrun · 2024-09-30T21:53:18Z

src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs

            ArrayBuilder<SyntaxNodeOrToken> nodesAndTokens,
            string? preferredIndentation,
-            bool forceTrailingComma)
+            bool forceTrailingComma,
+            bool moreToCome)


moreToCome

nit: might be nice if this method and AddCommaIfMissing were consistent on using last or moreToCome

ToddGrun

CyrusNajmabadi added 2 commits September 26, 2024 13:36

Detect more fluent cases for converting collection expressions

755208d

Add workitem

ee2f69d

CyrusNajmabadi requested a review from a team as a code owner September 26, 2024 20:39

dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 26, 2024

dotnet-policy-service bot added the VSCode label Sep 26, 2024

CyrusNajmabadi added 7 commits September 26, 2024 13:42

Fix

4e8140f

in progress

fcc527e

Fixes

7d557bf

in progress

8d137de

in progress

16e62ef

in progress

630649e

Fix test

2cf3055

build-analysis bot mentioned this pull request Sep 27, 2024

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

3 tasks

CyrusNajmabadi added 14 commits September 27, 2024 11:34

generics

e5d4dea

in progress

5d9233e

in progress

d0b2618

Strange

2495ba8

in progress

80ef9d0

in progress

9762535

in progress

d6b32a3

in progress

4f0f6ba

Fix

dea5c81

in progress

072425f

Fix

0bcb997

Fix

24611ad

Fix test

80538fa

Fix test

c7304ec

CyrusNajmabadi commented Sep 27, 2024

View reviewed changes

CyrusNajmabadi added 3 commits September 27, 2024 13:28

unitfy

0ae4ccb

renames

9fbea8d

renames

e378dea

CyrusNajmabadi commented Sep 27, 2024

View reviewed changes

CyrusNajmabadi requested review from ToddGrun and akhera99 September 30, 2024 17:05

ToddGrun reviewed Sep 30, 2024

View reviewed changes

ToddGrun approved these changes Sep 30, 2024

View reviewed changes

CyrusNajmabadi merged commit e56f996 into dotnet:main Sep 30, 2024
25 checks passed

CyrusNajmabadi deleted the moreFluentCases branch September 30, 2024 22:16

dotnet-policy-service bot added this to the Next milestone Sep 30, 2024

This was referenced Oct 9, 2024

[Automated] PRs inserted in VS build feature.debugger.main-35407.82 #75445

Closed

[Automated] PRs inserted in VS build main-35407.57 #75446

Closed

[Automated] PRs inserted in VS build feature.debugger.shadowDebug-35410.215 #75481

Closed

akhera99 modified the milestones: Next, 17.13 P1 Oct 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect more fluent cases for converting collection expressions #75267

Detect more fluent cases for converting collection expressions #75267

CyrusNajmabadi commented Sep 26, 2024 •

edited

Loading

CyrusNajmabadi Sep 27, 2024

CyrusNajmabadi Sep 27, 2024

CyrusNajmabadi Sep 27, 2024

CyrusNajmabadi Sep 27, 2024

CyrusNajmabadi Sep 27, 2024

CyrusNajmabadi Sep 27, 2024

CyrusNajmabadi Sep 27, 2024

CyrusNajmabadi commented Sep 30, 2024

ToddGrun Sep 30, 2024

CyrusNajmabadi Sep 30, 2024

CyrusNajmabadi Sep 30, 2024

ToddGrun Sep 30, 2024

ToddGrun left a comment


		private static async Task<ExpressionSyntax> GetNewObjectCreationAsync(

Detect more fluent cases for converting collection expressions #75267

Detect more fluent cases for converting collection expressions #75267

Conversation

CyrusNajmabadi commented Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyrusNajmabadi commented Sep 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ToddGrun left a comment

Choose a reason for hiding this comment

CyrusNajmabadi commented Sep 26, 2024 •

edited

Loading