SONARJAVA-6844: Implement S9394: "contentEquals()" should be used instead of "equals()" for "CharSequence" comparisons

[nathsou]

Summary

This PR implements rule S9394 (StringEqualsCharSequenceCheck): "contentEquals()" should be used instead of "equals()" for "CharSequence" comparisons.

String.equals(Object) requires an argument of type String and always returns false at runtime when invoked with non-String CharSequence implementations (e.g. StringBuilder, StringBuffer, CharBuffer, or interface CharSequence), even when representing the identical character sequence. To compare textual contents, contentEquals() should be used instead.

• Detects equals(Object) calls where the receiver is a String and the argument is a non-null, non-String subtype of CharSequence.
• Provides an automated JavaQuickFix replacing equals with contentEquals.
• Includes unit tests with and without semantic resolution (StringEqualsCharSequenceCheckTest and StringEqualsCharSequenceCheckSample).
• Adds rule metadata and enables rule in the Sonar way quality profile.

References

• Jira: SONARJAVA-6844
• RSPEC PR: https://github.com/SonarSource/rspec/pull/8174

AI disclosure

LLM model used for implementation: gemini-3.8-flash-high

[hashicorp-vault-sonar-prod]

SONARJAVA-6844

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

[datadog-sonarsource]

⚠️ Warnings

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

🚦 1 Pipeline job failed

Build | Test Analyze

View in Datadog · View in GitHub Actions

Error during SonarScanner Engine execution. Fail to download plugin [security].

Useful? React with 👍 / 👎

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

[gitar-bot]

⚠️ Bug: S2159 deferral loses issues S9394 cannot report (String-bounded receivers)

