feat(formatter): support consistent quote props#16677
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
consistent quote props
CodSpeed Performance ReportMerging #16677 will not alter performanceComparing Summary
Footnotes
|
3cf06a5 to
615e8e8
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements support for the consistent quote properties option in the oxc formatter, matching Prettier's behavior. When enabled, if any property in an object-like structure requires quotes (e.g., "prop-2"), all properties in that structure will be consistently quoted.
Key changes:
- Added stack-based quote tracking in
FormatContextto handle nested object-like structures independently - Implemented consistent quoting for
ObjectExpression,Class,TSTypeLiteral,TSInterfaceDeclaration, andWithClause - Refactored
FormatLiteralStringToken::clean_text()to acceptFormatterreference for accessing context state
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
crates/oxc_formatter/src/formatter/context.rs |
Added quote_needed_stack with push/pop/is_quote_needed methods for tracking quote consistency state |
crates/oxc_formatter/src/options.rs |
Updated QuoteProperties::Consistent documentation to remove "NOT SUPPORTED YET" note; added QuoteStyle::as_str() helper |
crates/oxc_formatter/src/utils/string.rs |
Refactored clean_text() to take Formatter reference; moved is_identifier_name_patched() to public API; changed is_quote_needed from enum-based to boolean |
crates/oxc_formatter/src/utils/object.rs |
Added should_preserve_quote() helper to determine if property key requires quotes |
crates/oxc_formatter/src/write/mod.rs |
Added quote wrapping for IdentifierName in property key contexts; added quote tracking for ObjectExpression and Vec<TSSignature> |
crates/oxc_formatter/src/write/class.rs |
Added quote tracking for ClassBody covering properties, methods, and accessors |
crates/oxc_formatter/src/write/import_declaration.rs |
Added quote tracking for WithClause in import/export attributes; updated ImportAttribute to use refactored clean_text() |
crates/oxc_formatter/src/utils/assignment_like.rs |
Updated call to clean_text() with new signature |
crates/oxc_formatter/tests/fixtures/mod.rs |
Added quoteProps option parsing support in test fixture configuration |
crates/oxc_formatter/tests/fixtures/{js,ts}/quote-props/* |
Comprehensive test fixtures covering all supported node types with various quoting scenarios |
tasks/prettier_conformance/src/lib.rs |
Removed filter skipping consistent quote properties tests |
tasks/prettier_conformance/snapshots/prettier.js.snap.md |
Updated compatibility stats showing two expected failures for numeric literal handling (intentional deviation from Prettier) |
Comments suppressed due to low confidence (1)
crates/oxc_formatter/src/formatter/context.rs:46
- The
Debugimplementation forFormatContextdoes not include the newly addedquote_needed_stackfield. Consider adding it to the debug output for better debugging experience:
impl std::fmt::Debug for FormatContext<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("FormatContext")
.field("options", &self.options)
.field("source_text", &self.source_text)
.field("source_type", &self.source_type)
.field("comments", &self.comments)
.field("cached_elements", &self.cached_elements)
.field("quote_needed_stack", &self.quote_needed_stack)
.finish()
}
}impl std::fmt::Debug for FormatContext<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("FormatContext")
.field("options", &self.options)
.field("source_text", &self.source_text)
.field("source_type", &self.source_type)
.field("comments", &self.comments)
.field("cached_elements", &self.cached_elements)
.finish()
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
543588b to
afc2f6b
Compare
leaysgur
left a comment
There was a problem hiding this comment.
@Dunqing I'll merge this now for subsequent tasks, but please check the following: 🙏🏻
- Regarding Prettier bugs and behavior differences, please note them to the Diff Discussion (or report Prettier?)
- The file
tasks/coverage/acorn-test262is probably unnecessary?
I think integration with Oxfmtrc is missing, but I'll handle that later.
Merge activity
|
### Summary close: #16247 The Prettier documentation is [Quote props](https://prettier.io/docs/next/options#quote-props). Support `consistent` in all object-like nodes, including `ObjectExpression`, `Class`, `TSTypeLiteral`, `TSInterfaceDeclaration`, and `WithClause`. ### Example: ```ts // ============================================ // ObjectExpression - object literals // ============================================ const obj = { normal: "value", "needs-quote": "value", // triggers consistent quoting for all keys }; // Nested objects are handled independently const nested = { outer: { inner: "value", "inner-key": "value", // only inner object gets quoted }, outerKey: "value", // outer stays unquoted }; // ============================================ // Class - properties, methods, and accessors // ============================================ class MyClass { prop1 = "value"; "prop-2" = "value"; // triggers consistent quoting method1() {} "method-2"() {} // triggers consistent quoting for methods too get getter1() { return 1; } get "getter-2"() { return 2; } // triggers consistent quoting for accessors too set setter1(v) {} set "setter-2"(v) {} } // ============================================ // WithClause - import/export attributes // ============================================ import data from "./data.json" with { type: "json", "custom-attr": "value", // triggers consistent quoting }; export { foo } from "./foo.js" with { key: "value", "x-y": "other", }; // ============================================ // TSInterfaceDeclaration (TypeScript) // ============================================ interface MyInterface { prop1: string; "prop-2": number; // triggers consistent quoting method1(): void; "method-2"(): string; // triggers consistent quoting for method signatures too } // ============================================ // TSTypeLiteral (TypeScript) // ============================================ type MyType = { field1: string; "field-2": number; // triggers consistent quoting method(): void; "method-2"(): string; // triggers consistent quoting for method signatures too }; // Inline type literal function process(param: { x: string; "y-z": number }): void {} ``` NOTE: I've found a Prettier bug, that is, `TSMethodSignature` is not being wrapped in quotes! But it should be, see [Prettier Playground](https://prettier.io/playground/#N4Igxg9gdgLgprEAuEMCeAHOACAtnGACwgBMBnbAXm2AB0ps8DiSAKASiWwDcIBLEgG56jAOT4ipALQAmURy68BwqAF8VIADQgIGGH2hlkoAIYAnMxADuABXMIjKEwBsrJtEe0AjMybABrAgBlE3wAGT4oOGQAMxcyOG0ILwArODAYAHVfDGQQDDM4BLNuaO9fAOCMP0iAc2QYMwBXRJAE3D4G5ta4AA8sMz58WBcAeQGTGAgzGwgyPn1oPIQSLRA+gaGEGBcAFTgzKHM+Itj41vmoWuc4AEUmiHgz5wTtFLJeoLqb+8fopDiL1aAEcHvAbJYMI5wIY+GR4IhtI0THxnHUAMIQXC4Ex5FzONaXa5wACCMEafC8TXBBwiUWerxAhBguGcmUICyK1TAcCCDgWfG4CzQeTAZE8IG4LQAklASNsgmBBnoSXKgugbgzWgU5nBsiZcigCkUDqU1itRjE6f8QM4YmtIsUYBCTLUcVrtNUzMU8l4TF44M4pDAJQVIlkBERkAAOAAM2kKoL4hRdbtxAPOSP9mUjhGQMm0TQSu39jkBjLguADJHlJDCJiuTVdcAAYtMceS6njqRAQKpVEA) ### Not implemented case There are still two failed tests because NumericLiterals doesn't wrap in quotes in `consistent`, which is intended because Prettier only wraps them when the parser is `JavaScript` or `Flow` For example: Input: ``` const A = { "k-b": a, 2: b, } ``` Output: With `babel` or `flow` parser ```ts const A = { "k-b": a, "2": b, }; ``` With `babel-ts` or `typescript` parser ```ts const A = { "k-b": a, 2: b, }; ``` So, for simplicity, and since oxc includes parsing TS, we don't wrap `NumericLiteral` in quotes at all. ### Tests All tests are generated by Claude with the proper prompt, covering all code in this PR.
afc2f6b to
686f2b7
Compare
Will report to Prettier
Opps, yes!
Thank you! |

Summary
close: #16247
The Prettier documentation is Quote props.
Support
consistentin all object-like nodes, includingObjectExpression,Class,TSTypeLiteral,TSInterfaceDeclaration, andWithClause.Example:
NOTE: I've found a Prettier bug, that is,
TSMethodSignatureis not being wrapped in quotes! But it should be, see Prettier PlaygroundNot implemented case
There are still two failed tests because NumericLiterals doesn't wrap in quotes in
consistent, which is intended because Prettier only wraps them when the parser isJavaScriptorFlowFor example:
Input:
Output:
With
babelorflowparserWith
babel-tsortypescriptparserSo, for simplicity, and since oxc includes parsing TS, we don't wrap
NumericLiteralin quotes at all.Tests
All tests are generated by Claude with the proper prompt, covering all code in this PR.