SONARJAVA-6454 : Rule S8714 - implement quickfix by rombirli · Pull Request #5663 · SonarSource/sonar-java

rombirli · 2026-06-08T08:05:17Z

Summary by Gitar

New implementation:
- Added quickfix support for AssertThrowsInsteadOfTryCatchFailCheck for both JUnit and AssertJ.
- Implemented TryCatchUtils helper to centralize catch block type extraction logic.
Refactoring:
- Migrated getCaughtTypes logic from InterruptedExceptionCheck to the new shared TryCatchUtils helper.
Test updates:
- Updated AssertThrowsInsteadOfTryCatchFailCheckSample to correctly categorize JUnit failure cases and add new test coverage.
- Added TryCatchUtilsTest to verify catch block type resolution.
Quickfix features:
- Introduced Replacements record to handle automated code transformations for JUnit assertThrows and AssertJ assertThatCode patterns.

_{This will update automatically on new commits.}

… Supplier

…rrectly

… method invocation

sonarqubecloud · 2026-06-08T08:05:25Z

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.

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

Rule	File	Line	Message
`java:S125`	`java-checks-test-sources/default/src/test/java/checks/tests/AssertThrowsInsteadOfTryCatchFailCheckSample.java`	91	This block of commented-out lines of code should be removed.
`java:S139`	`java-checks-test-sources/default/src/test/java/checks/tests/AssertThrowsInsteadOfTryCatchFailCheckSample.java`	91	Move this trailing comment on the previous empty line.
`java:S125`	`java-checks-test-sources/default/src/test/java/checks/tests/AssertThrowsInsteadOfTryCatchFailCheckSample.java`	99	This block of commented-out lines of code should be removed.
`java:S139`	`java-checks-test-sources/default/src/test/java/checks/tests/AssertThrowsInsteadOfTryCatchFailCheckSample.java`	99	Move this trailing comment on the previous empty line.
`java:S108`	`java-checks-test-sources/default/src/test/java/checks/tests/AssertThrowsInsteadOfTryCatchFailCheckSample.java`	101	Remove this block of code, fill it in, or add a comment explaining why it is empty.
`java:S139`	`java-checks-test-sources/default/src/test/java/checks/tests/AssertThrowsInsteadOfTryCatchFailCheckSample.java`	120	Move this trailing comment on the previous empty line.

_{Analyzed by SonarQube Agentic Analysis in 2.9 s}

hashicorp-vault-sonar-prod · 2026-06-08T08:05:32Z

SONARJAVA-6406

tomasz-tylenda-sonarsource

The contents of the PR changed. Let me send you the comments I had. I'll finish the review when the contents is stable.

tomasz-tylenda-sonarsource · 2026-06-08T08:31:22Z

