refactor(compiler-cli): better error messages when external strings u…

…sed for template and styles in local compilation mode

In local compilation mode it is not possible to use an imported string for component's template or styles as it cannot be resolved statically in compile time. There are some such use cases in g3 and potentially devs might incorporate such pattern. At the moment such pattern will cause the local compilation fail with generic error messages (e.g., so and so at position 1 is not a reference, etc). This change makes specific error messages with helpful hints for such cases. These new error messages can help devs to quickly resolve the issue as well as make it possible to identify existing issues in g3.

goldens/public-api/compiler-cli/error_code.md

@@ -51,6 +51,8 @@ export enum ErrorCode {
     INLINE_TCB_REQUIRED = 8900,
     INLINE_TYPE_CTOR_REQUIRED = 8901,
     INVALID_BANANA_IN_BOX = 8101,
+    LOCAL_COMPILATION_IMPORTED_STYLES_STRING = 11002,
+    LOCAL_COMPILATION_IMPORTED_TEMPLATE_STRING = 11001,
     MISSING_CONTROL_FLOW_DIRECTIVE = 8103,
     MISSING_NGFOROF_LET = 8105,
     MISSING_PIPE = 8004,

packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts

@@ -27,7 +27,7 @@ import {ExtendedTemplateChecker} from '../../../typecheck/extended/api';
 import {getSourceFile} from '../../../util/src/typescript';
 import {Xi18nContext} from '../../../xi18n';
 import {combineResolvers, compileDeclareFactory, compileInputTransformFields, compileNgFactoryDefField, compileResults, extractClassMetadata, extractSchemas, findAngularDecorator, forwardRefResolver, getDirectiveDiagnostics, getProviderDiagnostics, InjectableClassRegistry, isExpressionForwardReference, readBaseClass, ReferencesRegistry, resolveEnumValue, resolveImportedFile, resolveLiteral, resolveProvidersRequiringFactory, ResourceLoader, toFactoryMetadata, validateHostDirectives, wrapFunctionExpressionsInParens,} from '../../common';
-import {extractDirectiveMetadata, parseFieldStringArrayValue} from '../../directive';
+import {extractDirectiveMetadata, parseDirectiveStyles} from '../../directive';
 import {createModuleWithProvidersResolver, NgModuleSymbol} from '../../ng_module';
 
 import {checkCustomElementSelectorForErrors, makeCyclicImportInfo} from './diagnostics';
@@ -167,7 +167,7 @@ export class ComponentDecoratorHandler implements
         preloadAndParseTemplate(
             this.evaluator, this.resourceLoader, this.depTracker, this.preanalyzeTemplateCache,
             node, decorator, component, containingFile, this.defaultPreserveWhitespaces,
-            this.extractTemplateOptions)
+            this.extractTemplateOptions, this.compilationMode)
             .then((template: ParsedTemplateWithSource|null): Promise<void>|undefined => {
               if (template === null) {
                 return undefined;
@@ -183,7 +183,7 @@ export class ComponentDecoratorHandler implements
     // Extract inline styles, process, and cache for use in synchronous analyze phase
     let inlineStyles;
     if (component.has('styles')) {
-      const litStyles = parseFieldStringArrayValue(component, 'styles', this.evaluator);
+      const litStyles = parseDirectiveStyles(component, this.evaluator, this.compilationMode);
       if (litStyles === null) {
         this.preanalyzeStylesCache.set(node, null);
       } else {
@@ -355,7 +355,8 @@ export class ComponentDecoratorHandler implements
             i18nNormalizeLineEndingsInICUs: this.i18nNormalizeLineEndingsInICUs,
             usePoisonedData: this.usePoisonedData,
             enabledBlockTypes: this.enabledBlockTypes,
-          });
+          },
+          this.compilationMode);
     }
     const templateResource =
         template.declaration.isInline ? {path: null, expression: component.get('template')!} : {
@@ -433,7 +434,7 @@ export class ComponentDecoratorHandler implements
       }
 
       if (component.has('styles')) {
-        const litStyles = parseFieldStringArrayValue(component, 'styles', this.evaluator);
+        const litStyles = parseDirectiveStyles(component, this.evaluator, this.compilationMode);
         if (litStyles !== null) {
           inlineStyles = [...litStyles];
           styles.push(...litStyles);
@@ -948,7 +949,7 @@ export class ComponentDecoratorHandler implements
     if (!templateDecl.isInline) {
       analysis.template = extractTemplate(
           node, templateDecl, this.evaluator, this.depTracker, this.resourceLoader,
-          this.extractTemplateOptions);
+          this.extractTemplateOptions, this.compilationMode);
     }
 
     // Update any external stylesheets and rebuild the combined 'styles' list.

packages/compiler-cli/src/ngtsc/annotations/component/src/resources.ts

@@ -13,8 +13,9 @@ import {ErrorCode, FatalDiagnosticError} from '../../../diagnostics';
 import {absoluteFrom} from '../../../file_system';
 import {DependencyTracker} from '../../../incremental/api';
 import {Resource} from '../../../metadata';
-import {PartialEvaluator} from '../../../partial_evaluator';
+import {DynamicValue, PartialEvaluator, traceDynamicValue} from '../../../partial_evaluator';
 import {ClassDeclaration, DeclarationNode, Decorator} from '../../../reflection';
+import {CompilationMode} from '../../../transform';
 import {TemplateSourceMapping} from '../../../typecheck/api';
 import {createValueHasWrongTypeError, isStringArray, ResourceLoader} from '../../common';
 
@@ -124,7 +125,7 @@ export interface ExtractTemplateOptions {
 export function extractTemplate(
     node: ClassDeclaration, template: TemplateDeclaration, evaluator: PartialEvaluator,
     depTracker: DependencyTracker|null, resourceLoader: ResourceLoader,
-    options: ExtractTemplateOptions): ParsedTemplateWithSource {
+    options: ExtractTemplateOptions, compilationMode: CompilationMode): ParsedTemplateWithSource {
   if (template.isInline) {
     let sourceStr: string;
     let sourceParseRange: LexerRange|null = null;
@@ -148,6 +149,27 @@ export function extractTemplate(
       sourceMapUrl = template.resolvedTemplateUrl;
     } else {
       const resolvedTemplate = evaluator.evaluate(template.expression);
+
+      // In local compilation mode we use imported strings from another file as template as it
+      // cannot be resolved in single file mode. We warn the user here about the situation
+      // explicitly.
+      if (compilationMode === CompilationMode.LOCAL && resolvedTemplate instanceof DynamicValue &&
+          resolvedTemplate.isFromUnknownIdentifier()) {
+        const relatedInformation = traceDynamicValue(template.expression, resolvedTemplate);
+
+        const chain: ts.DiagnosticMessageChain = {
+          messageText: `Unknown identifier used as template string: ${
+              template.expression
+                  .getText()} (did you import this string from another file? This is not allowed in local compilation mode. Please either inline it or move it to a separate file and include it using 'templateUrl')`,
+          category: ts.DiagnosticCategory.Error,
+          code: 0,
+        };
+
+        throw new FatalDiagnosticError(
+            ErrorCode.LOCAL_COMPILATION_IMPORTED_TEMPLATE_STRING, template.expression, chain,
+            relatedInformation);
+      }
+
       if (typeof resolvedTemplate !== 'string') {
         throw createValueHasWrongTypeError(
             template.expression, resolvedTemplate, 'template must be a string');
@@ -325,8 +347,8 @@ export function preloadAndParseTemplate(
     evaluator: PartialEvaluator, resourceLoader: ResourceLoader, depTracker: DependencyTracker|null,
     preanalyzeTemplateCache: Map<DeclarationNode, ParsedTemplateWithSource>, node: ClassDeclaration,
     decorator: Decorator, component: Map<string, ts.Expression>, containingFile: string,
-    defaultPreserveWhitespaces: boolean,
-    options: ExtractTemplateOptions): Promise<ParsedTemplateWithSource|null> {
+    defaultPreserveWhitespaces: boolean, options: ExtractTemplateOptions,
+    compilationMode: CompilationMode): Promise<ParsedTemplateWithSource|null> {
   if (component.has('templateUrl')) {
     // Extract the templateUrl and preload it.
     const templateUrlExpr = component.get('templateUrl')!;
@@ -347,8 +369,8 @@ export function preloadAndParseTemplate(
           const templateDecl = parseTemplateDeclaration(
               node, decorator, component, containingFile, evaluator, depTracker, resourceLoader,
               defaultPreserveWhitespaces);
-          const template =
-              extractTemplate(node, templateDecl, evaluator, depTracker, resourceLoader, options);
+          const template = extractTemplate(
+              node, templateDecl, evaluator, depTracker, resourceLoader, options, compilationMode);
           preanalyzeTemplateCache.set(node, template);
           return template;
         });
@@ -369,8 +391,8 @@ export function preloadAndParseTemplate(
     const templateDecl = parseTemplateDeclaration(
         node, decorator, component, containingFile, evaluator, depTracker, resourceLoader,
         defaultPreserveWhitespaces);
-    const template =
-        extractTemplate(node, templateDecl, evaluator, depTracker, resourceLoader, options);
+    const template = extractTemplate(
+        node, templateDecl, evaluator, depTracker, resourceLoader, options, compilationMode);
     preanalyzeTemplateCache.set(node, template);
     return Promise.resolve(template);
   }

packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts

@@ -12,7 +12,7 @@ import ts from 'typescript';
 import {ErrorCode, FatalDiagnosticError, makeRelatedInformation} from '../../../diagnostics';
 import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitter} from '../../../imports';
 import {ClassPropertyMapping, HostDirectiveMeta, InputMapping, InputTransform} from '../../../metadata';
-import {DynamicValue, EnumValue, PartialEvaluator, ResolvedValue} from '../../../partial_evaluator';
+import {DynamicValue, EnumValue, PartialEvaluator, ResolvedValue, traceDynamicValue} from '../../../partial_evaluator';
 import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembersWithDecorator, isNamedClassDeclaration, ReflectionHost, reflectObjectLiteral} from '../../../reflection';
 import {CompilationMode} from '../../../transform';
 import {createSourceSpan, createValueHasWrongTypeError, forwardRefResolver, getConstructorDependencies, ReferencesRegistry, toR3Reference, tryUnwrapForwardRef, unwrapConstructorDependencies, unwrapExpression, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference,} from '../../common';
@@ -461,6 +461,48 @@ function extractQueriesFromDecorator(
   return {content, view};
 }
 
+export function parseDirectiveStyles(
+    directive: Map<string, ts.Expression>, evaluator: PartialEvaluator,
+    compilationMode: CompilationMode): null|string[] {
+  const expression = directive.get('styles');
+
+  if (!expression) {
+    return null;
+  }
+
+  const value = evaluator.evaluate(expression);
+
+  // Create specific error if any string is imported from external file in local compilation mode
+  if (compilationMode === CompilationMode.LOCAL) {
+    if (Array.isArray(value)) {
+      for (const entry of value) {
+        if (entry instanceof DynamicValue && entry.isFromUnknownIdentifier()) {
+          const relatedInformation = traceDynamicValue(expression, entry);
+
+          const chain: ts.DiagnosticMessageChain = {
+            messageText: `Unknown identifier used as styles string: ${
+                entry.node
+                    .getText()} (did you import this string from another file? This is not allowed in local compilation mode. Please either inline it or move it to a separate file and include it using'styleUrls')`,
+            category: ts.DiagnosticCategory.Error,
+            code: 0,
+          };
+
+          throw new FatalDiagnosticError(
+              ErrorCode.LOCAL_COMPILATION_IMPORTED_STYLES_STRING, expression, chain,
+              relatedInformation);
+        }
+      }
+    }
+  }
+
+  if (!isStringArrayOrDie(value, 'styles', expression)) {
+    throw createValueHasWrongTypeError(
+        expression, value, `Failed to resolve @Directive.styles to a string array`);
+  }
+
+  return value;
+}
+
 export function parseFieldStringArrayValue(
     directive: Map<string, ts.Expression>, field: string, evaluator: PartialEvaluator): null|
     string[] {

packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts

@@ -383,4 +383,16 @@ export enum ErrorCode {
    * type inference.
    */
   SUGGEST_SUBOPTIMAL_TYPE_INFERENCE = 10002,
+
+  /**
+   * A string is imported from another file to be used as template string for a component in local
+   * compilation mode.
+   */
+  LOCAL_COMPILATION_IMPORTED_TEMPLATE_STRING = 11001,
+
+  /**
+   * A string is imported from another file to be used as styles string for a component in local
+   * compilation mode.
+   */
+  LOCAL_COMPILATION_IMPORTED_STYLES_STRING = 11002,
 }

packages/compiler-cli/test/ngtsc/local_compilation_spec.ts

@@ -6,6 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import ts from 'typescript';
+
+import {ErrorCode, ngErrorCode} from '../../src/ngtsc/diagnostics';
 import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
 import {loadStandardTestFiles} from '../../src/ngtsc/testing';
 
@@ -501,5 +504,63 @@ runInEachFileSystem(() => {
                    `MainModule.ɵfac = function MainModule_Factory(t) { return new (t || MainModule)(i0.ɵɵinject(SomeService1), i0.ɵɵinject(SomeService2), i0.ɵɵinject(SomeWhere3.SomeService3), i0.ɵɵinjectAttribute('title'), i0.ɵɵinject(MESSAGE_TOKEN)); };`);
          });
     });
+
+    describe('local compilation specific errors', () => {
+      it('should show extensive error message when using an imported symbol for component template',
+         () => {
+           env.write('test.ts', `
+          import {Component} from '@angular/core';
+          import {ExternalString} from './some-where';
+
+          @Component({            
+            template: ExternalString,
+          })
+          export class Main {
+          }
+          `);
+
+           const errors = env.driveDiagnostics();
+
+           expect(errors.length).toBe(1);
+
+           const {code, messageText} = errors[0];
+
+           expect(code).toBe(ngErrorCode(ErrorCode.
+
+                                         LOCAL_COMPILATION_IMPORTED_TEMPLATE_STRING));
+           const text = ts.flattenDiagnosticMessageText(messageText, '\n');
+
+           expect(text).toContain('Unknown identifier used as template string: ExternalString');
+           expect(text).toContain('either inline it or move it to a separate file');
+         });
+
+      it('should show extensive error message when using an imported symbol for component styles',
+         () => {
+           env.write('test.ts', `
+          import {Component} from '@angular/core';
+          import {ExternalString} from './some-where';
+
+          @Component({            
+            styles: [ExternalString],
+            template: '',
+
+          })
+          export class Main {
+          }
+          `);
+
+           const errors = env.driveDiagnostics();
+
+           expect(errors.length).toBe(1);
+
+           const {code, messageText} = errors[0];
+
+           expect(code).toBe(ngErrorCode(ErrorCode.LOCAL_COMPILATION_IMPORTED_STYLES_STRING));
+           const text = ts.flattenDiagnosticMessageText(messageText, '\n');
+
+           expect(text).toContain('Unknown identifier used as styles string: ExternalString');
+           expect(text).toContain('either inline it or move it to a separate file');
+         });
+    });
   });
 });