The new early return in checkWhenOwnerIsNotArray uses the erased receiver type, so it also silences receivers whose static type is a String-bounded type variable or capture (e.g. <T extends String> boolean f(T t, StringBuilder sb) { return t.equals(sb); }, or stringList.get(0).equals(sb) with List<? extends String>): erasure(T) is java.lang.String, so S2159 returns. S9394 cannot pick these up, because MethodMatchers.ofTypes("java.lang.String") tests the call-site type with exact Type.is() (MethodMatchersBuilder#getCallSiteType/ofTypes), and a type variable/capture is not is("java.lang.String"). Result: a comparison that always returns false at runtime was reported before this PR and is now reported by no rule; switching the matcher to ofSubTypes("java.lang.String") closes the gap (and the receiver-side generic case deserves a sample line, since the new samples only exercise the argument side).

Match String-bounded type variables/captures as receivers in StringEqualsCharSequenceCheck so the cases S2159 now defers are still reported (add a <T extends String> ... t.equals(sb) Noncompliant line to StringEqualsCharSequenceCheckSample).:

private static final MethodMatchers EQUALS_MATCHER = MethodMatchers.create()
  .ofSubTypes("java.lang.String")
  .names("equals")
  .addParametersMatcher("java.lang.Object")
  .build();

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

💡 Quality: New compliant sample lines lack "// Compliant" annotation

string.equals(stringBuilder); is added with no marker, while every other non-reporting line in SillyEqualsCheckSample carries // Compliant plus a reason; the same applies to the new compliant lines in StringEqualsCharSequenceCheckSample (str.equals(genericStr), str.equals(stringList.get(0))). Without the marker a reader cannot tell whether the absence of an expectation is intentional or a forgotten // Noncompliant, which matters here because the S2159 line encodes the deliberate deferral to S9394.

Annotate the intentionally compliant line in SillyEqualsCheckSample (and add // Compliant to the two new compliant lines in StringEqualsCharSequenceCheckSample).:

string.equals(stringBuilder); // Compliant, reported by S9394 instead

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

CI failed: Maven build failed due to a server-side 503 error while downloading the SonarQube security plugin during the scanner analysis phase.

Overview

1 CI job failed due to a temporary infrastructure/server issue preventing the SonarScanner from downloading a required plugin (HTTP 503).

Failures

SonarScanner Plugin Download Failure (confidence: high)

• Type: infrastructure
• Affected jobs: 104018060130
• Related to change: no
• Root cause: The SonarQube server returned a 503 Service Unavailable error when attempting to download the 'security' plugin during the scanner execution step.
• Suggested fix: Re-run the CI job to retry the download or check the availability of the SonarQube server.

Summary

• Change-related failures: 0
• Infrastructure/flaky failures: 1 failure due to a server-side HTTP 503 error when downloading a plugin.
• Recommended action: Re-trigger the failed workflow job.

Code Review ⚠️ Changes requested 3 resolved / 5 findings

Implements rule S9394 to detect equals() calls on String receivers with CharSequence arguments and suggest contentEquals() instead. However, the deferral from S2159 uses exact type matching that loses coverage for String-bounded type variables and wildcards (e.g., <T extends String> boolean f(T t, StringBuilder sb) { return t.equals(sb); }), causing such cases to go unreported. Switch the S9394 matcher to ofSubTypes("java.lang.String") to close the gap. Additionally, new compliant sample lines lack // Compliant annotations, making it unclear whether the absence of expectations is intentional.

⚠️ Bug: S2159 deferral loses issues S9394 cannot report (String-bounded receivers)

📄 java-checks/src/main/java/org/sonar/java/checks/SillyEqualsCheck.java:85-86 📄 java-checks/src/main/java/org/sonar/java/checks/StringEqualsCharSequenceCheck.java:35-39

The new early return in checkWhenOwnerIsNotArray uses the erased receiver type, so it also silences receivers whose static type is a String-bounded type variable or capture (e.g. <T extends String> boolean f(T t, StringBuilder sb) { return t.equals(sb); }, or stringList.get(0).equals(sb) with List<? extends String>): erasure(T) is java.lang.String, so S2159 returns. S9394 cannot pick these up, because MethodMatchers.ofTypes("java.lang.String") tests the call-site type with exact Type.is() (MethodMatchersBuilder#getCallSiteType/ofTypes), and a type variable/capture is not is("java.lang.String"). Result: a comparison that always returns false at runtime was reported before this PR and is now reported by no rule; switching the matcher to ofSubTypes("java.lang.String") closes the gap (and the receiver-side generic case deserves a sample line, since the new samples only exercise the argument side).

Match String-bounded type variables/captures as receivers in StringEqualsCharSequenceCheck so the cases S2159 now defers are still reported (add a `<T extends String> ... t.equals(sb)` Noncompliant line to StringEqualsCharSequenceCheckSample).

private static final MethodMatchers EQUALS_MATCHER = MethodMatchers.create()
  .ofSubTypes("java.lang.String")
  .names("equals")
  .addParametersMatcher("java.lang.Object")
  .build();

💡 Quality: New compliant sample lines lack "// Compliant" annotation

📄 java-checks-test-sources/default/src/main/java/checks/SillyEqualsCheckSample.java:45 📄 java-checks-test-sources/default/src/main/java/checks/StringEqualsCharSequenceCheckSample.java:49 📄 java-checks-test-sources/default/src/main/java/checks/StringEqualsCharSequenceCheckSample.java:60

string.equals(stringBuilder); is added with no marker, while every other non-reporting line in SillyEqualsCheckSample carries // Compliant plus a reason; the same applies to the new compliant lines in StringEqualsCharSequenceCheckSample (str.equals(genericStr), str.equals(stringList.get(0))). Without the marker a reader cannot tell whether the absence of an expectation is intentional or a forgotten // Noncompliant, which matters here because the S2159 line encodes the deliberate deferral to S9394.

Annotate the intentionally compliant line in SillyEqualsCheckSample (and add `// Compliant` to the two new compliant lines in StringEqualsCharSequenceCheckSample).

string.equals(stringBuilder); // Compliant, reported by S9394 instead

✅ 3 resolved

✅ Bug: S9394 double-reports with S2159 on String.equals(StringBuilder)

📄 java-checks/src/main/java/org/sonar/java/checks/StringEqualsCharSequenceCheck.java:53-62 📄 java-checks-test-sources/default/src/main/java/checks/StringEqualsCharSequenceCheckSample.java:8 📄 java-checks-test-sources/default/src/main/java/checks/StringEqualsCharSequenceCheckSample.java:13 📄 java-checks-test-sources/default/src/main/java/checks/StringEqualsCharSequenceCheckSample.java:23 📄 java-checks-test-sources/default/src/main/java/checks/StringEqualsCharSequenceCheckSample.java:28
For str.equals(sb) the existing S2159 (SillyEqualsCheck) already reports: owner erasure is java.lang.String, argument erasure java.lang.StringBuilder is a class, areNotRelated is true (neither is a subtype of the other) and areNeitherInterfaces is true, so it raises "comparisons between unrelated types always return false" — the same path that flags string.equals(1) in SillyEqualsCheckSample.java:43. S2159 is in Sonar way (profiles/Sonar_way/S2159), so four of the five noncompliant lines of the new sample (StringBuilder, StringBuffer, CharBuffer, and the literal receiver) will now produce two issues on the same identifier with contradictory advice; only the CharSequence-interface argument (String is a subtype of CharSequence, so areNotRelated is false) is unique to S9394. Either narrow S2159 to skip String receivers with CharSequence arguments so S9394 owns that pattern, or restrict S9394 to the cases S2159 cannot see.

✅ Edge Case: Exact is("java.lang.String") misses String-bounded/captured types

📄 java-checks/src/main/java/org/sonar/java/checks/StringEqualsCharSequenceCheck.java:53 📄 java-checks-test-sources/default/src/main/java/checks/StringEqualsCharSequenceCheckSample.java:33-42
The exclusion uses exact type identity, so an argument whose static type is a type variable or capture bounded by String (e.g. <T extends String> boolean eq(String s, T t) { return s.equals(t); }, or list.get(0) on a List<? extends String>) is not is("java.lang.String") but is a subtype of CharSequence, producing a false positive on a comparison that behaves correctly. Using !argumentType.isSubtypeOf("java.lang.String") covers both String itself and String-bounded types; the sample has no coverage for generic/user-defined CharSequence arguments, so add such cases.

✅ Edge Case: Quick fix can turn a false result into a NullPointerException

📄 java-checks/src/main/java/org/sonar/java/checks/StringEqualsCharSequenceCheck.java:50-61 📄 java-checks-test-sources/default/src/main/java/checks/StringEqualsCharSequenceCheckSample.java:8-11
String.equals(null) returns false, but String.contentEquals(null) dereferences the argument (cs.length()) and throws NullPointerException. The guard only rejects the null literal and the null type, so for StringBuilder sb = maybeNull(); if (str.equals(sb)) — which today evaluates to false — applying the offered quick fix makes the code throw at runtime. Either mark the quick fix as non-behaviour-preserving guidance (report without an automated fix when nullness of the argument is not provable) or gate the fix on an argument that is known non-null; the PR description already claims a "non-null" argument, which the current condition does not establish.

🤖 Prompt for agents

Code Review: Implements rule S9394 to detect `equals()` calls on `String` receivers with `CharSequence` arguments and suggest `contentEquals()` instead. However, the deferral from S2159 uses exact type matching that loses coverage for String-bounded type variables and wildcards (e.g., `<T extends String> boolean f(T t, StringBuilder sb) { return t.equals(sb); }`), causing such cases to go unreported. Switch the S9394 matcher to `ofSubTypes("java.lang.String")` to close the gap. Additionally, new compliant sample lines lack `// Compliant` annotations, making it unclear whether the absence of expectations is intentional.

1. ⚠️ Bug: S2159 deferral loses issues S9394 cannot report (String-bounded receivers)
   Files: java-checks/src/main/java/org/sonar/java/checks/SillyEqualsCheck.java:85-86, java-checks/src/main/java/org/sonar/java/checks/StringEqualsCharSequenceCheck.java:35-39

   The new early return in `checkWhenOwnerIsNotArray` uses the *erased* receiver type, so it also silences receivers whose static type is a String-bounded type variable or capture (e.g. `<T extends String> boolean f(T t, StringBuilder sb) { return t.equals(sb); }`, or `stringList.get(0).equals(sb)` with `List<? extends String>`): erasure(T) is `java.lang.String`, so S2159 returns. S9394 cannot pick these up, because `MethodMatchers.ofTypes("java.lang.String")` tests the *call-site* type with exact `Type.is()` (MethodMatchersBuilder#getCallSiteType/ofTypes), and a type variable/capture is not `is("java.lang.String")`. Result: a comparison that always returns `false` at runtime was reported before this PR and is now reported by no rule; switching the matcher to `ofSubTypes("java.lang.String")` closes the gap (and the receiver-side generic case deserves a sample line, since the new samples only exercise the argument side).

   Fix (Match String-bounded type variables/captures as receivers in StringEqualsCharSequenceCheck so the cases S2159 now defers are still reported (add a `<T extends String> ... t.equals(sb)` Noncompliant line to StringEqualsCharSequenceCheckSample).):
   private static final MethodMatchers EQUALS_MATCHER = MethodMatchers.create()
     .ofSubTypes("java.lang.String")
     .names("equals")
     .addParametersMatcher("java.lang.Object")
     .build();

2. 💡 Quality: New compliant sample lines lack "// Compliant" annotation
   Files: java-checks-test-sources/default/src/main/java/checks/SillyEqualsCheckSample.java:45, java-checks-test-sources/default/src/main/java/checks/StringEqualsCharSequenceCheckSample.java:49, java-checks-test-sources/default/src/main/java/checks/StringEqualsCharSequenceCheckSample.java:60

   `string.equals(stringBuilder);` is added with no marker, while every other non-reporting line in SillyEqualsCheckSample carries `// Compliant` plus a reason; the same applies to the new compliant lines in StringEqualsCharSequenceCheckSample (`str.equals(genericStr)`, `str.equals(stringList.get(0))`). Without the marker a reader cannot tell whether the absence of an expectation is intentional or a forgotten `// Noncompliant`, which matters here because the S2159 line encodes the deliberate deferral to S9394.

   Fix (Annotate the intentionally compliant line in SillyEqualsCheckSample (and add `// Compliant` to the two new compliant lines in StringEqualsCharSequenceCheckSample).):
   string.equals(stringBuilder); // Compliant, reported by S9394 instead

Implementation Status ✅ 1 of 1 objectives covered

✅ SONARJAVA-6844 - 1 of 1 objectives covered

This PR implements rule S9394 to flag usage of equals() instead of contentEquals() for CharSequence comparisons.

✅ 1 covered here

• ✅ Implement rule S9394 to flag usage of equals() instead of contentEquals() for CharSequence comparisons

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.
Unblock → Override a blocking verdict and allow merging.

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

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

_{Was this helpful? React with 👍 / 👎 | Gitar}