Skip to content

Fix no-prototype-builtins issue in Ruleset variables()#4404

Merged
matthew-dean merged 1 commit intoless:masterfrom
matthew-dean:pr-4272-rebased
Mar 9, 2026
Merged

Fix no-prototype-builtins issue in Ruleset variables()#4404
matthew-dean merged 1 commit intoless:masterfrom
matthew-dean:pr-4272-rebased

Conversation

@matthew-dean
Copy link
Copy Markdown
Member

@matthew-dean matthew-dean commented Mar 9, 2026

Rebased version of #4272 by @Krinkle. Resolves conflicts with current master.

Original PR: #4272

Credit: @Krinkle

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced property validation and CSS rule deduplication to prevent potential edge cases and improve compiler stability.
  • Tests

    • Added test coverage for CSS property handling scenarios.

@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Mar 9, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e2fcc881-2a37-4487-9119-d4491c310b31

📥 Commits

Reviewing files that changed from the base of the PR and between 774f188 and 6e78f77.

📒 Files selected for processing (4)
  • packages/less/src/less/tree/ruleset.js
  • packages/less/src/less/visitors/to-css-visitor.js
  • packages/test-data/tests-unit/rulesets/rulesets.css
  • packages/test-data/tests-unit/rulesets/rulesets.less

📝 Walkthrough

Walkthrough

This PR improves code robustness by replacing direct hasOwnProperty calls with safer Object.prototype.hasOwnProperty.call() checks to mitigate prototype-chain interference risks. Additionally, duplicate-rule removal logic in the CSS visitor is refactored to normalize ruleList handling and improve type consistency. Test cases add a "hasOwnProperty" CSS property declaration to validate these safety improvements.

Changes

Cohort / File(s) Summary
Safety improvements in core logic
packages/less/src/less/tree/ruleset.js, packages/less/src/less/visitors/to-css-visitor.js
Both files replace direct hasOwnProperty() calls with Object.prototype.hasOwnProperty.call() for safer property existence checks. The to-css-visitor also refactors duplicate-rule detection to use loop-scoped variables and normalize ruleList to consistently handle arrays, improving type robustness.
Test data additions
packages/test-data/tests-unit/rulesets/rulesets.css, packages/test-data/tests-unit/rulesets/rulesets.less
Both test files add a new CSS property declaration hasOwnProperty: blue; to validate that the codebase correctly handles property names that could collide with prototype methods.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A safer way to check what's there,
No prototype tricks hiding anywhere!
With Object.call we guard the way,
hasOwnProperty won't lead astray.
Our rules dedup with newfound care,
Robustness blooming in the air! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: fixing no-prototype-builtins issues by replacing unsafe hasOwnProperty calls with safer Object.prototype.hasOwnProperty.call patterns in the Ruleset module and related code.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@matthew-dean matthew-dean merged commit 9c91714 into less:master Mar 9, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants