Skip to content
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

EmptyCatchBlock check does not work properly with non-system line endings in files #6513

Closed
Vampire opened this issue Mar 6, 2019 · 3 comments

Comments

Projects
None yet
3 participants
@Vampire
Copy link
Contributor

commented Mar 6, 2019

/var/tmp $ javac Foo.java

/var/tmp $ cat config.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
        "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
        "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
    <module name="TreeWalker">
        <module name="EmptyCatchBlock">
            <property name="commentFormat" value="This is expected"/>
        </module>
    </module>
</module>

/var/tmp $ cat Foo.java
import java.io.IOException;

class Foo {
    private void foo() {
        try {
            throw new IOException();
        } catch (IOException e) {
            /*
             * some comment
             * This is expected
             */
        }
    }
}

/var/tmp $ xxd Foo.java
00000000: 696d 706f 7274 206a 6176 612e 696f 2e49  import java.io.I
00000010: 4f45 7863 6570 7469 6f6e 3b0a 0a63 6c61  OException;..cla
00000020: 7373 2046 6f6f 207b 0a20 2020 2070 7269  ss Foo {.    pri
00000030: 7661 7465 2076 6f69 6420 666f 6f28 2920  vate void foo()
00000040: 7b0a 2020 2020 2020 2020 7472 7920 7b0a  {.        try {.
00000050: 2020 2020 2020 2020 2020 2020 7468 726f              thro
00000060: 7720 6e65 7720 494f 4578 6365 7074 696f  w new IOExceptio
00000070: 6e28 293b 0a20 2020 2020 2020 207d 2063  n();.        } c
00000080: 6174 6368 2028 494f 4578 6365 7074 696f  atch (IOExceptio
00000090: 6e20 6529 207b 0a20 2020 2020 2020 2020  n e) {.
000000a0: 2020 202f 2a0a 2020 2020 2020 2020 2020     /*.
000000b0: 2020 202a 2073 6f6d 6520 636f 6d6d 656e     * some commen
000000c0: 740a 2020 2020 2020 2020 2020 2020 202a  t.             *
000000d0: 2054 6869 7320 6973 2065 7870 6563 7465   This is expecte
000000e0: 640a 2020 2020 2020 2020 2020 2020 202a  d.             *
000000f0: 2f0a 2020 2020 2020 2020 7d0a 2020 2020  /.        }.
00000100: 7d0a 7d0a                                }.}.

/var/tmp $ java -Duser.language=en -Duser.country=US -Dline.separator=$'\n' -jar checkstyle-8.18-all.jar -c config.xml Foo.java
Starting audit...
[ERROR] D:\Sourcecode\other\discord-logger\tmp\Foo.java:7: Empty catch block. [EmptyCatchBlock]
Audit done.
Checkstyle ends with 1 errors.

/var/tmp $ java -Duser.language=en -Duser.country=US -Dline.separator=$'\r\n' -jar checkstyle-8.18-all.jar -c config.xml Foo.java
Starting audit...
Audit done.

When having non-system line endings in files as is quite common for cross-platform projects, the EmptyCatchBlock check fails to properly extract the first line of a block comment with CS 8.18 to check it against the comment format pattern.
Instead for example the full comment will be used when a file has linux line endings on windows, or the comment to test has an extra carriage return character in the end when a file has windows line endings on linux.
This even happens in EmptyCatchBlockCheckTest#testWithUserSetValues when the InputEmptyCatchBlockDefault.java file has linux line endings and you run the test on a windows system.

Vampire added a commit to Vampire/checkstyle that referenced this issue Mar 6, 2019

Vampire added a commit to Vampire/checkstyle that referenced this issue Mar 6, 2019

Vampire added a commit to Vampire/checkstyle that referenced this issue Mar 10, 2019

Vampire added a commit to Vampire/checkstyle that referenced this issue Mar 11, 2019

Vampire added a commit to Vampire/checkstyle that referenced this issue Mar 18, 2019

@rnveach

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

@romani Please review the issue.

@romani romani added the approved label Apr 27, 2019

@romani romani removed their assignment Apr 27, 2019

@romani

This comment has been minimized.

Copy link
Member

commented Apr 27, 2019

approved.
such Check should work with any line-ending, it is not his business to validate or depend on this.

Vampire added a commit to Vampire/checkstyle that referenced this issue May 2, 2019

Vampire added a commit to Vampire/checkstyle that referenced this issue May 2, 2019

Vampire added a commit to Vampire/checkstyle that referenced this issue May 2, 2019

Vampire added a commit to Vampire/checkstyle that referenced this issue May 2, 2019

@romani romani closed this in #6515 May 2, 2019

romani added a commit that referenced this issue May 2, 2019

@romani romani added the bug label May 2, 2019

@romani romani added this to the 8.21 milestone May 2, 2019

@romani

This comment has been minimized.

Copy link
Member

commented May 2, 2019

@Vampire , thanks a lot for your contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.