Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Improve formatting of lambda expressions #1066

Merged
merged 3 commits into from
Dec 12, 2023
Merged

feat: Improve formatting of lambda expressions #1066

merged 3 commits into from
Dec 12, 2023

Conversation

Rudomitori
Copy link
Contributor

@Rudomitori
Copy link
Contributor Author

Examples from #672

#672 (comment)

Source Formatted
builder.Entity<IdentityUserToken<string>>(b =>
{
    b.HasKey(
        l =>
            new
            {
                l.UserId,
                l.LoginProvider,
                l.Name
            }
    );
    b.ToTable("AspNetUserTokens");
});

table.PrimaryKey(
    "PK_AspNetUserTokens",
    x =>
        new
        {
            x.UserId,
            x.LoginProvider,
            x.Name
        }
);
builder.Entity<IdentityUserToken<string>>(b =>
{
    b.HasKey(l => new
    {
        l.UserId,
        l.LoginProvider,
        l.Name
    });
    b.ToTable("AspNetUserTokens");
});

table.PrimaryKey(
    "PK_AspNetUserTokens",
    x => new
    {
        x.UserId,
        x.LoginProvider,
        x.Name
    }
);

#672 (comment)

Source Formatted
builder.Entity<EntityWithThreeProperties>(e =>
{
    e.HasIndex(
        t =>
            new
            {
                t.X,
                t.Y,
                t.Z
            },
        "IX_unspecified"
    );
    e.HasIndex(
            t =>
                new
                {
                    t.X,
                    t.Y,
                    t.Z
                },
            "IX_empty"
        )
        .IsDescending();
    e.HasIndex(
            t =>
                new
                {
                    t.X,
                    t.Y,
                    t.Z
                },
            "IX_all_ascending"
        )
        .IsDescending(false, false, false);
    e.HasIndex(
            t =>
                new
                {
                    t.X,
                    t.Y,
                    t.Z
                },
            "IX_all_descending"
        )
        .IsDescending(true, true, true);
    e.HasIndex(
            t =>
                new
                {
                    t.X,
                    t.Y,
                    t.Z
                },
            "IX_mixed"
        )
        .IsDescending(false, true, false);
});
builder.Entity<EntityWithThreeProperties>(e =>
{
    e.HasIndex(
        t => new
        {
            t.X,
            t.Y,
            t.Z
        },
        "IX_unspecified"
    );
    e.HasIndex(
            t => new
            {
                t.X,
                t.Y,
                t.Z
            },
            "IX_empty"
        )
        .IsDescending();
    e.HasIndex(
            t => new
            {
                t.X,
                t.Y,
                t.Z
            },
            "IX_all_ascending"
        )
        .IsDescending(false, false, false);
    e.HasIndex(
            t => new
            {
                t.X,
                t.Y,
                t.Z
            },
            "IX_all_descending"
        )
        .IsDescending(true, true, true);
    e.HasIndex(
            t => new
            {
                t.X,
                t.Y,
                t.Z
            },
            "IX_mixed"
        )
        .IsDescending(false, true, false);
});

#672 (comment)

I think, it can be better

Source Formatted
class ClassName
{
    void MethodName()
    {
        var searchRequest = new SearchDescriptor<ElasticsearchProduct>().WithAggregations(
            o =>
                o.WithFilter(
                    key,
                    f =>
                        f.WithFilter(fd => filter)
                            .WithAggregations(
                                ad =>
                                    ad.WithTerm(
                                        key,
                                        cd =>
                                            cd.WithField(facetField).WithSize(MaximumCategoryFacets)
                                    )
                            )
                )
        );
    }
}
class ClassName
{
    void MethodName()
    {
        var searchRequest = new SearchDescriptor<ElasticsearchProduct>().WithAggregations(o =>
            o.WithFilter(
                key,
                f =>
                    f.WithFilter(fd => filter)
                        .WithAggregations(ad =>
                            ad.WithTerm(
                                key,
                                cd => cd.WithField(facetField).WithSize(MaximumCategoryFacets)
                            )
                        )
            )
        );
    }
}

@Rudomitori
Copy link
Contributor Author

Example from #766

Source Formatted
var affectedRows = await _dbContext.SomeEntities
    .Where(x => x.id == command.EntityId)
    .Where(authFilter)
    .ExecuteUpdateAsync(
        x => 
            x.SetProperty(x => x.Name, x => command.NewName)
                .SetProperty(x => x.Title, x => command.NewTItle)
                .SetProperty(x => x.Count, x => x.Command.NewCount)
    );
