Skip to content

Commit 46ddfe6

Browse files
Brooooooklynclaude
andcommitted
fix(directive): assume globals declared outside the file are transform functions
ngtsc resolves a global like the DOM's `atob` or a project's `declare function f(v: string): number` to its declaration, and accepts it as an input transform. oxc only knew the ES2022 lib, so it reported "Input transform must be a function" for these. A name the file doesn't declare or import, and that isn't an ES2022 lib global, is now assumed to be a function, like an imported transform: it's emitted as written and only the `ngAcceptInputType_` clash is checked. Everywhere else it stays dynamic, as before, and so does a shorthand `{ transform }` naming one, which ngtsc never accepts. `Intl` and `Reflect` join the known lib globals (so they keep ngtsc's "reference to" error), `globalThis` stays dynamic, and a namespace or `import x = ...` declared in the file isn't taken for a global. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 71b0f7f commit 46ddfe6

3 files changed

Lines changed: 666 additions & 19 deletions

File tree

‎crates/oxc_angular_compiler/src/directive/evaluator.rs‎

Lines changed: 59 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ pub(crate) struct FileScope<'a> {
6161
/// ngtsc's diagnostics point at it: the whole statement (with `export`) for
6262
/// a function or class, the declarator (`x: T`) for a variable.
6363
declaration_spans: HashMap<&'a str, Span>,
64+
/// Namespaces and `import x = ...` aliases: declared in the file, but not
65+
/// read by the evaluator (their values are dynamic).
66+
unread: HashSet<&'a str>,
6467
}
6568

6669
/// A top-level variable binding.
@@ -150,6 +153,21 @@ impl<'a> FileScope<'a> {
150153
}
151154
}
152155
}
156+
for stmt in &program.body {
157+
let decl = match stmt {
158+
Statement::ExportDeclaration(export) => Some(&export.declaration),
159+
_ => stmt.as_declaration(),
160+
};
161+
match decl {
162+
Some(Declaration::TSNamespaceDeclaration(ns)) => {
163+
scope.unread.insert(ns.id.name.as_str());
164+
}
165+
Some(Declaration::TSImportEqualsDeclaration(alias)) => {
166+
scope.unread.insert(alias.id.name.as_str());
167+
}
168+
_ => {}
169+
}
170+
}
153171
scope
154172
}
155173

@@ -447,6 +465,13 @@ pub(crate) enum RefKind<'a> {
447465
/// An identifier with no declaration in this file that names a standard
448466
/// ECMAScript global (see [`ES_GLOBALS`]).
449467
Global,
468+
/// Any other identifier this file doesn't declare or import: a global
469+
/// declared outside it, in a lib such as the DOM's (`atob`, `window`) or in
470+
/// a project `.d.ts`, which ngtsc resolves to that declaration, or a name
471+
/// declared nowhere, which ngtsc can't resolve. oxc can't tell these apart,
472+
/// so it's dynamic (see [`Value::is_dynamic`]), except that an input
473+
/// transform assumes it's a function, as it does an imported one.
474+
Ambient,
450475
/// A static getter or setter, or a static property without an
451476
/// initializer, declared at the span.
452477
StaticMember(Span),
@@ -526,6 +551,12 @@ impl<'a> Value<'a> {
526551
matches!(self, Value::Reference { kind: RefKind::Import { .. }, .. })
527552
}
528553

