belav / csharpier

CSharpier is an opinionated code formatter for c#.
https://csharpier.com
MIT License
1.4k stars 97 forks source link

Avoid wrapping method arguments including a block-bodied lambda #1186

Open jcracknell opened 8 months ago

jcracknell commented 8 months ago

Input:

await SessionManager.Session(cancellationToken, async context =>
{
    var records = await context
        .DbContext.Entity.AsNoTracking()
        .Where(
            context.DbContext.PredicateFor(criteria).TranslatedBy(MkEntityPredicates)
        )
        .ToListAsync(context.CancellationToken);

    var entities = await MarshalEntities(records);

    return entities.ToImmutableDictionary(e => e.Id.Value);
});

Output:

await SessionManager.Session(
    cancellationToken,
    async context =>
    {
        var records = await context
            .DbContext.Entity.AsNoTracking()
            .Where(
                context.DbContext.PredicateFor(criteria).TranslatedBy(MkEntityPredicates)
            )
            .ToListAsync(context.CancellationToken);

        var entities = await MarshalEntities(records);

        return entities.ToImmutableDictionary(e => e.Id.Value);
    }
);

Expected behavior:

It would be nice if the formatter could avoid wrapping arguments to method calls where the call includes a block-bodied lambda on the assumption that the method call represents some form of flow control.

belav commented 8 months ago

FWIW prettier appears to avoid breaking arguments when one of them is a lambda. I haven't looked into the code to see if this was intenional or not, but I assume that it was.

Prettier output


await SessionManager.Session(cancellationToken, async (context) => {
  var records = await context.DbContext.Entity.AsNoTracking()
    .Where(
      context.DbContext.PredicateFor(criteria).TranslatedBy(MkEntityPredicates),
    )
    .ToListAsync(context.CancellationToken);

  var entities = await MarshalEntities(records);

  return entities.ToImmutableDictionary((e) => e.Id.Value);
});
jcracknell commented 8 months ago

Sorry, to be clear this is using prettier with prettier-plugin-csharp?

I'd be fine with wrapping arguments when they contain a lambda which is not block-bodied, e.g.

var exception = Assert.Throws<Exception>(
    () => my.Incredibly.Long.Ass.Lambda.Expression
);

but blocks should probably be treated as blocks regardless of where they appear:

var exception = Assert.Throws<Exception>(() =>
{
    throw new Exception();
});
Rudomitori commented 8 months ago

Some minimal API samples from the Microsoft's documentation

app.MapPost("/todoitems", async (Todo todo, TodoDb db) =>
{
    db.Todos.Add(todo);
    await db.SaveChangesAsync();

    return Results.Created($"/todoitems/{todo.Id}", todo);
});

app.MapPut("/todoitems/{id}", async (int id, Todo inputTodo, TodoDb db) =>
{
    var todo = await db.Todos.FindAsync(id);

    if (todo is null) return Results.NotFound();

    todo.Name = inputTodo.Name;
    todo.IsComplete = inputTodo.IsComplete;

    await db.SaveChangesAsync();

    return Results.NoContent();
});

app.MapDelete("/todoitems/{id}", async (int id, TodoDb db) =>
{
    if (await db.Todos.FindAsync(id) is Todo todo)
    {
        db.Todos.Remove(todo);
        await db.SaveChangesAsync();
        return Results.NoContent();
    }

    return Results.NotFound();
});