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"

PR updated using uv run update_with_claude.py --prompt "Implement the action plan described in the document Use the rule-api to update the description and metadata for S1695 to the latest version of master on RSPEC.." -a "Use the rule-api to update the description and metadata for S1695 to the latest version of master on RSPEC." -g "claude"

PR updated using uv run update_with_claude.py --prompt "Implement the action plan described in the document Use the rule-api to update the description and metadata for S1695 to the latest version of master on RSPEC.." -a "Use the rule-api to update the description and metadata for S1695 to the latest version of master on RSPEC." -g "claude"

PR updated using uv run update_with_claude.py --prompt "Implement the action plan described in the document Fetch and rebase on origin/master." -a "Fetch and rebase on origin/master" -g "claude"

---

Summary by Gitar

• Behavior change:
  • Skip raising the issue when NullPointerException is created with a message argument
• Rule profile and metadata:
  • Added S1695 to the Sonar_way profile and changed the remediation cost to 5min
  • Added cert and convention tags and a CERT ERR08-J security standard mapping
• Rule description:
  • Rewrote S1695.html with new guidance on Objects.requireNonNull(), despite the description saying it is unchanged
• Ruling results:
  • Updated expected ruling results for commons-beanutils, eclipse-jetty, and guava

_{This will update automatically on new commits.}

[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

[datadog-sonarsource]

❌ Errors

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

🚦 2 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)

View more details · View in GitHub Actions

Useful? React with 👍 / 👎

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

[romainbrenguier]

We should merge the rspec change first:
https://github.com/SonarSource/rspec/pull/8333

[github-actions]

⚖️ Ruling update ready for review: #6368

[github-actions]

Ruling Diff Summary

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

S1695 (java) on commons-beanutils - 8 issues removed, 0 issues added

