New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Indentation check fails for Line Wrapped Anonymous Inner Class with LCurly on newline #3612

Closed
JLLeitschuh opened this Issue Dec 6, 2016 · 2 comments

Comments

Projects
None yet
3 participants
@JLLeitschuh

JLLeitschuh commented Dec 6, 2016

$ cat TestClass.java
public class TestClass
{
    public static class SimpleClass {
        public void method() {}
    }

    void method()
    {
        final SimpleClass test =
                new SimpleClass() { //indent:16
                    public void method() {}
                }; //indent:16
        final SimpleClass test2 =
                new SimpleClass() //indent:16
                { //indent:16 violation
                    public void method() {}
                }; //indent:16
    }
}

$ cat TestConfig.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

    <module name="TreeWalker">
<module name="Indentation">
  <property name="tabWidth" value="4" />
  <property name="basicOffset" value="4" />
  <property name="braceAdjustment" value="0" />
  <property name="caseIndent" value="4" />
  <property name="lineWrappingIndentation" value="8" />
  <property name="throwsIndent" value="4" />
  <property name="arrayInitIndent" value="4" />
</module>
    </module>
</module>

$ java -jar checkstyle-7.4-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[ERROR] TestClass.java:15: 'object def lcurly' have incorrect indentation level 16, expected level should be one of the following: 8, 12. [Indentation]
Audit done.
Checkstyle ends with 1 errors.

There are two test cases here, the first, test passes because the LCURLY is on the same line.
test2 fails because the LCURLY is on its own line alone.

the current validation code wants this:

        final SimpleClass test2 =
                new SimpleClass() //indent:16
            { //indent:12
                public void method() {}
            }; //indent:12

JLLeitschuh@661ad24#diff-41124079fe0fabb5d72bbb58a1f17c8c
Above has test case and fix.


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@JLLeitschuh JLLeitschuh changed the title from Indentation check fails for Line Wrapped Anonymous Inner Class with LCurly on newline and to Indentation check fails for Line Wrapped Anonymous Inner Class with LCurly on newline Dec 6, 2016

JLLeitschuh added a commit to JLLeitschuh/checkstyle that referenced this issue Dec 6, 2016

JLLeitschuh added a commit to JLLeitschuh/checkstyle that referenced this issue Dec 6, 2016

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Dec 6, 2016

Member

@romani The issue looks valid to me. Braces should be at the same identation for their respective lines.

The issue shows 2 similar cases, with the only difference is where the opening brace is located.
Simplified version:

                new SimpleClass() { //indent:16
                }; //indent:16
                new SimpleClass() //indent:16
                { //indent:16 violation
                }; //indent:16

The line for the braces are at the same indentation (both 16), and we accept the location of the new SimpleClass, so I don't think there should be any violation in the example.

Current violation wants the code to look something like:

                new SimpleClass() { //indent:16
                }; //indent:16
                new SimpleClass() //indent:16
            { //indent:12
                }; //indent:16

Which definitely looks wrong.

Member

rnveach commented Dec 6, 2016

@romani The issue looks valid to me. Braces should be at the same identation for their respective lines.

The issue shows 2 similar cases, with the only difference is where the opening brace is located.
Simplified version:

                new SimpleClass() { //indent:16
                }; //indent:16
                new SimpleClass() //indent:16
                { //indent:16 violation
                }; //indent:16

The line for the braces are at the same indentation (both 16), and we accept the location of the new SimpleClass, so I don't think there should be any violation in the example.

Current violation wants the code to look something like:

                new SimpleClass() { //indent:16
                }; //indent:16
                new SimpleClass() //indent:16
            { //indent:12
                }; //indent:16

Which definitely looks wrong.

JLLeitschuh added a commit to JLLeitschuh/checkstyle that referenced this issue Dec 6, 2016

@romani romani added the approved label Jan 4, 2017

@rnveach rnveach added the indentaion label Jan 6, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 10, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 10, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 10, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 10, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 10, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 10, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 10, 2017

romani added a commit that referenced this issue Mar 23, 2017

@romani romani added this to the 7.7 milestone Mar 23, 2017

@romani romani added the bug label Mar 23, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 23, 2017

Member

fix is merged.

Member

romani commented Mar 23, 2017

fix is merged.

@romani romani closed this Mar 23, 2017

GitToasterhub added a commit to GitToasterhub/checkstyle that referenced this issue Mar 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment