fix(ivy): references track the identifier they were discovered under (#24862)

Previously, references had the concept of an identifier, but would not
properly detect whether the identifier should be used or not when
generating an expression. This change fixes that logic.

Additionally, now whenever an identifier resolves to a reference (even
one imported from another module) as part of resolving an expression,
the reference is updated to use that identifier. This ensures that for
a class Foo declared in foo.ts, but referenced in an expression in
bar.ts, the Reference returned includes the identifier from bar.ts,
meaning that writing an expression in bar.ts for the Reference will not
generate an import.

PR Close #24862
This commit is contained in:
Alex Rickabaugh 2018-07-12 15:04:07 -07:00 committed by Victor Berchet
parent 6f8ec256ef
commit 139f5b3672
2 changed files with 37 additions and 15 deletions

View File

@ -100,6 +100,8 @@ export abstract class Reference {
* import if needed. * import if needed.
*/ */
abstract toExpression(context: ts.SourceFile): Expression|null; abstract toExpression(context: ts.SourceFile): Expression|null;
abstract withIdentifier(identifier: ts.Identifier): Reference;
} }
/** /**
@ -112,6 +114,8 @@ export class NodeReference extends Reference {
constructor(node: ts.Node, readonly moduleName: string|null) { super(node); } constructor(node: ts.Node, readonly moduleName: string|null) { super(node); }
toExpression(context: ts.SourceFile): null { return null; } toExpression(context: ts.SourceFile): null { return null; }
withIdentifier(identifier: ts.Identifier): NodeReference { return this; }
} }
/** /**
@ -125,7 +129,7 @@ export class ResolvedReference extends Reference {
readonly expressable = true; readonly expressable = true;
toExpression(context: ts.SourceFile): Expression { toExpression(context: ts.SourceFile): Expression {
if (ts.getOriginalNode(context) === ts.getOriginalNode(this.node).getSourceFile()) { if (ts.getOriginalNode(context) === ts.getOriginalNode(this.identifier).getSourceFile()) {
return new WrappedNodeExpr(this.identifier); return new WrappedNodeExpr(this.identifier);
} else { } else {
// Relative import from context -> this.node.getSourceFile(). // Relative import from context -> this.node.getSourceFile().
@ -150,6 +154,10 @@ export class ResolvedReference extends Reference {
} }
} }
} }
withIdentifier(identifier: ts.Identifier): ResolvedReference {
return new ResolvedReference(this.node, identifier);
}
} }
/** /**
@ -168,12 +176,16 @@ export class AbsoluteReference extends Reference {
readonly expressable = true; readonly expressable = true;
toExpression(context: ts.SourceFile): Expression { toExpression(context: ts.SourceFile): Expression {
if (ts.getOriginalNode(context) === ts.getOriginalNode(this.node.getSourceFile())) { if (ts.getOriginalNode(context) === ts.getOriginalNode(this.identifier).getSourceFile()) {
return new WrappedNodeExpr(this.identifier); return new WrappedNodeExpr(this.identifier);
} else { } else {
return new ExternalExpr(new ExternalReference(this.moduleName, this.symbolName)); return new ExternalExpr(new ExternalReference(this.moduleName, this.symbolName));
} }
} }
withIdentifier(identifier: ts.Identifier): AbsoluteReference {
return new AbsoluteReference(this.node, identifier, this.moduleName, this.symbolName);
}
} }
/** /**
@ -379,8 +391,13 @@ class StaticInterpreter {
if (decl === null) { if (decl === null) {
return DYNAMIC_VALUE; return DYNAMIC_VALUE;
} }
return this.visitDeclaration( const result = this.visitDeclaration(
decl.node, {...context, absoluteModuleName: decl.viaModule || context.absoluteModuleName}); decl.node, {...context, absoluteModuleName: decl.viaModule || context.absoluteModuleName});
if (result instanceof Reference) {
return result.withIdentifier(node);
} else {
return result;
}
} }
private visitDeclaration(node: ts.Declaration, context: Context): ResolvedValue { private visitDeclaration(node: ts.Declaration, context: Context): ResolvedValue {

View File

@ -6,12 +6,12 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {ExternalExpr} from '@angular/compiler'; import {WrappedNodeExpr} from '@angular/compiler';
import * as ts from 'typescript'; import * as ts from 'typescript';
import {TypeScriptReflectionHost} from '..'; import {TypeScriptReflectionHost} from '..';
import {getDeclaration, makeProgram} from '../../testing/in_memory_typescript'; import {getDeclaration, makeProgram} from '../../testing/in_memory_typescript';
import {Reference, ResolvedValue, staticallyResolve} from '../src/resolver'; import {AbsoluteReference, Reference, ResolvedValue, staticallyResolve} from '../src/resolver';
function makeSimpleProgram(contents: string): ts.Program { function makeSimpleProgram(contents: string): ts.Program {
return makeProgram([{name: 'entry.ts', contents}]).program; return makeProgram([{name: 'entry.ts', contents}]).program;
@ -145,11 +145,13 @@ describe('ngtsc metadata', () => {
expect(ts.isFunctionDeclaration(resolved.node)).toBe(true); expect(ts.isFunctionDeclaration(resolved.node)).toBe(true);
expect(resolved.expressable).toBe(true); expect(resolved.expressable).toBe(true);
const reference = resolved.toExpression(program.getSourceFile('entry.ts') !); const reference = resolved.toExpression(program.getSourceFile('entry.ts') !);
if (!(reference instanceof ExternalExpr)) { if (!(reference instanceof WrappedNodeExpr)) {
return fail('Expected expression reference to be an external (import) expression'); return fail('Expected expression reference to be a wrapped node');
} }
expect(reference.value.moduleName).toBe('./second'); if (!ts.isIdentifier(reference.node)) {
expect(reference.value.name).toBe('foo'); return fail('Expected expression to be an Identifier');
}
expect(reference.node.getSourceFile()).toEqual(program.getSourceFile('entry.ts') !);
}); });
it('absolute imports work', () => { it('absolute imports work', () => {
@ -168,17 +170,20 @@ describe('ngtsc metadata', () => {
const result = getDeclaration(program, 'entry.ts', 'target$', ts.isVariableDeclaration); const result = getDeclaration(program, 'entry.ts', 'target$', ts.isVariableDeclaration);
const expr = result.initializer !; const expr = result.initializer !;
const resolved = staticallyResolve(expr, host, checker); const resolved = staticallyResolve(expr, host, checker);
if (!(resolved instanceof Reference)) { if (!(resolved instanceof AbsoluteReference)) {
return fail('Expected expression to resolve to a reference'); return fail('Expected expression to resolve to an absolute reference');
} }
expect(resolved.moduleName).toBe('some_library');
expect(ts.isFunctionDeclaration(resolved.node)).toBe(true); expect(ts.isFunctionDeclaration(resolved.node)).toBe(true);
expect(resolved.expressable).toBe(true); expect(resolved.expressable).toBe(true);
const reference = resolved.toExpression(program.getSourceFile('entry.ts') !); const reference = resolved.toExpression(program.getSourceFile('entry.ts') !);
if (!(reference instanceof ExternalExpr)) { if (!(reference instanceof WrappedNodeExpr)) {
return fail('Expected expression reference to be an external (import) expression'); return fail('Expected expression reference to be a wrapped node');
} }
expect(reference.value.moduleName).toBe('some_library'); if (!ts.isIdentifier(reference.node)) {
expect(reference.value.name).toBe('foo'); return fail('Expected expression to be an Identifier');
}
expect(reference.node.getSourceFile()).toEqual(program.getSourceFile('entry.ts') !);
}); });
it('reads values from default exports', () => { it('reads values from default exports', () => {