554+
/// Whether ngtsc's value is unknown: dynamic, or a global declared outside
555+
/// the file, which is only kept as a name so a transform can refer to it.
556+
pub(crate) fn is_dynamic(&self) -> bool {
557+
matches!(self, Value::Dynamic | Value::Reference { kind: RefKind::Ambient, .. })
558+
}
559+
529560
/// ngtsc's `describeResolvedType`, one level deep like its diagnostics.
530561
pub(crate) fn describe(&self) -> String {
531562
self.describe_to(1)
@@ -563,6 +594,7 @@ impl<'a> Value<'a> {
563594
.join(", ")
564595
),
565596
Value::Module => "(module)".into(),
597+
_ if self.is_dynamic() => "(not statically analyzable)".into(),
566598
Value::Reference { name, .. } | Value::Enum { name, .. } => name.clone(),
567599
Value::Builtin(_) => "Function".into(),
568600
Value::Dynamic | Value::Function(_) => "(not statically analyzable)".into(),
@@ -572,9 +604,8 @@ impl<'a> Value<'a> {
572604
/// The chained line ngtsc's `createValueHasWrongTypeError` adds after a message.
573605
pub(crate) fn wrong_type_suffix(&self) -> String {
574606
match self {
575-
Value::Dynamic | Value::Function(_) => {
576-
" Value could not be determined statically.".into()
577-
}
607+
Value::Function(_) => " Value could not be determined statically.".into(),
608+
_ if self.is_dynamic() => " Value could not be determined statically.".into(),
578609
Value::Reference { name, .. } => format!(" Value is a reference to '{name}'."),
579610
_ => format!(" Value is of type '{}'.", self.describe()),
580611
}
@@ -617,10 +648,11 @@ fn quote_key(key: &str) -> String {
617648
}
618649
}
619650

620-
/// The `declare var` / `declare function` globals of TypeScript's ES2022
621-
/// library (lib.es5.d.ts ... lib.es2022.*.d.ts). ngtsc resolves these to a
622-
/// reference to their declaration; any other undeclared name is unknown to it.
623-
/// DOM globals depend on the `lib` option and are treated as unknown.
651+
/// The `declare var` / `declare function` / `declare namespace` globals of
652+
/// TypeScript's ES2022 library (lib.es5.d.ts ... lib.es2022.*.d.ts), which
653+
/// ngtsc resolves to a reference to their declaration. Other globals (the
654+
/// DOM's, a project's `.d.ts`) depend on the program, and are
655+
/// [`RefKind::Ambient`].
624656
const ES_GLOBALS: &[&str] = &[
625657
"AggregateError",
626658
"Array",
@@ -648,6 +680,7 @@ const ES_GLOBALS: &[&str] = &[
648680
"Int16Array",
649681
"Int32Array",
650682
"Int8Array",
683+
"Intl",
651684
"isFinite",
652685
"isNaN",
653686
"JSON",
@@ -662,6 +695,7 @@ const ES_GLOBALS: &[&str] = &[
662695
"Proxy",
663696
"RangeError",
664697
"ReferenceError",
698+
"Reflect",
665699
"RegExp",
666700
"Set",
667701
"SharedArrayBuffer",
@@ -969,7 +1003,12 @@ impl<'s, 'a> Evaluator<'s, 'a> {
9691003
None if self.consts.scope().variables.contains_key(name) => {
9701004
self.top_level(name, depth)
9711005
}
972-
None => self.eval(&p.value, depth, frame),
1006+
// ngtsc never resolves a shorthand to a global
1007+
// it can use as a transform.
1008+
None => match self.eval(&p.value, depth, frame) {
1009+
value if value.is_dynamic() => Value::Dynamic,
1010+
value => value,
1011+
},
9731012
}
9741013
}
9751014
(value, None) => self.eval(value, depth, frame),
@@ -1022,7 +1061,7 @@ impl<'s, 'a> Evaluator<'s, 'a> {
10221061
frame: &Frame<'a>,
10231062
) -> Value<'a> {
10241063
let object = self.eval(&m.object, depth, frame);
1025-
if matches!(object, Value::Dynamic) {
1064+
if object.is_dynamic() {
10261065
return Value::Dynamic;
10271066
}
10281067
match self.eval(&m.expression, depth, frame) {
@@ -1041,7 +1080,7 @@ impl<'s, 'a> Evaluator<'s, 'a> {
10411080
frame: &Frame<'a>,
10421081
) -> Value<'a> {
10431082
match self.eval(&c.test, depth, frame) {
1044-
Value::Dynamic => Value::Dynamic,
1083+
test if test.is_dynamic() => Value::Dynamic,
10451084
test if test.is_import() => test,
10461085
test if test.truthy() => self.eval(&c.consequent, depth, frame),
10471086
_ => self.eval(&c.alternate, depth, frame),
@@ -1060,7 +1099,7 @@ impl<'s, 'a> Evaluator<'s, 'a> {
10601099
return Value::Dynamic;
10611100
}
10621101
match self.eval(&u.argument, depth, frame) {
1063-
Value::Dynamic => Value::Dynamic,
1102+
value if value.is_dynamic() => Value::Dynamic,
10641103
value if value.is_import() => value,
10651104
value => unary(u.operator, &value),
10661105
}
@@ -1095,7 +1134,7 @@ impl<'s, 'a> Evaluator<'s, 'a> {
10951134
let left = self.eval(&l.left, depth, frame);
10961135
let right = self.eval(&l.right, depth, frame);
10971136
match (left, right) {
1098-
(Value::Dynamic, _) | (_, Value::Dynamic) => Value::Dynamic,
1137+
(left, right) if left.is_dynamic() || right.is_dynamic() => Value::Dynamic,
10991138
(value, _) | (_, value) if value.is_import() => value,
11001139
(left, right) => match (l.operator, left.truthy()) {
11011140
(LogicalOperator::And, true) | (LogicalOperator::Or, false) => right,
@@ -1187,8 +1226,11 @@ impl<'s, 'a> Evaluator<'s, 'a> {
11871226
_ if ES_GLOBALS.contains(&name) => {
11881227
Value::Reference { name: name.into(), kind: RefKind::Global }
11891228
}
1190-
// ngtsc can't find a declaration for it.
1191-
_ => Value::Dynamic,
1229+
// TypeScript declares `globalThis` without a declaration ngtsc
1230+
// can find.
1231+
"globalThis" => Value::Dynamic,
1232+
_ if scope.unread.contains(name) => Value::Dynamic,
1233+
_ => Value::Reference { name: name.into(), kind: RefKind::Ambient },
11921234
}
11931235
}
11941236

@@ -1720,6 +1762,9 @@ pub(crate) fn transform_error<'a>(
17201762
{
17211763
return clash();
17221764
}
1765+
// A global declared outside the file (`atob`) is assumed to be a
1766+
// function, like an imported one.
1767+
Value::Reference { kind: RefKind::Ambient, .. } => return clash(),
17231768
Value::Reference { .. } | Value::Dynamic => {
17241769
return Some((format!("Input transform must be a function{suffix}"), node()));
17251770
}

‎crates/oxc_angular_compiler/tests/decorator_metadata_ngtsc_test.rs‎

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ fn decorator_metadata_matches_ngtsc() {
299299
failures.len(),
300300
failures.join("\n\n")
301301
);
302-
assert_eq!(compared, 431, "fixtures compared");
302+
assert_eq!(compared, 451, "fixtures compared");
303303
}
304304

305305
fn transform(source: &str) -> TransformResult {
@@ -663,10 +663,10 @@ export class Dir { @Input({transform: core.booleanAttribute}) v: any; }
663663
}
664664

665665
/// A transform declared in another file (an import, a namespace member or a
666-
/// global from TypeScript's lib): ngtsc 22.1.7 reports these at that
667-
/// declaration, which the snapshot can't record (the fixtures below are
668-
/// skipped for that reason, with the diagnostics ngtsc reported). oxc reports
669-
/// the same message on the expression.
666+
/// global from TypeScript's lib, like `Intl` or the DOM's `atob`): ngtsc
667+
/// 22.1.7 reports these at that declaration, which the snapshot can't record
668+
/// (the fixtures below are skipped for that reason, with the diagnostics ngtsc
669+
/// reported). oxc reports the same message on the expression.
670670
#[test]
671671
fn transform_declared_in_another_file_is_reported_on_the_expression() {
672672
let fixtures: Value = serde_json::from_str(FIXTURES).unwrap();
@@ -678,6 +678,9 @@ fn transform_declared_in_another_file_is_reported_on_the_expression() {
678678
("probe: transform-globalNumberClash", "Number"),
679679
("probe: transform-globalString", "String"),
680680
("probe: transform-nsClash", "u.toNum"),
681+
("probe: transform-global-Intl", "Intl"),
682+
("probe: transform-global-Reflect", "Reflect"),
683+
("probe: transform-global-clashAtob", "atob"),
681684
];
682685
for (name, expression) in cases {
683686
let fixture = fixtures["fixtures"]
@@ -697,6 +700,58 @@ fn transform_declared_in_another_file_is_reported_on_the_expression() {
697700
}
698701
}
699702

703+
/// A name the file declares as a namespace or an `import x = ...` alias isn't
704+
/// a global declared elsewhere, so it isn't assumed to be a function the way
705+
/// `atob` is. ngtsc 22.1.7 rejects `transform: U` too, as "Value is a reference
706+
/// to 'U'" (oxc doesn't evaluate namespaces), and accepts `transform: f`
707+
/// (oxc doesn't follow the alias).
708+
#[test]
709+
fn file_namespaces_and_aliases_are_not_assumed_to_be_global_functions() {
710+
let namespace = "namespace U { export function f(v: string) { return 1; } }\n";
711+
for (pre, expr) in
712+
[(namespace.to_string(), "U"), (format!("{namespace}import f = U.f;\n"), "f")]
713+
{
714+
let source = format!(
715+
"import {{Directive, Input}} from '@angular/core';\n{pre}\
716+
@Directive({{selector: '[d]'}})\n\
717+
export class Dir {{\n @Input({{transform: {expr}}}) x: any;\n}}\n"
718+
);
719+
assert_eq!(
720+
errors(&transform(&source), &source),
721+
vec![(
722+
"Input transform must be a function Value could not be determined statically."
723+
.to_string(),
724+
expr.to_string()
725+
)],
726+
"{expr}"
727+
);
728+
}
729+
}
730+
731+
/// A shorthand `{ transform }` naming a global is looked up through
732+
/// TypeScript's shorthand symbol, which ngtsc 22.1.7 never accepts as a
733+
/// transform: "could not be determined statically" when nothing declares
734+
/// it, and an error about the declaration when a lib or `.d.ts` does (at
735+
/// that declaration). So unlike `transform: atob`, it isn't assumed to be a
736+
/// function.
737+
#[test]
738+
fn shorthand_transform_naming_a_global_is_rejected() {
739+
let source = "import {Directive} from '@angular/core';
740+
@Directive({selector: '[d]', inputs: [{name: 'x', transform}]})
741+
export class Dir {
742+
x!: any;
743+
}
744+
";
745+
assert_eq!(
746+
errors(&transform(source), source),
747+
vec![(
748+
"Input transform must be a function Value could not be determined statically."
749+
.to_string(),
750+
"transform".to_string()
751+
)]
752+
);
753+
}
754+
700755
/// ngtsc checks an overloaded static method's first declaration, not its
701756
/// implementation (checked with @angular/compiler-cli 22.1.7, which compiles
702757
/// this; the snapshot can't hold it because ngtsc emits the method's bare name,

0 commit comments

Comments
 (0)