feat(language-service): Introduce 'angularOnly' flag (#31935)

This PR changes the language service to work in two different modes:

1. TS + Angular
   Plugin augments TS language service to provide additonal Angular
   information. This only works with inline template and is meant to be
   used as a local plugin (configured via tsconfig.json).
2. Angular only
   Plugin only provides information on Angular templates, no TS info at
   all. This effectively disables native TS features and is meant for
   internal use only.

Default mode is `angularOnly = false` so that we don't break any users
already using Angular LS as local plugin.

As part of the refactoring, `undefined` is removed from type aliases
because it is considered bad practice.

go/tsstyle#nullableundefined-type-aliases
```
Type aliases must not include |null or |undefined in a union type.
Nullable aliases typically indicate that null values are being passed
around through too many layers of an application, and this clouds the
source of the original issue that resulted in null. They also make it
unclear when specific values on a class or interface might be absent.
```

PR Close #31935

packages/language-service/src/completions.ts

@@ -279,7 +279,7 @@ function voidElementAttributeCompletions(info: TemplateInfo, path: AstPath<HtmlA
 
 class ExpressionVisitor extends NullTemplateVisitor {
   private getExpressionScope: () => SymbolTable;
-  result: Completions;
+  result: Completion[]|undefined;
 
   constructor(
       private info: TemplateInfo, private position: number, private attr?: Attribute,

packages/language-service/src/definitions.ts

@@ -6,11 +6,28 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import * as tss from 'typescript/lib/tsserverlibrary';
+
 import {TemplateInfo} from './common';
 import {locateSymbol} from './locate_symbol';
-import {Definition} from './types';
+import {Location} from './types';
 
-export function getDefinition(info: TemplateInfo): Definition {
+export function getDefinition(info: TemplateInfo): Location[]|undefined {
   const result = locateSymbol(info);
   return result && result.symbol.definition;
 }
+
+export function ngLocationToTsDefinitionInfo(loc: Location): tss.DefinitionInfo {
+  return {
+    fileName: loc.fileName,
+    textSpan: {
+      start: loc.span.start,
+      length: loc.span.end - loc.span.start,
+    },
+    // TODO(kyliau): Provide more useful info for name, kind and containerKind
+    name: '',  // should be name of symbol but we don't have enough information here.
+    kind: tss.ScriptElementKind.unknown,
+    containerName: loc.fileName,
+    containerKind: tss.ScriptElementKind.unknown,
+  };
+}

packages/language-service/src/language_service.ts

@@ -13,7 +13,7 @@ import {getTemplateCompletions} from './completions';
 import {getDefinition} from './definitions';
 import {getDeclarationDiagnostics} from './diagnostics';
 import {getHover} from './hover';
-import {Completions, Definition, Diagnostic, DiagnosticKind, Diagnostics, Hover, LanguageService, LanguageServiceHost, Span, TemplateSource} from './types';
+import {Completion, Diagnostic, DiagnosticKind, Diagnostics, Hover, LanguageService, LanguageServiceHost, Location, Span, TemplateSource} from './types';
 import {offsetSpan, spanOf} from './utils';
 
 
@@ -34,14 +34,14 @@ class LanguageServiceImpl implements LanguageService {
 
   getTemplateReferences(): string[] { return this.host.getTemplateReferences(); }
 
-  getDiagnostics(fileName: string): Diagnostics|undefined {
+  getDiagnostics(fileName: string): Diagnostic[] {
-    let results: Diagnostics = [];
+    const results: Diagnostic[] = [];
-    let templates = this.host.getTemplates(fileName);
+    const templates = this.host.getTemplates(fileName);
     if (templates && templates.length) {
       results.push(...this.getTemplateDiagnostics(fileName, templates));
     }
 
-    let declarations = this.host.getDeclarations(fileName);
+    const declarations = this.host.getDeclarations(fileName);
     if (declarations && declarations.length) {
       const summary = this.host.getAnalyzedModules();
       results.push(...getDeclarationDiagnostics(declarations, summary));
@@ -58,14 +58,14 @@ class LanguageServiceImpl implements LanguageService {
     return [];
   }
 
-  getCompletionsAt(fileName: string, position: number): Completions {
+  getCompletionsAt(fileName: string, position: number): Completion[]|undefined {
     let templateInfo = this.host.getTemplateAstAtPosition(fileName, position);
     if (templateInfo) {
       return getTemplateCompletions(templateInfo);
     }
   }
 
-  getDefinitionAt(fileName: string, position: number): Definition {
+  getDefinitionAt(fileName: string, position: number): Location[]|undefined {
     let templateInfo = this.host.getTemplateAstAtPosition(fileName, position);
     if (templateInfo) {
       return getDefinition(templateInfo);
@@ -112,25 +112,20 @@ class LanguageServiceImpl implements LanguageService {
   }
 }
 
-function uniqueBySpan < T extends {
+function uniqueBySpan<T extends{span: Span}>(elements: T[]): T[] {
-  span: Span;
+  const result: T[] = [];
-}
+  const map = new Map<number, Set<number>>();
-> (elements: T[] | undefined): T[]|undefined {
+  for (const element of elements) {
-  if (elements) {
+    const {span} = element;
-    const result: T[] = [];
+    let set = map.get(span.start);
-    const map = new Map<number, Set<number>>();
+    if (!set) {
-    for (const element of elements) {
+      set = new Set();
-      let span = element.span;
+      map.set(span.start, set);
-      let set = map.get(span.start);
+    }
-      if (!set) {
+    if (!set.has(span.end)) {
-        set = new Set();
+      set.add(span.end);
-        map.set(span.start, set);
+      result.push(element);
-      }
-      if (!set.has(span.end)) {
-        set.add(span.end);
-        result.push(element);
-      }
     }
-    return result;
   }
+  return result;
 }

packages/language-service/src/ts_plugin.ts

@@ -9,6 +9,7 @@
 import * as ts from 'typescript'; // used as value, passed in by tsserver at runtime
 import * as tss from 'typescript/lib/tsserverlibrary'; // used as type only
 
+import {ngLocationToTsDefinitionInfo} from './definitions';
 import {createLanguageService} from './language_service';
 import {Completion, Diagnostic, DiagnosticMessageChain, Location} from './types';
 import {TypeScriptServiceHost} from './typescript_host';
@@ -23,7 +24,7 @@ export function getExternalFiles(project: tss.server.Project): string[]|undefine
   }
 }
 
-function completionToEntry(c: Completion): ts.CompletionEntry {
+function completionToEntry(c: Completion): tss.CompletionEntry {
   return {
     // TODO: remove any and fix type error.
     kind: c.kind as any,
@@ -44,15 +45,15 @@ function diagnosticChainToDiagnosticChain(chain: DiagnosticMessageChain):
 }
 
 function diagnosticMessageToDiagnosticMessageText(message: string | DiagnosticMessageChain): string|
-    ts.DiagnosticMessageChain {
+    tss.DiagnosticMessageChain {
   if (typeof message === 'string') {
     return message;
   }
   return diagnosticChainToDiagnosticChain(message);
 }
 
-function diagnosticToDiagnostic(d: Diagnostic, file: ts.SourceFile): ts.Diagnostic {
+function diagnosticToDiagnostic(d: Diagnostic, file: tss.SourceFile | undefined): tss.Diagnostic {
-  const result = {
+  return {
     file,
     start: d.span.start,
     length: d.span.end - d.span.start,
@@ -61,154 +62,131 @@ function diagnosticToDiagnostic(d: Diagnostic, file: ts.SourceFile): ts.Diagnost
     code: 0,
     source: 'ng'
   };
-  return result;
 }
 
-export function create(info: tss.server.PluginCreateInfo): ts.LanguageService {
+export function create(info: tss.server.PluginCreateInfo): tss.LanguageService {
-  const oldLS: ts.LanguageService = info.languageService;
+  const {project, languageService: tsLS, languageServiceHost: tsLSHost, config} = info;
-  const proxy: ts.LanguageService = Object.assign({}, oldLS);
+  // This plugin could operate under two different modes:
-  const logger = info.project.projectService.logger;
+  // 1. TS + Angular
-
+  //    Plugin augments TS language service to provide additional Angular
-  function tryOperation<T>(attempting: string, callback: () => T): T|null {
+  //    information. This only works with inline templates and is meant to be
-    try {
+  //    used as a local plugin (configured via tsconfig.json)
-      return callback();
+  // 2. Angular only
-    } catch (e) {
+  //    Plugin only provides information on Angular templates, no TS info at all.
-      logger.info(`Failed to ${attempting}: ${e.toString()}`);
+  //    This effectively disables native TS features and is meant for internal
-      logger.info(`Stack trace: ${e.stack}`);
+  //    use only.
-      return null;
+  const angularOnly = config ? config.angularOnly === true : false;
-    }
+  const proxy: tss.LanguageService = Object.assign({}, tsLS);
-  }
+  const ngLSHost = new TypeScriptServiceHost(tsLSHost, tsLS);
-
+  const ngLS = createLanguageService(ngLSHost);
-  const serviceHost = new TypeScriptServiceHost(info.languageServiceHost, oldLS);
+  projectHostMap.set(project, ngLSHost);
-  const ls = createLanguageService(serviceHost);
-  projectHostMap.set(info.project, serviceHost);
 
   proxy.getCompletionsAtPosition = function(
-      fileName: string, position: number, options: ts.GetCompletionsAtPositionOptions|undefined) {
+      fileName: string, position: number, options: tss.GetCompletionsAtPositionOptions|undefined) {
-    let base = oldLS.getCompletionsAtPosition(fileName, position, options) || {
+    if (!angularOnly) {
+      const results = tsLS.getCompletionsAtPosition(fileName, position, options);
+      if (results && results.entries.length) {
+        // If TS could answer the query, then return results immediately.
+        return results;
+      }
+    }
+    const results = ngLS.getCompletionsAt(fileName, position);
+    if (!results || !results.length) {
+      return;
+    }
+    return {
       isGlobalCompletion: false,
       isMemberCompletion: false,
       isNewIdentifierLocation: false,
-      entries: []
+      entries: results.map(completionToEntry),
     };
-    tryOperation('get completions', () => {
-      const results = ls.getCompletionsAt(fileName, position);
-      if (results && results.length) {
-        if (base === undefined) {
-          base = {
-            isGlobalCompletion: false,
-            isMemberCompletion: false,
-            isNewIdentifierLocation: false,
-            entries: []
-          };
-        }
-        for (const entry of results) {
-          base.entries.push(completionToEntry(entry));
-        }
-      }
-    });
-    return base;
   };
 
-  proxy.getQuickInfoAtPosition = function(fileName: string, position: number): ts.QuickInfo |
+  proxy.getQuickInfoAtPosition = function(fileName: string, position: number): tss.QuickInfo |
       undefined {
-        const base = oldLS.getQuickInfoAtPosition(fileName, position);
+        if (!angularOnly) {
-        const ours = ls.getHoverAt(fileName, position);
+          const result = tsLS.getQuickInfoAtPosition(fileName, position);
-        if (!ours) {
+          if (result) {
-          return base;
+            // If TS could answer the query, then return results immediately.
+            return result;
+          }
         }
-        const result: ts.QuickInfo = {
+        const result = ngLS.getHoverAt(fileName, position);
+        if (!result) {
+          return;
+        }
+        return {
+          // TODO(kyliau): Provide more useful info for kind and kindModifiers
           kind: ts.ScriptElementKind.unknown,
           kindModifiers: ts.ScriptElementKindModifier.none,
           textSpan: {
-            start: ours.span.start,
+            start: result.span.start,
-            length: ours.span.end - ours.span.start,
+            length: result.span.end - result.span.start,
           },
-          displayParts: ours.text.map(part => {
+          displayParts: result.text.map((part) => {
             return {
               text: part.text,
               kind: part.language || 'angular',
             };
           }),
-          documentation: [],
         };
-        if (base && base.tags) {
-          result.tags = base.tags;
-        }
-        return result;
       };
 
-  proxy.getSemanticDiagnostics = function(fileName: string) {
+  proxy.getSemanticDiagnostics = function(fileName: string): tss.Diagnostic[] {
-    let result = oldLS.getSemanticDiagnostics(fileName);
+    const results: tss.Diagnostic[] = [];
-    const base = result || [];
+    if (!angularOnly) {
-    tryOperation('get diagnostics', () => {
+      const tsResults = tsLS.getSemanticDiagnostics(fileName);
-      logger.info(`Computing Angular semantic diagnostics...`);
+      results.push(...tsResults);
-      const ours = ls.getDiagnostics(fileName);
+    }
-      if (ours && ours.length) {
+    // For semantic diagnostics we need to combine both TS + Angular results
-        const file = oldLS.getProgram() !.getSourceFile(fileName);
+    const ngResults = ngLS.getDiagnostics(fileName);
-        if (file) {
+    if (!ngResults.length) {
-          base.push.apply(base, ours.map(d => diagnosticToDiagnostic(d, file)));
+      return results;
-        }
+    }
-      }
+    const sourceFile = fileName.endsWith('.ts') ? ngLSHost.getSourceFile(fileName) : undefined;
-    });
+    results.push(...ngResults.map(d => diagnosticToDiagnostic(d, sourceFile)));
-
+    return results;
-    return base;
   };
 
   proxy.getDefinitionAtPosition = function(fileName: string, position: number):
-                                      ReadonlyArray<ts.DefinitionInfo>|
+                                      ReadonlyArray<tss.DefinitionInfo>|
       undefined {
-        const base = oldLS.getDefinitionAtPosition(fileName, position);
+        if (!angularOnly) {
-        if (base && base.length) {
+          const results = tsLS.getDefinitionAtPosition(fileName, position);
-          return base;
+          if (results) {
+            // If TS could answer the query, then return results immediately.
+            return results;
+          }
         }
-        const ours = ls.getDefinitionAt(fileName, position);
+        const results = ngLS.getDefinitionAt(fileName, position);
-        if (ours && ours.length) {
+        if (!results) {
-          return ours.map((loc: Location) => {
+          return;
-            return {
-              fileName: loc.fileName,
-              textSpan: {
-                start: loc.span.start,
-                length: loc.span.end - loc.span.start,
-              },
-              name: '',
-              kind: ts.ScriptElementKind.unknown,
-              containerName: loc.fileName,
-              containerKind: ts.ScriptElementKind.unknown,
-            };
-          });
         }
+        return results.map(ngLocationToTsDefinitionInfo);
       };
 
   proxy.getDefinitionAndBoundSpan = function(fileName: string, position: number):
-                                        ts.DefinitionInfoAndBoundSpan |
+                                        tss.DefinitionInfoAndBoundSpan |
       undefined {
-        const base = oldLS.getDefinitionAndBoundSpan(fileName, position);
+        if (!angularOnly) {
-        if (base && base.definitions && base.definitions.length) {
+          const result = tsLS.getDefinitionAndBoundSpan(fileName, position);
-          return base;
+          if (result) {
+            // If TS could answer the query, then return results immediately.
+            return result;
+          }
         }
-        const ours = ls.getDefinitionAt(fileName, position);
+        const results = ngLS.getDefinitionAt(fileName, position);
-        if (ours && ours.length) {
+        if (!results || !results.length) {
-          return {
+          return;
-            definitions: ours.map((loc: Location) => {
-              return {
-                fileName: loc.fileName,
-                textSpan: {
-                  start: loc.span.start,
-                  length: loc.span.end - loc.span.start,
-                },
-                name: '',
-                kind: ts.ScriptElementKind.unknown,
-                containerName: loc.fileName,
-                containerKind: ts.ScriptElementKind.unknown,
-              };
-            }),
-            textSpan: {
-              start: ours[0].span.start,
-              length: ours[0].span.end - ours[0].span.start,
-            },
-          };
         }
+        const {span} = results[0];
+        return {
+          definitions: results.map(ngLocationToTsDefinitionInfo),
+          textSpan: {
+            start: span.start,
+            length: span.end - span.start,
+          },
+        };
       };
 
   return proxy;

packages/language-service/src/types.ts

@@ -243,9 +243,9 @@ export interface Completion {
 /**
  * A sequence of completions.
  *
- * @publicApi
+ * @deprecated
  */
-export type Completions = Completion[] | undefined;
+export type Completions = Completion[];
 
 /**
  * A file and span.
@@ -312,7 +312,7 @@ export interface Diagnostic {
 /**
  * A sequence of diagnostic message.
  *
- * @publicApi
+ * @deprecated
  */
 export type Diagnostics = Diagnostic[];
 
@@ -384,17 +384,17 @@ export interface LanguageService {
   /**
    * Returns a list of all error for all templates in the given file.
    */
-  getDiagnostics(fileName: string): Diagnostics|undefined;
+  getDiagnostics(fileName: string): Diagnostic[];
 
   /**
    * Return the completions at the given position.
    */
-  getCompletionsAt(fileName: string, position: number): Completions|undefined;
+  getCompletionsAt(fileName: string, position: number): Completion[]|undefined;
 
   /**
    * Return the definition location for the symbol at position.
    */
-  getDefinitionAt(fileName: string, position: number): Definition|undefined;
+  getDefinitionAt(fileName: string, position: number): Location[]|undefined;
 
   /**
    * Return the hover information for the symbol at position.

packages/language-service/test/completions_spec.ts

@@ -10,7 +10,7 @@ import 'reflect-metadata';
 import * as ts from 'typescript';
 
 import {createLanguageService} from '../src/language_service';
-import {Completions} from '../src/types';
+import {Completion} from '../src/types';
 import {TypeScriptServiceHost} from '../src/typescript_host';
 
 import {toh} from './test_data';
@@ -228,7 +228,8 @@ export class MyComponent {
 });
 
 
-function expectEntries(locationMarker: string, completions: Completions, ...names: string[]) {
+function expectEntries(
+    locationMarker: string, completions: Completion[] | undefined, ...names: string[]) {
   let entries: {[name: string]: boolean} = {};
   if (!completions) {
     throw new Error(

packages/language-service/test/ts_plugin_spec.ts

@@ -7,21 +7,16 @@
  */
 
 import 'reflect-metadata';
-
 import * as ts from 'typescript';
-
 import {create} from '../src/ts_plugin';
-
 import {toh} from './test_data';
 import {MockTypescriptHost} from './test_utils';
 
 describe('plugin', () => {
-  let documentRegistry = ts.createDocumentRegistry();
+  const mockHost = new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts'], toh);
-  let mockHost = new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts'], toh);
+  const service = ts.createLanguageService(mockHost);
-  let service = ts.createLanguageService(mockHost, documentRegistry);
+  const program = service.getProgram();
-  let program = service.getProgram();
+  const plugin = createPlugin(service, mockHost);
-
-  const mockProject = {projectService: {logger: {info: function() {}}}};
 
   it('should not report errors on tour of heroes', () => {
     expectNoDiagnostics(service.getCompilerOptionsDiagnostics());
@@ -31,15 +26,6 @@ describe('plugin', () => {
     }
   });
 
-
-  let plugin = create({
-    languageService: service,
-    project: mockProject as any,
-    languageServiceHost: mockHost,
-    serverHost: {} as any,
-    config: {},
-  });
-
   it('should not report template errors on tour of heroes', () => {
     for (let source of program !.getSourceFiles()) {
       // Ignore all 'cases.ts' files as they intentionally contain errors.
@@ -197,8 +183,54 @@ describe('plugin', () => {
             'implicit', 'The template context does not defined a member called \'unknown\'');
       });
     });
+
+    describe(`with config 'angularOnly = true`, () => {
+      const ngLS = createPlugin(service, mockHost, {angularOnly: true});
+      it('should not report template errors on TOH', () => {
+        const sourceFiles = ngLS.getProgram() !.getSourceFiles();
+        expect(sourceFiles.length).toBeGreaterThan(0);
+        for (const {fileName} of sourceFiles) {
+          // Ignore all 'cases.ts' files as they intentionally contain errors.
+          if (!fileName.endsWith('cases.ts')) {
+            expectNoDiagnostics(ngLS.getSemanticDiagnostics(fileName));
+          }
+        }
+      });
+
+      it('should be able to get entity completions', () => {
+        const fileName = 'app/app.component.ts';
+        const marker = 'entity-amp';
+        const position = getMarkerLocation(fileName, marker);
+        const results = ngLS.getCompletionsAtPosition(fileName, position, {} /* options */);
+        expect(results).toBeTruthy();
+        expectEntries(marker, results !, ...['&', '>', '<', 'ι']);
+      });
+
+      it('should report template diagnostics', () => {
+        // TODO(kyliau): Rename these to end with '-error.ts'
+        const fileName = 'app/expression-cases.ts';
+        const diagnostics = ngLS.getSemanticDiagnostics(fileName);
+        expect(diagnostics.map(d => d.messageText)).toEqual([
+          `Identifier 'foo' is not defined. The component declaration, template variable declarations, and element references do not contain such a member`,
+          `Identifier 'nam' is not defined. 'Person' does not contain such a member`,
+          `Identifier 'myField' refers to a private member of the component`,
+          `Expected a numeric type`,
+        ]);
+      });
+    });
   });
 
+  function createPlugin(tsLS: ts.LanguageService, tsLSHost: ts.LanguageServiceHost, config = {}) {
+    const project = {projectService: {logger: {info() {}}}};
+    return create({
+      languageService: tsLS,
+      languageServiceHost: tsLSHost,
+      project: project as any,
+      serverHost: {} as any,
+      config: {...config},
+    });
+  }
+
   function getMarkerLocation(fileName: string, locationMarker: string): number {
     const location = mockHost.getMarkerLocations(fileName) ![locationMarker];
     if (location == null) {