palantir-java-format icon indicating copy to clipboard operation
palantir-java-format copied to clipboard

Allow Inlined Annotations for Mocks

Open gm2211 opened this issue 4 years ago • 2 comments

What happened?

 @Mock(answer = Answers.RETURNS_DEEP_STUBS) private Job job;

gets reformatted as

@Mock(answer = Answers.RETURNS_DEEP_STUBS) 
private Job job;

Which is not very space-efficient if you have a bunch of mocks

What did you want to happen?

I'd like the code to stay with the inlined annotation.

gm2211 avatar Feb 21 '20 14:02 gm2211

The main goal of the formatter is not space efficiency, but readability. Even if we can't possibly write heuristics that always produce the best readability im all cases, that is the goal we're moving towards, and we'll gladly make things more space efficient by inlining if it improves visibility. Or rather, if the alternative hurts readability.

In this case though, I think inlining hurts readability, because it's now harder to quickly scan variable types/names by just looking at the left column, as your eyes have to scan right every time, and the column where they get defined will vary too with the length of each annotation.

Furthermore, when having annotated fields like this, PJF currently inserts a space in between them too, to make it even easier to scan.

@Mock(answer = Answers.RETURNS_DEEP_STUBS) private Foo foo;
@Mock(answer = Answers.RETURNS_DEEP_STUBS) private Bar bar;

will get reformatted as this, which I would argue has a lot more readability:

@Mock(answer = Answers.RETURNS_DEEP_STUBS)
private Foo foo;

@Mock(answer = Answers.RETURNS_DEEP_STUBS)
private Bar bar;

dansanduleac avatar Mar 27 '20 11:03 dansanduleac

To be honest, I don't really care about space-efficiency per-se, in fact people complain about Java being verbose, but I have no issues with that.

What I do care about is when visual clutter makes things harder to navigate or overwhelms the reader.

I guess this is very subjective, but I disagree that the latter option you propose is any more readable, especially in classes that have many mocks.

Also, I probably picked the wrong example by using a mock annotation with parameters, but consider this:

private static final SomeType SOME = SomeType.builder()
     ...
    .build();
private static final SomeType2 SOME_2 = SomeType2.builder()
     ...
    .build();
private static final SomeType3 SOME_3 = SomeType3.builder()
     ...
    .build();
private static final SomeType4 SOME_4 = SomeType4.builder()
     ...
    .build();

@Mock private SomeObjMetadata meta;
@Mock private SomeContainerObject containerObj;
@Mock private Foo foo;
@Mock private Bar bar;
@Mock private Baz baz;
@Mock private Lorem lorem;
@Mock private Ipsum ipsum;

versus

private static final SomeType SOME = SomeType.builder()
     ...
    .build();

private static final SomeType2 SOME_2 = SomeType2.builder()
     ...
    .build();

private static final SomeType3 SOME_3 = SomeType3.builder()
     ...
    .build();

private static final SomeType4 SOME_4 = SomeType4.builder()
     ...
    .build();

@Mock 
private SomeObjMetadata meta;

@Mock 
private SomeContainerObject containerObj;

@Mock 
private Foo foo;

@Mock 
private Bar bar;

@Mock 
private Baz baz;

@Mock 
private Lorem lorem;

@Mock 
private Ipsum ipsum;

I certainly find the former easier to read and less taxing on the eye - but then again, this is subjective, so it only matters if enough people agree.

gm2211 avatar Apr 15 '20 05:04 gm2211