Skip to content

SONARJAVA-6673 S1602: Fix false positives - #6113

Open
asya-vorobeva wants to merge 2 commits into
masterfrom
asya/fix-s1602-fps
Open

SONARJAVA-6673 S1602: Fix false positives#6113
asya-vorobeva wants to merge 2 commits into
masterfrom
asya/fix-s1602-fps

Conversation

@asya-vorobeva

@asya-vorobeva asya-vorobeva commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Stop raising issues for Spring JDBC query overloads, MethodHandle signature-polymorphic invocations, and multiline lambda bodies

…oads, MethodHandle signature-polymorphic invocations, and multiline lambda bodies

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

SONARJAVA-6673

@github-actions

Copy link
Copy Markdown
Contributor

Ruling Diff Summary

Detected changes in 2 rule files: 2 issues removed, 0 issues added.

S1602 (java) on eclipse-jetty - 1 issues removed, 0 issues added - deleted ruling file

Removed jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java (line 164)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java)
S1602 (java) on eclipse-jetty-similar-to-main - 1 issues removed, 0 issues added - deleted ruling file

Removed jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java (line 164)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java)

…er lambdas and update rule description

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@gitar-bot

gitar-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 4 resolved / 4 findings

Fixes false positives in S1602 for Spring JDBC query overloads, MethodHandle signature-polymorphic invocations, and multiline lambda bodies. Multiline-body exemption contradicts the rule description and should be reconsidered, Spring JDBC exemption is broader than necessary for the ambiguity it addresses, and test coverage was reduced with removal of nested blocks and an untested return branch.

✅ 4 resolved
Bug: Multiline-body exemption contradicts the S1602 description

📄 java-checks/src/main/java/org/sonar/java/checks/LambdaSingleExpressionCheck.java:101-104 📄 java-checks-test-sources/default/src/main/java/checks/LambdaSingleExpressionCheckNoVersionSample.java:10-12
hasMultilineBody silences the rule for every one-statement block lambda whose braces sit on different lines, while S1602.html still states that "when the code block consists of only one statement (which may or may not be a return statement), it can be rewritten using expression notation" with no formatting exception. A user writing the documented noncompliant example across two lines (x -> { System.out.println(x+1); }) now gets no issue, which is the common formatting in real code (the previously-expected jetty issue and the no_version sample expectation were both dropped for exactly that reason). Either restrict the exemption to cases the rule genuinely cannot simplify, or update the rule description/RSPEC so the documented behaviour matches the implementation.

Bug: Spring JDBC exemption is broader than the ambiguity it fixes

📄 java-checks/src/main/java/org/sonar/java/checks/LambdaSingleExpressionCheck.java:106-113
isInsideSpringJdbcQuery suppresses every block lambda that is a direct argument of any query overload on JdbcOperations/NamedParameterJdbcOperations, not only the one-parameter, value-producing case that is ambiguous between RowCallbackHandler and ResultSetExtractor. Unambiguous targets are now silently missed, e.g. jdbc.query(sql, (rs, rowNum) -> { return rs.getString(1); }) (arity 2, only RowMapper applies) or jdbc.query(con -> { return con.prepareStatement(sql); }, mapper). Narrow the exemption to single-parameter lambdas whose single statement is an expression statement with a non-void type.

Quality: Test coverage lost: nested blocks removed, return branch untested

📄 java-checks-test-sources/default/src/main/java/checks/LambdaSingleExpressionCheckNoVersionSample.java:27-28 📄 java-checks/src/main/java/org/sonar/java/checks/LambdaSingleExpressionCheck.java:115-125 📄 java-checks-test-sources/default/src/main/java/checks/LambdaSingleExpressionCheckSample.java:52-56
The renamed sample drops the "Nested blocks" case, so the recursive isBlockWithOneStatement/isRefactorizable path (still reachable, e.g. x -> { { { return x + 1; } } } on one line) is no longer exercised by any test. Likewise the Tree.Kind.RETURN_STATEMENT branch of the new isSingleMethodHandleInvocation has no sample (() -> { return handle.invoke(); }), so half of the new code is unverified. Re-add a single-line nested-block case and a MethodHandle invocation in return position.

Bug: Without-semantic sample misses two expected issues (test fails)

📄 java-checks-test-sources/default/src/main/java/checks/LambdaSingleExpressionCheckSampleWithoutSemantic.java:28-31 📄 java-checks/src/main/java/org/sonar/java/checks/LambdaSingleExpressionCheck.java:115-125 📄 java-checks/src/test/java/org/sonar/java/checks/LambdaSingleExpressionCheckTest.java:43-50
In LambdaSingleExpressionCheckSampleWithoutSemantic.java the two MethodHandle lambdas on lines 29-30 are single-line one-statement blocks, and METHOD_HANDLE_INVOKE_MATCHER cannot match when symbols are unknown (the same reason lines 23/25 are annotated // Noncompliant). The check therefore raises an issue on both lines, but neither carries a // Noncompliant marker, so LambdaSingleExpressionCheckTest.test_without_semantic() fails with unexpected issues. Add the markers (or exclude the two calls from this sample).

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

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

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

Copy link
Copy Markdown
Contributor

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