feat: add no-macro-inside-macro rule, narrow no-expression-in-message#132
Open
edzis wants to merge 1 commit into
Open
feat: add no-macro-inside-macro rule, narrow no-expression-in-message#132edzis wants to merge 1 commit into
edzis wants to merge 1 commit into
Conversation
Closes lingui#90. Forbids nesting Lingui translation macros inside each other. Covers: - message macro (t/msg/defineMessage) inside another message macro - any translation macro inside a <Plural>/<Select>/<SelectOrdinal> branch attribute or a plural()/select()/selectOrdinal() option value - component macro (Trans/Plural/Select/SelectOrdinal) interpolated into a message macro template Legitimate composition patterns are preserved: - choice calls as template interpolation (compose into ICU) - descriptor passthrough (t(msg`...`)) Also narrows no-expression-in-message to skip nested Lingui macros in message templates so users get one targeted diagnostic instead of a misleading generic one. Its Trans-children handler is untouched. Added to the recommended config at 'warn', matching the level of the peer structural rules (no-trans-inside-trans, no-plural-inside-trans).
There was a problem hiding this comment.
Pull request overview
Adds a new structural ESLint rule to prevent nested Lingui translation macros (which can break extraction output), and adjusts the existing no-expression-in-message rule to avoid duplicate diagnostics for the newly-covered nesting cases.
Changes:
- Introduce
no-macro-inside-macrorule (plus docs and comprehensive tests) to forbid nesting Lingui message/component macros in other Lingui macros and choice branches/options. - Narrow
no-expression-in-messageso message-template interpolations that are themselves Lingui macros are skipped (delegated to the new rule). - Register the new rule in the plugin and add it to the recommended configuration and README rule index.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/src/rules/no-macro-inside-macro.test.ts | Adds valid/invalid coverage for the new nesting rule across message macros, choice components, and choice calls. |
| tests/src/rules/no-expression-in-message.test.ts | Adds valid cases asserting nested Lingui macros in message templates no longer trigger this rule. |
| src/rules/no-macro-inside-macro.ts | Implements the new rule detecting illegal macro nesting and reporting context-specific diagnostics. |
| src/rules/no-expression-in-message.ts | Adds nested-macro detection to skip reporting in message-template interpolations. |
| src/index.ts | Registers the new rule and enables it (warn) in the recommended configs. |
| README.md | Adds the new rule to the documented rule list. |
| docs/rules/no-macro-inside-macro.md | Documents the new rule’s intent, scope, exceptions, and examples. |
| docs/rules/no-expression-in-message.md | Clarifies that nested Lingui macros are handled by no-macro-inside-macro. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const EAGER_MESSAGE_MACRO_NAMES = new Set(['t']) | ||
|
|
||
| // Lazy message macros produce a MessageDescriptor value. | ||
| // Safe as a value passed to t() or interpolated into another message, but never safe |
Comment on lines
+147
to
+154
| // A lazy message macro is valid *only* as a direct call argument to another message macro call; | ||
| // anywhere else it's interpolated/stringified as `[object Object]` at runtime. | ||
| function isLazyPassthrough(node: TSESTree.Node, inner: InnerMacro): boolean { | ||
| if (inner.kind !== 'lazy') return false | ||
| const parent = node.parent | ||
| if (parent?.type !== TSESTree.AST_NODE_TYPES.CallExpression) return false | ||
| if (!parent.arguments.includes(node as TSESTree.CallExpressionArgument)) return false | ||
| return isMessageMacroNode(parent) !== null |
Comment on lines
34
to
+62
| const linguiMacroFunctionNames = ['plural', 'select', 'selectOrdinal', 'ph'] | ||
| const nestedMessageMacroNames = ['t', 'msg', 'defineMessage'] | ||
| const nestedComponentMacroNames = ['Trans', 'Plural', 'Select', 'SelectOrdinal'] | ||
|
|
||
| // Nested Lingui macros (message macros or JSX component macros) are the | ||
| // domain of `no-macro-inside-macro`, which reports them with a targeted | ||
| // message. Skip them here so users get one clear diagnostic instead of two. | ||
| function isNestedLinguiMacro(expression: TSESTree.Expression): boolean { | ||
| if (expression.type === TSESTree.AST_NODE_TYPES.TaggedTemplateExpression) { | ||
| return ( | ||
| expression.tag.type === TSESTree.AST_NODE_TYPES.Identifier && | ||
| nestedMessageMacroNames.includes(expression.tag.name) | ||
| ) | ||
| } | ||
| if (expression.type === TSESTree.AST_NODE_TYPES.CallExpression) { | ||
| return ( | ||
| expression.callee.type === TSESTree.AST_NODE_TYPES.Identifier && | ||
| nestedMessageMacroNames.includes(expression.callee.name) | ||
| ) | ||
| } | ||
| if (expression.type === TSESTree.AST_NODE_TYPES.JSXElement) { | ||
| const tag = expression.openingElement.name | ||
| return ( | ||
| tag.type === TSESTree.AST_NODE_TYPES.JSXIdentifier && | ||
| nestedComponentMacroNames.includes(tag.name) | ||
| ) | ||
| } | ||
| return false | ||
| } |
|
|
||
| ## Scope | ||
|
|
||
| This rule flags member expressions (`${obj.prop}`) and non-Lingui function calls (`${func()}`) interpolated into a message macro template. It does **not** flag nested Lingui macros — those are covered by [`no-macro-inside-macro`](./no-macro-inside-macro.md), which produces a targeted diagnostic. Enable both rules to get full coverage. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #90.
Adds a new
no-macro-inside-macrorule and narrowsno-expression-in-messageto avoid duplicate diagnostics on the overlapping cases.Motivation
Lingui's extractor silently produces broken
.pooutput when a translation macro is nested inside another — the inner macro becomes an opaque expression, the outer message falls back to positional placeholders ({0},{1}), and placeholder comments can balloon with source fragments (see lingui/js-lingui#2260). No existing rule caught all the ways this can happen, and the partial coverage inno-expression-in-messagewas reported with a misleading "Should be ${variable}, not ${object.property}" message.New rule:
no-macro-inside-macroForbids translation macros from being nested inside another translation macro. Scope:
t/msg/defineMessage, tagged-template or call form) inside another message macro's template literal or message-option body<Plural>/<Select>/<SelectOrdinal>branch JSXAttribute (exceptvalue/offset)plural()/select()/selectOrdinal()option value (exceptvalue/offset)<Trans>/<Plural>/<Select>/<SelectOrdinal>) interpolated into a message macro templateTwo exceptions are legitimate composition, not nesting:
t`${plural(n, { one: '…', other: '…' })}`composes the ICU plural into the outer message.t(msg`…`)passes a lazyMessageDescriptoras a direct argument for translation. Allowed only when the lazy macro is a direct argument to a message macro call; interpolating it into a template literal still stringifies as[object Object]at runtime and is flagged.Name follows the existing convention (
no-trans-inside-trans,no-plural-inside-trans).Files
src/rules/no-macro-inside-macro.ts— rule (229 lines)tests/src/rules/no-macro-inside-macro.test.ts— 26 valid + 23 invalid cases, all named, grouped by// ==================== Section ====================headers matchingno-unlocalized-strings.test.tsconventionsdocs/rules/no-macro-inside-macro.md— user-facing docs with scope, examples, and a performance notesrc/index.ts— register rule; add torecommendedRulesat'warn'(matching peer structural rules likeno-trans-inside-trans)README.md— rule index entryChange: narrow
no-expression-in-messageThe rule's docs describe its purpose as "member or function expressions in templates," but its
checkExpressionfall-through branch was also firing on any other expression type (nestedTaggedTemplateExpression,CallExpression,JSXElement) with a misleading generic message. That behaviour duplicated coverage the new rule now owns with a targeted diagnostic.This PR adds an
isNestedLinguiMacrohelper and skips nested macro interpolations in the message-template handler only. The Trans-children handler is untouched, so<Trans>Hello {func()}</Trans>and<Trans>Hello {obj.prop}</Trans>continue to fire as before.No existing test case regresses — all existing invalid cases are
${obj.prop}/${func()}/{obj.prop}shaped. Added 8 new valid cases documenting that nested macros in templates are now deferred.Clean split of responsibility
t`${obj.prop}`no-expression-in-messaget`${func()}`no-expression-in-message<Trans>{obj.prop}</Trans>no-expression-in-message<Trans>{func()}</Trans>no-expression-in-messaget`${t`x`}`no-macro-inside-macrot`${msg`x`}`no-macro-inside-macrot`${<Trans/>}`no-macro-inside-macro<Plural one={t`x`}/>no-macro-inside-macroplural(n, { one: t`x` })no-macro-inside-macro<Trans>{<Plural/>}</Trans>no-plural-inside-trans(pre-existing; not touched here)<Trans>{<Trans/>}</Trans>no-trans-inside-trans(pre-existing; not touched here)Pre-existing overlap not addressed
no-expression-in-message's Trans-children handler still overlaps withno-plural-inside-transandno-trans-inside-trans— all three fire on<Trans>{<Plural/>}</Trans>and<Trans>{<Trans/>}</Trans>. That's pre-existing and out of scope here; a follow-up could narrow the Trans-children fall-through the same way (skip when the expression is itself a component macro).Performance
The new rule fires only on nodes named
t/msg/defineMessageor JSX elements namedTrans/Plural/Select/SelectOrdinal(name-filtered at the selector level). Per match it walks up to the nearest containing Lingui macro — typically 1–5 hops, or to the program root if none is found. Linear in AST depth, no memoization, no quadratic paths. Comparable to the existing descendant-combinator rules (no-plural-inside-trans,no-trans-inside-trans). IIFE-bridged nesting is intentionally detected (the walk does not stop at function boundaries).The narrowing in
no-expression-in-messageadds one O(1) name-set check per template interpolation, skipping acontext.reportcall when it matches — net-positive on codebases that nest macros.Tests
yarn test— 361 tests pass across 12 suites (existing 312 + new 49 fromno-macro-inside-macro.test.ts+ 8 new valid cases inno-expression-in-message.test.ts). No regressions.