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

New Check: Useless Catch Check #267

Closed
baratali opened this Issue Oct 30, 2014 · 4 comments

Comments

Projects
None yet
3 participants
@baratali
Contributor

baratali commented Oct 30, 2014

Forbid catch blocks that just throw exception without wrapping.

try {
    ...
}
catch(UnsupportedOperationException e) { // warning
    throw e;
}
catch(Exception e) { // no warning
    throw new RuntimeException(e);
}
@alex-zuy

This comment has been minimized.

Show comment
Hide comment
@alex-zuy

alex-zuy Nov 25, 2014

Contributor

Consider following example

public class User
{
    static class Ex extends Exception { }

    static class Ex1 extends Ex { }
    static class Ex2 extends Ex { }
    static class Ex3 extends Ex { }

    public void run() throws Exception
    {
        try {
            ....
        }
        catch(Ex1 e) {
            throw e;
        }
        catch(Ex e) {
            e.printStackTrace();
        }
    }
}

try block can throw exceptions of type Ex, Ex1, Ex2, Ex3. User dont want to handle exception of type Ex1 in run() method, but wants to handle all another exceptions of type Ex and it derivatives.
In this case first catch block is not useless. Checkstyle gives no opportunity to determine type hierarcy of classes, so we cannot distinguish such cases. This means that check should not look for multi-catch cases and only consider sigle-catch ones.

Contributor

alex-zuy commented Nov 25, 2014

Consider following example

public class User
{
    static class Ex extends Exception { }

    static class Ex1 extends Ex { }
    static class Ex2 extends Ex { }
    static class Ex3 extends Ex { }

    public void run() throws Exception
    {
        try {
            ....
        }
        catch(Ex1 e) {
            throw e;
        }
        catch(Ex e) {
            e.printStackTrace();
        }
    }
}

try block can throw exceptions of type Ex, Ex1, Ex2, Ex3. User dont want to handle exception of type Ex1 in run() method, but wants to handle all another exceptions of type Ex and it derivatives.
In this case first catch block is not useless. Checkstyle gives no opportunity to determine type hierarcy of classes, so we cannot distinguish such cases. This means that check should not look for multi-catch cases and only consider sigle-catch ones.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Nov 26, 2014

Member

I do not any way for Checkstyle to validate that code correctly, tools with type awareness need to do that.

Member

romani commented Nov 26, 2014

I do not any way for Checkstyle to validate that code correctly, tools with type awareness need to do that.

@romani romani closed this Nov 26, 2014

@alex-zuy

This comment has been minimized.

Show comment
Hide comment
@alex-zuy

alex-zuy Nov 26, 2014

Contributor

Check can handle single-catch cases correctly without any type information.

Contributor

alex-zuy commented Nov 26, 2014

Check can handle single-catch cases correctly without any type information.

alex-zuy added a commit to alex-zuy/sevntu.checkstyle that referenced this issue Nov 26, 2014

@alex-zuy

This comment has been minimized.

Show comment
Hide comment
@alex-zuy

alex-zuy Nov 26, 2014

Contributor

Sevntu checkstyle google group thread

Contributor

alex-zuy commented Nov 26, 2014

Sevntu checkstyle google group thread

@romani romani reopened this Nov 26, 2014

alex-zuy added a commit to alex-zuy/sevntu.checkstyle that referenced this issue Nov 26, 2014

alex-zuy added a commit to alex-zuy/sevntu.checkstyle that referenced this issue Nov 27, 2014

alex-zuy added a commit to alex-zuy/sevntu.checkstyle that referenced this issue Nov 27, 2014

alex-zuy added a commit to alex-zuy/sevntu.checkstyle that referenced this issue Nov 29, 2014

alex-zuy added a commit to alex-zuy/sevntu.checkstyle that referenced this issue Dec 1, 2014

@romani romani closed this Dec 3, 2014

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