JS-1975 Implement private assertion layer for assertions-in-tests

[francois-mora-sonarsource]

Summary

• add a private assertion detector layer for S2699 without changing shared assertion detection
• prove S2699 accepts the rule-local placeholder in both JS and TS paths
• add a regression test showing the shared detector still ignores the private placeholder

Validation

• npx tsx --tsconfig packages/tsconfig.test.json --test --test-concurrency=1 --test-reporter=spec --test-reporter-destination stdout packages/analysis/src/jsts/rules/S2699/unit.test.ts

[hashicorp-vault-sonar-prod]

JS-1975

[github-actions]

Ruling Report

✅ No changes to ruling expected issues in this PR

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
86.7% 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 a private assertion layer for rule S2699 to isolate test-specific detection logic. This change resolves the inclusion of test-only placeholders and mutable registries in production code.

✅ 2 resolved

✅ Quality: Test-only placeholder shipped in production rule path

📄 packages/analysis/src/jsts/rules/S2699/assertion-detectors.ts:22-36 📄 packages/analysis/src/jsts/rules/S2699/rule.ts:227 📄 packages/analysis/src/jsts/rules/S2699/rule.ts:266
assertion-detectors.ts hardcodes __sonarPrivateAssertionForTestsOnly and wires it into the actual S2699 rule via isAdditionalAssertion/isAdditionalTSAssertion at rule.ts:227 and :266. This means the placeholder detection ships in production, not only in tests: any real user code that happens to call a function named __sonarPrivateAssertionForTestsOnly would be silently treated as an assertion and could suppress a legitimate S2699 report. Collision risk is very low given the deliberately obscure name, and the PR description states this is scaffolding to prove the private-layer concept. Before this leaves draft, either replace the placeholder with the real intended detectors, or gate the placeholder behind a test-only path so it isn't reachable in shipped analysis. The void context;/void services; no-ops also signal these are stubs rather than finished detectors.

✅ Quality: Test-only helper and mutable registry live in production module

📄 packages/analysis/src/jsts/rules/S2699/assertion-detectors.ts:28-42
withAdditionalAssertionDetectorsForTesting (name explicitly says ForTesting) and the module-level mutable arrays additionalAssertionDetectors / additionalTSAssertionDetectors ship in the production assertion-detectors.ts module that rule.ts imports. This resolves the previous finding (the hardcoded __sonarPrivateAssertionForTestsOnly placeholder is gone from the production detection path), but it replaces it with test-only mutation infrastructure that is still bundled and exported from production code.

Concerns:

• The helper mutates shared module-global state via splice. It is safe only because RuleTester.run is synchronous and node:test it blocks run sequentially; if any future caller invokes it around asynchronous work, or if tests run concurrently against this module, the registry could leak detectors across cases. There is no guard preventing async misuse.
• Exporting a ...ForTesting mutator from production widens the production API surface for something only unit tests use.

Suggested fix: if this is genuinely test-only, move the registry and the with...ForTesting helper into a test-support module (or gate registration behind an injected parameter), and keep assertion-detectors.ts limited to the pure isAdditionalAssertion / isAdditionalTSAssertion reads. If the extension API is intended for real (non-test) detectors, drop the ForTesting suffix and document the synchronous-only contract.

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}