velo / maven-formatter-plugin

Maven2 Java Formatter Plugin - Exported from code.google.com/p/maven-java-formatter-plugin
Other
11 stars 7 forks source link

Indentation of dangling single line comments is not consistent with Eclipse #21

Open Legioth opened 7 years ago

Legioth commented 7 years ago

In cases where a single line comment at the end of an already long line has been split up into manly lines, the plugin adds additional indentation to all the dangling lines. This is not consistent with how the same code is formatted in Eclipse Neon SR1.

If it matters, the configuration file I've been testing with is https://github.com/vaadin/framework/blob/8491ea68d32a275bb70998db5f66759265d367b8/eclipse/VaadinJavaConventions.xml. All other cases seem to be formatted consistently between Eclipse and the plugin, except this case and https://github.com/velo/maven-formatter-plugin/issues/20.

An extreme example of where this happens is https://github.com/vaadin/framework/blob/95d016c08569f74a2c76eac40c4f5656bb773951/server/src/test/java/com/vaadin/tests/design/ComponentFactoryTest.java#L92 where the comment in testComponentFactoryThrowingStuff gets formatted as

                DesignContext context) -> defaultFactory.createComponent(
                        "foobar." + fullyQualifiedClassName, context) // Will
                                                                                                                                                                                               // throw
                                                                                                                                                                                                               // because
                                                                                                                                                                                                               // class
                                                                                                                                                                                                               // is not
                                                                                                                                                                                                               // found

It's especially interesting that the first dangling comment line gets slightly less indentation compared to the rest of the lines.

The expected formatting that is produced produced by Eclipse Neon SR1 using the same formatting settings is

                DesignContext context) -> defaultFactory.createComponent(
                        "foobar." + fullyQualifiedClassName, context) // Will
                                                                      // throw
                                                                      // because
                                                                      // class
                                                                      // is not
                                                                      // found

Encountered with version 1.8.1 of the plugin.

Legioth commented 7 years ago

I also verified that this issue does not seem to be caused by the same kind of legacy settings file that caused #20.

velo commented 7 years ago

Can you create a pull request adding the change here: https://github.com/velo/maven-formatter-plugin/pull/22/files#diff-2844c1574936a15b1f30d77f17e959ecR38

So I can see the test failing.

Legioth commented 7 years ago

Can you create a pull request adding the change here

I tried, but it the entire test implodes for any single-line comment in AnyClass.java.

I didn't get further than adding this to the end of AnyClass:


        public void somethingWithDanglingComment() {
            // Comment
        }   

With this addition, the end of target/test-classes/AnyClass.java looks like this after running mvn verify:

        @Test          public void testAddressNestedPropertyInvalidPostalCodeFails() {         assertFails(100_000, "must be less than or equal to 99999", validator("address.postalCode")); }

public void somethingWithDanglingComment() { // Comment       }   }        

Removing // Comment makes the test pass and target/test-classes/AnyClass.java looks decent after running.

velo commented 7 years ago

Ha!

That unit test is removing all line separation in a effort to scramble the source file. Then it formats and compares against the original.

Need to change the test so it add a new line after comments.