+
+  public static List<Type> getCaughtTypes(CatchTree tree) {
+    VariableTree parameter = tree.parameter();
+    if (parameter.type().is(Tree.Kind.UNION_TYPE)) {


Can we do ... instanceof UnionTypeTree unio.TypeTree and avoid cast below?

tomasz-tylenda-sonarsource · 2026-06-08T08:36:49Z

+    /* This utility class should not be instantiated */
+  }
+
+  public static List<Type> getCaughtTypes(CatchTree tree) {


This method should have a test.

Done. But the test is actually more complicated and contains more logic than the method itself (it tests more test code than prod code)

tomasz-tylenda-sonarsource · 2026-06-08T08:51:43Z

+              junitReplacement(failArguments, tryStatement, isTryBlock);
+            issueBuilder.withQuickFix(() ->
+              JavaQuickFix.newQuickFix(issueMessage).addTextEdit(
+                JavaTextEdit.replaceTree(tryStatement.tryKeyword(), replacements.replaceTryWith),


I commented out code here and the unit tests still passed. Is this at all executed in tests?

Yes : currently unit testing quickfixes is very hard to setup because of improper logging, and those unit tests don't make much sense. They only assert that the list of edits matches the expected list of edits but not that the fixed version of the code matches the expected fixed code. Let me know if you find it strictly necessary to test this part, i can cover it (and make it fail if you comment it out).

…t's remove it."

gitar-bot · 2026-06-08T14:57:01Z

Code Review ✅ Approved 5 resolved / 5 findings

Implements quickfix support for S8714 by introducing TryCatchUtils and handling JUnit/AssertJ transformations. Resolved issues include removing debug print statements, fixing wildcard imports, and preventing potential NPEs in catch block analysis.

✅ 5 resolved

✅ Bug: Test comment typo '// NonCompliant' is not recognized by verifier

📄 java-checks-test-sources/default/src/test/java/checks/tests/AssertThrowsInsteadOfTryCatchFailCheckSample.java:12
In AssertThrowsInsteadOfTryCatchFailCheckSample.java:12 the expected-issue marker was changed from // Noncompliant to // NonCompliant. The CheckVerifier only recognizes the case-sensitive flag Noncompliant (see Expectations.java NONCOMPLIANT_FLAG = "Noncompliant" with regex //\s+Noncompliant). NonCompliant (capital C) does not match, so this comment is treated as an ordinary comment. The check still reports a real issue on that fail(); line, which the verifier will now flag as an unexpected issue, causing the test to fail. Additionally, the intended assertion (message text and ^^^^^^ location) is no longer validated. Revert to // Noncompliant.

✅ Quality: Wildcard imports violate rule S2208 (self-dogfooding)

📄 java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java:27 📄 java-checks/src/main/java/org/sonar/java/checks/helpers/TryCatchUtils.java:20 📄 java-checks/src/main/java/org/sonar/java/checks/helpers/TryCatchUtils.java:22
AssertThrowsInsteadOfTryCatchFailCheck.java:27 (import org.sonar.plugins.java.api.tree.*;) and TryCatchUtils.java:20,22 (...api.tree.*, java.util.*) use wildcard imports. The sonar-java codebase dogfoods rule S2208 ("Wildcard imports should not be used") and elsewhere uses explicit imports. Replace with explicit single-type imports (Arguments, BaseTreeVisitor, BlockTree, MethodTree, Tree, TryStatementTree, etc. and List, Collections).

✅ Bug: Quickfix throws IndexOutOfBounds for try-block fail() without catch

📄 java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java:82-96 📄 java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java:116 📄 java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java:135 📄 java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java:156-158
firstCaughtTypeInTry (line 156-158) and the quickfix edit (line 90-93) unconditionally call tryStatement.catches().get(0) / catches().get(size-1). The rule reports fail() in a try block regardless of whether catch clauses exist (e.g. a try { raise(); fail(); } finally { ... } with no catch). For the try-block path, junitReplacement/assertJReplacement call firstCaughtTypeInTry eagerly at lines 116/135 while building Replacements (before withQuickFix), so an empty catches() list will throw IndexOutOfBoundsException during analysis and crash the rule on otherwise valid code. Guard against an empty catch list: skip the quickfix (still report the issue) when tryStatement.catches().isEmpty().

✅ Quality: Leftover debug System.out.println in new test

📄 java-checks/src/test/java/org/sonar/java/checks/helpers/TryCatchUtilsTest.java:39
The new parseTry helper contains a leftover debugging statement System.out.println(compilationUnitTree);. This pollutes test output and serves no purpose in the assertion. Remove it before merging.

✅ Edge Case: AssertJ quickfix misuses non-String fail() argument as fail message

📄 java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java:125-139
assertJReplacement (line 131) always treats failArguments.get(0) as the failure message via contentFor(...) and emits .withFailMessage(<arg0>). But the check also matches AssertJ overloads whose first argument is not a String message, e.g. org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown(IllegalStateException.class) (exercised at sample line 66). That produces .withFailMessage(IllegalStateException.class) which does not compile / is semantically wrong. Also failArguments.get(0) assumes at least one argument, which can throw for no-arg AssertJ fail() variants. Consider only emitting withFailMessage(...) when the first argument is a String, and guard against empty arguments.

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}

sonarqube-next · 2026-06-08T15:04:23Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
93.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

rombirli added 15 commits June 8, 2026 09:27

quickfix skeleton

211440a

quickfix skeleton 2

19300f9

QuickFix skleton part3

23fb64a

Initial quickfixes implementation, arguments not aware

821a7d0

Fix for junit assertDoesntThrow (doesnt takes any throwable as arg)

ae3956f

Junit arguments aware

b4a432b

Add quickfix arguments for assertJ

5a5d08e

Fix runtime issues (findfirst) and missing class names for String and…

2e8ea2b

… Supplier

Filter out fail method invocation from replacement

8560cd2

Fix quickfix fail filterign & try to test quickfix

186df6f

Improve test (still failing), add comment how to generate quickfix co…

cfa4b99

…rrectly

Refine implementation plan for 3 steps replacement for excluding fail…

dc9c19f

… method invocation

3 steps edit implementation

b34d851

Fix tests, try to remove semicolon after fail in quickfix (dont work)

e8cfbb2

Fix : report only on junit5 tests / assertJ fail

3c122bc

rombirli temporarily deployed to sca-checking June 8, 2026 08:05 — with GitHub Actions Inactive

gitar-bot Bot reviewed Jun 8, 2026

View reviewed changes

Comment thread ...sources/default/src/test/java/checks/tests/AssertThrowsInsteadOfTryCatchFailCheckSample.java Outdated

gitar-bot Bot reviewed Jun 8, 2026

View reviewed changes

Comment thread java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java

gitar-bot Bot reviewed Jun 8, 2026

View reviewed changes

Comment thread java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java

gitar-bot Bot reviewed Jun 8, 2026

View reviewed changes

Comment thread java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java Outdated

rombirli requested a review from tomasz-tylenda-sonarsource June 8, 2026 08:12

remove import *

d6890e4

rombirli temporarily deployed to sca-checking June 8, 2026 08:14 — with GitHub Actions Inactive

NonCompliant -> Noncompliant

66841ff

rombirli temporarily deployed to sca-checking June 8, 2026 08:15 — with GitHub Actions Inactive

Support Supplier<String> as argument for junit fail

da01116

rombirli temporarily deployed to sca-checking June 8, 2026 08:47 — with GitHub Actions Inactive

Remove import * in TryCatchUtils.java

cbad931

rombirli temporarily deployed to sca-checking June 8, 2026 08:49 — with GitHub Actions Inactive

Remove import * in TryCatchUtils.java

5d27fcd

rombirli temporarily deployed to sca-checking June 8, 2026 08:50 — with GitHub Actions Inactive

tomasz-tylenda-sonarsource reviewed Jun 8, 2026

View reviewed changes

Avoid NPE crash when there is no catch

0336d07

rombirli temporarily deployed to sca-checking June 8, 2026 09:22 — with GitHub Actions Inactive

rombirli changed the title ~~SONARJAVA-6406 : Rule S8714 - implement quickfix~~ SONARJAVA-6454 : Rule S8714 - implement quickfix Jun 8, 2026

Clarify intentionality of sample changes

8197a53

rombirli temporarily deployed to sca-checking June 8, 2026 14:20 — with GitHub Actions Inactive

Fix autoscan

5bc8f38

rombirli temporarily deployed to sca-checking June 8, 2026 14:21 — with GitHub Actions Inactive

FIX (Address comments, refactor, ...) TryCatchUtils and unit test it

daa7e91

rombirli temporarily deployed to sca-checking June 8, 2026 14:44 — with GitHub Actions Inactive

gitar-bot Bot reviewed Jun 8, 2026

View reviewed changes

Comment thread java-checks/src/test/java/org/sonar/java/checks/helpers/TryCatchUtilsTest.java Outdated

Address comment : "The fourth argument is a function of the third. Le…

25ee664

…t's remove it."

rombirli temporarily deployed to sca-checking June 8, 2026 14:52 — with GitHub Actions Inactive

Fix QG : use !isEmpty() instead of size() > 0

d8a0c38

rombirli temporarily deployed to sca-checking June 8, 2026 14:54 — with GitHub Actions Inactive

Remove Leftover debug System.out.println in new test

1affe0f

rombirli temporarily deployed to sca-checking June 8, 2026 14:55 — with GitHub Actions Inactive

rombirli requested a review from tomasz-tylenda-sonarsource June 8, 2026 16:11

Conversation

rombirli commented Jun 8, 2026 • edited by gitar-bot Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Gitar

Uh oh!

sonarqubecloud Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Agentic Analysis: Early Results

Uh oh!

hashicorp-vault-sonar-prod Bot commented Jun 8, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tomasz-tylenda-sonarsource left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tomasz-tylenda-sonarsource Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tomasz-tylenda-sonarsource Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

rombirli Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

tomasz-tylenda-sonarsource Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

rombirli Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gitar-bot Bot commented Jun 8, 2026

Uh oh!

sonarqube-next Bot commented Jun 8, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rombirli commented Jun 8, 2026 •

edited by gitar-bot Bot

Loading

sonarqubecloud Bot commented Jun 8, 2026 •

edited

Loading

hashicorp-vault-sonar-prod Bot commented Jun 8, 2026 •

edited by atlassian Bot

Loading