csharpier icon indicating copy to clipboard operation
csharpier copied to clipboard

Avoid wrapping method arguments including a block-bodied lambda

Open jcracknell opened this issue 1 year ago • 3 comments

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.

jcracknell avatar Feb 21 '24 21:02 jcracknell

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);
});

belav avatar Feb 23 '24 16:02 belav

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();
});

jcracknell avatar Feb 23 '24 16:02 jcracknell

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();
});

Rudomitori avatar Feb 23 '24 17:02 Rudomitori