Indentation: throwsIndent should configure indentation of `throws` on next line #2763

Closed
maikelsteneker opened this Issue Dec 22, 2015 · 4 comments

Comments

Projects
None yet
3 participants
@maikelsteneker

This is a PR based on issue #1272. This feature was previously implemented for issue #63. Other information can be found at http://sourceforge.net/p/checkstyle/feature-requests/294/ (already applied by #80).

The intention of this feature is to separately configure the indentation of the throws keyword for a method definition when it appears on the next line.

Such a definition looks like this: void myMethod() throws Exception {
When throws Exception appears on the next line, it is currently required to be at the same indentation as the declaration on the line above. Some styles would choose to indent this next line one level further. The intent is that if throwsIndent is set, this is the expected indentation for this second line.

I originally implemented this feature and published my work on GitHub. This was before CheckStyle moved to GitHub, so this was a copy of the project instead of a fork. @ksclarke worked on integrating this work on CheckStyle 5.7 and providing test cases for it. I currently do not have the time to reimplement this feature, but I hope this information will help.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Feb 18, 2016

Member

The property throwsIndent is not used anywhere in indentation check at this time.

It was added in 1ff166e with the method MethodDefHandler.checkThrows.
It was then removed in e3a48d1 for currently unknown reason.

I will work on restoring this.

Member

rnveach commented Feb 18, 2016

The property throwsIndent is not used anywhere in indentation check at this time.

It was added in 1ff166e with the method MethodDefHandler.checkThrows.
It was then removed in e3a48d1 for currently unknown reason.

I will work on restoring this.

rnveach added a commit to rnveach/checkstyle that referenced this issue Feb 19, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Feb 19, 2016

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Feb 24, 2016

Member

As agreed, here is description of auto formatters for throws indentation.

Showcase of issue:

$ cat TestClass.java
public abstract class Test { //indent:0
 public void m1() throws Exception { //indent:1 line:2
 } //indent:1
 public void m2() throws //indent:1 line:4
Exception { //indent:0 line:5
 } //indent:1
 public void m3() throws Exception, //indent:1 line:7
NullPointerException { //indent:0 line:8
 } //indent:1
 public void m4() //indent:1 line:10
throws Exception { //indent:0 line:11
 } //indent:1
 public abstract void m5() //indent:1 line:13
throws Exception; //indent:0 line:14
 public void m6() //indent:1 line:15
throws //indent:0 line:16
Exception { //indent:0 line:17
 } //indent:1
 public void m7() //indent:1 line:19
throws //indent:0 line:20
Exception, //indent:0 line:21
NullPointerException { //indent:0 line:22
 } //indent:1
 public void //indent:1 line:24
    m8() //indent:4 line:25
throws //indent:0 line:26
Exception, //indent:0 line:27
NullPointerException { //indent:0 line:28
 } //indent:1
}

$ 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="basicOffset" value="1"/>
            <property name="lineWrappingIndentation" value="3"/>
            <property name="throwsIndent" value="5"/>
        </module>
    </module>
</module>

$ java -jar checkstyle-6.15-all.jar -c TestConfig.xml TestClass.java
Starting audit...
Audit done.

Default Auto Formatters:
IntelliJ: All line wrapped throw lines are indented +8, regardless if the throws starts on the same line as method or on the next.
Similar to Eclipse but spaces instead of indents.

Eclipse: All line wrapped throw lines are indented +8, regardless if the throws starts on the same line as method or on the next.

    public void // indent:1 line:24
    m8() // indent:4 line:25
            throws // indent:0 line:26
            Exception, // indent:0 line:27
            NullPointerException { // indent:0 line:28
    } // indent:1

NetBeans: All line wrapped throw lines are indented +8, regardless if the throws starts on the same line as method or on the next.
One difference between the others is m8, the throws line is on the same indent as the method name, which is +8.

    public void //indent:1 line:24
            m8() //indent:4 line:25
            throws //indent:0 line:26
            Exception, //indent:0 line:27
            NullPointerException { //indent:0 line:28
    } //indent:1
Member

rnveach commented Feb 24, 2016

As agreed, here is description of auto formatters for throws indentation.

Showcase of issue:

$ cat TestClass.java
public abstract class Test { //indent:0
 public void m1() throws Exception { //indent:1 line:2
 } //indent:1
 public void m2() throws //indent:1 line:4
Exception { //indent:0 line:5
 } //indent:1
 public void m3() throws Exception, //indent:1 line:7
NullPointerException { //indent:0 line:8
 } //indent:1
 public void m4() //indent:1 line:10
throws Exception { //indent:0 line:11
 } //indent:1
 public abstract void m5() //indent:1 line:13
throws Exception; //indent:0 line:14
 public void m6() //indent:1 line:15
throws //indent:0 line:16
Exception { //indent:0 line:17
 } //indent:1
 public void m7() //indent:1 line:19
throws //indent:0 line:20
Exception, //indent:0 line:21
NullPointerException { //indent:0 line:22
 } //indent:1
 public void //indent:1 line:24
    m8() //indent:4 line:25
throws //indent:0 line:26
Exception, //indent:0 line:27
NullPointerException { //indent:0 line:28
 } //indent:1
}

$ 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="basicOffset" value="1"/>
            <property name="lineWrappingIndentation" value="3"/>
            <property name="throwsIndent" value="5"/>
        </module>
    </module>
</module>

$ java -jar checkstyle-6.15-all.jar -c TestConfig.xml TestClass.java
Starting audit...
Audit done.

Default Auto Formatters:
IntelliJ: All line wrapped throw lines are indented +8, regardless if the throws starts on the same line as method or on the next.
Similar to Eclipse but spaces instead of indents.

Eclipse: All line wrapped throw lines are indented +8, regardless if the throws starts on the same line as method or on the next.

    public void // indent:1 line:24
    m8() // indent:4 line:25
            throws // indent:0 line:26
            Exception, // indent:0 line:27
            NullPointerException { // indent:0 line:28
    } // indent:1

NetBeans: All line wrapped throw lines are indented +8, regardless if the throws starts on the same line as method or on the next.
One difference between the others is m8, the throws line is on the same indent as the method name, which is +8.

    public void //indent:1 line:24
            m8() //indent:4 line:25
            throws //indent:0 line:26
            Exception, //indent:0 line:27
            NullPointerException { //indent:0 line:28
    } //indent:1
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Feb 27, 2016

Member

http://checkstyle.sourceforge.net/config_misc.html#Indentation

that is really weird why we do have option throwsIndent and it does not work at all - should be fixed.

I would prefer to follow behavior of IDEs to indent "throws" by special indentation, and have all following wrapping of Exception names to be indented by throwsIndent

throwsIndent=4 :

public void m1() throws Exception { //indent:0 -
} //indent:0

public void m1() // indent:0
    throws Exception { //indent:4 -
} //indent:0

public void m1() // indent:0
    throws //indent:4 -
    Exception { //indent:4 -
} //indent:0

public void m1() // indent:0
    throws //indent:4 -
    Exception,  //indent:4 -
    Exception2 { //indent:4 -
} //indent:0


public void m1() // indent:0
        throws Exception { //indent:8 - violation!!!
} //indent:0

public void m1() // indent:0
    throws //indent:4 -
        Exception { //indent:8 - violation!!!
} //indent:0

public void m1() throws // indent:0
        Exception { //indent:8 - violation!!!
} //indent:0

public void m1() // indent:0
    throws //indent:4 -
    Exception,  //indent:4 -
        Exception2 { //indent:8 - violation!!!
} //indent:0



Member

romani commented Feb 27, 2016

http://checkstyle.sourceforge.net/config_misc.html#Indentation

that is really weird why we do have option throwsIndent and it does not work at all - should be fixed.

I would prefer to follow behavior of IDEs to indent "throws" by special indentation, and have all following wrapping of Exception names to be indented by throwsIndent

throwsIndent=4 :

public void m1() throws Exception { //indent:0 -
} //indent:0

public void m1() // indent:0
    throws Exception { //indent:4 -
} //indent:0

public void m1() // indent:0
    throws //indent:4 -
    Exception { //indent:4 -
} //indent:0

public void m1() // indent:0
    throws //indent:4 -
    Exception,  //indent:4 -
    Exception2 { //indent:4 -
} //indent:0


public void m1() // indent:0
        throws Exception { //indent:8 - violation!!!
} //indent:0

public void m1() // indent:0
    throws //indent:4 -
        Exception { //indent:8 - violation!!!
} //indent:0

public void m1() throws // indent:0
        Exception { //indent:8 - violation!!!
} //indent:0

public void m1() // indent:0
    throws //indent:4 -
    Exception,  //indent:4 -
        Exception2 { //indent:8 - violation!!!
} //indent:0



rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 24, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 24, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 28, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 28, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 28, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 28, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 28, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 28, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 28, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 29, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 29, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 29, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Apr 29, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue May 7, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue May 7, 2016

@romani romani added the bug label May 7, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue May 7, 2016

@romani romani added this to the 6.19 milestone May 7, 2016

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani May 7, 2016

Member

fix is merged

Member

romani commented May 7, 2016

fix is merged

@romani romani closed this May 7, 2016

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