Skip to content

Commit d808cb0

Browse files
GabeSilvaDevBrooooooklynclaude
authored
fix(i18n): reject a meaning or description that would close its JSDoc comment, and escape @ in it (#650)
* fix(i18n): reject a meaning or description that would close its JSDoc comment, and escape `@` in it * fix(i18n): report a `*/` meaning or description only when its JSDoc is printed Angular's `tagToString` throws when it prints the JSDoc, so `ngc` compiles a template whose `*/` meta has no message: an empty or comment-only `i18n` element, `<ng-content i18n>`, an `i18n-title` that a later one replaces, an `i18n-alt` with no value, and partial compilation. The check now runs where the message's JSDoc is built and goes out as an error from full, template, HMR and linker compilation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: LongYinan <lynweklm@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 9f79265 commit d808cb0

5 files changed

Lines changed: 117 additions & 3 deletions

File tree

‎crates/oxc_angular_compiler/src/component/transform.rs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5822,6 +5822,11 @@ fn compile_component_full<'a>(
58225822

58235823
let compiled = compile_template(&mut job);
58245824

5825+
if !job.emit_errors.is_empty() {
5826+
diagnostics.append(&mut job.emit_errors);
5827+
return Err(diagnostics);
5828+
}
5829+
58255830
// Stage 6: Compile host bindings (if any)
58265831
// Pass the template pool's current index to ensure host binding constants
58275832
// continue from where template compilation left off (avoiding duplicate names)
@@ -6203,6 +6208,11 @@ pub fn compile_component_template<'a>(
62036208
// Collect any diagnostics from the compilation job
62046209
diagnostics.extend(job.diagnostics.into_iter());
62056210

6211+
if !job.emit_errors.is_empty() {
6212+
diagnostics.append(&mut job.emit_errors);
6213+
return Err(diagnostics);
6214+
}
6215+
62066216
Ok((compiled.template_fn, diagnostics))
62076217
}
62086218

@@ -6338,6 +6348,11 @@ pub fn compile_template_to_js_with_options<'a>(
63386348
// Collect any diagnostics from the compilation job
63396349
diagnostics.extend(job.diagnostics.into_iter());
63406350

6351+
if !job.emit_errors.is_empty() {
6352+
diagnostics.append(&mut job.emit_errors);
6353+
return Err(diagnostics);
6354+
}
6355+
63416356
let emitter = JsEmitter::new();
63426357

63436358
// Build a list of all statements to emit:
@@ -6528,6 +6543,11 @@ pub fn compile_template_for_hmr<'a>(
65286543
// Collect any diagnostics from the compilation job
65296544
diagnostics.extend(job.diagnostics.into_iter());
65306545

6546+
if !job.emit_errors.is_empty() {
6547+
diagnostics.append(&mut job.emit_errors);
6548+
return Err(diagnostics);
6549+
}
6550+
65316551
// A pipeline error means the emitted code is already wrong — an update
65326552
// module must not be generated from it, and `decls`/`vars` may not be
65336553
// reliable (they default to 0 when slot allocation never ran, which would
@@ -7278,6 +7298,11 @@ pub fn compile_template_for_linker<'a>(
72787298
// Collect diagnostics
72797299
diagnostics.extend(job.diagnostics.into_iter());
72807300

7301+
if !job.emit_errors.is_empty() {
7302+
diagnostics.append(&mut job.emit_errors);
7303+
return Err(diagnostics);
7304+
}
7305+
72817306
// Extract numeric metadata from the compilation job
72827307
let decls = job.root.decl_count.unwrap_or(0);
72837308
let vars = job.root.vars.unwrap_or(0);

