fix(language-service): force compileNonExportedClasses: false in LS (#…

…40092)

Projects opened in the LS are often larger in scope than the compilation
units seen by the compiler when actually building. For example, in the LS
it's not uncommon for the project to include both application as well as
test files. This can create issues when the combination of files results
in errors that are not otherwise present - for example, if test files
have inline NgModules that re-declare components (a common Angular pattern).
Such code is valid when compiling the app only (test files are excluded, so
only one declaration is seen by the compiler) or when compiling tests only
(since tests run in JIT mode and are not seen by the AOT compiler), but when
both sets of files are mixed into a single compilation unit, the compiler
sees the double declaration as an error.

This commit attempts to mitigate the problem by forcing the compiler flag
`compileNonExportedClasses` to `false` in a LS context. When tests contain
duplicate declarations, often such declarations are inline in specs and not
exported from the top level, so this flag can be used to greatly improve the
IDE experience.

PR Close #40092

packages/language-service/ivy/language_service.ts

@@ -32,6 +32,18 @@ export class LanguageService {
   constructor(project: ts.server.Project, private readonly tsLS: ts.LanguageService) {
     this.parseConfigHost = new LSParseConfigHost(project.projectService.host);
     this.options = parseNgCompilerOptions(project, this.parseConfigHost);
+
+    // Projects loaded into the Language Service often include test files which are not part of the
+    // app's main compilation unit, and these test files often include inline NgModules that declare
+    // components from the app. These declarations conflict with the main declarations of such
+    // components in the app's NgModules. This conflict is not normally present during regular
+    // compilation because the app and the tests are part of separate compilation units.
+    //
+    // As a temporary mitigation of this problem, we instruct the compiler to ignore classes which
+    // are not exported. In many cases, this ensures the test NgModules are ignored by the compiler
+    // and only the real component declaration is used.
+    this.options.compileNonExportedClasses = false;
+
     this.strategy = createTypeCheckingProgramStrategy(project);
     this.adapter = new LanguageServiceAdapter(project);
     this.compilerFactory = new CompilerFactory(this.adapter, this.strategy, this.options);

packages/language-service/ivy/test/compiler_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {absoluteFrom} from '@angular/compiler-cli/src/ngtsc/file_system';
+import {absoluteFrom, getSourceFileOrError} from '@angular/compiler-cli/src/ngtsc/file_system';
 import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
 
 import {LanguageServiceTestEnvironment} from './env';
@@ -16,6 +16,63 @@ describe('language-service/compiler integration', () => {
     initMockFileSystem('Native');
   });
 
+  it('should not produce errors from inline test declarations mixing with those of the app', () => {
+    const appCmpFile = absoluteFrom('/test.cmp.ts');
+    const appModuleFile = absoluteFrom('/test.mod.ts');
+    const testFile = absoluteFrom('/test_spec.ts');
+
+    const env = LanguageServiceTestEnvironment.setup([
+      {
+        name: appCmpFile,
+        contents: `
+          import {Component} from '@angular/core';
+
+          @Component({
+            selector: 'app-cmp',
+            template: 'Some template',
+          })
+          export class AppCmp {}
+        `,
+        isRoot: true,
+      },
+      {
+        name: appModuleFile,
+        contents: `
+          import {NgModule} from '@angular/core';
+          import {AppCmp} from './test.cmp';
+
+          @NgModule({
+            declarations: [AppCmp],
+          })
+          export class AppModule {}
+        `,
+        isRoot: true,
+      },
+      {
+        name: testFile,
+        contents: `
+          import {NgModule} from '@angular/core';
+          import {AppCmp} from './test.cmp';
+
+          export function test(): void {
+            @NgModule({
+              declarations: [AppCmp],
+            })
+            class TestModule {}
+          }
+        `,
+        isRoot: true,
+      }
+    ]);
+
+    // Expect that this program is clean diagnostically.
+    const ngCompiler = env.ngLS.compilerFactory.getOrCreate();
+    const program = ngCompiler.getNextProgram();
+    expect(ngCompiler.getDiagnostics(getSourceFileOrError(program, appCmpFile))).toEqual([]);
+    expect(ngCompiler.getDiagnostics(getSourceFileOrError(program, appModuleFile))).toEqual([]);
+    expect(ngCompiler.getDiagnostics(getSourceFileOrError(program, testFile))).toEqual([]);
+  });
+
   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