SONARJAVA-7152 S1695: Update issue message to suggest Objects.requireNonNull()

[romainbrenguier]

Updates the S1695 issue message to: Use Objects.requireNonNull() instead of explicitly throwing NullPointerException.

The test sample expectations are updated accordingly.

Issues raised on throws NullPointerException clauses use a dedicated message: Remove NullPointerException from the throws clause.

The rule description (S1695.html) is not changed here. It is being updated on RSPEC in https://github.com/SonarSource/rspec/pull/8333 so that it matches the new message; the metadata will be synced from RSPEC once that PR is merged.

🤖 Generated with Claude Code

Agent workflow

Tool link: https://github.com/SonarSource/languages-experimental-tooling/tree/romain/my-tickets/personal/romain-brenguier
PR created using uv run create_with_claude.py update the issue message in S1695 to be: Use Objects.requireNonNull() instead of explicitly throwing NullPointerException.

PR updated using uv run update_with_claude.py --prompt "Implement the action plan described in the document Code Review: Updates the S1695 issue message to suggest Objects.requireNonNull()instead of explicitly throwingNullPointerException`, with test sample expectations adjusted to match.

• The new message does not fit throws NullPointerException clauses in NPEThrowCheck.java, which still raise the same issue; a separate message for that case is suggested.
• The message conflicts with the bundled S1695 rule description, which recommends @NotNull and never mentions Objects.requireNonNull(); update the RSPEC metadata in the same change or link an RSPEC update.

1. 💡 Quality: New message doesn't fit issues raised on throws NullPointerException clauses
   Files: java-checks/src/main/java/org/sonar/java/checks/NPEThrowCheck.java:45-47, java-checks/src/main/java/org/sonar/java/checks/NPEThrowCheck.java:53, java-checks-test-sources/default/src/main/java/checks/NPEThrowCheckSample.java:4

   raiseIssueOnNpe is also called for every throws clause entry (NPEThrowCheck.java:45-47), and the sample still checks that case (void foo() throws NullPointerException, line 4). There, the new message "Use Objects.requireNonNull() instead of explicitly throwing NullPointerException." doesn't apply: the method throws nothing explicitly, and calling Objects.requireNonNull() wouldn't clear the issue. The fix is to remove the unchecked exception from the throws clause. Consider a separate message for the throws-clause case, e.g. "Remove NullPointerException from the throws clause."

   Fix (Pass a context-specific message for the throws clause):
   @OverRide
   public void visitNode(Tree tree) {
   if (tree.is(Tree.Kind.THROW_STATEMENT)) {
   ExpressionTree expressionTree = ((ThrowStatementTree) tree).expression();
   raiseIssueOnNpe(expressionTree, expressionTree.symbolType(), "Use Objects.requireNonNull() instead of explicitly throwing NullPointerException.");
   } else {
   for (TypeTree throwClause : ((MethodTree) tree).throwsClauses()) {
   raiseIssueOnNpe(throwClause, throwClause.symbolType(), "Remove NullPointerException from the throws clause.");
   }
   }
   }

2. 💡 Quality: Issue message contradicts the shipped S1695 rule description
   Files: java-checks/src/main/java/org/sonar/java/checks/NPEThrowCheck.java:53

   The bundled description (S1695.html) says purposely throwing an NPE is bad practice and that callers should "use the @NotNull annotation instead". Its compliant example only shows @NotNull. The new message instead tells users to call Objects.requireNonNull(), which purposely throws an NPE. So a user who opens the rule description from the issue gets the opposite advice. Update the RSPEC metadata (S1695.html/json) in the same change, or link the RSPEC update, so the message and the description agree.." -a "Code Review: Updates the S1695 issue message to suggest Objects.requireNonNull() instead of explicitly throwing NullPointerException, with test sample expectations adjusted to match.

• The new message does not fit throws NullPointerException clauses in NPEThrowCheck.java, which still raise the same issue; a separate message for that case is suggested.
• The message conflicts with the bundled S1695 rule description, which recommends @NotNull and never mentions Objects.requireNonNull(); update the RSPEC metadata in the same change or link an RSPEC update.