‎crates/oxc_angular_compiler/src/output/emitter.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -517,13 +517,14 @@ impl JsEmitter {
517517
// Format JSDoc comment with @desc, @meaning, and @suppress tags
518518
// See: packages/compiler/src/output/output_jit_trusted_types.ts
519519
ctx.print("/**");
520+
// `@` in the text is escaped, as Angular's `tagToString` does, so it starts no tag.
520521
if let Some(ref desc) = jsdoc.description {
521522
ctx.print(" @desc ");
522-
ctx.print(desc.as_str());
523+
ctx.print(&desc.as_str().replace('@', "\\@"));
523524
}
524525
if let Some(ref meaning) = jsdoc.meaning {
525526
ctx.print(" @meaning ");
526-
ctx.print(meaning.as_str());
527+
ctx.print(&meaning.as_str().replace('@', "\\@"));
527528
}
528529
if jsdoc.suppress_msg_descriptions {
529530
ctx.print(" @suppress {msgDescriptions}");

‎crates/oxc_angular_compiler/src/pipeline/compilation.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,11 @@ pub struct ComponentCompilationJob<'a> {
235235
pub has_hoisted_arrow_functions: bool,
236236
/// Diagnostics collected during compilation.
237237
pub diagnostics: std::vec::Vec<OxcDiagnostic>,
238+
/// Errors that make the generated code unsafe to emit.
239+
///
240+
/// Unlike `diagnostics`, which full compilation drops, every entry point returns these as an
241+
/// error.
242+
pub emit_errors: std::vec::Vec<OxcDiagnostic>,
238243
}
239244

240245
impl<'a> ComponentCompilationJob<'a> {
@@ -287,6 +292,7 @@ impl<'a> ComponentCompilationJob<'a> {
287292
core_namespace: Ident::from(crate::r3::CORE_ALIAS),
288293
has_hoisted_arrow_functions: false,
289294
diagnostics: std::vec::Vec::new(),
295+
emit_errors: std::vec::Vec::new(),
290296
}
291297
}
292298

‎crates/oxc_angular_compiler/src/pipeline/phases/i18n_const_collection.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use std::ptr::NonNull;
99

1010
use oxc_allocator::Vec as ArenaVec;
11+
use oxc_diagnostics::OxcDiagnostic;
1112
use oxc_str::Ident;
1213
use rustc_hash::FxHashMap;
1314

@@ -202,6 +203,7 @@ pub fn collect_i18n_consts(job: &mut ComponentCompilationJob<'_>) {
202203
&file_based_i18n_suffix,
203204
job.i18n_use_external_ids,
204205
&mut i18n_var_counter,
206+
&mut job.emit_errors,
205207
);
206208

207209
if let Some(block_xref) = msg_info.i18n_block {
@@ -475,6 +477,7 @@ fn collect_message<'a>(
475477
file_suffix: &str,
476478
use_external_ids: bool,
477479
counter: &mut usize,
480+
errors: &mut std::vec::Vec<OxcDiagnostic>,
478481
) -> (String, ArenaVec<'a, OutputStatement<'a>>) {
479482
let mut all_statements = ArenaVec::new_in(&allocator);
480483

@@ -494,6 +497,7 @@ fn collect_message<'a>(
494497
file_suffix,
495498
use_external_ids,
496499
counter,
500+
errors,
497501
);
498502
all_statements.extend(sub_statements);
499503

@@ -542,6 +546,16 @@ fn collect_message<'a>(
542546
);
543547
*counter += 1;
544548

