-
-
Notifications
You must be signed in to change notification settings - Fork 857
fix(lint): exclude GitHub Actions expressions from noTemplateCurlyInString #8665
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(lint): exclude GitHub Actions expressions from noTemplateCurlyInString #8665
Conversation
🦋 Changeset detectedLatest commit: c34533f The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughReplaces a byte-scan with a peekable iterator in the linter rule no-template-curly-in-string to handle both Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.rs📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
🧠 Learnings (7)📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2025-12-22T09:26:56.943ZApplied to files:
📚 Learning: 2025-11-24T18:05:27.810ZApplied to files:
📚 Learning: 2025-12-21T21:15:03.796ZApplied to files:
🔇 Additional comments (3)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_js_analyze/tests/specs/suspicious/noTemplateCurlyInString/valid.js (1)
17-21: Consider adding edge case test coverage.The test cases cover common GitHub Actions patterns well, but consider adding tests for edge cases:
- Nested braces:
"${{ fromJSON('{\"key\": \"value\"}') }}"- Multiple expressions:
"foo ${{ a }} bar ${{ b }}"- Mixed syntax:
"${ foo } ${{ bar }}"(should flag the first, skip the second)These would help ensure the implementation handles complex expressions correctly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/suspicious/noTemplateCurlyInString/valid.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (2)
crates/biome_js_analyze/src/lint/suspicious/no_template_curly_in_string.rscrates/biome_js_analyze/tests/specs/suspicious/noTemplateCurlyInString/valid.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use thedbg!()macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests
Files:
crates/biome_js_analyze/src/lint/suspicious/no_template_curly_in_string.rs
🧠 Learnings (5)
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Avoid string allocations by using `&str` or `TokenText` instead of `to_string()`
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/no_template_curly_in_string.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `Markup!` macro for diagnostic messages and code action descriptions to ensure proper formatting
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/no_template_curly_in_string.rs
📚 Learning: 2025-12-22T09:26:56.943Z
Learnt from: ematipico
Repo: biomejs/biome PR: 8537
File: crates/biome_js_analyze/src/lint/nursery/no_leaked_render.rs:167-210
Timestamp: 2025-12-22T09:26:56.943Z
Learning: When defining lint rules (declare_lint_rule!), only specify fix_kind if the rule implements an action(...) function. Rules that only emit diagnostics without a code fix should omit fix_kind. This applies to all Rust lint rule definitions under crates/.../src/lint (e.g., crates/biome_js_analyze/src/lint/...).
Applied to files:
crates/biome_js_analyze/src/lint/suspicious/no_template_curly_in_string.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/*.jsonc : Use `.jsonc` format for test files containing multiple code snippets, where each snippet is a string in an array
Applied to files:
crates/biome_js_analyze/tests/specs/suspicious/noTemplateCurlyInString/valid.js
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/* : Create test files with `invalid` and `valid` prefixes to represent code that should and should not trigger the rule
Applied to files:
crates/biome_js_analyze/tests/specs/suspicious/noTemplateCurlyInString/valid.js
🔇 Additional comments (2)
crates/biome_js_analyze/src/lint/suspicious/no_template_curly_in_string.rs (2)
43-47: Helpful documentation addition.The example clearly illustrates the GitHub Actions pattern that should be excluded from diagnostics.
70-70: Correct use of peekable iterator.The peekable wrapper is necessary for the lookahead logic that follows. The performance overhead is negligible for this use case.
crates/biome_js_analyze/src/lint/suspicious/no_template_curly_in_string.rs
Outdated
Show resolved
Hide resolved
70ea3a1 to
f44fcfa
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
ematipico
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! The changeset is missing, please add one. This information is available in our PR template, which was removed from the AI bot. Please restore it, because it removed sections that need to be filled.
I'm sorry if this comes across harshly, but I have seen a lot of missing info lately because of this.
crates/biome_js_analyze/src/lint/suspicious/no_template_curly_in_string.rs
Outdated
Show resolved
Hide resolved
No problem :) - I'll look into it |
f44fcfa to
8df9605
Compare
…tring
GitHub Actions uses `${{ ... }}` syntax (double curly braces) which was
incorrectly flagged as a template literal placeholder. This fix detects
and skips patterns that have double opening braces followed by double
closing braces, as these are GitHub Actions expressions, not JavaScript
template literal mistakes.
8df9605 to
c34533f
Compare
dyc3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
| /// GitHub Actions expressions using double curly braces are also valid: | ||
| /// | ||
| /// ```js | ||
| /// const a = "${{ inputs.abc }}"; | ||
| /// ``` | ||
| /// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Not sure if its worth calling this out in the rule docs. Feels like an implementation detail.
CodSpeed Performance ReportMerging #8665 will not alter performanceComparing Summary
Footnotes
|
This is completely generated with Claude by prompting it to resolve our use case of requiring the valid case of
"${{ inputs.abc }}". We use https://github.com/ArmaanT/cdkactions to programatically generate our GitHub workflows and Biome warns us consistently about this.Summary
GitHub Actions uses
${{ ... }}syntax (double curly braces) which was incorrectly flagged as a template literal placeholder. This fix detects and skips patterns that have double opening braces followed by double closing braces, as these are GitHub Actions expressions, not JavaScript template literal mistakes.Test Plan
Snapshot testing is implemented to account for the new cases.