angular · petebacondarwin · Feb 8, 2019 · Feb 8, 2019 · Feb 8, 2019 · Feb 8, 2019
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -15,4 +15,5 @@
     "**/bazel-out": true,
     "**/dist": true,
   },
+  "git.ignoreLimitWarning": true,
 }
diff --git a/packages/compiler-cli/src/diagnostics/expression_diagnostics.ts b/packages/compiler-cli/src/diagnostics/expression_diagnostics.ts
@@ -272,8 +272,7 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
     const path = findNode(this.info.htmlAst, ast.sourceSpan.start.offset);
     const last = path.tail;
     if (last instanceof Attribute && last.valueSpan) {
-      // Add 1 for the quote.
-      return last.valueSpan.start.offset + 1;
+      return last.valueSpan.start.offset;
     }
     return ast.sourceSpan.start.offset;
   }

diff --git a/packages/compiler-cli/src/ngtsc/annotations/BUILD.bazel b/packages/compiler-cli/src/ngtsc/annotations/BUILD.bazel
@@ -18,6 +18,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/routing",
         "//packages/compiler-cli/src/ngtsc/transform",
         "//packages/compiler-cli/src/ngtsc/typecheck",
+        "//packages/compiler-cli/src/ngtsc/util",
         "@ngdeps//@types/node",
         "@ngdeps//typescript",
     ],

diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts
@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ConstantPool, CssSelector, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, ElementSchemaRegistry, Expression, ExternalExpr, InterpolationConfig, R3ComponentMetadata, R3DirectiveMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr, compileComponentFromMetadata, makeBindingParser, parseTemplate} from '@angular/compiler';
+import {ConstantPool, CssSelector, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, InterpolationConfig, LexerRange, R3ComponentMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr, compileComponentFromMetadata, makeBindingParser, parseTemplate} from '@angular/compiler';
 import * as path from 'path';
 import * as ts from 'typescript';
 
@@ -16,7 +16,8 @@ import {ModuleResolver, Reference, ResolvedReference} from '../../imports';
 import {EnumValue, PartialEvaluator} from '../../partial_evaluator';
 import {Decorator, ReflectionHost, filterToMembersWithDecorator, reflectObjectLiteral} from '../../reflection';
 import {AnalysisOutput, CompileResult, DecoratorHandler} from '../../transform';
-import {TypeCheckContext, TypeCheckableDirectiveMeta} from '../../typecheck';
+import {TypeCheckContext} from '../../typecheck';
+import {tsSourceMapBug29300Fixed} from '../../util/src/ts_source_map_bug_29300';
 
 import {ResourceLoader} from './api';
 import {extractDirectiveMetadata, extractQueriesFromDecorator, parseFieldArrayValue, queriesFromFields} from './directive';
@@ -119,24 +120,56 @@ export class ComponentDecoratorHandler implements
     // Next, read the `@Component`-specific fields.
     const {decoratedElements, decorator: component, metadata} = directiveResult;
 
+    // Go through the root directories for this project, and select the one with the smallest
+    // relative path representation.
+    const filePath = node.getSourceFile().fileName;
+    const relativeContextFilePath = this.rootDirs.reduce<string|undefined>((previous, rootDir) => {
+      const candidate = path.posix.relative(rootDir, filePath);
+      if (previous === undefined || candidate.length < previous.length) {
+        return candidate;
+      } else {
+        return previous;
+      }
+    }, undefined) !;
+
     let templateStr: string|null = null;