549+
// The meaning and description go into a JSDoc comment, so, as Angular's `tagToString` throws,
550+
// neither may end that comment or start another.
551+
if [&msg_info.description, &msg_info.meaning]
552+
.into_iter()
553+
.flatten()
554+
.any(|text| text.contains("/*") || text.contains("*/"))
555+
{
556+
errors.push(OxcDiagnostic::error(r#"JSDoc text cannot contain "/*" and "*/""#));
557+
}
558+
545559
// Create metadata for JSDoc
546560
let meta = if msg_info.description.is_some() || msg_info.meaning.is_some() {
547561
let desc = msg_info.description.as_ref().map(|d| {

‎crates/oxc_angular_compiler/tests/integration_test.rs‎

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55

66
use oxc_allocator::Allocator;
77
use oxc_angular_compiler::{
8-
AngularVersion, R3Node, ResolvedResources, TransformOptions as ComponentTransformOptions,
8+
AngularVersion, CompilationMode, R3Node, ResolvedResources,
9+
TransformOptions as ComponentTransformOptions,
910
output::ast::FunctionExpr,
1011
output::emitter::JsEmitter,
1112
parser::html::HtmlParser,
@@ -5902,6 +5903,73 @@ fn test_i18n_legacy_message_ids_untrimmed_attribute_and_nested_icu() {
59025903
assert_contains(&js, ":ICU@@5870918674674044418:");
59035904
}
59045905

5906+
/// A meaning or description goes in a JSDoc comment, so `*/` in it would end the comment and
5907+
/// leave the rest as code. Angular's `tagToString` throws `JSDoc text cannot contain "/*" and
5908+
/// "*/"`, and the template is reported with that error instead of compiled.
5909+
#[test]
5910+
fn test_i18n_meta_that_would_close_its_jsdoc_comment_is_an_error() {
5911+
for template in [
5912+
r#"<p i18n="m|d */ steal() /*">Hello</p>"#,
5913+
r#"<img i18n-alt="*/ } steal(); { /*|d" alt="Logo" />"#,
5914+
r#"<p i18n="m /* x|d">Hello</p>"#,
5915+
] {
5916+
let allocator = Allocator::default();
5917+
let source = format!(
5918+
"import {{ Component }} from '@angular/core';\n\
5919+
@Component({{ selector: 'app-repro', template: `{template}` }})\n\
5920+
export class Repro {{}}\n"
5921+
);
5922+
let result = transform_angular_file(&allocator, "repro.ts", &source, None, None);
5923+
let messages: Vec<String> = result.diagnostics.iter().map(|d| d.to_string()).collect();
5924+
assert!(
5925+
messages.iter().any(|m| m.contains(r#"JSDoc text cannot contain "/*" and "*/""#)),
5926+
"{template}: {messages:?}"
5927+
);
5928+
assert!(!result.code.contains("/**"), "{}", result.code);
5929+
}
5930+
}
5931+
5932+
/// Angular throws only when it prints the JSDoc, so `*/` is fine where no message is emitted,
5933+
/// and in partial compilation, which emits the template source. Angular 22.2 `ngc` compiles
5934+
/// each of these.
5935+
#[test]
5936+
fn test_i18n_meta_that_would_close_its_jsdoc_comment_is_fine_with_no_message() {
5937+
let full = ComponentTransformOptions::default();
5938+
let partial = ComponentTransformOptions {
5939+
compilation_mode: CompilationMode::Partial,
5940+
..ComponentTransformOptions::default()
5941+
};
5942+
for (template, options) in [
5943+
(r#"<p i18n="*/x"></p>"#, &full),
5944+
(r#"<p i18n="*/x"> </p>"#, &full),
5945+
(r#"<p i18n="*/x"><!-- c --></p>"#, &full),
5946+
(r#"<ng-container i18n="*/x"></ng-container>"#, &full),
5947+
(r#"<ng-content i18n="*/x"></ng-content>"#, &full),
5948+
(r#"<p title="a" i18n-title="*/x" i18n-title="ok">x</p>"#, &full),
5949+
(r#"<img i18n-alt="*/x" />"#, &full),
5950+
(r#"<img i18n-alt="*/x" [alt]="alt" />"#, &full),
5951+
(r#"<p i18n="m|d */ steal() /*">Hello</p>"#, &partial),
5952+
] {
5953+
let allocator = Allocator::default();
5954+
let source = format!(
5955+
"import {{ Component }} from '@angular/core';\n\
5956+
@Component({{ selector: 'app-repro', template: `{template}` }})\n\
5957+
export class Repro {{ alt = ''; }}\n"
5958+
);
5959+
let result = transform_angular_file(&allocator, "repro.ts", &source, Some(options), None);
5960+
assert!(!result.has_errors(), "{template}: {:?}", result.diagnostics);
5961+
assert!(result.code.contains("Repro"), "{}", result.code);
5962+
}
5963+
}
5964+
5965+
/// `@` in a meaning or description is escaped, as Angular's `tagToString` does, so it does not
5966+
/// start a JSDoc tag.
5967+
#[test]
5968+
fn test_i18n_meta_at_sign_is_escaped_in_jsdoc() {
5969+
let js = compile_i18n_component(r#"<p i18n="m@x|mail a@b.c">Hello</p>"#);
5970+
assert_contains(&js, r"/** @desc mail a\@b.c @meaning m\@x */");
5971+
}
5972+
59055973
/// `consts` with i18n initializers is an arrow function with a block body, and a message is
59065974
/// a native `$localize` tagged template. Angular 22.2 `ngc` (ES2022):
59075975
/// consts: () => { let i18n_0; ... i18n_0 = $localize `:@@close:Close`; } return [...]; }

0 commit comments

Comments
 (0)