Removed src/main/java/org/apache/commons/beanutils2/BasicDynaBean.java (line 121)

       116 |     @Override
       117 |     public boolean contains(final String name, final String key) {
       118 | 
       119 |         final Object value = values.get(name);
       120 |         if (value == null) {
>>>    121 |             throw new NullPointerException
       122 |                     ("No mapped value for '" + name + "(" + key + ")'");
       123 |         } else if (value instanceof Map) {
       124 |             return ((Map<?, ?>) value).containsKey(key);
       125 |         } else {
       126 |             throw new IllegalArgumentException

Removed src/main/java/org/apache/commons/beanutils2/BasicDynaBean.java (line 202)

       197 |     @Override
       198 |     public Object get(final String name, final int index) {
       199 | 
       200 |         final Object value = values.get(name);
       201 |         if (value == null) {
>>>    202 |             throw new NullPointerException
       203 |                     ("No indexed value for '" + name + "[" + index + "]'");
       204 |         } else if (value.getClass().isArray()) {
       205 |             return Array.get(value, index);
       206 |         } else if (value instanceof List) {
       207 |             return ((List<?>) value).get(index);

Removed src/main/java/org/apache/commons/beanutils2/BasicDynaBean.java (line 234)

       229 |     @Override
       230 |     public Object get(final String name, final String key) {
       231 | 
       232 |         final Object value = values.get(name);
       233 |         if (value == null) {
>>>    234 |             throw new NullPointerException
       235 |                     ("No mapped value for '" + name + "(" + key + ")'");
       236 |         } else if (value instanceof Map) {
       237 |             return ((Map<?, ?>) value).get(key);
       238 |         } else {
       239 |             throw new IllegalArgumentException

Removed src/main/java/org/apache/commons/beanutils2/LazyDynaBean.java (line 501)

       496 | 
       497 |         final DynaProperty descriptor = dynaClass.getDynaProperty(name);
       498 | 
       499 |         if (value == null) {
       500 |             if (descriptor.getType().isPrimitive()) {
>>>    501 |                 throw new NullPointerException
       502 |                         ("Primitive value for '" + name + "'");
       503 |             }
       504 |         } else if (!isAssignable(descriptor.getType(), value.getClass())) {
       505 |             throw new ConversionException
       506 |                     ("Cannot assign value of type '" +

S1695 (java) on eclipse-jetty - 1 issues removed, 0 issues added

Removed jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java (line 43)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/PrecompressedHttpContent.java)

S1695 (java) on guava - 7 issues removed, 0 issues added

Removed src/com/google/common/base/Preconditions.java (line 228)

       223 |    * @return the non-null reference that was validated
       224 |    * @throws NullPointerException if {@code reference} is null
       225 |    */
       226 |   public static <T> T checkNotNull(T reference, @Nullable Object errorMessage) {
       227 |     if (reference == null) {
>>>    228 |       throw new NullPointerException(String.valueOf(errorMessage));
       229 |     }
       230 |     return reference;
       231 |   }
       232 | 
       233 |   /**

Removed src/com/google/common/base/Preconditions.java (line 251)

       246 |    */
       247 |   public static <T> T checkNotNull(
       248 |       T reference, @Nullable String errorMessageTemplate, @Nullable Object... errorMessageArgs) {
       249 |     if (reference == null) {
       250 |       // If either of these parameters is null, the right thing happens anyway
>>>    251 |       throw new NullPointerException(format(errorMessageTemplate, errorMessageArgs));
       252 |     }
       253 |     return reference;
       254 |   }
       255 | 
       256 |   /*

Removed src/com/google/common/collect/CollectPreconditions.java (line 31)

        26 | @GwtCompatible
        27 | final class CollectPreconditions {
        28 | 
        29 |   static void checkEntryNotNull(Object key, Object value) {
        30 |     if (key == null) {
>>>     31 |       throw new NullPointerException("null key in entry: null=" + value);
        32 |     } else if (value == null) {
        33 |       throw new NullPointerException("null value in entry: " + key + "=null");
        34 |     }
        35 |   }
        36 | 

Removed src/com/google/common/collect/CollectPreconditions.java (line 33)

        28 | 
        29 |   static void checkEntryNotNull(Object key, Object value) {
        30 |     if (key == null) {
        31 |       throw new NullPointerException("null key in entry: null=" + value);
        32 |     } else if (value == null) {
>>>     33 |       throw new NullPointerException("null value in entry: " + key + "=null");
        34 |     }
        35 |   }
        36 | 
        37 |   static int checkNonnegative(int value, String name) {
        38 |     if (value < 0) {

Removed src/com/google/common/collect/ImmutableMultimap.java (line 216)

       211 |      *     element in {@code values} is null. The builder is left in an invalid
       212 |      *     state.
       213 |      */
       214 |     public Builder<K, V> putAll(K key, Iterable<? extends V> values) {
       215 |       if (key == null) {
>>>    216 |         throw new NullPointerException("null key in entry: null=" + Iterables.toString(values));
       217 |       }
       218 |       Collection<V> valueList = builderMultimap.get(key);
       219 |       for (V value : values) {
       220 |         checkEntryNotNull(key, value);
       221 |         valueList.add(value);

Removed src/com/google/common/collect/MapMaker.java (line 890)

       885 |         Throwables.propagateIfInstanceOf(cause, ComputationException.class);
       886 |         throw new ComputationException(cause);
       887 |       }
       888 | 
       889 |       if (value == null) {
>>>    890 |         throw new NullPointerException(computingFunction + " returned null for key " + key + ".");
       891 |       }
       892 |       return value;
       893 |     }
       894 |   }
       895 | }

Removed src/com/google/common/collect/ObjectArrays.java (line 240)

       235 | 
       236 |   // We do this instead of Preconditions.checkNotNull to save boxing and array
       237 |   // creation cost.
       238 |   static Object checkElementNotNull(Object element, int index) {
       239 |     if (element == null) {
>>>    240 |       throw new NullPointerException("at index " + index);
       241 |     }
       242 |     return element;
       243 |   }
       244 | }

[gitar-bot]

CI failed: One ruling integration test (JavaRulingTest.eclipse_jetty_incremental) failed with a single unexpected issue difference tied to the S1695 change; the other Ruling QA job shows no visible error in its truncated log.

Overview

Two unique error templates were found across 10 analyzed logs. One is a related ruling-test failure; the other job's log is truncated with no confirmed failing step, so its cause is unknown.

Failures

JavaRulingTest.eclipse_jetty_incremental expects no differences but finds 1 (confidence: medium)

• Type: test
• Affected jobs: 116981682248
• Related to change: yes
• Root cause: The ruling comparison in its/ruling reported "Issues differences: 1" for Eclipse Jetty at JavaRulingTest.java:569. The PR changes S1695 behavior and its message and updates the Jetty baseline, so the diff is plausibly related. The excerpt does not show the actual-vs-expected delta, so it is unclear whether this is an intended result or an incremental-analysis regression.
• Suggested fix: Inspect the actual-vs-expected difference artifact for Eclipse Jetty. If the single delta is the intended S1695 change, update the ruling baseline; otherwise fix the S1695 or incremental-analysis behavior and rerun the test.

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

• Type: other
• Affected jobs: 116981682221
• Related to change: unclear
• Root cause: The visible steps (ruling diff generation, artifact upload, cleanup) complete without error, and cache misses and StepSecurity warnings are not job failures. The log is truncated, so the failing step, if any, is not visible.
• Suggested fix: Review the full job log, especially the section omitted before diff report generation, to identify any nonzero exit. If it is a ruling mismatch, compare actual and expected S1695 results.

Summary

• Change-related failures: 1 (Eclipse Jetty ruling difference for S1695, related to change: yes).
• Infrastructure/flaky failures: none confirmed. The cache misses and warnings seen are not failures.
• Unclear: 1 (Ruling QA job with truncated log, related to change: unclear).
• Recommended action: Check the Eclipse Jetty difference artifact to decide between updating the S1695 ruling baseline and fixing the behavior, and review the full log of the truncated Ruling QA job.

Code Review ✅ Approved 2 closed / 2 findings

🟡 Medium risk · Adding S1695 to Sonar way and suppressing reports for argument-bearing NullPointerException constructors could alter findings and miss manual throws.

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 Rules 1/1 met Repos 3 inspected

Functional validation No results

Rules 1 passed 2 not applicable

✅ Repository license compliance

Broader impact Repos 3 inspected

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

_{For more verification details, comment gitar display:verbose.}

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 compact verification results.

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