feat(compiler): Propagate source span and value span to Variable AST

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

@@ -1331,7 +1331,7 @@ export declare class AnimationEvent {
       const diags = env.driveDiagnostics();
       expect(diags.length).toEqual(1);
       expect(diags[0].code).toEqual(ngErrorCode(ErrorCode.DUPLICATE_VARIABLE_DECLARATION));
-      expect(getSourceCodeForDiagnostic(diags[0])).toContain('let i of items;');
+      expect(getSourceCodeForDiagnostic(diags[0])).toContain('let i = index');
     });
 
     it('should still type-check when fileToModuleName aliasing is enabled, but alias exports are not in the .d.ts file',

packages/compiler/src/expression_parser/ast.ts

@@ -743,8 +743,14 @@ export class ParsedEvent {
       public handlerSpan: ParseSourceSpan) {}
 }
 
+/**
+ * ParsedVariable represents a variable declaration in a microsyntax expression.
+ */
 export class ParsedVariable {
-  constructor(public name: string, public value: string, public sourceSpan: ParseSourceSpan) {}
+  constructor(
+      public readonly name: string, public readonly value: string,
+      public readonly sourceSpan: ParseSourceSpan, public readonly keySpan: ParseSourceSpan,
+      public readonly valueSpan?: ParseSourceSpan) {}
 }
 
 export const enum BindingType {

packages/compiler/src/render3/r3_template_transform.ts

@@ -161,8 +161,8 @@ class HtmlAstToIvyAst implements html.Visitor {
         this.bindingParser.parseInlineTemplateBinding(
             templateKey, templateValue, attribute.sourceSpan, absoluteValueOffset, [],
             templateParsedProperties, parsedVariables);
-        templateVariables.push(
-            ...parsedVariables.map(v => new t.Variable(v.name, v.value, v.sourceSpan)));
+        templateVariables.push(...parsedVariables.map(
+            v => new t.Variable(v.name, v.value, v.sourceSpan, v.valueSpan)));
       } else {
         // Check for variables, events, property bindings, interpolation
         hasBinding = this.parseAttribute(

packages/compiler/src/template_parser/binding_parser.ts

@@ -8,11 +8,11 @@
 
 import {CompileDirectiveSummary, CompilePipeSummary} from '../compile_metadata';
 import {SecurityContext} from '../core';
-import {ASTWithSource, BindingPipe, BindingType, BoundElementProperty, EmptyExpr, ParsedEvent, ParsedEventType, ParsedProperty, ParsedPropertyType, ParsedVariable, ParserError, RecursiveAstVisitor, TemplateBinding, VariableBinding} from '../expression_parser/ast';
+import {ASTWithSource, AbsoluteSourceSpan, BindingPipe, BindingType, BoundElementProperty, EmptyExpr, ParsedEvent, ParsedEventType, ParsedProperty, ParsedPropertyType, ParsedVariable, ParserError, RecursiveAstVisitor, TemplateBinding, VariableBinding} from '../expression_parser/ast';
 import {Parser} from '../expression_parser/parser';
 import {InterpolationConfig} from '../ml_parser/interpolation_config';
 import {mergeNsAndName} from '../ml_parser/tags';
-import {ParseError, ParseErrorLevel, ParseSourceSpan} from '../parse_util';
+import {ParseError, ParseErrorLevel, ParseLocation, ParseSourceSpan} from '../parse_util';
 import {ElementSchemaRegistry} from '../schema/element_schema_registry';
 import {CssSelector} from '../selector';
 import {splitAtColon, splitAtPeriod} from '../util';
@@ -21,7 +21,7 @@ const PROPERTY_PARTS_SEPARATOR = '.';
 const ATTRIBUTE_PREFIX = 'attr';
 const CLASS_PREFIX = 'class';
 const STYLE_PREFIX = 'style';
-
+const TEMPLATE_ATTR_PREFIX = '*';
 const ANIMATE_PROP_PREFIX = 'animate-';
 
 /**
@@ -129,16 +129,21 @@ export class BindingParser {
       tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, absoluteValueOffset: number,
       targetMatchableAttrs: string[][], targetProps: ParsedProperty[],
       targetVars: ParsedVariable[]) {
-    const absoluteKeyOffset = sourceSpan.start.offset;
+    const absoluteKeyOffset = sourceSpan.start.offset + TEMPLATE_ATTR_PREFIX.length;
     const bindings = this._parseTemplateBindings(
         tplKey, tplValue, sourceSpan, absoluteKeyOffset, absoluteValueOffset);
 
-    for (let i = 0; i < bindings.length; i++) {
-      const binding = bindings[i];
+    for (const binding of bindings) {
+      // sourceSpan is for the entire HTML attribute. bindingSpan is for a particular
+      // binding within the microsyntax expression so it's more narrow than sourceSpan.
+      const bindingSpan = moveParseSourceSpan(sourceSpan, binding.sourceSpan);
       const key = binding.key.source;
+      const keySpan = moveParseSourceSpan(sourceSpan, binding.key.span);
       if (binding instanceof VariableBinding) {
         const value = binding.value ? binding.value.source : '$implicit';
-        targetVars.push(new ParsedVariable(key, value, sourceSpan));
+        const valueSpan =
+            binding.value ? moveParseSourceSpan(sourceSpan, binding.value.span) : undefined;
+        targetVars.push(new ParsedVariable(key, value, bindingSpan, keySpan, valueSpan));
       } else if (binding.value) {
         this._parsePropertyAst(
             key, binding.value, sourceSpan, undefined, targetMatchableAttrs, targetProps);
@@ -518,3 +523,18 @@ export function calcPossibleSecurityContexts(
   });
   return ctxs.length === 0 ? [SecurityContext.NONE] : Array.from(new Set(ctxs)).sort();
 }
+
+/**
+ * Compute a new ParseSourceSpan based off an original `sourceSpan` by using
+ * absolute offsets from the specified `absoluteSpan`.
+ *
+ * @param sourceSpan original source span
+ * @param absoluteSpan absolute source span to move to
+ */
+function moveParseSourceSpan(
+    sourceSpan: ParseSourceSpan, absoluteSpan: AbsoluteSourceSpan): ParseSourceSpan {
+  // The difference of two absolute offsets provide the relative offset
+  const startDiff = absoluteSpan.start - sourceSpan.start.offset;
+  const endDiff = absoluteSpan.end - sourceSpan.end.offset;
+  return new ParseSourceSpan(sourceSpan.start.moveBy(startDiff), sourceSpan.end.moveBy(endDiff));
+}

packages/compiler/src/template_parser/template_ast.ts

@@ -161,10 +161,12 @@ export class ReferenceAst implements TemplateAst {
  * A variable declaration on a <ng-template> (e.g. `var-someName="someLocalName"`).
  */
 export class VariableAst implements TemplateAst {
-  constructor(public name: string, public value: string, public sourceSpan: ParseSourceSpan) {}
+  constructor(
+      public readonly name: string, public readonly value: string,
+      public readonly sourceSpan: ParseSourceSpan, public readonly valueSpan?: ParseSourceSpan) {}
 
   static fromParsedVariable(v: ParsedVariable) {
-    return new VariableAst(v.name, v.value, v.sourceSpan);
+    return new VariableAst(v.name, v.value, v.sourceSpan, v.valueSpan);
   }
 
   visit(visitor: TemplateAstVisitor, context: any): any {

packages/compiler/test/render3/r3_ast_spans_spec.ts

@@ -233,7 +233,7 @@ describe('R3 AST source spans', () => {
         ['Template', '0:32', '0:32', '32:38'],
         ['TextAttribute', '5:31', '<empty>'],
         ['BoundAttribute', '5:31', '<empty>'],
-        ['Variable', '5:31', '<empty>'],
+        ['Variable', '13:22', '<empty>'],  // let item
         ['Element', '0:38', '0:32', '32:38'],
       ]);
 
@@ -255,7 +255,7 @@ describe('R3 AST source spans', () => {
       expectFromHtml('<div *ngIf="let a=b"></div>').toEqual([
         ['Template', '0:21', '0:21', '21:27'],
         ['TextAttribute', '5:20', '<empty>'],
-        ['Variable', '5:20', '<empty>'],
+        ['Variable', '12:19', '18:19'],  // let a=b -> b
         ['Element', '0:27', '0:21', '21:27'],
       ]);
     });
@@ -264,7 +264,7 @@ describe('R3 AST source spans', () => {
       expectFromHtml('<div *ngIf="expr as local"></div>').toEqual([
         ['Template', '0:27', '0:27', '27:33'],
         ['BoundAttribute', '5:26', '<empty>'],
-        ['Variable', '5:26', '<empty>'],
+        ['Variable', '6:25', '6:10'],  // ngIf="expr as local -> ngIf
         ['Element', '0:33', '0:27', '27:33'],
       ]);
     });

packages/language-service/test/diagnostics_spec.ts

@@ -292,7 +292,7 @@ describe('diagnostics', () => {
     it('should suggest refining a template context missing a property', () => {
       mockHost.override(
           TEST_TEMPLATE,
-          `<button type="button" ~{start-emb}*counter="let hero of heroes"~{end-emb}></button>`);
+          `<button type="button" *counter="~{start-emb}let hero ~{end-emb}of heroes"></button>`);
       const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
       expect(diags.length).toBe(1);
       const {messageText, start, length, category} = diags[0];
@@ -310,7 +310,7 @@ describe('diagnostics', () => {
     it('should report an unknown context reference', () => {
       mockHost.override(
           TEST_TEMPLATE,
-          `<div ~{start-emb}*ngFor="let hero of heroes; let e = even_1"~{end-emb}></div>`);
+          `<div *ngFor="let hero of heroes; ~{start-emb}let e = even_1~{end-emb}"></div>`);
       const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
       expect(diags.length).toBe(1);
       const {messageText, start, length, category} = diags[0];