feat(formatter/yaml): setup stub implementation#8000
Conversation
|
CodSpeed Performance ReportMerging #8000 will not alter performanceComparing Summary
Footnotes
|
WalkthroughAdds a new YAML formatter crate (crates/biome_yaml_formatter) and wires YAML into the formatting pipeline. Introduces YamlFormatContext and YamlFormatOptions, YAML comment handling/suppression, verbatim formatting paths, many generated FormatRule/AsFormat/IntoFormat implementations for YAML AST nodes, crate-local auxiliary/list/bogus modules, and token formatting glue in the crate root. Also updated codegen (xtask) to support a YAML dialect and re-exported YamlFileSource from biome_yaml_syntax. A small unrelated tweak removed an #[expect(dead_code)] annotation. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (12)
crates/biome_yaml_formatter/src/yaml/auxiliary/document.rs (1)
7-9: Stub implementation – replace with real formatting later.This verbatim pass-through is fine for initial scaffolding, but you'll want to implement proper YAML document formatting with biome_formatter utilities in a follow-up.
Based on learnings.
crates/biome_yaml_formatter/src/yaml/auxiliary/block_sequence_entry.rs (1)
7-9: Stub implementation – replace with real formatting later.The verbatim approach is acceptable for scaffolding, but implement proper block sequence entry formatting in a subsequent change.
Based on learnings.
crates/biome_yaml_formatter/src/yaml/auxiliary/flow_json_node.rs (1)
7-9: Stub implementation – replace with real formatting later.Verbatim formatting serves as a placeholder. Implement proper flow JSON node formatting using biome_formatter utilities in follow-up work.
Based on learnings.
crates/biome_yaml_formatter/src/yaml/auxiliary/double_quoted_scalar.rs (1)
7-9: Stub implementation – replace with real formatting later.Currently passes through as-is. Future work should handle double-quoted scalar formatting properly (escape sequences, line breaking, etc.) using biome_formatter utilities.
Based on learnings.
crates/biome_yaml_formatter/src/yaml/auxiliary/block_strip_indicator.rs (1)
7-13: Stub implementation – replace with real formatting later.Verbatim pass-through is fine for now. Eventually implement proper block strip indicator formatting with biome_formatter utilities.
Based on learnings.
crates/biome_yaml_formatter/src/yaml/auxiliary/properties_tag_first.rs (1)
7-9: Stub implementation acknowledged.This uses verbatim formatting as expected for the initial setup. Replace with proper formatting logic using biome_formatter utilities when implementing real YAML formatting behaviour.
Based on learnings.
crates/biome_yaml_formatter/src/yaml/auxiliary/indentation_indicator.rs (1)
7-13: Stub implementation acknowledged.This uses verbatim formatting as expected for the initial setup. Replace with proper formatting logic using biome_formatter utilities when implementing real YAML formatting behaviour.
Based on learnings.
crates/biome_yaml_formatter/src/yaml/auxiliary/block_map_explicit_entry.rs (1)
7-13: Stub implementation acknowledged.This uses verbatim formatting as expected for the initial setup. Replace with proper formatting logic using biome_formatter utilities when implementing real YAML formatting behaviour.
Based on learnings.
crates/biome_yaml_formatter/src/yaml/auxiliary/tag_property.rs (1)
7-9: Stub implementation acknowledged.This uses verbatim formatting as expected for the initial setup. Replace with proper formatting logic using biome_formatter utilities when implementing real YAML formatting behaviour.
Based on learnings.
crates/biome_yaml_formatter/src/yaml/lists/flow_sequence_entry_list.rs (1)
7-9: Inconsistent list formatting pattern.Unlike
directive_list.rs(Line 8) which implements proper list formatting viaf.join().entries(node.iter().formatted()).finish(), this list formatter uses verbatim. For consistency, consider using the join/entries pattern here as well.crates/biome_yaml_formatter/src/yaml/auxiliary/block_content.rs (1)
7-9: Stub implementation acknowledged.This uses verbatim formatting as expected for the initial setup. Replace with proper formatting logic using biome_formatter utilities when implementing real YAML formatting behaviour.
Based on learnings.
crates/biome_yaml_formatter/src/yaml/auxiliary/block_sequence.rs (1)
7-9: Stub implementation acknowledged.This uses verbatim formatting as expected for the initial setup. Replace with proper formatting logic using biome_formatter utilities when implementing real YAML formatting behaviour.
Based on learnings.
| fn source_map(&self) -> Option<&TransformSourceMap> { | ||
| None | ||
| } |
There was a problem hiding this comment.
Return the stored source map.
You stash the TransformSourceMap in Line 33, but Line 46 always returns None, so downstream consumers never see it. Please surface the stored map.
fn source_map(&self) -> Option<&TransformSourceMap> {
- None
+ self.source_map.as_ref()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn source_map(&self) -> Option<&TransformSourceMap> { | |
| None | |
| } | |
| fn source_map(&self) -> Option<&TransformSourceMap> { | |
| self.source_map.as_ref() | |
| } |
🤖 Prompt for AI Agents
In crates/biome_yaml_formatter/src/context.rs around lines 45 to 47, the
source_map() method currently always returns None even though a
TransformSourceMap was stored earlier (around line 33); change the method to
return a reference to the stored map (e.g., return self.source_map.as_ref()) so
downstream consumers receive the actual Option<&TransformSourceMap> instead of
None, keeping the signature the same.
There was a problem hiding this comment.
biome_json_formatter also does the same thing 🤔
https://github.com/siketyan/biome/blob/f5fc882717e9288e0baee21fac370c13900be4ff/crates/biome_json_formatter/src/context.rs#L47
|
FWIW, having the formatter crate present can make working on the parser a bit more tedious. If you add new nodes or add/remove fields, and only run |
|
Personally I'm fine with that. I think a large portion of users only use YAML as config or translation files, which only concern simple lists/map. With this in place we can deliver YAML tooling to these users much sooner |
l0ngvh
left a comment
There was a problem hiding this comment.
Should we have a few snapshot tests to show that the formatter is running?
|
@l0ngvh The current implementation only prints all tokens as-is, so we don't have anything to test yet. I'll definitely add some tests on adding format rules for simple elements! |
d3cadfb to
7dd508b
Compare
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Summary
Setup the formatter crate for the YAML language. It doesn't do anything, just printing the code as-is.
Test Plan
Added a smoke test.
Docs
N/A