1. 💡 Quality: New message doesn't fit issues raised on throws NullPointerException clauses
   Files: java-checks/src/main/java/org/sonar/java/checks/NPEThrowCheck.java:45-47, java-checks/src/main/java/org/sonar/java/checks/NPEThrowCheck.java:53, java-checks-test-sources/default/src/main/java/checks/NPEThrowCheckSample.java:4

   raiseIssueOnNpe is also called for every throws clause entry (NPEThrowCheck.java:45-47), and the sample still checks that case (void foo() throws NullPointerException, line 4). There, the new message "Use Objects.requireNonNull() instead of explicitly throwing NullPointerException." doesn't apply: the method throws nothing explicitly, and calling Objects.requireNonNull() wouldn't clear the issue. The fix is to remove the unchecked exception from the throws clause. Consider a separate message for the throws-clause case, e.g. "Remove NullPointerException from the throws clause."

   Fix (Pass a context-specific message for the throws clause):
   @OverRide
   public void visitNode(Tree tree) {
   if (tree.is(Tree.Kind.THROW_STATEMENT)) {
   ExpressionTree expressionTree = ((ThrowStatementTree) tree).expression();
   raiseIssueOnNpe(expressionTree, expressionTree.symbolType(), "Use Objects.requireNonNull() instead of explicitly throwing NullPointerException.");
   } else {
   for (TypeTree throwClause : ((MethodTree) tree).throwsClauses()) {
   raiseIssueOnNpe(throwClause, throwClause.symbolType(), "Remove NullPointerException from the throws clause.");
   }
   }
   }

2. 💡 Quality: Issue message contradicts the shipped S1695 rule description
   Files: java-checks/src/main/java/org/sonar/java/checks/NPEThrowCheck.java:53

   The bundled description (S1695.html) says purposely throwing an NPE is bad practice and that callers should "use the @NotNull annotation instead". Its compliant example only shows @NotNull. The new message instead tells users to call Objects.requireNonNull(), which purposely throws an NPE. So a user who opens the rule description from the issue gets the opposite advice. Update the RSPEC metadata (S1695.html/json) in the same change, or link the RSPEC update, so the message and the description agree." -g "claude"`

PR updated using uv run update_with_claude.py --prompt "Implement the action plan described in the document Revert the changes to the html and instead point the reviewer to PR https://github.com/SonarSource/rspec/pull/8333 on RSPEC.." -a "Revert the changes to the html and instead point the reviewer to PR https://github.com/SonarSource/rspec/pull/8333 on RSPEC." -g "claude"

PR updated using uv run update_with_claude.py --prompt "Implement the action plan described in the document We should not raise when the thrown NullPointerException as a message argument. Add a test case for that case.." -a "We should not raise when the thrown NullPointerException as a message argument. Add a test case for that case." -g "claude"

[hashicorp-vault-sonar-prod]

SONARJAVA-7152

[rombirli]

the ticket is about adding this rule to sonar-way, which i wasn't able to see in this PR

[gitar-bot]

CI failed: The Ruling Update job failed while building an automated fix PR: applying the generated S1695 ruling patch hit a merge conflict in commons-beanutils expected results, and a second ruling QA job shows no clear failure in its excerpt. Neither failure is shown to be caused by this PR's S1695 message change.

Overview

Two unique error templates were analyzed from 3 logs. One is a concrete tooling failure in the Ruling Update and Notify job, where git apply --3way --index conflicted on its/ruling/src/test/resources/expected/commons-beanutils/java-S1695.json. The other is a Windows ruling QA job whose excerpt shows no failing step, so no root cause can be established from it. Overall, the logs show a ruling-related problem but do not establish that this PR caused it.

Failures

Ruling fix-PR patch conflict on S1695 expected results (confidence: high)

• Type: tooling
• Affected jobs: 115381841971
• Related to change: unclear
• Root cause: Ruling QA had already reported failure. The automated fix-PR step then failed because the generated patch conflicted on the S1695 expected ruling file for commons-beanutils. The log does not show the patch contents or why the conflict occurred, so it does not confirm that the S1695 message change caused it.
• Suggested fix: Inspect the generated ruling diff and the conflicting commons-beanutils/java-S1695.json, resolve the conflict while keeping the intended S1695 result, then commit the ruling update or rerun the workflow. Check the ruling QA artifacts to find the original QA failure.

