Skip to content

Commit 4aa4e6f

Browse files
JoostKatscott
authored andcommitted
fix(compiler): handle type references to namespaced symbols correctly (#36106)
When the compiler needs to convert a type reference to a value expression, it may encounter a type that refers to a namespaced symbol. Such namespaces need to be handled specially as there's various forms available. Consider a namespace named "ns": 1. One can refer to a namespace by itself: `ns`. A namespace is only allowed to be used in a type position if it has been merged with a class, but even if this is the case it may not be possible to convert that type into a value expression depending on the import form. More on this later (case a below) 2. One can refer to a type within the namespace: `ns.Foo`. An import needs to be generated to `ns`, from which the `Foo` property can then be read. 3. One can refer to a type in a nested namespace within `ns`: `ns.Foo.Bar` and possibly even deeper nested. The value representation is similar to case 2, but includes additional property accesses. The exact strategy of how to deal with these cases depends on the type of import used. There's two flavors available: a. A namespaced import like `import * as ns from 'ns';` that creates a local namespace that is irrelevant to the import that needs to be generated (as said import would be used instead of the original import). If the local namespace "ns" itself is referred to in a type position, it is invalid to convert it into a value expression. Some JavaScript libraries publish a value as default export using `export = MyClass;` syntax, however it is illegal to refer to that value using "ns". Consequently, such usage in a type position *must* be accompanied by an `@Inject` decorator to provide an explicit token. b. An explicit namespace declaration within a module, that can be imported using a named import like `import {ns} from 'ns';` where the "ns" module declares a namespace using `declare namespace ns {}`. In this case, it's the namespace itself that needs to be imported, after which any qualified references into the namespace are converted into property accesses. Before this change, support for namespaces in the type-to-value conversion was limited and only worked correctly for a single qualified name using a namespace import (case 2a). All other cases were either producing incorrect code or would crash the compiler (case 1a). Crashing the compiler is not desirable as it does not indicate where the issue is. Moreover, the result of a type-to-value conversion is irrelevant when an explicit injection token is provided using `@Inject`, so referring to a namespace in a type position (case 1) could still be valid. This commit introduces logic to the type-to-value conversion to be able to properly deal with all type references to namespaced symbols. Fixes #36006 Resolves FW-1995 PR Close #36106
1 parent 078b0be commit 4aa4e6f

File tree

7 files changed

+263
-80
lines changed

7 files changed

+263
-80
lines changed

packages/compiler-cli/ngcc/src/host/esm2015_host.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1436,7 +1436,8 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
14361436
local: false,
14371437
valueDeclaration: decl.node,
14381438
moduleName: decl.viaModule,
1439-
name: decl.node.name.text,
1439+
importedName: decl.node.name.text,
1440+
nestedPath: null,
14401441
};
14411442
} else {
14421443
typeValueReference = {

packages/compiler-cli/ngcc/test/host/util.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export function expectTypeValueReferencesForParameters(
3232
}
3333
} else if (param.typeValueReference !== null) {
3434
expect(param.typeValueReference.moduleName).toBe(fromModule!);
35-
expect(param.typeValueReference.name).toBe(expected);
35+
expect(param.typeValueReference.importedName).toBe(expected);
3636
}
3737
}
3838
});

packages/compiler-cli/src/ngtsc/annotations/src/util.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {Expression, ExternalExpr, LiteralExpr, ParseLocation, ParseSourceFile, ParseSourceSpan, R3DependencyMetadata, R3Reference, R3ResolvedDependencyType, WrappedNodeExpr} from '@angular/compiler';
9+
import {Expression, ExternalExpr, LiteralExpr, ParseLocation, ParseSourceFile, ParseSourceSpan, R3DependencyMetadata, R3Reference, R3ResolvedDependencyType, ReadPropExpr, WrappedNodeExpr} from '@angular/compiler';
1010
import * as ts from 'typescript';
1111

