Regression in 2.6.0

[bsideup]

Prettier-Java 0.6.0

# Options (if any):
--print-width 120

It looks like #632 , despite being a great improvement overall, introduced a regression when the first method call needs to be multi-line due to long arguments, leading to a closing ) misaligned.

Input:

public static final MethodDescriptor<ByteBuf, ByteBuf> NEW_TUNNEL_METHOD = MethodDescriptor
		.newBuilder(ByteBufMarshaller.INSTANCE, ByteBufMarshaller.INSTANCE)
		.setFullMethodName(TUNNEL_SERVICE + "/new")
		.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
		.build();

Output:

public static final MethodDescriptor<ByteBuf, ByteBuf> NEW_TUNNEL_METHOD = MethodDescriptor.newBuilder(
		ByteBufMarshaller.INSTANCE,
		ByteBufMarshaller.INSTANCE
	)
		.setFullMethodName(TUNNEL_SERVICE + "/new")
		.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
		.build();

Expected behavior:

public static final MethodDescriptor<ByteBuf, ByteBuf> NEW_TUNNEL_METHOD = MethodDescriptor
		.newBuilder(ByteBufMarshaller.INSTANCE, ByteBufMarshaller.INSTANCE)
		.setFullMethodName(TUNNEL_SERVICE + "/new")
		.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
		.build();

Alternative expected behavior:

public static final MethodDescriptor<ByteBuf, ByteBuf> NEW_TUNNEL_METHOD = MethodDescriptor.newBuilder(
			ByteBufMarshaller.INSTANCE,
			ByteBufMarshaller.INSTANCE
		)
		.setFullMethodName(TUNNEL_SERVICE + "/new")
		.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
		.build();

See bsideup/grpc-bidi@bd35134

[jtkiesel]

Thank you for creating this issue! However, I believe this is intended behavior. At least, it mimics Prettier's own formatting for a similar code snippet in TypeScript, which is what most (if not all) of the formatting decisions in #632 were based upon.

Prettier Java example:

class Example {

	public static final MethodDescriptor<ByteBuf, ByteBuf> NEW_TUNNEL_METHOD = MethodDescriptor.newBuilder(
		ByteBufMarshaller.INSTANCE,
		ByteBufMarshaller.INSTANCE
	)
		.setFullMethodName(TUNNEL_SERVICE + "/new")
		.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
		.build();
}

Prettier TypeScript example:

class Example {
	public static readonly NEW_TUNNEL_METHOD: MethodDescriptor<ByteBuf, ByteBuf> = MethodDescriptor.newBuilder(
		ByteBufMarshaller.INSTANCE,
		ByteBufMarshaller.INSTANCE,
	)
		.setFullMethodName(TUNNEL_SERVICE + "/new")
		.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
		.build();
}

Because of that, I'm doubtful that we will revert this change, as we're trying our best to keep Prettier Java aligned with Prettier's own formatting decisions, specifically in TypeScript (which tends to be the "native" language most similar to Java).

[lppedd]

I'll be honest tho, the current actual behavior looks noticeable worse than what we'd expect.
It's like seeing a lost parenthesis in the middle of the code.

[bsideup]

@jtkiesel ah, thanks, I missed that! I wonder if it would be considered bad for TypeScript too and is just yet to be fixed. Otherwise, I agree with @lppedd that it looks like a black sheep of otherwise beautifully formatted code.

Would you be open to reconsider the current behavior (even if it means slightly diverging from TS, at least until they reconsider it too)? I do agree and accept all the opinionatedness of prettier-java except this one that does look like a bug in the formatter and does not match any other formatting logic in the rest of code 😅

[jtkiesel]

There is an existing issue against Prettier related to this, but it hasn't gotten much traffic. Linking it here for future reference, in case that changes: prettier/prettier#16688