openrewrite / openrewrite/rewrite-static-analysis
CombineSemanticallyEqualCatchBlocks indexes J.Case#getCaseLabels() unguarded, and only ever compares the first label
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 62
- Forks
- 112
- Avg merge
- 1d 19h
- Merged PRs (30d)
- 40
Description
Found while auditing for the crash class fixed in the PR that guards ForLoopIncrementInUpdate, WhileInsteadOfFor and DefaultComesLastVisitor against J collections that non-Java parsers leave empty.
CombineSemanticallyEqualCatchBlocks.java:599:
J.Case compareTo = (J.Case) j;
if (_case.getStatements().size() != compareTo.getStatements().size() ||
doesNotContainSameComments(_case.getPrefix(), compareTo.getPrefix())) {
isEqual.set(false);
return _case;
}
this.visit(_case.getCaseLabels().get(0), compareTo.getCaseLabels().get(0));
Two problems:
-
Unguarded
.get(0). The size check above coversgetStatements(), notgetCaseLabels(). The Java parser always puts at least one label on aJ.Case(default:becomes aJ.Identifiernamed"default"), but other parsers do not — Go emits aJ.Casewith no labels at all, which is what madeDefaultComesLastVisitor.isDefaultCasethrowIndexOutOfBoundsExceptionin production. Not reachable from Go here, since this comparator only descends intocatchbodies and Go has no try/catch, but reachable from anyJ-based language that has both try/catch and a label-less case. -
Only label 0 is compared. For a multi-label case the remaining labels are ignored, so
case 1, 2:compares equal tocase 1, 3:and two catch blocks that are not semantically equal can be combined. This one bites plain Java today.
The natural fix closes both: add getCaseLabels().size() to the early-return size check, then loop over all labels rather than indexing 0. Since that changes Java behaviour for multi-label cases it wants its own tests, which is why it was left out of the crash-fix PR.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at CombineSemanticallyEqualCatchBlocks.java:599 and inspect the surrounding comparison logic and existing tests for catch-block combination. Done means label-less cases no longer cause an unguarded access, every case label participates in comparison, and tests cover differing multi-label cases; run the relevant Java test suite.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- devtools, testing-qa
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100