Skip to content

Commit 4ef8689

Browse files
chuckjazchuckjaz
authored
fix(compiler): Ignore references to declared modules and unneeded types (angular#9776)
Fixes: angular#9670
1 parent eb5763c commit 4ef8689

File tree

7 files changed

+70
-46
lines changed

7 files changed

+70
-46
lines changed

‎modules/@angular/compiler-cli/src/reflector_host.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,10 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator {
123123
const filePath = this.resolve(module, containingFile);
124124

125125
if (!filePath) {
126-
throw new Error(`Could not resolve module ${module} relative to ${containingFile}`);
126+
// If the file cannot be found the module is probably referencing a declared module
127+
// for which there is no disambiguating file and we also don't need to track
128+
// re-exports. Just use the module name.
129+
return this.getStaticSymbol(module, symbolName);
127130
}
128131

129132
const tc = this.program.getTypeChecker();
@@ -174,10 +177,12 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator {
174177
return result;
175178
}
176179

177-
// TODO(alexeagle): take a statictype
178180
getMetadataFor(filePath: string): ModuleMetadata {
179181
if (!this.context.exists(filePath)) {
180-
throw new Error(`No such file '${filePath}'`);
182+
// If the file doesn't exists then we cannot return metadata for the file.
183+
// This will occur if the user refernced a declared module for which no file
184+
// exists for the module (i.e. jQuery or angularjs).
185+
return;
181186
}
182187
if (DTS.test(filePath)) {
183188
const metadataPath = filePath.replace(DTS, '.metadata.json');

‎modules/@angular/compiler-cli/src/static_reflector.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ export class StaticReflector implements ReflectorReader {
474474
let message = produceErrorMessage(expression);
475475
if (expression['line']) {
476476
message =
477-
`${message} (position ${expression['line']}:${expression['character']} in the original .ts file)`;
477+
`${message} (position ${expression['line']+1}:${expression['character']+1} in the original .ts file)`;
478478
}
479479
throw new Error(message);
480480
}

‎modules/@angular/compiler-cli/test/reflector_host_spec.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,17 @@ describe('reflector_host', () => {
8282
.toBeDefined();
8383
});
8484

85+
8586
it('should be produce the same symbol if asked twice', () => {
8687
let foo1 = reflectorHost.getStaticSymbol('main.ts', 'foo');
8788
let foo2 = reflectorHost.getStaticSymbol('main.ts', 'foo');
8889
expect(foo1).toBe(foo2);
8990
});
9091

92+
it('should be able to produce a symbol for a module with no file', () => {
93+
expect(reflectorHost.getStaticSymbol('angularjs', 'SomeAngularSymbol')).toBeDefined();
94+
});
95+
9196
it('should be able to read a metadata file',
9297
() => {
9398
expect(reflectorHost.getMetadataFor('node_modules/@angular/core.d.ts'))
@@ -96,6 +101,10 @@ describe('reflector_host', () => {
96101
it('should be able to read metadata from an otherwise unused .d.ts file ', () => {
97102
expect(reflectorHost.getMetadataFor('node_modules/@angular/unused.d.ts')).toBeUndefined();
98103
});
104+
105+
it('should return undefined for missing modules', () => {
106+
expect(reflectorHost.getMetadataFor('node_modules/@angular/missing.d.ts')).toBeUndefined();
107+
});
99108
});
100109

101110
const dummyModule = 'export let foo: any[];'

‎modules/@angular/compiler-cli/test/static_reflector_spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ describe('StaticReflector', () => {
112112
let SomeClass = host.findDeclaration('src/error-reporting', 'SomeClass');
113113
expect(() => reflector.annotations(SomeClass))
114114
.toThrow(new Error(
115-
'Error encountered resolving symbol values statically. A reasonable error message (position 12:33 in the original .ts file), resolving symbol ErrorSym in /tmp/src/error-references.d.ts, resolving symbol Link2 in /tmp/src/error-references.d.ts, resolving symbol Link1 in /tmp/src/error-references.d.ts, resolving symbol SomeClass in /tmp/src/error-reporting.d.ts, resolving symbol SomeClass in /tmp/src/error-reporting.d.ts'));
115+
'Error encountered resolving symbol values statically. A reasonable error message (position 13:34 in the original .ts file), resolving symbol ErrorSym in /tmp/src/error-references.d.ts, resolving symbol Link2 in /tmp/src/error-references.d.ts, resolving symbol Link1 in /tmp/src/error-references.d.ts, resolving symbol SomeClass in /tmp/src/error-reporting.d.ts, resolving symbol SomeClass in /tmp/src/error-reporting.d.ts'));
116116
});
117117

118118
it('should simplify primitive into itself', () => {

‎tools/@angular/tsc-wrapped/src/collector.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import * as ts from 'typescript';
22

33
import {Evaluator, errorSymbol, isPrimitive} from './evaluator';
4-
import {ClassMetadata, ConstructorMetadata, MemberMetadata, MetadataError, MetadataMap, MetadataSymbolicExpression, MetadataSymbolicReferenceExpression, MetadataValue, MethodMetadata, ModuleMetadata, VERSION, isMetadataError, isMetadataSymbolicReferenceExpression} from './schema';
4+
import {ClassMetadata, ConstructorMetadata, MemberMetadata, MetadataError, MetadataMap, MetadataSymbolicExpression, MetadataSymbolicReferenceExpression, MetadataSymbolicSelectExpression, MetadataValue, MethodMetadata, ModuleMetadata, VERSION, isMetadataError, isMetadataSymbolicReferenceExpression, isMetadataSymbolicSelectExpression} from './schema';
55
import {Symbols} from './symbols';
66

77

8+
89
/**
910
* Collect decorator metadata from a TypeScript module.
1011
*/
@@ -38,9 +39,11 @@ export class MetadataCollector {
3839
return undefined;
3940
}
4041

41-
function referenceFrom(node: ts.Node): MetadataSymbolicReferenceExpression|MetadataError {
42+
function referenceFrom(node: ts.Node): MetadataSymbolicReferenceExpression|MetadataError|
43+
MetadataSymbolicSelectExpression {
4244
const result = evaluator.evaluateNode(node);
43-
if (isMetadataError(result) || isMetadataSymbolicReferenceExpression(result)) {
45+
if (isMetadataError(result) || isMetadataSymbolicReferenceExpression(result) ||
46+
isMetadataSymbolicSelectExpression(result)) {
4447
return result;
4548
} else {
4649
return errorSym('Symbol reference expected', node);
@@ -70,8 +73,9 @@ export class MetadataCollector {
7073
const methodDecorators = getDecorators(method.decorators);
7174
const parameters = method.parameters;
7275
const parameterDecoratorData: (MetadataSymbolicExpression | MetadataError)[][] = [];
73-
const parametersData: (MetadataSymbolicReferenceExpression | MetadataError | null)[] =
74-
[];
76+
const parametersData:
77+
(MetadataSymbolicReferenceExpression | MetadataError |
78+
MetadataSymbolicSelectExpression | null)[] = [];
7579
let hasDecoratorData: boolean = false;
7680
let hasParameterData: boolean = false;
7781
for (const parameter of parameters) {

‎tools/@angular/tsc-wrapped/src/evaluator.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -354,8 +354,8 @@ export class Evaluator {
354354
case ts.SyntaxKind.TypeReference:
355355
const typeReferenceNode = <ts.TypeReferenceNode>node;
356356
const typeNameNode = typeReferenceNode.typeName;
357-
const getReference: (typeNameNode: ts.Identifier | ts.QualifiedName) =>
358-
MetadataSymbolicReferenceExpression | MetadataError = node => {
357+
const getReference: (typeNameNode: ts.Identifier | ts.QualifiedName) => MetadataValue =
358+
node => {
359359
if (typeNameNode.kind === ts.SyntaxKind.QualifiedName) {
360360
const qualifiedName = <ts.QualifiedName>node;
361361
const left = this.evaluateNode(qualifiedName.left);
@@ -364,7 +364,8 @@ export class Evaluator {
364364
__symbolic: 'reference', module: left.module, name: qualifiedName.right.text
365365
}
366366
}
367-
return errorSymbol('Qualified type names not supported', node);
367+
// Record a type reference to a declared type as a select.
368+
return {__symbolic: 'select', expression: left, member: qualifiedName.right.text};
368369
} else {
369370
const identifier = <ts.Identifier>typeNameNode;
370371
let symbol = this.symbols.resolve(identifier.text);

‎tools/@angular/tsc-wrapped/test/evaluator.spec.ts‎

Lines changed: 38 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ describe('Evaluator', () => {
1818
beforeEach(() => {
1919
host = new Host(FILES, [
2020
'expressions.ts', 'consts.ts', 'const_expr.ts', 'forwardRef.ts', 'classes.ts',
21-
'newExpression.ts', 'errors.ts'
21+
'newExpression.ts', 'errors.ts', 'declared.ts'
2222
]);
2323
service = ts.createLanguageService(host, documentRegistry);
2424
program = service.getProgram();
@@ -39,15 +39,15 @@ describe('Evaluator', () => {
3939
});
4040

4141
it('should be able to fold literal expressions', () => {
42-
var consts = program.getSourceFile('consts.ts');
42+
const consts = program.getSourceFile('consts.ts');
4343
expect(evaluator.isFoldable(findVar(consts, 'someName').initializer)).toBeTruthy();
4444
expect(evaluator.isFoldable(findVar(consts, 'someBool').initializer)).toBeTruthy();
4545
expect(evaluator.isFoldable(findVar(consts, 'one').initializer)).toBeTruthy();
4646
expect(evaluator.isFoldable(findVar(consts, 'two').initializer)).toBeTruthy();
4747
});
4848

4949
it('should be able to fold expressions with foldable references', () => {
50-
var expressions = program.getSourceFile('expressions.ts');
50+
const expressions = program.getSourceFile('expressions.ts');
5151
symbols.define('someName', 'some-name');
5252
symbols.define('someBool', true);
5353
symbols.define('one', 1);
@@ -61,15 +61,15 @@ describe('Evaluator', () => {
6161
});
6262

6363
it('should be able to evaluate literal expressions', () => {
64-
var consts = program.getSourceFile('consts.ts');
64+
const consts = program.getSourceFile('consts.ts');
6565
expect(evaluator.evaluateNode(findVar(consts, 'someName').initializer)).toBe('some-name');
6666
expect(evaluator.evaluateNode(findVar(consts, 'someBool').initializer)).toBe(true);
6767
expect(evaluator.evaluateNode(findVar(consts, 'one').initializer)).toBe(1);
6868
expect(evaluator.evaluateNode(findVar(consts, 'two').initializer)).toBe(2);
6969
});
7070

7171
it('should be able to evaluate expressions', () => {
72-
var expressions = program.getSourceFile('expressions.ts');
72+
const expressions = program.getSourceFile('expressions.ts');
7373
symbols.define('someName', 'some-name');
7474
symbols.define('someBool', true);
7575
symbols.define('one', 1);
@@ -117,29 +117,29 @@ describe('Evaluator', () => {
117117
});
118118

119119
it('should report recursive references as symbolic', () => {
120-
var expressions = program.getSourceFile('expressions.ts');
120+
const expressions = program.getSourceFile('expressions.ts');
121121
expect(evaluator.evaluateNode(findVar(expressions, 'recursiveA').initializer))
122122
.toEqual({__symbolic: 'reference', name: 'recursiveB'});
123123
expect(evaluator.evaluateNode(findVar(expressions, 'recursiveB').initializer))
124124
.toEqual({__symbolic: 'reference', name: 'recursiveA'});
125125
});
126126

127127
it('should correctly handle special cases for CONST_EXPR', () => {
128-
var const_expr = program.getSourceFile('const_expr.ts');
128+
const const_expr = program.getSourceFile('const_expr.ts');
129129
expect(evaluator.evaluateNode(findVar(const_expr, 'bTrue').initializer)).toEqual(true);
130130
expect(evaluator.evaluateNode(findVar(const_expr, 'bFalse').initializer)).toEqual(false);
131131
});
132132

133133
it('should resolve a forwardRef', () => {
134-
var forwardRef = program.getSourceFile('forwardRef.ts');
134+
const forwardRef = program.getSourceFile('forwardRef.ts');
135135
expect(evaluator.evaluateNode(findVar(forwardRef, 'bTrue').initializer)).toEqual(true);
136136
expect(evaluator.evaluateNode(findVar(forwardRef, 'bFalse').initializer)).toEqual(false);
137137
});
138138

139139
it('should return new expressions', () => {
140140
symbols.define('Value', {__symbolic: 'reference', module: './classes', name: 'Value'});
141141
evaluator = new Evaluator(symbols);
142-
var newExpression = program.getSourceFile('newExpression.ts');
142+
const newExpression = program.getSourceFile('newExpression.ts');
143143
expect(evaluator.evaluateNode(findVar(newExpression, 'someValue').initializer)).toEqual({
144144
__symbolic: 'new',
145145
expression: {__symbolic: 'reference', name: 'Value', module: './classes'},
@@ -152,54 +152,57 @@ describe('Evaluator', () => {
152152
});
153153
});
154154

155-
it('should return errors for unsupported expressions', () => {
156-
let errors = program.getSourceFile('errors.ts');
157-
let aDecl = findVar(errors, 'a');
155+
it('should support referene to a declared module type', () => {
156+
const declared = program.getSourceFile('declared.ts');
157+
const aDecl = findVar(declared, 'a');
158158
expect(evaluator.evaluateNode(aDecl.type)).toEqual({
159-
__symbolic: 'error',
160-
message: 'Qualified type names not supported',
161-
line: 5,
162-
character: 10
159+
__symbolic: 'select',
160+
expression: {__symbolic: 'reference', name: 'Foo'},
161+
member: 'A'
163162
});
164-
let fDecl = findVar(errors, 'f');
163+
});
164+
165+
it('should return errors for unsupported expressions', () => {
166+
const errors = program.getSourceFile('errors.ts');
167+
const fDecl = findVar(errors, 'f');
165168
expect(evaluator.evaluateNode(fDecl.initializer))
166169
.toEqual(
167-
{__symbolic: 'error', message: 'Function call not supported', line: 6, character: 11});
168-
let eDecl = findVar(errors, 'e');
170+
{__symbolic: 'error', message: 'Function call not supported', line: 1, character: 11});
171+
const eDecl = findVar(errors, 'e');
169172
expect(evaluator.evaluateNode(eDecl.type)).toEqual({
170173
__symbolic: 'error',
171174
message: 'Could not resolve type',
172-
line: 7,
175+
line: 2,
173176
character: 10,
174177
context: {typeName: 'NotFound'}
175178
});
176-
let sDecl = findVar(errors, 's');
179+
const sDecl = findVar(errors, 's');
177180
expect(evaluator.evaluateNode(sDecl.initializer)).toEqual({
178181
__symbolic: 'error',
179182
message: 'Name expected',
180-
line: 8,
183+
line: 3,
181184
character: 13,
182185
context: {received: '1'}
183186
});
184-
let tDecl = findVar(errors, 't');
187+
const tDecl = findVar(errors, 't');
185188
expect(evaluator.evaluateNode(tDecl.initializer)).toEqual({
186189
__symbolic: 'error',
187190
message: 'Expression form not supported',
188-
line: 9,
191+
line: 4,
189192
character: 11
190193
});
191194
});
192195

193196
it('should be able to fold an array spread', () => {
194-
let expressions = program.getSourceFile('expressions.ts');
197+
const expressions = program.getSourceFile('expressions.ts');
195198
symbols.define('arr', [1, 2, 3, 4]);
196-
let arrSpread = findVar(expressions, 'arrSpread');
199+
const arrSpread = findVar(expressions, 'arrSpread');
197200
expect(evaluator.evaluateNode(arrSpread.initializer)).toEqual([0, 1, 2, 3, 4, 5]);
198201
});
199202

200203
it('should be able to produce a spread expression', () => {
201-
let expressions = program.getSourceFile('expressions.ts');
202-
let arrSpreadRef = findVar(expressions, 'arrSpreadRef');
204+
const expressions = program.getSourceFile('expressions.ts');
205+
const arrSpreadRef = findVar(expressions, 'arrSpreadRef');
203206
expect(evaluator.evaluateNode(arrSpreadRef.initializer)).toEqual([
204207
0, {__symbolic: 'spread', expression: {__symbolic: 'reference', name: 'arrImport'}}, 5
205208
]);
@@ -296,14 +299,16 @@ const FILES: Directory = {
296299
export const complex = CONST_EXPR(new Value("name", forwardRef(() => 12)));
297300
`,
298301
'errors.ts': `
299-
declare namespace Foo {
300-
type A = string;
301-
}
302-
303-
let a: Foo.A = 'some value';
304302
let f = () => 1;
305303
let e: NotFound;
306304
let s = { 1: 1, 2: 2 };
307305
let t = typeof 12;
308306
`,
307+
'declared.ts': `
308+
declare namespace Foo {
309+
type A = string;
310+
}
311+
312+
let a: Foo.A = 'some value';
313+
`
309314
};

0 commit comments

Comments
 (0)