honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ivy): handle namespaced imports correctly (#31367) 

The ngcc tool adds namespaced imports to files when compiling. The ngtsc
tooling was not processing types correctly when they were imported via
such namespaces. For example:

```
export declare class SomeModule {
    static withOptions(...): ModuleWithProviders<ɵngcc1.BaseModule>;
```

In this case the `BaseModule` was being incorrectly attributed to coming
from the current module rather than the imported module, represented by
`ɵngcc1`.

Fixes #31342

PR Close #31367
This commit is contained in:
Pete Bacon Darwin 2019-07-01 14:05:55 +01:00 committed by Jason Aden
parent 36d3062a42
commit 98a68ad3e7
5 changed files with 228 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts
View File

@ -12,7 +12,7 @@ import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ng
import {TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
import {ClassMemberKind, Import, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection';
import {getDeclaration} from '../../../src/ngtsc/testing';
import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
import {loadFakeCore, loadTestFiles, loadTsLib} from '../../../test/helpers';
import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
import {MockLogger} from '../helpers/mock_logger';
import {convertToDirectTsLibImport, makeTestBundleProgram} from '../helpers/utils';
@ -122,6 +122,7 @@ runInEachFileSystem(() => {
describe(`[${label}]`, () => {
beforeEach(() => {
const fs = getFileSystem();
loadTsLib(fs);
loadFakeCore(fs);
loadTestFiles(FILES[label]);
});

3
packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts
View File

@ -11,7 +11,7 @@ import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ng
import {TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
import {ClassMemberKind, Import, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection';
import {getDeclaration} from '../../../src/ngtsc/testing';
import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
import {loadFakeCore, loadTestFiles, loadTsLib} from '../../../test/helpers';
import {Esm5ReflectionHost} from '../../src/host/esm5_host';
import {MockLogger} from '../helpers/mock_logger';
import {convertToDirectTsLibImport, makeTestBundleProgram} from '../helpers/utils';
@ -143,6 +143,7 @@ export { SomeDirective };
describe(`[${label}]`, () => {
beforeEach(() => {
const fs = getFileSystem();
loadTsLib(fs);
loadFakeCore(fs);
loadTestFiles(FILES[label]);
});

38
packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts
View File

@ -97,7 +97,7 @@ export class TypeScriptReflectionHost implements ReflectionHost {
}
this.checker.getExportsOfModule(symbol).forEach(exportSymbol => {
// Map each exported Symbol to a Declaration and add it to the map.
const decl = this.getDeclarationOfSymbol(exportSymbol);
const decl = this.getDeclarationOfSymbol(exportSymbol, null);
if (decl !== null) {
map.set(exportSymbol.name, decl);
}
@ -122,7 +122,7 @@ export class TypeScriptReflectionHost implements ReflectionHost {
if (symbol === undefined) {
return null;
}
return this.getDeclarationOfSymbol(symbol);
return this.getDeclarationOfSymbol(symbol, id);
}
getDefinitionOfFunction(node: ts.Node): FunctionDefinition|null {
@ -244,7 +244,8 @@ export class TypeScriptReflectionHost implements ReflectionHost {
*
* @internal
*/
protected getDeclarationOfSymbol(symbol: ts.Symbol): Declaration|null {
private getDeclarationOfSymbol(symbol: ts.Symbol, originalId: ts.Identifier|null): Declaration
|null {
// If the symbol points to a ShorthandPropertyAssignment, resolve it.
if (symbol.valueDeclaration !== undefined &&
ts.isShorthandPropertyAssignment(symbol.valueDeclaration)) {
@ -253,32 +254,15 @@ export class TypeScriptReflectionHost implements ReflectionHost {
if (shorthandSymbol === undefined) {
return null;
}
return this.getDeclarationOfSymbol(shorthandSymbol);
}
let viaModule: string|null = null;
// Look through the Symbol's immediate declarations, and see if any of them are import-type
// statements.
if (symbol.declarations !== undefined && symbol.declarations.length > 0) {
for (let i = 0; i < symbol.declarations.length; i++) {
const decl = symbol.declarations[i];
if (ts.isImportSpecifier(decl) && decl.parent !== undefined &&
decl.parent.parent !== undefined && decl.parent.parent.parent !== undefined) {
// Find the ImportDeclaration that imported this Symbol.
const importDecl = decl.parent.parent.parent;
// The moduleSpecifier should always be a string.
if (ts.isStringLiteral(importDecl.moduleSpecifier)) {
// Check if the moduleSpecifier is absolute. If it is, this symbol comes from an
// external module, and the import path becomes the viaModule.
const moduleSpecifier = importDecl.moduleSpecifier.text;
if (!moduleSpecifier.startsWith('.')) {
viaModule = moduleSpecifier;
break;
}
}
}
}
return this.getDeclarationOfSymbol(shorthandSymbol, originalId);
}
const importInfo = originalId && this.getImportOfIdentifier(originalId);
const viaModule =
importInfo !== null && importInfo.from !== null && !importInfo.from.startsWith('.') ?
importInfo.from :
null;
// Now, resolve the Symbol to its declaration by following any and all aliases.
while (symbol.flags & ts.SymbolFlags.Alias) {
symbol = this.checker.getAliasedSymbol(symbol);

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

@ -19,7 +19,7 @@ runInEachFileSystem(() => {
beforeEach(() => _ = absoluteFrom);
describe('ctor params', () => {
describe('getConstructorParameters()', () => {
it('should reflect a single argument', () => {
const {program} = makeProgram([{
name: _('/entry.ts'),
@ -213,6 +213,64 @@ runInEachFileSystem(() => {
});
});
describe('getImportOfIdentifier()', () => {
it('should resolve a direct import', () => {
const {program} = makeProgram([
{name: _('/node_modules/absolute/index.ts'), contents: 'export class Target {}'},
{
name: _('/entry.ts'),
contents: `
import {Target} from 'absolute';
let foo: Target;
`
},
]);
const checker = program.getTypeChecker();
const host = new TypeScriptReflectionHost(checker);
const foo = getDeclaration(program, _('/entry.ts'), 'foo', ts.isVariableDeclaration);
if (foo.type === undefined || !ts.isTypeReferenceNode(foo.type) ||
!ts.isIdentifier(foo.type.typeName)) {
return fail('Unexpected type for foo');
}
const Target = foo.type.typeName;
const directImport = host.getImportOfIdentifier(Target);
expect(directImport).toEqual({
name: 'Target',
from: 'absolute',
});
});
it('should resolve a namespaced import', () => {
const {program} = makeProgram([
{name: _('/node_modules/absolute/index.ts'), contents: 'export class Target {}'},
{
name: _('/entry.ts'),
contents: `
import * as abs from 'absolute';
let foo: abs.Target;
`
},
]);
const checker = program.getTypeChecker();
const host = new TypeScriptReflectionHost(checker);
const foo = getDeclaration(program, _('/entry.ts'), 'foo', ts.isVariableDeclaration);
if (foo.type === undefined || !ts.isTypeReferenceNode(foo.type) ||
!ts.isQualifiedName(foo.type.typeName)) {
return fail('Unexpected type for foo');
}
const Target = foo.type.typeName.right;
const namespacedImport = host.getImportOfIdentifier(Target);
expect(namespacedImport).toEqual({
name: 'Target',
from: 'absolute',
});
});
});
describe('getDeclarationOfIdentifier()', () => {
it('should reflect a re-export', () => {
const {program} = makeProgram([
{name: _('/node_modules/absolute/index.ts'), contents: 'export class Target {}'},
@ -254,6 +312,117 @@ runInEachFileSystem(() => {
expect(directTargetDecl.viaModule).toBe('absolute');
expect(directTargetDecl.node).toBe(targetDecl.node);
});
it('should resolve a direct import', () => {
const {program} = makeProgram([
{name: _('/node_modules/absolute/index.ts'), contents: 'export class Target {}'},
{
name: _('/entry.ts'),
contents: `
import {Target} from 'absolute';
let foo: Target;
`
},
]);
const checker = program.getTypeChecker();
const host = new TypeScriptReflectionHost(checker);
const targetDecl = getDeclaration(
program, _('/node_modules/absolute/index.ts'), 'Target', ts.isClassDeclaration);
const foo = getDeclaration(program, _('/entry.ts'), 'foo', ts.isVariableDeclaration);
if (foo.type === undefined || !ts.isTypeReferenceNode(foo.type) ||
!ts.isIdentifier(foo.type.typeName)) {
return fail('Unexpected type for foo');
}
const Target = foo.type.typeName;
const decl = host.getDeclarationOfIdentifier(Target);
expect(decl).toEqual({
node: targetDecl,
viaModule: 'absolute',
});
});
it('should resolve a namespaced import', () => {
const {program} = makeProgram([
{name: _('/node_modules/absolute/index.ts'), contents: 'export class Target {}'},
{
name: _('/entry.ts'),
contents: `
import * as abs from 'absolute';
let foo: abs.Target;
`
},
]);
const checker = program.getTypeChecker();
const host = new TypeScriptReflectionHost(checker);
const targetDecl = getDeclaration(
program, _('/node_modules/absolute/index.ts'), 'Target', ts.isClassDeclaration);
const foo = getDeclaration(program, _('/entry.ts'), 'foo', ts.isVariableDeclaration);
if (foo.type === undefined || !ts.isTypeReferenceNode(foo.type) ||
!ts.isQualifiedName(foo.type.typeName)) {
return fail('Unexpected type for foo');
}
const Target = foo.type.typeName.right;
const decl = host.getDeclarationOfIdentifier(Target);
expect(decl).toEqual({
node: targetDecl,
viaModule: 'absolute',
});
});
});
describe('getExportsOfModule()', () => {
it('should handle simple exports', () => {
const {program} = makeProgram([
{
name: _('/entry.ts'),
contents: `
export const x = 10;
export function foo() {}
export type T = string;
export interface I {}
export enum E {}
`
},
]);
const checker = program.getTypeChecker();
const host = new TypeScriptReflectionHost(checker);
const exportedDeclarations =
host.getExportsOfModule(program.getSourceFile(_('/entry.ts')) !);
expect(Array.from(exportedDeclarations !.keys())).toEqual(['foo', 'x', 'T', 'I', 'E']);
expect(Array.from(exportedDeclarations !.values()).map(v => v.viaModule)).toEqual([
null, null, null, null, null
]);
});
it('should handle re-exports', () => {
const {program} = makeProgram([
{name: _('/node_modules/absolute/index.ts'), contents: 'export class Target {}'},
{name: _('/local1.ts'), contents: `export {Target as AliasTarget} from 'absolute';`},
{name: _('/local2.ts'), contents: `export {AliasTarget as Target} from './local1';`},
{
name: _('/entry.ts'),
contents: `
export {Target as Target1} from 'absolute';
export {AliasTarget} from './local1';
export {Target as AliasTarget2} from './local2';
export * from 'absolute';
`
},
]);
const checker = program.getTypeChecker();
const host = new TypeScriptReflectionHost(checker);
const exportedDeclarations =
host.getExportsOfModule(program.getSourceFile(_('/entry.ts')) !);
expect(Array.from(exportedDeclarations !.keys())).toEqual([
'Target1', 'AliasTarget', 'AliasTarget2', 'Target'
]);
expect(Array.from(exportedDeclarations !.values()).map(v => v.viaModule)).toEqual([
null, null, null, null
]);
});
});
});
function expectParameter(

9
packages/compiler-cli/test/helpers/src/mock_file_loading.ts
View File

@ -31,9 +31,7 @@ export function loadStandardTestFiles(
tmpFs, resolveNpmTreeArtifact('typescript'),
tmpFs.resolve(basePath, 'node_modules/typescript'));
loadTestDirectory(
tmpFs, resolveNpmTreeArtifact('tslib'), tmpFs.resolve(basePath, 'node_modules/tslib'));
loadTsLib(tmpFs, basePath);
if (fakeCore) {
loadFakeCore(tmpFs, basePath);
@ -51,6 +49,11 @@ export function loadStandardTestFiles(
return tmpFs.dump();
}
export function loadTsLib(fs: FileSystem, basePath: string = '/') {
loadTestDirectory(
fs, resolveNpmTreeArtifact('tslib'), fs.resolve(basePath, 'node_modules/tslib'));
}
export function loadFakeCore(fs: FileSystem, basePath: string = '/') {
loadTestDirectory(
fs, resolveNpmTreeArtifact('angular/packages/compiler-cli/test/ngtsc/fake_core/npm_package'),