refactor: labeled expressions implementation#244
Conversation
# Conflicts: # crates/lingui_macro/src/builder.rs # crates/lingui_macro/tests/jsx.rs
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #244 +/- ##
==========================================
- Coverage 95.06% 94.83% -0.24%
==========================================
Files 9 9
Lines 2251 2325 +74
==========================================
+ Hits 2140 2205 +65
- Misses 111 120 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Refactors the SWC Lingui macro tokenization/indexing to more closely mirror the Babel implementation, with a focus on consistent numeric placeholder allocation (source-order for ICU value, and not counting named placeholders) and uniform ph(...) handling across tokenization paths.
Changes:
- Replaced expression/ICU-choice tokens with a unified
MsgArgmodel (name + value + optional ICU format/cases) and updated the builder accordingly. - Introduced a split context model (
TransformCtxper file +MacroCtxper macro invocation) and moved tokenization into free functions that operate on&mut MacroCtx. - Reorganized/expanded tests and snapshots for labeled expressions and added coverage for ICU
valueindex allocation order.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/lingui_macro/src/tokens.rs | Refactors message tokens to use MsgArg for expressions/ICU arguments. |
| crates/lingui_macro/src/macro_utils.rs | Introduces TransformCtx/MacroCtx split and new tokenization helpers; adds ph(...) unwrapping and labeled-expression handling. |
| crates/lingui_macro/src/lib.rs | Updates transform entrypoints to use the new per-invocation MacroCtx and custom JSX visitor traversal. |
| crates/lingui_macro/src/jsx_visitor.rs | Reworks JSX traversal/tokenization to produce MsgArg tokens and handle ICU macros in source order. |
| crates/lingui_macro/src/js_macro_folder.rs | Adapts JS macro folding to new tokenization APIs and context types. |
| crates/lingui_macro/src/builder.rs | Updates message construction to consume MsgArg tokens (including ICU format/cases) and simplify value handling. |
| crates/lingui_macro/src/ast_utils.rs | Removes TS as expansion helper (moved/handled elsewhere). |
| crates/lingui_macro/tests/labeled_expressions.rs | New consolidated test file for labeled-expressions and ph scenarios (including error cases). |
| crates/lingui_macro/tests/jsx.rs | Removes labeled-expression test cases moved into labeled_expressions.rs. |
| crates/lingui_macro/tests/jsx_icu.rs | Adds regression tests for ICU value index allocation ordering. |
| crates/lingui_macro/tests/js_t.rs | Removes labeled-expression test cases moved into labeled_expressions.rs. |
| crates/lingui_macro/tests/snapshots/labeled_expressions__jsx_ph_with_non_object_arg.snap | New snapshot covering invalid ph(variable) usage in JSX. |
| crates/lingui_macro/tests/snapshots/labeled_expressions__jsx_ph_labels.snap | Updates snapshot source location and removes multi-prop case (now an error test). |
| crates/lingui_macro/tests/snapshots/labeled_expressions__jsx_ph_labeled_expression_multiple_props.snap | New snapshot covering multi-prop labeled-expression error via ph({a,b}). |
| crates/lingui_macro/tests/snapshots/labeled_expressions__jsx_ph_label_with_nested_plural.snap | Updates snapshot source location after test move. |
| crates/lingui_macro/tests/snapshots/labeled_expressions__jsx_nested_labels.snap | Updates snapshot source location after test move. |
| crates/lingui_macro/tests/snapshots/labeled_expressions__jsx_labeled_expression_multiple_props.snap | New snapshot covering multi-prop labeled-expression error in JSX explicit label. |
| crates/lingui_macro/tests/snapshots/labeled_expressions__jsx_icu_with_labeled_expression_as_value.snap | New snapshot covering labeled-expression as ICU value. |
| crates/lingui_macro/tests/snapshots/labeled_expressions__jsx_icu_with_labeled_expression_as_value_with_ph.snap | New snapshot covering ph({count: ...}) used as ICU value. |
| crates/lingui_macro/tests/snapshots/labeled_expressions__jsx_explicit_labels.snap | Updates snapshot source location and removes multi-prop case (now an error test). |
| crates/lingui_macro/tests/snapshots/labeled_expressions__jsx_explicit_labels_with_as_statement.snap | Updates snapshot source location after test move. |
| crates/lingui_macro/tests/snapshots/labeled_expressions__js_ph_with_non_object_arg.snap | New snapshot covering invalid ph(variable) usage in template literals. |
| crates/lingui_macro/tests/snapshots/labeled_expressions__js_ph_labels_in_tpl_literal.snap | Updates snapshot source location and removes multi-prop case (now an error test). |
| crates/lingui_macro/tests/snapshots/labeled_expressions__js_ph_labeled_expression_multiple_props.snap | New snapshot covering multi-prop labeled-expression error via ph({a,b}) in template literal. |
| crates/lingui_macro/tests/snapshots/labeled_expressions__js_labeled_expression_multiple_props.snap | New snapshot covering multi-prop labeled-expression error in template literal explicit label. |
| crates/lingui_macro/tests/snapshots/labeled_expressions__js_explicit_labels_in_tpl_literal.snap | Updates snapshot source location and removes multi-prop case (now an error test). |
| crates/lingui_macro/tests/snapshots/labeled_expressions__js_choice_labels_in_tpl_literal.snap | Updates snapshot source location after test move. |
| crates/lingui_macro/tests/snapshots/jsx_icu__with_value_index_in_source_order.snap | New regression snapshot for source-order ICU value index allocation. |
| crates/lingui_macro/tests/snapshots/jsx_icu__with_non_identifier_value_first.snap | New regression snapshot for ICU value when it appears first. |
| crates/lingui_macro/tests/snapshots/js_t__js_substitution_in_tpl_literal.snap | Snapshot updates reflecting new values ordering/allocation behavior. |
| crates/lingui_macro/tests/snapshots/js_t__js_custom_i18n_passed.snap | Snapshot updates reflecting new values ordering/allocation behavior. |
| crates/lingui_macro/tests/snapshots/js_icu__js_choices_may_contain_expressions.snap | Snapshot updates reflecting new values ordering/allocation behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@andrii-bodnar this one is ready |
This is my attempt on fixing #241 and #239
This is more systematic refactoring rather ad-hoc fix. The refactoring make closer Rust implementation to a babel's one, so they not diverge.
Previously indexes for expression were allocated during processing a Message Phase in a message builder, in babel version those indexes where assigned during AST scanning phase, so it produced a diffrent result described in #241
Additionally
phmacro was not processed uniformly. It was processed in some places but were skipped in other, for example:With this refactoring, ph support is added everywhere and processed uniformly.
Other changes:
TransformCtx(formerlyMacroCtx) — global per-file context holding macro imports, options, directives, and runtime identifiers.MacroCtx<'a>— short-lived, created fresh for each macro invocation. Borrows&'a mut TransformCtxand owns its own expression index counter starting at 0.tokenize_tpl,tokenize_expr_to_arg,try_tokenize_call_expr_as_choice_cmp,try_tokenize_expr, andget_choice_cases_from_objare now free functions taking&mut MacroCtxexplicitly, making the data flow clear.