var affectedRows = await _dbContext
    .SomeEntities
    .Where(x => x.id == command.EntityId)
    .Where(authFilter)
    .ExecuteUpdateAsync(x =>
        x.SetProperty(x => x.Name, x => command.NewName)
            .SetProperty(x => x.Title, x => command.NewTItle)
            .SetProperty(x => x.Count, x => x.Command.NewCount)
    );

@Rudomitori
Copy link
Contributor Author

Examples from #836

#836 (comment)

Source Formatted
table.Where(
    t =>
        t.SomeLongColumn == Guid.Parse("a112c293-5a3a-ed11-9db1-000d3a24f3d4")
        || t.OtherLongColumn == Guid.Parse("a112c293-5a3a-ed11-9db1-000d3a24f3d4")
);
table.Where(t =>
    t.SomeLongColumn == Guid.Parse("a112c293-5a3a-ed11-9db1-000d3a24f3d4")
    || t.OtherLongColumn == Guid.Parse("a112c293-5a3a-ed11-9db1-000d3a24f3d4")
);

#836 (comment)

Source Formatted
await client
    .For<Country>()
    .Key(id)
    .Select(
        c =>
            new
            {
              c.Mim_CountryId,
              c.Mim_Name,
              c.Mim_Code
            }
    )
    .FindEntryAsync();
await client
    .For<Country>()
    .Key(id)
    .Select(c => new
    {
        c.Mim_CountryId,
        c.Mim_Name,
        c.Mim_Code
    })
    .FindEntryAsync();

@Rudomitori
Copy link
Contributor Author

Example from #451

Source Formatted
var y = someList.Where(
    o =>
        someLongValue_______________________
        && theseShouldNotIndent_________________
        && theseShouldNotIndent_________________
            > butThisOneShould_________________________________________
);
var y = someList.Where(o =>
    someLongValue_______________________
    && theseShouldNotIndent_________________
    && theseShouldNotIndent_________________
        > butThisOneShould_________________________________________
);

Copy link
Collaborator

@shocklateboy92 shocklateboy92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. The code looks mostly good, but I do have a minor reservation about the new formatting.

Please let me know what you think about the case I commented on.

@shocklateboy92 shocklateboy92 enabled auto-merge (squash) December 12, 2023 22:26
@shocklateboy92 shocklateboy92 merged commit 433ceee into belav:main Dec 12, 2023
3 checks passed
@jods4
Copy link

jods4 commented Dec 13, 2023

@Rudomitori Thank you! 🧡
This is a great improvement for code that has lots of lambdas.
I'm looking forward to the next release!

@jods4
Copy link

jods4 commented Dec 17, 2023

What happened to this? I don't see the merge commit on main anymore, did you force push a revert?

@Rudomitori
Copy link
Contributor Author

@belav, is there something wrong with this PR? I have free time these days, and can try to fix it, or at least discuss it

@belav
Copy link
Owner

belav commented Dec 17, 2023

@belav, is there something wrong with this PR? I have free time these days, and can try to fix it, or at least discuss it

Not at all, sorry I should have said something here and not just on discord. I pulled it out of main a couple of times now to release a 0.26.X version, I am trying to stick to only bug fix types changes until 0.27.0

I'll have it back in main shortly.

belav pushed a commit that referenced this pull request Dec 17, 2023
# Conflicts:
#	Src/CSharpier.Tests/FormattingTests/TestFiles/cs/MemberChains.test
@belav
Copy link
Owner

belav commented Dec 17, 2023

@Rudomitori I also ran this change against a repo full of test code, and it looks there is one bug it introduces, otherwise I agree that this is a great improvement!

private static readonly ModelBinderProviderCollection _providers =
    CreateDefaultCollection();

// formats as
private static readonly ModelBinderProviderCollection _providers = CreateDefaultCollection(

);

From https://github.com/belav/csharpier-repos/pull/96/files, there appear to be a few variations of this extra blank line, but they are probably all the same issue.

@Rudomitori
Copy link
Contributor Author

I've created an issue for the bugs introduced by this PR

shocklateboy92 pushed a commit that referenced this pull request Dec 20, 2023
* fix: Bug with empty argument list

* fix: Incorrect formatting of lambdas with nested lambdas

Group id used in ArgumentListLikeSyntax was not unique
and the nested lambda overrode the state of the parent lambda

* style: Use switch expression instead of statement

* fix: Second bug in 1077

Closes #1077
@belav belav added this to the 0.27.0 milestone Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants