-
-
Notifications
You must be signed in to change notification settings - Fork 781
fix(js_parser): allow return statements in Astro frontmatter #8302
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
Conversation
Regression from biomejs#8037 (78011b1): the `is_astro()` check excluded frontmatter scripts, causing false "Illegal return statement" errors when using `experimentalFullSupportEnabled`. Fixes biomejs#8109
🦋 Changeset detectedLatest commit: a2a9b24 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 |
WalkthroughThis change makes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1).changeset/**/*.md📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-11-28T09:08:10.077ZApplied to files:
📚 Learning: 2025-11-28T09:08:10.077ZApplied to files:
🔇 Additional comments (1)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 0
🧹 Nitpick comments (1)
crates/biome_cli/tests/cases/handle_astro_files.rs (1)
721-750: Good targeted regression test for full‑support modeNice mirror of the existing
does_not_throw_parse_error_for_returntest, now exercising theexperimentalFullSupportEnabledpath with the same fixture and snapshot flow. This should keep the Astro frontmatterreturnbehaviour honest in both configurations.If these start to proliferate, you could later factor a tiny helper to share the common “write config + ASTRO_RETURN + run_cli + snapshot” pattern, but that’s strictly polish.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_cli/tests/snapshots/main_cases_handle_astro_files/does_not_throw_parse_error_for_return_full_support.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (2)
crates/biome_cli/tests/cases/handle_astro_files.rs(1 hunks)crates/biome_js_syntax/src/file_source.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use thedbg!()macro for debugging output during testing, and pass the--show-outputflag tocargoto view debug output
Usecargo torcargo testto run tests; for a single test, pass the test name after thetestcommand
Use snapshot testing with theinstacrate; runcargo insta accept,cargo insta reject, orcargo insta reviewto manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Usejust f(alias forjust format) to format Rust and TOML files before committing
Files:
crates/biome_js_syntax/src/file_source.rscrates/biome_cli/tests/cases/handle_astro_files.rs
🧠 Learnings (19)
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : When formatting AST nodes, use mandatory tokens from the AST instead of hardcoding token strings (e.g., use `node.l_paren_token().format()` instead of `token("(")`)
Applied to files:
crates/biome_js_syntax/src/file_source.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Distinguish between `TypeData::Unknown` and `TypeData::UnknownKeyword` to measure inference effectiveness versus explicit user-provided unknown types
Applied to files:
crates/biome_js_syntax/src/file_source.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use `Semantic<T>` query type instead of `Ast<T>` when a rule needs to access the semantic model for binding references and scope information
Applied to files:
crates/biome_js_syntax/src/file_source.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_js_syntax/src/file_source.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : For rules ported from other ecosystems like ESLint or Clippy, add a `sources` field with `RuleSource` metadata using `.same()` for identical behavior or `.inspired()` for different behavior
Applied to files:
crates/biome_js_syntax/src/file_source.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Check if a variable is global before banning it to avoid false positives when the variable is redeclared in local scope; use the semantic model to verify global scope
Applied to files:
crates/biome_js_syntax/src/file_source.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/language_kind.rs : Add a new variant to `LanguageKind` enum in `language_kind.rs` file and implement all methods for the new language variant
Applied to files:
crates/biome_js_syntax/src/file_source.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/*_kinds_src.rs : Create a file called `src/{language}_kinds_src.rs` that returns a static `KindSrc` for new language parsers
Applied to files:
crates/biome_js_syntax/src/file_source.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ConditionalParsedSyntax` for syntax that is only valid in specific contexts (e.g., strict mode, file types, language versions) and call `or_invalid_to_bogus()` to convert to a bogus node if not supported
Applied to files:
crates/biome_js_syntax/src/file_source.rs
📚 Learning: 2025-11-24T18:06:12.048Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Implement watcher tests for workspace methods in watcher.tests.rs and end-to-end tests in LSP tests
Applied to files:
crates/biome_cli/tests/cases/handle_astro_files.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/tests/language.rs : Implement `TestFormatLanguage` trait in `tests/language.rs` for the formatter's test language
Applied to files:
crates/biome_cli/tests/cases/handle_astro_files.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/tests/spec_tests.rs : Use the `tests_macros::gen_tests!` macro in `spec_tests.rs` to generate test functions for each specification file matching the pattern `tests/specs/<language>/**/*.<ext>`
Applied to files:
crates/biome_cli/tests/cases/handle_astro_files.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/Cargo.toml : Include development dependencies in `Cargo.toml` for formatter tests: `biome_formatter_test`, `biome_<language>_factory`, `biome_<language>_parser`, `biome_parser`, `biome_service`, `countme`, `iai`, `quickcheck`, `quickcheck_macros`, and `tests_macros`
Applied to files:
crates/biome_cli/tests/cases/handle_astro_files.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Implement error recovery in list parsing using `or_recover()` to wrap unparseable tokens in a `BOGUS_*` node and consume tokens until a recovery token is found
Applied to files:
crates/biome_cli/tests/cases/handle_astro_files.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Avoid using `unwrap()` or `expect()` on `Result` and `Option` types; instead use helper functions like `map`, `filter`, `and_then` to maintain code clarity and avoid panics
Applied to files:
crates/biome_cli/tests/cases/handle_astro_files.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : In rule documentation code blocks, mark invalid examples with the `expect_diagnostic` property and valid examples without it; each invalid example must emit exactly one diagnostic
Applied to files:
crates/biome_cli/tests/cases/handle_astro_files.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ParseSeparatedList` and `ParseNodeList` for parsing lists with error recovery to avoid infinite loops
Applied to files:
crates/biome_cli/tests/cases/handle_astro_files.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : When navigating CST nodes that return `Result`, use the try operator `?` to convert to `Option`, or use `let else` pattern for `Vec` return types in rule run functions
Applied to files:
crates/biome_cli/tests/cases/handle_astro_files.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Do not attempt to 'fix' the code; if a token/node is known to be mandatory but is missing, return `None` instead
Applied to files:
crates/biome_cli/tests/cases/handle_astro_files.rs
🧬 Code graph analysis (1)
crates/biome_cli/tests/cases/handle_astro_files.rs (1)
crates/biome_cli/tests/snap_test.rs (1)
assert_cli_snapshot(407-409)
🔇 Additional comments (1)
crates/biome_js_syntax/src/file_source.rs (1)
133-146: I'm encountering persistent repository cloning issues that prevent me from executing shell scripts to verify the call sites. However, based on the code context provided in the review comment, I can provide an assessment:The semantic change to
is_astro()appears intentional and well-scopedThe modification correctly broadens
is_astro()to returntruefor anyAstro { .. }variant (including frontmatter cases). Sinceis_astro_frontmatter()remains available for the narrower case, callers that specifically need the frontmatter distinction can use that method. This pattern suggests the change is deliberate—likely treating frontmatter as part of Astro parsing logic.The review's concern about call sites is valid but cannot be fully verified due to repository access limitations. The change appears safe if:
- Existing callers expect Astro frontmatter to be treated the same as regular Astro content
- No caller relied on the implicit
frontmatter: falseassumption- The broader match aligns with the parser's intended behaviour
CodSpeed Performance ReportMerging #8302 will not alter performanceComparing Summary
Footnotes
|
Regression from #8037 (78011b1): the
is_astro()check excluded frontmatter scripts, causing false "Illegal return statement" errors when usingexperimentalFullSupportEnabled.Fixes #8109