1212
import {ErrorCode, FatalDiagnosticError, makeDiagnostic} from '../../diagnostics';
@@ -138,7 +138,19 @@ export function valueReferenceToExpression(
138138
return new WrappedNodeExpr(valueRef.expression);
139139
} else {
140140
// TODO(alxhub): this cast is necessary because the g3 typescript version doesn't narrow here.
141-
return new ExternalExpr(valueRef as {moduleName: string, name: string});
141+
const ref = valueRef as {
142+
moduleName: string;
143+
importedName: string;
144+
nestedPath: string[]|null;
145+
};
146+
let importExpr: Expression =
147+
new ExternalExpr({moduleName: ref.moduleName, name: ref.importedName});
148+
if (ref.nestedPath !== null) {
149+
for (const property of ref.nestedPath) {
150+
importExpr = new ReadPropExpr(importExpr, property);
151+
}
152+
}
153+
return importExpr;
142154
}
143155
}
144156

packages/compiler-cli/src/ngtsc/reflection/src/host.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,24 @@ export type TypeValueReference = {
243243
local: true; expression: ts.Expression; defaultImportStatement: ts.ImportDeclaration | null;
244244
}|{
245245
local: false;
246-
name: string;
246+
247+
/**
248+
* The module specifier from which the `importedName` symbol should be imported.
249+
*/
247250
moduleName: string;
251+
252+
/**
253+
* The name of the top-level symbol that is imported from `moduleName`. If `nestedPath` is also
254+
* present, a nested object is being referenced from the top-level symbol.
255+
*/
256+
importedName: string;
257+
258+
/**
259+
* If present, represents the symbol names that are referenced from the top-level import.
260+
* When `null` or empty, the `importedName` itself is the symbol being referenced.
261+
*/
262+
nestedPath: string[]|null;
263+
248264
valueDeclaration: ts.Declaration;
249265
};
250266

packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts

