Skip to content

Commit 9e55a25

Browse files
longlhoclaude
longlho
and
claude
authored
fix(@formatjs/ts-transformer): include source location in extraction warnings (#6432)
## Summary - Add `file:line:col` context to extraction error/warning messages in both the TypeScript transformer and Rust CLI extractor - TS side: `handleError` now accepts an optional AST node and uses `ts.getLineAndCharacterOfPosition()` to prepend location - Rust side: `MessageExtractor` stores source text and uses `get_line_col()` to format `file:line:col` in warnings/panics. Also emits `eprintln!` warnings when `throws=false` (previously silently skipped) **Before:** `[FormatJS] \`id\` must be a string literal to be extracted.` **After:** `src/components/Foo.tsx:12:5 [FormatJS] \`id\` must be a string literal to be extracted.` Closes #6428 ## Test plan - [x] `bazel test //crates/formatjs_cli:formatjs_cli_test` — 176 tests pass - [x] `bazel test //packages/ts-transformer:unit_test` — all tests pass - [x] `bazel test //packages/cli/integration-tests:conformance_test` — 60/60 conformance tests pass - [x] All pre-commit hooks pass (oxfmt, rustfmt, gazelle, ast-grep, oxlint, commitlint) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 15adcca commit 9e55a25

3 files changed

Lines changed: 246 additions & 22 deletions

File tree

‎crates/formatjs_cli/src/extractor.rs‎

Lines changed: 204 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ pub fn extract_messages_from_source(
9191
// Visit the AST to extract messages
9292
let mut visitor = MessageExtractor::new(
9393
file_path,
94+
source_text,
9495
extract_source_location,
9596
component_names,
9697
function_names,
@@ -185,6 +186,7 @@ pub fn determine_source_type(path: &Path) -> Result<SourceType> {
185186
/// AST visitor to extract message descriptors
186187
struct MessageExtractor<'a> {
187188
file_path: &'a Path,
189+
source_text: &'a str,
188190
extract_source_location: bool,
189191
component_names: &'a [String],
190192
function_names: &'a [String],
@@ -197,6 +199,7 @@ struct MessageExtractor<'a> {
197199
impl<'a> MessageExtractor<'a> {
198200
fn new(
199201
file_path: &'a Path,
202+
source_text: &'a str,
200203
extract_source_location: bool,
201204
component_names: &'a [String],
202205
function_names: &'a [String],
@@ -206,6 +209,7 @@ impl<'a> MessageExtractor<'a> {
206209
) -> Self {
207210
Self {
208211
file_path,
212+
source_text,
209213
extract_source_location,
210214
component_names,
211215
function_names,
@@ -216,6 +220,12 @@ impl<'a> MessageExtractor<'a> {
216220
}
217221
}
218222

223+
/// Format a source location string like "file.tsx:line:col"
224+
fn format_location(&self, offset: u32) -> String {
225+
let (line, col) = get_line_col(self.source_text, offset as usize);
226+
format!("{}:{}:{}", self.file_path.display(), line, col)
227+
}
228+
219229
fn extract_string_literal(
220230
&self,
221231
expr: &Expression,
@@ -380,20 +390,34 @@ impl<'a> MessageExtractor<'a> {
380390
match attr_name {
381391
"id" => {
382392
descriptor.id = self.extract_string_literal(expr, None);
383-
// Validate id is static if throws is enabled
384-
if self.throws && descriptor.id.is_none() {
385-
panic!(
386-
"defaultMessage must be a string literal to be extracted."
393+
if descriptor.id.is_none() {
394+
let loc = self.format_location(jsx_attr.span.start);
395+
if self.throws {
396+
panic!(
397+
"{} [FormatJS] `id` must be a string literal to be extracted.",
398+
loc
399+
);
400+
}
401+
eprintln!(
402+
"{} [FormatJS] `id` must be a string literal to be extracted.",
403+
loc
387404
);
388405
}
389406
}
390407
"defaultMessage" => {
391408
descriptor.default_message =
392409
self.extract_string_literal(expr, None);
393-
// Validate defaultMessage is static if throws is enabled
394-
if self.throws && descriptor.default_message.is_none() {
395-
panic!(
396-
"defaultMessage must be a string literal to be extracted."
410+
if descriptor.default_message.is_none() {
411+
let loc = self.format_location(jsx_attr.span.start);
412+
if self.throws {
413+
panic!(
414+
"{} [FormatJS] `defaultMessage` must be a string literal to be extracted.",
415+
loc
416+
);
417+
}
418+
eprintln!(
419+
"{} [FormatJS] `defaultMessage` must be a string literal to be extracted.",
420+
loc
397421
);
398422
}
399423
}
@@ -498,17 +522,35 @@ impl<'a> MessageExtractor<'a> {
498522
match key.name.as_str() {
499523
"id" => {
500524
descriptor.id = self.extract_string_literal(&p.value, None);
501-
// Validate id is static if throws is enabled
502-
if self.throws && descriptor.id.is_none() {
503-
panic!("defaultMessage must be a string literal to be extracted.");
525+
if descriptor.id.is_none() {
526+
let loc = self.format_location(p.span.start);
527+
if self.throws {
528+
panic!(
529+
"{} [FormatJS] `id` must be a string literal to be extracted.",
530+
loc
531+
);
532+
}
533+
eprintln!(
534+
"{} [FormatJS] `id` must be a string literal to be extracted.",
535+
loc
536+
);
504537
}
505538
}
506539
"defaultMessage" => {
507540
descriptor.default_message =
508541
self.extract_string_literal(&p.value, None);
509-
// Validate defaultMessage is static if throws is enabled
510-
if self.throws && descriptor.default_message.is_none() {
511-
panic!("defaultMessage must be a string literal to be extracted.");
542+
if descriptor.default_message.is_none() {
543+
let loc = self.format_location(p.span.start);
544+
if self.throws {
545+
panic!(
546+
"{} [FormatJS] `defaultMessage` must be a string literal to be extracted.",
547+
loc
548+
);
549+
}
550+
eprintln!(
551+
"{} [FormatJS] `defaultMessage` must be a string literal to be extracted.",
552+
loc
553+
);
512554
}
513555
}
514556
"description" => {
@@ -1616,4 +1658,152 @@ export default function Test() {
16161658
assert!(error.contains("Cannot hoist plural/select within a tag element"), "Error should include original error message");
16171659
assert!(error.contains("<b>{count"), "Error should include problematic message");
16181660
}
1661+
1662+
#[test]
1663+
fn test_non_static_id_throws_includes_location() {
1664+
// When throws=true and id is non-static, the panic message should include file:line:col
1665+
let source = r#"import { defineMessage } from 'react-intl';
1666+
const dynamicId = 'foo';
1667+
const msg = defineMessage({
1668+
id: dynamicId,
1669+
defaultMessage: 'Hello',
1670+
});
1671+
"#;
1672+
1673+
let file_path = PathBuf::from("src/App.tsx");
1674+
let source_type = SourceType::default().with_typescript(true).with_jsx(true);
1675+
let component_names = vec!["FormattedMessage".to_string()];
1676+
let function_names = vec![
1677+
"defineMessage".to_string(),
1678+
"defineMessages".to_string(),
1679+
"formatMessage".to_string(),
1680+
];
1681+
1682+
let result = std::panic::catch_unwind(|| {
1683+
extract_messages_from_source(
1684+
source,
1685+
&file_path,
1686+
source_type,
1687+
false,
1688+
&component_names,
1689+
&function_names,
1690+
HashMap::new(),
1691+
false,
1692+
false,
1693+
true, // throws = true
1694+
)
1695+
});
1696+
1697+
assert!(result.is_err(), "Should panic with throws=true on non-static id");
1698+
let panic_msg = result
1699+
.unwrap_err()
1700+
.downcast_ref::<String>()
1701+
.cloned()
1702+
.unwrap_or_default();
1703+
assert!(
1704+
panic_msg.contains("src/App.tsx:4:"),
1705+
"Panic message should include file:line:col, got: {}",
1706+
panic_msg
1707+
);
1708+
assert!(
1709+
panic_msg.contains("[FormatJS]"),
1710+
"Panic message should include [FormatJS] prefix, got: {}",
1711+
panic_msg
1712+
);
1713+
}
1714+
1715+
#[test]
1716+
fn test_non_static_default_message_throws_includes_location() {
1717+
// When throws=true and defaultMessage is non-static, the panic message should include file:line:col
1718+
let source = r#"import { FormattedMessage } from 'react-intl';
1719+
export default function App() {
1720+
const msg = getDynamicMessage();
1721+
return <FormattedMessage id="test" defaultMessage={msg} />;
1722+
}
1723+
"#;
1724+
1725+
let file_path = PathBuf::from("src/Component.tsx");
1726+
let source_type = SourceType::default().with_typescript(true).with_jsx(true);
1727+
let component_names = vec!["FormattedMessage".to_string()];
1728+
let function_names = vec!["formatMessage".to_string()];
1729+
1730+
let result = std::panic::catch_unwind(|| {
1731+
extract_messages_from_source(
1732+
source,
1733+
&file_path,
1734+
source_type,
1735+
false,
1736+
&component_names,
1737+
&function_names,
1738+
HashMap::new(),
1739+
false,
1740+
false,
1741+
true, // throws = true
1742+
)
1743+
});
1744+
1745+
assert!(result.is_err(), "Should panic with throws=true on non-static defaultMessage");
1746+
let panic_msg = result
1747+
.unwrap_err()
1748+
.downcast_ref::<String>()
1749+
.cloned()
1750+
.unwrap_or_default();
1751+
assert!(
1752+
panic_msg.contains("src/Component.tsx:4:"),
1753+
"Panic message should include file:line:col, got: {}",
1754+
panic_msg
1755+
);
1756+
assert!(
1757+
panic_msg.contains("`defaultMessage`"),
1758+
"Panic message should mention defaultMessage, got: {}",
1759+
panic_msg
1760+
);
1761+
}
1762+
1763+
#[test]
1764+
fn test_non_static_values_no_throw_skips_and_warns() {
1765+
// When throws=false and defaultMessage is non-static, messages should be skipped (not panic)
1766+
let source = r#"import { defineMessage } from 'react-intl';
1767+
const dynamic = getDynamic();
1768+
const msg1 = defineMessage({
1769+
id: 'skip.me',
1770+
defaultMessage: dynamic,
1771+
});
1772+
const msg2 = defineMessage({
1773+
id: 'valid.message',
1774+
defaultMessage: 'This is valid',
1775+
});
1776+
"#;
1777+
1778+
let file_path = PathBuf::from("src/App.tsx");
1779+
let source_type = SourceType::default().with_typescript(true).with_jsx(true);
1780+
let component_names = vec!["FormattedMessage".to_string()];
1781+
let function_names = vec![
1782+
"defineMessage".to_string(),
1783+
"defineMessages".to_string(),
1784+
"formatMessage".to_string(),
1785+
];
1786+
1787+
let messages = extract_messages_from_source(
1788+
source,
1789+
&file_path,
1790+
source_type,
1791+
false,
1792+
&component_names,
1793+
&function_names,
1794+
HashMap::new(),
1795+
false,
1796+
false,
1797+
false, // throws = false
1798+
)
1799+
.unwrap();
1800+
1801+
// Only the valid message should be extracted (the one with non-static defaultMessage is skipped)
1802+
assert_eq!(messages.len(), 1, "Should extract only the valid message");
1803+
assert_eq!(messages[0].id.as_deref(), Some("valid.message"));
1804+
assert_eq!(
1805+
messages[0].default_message.as_deref(),
1806+
Some("This is valid")
1807+
);
1808+
}
16191809
}

‎packages/ts-transformer/tests/index.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,26 @@ describe('emit asserts for', function () {
726726
/\[formatjs\] Cannot flatten message in file.*flattenError\.tsx.*at line \d+, column \d+.*with id "test\.message".*Cannot hoist plural\/select within a tag element/
727727
)
728728
})
729+
730+
it('GH #6428 - extraction error with throws=true includes file:line:col', async function () {
731+
await expect(
732+
compile(join(FIXTURES_DIR, 'nonStaticMessages.tsx'), {})
733+
).rejects.toThrow(/nonStaticMessages\.tsx:\d+:\d+ \[FormatJS\]/)
734+
})
735+
736+
it('GH #6428 - extraction error with throws=false reports file:line:col via onMsgError', async function () {
737+
const errors: Error[] = []
738+
await compile(join(FIXTURES_DIR, 'nonStaticMessages.tsx'), {
739+
throws: false,
740+
onMsgError: (_filePath: string, error: Error) => {
741+
errors.push(error)
742+
},
743+
})
744+
expect(errors.length).toBeGreaterThan(0)
745+
expect(errors[0].message).toMatch(
746+
/nonStaticMessages\.tsx:\d+:\d+ \[FormatJS\]/
747+
)
748+
})
729749
})
730750

731751
async function compile(filePath: string, options?: Partial<Opts>) {

‎packages/ts-transformer/transform.ts‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,16 @@ function extractMessageDescriptor(
286286
let extractionError: Error | null = null
287287

288288
// Helper to handle errors based on throws option
289-
function handleError(errorMsg: string): void {
290-
const error = new Error(errorMsg)
289+
function handleError(errorMsg: string, errorNode?: typescript.Node): void {
290+
let locationMsg = errorMsg
291+
if (errorNode) {
292+
const {line, character} = ts.getLineAndCharacterOfPosition(
293+
sf,
294+
errorNode.getStart(sf)
295+
)
296+
locationMsg = `${sf.fileName}:${line + 1}:${character + 1} ${errorMsg}`
297+
}
298+
const error = new Error(locationMsg)
291299
if (throws) {
292300
throw error
293301
}
@@ -364,7 +372,8 @@ function extractMessageDescriptor(
364372
const {template} = initializer
365373
if (!ts.isNoSubstitutionTemplateLiteral(template)) {
366374
handleError(
367-
'[FormatJS] Tagged template expression must be no substitution'
375+
'[FormatJS] Tagged template expression must be no substitution',
376+
prop
368377
)
369378
return
370379
}
@@ -437,7 +446,8 @@ function extractMessageDescriptor(
437446
} = initializer
438447
if (!ts.isNoSubstitutionTemplateLiteral(template)) {
439448
handleError(
440-
'[FormatJS] Tagged template expression must be no substitution'
449+
'[FormatJS] Tagged template expression must be no substitution',
450+
prop
441451
)
442452
return
443453
}
@@ -475,7 +485,8 @@ function extractMessageDescriptor(
475485
) {
476486
// Non-static expression for defaultMessage or id
477487
handleError(
478-
`[FormatJS] \`${name.text}\` must be a string literal or statically evaluable expression to be extracted.`
488+
`[FormatJS] \`${name.text}\` must be a string literal or statically evaluable expression to be extracted.`,
489+
prop
479490
)
480491
return
481492
}
@@ -486,7 +497,8 @@ function extractMessageDescriptor(
486497
name.text !== 'description'
487498
) {
488499
handleError(
489-
`[FormatJS] \`${name.text}\` must be a string literal to be extracted.`
500+
`[FormatJS] \`${name.text}\` must be a string literal to be extracted.`,
501+
prop
490502
)
491503
return
492504
}
@@ -512,7 +524,8 @@ function extractMessageDescriptor(
512524
) {
513525
// Non-static expression for defaultMessage or id
514526
handleError(
515-
`[FormatJS] \`${name.text}\` must be a string literal or statically evaluable expression to be extracted.`
527+
`[FormatJS] \`${name.text}\` must be a string literal or statically evaluable expression to be extracted.`,
528+
prop
516529
)
517530
return
518531
}
@@ -530,7 +543,8 @@ function extractMessageDescriptor(
530543
name.text !== 'description'
531544
) {
532545
handleError(
533-
`[FormatJS] \`${name.text}\` must be a string literal to be extracted.`
546+
`[FormatJS] \`${name.text}\` must be a string literal to be extracted.`,
547+
prop
534548
)
535549
return
536550
}

0 commit comments

Comments
 (0)