Conversation
WalkthroughThe Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/formatters/base.ts`:
- Around line 889-907: The remove(...) modifier logic in base.ts currently
treats unquoted/malformed args like remove(foo) as having zero args and silently
no-ops; change the parsing in the block handling _mod (and the analogous array
branch around lines 954-968) to detect non-empty content that doesn't contain
any quoted arguments: after extracting content from _mod (const content =
_mod.substring(7, _mod.length - 1)), if content.trim() is non-empty but the
regex extraction produced args.length === 0, return undefined to signal an
invalid modifier; keep the existing behavior where an empty content (remove())
returns the original variable as a no-op. Ensure the same guard is added to both
the string and array branches that use regex, args, content, and _mod.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/core/src/formatters/base.ts (1)
895-912:⚠️ Potential issue | 🟡 MinorRequire full-shape validation for
remove(...)arguments.The current extractor still accepts partially malformed content (for example,
remove("a", foo)), because it pulls quoted tokens and ignores invalid leftovers. That can silently apply partial removals instead of surfacing an unknown modifier.Proposed fix (apply in both string and array branches)
const content = _mod.substring(7, _mod.length - 1); +const validRemoveArgs = + /^\s*(?:"[^"]*"|'[^']*')\s*(?:,\s*(?:"[^"]*"|'[^']*')\s*)*$/; +if (!validRemoveArgs.test(content)) return undefined; // Extract options from remove("...", "...", ...) const regex = /"([^"]*)"|'([^']*)'/g; const args: string[] = [];Also applies to: 960-973
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/formatters/base.ts` around lines 895 - 912, The extractor for remove(...) (using _mod, content, regex, args) accepts partially malformed input because it extracts quoted tokens but ignores leftover invalid text; update the logic to enforce full-shape validation by ensuring the entire content matches only a comma-separated list of quoted strings (e.g. with a full-match regex or by reconstructing the expected string and comparing lengths) before proceeding to build args; if the full-shape check fails, return undefined (i.e. treat as unknown modifier) instead of performing partial replacements. Apply the same strict validation to both the string and array branches (the other block at the 960-973 range) so malformed inputs like remove("a", foo) are rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/core/src/formatters/base.ts`:
- Around line 895-912: The extractor for remove(...) (using _mod, content,
regex, args) accepts partially malformed input because it extracts quoted tokens
but ignores leftover invalid text; update the logic to enforce full-shape
validation by ensuring the entire content matches only a comma-separated list of
quoted strings (e.g. with a full-match regex or by reconstructing the expected
string and comparing lengths) before proceeding to build args; if the full-shape
check fails, return undefined (i.e. treat as unknown modifier) instead of
performing partial replacements. Apply the same strict validation to both the
string and array branches (the other block at the 960-973 range) so malformed
inputs like remove("a", foo) are rejected.
Allows you to put multiple removes in one instead of chaining them.
Example:
::remove('1','2')
Summary by CodeRabbit
New Features
Bug Fixes / Behaviour Change