+    let templateUrl: string = '';
+    let templateRange: LexerRange|undefined;
+    let escapedString: boolean = false;
+
     if (component.has('templateUrl')) {
       const templateUrlExpr = component.get('templateUrl') !;
-      const templateUrl = this.evaluator.evaluate(templateUrlExpr);
-      if (typeof templateUrl !== 'string') {
+      const evalTemplateUrl = this.evaluator.evaluate(templateUrlExpr);
+      if (typeof evalTemplateUrl !== 'string') {
         throw new FatalDiagnosticError(
             ErrorCode.VALUE_HAS_WRONG_TYPE, templateUrlExpr, 'templateUrl must be a string');
       }
-      const resolvedTemplateUrl = this.resourceLoader.resolve(templateUrl, containingFile);
-      templateStr = this.resourceLoader.load(resolvedTemplateUrl);
+      templateUrl = this.resourceLoader.resolve(evalTemplateUrl, containingFile);
+      templateStr = this.resourceLoader.load(templateUrl);
+      if (!tsSourceMapBug29300Fixed()) {
+        // By removing the template URL we are telling the translator not to try to
+        // map the external source file to the generated code, since the version
+        // of TS that is running does not support it.
+        templateUrl = '';
+      }
     } else if (component.has('template')) {
       const templateExpr = component.get('template') !;
-      const resolvedTemplate = this.evaluator.evaluate(templateExpr);
-      if (typeof resolvedTemplate !== 'string') {
-        throw new FatalDiagnosticError(
-            ErrorCode.VALUE_HAS_WRONG_TYPE, templateExpr, 'template must be a string');
+      // We only support SourceMaps for inline templates that are simple string literals.
+      if (ts.isStringLiteral(templateExpr) || ts.isNoSubstitutionTemplateLiteral(templateExpr)) {
+        // the start and end of the `templateExpr` node includes the quotation marks, which we must
+        // strip
+        templateRange = getTemplateRange(templateExpr);
+        templateStr = templateExpr.getSourceFile().text;
+        templateUrl = relativeContextFilePath;
+        escapedString = true;
+      } else {
+        const resolvedTemplate = this.evaluator.evaluate(templateExpr);
+        if (typeof resolvedTemplate !== 'string') {
+          throw new FatalDiagnosticError(
+              ErrorCode.VALUE_HAS_WRONG_TYPE, templateExpr, 'template must be a string');
+        }
+        templateStr = resolvedTemplate;
       }
-      templateStr = resolvedTemplate;
     } else {
       throw new FatalDiagnosticError(
           ErrorCode.COMPONENT_MISSING_TEMPLATE, decorator.node, 'component is missing a template');
@@ -157,18 +190,6 @@ export class ComponentDecoratorHandler implements
         new WrappedNodeExpr(component.get('viewProviders') !) :
         null;
 
-    // Go through the root directories for this project, and select the one with the smallest
-    // relative path representation.
-    const filePath = node.getSourceFile().fileName;
-    const relativeContextFilePath = this.rootDirs.reduce<string|undefined>((previous, rootDir) => {
-      const candidate = path.posix.relative(rootDir, filePath);
-      if (previous === undefined || candidate.length < previous.length) {
-        return candidate;
-      } else {
-        return previous;
-      }
-    }, undefined) !;
-
     let interpolation: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG;
     if (component.has('interpolation')) {
       const expr = component.get('interpolation') !;
@@ -182,9 +203,11 @@ export class ComponentDecoratorHandler implements
       interpolation = InterpolationConfig.fromArray(value as[string, string]);
     }
 
-    const template = parseTemplate(
-        templateStr, `${node.getSourceFile().fileName}#${node.name!.text}/template.html`,
-        {preserveWhitespaces, interpolationConfig: interpolation});
+    const template = parseTemplate(templateStr, templateUrl, {
+      preserveWhitespaces,
+      interpolationConfig: interpolation,
+      range: templateRange, escapedString
+    });
     if (template.errors !== undefined) {
       throw new Error(
           `Errors parsing template: ${template.errors.map(e => e.toString()).join(', ')}`);
@@ -402,3 +425,15 @@ export class ComponentDecoratorHandler implements
     return this.cycleAnalyzer.wouldCreateCycle(origin, imported);
   }
 }
+
+function getTemplateRange(templateExpr: ts.Expression) {
+  const startPos = templateExpr.getStart() + 1;
+  const {line, character} =
+      ts.getLineAndCharacterOfPosition(templateExpr.getSourceFile(), startPos);
+  return {
+    startPos,
+    startLine: line,
+    startCol: character,
+    endPos: templateExpr.getEnd() - 1,
+  };
+}
diff --git a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts
@@ -91,6 +91,7 @@ export function translateType(type: Type, imports: ImportManager): ts.TypeNode {
 }
 
 class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor {
+  private externalSourceFiles = new Map<string, ts.SourceMapSource>();
   constructor(private imports: ImportManager) {}
 
   visitDeclareVarStmt(stmt: DeclareVarStmt, context: Context): ts.VariableStatement {
@@ -153,7 +154,9 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor
   }
 
   visitReadVarExpr(ast: ReadVarExpr, context: Context): ts.Identifier {
-    return ts.createIdentifier(ast.name !);
+    const identifier = ts.createIdentifier(ast.name !);
+    this.setSourceMapRange(identifier, ast);
+    return identifier;
   }
 
   visitWriteVarExpr(expr: WriteVarExpr, context: Context): ts.Expression {
@@ -175,9 +178,11 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor
 
   visitInvokeMethodExpr(ast: InvokeMethodExpr, context: Context): ts.CallExpression {
     const target = ast.receiver.visitExpression(this, context);
-    return ts.createCall(
+    const call = ts.createCall(
         ast.name !== null ? ts.createPropertyAccess(target, ast.name) : target, undefined,
         ast.args.map(arg => arg.visitExpression(this, context)));
+    this.setSourceMapRange(call, ast);
+    return call;
   }
 
   visitInvokeFunctionExpr(ast: InvokeFunctionExpr, context: Context): ts.CallExpression {
@@ -187,6 +192,7 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor
     if (ast.pure) {
       ts.addSyntheticLeadingComment(expr, ts.SyntaxKind.MultiLineCommentTrivia, '@__PURE__', false);
     }
+    this.setSourceMapRange(expr, ast);
     return expr;
   }
 
@@ -197,13 +203,16 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor
   }
 
   visitLiteralExpr(ast: LiteralExpr, context: Context): ts.Expression {
+    let expr: ts.Expression;
     if (ast.value === undefined) {
-      return ts.createIdentifier('undefined');
+      expr = ts.createIdentifier('undefined');
     } else if (ast.value === null) {
-      return ts.createNull();
+      expr = ts.createNull();
     } else {
-      return ts.createLiteral(ast.value);
+      expr = ts.createLiteral(ast.value);
     }
+    this.setSourceMapRange(expr, ast);
+    return expr;
   }
 
   visitExternalExpr(ast: ExternalExpr, context: Context): ts.PropertyAccessExpression
@@ -269,15 +278,20 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor
   }
 
   visitLiteralArrayExpr(ast: LiteralArrayExpr, context: Context): ts.ArrayLiteralExpression {
-    return ts.createArrayLiteral(ast.entries.map(expr => expr.visitExpression(this, context)));
+    const expr =
+        ts.createArrayLiteral(ast.entries.map(expr => expr.visitExpression(this, context)));
+    this.setSourceMapRange(expr, ast);
+    return expr;
   }
 
   visitLiteralMapExpr(ast: LiteralMapExpr, context: Context): ts.ObjectLiteralExpression {
     const entries = ast.entries.map(
         entry => ts.createPropertyAssignment(
             entry.quoted ? ts.createLiteral(entry.key) : ts.createIdentifier(entry.key),
             entry.value.visitExpression(this, context)));
-    return ts.createObjectLiteral(entries);
+    const expr = ts.createObjectLiteral(entries);
+    this.setSourceMapRange(expr, ast);
+    return expr;
   }
 
   visitCommaExpr(ast: CommaExpr, context: Context): never {
@@ -289,6 +303,20 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor
   visitTypeofExpr(ast: TypeofExpr, context: Context): ts.TypeOfExpression {
     return ts.createTypeOf(ast.expr.visitExpression(this, context));
   }
+
+  private setSourceMapRange(expr: ts.Expression, ast: Expression) {
+    if (ast.sourceSpan) {
+      const {start, end} = ast.sourceSpan;
+      const {url, content} = start.file;
+      if (url) {
+        if (!this.externalSourceFiles.has(url)) {
+          this.externalSourceFiles.set(url, ts.createSourceMapSource(url, content, pos => pos));
+        }
+        const source = this.externalSourceFiles.get(url);
+        ts.setSourceMapRange(expr, {pos: start.offset, end: end.offset, source});
+      }
+    }
+  }
 }
 
 export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor {

diff --git a/packages/compiler-cli/src/ngtsc/util/src/ts_source_map_bug_29300.ts b/packages/compiler-cli/src/ngtsc/util/src/ts_source_map_bug_29300.ts
@@ -0,0 +1,68 @@
+/**
+ * @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 * as ts from 'typescript';
+
+let _tsSourceMapBug29300Fixed: boolean|undefined;
+
+/**
+ * Test the current version of TypeScript to see if it has fixed the external SourceMap
+ * file bug: https://github.com/Microsoft/TypeScript/issues/29300.
+ *
+ * The bug is fixed in TS 3.3+ but this check avoid us having to rely upon the version number,
+ * and allows us to gracefully fail if the TS version still has the bug.
+ *
+ * We check for the bug by compiling a very small program `a;` and transforming it to `b;`,
+ * where we map the new `b` identifier to an external source file, which has different lines to
+ * the original source file.  If the bug is fixed then the output SourceMap should contain
+ * mappings that correspond ot the correct line/col pairs for this transformed node.
+ *
+ * @returns true if the bug is fixed.
+ */
+export function tsSourceMapBug29300Fixed() {
+  if (_tsSourceMapBug29300Fixed === undefined) {
+    let writtenFiles: {[filename: string]: string} = {};
+    const sourceFile =
+        ts.createSourceFile('test.ts', 'a;', ts.ScriptTarget.ES2015, true, ts.ScriptKind.TS);
+    const host = {
+      getSourceFile(): ts.SourceFile | undefined{return sourceFile;},
+      fileExists(): boolean{return true;},
+      readFile(): string | undefined{return '';},
+      writeFile(fileName: string, data: string) { writtenFiles[fileName] = data; },
+      getDefaultLibFileName(): string{return '';},
+      getCurrentDirectory(): string{return '';},
+      getDirectories(): string[]{return [];},
+      getCanonicalFileName(): string{return '';},
+      useCaseSensitiveFileNames(): boolean{return true;},
+      getNewLine(): string{return '\n';},
+    };
+
+    const transform = (context: ts.TransformationContext) => {
+      return (node: ts.SourceFile) => ts.visitNode(node, visitor);
+      function visitor(node: ts.Node): ts.Node {
+        if (ts.isIdentifier(node) && node.text === 'a') {
+          const newNode = ts.createIdentifier('b');
+          ts.setSourceMapRange(newNode, {
+            pos: 16,
+            end: 16,
+            source: ts.createSourceMapSource('test.html', 'abc\ndef\nghi\njkl\nmno\npqr')
+          });
+          return newNode;
+        }
+        return ts.visitEachChild(node, visitor, context);
+      }
+    };
+
+    const program = ts.createProgram(['test.ts'], {sourceMap: true}, host);
+    program.emit(sourceFile, undefined, undefined, undefined, {after: [transform]});
+    // The first two mappings in the source map should look like:
+    // [0,1,4,0] col 0 => source file 1, row 4, column 0)
+    // [1,0,0,0] col 1 => source file 1, row 4, column 0)
+    _tsSourceMapBug29300Fixed = /ACIA,CAAA/.test(writtenFiles['test.js.map']);
+  }
+  return _tsSourceMapBug29300Fixed;
+}
diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts
@@ -41,7 +41,8 @@ const extract = (from: string, regex: any, transformFn: (match: any[]) => any) =
 const verifyTranslationIds =
     (source: string, output: string, exceptions = {},
      interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG) => {
-      const parseResult = htmlParser.parse(source, 'path:://to/template', true);
+      const parseResult =
+          htmlParser.parse(source, 'path:://to/template', {tokenizeExpansionForms: true});
       const extractedIdToMsg = new Map<string, any>();
       const extractedIds = new Set<string>();
       const generatedIds = new Set<string>();

diff --git a/packages/compiler-cli/test/diagnostics/mocks.ts b/packages/compiler-cli/test/diagnostics/mocks.ts
@@ -157,12 +157,7 @@ export class DiagnosticContext {
       };
       const urlResolver = createOfflineCompileUrlResolver();
       const htmlParser = new class extends HtmlParser {
-        parse(
-            source: string, url: string, parseExpansionForms: boolean = false,
-            interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG):
-            ParseTreeResult {
-          return new ParseTreeResult([], []);
-        }
+        parse(): ParseTreeResult { return new ParseTreeResult([], []); }
       };
 
       // This tracks the CompileConfig in codegen.ts. Currently these options
@@ -209,7 +204,7 @@ function compileTemplate(context: DiagnosticContext, type: StaticSymbol, templat
     const parser = new TemplateParser(
         config, context.reflector, expressionParser, new DomElementSchemaRegistry(), htmlParser,
         null !, []);
-    const htmlResult = htmlParser.parse(template, '', true);
+    const htmlResult = htmlParser.parse(template, '', {tokenizeExpansionForms: true});
     const analyzedModules = context.analyzedModules;
     // let errors: Diagnostic[]|undefined = undefined;
     let ngModule = analyzedModules.ngModuleByPipeOrDirective.get(type);

diff --git a/packages/compiler-cli/test/ngtsc/BUILD.bazel b/packages/compiler-cli/test/ngtsc/BUILD.bazel
@@ -8,7 +8,10 @@ ts_library(
         "//packages/compiler",
         "//packages/compiler-cli",
         "//packages/compiler-cli/src/ngtsc/routing",
+        "//packages/compiler-cli/src/ngtsc/util",
         "//packages/compiler-cli/test:test_utils",
+        "@ngdeps//@types/source-map",
+        "@ngdeps//source-map",
         "@ngdeps//typescript",
     ],
 )