Skip to content

JS-2207: Implement S9135: Avoid mutating nested properties of shallow clones - #7706

Open
nathsou wants to merge 3 commits into
masterfrom
new-rule/S9135
Open

JS-2207: Implement S9135: Avoid mutating nested properties of shallow clones#7706
nathsou wants to merge 3 commits into
masterfrom
new-rule/S9135

Conversation

@nathsou

@nathsou nathsou commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Implement S9135 for direct nested mutations of uniquely bound Lodash, Lodash-es, and Underscore.js clone results.
  • Report the mutation as primary and the shallow clone call as a secondary location.
  • Provide structuredClone() and intentional-shared-state NOSONAR suggestions, with library-specific messages for Lodash and Underscore.js.
  • Add focused unit and custom ruling coverage.

Links

@nathsou nathsou self-assigned this Aug 4, 2026
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Aug 4, 2026

Copy link
Copy Markdown

JS-2207

Comment thread its/sources/custom/jsts/S9135.js Outdated
Comment thread packages/analysis/src/jsts/rules/S9135/rule.ts Outdated
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Ruling Report

New issues flagged (2 issues)

S9135

custom-jsts/S9135.js:25

    23 | 
    24 | const lodashCopy = lodash.clone(user);
>   25 | lodashCopy.address.city = 'Geneva';
    26 | 
    27 | const underscoreCopy = underscore.clone(user);

custom-jsts/S9135.js:28

    26 | 
    27 | const underscoreCopy = underscore.clone(user);
>   28 | underscoreCopy.address.city = 'Geneva';
    29 | 
    30 | const deepCopy = structuredClone(user);

Ruling passed with these expected-result updates already present in the branch. No fix PR was needed.

@datadog-sonarsource

This comment has been minimized.

@sonarqube-next

sonarqube-next Bot commented Aug 4, 2026

Copy link
Copy Markdown

@gitar-bot

gitar-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Implements rule S9135 to detect nested property mutations on shallow clones from Lodash and Underscore.js, addressing the stray copyright header and hardcoded diagnostic message findings.

✅ 2 resolved
Quality: Stray Closure Library copyright header in test fixture

📄 its/sources/custom/jsts/S9135.js:1-15
Line 1 adds // Copyright 2026 The Closure Library Authors. All Rights Reserved. directly above the SonarSource SSAL license block. This is a copy-paste artifact that wrongly attributes a SonarSource file to the Closure Library Authors and contradicts the license header directly below it. Remove line 1 so only the standard SonarSource header remains.

Quality: Diagnostic message hardcodes '_.clone()' for all import forms

📄 packages/analysis/src/jsts/rules/S9135/rule.ts:28-31
Both messages read "Mutating a nested property of this _.clone()...", but the rule also fires on default/named imports where the call is written as clone(user) (e.g. import clone from 'lodash/clone', tested at unit.test.ts:118-147). In those cases the _. prefix does not match the user's code and can be mildly confusing. Consider wording that does not assume the _. namespace (e.g. "this shallow clone") while still recommending structuredClone()/cloneDeep().

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

@nathsou
nathsou requested a review from guillemsarda August 4, 2026 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant