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

fix(ivy): don't run decorator handlers against declaration files (#34557) 

Currently the decorator handlers are run against all `SourceFile`s in the compilation, but we shouldn't be doing it against declaration files. This initially came up as a CI issue in #33264 where it was worked around only for the `DirectiveDecoratorHandler`. These changes move the logic into the `TraitCompiler` and `DecorationAnalyzer` so that it applies to all of the handlers.

PR Close #34557
This commit is contained in:
crisbeto 2019-12-27 08:18:21 +02:00 committed by atscott
parent 6d28a209e4
commit 6d534f10e6
8 changed files with 126 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts
View File

@ -137,6 +137,7 @@ export class DecorationAnalyzer {
analyzeProgram(): DecorationAnalyses {
const decorationAnalyses = new DecorationAnalyses();
const analyzedFiles = this.program.getSourceFiles()
.filter(sourceFile => !sourceFile.isDeclarationFile)
.filter(sourceFile => isWithinPackage(this.packagePath, sourceFile))
.map(sourceFile => this.analyzeFile(sourceFile))
.filter(isDefined);

25
packages/compiler-cli/ngcc/test/analysis/decoration_analyzer_spec.ts
View File

@ -11,7 +11,7 @@ import {FatalDiagnosticError, makeDiagnostic} from '../../../src/ngtsc/diagnosti
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system';
import {TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
import {ClassDeclaration, Decorator} from '../../../src/ngtsc/reflection';
import {DecoratorHandler, DetectResult} from '../../../src/ngtsc/transform';
import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../../src/ngtsc/transform';
import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
import {DecorationAnalyzer} from '../../src/analysis/decoration_analyzer';
import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registry';
@ -392,6 +392,29 @@ runInEachFileSystem(() => {
expect(diagnosticLogs[1]).toEqual(jasmine.objectContaining({code: -999998}));
});
});
describe('declaration files', () => {
it('should not run decorator handlers against declaration files', () => {
class FakeDecoratorHandler implements DecoratorHandler<{}|null, unknown, unknown> {
name = 'FakeDecoratorHandler';
precedence = HandlerPrecedence.PRIMARY;
detect(): undefined { throw new Error('detect should not have been called'); }
analyze(): AnalysisOutput<unknown> {
throw new Error('analyze should not have been called');
}
compile(): CompileResult { throw new Error('compile should not have been called'); }
}
const analyzer = setUpAnalyzer([{
name: _('/node_modules/test-package/index.d.ts'),
contents: 'export declare class SomeDirective {}',
}]);
analyzer.handlers = [new FakeDecoratorHandler()];
result = analyzer.analyzeProgram();
expect(result.size).toBe(0);
});
});
});
});
});

4
packages/compiler-cli/src/ngtsc/annotations/src/directive.ts
View File

@ -55,10 +55,6 @@ export class DirectiveDecoratorHandler implements
detect(node: ClassDeclaration, decorators: Decorator[]|null):
DetectResult<Decorator|null>|undefined {
// Compiling declaration files is invalid.
if (node.getSourceFile().isDeclarationFile) {
return undefined;
}
// If the class is undecorated, check if any of the fields have Angular decorators or lifecycle
// hooks, and if they do, label the class as an abstract directive.
if (!decorators) {

1
packages/compiler-cli/src/ngtsc/incremental/index.ts
View File

@ -6,4 +6,5 @@
* found in the LICENSE file at https://angular.io/license
*/
export {NOOP_INCREMENTAL_BUILD} from './src/noop';
export {IncrementalDriver} from './src/state';

13
packages/compiler-cli/src/ngtsc/incremental/src/noop.ts Normal file
View File

@ -0,0 +1,13 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {IncrementalBuild} from '../api';
export const NOOP_INCREMENTAL_BUILD: IncrementalBuild<any> = {
priorWorkFor: () => null
};

5
packages/compiler-cli/src/ngtsc/transform/src/compilation.ts
View File

@ -103,6 +103,11 @@ export class TraitCompiler {
private analyze(sf: ts.SourceFile, preanalyze: false): void;
private analyze(sf: ts.SourceFile, preanalyze: true): Promise<void>|undefined;
private analyze(sf: ts.SourceFile, preanalyze: boolean): Promise<void>|undefined {
// We shouldn't analyze declaration files.
if (sf.isDeclarationFile) {
return undefined;
}
// analyze() really wants to return `Promise<void>|void`, but TypeScript cannot narrow a return
// type of 'void', so `undefined` is used instead.
const promises: Promise<void>[] = [];

32
packages/compiler-cli/src/ngtsc/transform/test/BUILD.bazel Normal file
View File

@ -0,0 +1,32 @@
load("//tools:defaults.bzl", "jasmine_node_test", "ts_library")
package(default_visibility = ["//visibility:public"])
ts_library(
name = "test_lib",
testonly = True,
srcs = glob([
"**/*.ts",
]),
deps = [
"//packages:types",
"//packages/compiler",
"//packages/compiler-cli/src/ngtsc/file_system",
"//packages/compiler-cli/src/ngtsc/file_system/testing",
"//packages/compiler-cli/src/ngtsc/incremental",
"//packages/compiler-cli/src/ngtsc/perf",
"//packages/compiler-cli/src/ngtsc/reflection",
"//packages/compiler-cli/src/ngtsc/testing",
"//packages/compiler-cli/src/ngtsc/transform",
"@npm//typescript",
],
)
jasmine_node_test(
name = "test",
bootstrap = ["angular/tools/testing/init_node_no_angular_spec.js"],
deps = [
":test_lib",
"//tools/testing:node_no_angular",
],
)

50
packages/compiler-cli/src/ngtsc/transform/test/compilation_spec.ts Normal file
View File

@ -0,0 +1,50 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {absoluteFrom} from '../../file_system';
import {runInEachFileSystem} from '../../file_system/testing';
import {NOOP_INCREMENTAL_BUILD} from '../../incremental';
import {NOOP_PERF_RECORDER} from '../../perf';
import {ClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
import {makeProgram} from '../../testing';
import {DtsTransformRegistry, TraitCompiler} from '../../transform';
import {AnalysisOutput, CompileResult, DecoratorHandler, HandlerPrecedence} from '../src/api';
runInEachFileSystem(() => {
describe('TraitCompiler', () => {
let _: typeof absoluteFrom;
beforeEach(() => _ = absoluteFrom);
it('should not run decoration handlers against declaration files', () => {
class FakeDecoratorHandler implements DecoratorHandler<{}|null, unknown, unknown> {
name = 'FakeDecoratorHandler';
precedence = HandlerPrecedence.PRIMARY;
detect(): undefined { throw new Error('detect should not have been called'); }
analyze(): AnalysisOutput<unknown> {
throw new Error('analyze should not have been called');
}
compile(): CompileResult { throw new Error('compile should not have been called'); }
}
const {program} = makeProgram([{
name: _('/lib.d.ts'),
contents: `export declare class SomeDirective {}`,
}]);
const checker = program.getTypeChecker();
const reflectionHost = new TypeScriptReflectionHost(checker);
const compiler = new TraitCompiler(
[new FakeDecoratorHandler()], reflectionHost, NOOP_PERF_RECORDER, NOOP_INCREMENTAL_BUILD,
true, new DtsTransformRegistry());
const sourceFile = program.getSourceFile('lib.d.ts') !;
const analysis = compiler.analyzeSync(sourceFile);
expect(sourceFile.isDeclarationFile).toBe(true);
expect(analysis).toBeFalsy();
});
});
});