JS-1993 S5906: clarify that length matchers apply beyond collections by nathsou · Pull Request #7434 · SonarSource/SonarJS

nathsou · 2026-07-02T09:34:41Z

Summary

Reviewers have reported the S5906 rewrite expect(x.length).toBe(n) → expect(x).toHaveLength(n) (and the Jasmine/Chai equivalents toHaveSize/lengthOf) as a false positive when x isn't literally a collection, e.g. expect(diagram.length).toBe(1400) where diagram is a rendered string.

These matchers are actually generic: Jest/Vitest's toHaveLength and Jasmine's toHaveSize pass whenever the tested value exposes a numeric length/size property with the expected value, whatever that value represents (Vitest's own test suite even asserts expect({ length: 3 }).toHaveLength(3)). The rewrite is behaviorally identical regardless of what x is, so this isn't a detection bug — it's a communication gap.

This PR:

Tags length-based suggestions with kind: 'length' on the Suggestion type (assertion-utils.ts), set at every call site that builds a .length/.size rewrite across rule.ts and assertion-suggestions.ts.
Adds a dedicated preferSpecificLengthAssertion message that explicitly states the matcher works on any value with a numeric length property, not only arrays or collections, used instead of the generic preferSpecificAssertion message for these cases.
Updates existing length-related test expectations across unit.test.ts, chai-bdd.unit.test.ts, and integrations.unit.test.ts to assert the new message id.

RSPEC companion PR (adds the same clarification plus a string-based example to "Why is this an issue?"): https://github.com/SonarSource/rspec/pull/7279

Test plan

npx tsx --test packages/analysis/src/jsts/rules/S5906/*.test.ts — all 3 suites pass
npx tsc --noEmit on the analysis package — no new errors
npx prettier --check on changed files

Reviewers have flagged expect(x.length).toBe(n) -> expect(x).toHaveLength(n) rewrites as incorrect when x isn't a collection, but toHaveLength/toHaveSize/ lengthOf work on any value exposing a numeric length/size property. Tag these suggestions and use a dedicated message that states this explicitly.

hashicorp-vault-sonar-prod · 2026-07-02T10:03:41Z

JS-1993

github-actions · 2026-07-02T10:19:58Z

Ruling Report

Code no longer flagged (72 issues)

S5914

TypeScript/src/harness/unittests/cachingInServerLSHost.ts:134

   132 |                     // trigger synchronization to make sure that LSHost will try to find 'f2' module on disk
   133 |                     project.getLanguageService().getSemanticDiagnostics(imported.name);
>  134 |                     assert.isTrue(false, `should not find file '${imported.name}'`);
   135 |                 }
   136 |                 catch (e) {

TypeScript/src/harness/unittests/moduleResolution.ts:1076

  1074 |                 readFile: fileName => fileName === file.fileName ? file.text : undefined,
  1075 |                 resolveModuleNames() {
> 1076 |                     assert(false, "resolveModuleNames should not be called");
  1077 |                     return undefined;
  1078 |                 }

TypeScript/src/harness/unittests/services/preProcessFile.ts:21

    19 |         }
    20 |         if (!expected) {
>   21 |             assert.isTrue(false, `Expected ${JSON.stringify(expected)}, got ${JSON.stringify(actual)}`);
    22 |         }
    23 |         assert.equal(actual.length, expected.length, `[${kind}] Actual array's length does not match expected length. Expected files: ${JSON.stringify(expected)}, actual files: ${JSON.stringify(actual)}`);

TypeScript/src/harness/unittests/tsserverProjectSystem.ts:3648

  3646 |                 setRequest: requestId => {
  3647 |                     if (expectedRequestId === undefined) {
> 3648 |                         assert.isTrue(false, "unexpected call");
  3649 |                     }
  3650 |                     assert.equal(requestId, expectedRequestId);

TypeScript/src/harness/unittests/typingsInstaller.ts:77

    75 |                 }
    76 |                 installWorker(_requestId: number, _args: string[], _cwd: string, _cb: server.typingsInstaller.RequestCompletedAction) {
>   77 |                     assert(false, "should not be called");
    78 |                 }
    79 |             })();

TypeScript/src/harness/unittests/typingsInstaller.ts:199

   197 |                 }
   198 |                 enqueueInstallTypingsRequest() {
>  199 |                     assert(false, "auto discovery should not be enabled");
   200 |                 }
   201 |             })();

TypeScript/src/harness/unittests/typingsInstaller.ts:227

   225 |                 }
   226 |                 enqueueInstallTypingsRequest() {
>  227 |                     assert(false, "auto discovery should not be enabled");
   228 |                 }
   229 |             })();

TypeScript/src/harness/unittests/typingsInstaller.ts:1000

   998 |                 }
   999 |                 installWorker(_requestId: number, _args: string[], _cwd: string, _cb: server.typingsInstaller.RequestCompletedAction) {
> 1000 |                     assert(false, "runCommand should not be invoked");
  1001 |                 }
  1002 |             })();

console/src/actions/databrowser/tests/data.ts:172

   170 |       })
   171 |       .catch(() => {
>  172 |         expect(false).toBe(true)
   173 |       })
   174 |   })

console/src/actions/databrowser/tests/data.ts:184

   182 |       .catch((e) => {
   183 |         console.error(e)
>  184 |         expect(false).toBe(true)
   185 |       })
   186 |   })

...and 62 more

📋 View full report

Code no longer flagged (72)

S5914

S6582

vitest/packages/vitest/src/node/pool.ts:383

guillemsarda

I left two suggestions. Lmkwyt

Lets report() forward the message id directly instead of re-deriving it from a suggestion "kind", per review feedback.

Drop the "reads better, reports clearer failures" rationale from preferSpecificLengthAssertion per review feedback, keeping only the action and the length-property clarification.

Use "for better reporting" as a compact nod to the shared rationale with preferSpecificAssertion, and restore "numeric" to match the RSPEC wording.

sonarqube-next · 2026-07-06T09:51:18Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

guillemsarda

gitar-bot · 2026-07-06T15:25:29Z

Code Review ✅ Approved

Updates length-based matcher suggestions to include a dedicated message clarifying that these assertions apply to any value with a numeric length property, not just collections. No issues found.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

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}

…7434)

nathsou self-assigned this Jul 2, 2026

nathsou requested a review from a team July 2, 2026 09:34

nathsou added 3 commits July 2, 2026 11:46

S5906: shorten the length-assertion message

205acc5

S5906: explain why the specific assertion is preferred

415852f

S5906: restore the why in the length-assertion message

cdeef29

nathsou requested review from guillemsarda and removed request for a team July 2, 2026 09:54

hashicorp-vault-sonar-prod Bot changed the title ~~S5906: clarify that length matchers apply beyond collections~~ JS-1993 S5906: clarify that length matchers apply beyond collections Jul 2, 2026

guillemsarda reviewed Jul 2, 2026

View reviewed changes

Comment thread packages/analysis/src/jsts/rules/S5906/assertion-utils.ts Outdated

Comment thread packages/analysis/src/jsts/rules/S5906/rule.ts Outdated

nathsou added 3 commits July 6, 2026 11:04

S5906: carry messageId on Suggestion instead of kind

906efd4

Lets report() forward the message id directly instead of re-deriving it from a suggestion "kind", per review feedback.

S5906: shorten the length-assertion message

afe5f82

Drop the "reads better, reports clearer failures" rationale from preferSpecificLengthAssertion per review feedback, keeping only the action and the length-property clarification.

S5906: reword the length-assertion message

9f3698b

Use "for better reporting" as a compact nod to the shared rationale with preferSpecificAssertion, and restore "numeric" to match the RSPEC wording.

nathsou requested a review from guillemsarda July 6, 2026 09:38

guillemsarda approved these changes Jul 6, 2026

View reviewed changes

nathsou merged commit c05e4cd into master Jul 6, 2026
47 checks passed

nathsou deleted the S5906-clarify-length-matchers branch July 6, 2026 15:24

francois-mora-sonarsource pushed a commit that referenced this pull request Jul 7, 2026

JS-1993 S5906: clarify that length matchers apply beyond collections (#…

35c7a4c

…7434)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

JS-1993 S5906: clarify that length matchers apply beyond collections#7434

JS-1993 S5906: clarify that length matchers apply beyond collections#7434
nathsou merged 7 commits into
masterfrom
S5906-clarify-length-matchers

nathsou commented Jul 2, 2026

Uh oh!

hashicorp-vault-sonar-prod Bot commented Jul 2, 2026 •

edited by atlassian Bot

Loading

Uh oh!

github-actions Bot commented Jul 2, 2026 •

edited

Loading

Code no longer flagged (72)

Uh oh!

guillemsarda left a comment

Uh oh!

Uh oh!

Uh oh!

sonarqube-next Bot commented Jul 6, 2026

Uh oh!

guillemsarda left a comment

Uh oh!

Uh oh!

gitar-bot Bot commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

nathsou commented Jul 2, 2026

Summary

Test plan

Uh oh!

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

Uh oh!

Uh oh!

github-actions Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ruling Report

Code no longer flagged (72 issues)

S5914

Code no longer flagged (72)

Uh oh!

guillemsarda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sonarqube-next Bot commented Jul 6, 2026

Quality Gate passed

Uh oh!

guillemsarda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gitar-bot Bot commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hashicorp-vault-sonar-prod Bot commented Jul 2, 2026 •

edited by atlassian Bot

Loading

github-actions Bot commented Jul 2, 2026 •

edited

Loading