fix(compiler-cli): track poisoned scopes with a flag (#39923)
To avoid overwhelming a user with secondary diagnostics that derive from a
"root cause" error, the compiler has the notion of a "poisoned" NgModule.
An NgModule becomes poisoned when its declaration contains semantic errors:
declarations which are not components or pipes, imports which are not other
NgModules, etc. An NgModule also becomes poisoned if it imports or exports
another poisoned NgModule.
Previously, the compiler tracked this poisoned status as an alternate state
for each scope. Either a correct scope could be produced, or the entire
scope would be set to a sentinel error value. This meant that the compiler
would not track any information about a scope that was determined to be in
error.
This method presents several issues:
1. The compiler is unable to support the language service and return results
when a component or its module scope is poisoned.
This is fine for compilation, since diagnostics will be produced showing the
error(s), but the language service needs to still work for incorrect code.
2. `getComponentScopes()` does not return components with a poisoned scope,
which interferes with resource tracking of incremental builds.
If the component isn't included in that list, then the NgModule for it will
not have its dependencies properly tracked, and this can cause future
incremental build steps to produce incorrect results.
This commit changes the tracking of poisoned module scopes to use a flag on
the scope itself, rather than a sentinel value that replaces the scope. This
means that the scope itself will still be tracked, even if it contains
semantic errors. A test is added to the language service which verifies that
poisoned scopes can still be used in template type-checking.
PR Close #39923
2020-11-25 18:02:23 -05:00
|
|
|
/**
|
|
|
|
* @license
|
|
|
|
* Copyright Google LLC 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 {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
|
|
|
|
|
2021-02-02 17:20:40 -05:00
|
|
|
import {isNgSpecificDiagnostic, LanguageServiceTestEnv} from '../testing';
|
fix(compiler-cli): track poisoned scopes with a flag (#39923)
To avoid overwhelming a user with secondary diagnostics that derive from a
"root cause" error, the compiler has the notion of a "poisoned" NgModule.
An NgModule becomes poisoned when its declaration contains semantic errors:
declarations which are not components or pipes, imports which are not other
NgModules, etc. An NgModule also becomes poisoned if it imports or exports
another poisoned NgModule.
Previously, the compiler tracked this poisoned status as an alternate state
for each scope. Either a correct scope could be produced, or the entire
scope would be set to a sentinel error value. This meant that the compiler
would not track any information about a scope that was determined to be in
error.
This method presents several issues:
1. The compiler is unable to support the language service and return results
when a component or its module scope is poisoned.
This is fine for compilation, since diagnostics will be produced showing the
error(s), but the language service needs to still work for incorrect code.
2. `getComponentScopes()` does not return components with a poisoned scope,
which interferes with resource tracking of incremental builds.
If the component isn't included in that list, then the NgModule for it will
not have its dependencies properly tracked, and this can cause future
incremental build steps to produce incorrect results.
This commit changes the tracking of poisoned module scopes to use a flag on
the scope itself, rather than a sentinel value that replaces the scope. This
means that the scope itself will still be tracked, even if it contains
semantic errors. A test is added to the language service which verifies that
poisoned scopes can still be used in template type-checking.
PR Close #39923
2020-11-25 18:02:23 -05:00
|
|
|
|
|
|
|
describe('language-service/compiler integration', () => {
|
|
|
|
beforeEach(() => {
|
|
|
|
initMockFileSystem('Native');
|
|
|
|
});
|
|
|
|
|
2021-01-21 18:54:40 -05:00
|
|
|
it('should react to a change in an external template', () => {
|
2021-02-02 17:20:40 -05:00
|
|
|
const env = LanguageServiceTestEnv.setup();
|
|
|
|
const project = env.addProject('test', {
|
|
|
|
'test.ts': `
|
|
|
|
import {Component} from '@angular/core';
|
2021-01-21 18:54:40 -05:00
|
|
|
|
|
|
|
@Component({
|
|
|
|
selector: 'test-cmp',
|
|
|
|
templateUrl: './test.html',
|
|
|
|
})
|
|
|
|
export class TestCmp {}
|
2021-02-02 17:20:40 -05:00
|
|
|
`,
|
|
|
|
'test.html': `<other-cmp>Test</other-cmp>`
|
|
|
|
});
|
|
|
|
|
2021-03-03 14:54:40 -05:00
|
|
|
expect(project.getDiagnosticsForFile('test.html').length).toBeGreaterThan(0);
|
2021-02-02 17:20:40 -05:00
|
|
|
|
|
|
|
const tmplFile = project.openFile('test.html');
|
|
|
|
tmplFile.contents = '<div>Test</div>';
|
2021-03-03 14:54:40 -05:00
|
|
|
expect(project.getDiagnosticsForFile('test.html').length).toEqual(0);
|
2021-01-21 18:54:40 -05:00
|
|
|
});
|
|
|
|
|
2020-12-11 14:01:51 -05:00
|
|
|
it('should not produce errors from inline test declarations mixing with those of the app', () => {
|
2021-02-02 17:20:40 -05:00
|
|
|
const env = LanguageServiceTestEnv.setup();
|
|
|
|
const project = env.addProject('test', {
|
|
|
|
'cmp.ts': `
|
|
|
|
import {Component} from '@angular/core';
|
|
|
|
|
|
|
|
@Component({
|
|
|
|
selector: 'app-cmp',
|
|
|
|
template: 'Some template',
|
|
|
|
})
|
|
|
|
export class AppCmp {}
|
|
|
|
`,
|
|
|
|
'mod.ts': `
|
|
|
|
import {NgModule} from '@angular/core';
|
|
|
|
import {AppCmp} from './cmp';
|
|
|
|
|
|
|
|
@NgModule({
|
|
|
|
declarations: [AppCmp],
|
|
|
|
})
|
|
|
|
export class AppModule {}
|
|
|
|
`,
|
|
|
|
'test_spec.ts': `
|
|
|
|
import {NgModule} from '@angular/core';
|
|
|
|
import {AppCmp} from './cmp';
|
|
|
|
|
|
|
|
export function test(): void {
|
2020-12-11 14:01:51 -05:00
|
|
|
@NgModule({
|
|
|
|
declarations: [AppCmp],
|
|
|
|
})
|
2021-02-02 17:20:40 -05:00
|
|
|
class TestModule {}
|
|
|
|
}
|
|
|
|
`,
|
|
|
|
});
|
2020-12-11 14:01:51 -05:00
|
|
|
|
|
|
|
// Expect that this program is clean diagnostically.
|
2021-02-02 17:20:40 -05:00
|
|
|
expect(project.getDiagnosticsForFile('cmp.ts')).toEqual([]);
|
|
|
|
expect(project.getDiagnosticsForFile('mod.ts')).toEqual([]);
|
|
|
|
expect(project.getDiagnosticsForFile('test_spec.ts')).toEqual([]);
|
2020-12-11 14:01:51 -05:00
|
|
|
});
|
|
|
|
|
fix(compiler-cli): track poisoned scopes with a flag (#39923)
To avoid overwhelming a user with secondary diagnostics that derive from a
"root cause" error, the compiler has the notion of a "poisoned" NgModule.
An NgModule becomes poisoned when its declaration contains semantic errors:
declarations which are not components or pipes, imports which are not other
NgModules, etc. An NgModule also becomes poisoned if it imports or exports
another poisoned NgModule.
Previously, the compiler tracked this poisoned status as an alternate state
for each scope. Either a correct scope could be produced, or the entire
scope would be set to a sentinel error value. This meant that the compiler
would not track any information about a scope that was determined to be in
error.
This method presents several issues:
1. The compiler is unable to support the language service and return results
when a component or its module scope is poisoned.
This is fine for compilation, since diagnostics will be produced showing the
error(s), but the language service needs to still work for incorrect code.
2. `getComponentScopes()` does not return components with a poisoned scope,
which interferes with resource tracking of incremental builds.
If the component isn't included in that list, then the NgModule for it will
not have its dependencies properly tracked, and this can cause future
incremental build steps to produce incorrect results.
This commit changes the tracking of poisoned module scopes to use a flag on
the scope itself, rather than a sentinel value that replaces the scope. This
means that the scope itself will still be tracked, even if it contains
semantic errors. A test is added to the language service which verifies that
poisoned scopes can still be used in template type-checking.
PR Close #39923
2020-11-25 18:02:23 -05:00
|
|
|
it('should show type-checking errors from components with poisoned scopes', () => {
|
|
|
|
// Normally, the Angular compiler suppresses errors from components that belong to NgModules
|
|
|
|
// which themselves have errors (such scopes are considered "poisoned"), to avoid overwhelming
|
|
|
|
// the user with secondary errors that stem from a primary root cause. However, this prevents
|
|
|
|
// the generation of type check blocks and other metadata within the compiler which drive the
|
|
|
|
// Language Service's understanding of components. Therefore in the Language Service, the
|
|
|
|
// compiler is configured to make use of such data even if it's "poisoned". This test verifies
|
|
|
|
// that a component declared in an NgModule with a faulty import still generates template
|
|
|
|
// diagnostics.
|
|
|
|
|
2021-02-02 17:20:40 -05:00
|
|
|
const env = LanguageServiceTestEnv.setup();
|
|
|
|
const project = env.addProject('test', {
|
|
|
|
'test.ts': `
|
|
|
|
import {Component, Directive, Input, NgModule} from '@angular/core';
|
|
|
|
|
|
|
|
@Component({
|
|
|
|
selector: 'test-cmp',
|
|
|
|
template: '<div [dir]="3"></div>',
|
|
|
|
})
|
|
|
|
export class Cmp {}
|
|
|
|
|
|
|
|
@Directive({
|
|
|
|
selector: '[dir]',
|
|
|
|
})
|
|
|
|
export class Dir {
|
|
|
|
@Input() dir!: string;
|
|
|
|
}
|
|
|
|
|
|
|
|
export class NotAModule {}
|
|
|
|
|
|
|
|
@NgModule({
|
|
|
|
declarations: [Cmp, Dir],
|
|
|
|
imports: [NotAModule],
|
|
|
|
})
|
|
|
|
export class Mod {}
|
|
|
|
`,
|
|
|
|
});
|
|
|
|
|
|
|
|
const diags = project.getDiagnosticsForFile('test.ts').map(diag => diag.messageText);
|
|
|
|
expect(diags).toContain(`Type 'number' is not assignable to type 'string'.`);
|
fix(compiler-cli): track poisoned scopes with a flag (#39923)
To avoid overwhelming a user with secondary diagnostics that derive from a
"root cause" error, the compiler has the notion of a "poisoned" NgModule.
An NgModule becomes poisoned when its declaration contains semantic errors:
declarations which are not components or pipes, imports which are not other
NgModules, etc. An NgModule also becomes poisoned if it imports or exports
another poisoned NgModule.
Previously, the compiler tracked this poisoned status as an alternate state
for each scope. Either a correct scope could be produced, or the entire
scope would be set to a sentinel error value. This meant that the compiler
would not track any information about a scope that was determined to be in
error.
This method presents several issues:
1. The compiler is unable to support the language service and return results
when a component or its module scope is poisoned.
This is fine for compilation, since diagnostics will be produced showing the
error(s), but the language service needs to still work for incorrect code.
2. `getComponentScopes()` does not return components with a poisoned scope,
which interferes with resource tracking of incremental builds.
If the component isn't included in that list, then the NgModule for it will
not have its dependencies properly tracked, and this can cause future
incremental build steps to produce incorrect results.
This commit changes the tracking of poisoned module scopes to use a flag on
the scope itself, rather than a sentinel value that replaces the scope. This
means that the scope itself will still be tracked, even if it contains
semantic errors. A test is added to the language service which verifies that
poisoned scopes can still be used in template type-checking.
PR Close #39923
2020-11-25 18:02:23 -05:00
|
|
|
});
|
2020-12-01 17:55:23 -05:00
|
|
|
|
|
|
|
it('should handle broken imports during incremental build steps', () => {
|
|
|
|
// This test validates that the compiler's incremental APIs correctly handle a broken import
|
|
|
|
// when invoked via the Language Service. Testing this via the LS is important as only the LS
|
|
|
|
// requests Angular analysis in the presence of TypeScript-level errors. In the case of broken
|
|
|
|
// imports this distinction is especially important: Angular's incremental analysis is
|
2021-01-14 14:03:58 -05:00
|
|
|
// built on the compiler's dependency graph, and this graph must be able to function even
|
2020-12-01 17:55:23 -05:00
|
|
|
// with broken imports.
|
|
|
|
//
|
|
|
|
// The test works by creating a component/module pair where the module imports and declares a
|
|
|
|
// component from a separate file. That component is initially not exported, meaning the
|
|
|
|
// module's import is broken. Angular will correctly complain that the NgModule is declaring a
|
|
|
|
// value which is not statically analyzable.
|
|
|
|
//
|
|
|
|
// Then, the component file is fixed to properly export the component class, and an incremental
|
|
|
|
// build step is performed. The compiler should recognize that the module's previous analysis
|
|
|
|
// is stale, even though it was not able to fully understand the import during the first pass.
|
|
|
|
|
|
|
|
const componentSource = (isExported: boolean): string => `
|
|
|
|
import {Component} from '@angular/core';
|
|
|
|
|
|
|
|
@Component({
|
|
|
|
selector: 'some-cmp',
|
|
|
|
template: 'Not important',
|
|
|
|
})
|
|
|
|
${isExported ? 'export' : ''} class Cmp {}
|
|
|
|
`;
|
|
|
|
|
2021-02-02 17:20:40 -05:00
|
|
|
const env = LanguageServiceTestEnv.setup();
|
|
|
|
const project = env.addProject('test', {
|
|
|
|
'mod.ts': `
|
|
|
|
import {NgModule} from '@angular/core';
|
2021-01-06 17:17:45 -05:00
|
|
|
|
2021-02-02 17:20:40 -05:00
|
|
|
import {Cmp} from './cmp';
|
2021-01-06 17:17:45 -05:00
|
|
|
|
2021-02-02 17:20:40 -05:00
|
|
|
@NgModule({
|
|
|
|
declarations: [Cmp],
|
|
|
|
})
|
|
|
|
export class Mod {}
|
|
|
|
`,
|
|
|
|
'cmp.ts': componentSource(/* start with component not exported */ false),
|
|
|
|
});
|
2020-12-01 17:55:23 -05:00
|
|
|
|
|
|
|
// Angular should be complaining about the module not being understandable.
|
2021-02-02 17:20:40 -05:00
|
|
|
const ngDiagsBefore = project.getDiagnosticsForFile('mod.ts').filter(isNgSpecificDiagnostic);
|
2020-12-01 17:55:23 -05:00
|
|
|
expect(ngDiagsBefore.length).toBe(1);
|
|
|
|
|
|
|
|
// Fix the import.
|
2021-02-02 17:20:40 -05:00
|
|
|
const file = project.openFile('cmp.ts');
|
|
|
|
file.contents = componentSource(/* properly export the component */ true);
|
2020-12-01 17:55:23 -05:00
|
|
|
|
|
|
|
// Angular should stop complaining about the NgModule.
|
2021-02-02 17:20:40 -05:00
|
|
|
const ngDiagsAfter = project.getDiagnosticsForFile('mod.ts').filter(isNgSpecificDiagnostic);
|
2020-12-01 17:55:23 -05:00
|
|
|
expect(ngDiagsAfter.length).toBe(0);
|
|
|
|
});
|
2021-04-07 20:01:04 -04:00
|
|
|
|
|
|
|
it('detects source file change in between typecheck programs', () => {
|
|
|
|
const env = LanguageServiceTestEnv.setup();
|
|
|
|
const project = env.addProject('test', {
|
|
|
|
'module.ts': `
|
|
|
|
import {NgModule} from '@angular/core';
|
|
|
|
import {BarCmp} from './bar';
|
|
|
|
|
|
|
|
@NgModule({
|
|
|
|
declarations: [BarCmp],
|
|
|
|
})
|
|
|
|
export class AppModule {}
|
|
|
|
`,
|
|
|
|
'bar.ts': `
|
|
|
|
import {Component} from '@angular/core';
|
|
|
|
|
|
|
|
@Component({
|
|
|
|
template: '{{ bar }}',
|
|
|
|
})
|
|
|
|
export class BarCmp {
|
|
|
|
readonly bar = 'bar';
|
|
|
|
}
|
|
|
|
`,
|
|
|
|
});
|
|
|
|
// The opening of 'bar.ts' causes its version to change, because the internal
|
|
|
|
// representation switches from TextStorage to ScriptVersionCache.
|
|
|
|
const bar = project.openFile('bar.ts');
|
|
|
|
// When getDiagnostics is called, NgCompiler calls ensureAllShimsForOneFile
|
|
|
|
// and caches the source file for 'bar.ts' in the input program.
|
|
|
|
// The input program has not picked up the version change because the project
|
|
|
|
// is clean (rightly so since there's no user-initiated change).
|
|
|
|
expect(project.getDiagnosticsForFile('bar.ts').length).toBe(0);
|
|
|
|
// A new program is generated due to addition of typecheck file. During
|
|
|
|
// program creation, TS language service does a sweep of all source files,
|
|
|
|
// and detects the version change. Consequently, it creates a new source
|
|
|
|
// file for 'bar.ts'. This is a violation of our assumption that a SourceFile
|
|
|
|
// will never change in between typecheck programs.
|
|
|
|
bar.moveCursorToText(`template: '{{ b¦ar }}'`);
|
|
|
|
expect(bar.getQuickInfoAtPosition()).toBeDefined();
|
|
|
|
});
|
fix(compiler-cli): track poisoned scopes with a flag (#39923)
To avoid overwhelming a user with secondary diagnostics that derive from a
"root cause" error, the compiler has the notion of a "poisoned" NgModule.
An NgModule becomes poisoned when its declaration contains semantic errors:
declarations which are not components or pipes, imports which are not other
NgModules, etc. An NgModule also becomes poisoned if it imports or exports
another poisoned NgModule.
Previously, the compiler tracked this poisoned status as an alternate state
for each scope. Either a correct scope could be produced, or the entire
scope would be set to a sentinel error value. This meant that the compiler
would not track any information about a scope that was determined to be in
error.
This method presents several issues:
1. The compiler is unable to support the language service and return results
when a component or its module scope is poisoned.
This is fine for compilation, since diagnostics will be produced showing the
error(s), but the language service needs to still work for incorrect code.
2. `getComponentScopes()` does not return components with a poisoned scope,
which interferes with resource tracking of incremental builds.
If the component isn't included in that list, then the NgModule for it will
not have its dependencies properly tracked, and this can cause future
incremental build steps to produce incorrect results.
This commit changes the tracking of poisoned module scopes to use a flag on
the scope itself, rather than a sentinel value that replaces the scope. This
means that the scope itself will still be tracked, even if it contains
semantic errors. A test is added to the language service which verifies that
poisoned scopes can still be used in template type-checking.
PR Close #39923
2020-11-25 18:02:23 -05:00
|
|
|
});
|