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

[java] Detect identical catch statements #955

Closed
jsotuyod opened this Issue Mar 5, 2018 · 1 comment

Comments

Projects
None yet
3 participants
@jsotuyod
Member

jsotuyod commented Mar 5, 2018

Under Java 7 and later, it's possible to use multi-catch. We should have a rule detect such cases to help people migrate / reduce boilerplate.

Example:

try {
  // do something
} catch (ClassNotFoundException e) {
  if (LOG.isLoggable(Level.FINE)) {
    LOG.log(Level.FINE, "Could not find class " + className + ", due to: " + e);
  }
} catch (NoClassDefFoundError e) {
  if (LOG.isLoggable(Level.FINE)) {
    LOG.log(Level.FINE, "Could not find class " + className + ", due to: " + e);
  }
}

can be reduced to:

try {
  // do something
} catch (ClassNotFoundException | NoClassDefFoundError e) {
  if (LOG.isLoggable(Level.FINE)) {
    LOG.log(Level.FINE, "Could not find class " + className + ", due to: " + e);
  }
}
@oowekyala

This comment has been minimized.

Show comment
Hide comment
@oowekyala

oowekyala Mar 7, 2018

Member

We currently don't have a concept of equality between nodes, so we can't compare subtrees to implement that rule. We could either compare the lexer tokens, or the AST. My guess, is that comparing tokens would be simpler to implement, and probably as correct, since identical catch blocks are often an exact copy-paste.

Edit: except it wouldn't be sensitive to changes in the exception variable name

Member

oowekyala commented Mar 7, 2018

We currently don't have a concept of equality between nodes, so we can't compare subtrees to implement that rule. We could either compare the lexer tokens, or the AST. My guess, is that comparing tokens would be simpler to implement, and probably as correct, since identical catch blocks are often an exact copy-paste.

Edit: except it wouldn't be sensitive to changes in the exception variable name

djydewang added a commit to djydewang/pmd that referenced this issue Mar 9, 2018

djydewang added a commit to djydewang/pmd that referenced this issue Mar 9, 2018

djydewang added a commit to djydewang/pmd that referenced this issue Mar 21, 2018

Issue #955: add new rule to detect identical catch statement
It flags identical catch statements, based on token equality between the two blocks
Identical catch blocks.

djydewang added a commit to djydewang/pmd that referenced this issue Mar 22, 2018

djydewang added a commit to djydewang/pmd that referenced this issue Mar 22, 2018

djydewang added a commit to djydewang/pmd that referenced this issue Mar 22, 2018

djydewang added a commit to djydewang/pmd that referenced this issue Mar 26, 2018

Issue #955: add new rule to detect identical catch statement
It flags identical catch statements, based on token equality between the two blocks
Identical catch blocks.

djydewang added a commit to djydewang/pmd that referenced this issue Mar 26, 2018

djydewang added a commit to djydewang/pmd that referenced this issue Mar 26, 2018

@adangel adangel added this to the 6.4.0 milestone May 21, 2018

@adangel adangel added the has:pr label May 21, 2018

@adangel adangel closed this in 43f9237 May 21, 2018

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