Ruling QA job with no visible failing step (confidence: low)

• Type: other
• Affected jobs: 115372389122
• Related to change: unclear
• Root cause: The excerpt shows setup, ruling diff generation, artifact upload, and cleanup completing successfully. It contains no test assertion, ruling mismatch, non-zero exit, or failed action. The job is a ruling QA job and this PR touches S1695, but that overlap alone does not establish a PR-caused failure.
• Suggested fix: Retrieve the full job log, especially the ruling execution and final job conclusion, and review the uploaded diff artifact for expected-versus-actual mismatches.

Summary

• Change-related failures: None confirmed. The S1695 ruling conflict is plausibly related to this PR's S1695 changes, but the verdict is unclear.
• Infrastructure/flaky failures: None identified.
• Recommended action: Review the S1695 ruling diff and conflicting expected file, then get the complete ruling QA log to confirm whether the S1695 behavior change is responsible.

Code Review ✅ Approved 2 closed / 2 findings

🟡 Medium risk · Changing S1695 to skip message-bearing NullPointerException throws and revise diagnostics could suppress findings or alter guidance for analyzed code.

Updates the S1695 issue message to suggest Objects.requireNonNull() instead of explicitly throwing NullPointerException, with a dedicated message Remove NullPointerException from the throws clause. for throws clauses and no issue raised when the thrown NPE has a message argument. The rule description update is handled in RSPEC PR 8333. Both previously raised findings were resolved, and no open issues remain.

✅ 2 closed

✅ Quality: New message doesn't fit issues raised on throws NullPointerException clauses

📄 java-checks/src/main/java/org/sonar/java/checks/NPEThrowCheck.java:45-47 📄 java-checks/src/main/java/org/sonar/java/checks/NPEThrowCheck.java:53 📄 java-checks-test-sources/default/src/main/java/checks/NPEThrowCheckSample.java:4
raiseIssueOnNpe is also called for every throws clause entry (NPEThrowCheck.java:45-47), and the sample still checks that case (void foo() throws NullPointerException, line 4). There, the new message "Use Objects.requireNonNull() instead of explicitly throwing NullPointerException." doesn't apply: the method throws nothing explicitly, and calling Objects.requireNonNull() wouldn't clear the issue. The fix is to remove the unchecked exception from the throws clause. Consider a separate message for the throws-clause case, e.g. "Remove NullPointerException from the throws clause."

✅ Quality: Issue message contradicts the shipped S1695 rule description

📄 java-checks/src/main/java/org/sonar/java/checks/NPEThrowCheck.java:53
The bundled description (S1695.html) says purposely throwing an NPE is bad practice and that callers should "use the @NotNull annotation instead". Its compliant example only shows @NotNull. The new message instead tells users to call Objects.requireNonNull(), which purposely throws an NPE. So a user who opens the rule description from the issue gets the opposite advice. Update the RSPEC metadata (S1695.html/json) in the same change, or link the RSPEC update, so the message and the description agree.

Verification Repos 3 inspected

Functional validation No results

Rules No rules evaluated

Cross-repo 3 repositories inspected 0 open impacts

SonarSource/sonarlint-intellij Inspected
SonarSource/sonarlint-eclipse Inspected
SonarSource/sonarlint-vscode Inspected

Auto-approval Not enabled

Set up

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 → 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]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

[datadog-sonarsource]

❌ Errors

Your PR has failed checks. Please review the issues below and take necessary action before merging.

🚦 3 Pipeline jobs failed

Build | Ruling QA (warp-custom-ubuntu-24-04, without-sonarqube-project, LATEST_RELEASE) — 🔧 Needs a code fix, caused by this PR

View more details · View in GitHub Actions

Build | Ruling QA (warp-custom-windows-2022-l, without-sonarqube-project, LATEST_RELEASE) — 🔧 Needs a code fix, caused by this PR

View more details · View in GitHub Actions

Build | Ruling Update and Notify

View more details · View in GitHub Actions

Useful? React with 👍 / 👎

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 7eda71f | Docs | View more details | Give us feedback!}