Implement S8967: inline snapshots should not contain interpolations

[nathsou]

Summary

• implement S8967 for inline snapshot template interpolations in Jest and Vitest
• keep the detection narrow to built-in inline snapshot matchers rooted in expect(...), including not, resolves, and rejects
• add focused unit tests and sync the generated rule metadata/profile data

Links

• Jira: JS-1964
• RSPEC: https://github.com/SonarSource/rspec/pull/7269

---

Summary by Gitar

• New rule implementation:
  • Implemented S8967 to detect template interpolations in Jest and Vitest inline snapshots.
• Shared logic extraction:
  • Extracted chain-walking logic into a new helper, expect-call-chain.ts, to support S8967 and refactor S8780.
• Refactoring:
  • Updated S8780 to utilize the new collectCallChain and getRootCall helpers, improving handling of computed member access in expectation chains.
• Testing and metadata:
  • Added comprehensive unit tests for S8967 and updated Sonar_way_profile.json to include the new rule.

_{This will update automatically on new commits.}

[guillemsarda]

No big changes requested

[guillemsarda]

[github-actions]

Ruling Report

New issues flagged (4 issues)

S8967

vitest/test/browser/fixtures/expect-dom/toMatchScreenshot.test.ts:134

   132 |       expect(ratio).toMatch(/[01]\.\d{2}/)
   133 | 
>  134 |       expect(errorMessage).toMatchInlineSnapshot(`
   135 |         expect(element).toMatchScreenshot()
   136 | 

vitest/test/browser/fixtures/expect-dom/toMatchScreenshot.test.ts:182

   180 |       })
   181 | 
>  182 |       expect(errorMessage).toMatchInlineSnapshot(`
   183 |         expect(element).toMatchScreenshot()
   184 | 

vitest/test/coverage-test/test/mixed-versions-warning.unit.test.ts:25

    23 |   const message = warn.mock.calls[0][0]
    24 | 
>   25 |   expect(stripVTControlCharacters(message)).toMatchInlineSnapshot(`
    26 |     "Loaded  vitest@1.0.0  and  @vitest/coverage-v8@${version} .
    27 |     Running mixed versions is not supported and may lead into bugs

vitest/test/coverage-test/test/mixed-versions-warning.unit.test.ts:47

    45 |   const message = warn.mock.calls[0][0]
    46 | 
>   47 |   expect(stripVTControlCharacters(message)).toMatchInlineSnapshot(`
    48 |     "Loaded  vitest@1.0.0  and  @vitest/coverage-istanbul@${version} .
    49 |     Running mixed versions is not supported and may lead into bugs

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

[gitar-bot]

Code Review ✅ Approved 2 resolved / 2 findings

Implements rule S8967 to detect template interpolations in inline snapshots and refactors expectation chain logic into shared helpers. Resolved findings regarding documentation clarity in collectCallChain and Playwright async assertion suppression.

✅ 2 resolved

✅ Quality: collectCallChain doc says "innermost first" but returns outermost first

📄 packages/analysis/src/jsts/rules/helpers/expect-call-chain.ts:32-35
The JSDoc for collectCallChain states the segments are collected "innermost first", but the accompanying example and the actual implementation return them outermost-matcher first. For expect(x).not.resolves.toBe(y) the function returns [{toBe}, {resolves}, {not}] — i.e. toBe (the outermost matcher, farthest from the expect(...) root) is first and not (closest/innermost to the root) is last. This is exactly what S8967 relies on (chain[0] must be the inline-snapshot matcher, chain.slice(1) the modifiers), so the code is correct; only the wording is wrong.

Because this is a shared helper consumed by both S8780 and S8967, the mislabeled ordering could lead a future caller to index segments from the wrong end. Suggest updating the comment to say "outermost (matcher) first" to match the behavior.

✅ Edge Case: done-callback guard also suppresses Playwright async assertions

📄 packages/analysis/src/jsts/rules/S8780/rule.ts:120-133 📄 packages/analysis/src/jsts/rules/S8780/rule.ts:175-185 📄 packages/analysis/src/jsts/rules/S8780/rule.ts:309-312
The new usesDoneCallback guard is applied unconditionally at the top of the CallExpression:exit handler, so it runs for every framework — including Playwright. The doc comment justifies this by asserting that "Playwright's fixtures parameter is always a destructuring pattern, never a plain identifier". That is not guaranteed: Playwright's first fixtures argument can legally be a plain identifier, e.g. test('x', async (fixtures) => { await expect(fixtures.page).toBeVisible(); }). In that case usesDoneCallback returns true (first param is an Identifier) and the whole test body is skipped, so a genuinely missing await/return on a Playwright async matcher (getPlaywrightAsyncNode) is silently not reported — a false negative. Playwright never uses a done callback at all, so the guard is conceptually out of scope for it.

Suggested fix: only apply the done-callback guard to the frameworks that actually use it (jest/vitest/jasmine), leaving Playwright detection unaffected. No existing test covers the non-destructured Playwright fixtures parameter, so consider adding one.

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}