Lines changed: 94 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -42,31 +42,76 @@ export function typeToValue(
4242
// Look at the local `ts.Symbol`'s declarations and see if it comes from an import
4343
// statement. If so, extract the module specifier and the name of the imported type.
4444
const firstDecl = local.declarations && local.declarations[0];
45+
if (firstDecl !== undefined) {
46+
if (ts.isImportClause(firstDecl) && firstDecl.name !== undefined) {
47+
// This is a default import.
48+
// import Foo from 'foo';
4549

46-
if (firstDecl && ts.isImportClause(firstDecl) && firstDecl.name !== undefined) {
47-
// This is a default import.
48-
return {
49-
local: true,
50-
// Copying the name here ensures the generated references will be correctly transformed along
51-
// with the import.
52-
expression: ts.updateIdentifier(firstDecl.name),
53-
defaultImportStatement: firstDecl.parent,
54-
};
55-
} else if (firstDecl && isImportSource(firstDecl)) {
56-
const origin = extractModuleAndNameFromImport(firstDecl, symbols.importName);
57-
return {local: false, valueDeclaration: decl.valueDeclaration, ...origin};
58-
} else {
59-
const expression = typeNodeToValueExpr(typeNode);
60-
if (expression !== null) {
6150
return {
6251
local: true,
63-
expression,
64-
defaultImportStatement: null,
52+
// Copying the name here ensures the generated references will be correctly transformed
53+
// along with the import.
54+
expression: ts.updateIdentifier(firstDecl.name),
55+
defaultImportStatement: firstDecl.parent,
56+
};
57+
} else if (ts.isImportSpecifier(firstDecl)) {
58+
// The symbol was imported by name
59+
// import {Foo} from 'foo';
60+
// or
61+
// import {Foo as Bar} from 'foo';
62+
63+
// Determine the name to import (`Foo`) from the import specifier, as the symbol names of
64+
// the imported type could refer to a local alias (like `Bar` in the example above).
65+
const importedName = (firstDecl.propertyName || firstDecl.name).text;
66+
67+
// The first symbol name refers to the local name, which is replaced by `importedName` above.
68+
// Any remaining symbol names make up the complete path to the value.
69+
const [_localName, ...nestedPath] = symbols.symbolNames;
70+
71+
const moduleName = extractModuleName(firstDecl.parent.parent.parent);
72+
return {
73+
local: false,
74+
valueDeclaration: decl.valueDeclaration,
75+
moduleName,
76+
importedName,
77+
nestedPath
78+
};
79+
} else if (ts.isNamespaceImport(firstDecl)) {
80+
// The import is a namespace import
81+
// import * as Foo from 'foo';
82+
83+
if (symbols.symbolNames.length === 1) {
84+
// The type refers to the namespace itself, which cannot be represented as a value.
85+
return null;
86+
}
87+
88+
// The first symbol name refers to the local name of the namespace, which is is discarded
89+
// as a new namespace import will be generated. This is followed by the symbol name that needs
90+
// to be imported and any remaining names that constitute the complete path to the value.
91+
const [_ns, importedName, ...nestedPath] = symbols.symbolNames;
92+
93+
const moduleName = extractModuleName(firstDecl.parent.parent);
94+
return {
95+
local: false,
96+
valueDeclaration: decl.valueDeclaration,
97+
moduleName,
98+
importedName,
99+
nestedPath
65100
};
66-
} else {
67-
return null;
68101
}
69102
}
103+
104+
// If the type is not imported, the type reference can be converted into an expression as is.
105+
const expression = typeNodeToValueExpr(typeNode);
106+
if (expression !== null) {
107+
return {
108+
local: true,
109+
expression,
110+
defaultImportStatement: null,
111+
};
112+
} else {
113+
return null;
114+
}
70115
}
71116

72117
/**
@@ -88,47 +133,51 @@ export function typeNodeToValueExpr(node: ts.TypeNode): ts.Expression|null {
88133
*
89134
* In the event that the `TypeReference` refers to a locally declared symbol, these will be the
90135
* same. If the `TypeReference` refers to an imported symbol, then `decl` will be the fully resolved
91-
* `ts.Symbol` of the referenced symbol. `local` will be the `ts.Symbol` of the `ts.Identifer` which
92-
* points to the import statement by which the symbol was imported.
136+
* `ts.Symbol` of the referenced symbol. `local` will be the `ts.Symbol` of the `ts.Identifier`
137+
* which points to the import statement by which the symbol was imported.
93138
*
94-
* In the event `typeRef` refers to a default import, an `importName` will also be returned to
95-
* give the identifier name within the current file by which the import is known.
139+
* All symbol names that make up the type reference are returned left-to-right into the
140+
* `symbolNames` array, which is guaranteed to include at least one entry.
96141
*/
97142
function resolveTypeSymbols(typeRef: ts.TypeReferenceNode, checker: ts.TypeChecker):
98-
{local: ts.Symbol, decl: ts.Symbol, importName: string|null}|null {
143+
{local: ts.Symbol, decl: ts.Symbol, symbolNames: string[]}|null {
99144
const typeName = typeRef.typeName;
100145
// typeRefSymbol is the ts.Symbol of the entire type reference.
101146
const typeRefSymbol: ts.Symbol|undefined = checker.getSymbolAtLocation(typeName);
102147
if (typeRefSymbol === undefined) {
103148
return null;
104149
}
105150

106-
// local is the ts.Symbol for the local ts.Identifier for the type.
151+
// `local` is the `ts.Symbol` for the local `ts.Identifier` for the type.
107152
// If the type is actually locally declared or is imported by name, for example:
108153
// import {Foo} from './foo';
109-
// then it'll be the same as top. If the type is imported via a namespace import, for example:
154+
// then it'll be the same as `typeRefSymbol`.
155+
//
156+
// If the type is imported via a namespace import, for example:
110157
// import * as foo from './foo';
111158
// and then referenced as:
112159
// constructor(f: foo.Foo)
113-
// then local will be the ts.Symbol of `foo`, whereas top will be the ts.Symbol of `foo.Foo`.
114-
// This allows tracking of the import behind whatever type reference exists.
160+
// then `local` will be the `ts.Symbol` of `foo`, whereas `typeRefSymbol` will be the `ts.Symbol`
161+
// of `foo.Foo`. This allows tracking of the import behind whatever type reference exists.
115162
let local = typeRefSymbol;
116-
let importName: string|null = null;
117163

118-
// TODO(alxhub): this is technically not correct. The user could have any import type with any
119-
// amount of qualification following the imported type:
120-
//
121-
// import * as foo from 'foo'
122-
// constructor(inject: foo.X.Y.Z)
123-
//
124-
// What we really want is the ability to express the arbitrary operation of `.X.Y.Z` on top of
125-
// whatever import we generate for 'foo'. This logic is sufficient for now, though.
126-
if (ts.isQualifiedName(typeName) && ts.isIdentifier(typeName.left) &&
127-
ts.isIdentifier(typeName.right)) {
128-
const localTmp = checker.getSymbolAtLocation(typeName.left);
164+
// Destructure a name like `foo.X.Y.Z` as follows:
165+
// - in `leftMost`, the `ts.Identifier` of the left-most name (`foo`) in the qualified name.
166+
// This identifier is used to resolve the `ts.Symbol` for `local`.
167+
// - in `symbolNames`, all names involved in the qualified path, or a single symbol name if the
168+
// type is not qualified.
169+
let leftMost = typeName;
170+
const symbolNames: string[] = [];
171+
while (ts.isQualifiedName(leftMost)) {
172+
symbolNames.unshift(leftMost.right.text);
173+
leftMost = leftMost.left;
174+
}
175+
symbolNames.unshift(leftMost.text);
176+
177+
if (leftMost !== typeName) {
178+
const localTmp = checker.getSymbolAtLocation(leftMost);
129179
if (localTmp !== undefined) {
130180
local = localTmp;
131-
importName = typeName.right.text;
132181
}
133182
}
134183

@@ -137,7 +186,7 @@ function resolveTypeSymbols(typeRef: ts.TypeReferenceNode, checker: ts.TypeCheck
137186
if (typeRefSymbol.flags & ts.SymbolFlags.Alias) {
138187
decl = checker.getAliasedSymbol(typeRefSymbol);
139188
}
140-
return {local, decl, importName};
189+
return {local, decl, symbolNames};
141190
}
142191

143192
function entityNameToValue(node: ts.EntityName): ts.Expression|null {
@@ -151,38 +200,9 @@ function entityNameToValue(node: ts.EntityName): ts.Expression|null {
151200
}
152201
}
153202

154-
function isImportSource(node: ts.Declaration): node is(ts.ImportSpecifier | ts.NamespaceImport) {
155-
return ts.isImportSpecifier(node) || ts.isNamespaceImport(node);
156-
}
157-
158-
function extractModuleAndNameFromImport(
159-
node: ts.ImportSpecifier|ts.NamespaceImport|ts.ImportClause,
160-
localName: string|null): {name: string, moduleName: string} {
161-
let name: string;
162-
let moduleSpecifier: ts.Expression;
163-
switch (node.kind) {
164-
case ts.SyntaxKind.ImportSpecifier:
165-
// The symbol was imported by name, in a ts.ImportSpecifier.
166-
name = (node.propertyName || node.name).text;
167-
moduleSpecifier = node.parent.parent.parent.moduleSpecifier;
168-
break;
169-
case ts.SyntaxKind.NamespaceImport:
170-
// The symbol was imported via a namespace import. In this case, the name to use when
171-
// importing it was extracted by resolveTypeSymbols.
172-
if (localName === null) {
173-
// resolveTypeSymbols() should have extracted the correct local name for the import.
174-
throw new Error(`Debug failure: no local name provided for NamespaceImport`);
175-
}
176-
name = localName;
177-
moduleSpecifier = node.parent.parent.moduleSpecifier;
178-
break;
179-
default:
180-
throw new Error(`Unreachable: ${ts.SyntaxKind[(node as ts.Node).kind]}`);
181-
}
182-
183-
if (!ts.isStringLiteral(moduleSpecifier)) {
203+
function extractModuleName(node: ts.ImportDeclaration): string {
204+
if (!ts.isStringLiteral(node.moduleSpecifier)) {
184205
throw new Error('not a module specifier');
185206
}
186-
const moduleName = moduleSpecifier.text;
187-
return {moduleName, name};
207+
return node.moduleSpecifier.text;
188208
}

packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ runInEachFileSystem(() => {
464464
expect(argExpressionToString(param.typeValueReference.expression)).toEqual(type);
465465
} else if (!param.typeValueReference.local && typeof type !== 'string') {
466466
expect(param.typeValueReference.moduleName).toEqual(type.moduleName);
467-
expect(param.typeValueReference.name).toEqual(type.name);
467+
expect(param.typeValueReference.importedName).toEqual(type.name);
468468
} else {
469469
return fail(`Mismatch between typeValueReference and expected type: ${param.name} / ${
470470
param.typeValueReference.local}`);

0 commit comments

Comments
 (0)