Skip to content

SONARJAVA-6406 Create rule S8714 - Use assertThrows/assertDoesNotThrow instead of try-catch with fail()#5642

Draft
rombirli wants to merge 13 commits into
masterfrom
rombiri/sonarjava-6406-create-rule-S8714
Draft

SONARJAVA-6406 Create rule S8714 - Use assertThrows/assertDoesNotThrow instead of try-catch with fail()#5642
rombirli wants to merge 13 commits into
masterfrom
rombiri/sonarjava-6406-create-rule-S8714

Conversation

@rombirli
Copy link
Copy Markdown
Contributor

@rombirli rombirli commented May 28, 2026


Summary by Gitar

  • Integration and Quality Gate:
    • Added java-S8714.json to the ruling test resources, documenting test case coverage and expectations for the new rule.

This will update automatically on new commits.

@rombirli rombirli marked this pull request as draft May 28, 2026 14:30
@hashicorp-vault-sonar-prod
Copy link
Copy Markdown
Contributor

hashicorp-vault-sonar-prod Bot commented May 28, 2026

SONARJAVA-6406

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 28, 2026

Agentic Analysis: Early Results

Agentic Analysis and Context Augmentation are available on your project. Here are some issues that could have been prevented. Follow the links to learn how to put them into action.

17 issue(s) found across 1 file(s):

Rule File Line Message
java:S125 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 10 This block of commented-out lines of code should be removed.
java:S139 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 10 Move this trailing comment on the previous empty line.
java:S125 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 20 This block of commented-out lines of code should be removed.
java:S139 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 20 Move this trailing comment on the previous empty line.
java:S125 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 25 This block of commented-out lines of code should be removed.
java:S139 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 25 Move this trailing comment on the previous empty line.
java:S1192 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 28 Define a constant instead of duplicating this literal "expected exception" 5 times.
java:S125 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 33 This block of commented-out lines of code should be removed.
java:S139 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 33 Move this trailing comment on the previous empty line.
java:S125 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 41 This block of commented-out lines of code should be removed.
java:S139 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 41 Move this trailing comment on the previous empty line.
java:S125 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 49 This block of commented-out lines of code should be removed.
java:S139 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 49 Move this trailing comment on the previous empty line.
java:S125 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 57 This block of commented-out lines of code should be removed.
java:S139 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 57 Move this trailing comment on the previous empty line.
java:S125 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 65 This block of commented-out lines of code should be removed.
java:S139 java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java 65 Move this trailing comment on the previous empty line.

Analyzed by SonarQube Agentic Analysis in 10.4 s

gitar-bot[bot]

This comment was marked as resolved.

@gitar-bot gitar-bot Bot dismissed their stale review May 29, 2026 10:55

✅ Code review updated (blocking issues remain unresolved).

Configure merge blocking

Comment thread its/autoscan/src/test/resources/autoscan/diffs/diff_S8714.json
@rombirli rombirli force-pushed the rombiri/sonarjava-6406-create-rule-S8714 branch from 319c62c to 0cb3ee1 Compare May 29, 2026 13:01
@sonarqube-next
Copy link
Copy Markdown

@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented May 29, 2026

Code Review ✅ Approved 4 resolved / 4 findings

Implements rule S8714 to replace try-catch blocks with JUnit assertions, resolving compilation errors in the test suite and refining the suggestion logic.

✅ 4 resolved
Bug: Test uses no-op check so verifyIssues() will always fail

📄 java-checks/src/test/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheckTest.java:28-34
The test passes a no-op lambda context -> { // do nothing } as the check. Since the check never reports any issues, verifyIssues() will fail because the sample file contains two // Noncompliant annotations that expect issues to be raised. The test should instantiate the actual rule check class (e.g., new AssertThrowsInsteadOfTryCatchFailCheck()) once it exists, or at minimum use verifyNoIssues() if this is a placeholder. As written, this test will not pass.

Bug: Method doesBlockFail does not exist; code won't compile

📄 java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java:43 📄 java-checks/src/main/java/org/sonar/java/checks/tests/AbstractOneExpectedExceptionRule.java:38 📄 java-checks/src/main/java/org/sonar/java/checks/tests/AbstractOneExpectedExceptionRule.java:123
The check calls UnitTestUtils.doesBlockFail(block) (line 43) and AbstractOneExpectedExceptionRule imports UnitTestUtils.doesBlockFail, but the actual method in UnitTestUtils.java is still named isTryCatchFail (line 257). The rename was never applied to the utility class, so both the new check and the existing AbstractOneExpectedExceptionRule will fail to compile.

Bug: Wrong suggestion: try-block fail() should recommend assertThrows

📄 java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java:38
When fail() is the last statement in the try block (e.g. try { raise(); fail(); } catch (...) {}), it means the test expects an exception to be thrown — reaching fail() means no exception occurred, which is the failure case. The correct replacement is assertThrows(), not assertDoesNotThrow(). The current message on line 38 incorrectly suggests assertDoesNotThrow() for both cases.

assertDoesNotThrow() is only correct when fail() is in the catch block (the test expects no exception).

Bug: Autoscan shows rule S8714 finds zero true positives (33 false negatives)

📄 its/autoscan/src/test/resources/autoscan/diffs/diff_S8714.json:3-4
The autoscan expectations changed from hasTruePositives: true / falseNegatives: 0 to hasTruePositives: false / falseNegatives: 33. This means the rule no longer detects any issues in the real-world autoscan corpus — a complete regression in detection capability.

The commit message "Fix ruling/autoscan?" (with a question mark) suggests uncertainty. This likely indicates a bug in the rule implementation that prevents it from matching patterns found in real projects. Consider investigating which of the 33 expected matches are no longer triggered and why, rather than adjusting the expectations file to pass CI.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant