feat(compiler): add name spans for property reads and method calls (#36826)

ASTs for property read and method calls contain information about the entire span of the expression, including its receiver. Use cases like a language service and compile error messages may be more interested in the span of the direct identifier for which the expression is constructed (i.e. an accessed property). To support this, this commit adds a `nameSpan` property on - `PropertyRead`s - `SafePropertyRead`s - `PropertyWrite`s - `MethodCall`s - `SafeMethodCall`s The `nameSpan` property already existed for `BindingPipe`s. This commit also updates usages of these expressions' `sourceSpan`s in Ngtsc and the langauge service to use `nameSpan`s where appropriate. PR Close #36826
2020-04-27 18:54:30 -07:00
parent 1142c378fd
commit eb34aa551a
20 changed files with 496 additions and 199 deletions
--- a/packages/compiler/src/compiler_util/expression_converter.ts
+++ b/packages/compiler/src/compiler_util/expression_converter.ts
@ -665,14 +665,14 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
      this._nodeMap.set(
          leftMostSafe,
          new cdAst.MethodCall(
-              leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.receiver, leftMostSafe.name,
-              leftMostSafe.args));
+              leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.nameSpan,
+              leftMostSafe.receiver, leftMostSafe.name, leftMostSafe.args));
    } else {
      this._nodeMap.set(
          leftMostSafe,
          new cdAst.PropertyRead(
-              leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.receiver,
-              leftMostSafe.name));
+              leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.nameSpan,
+              leftMostSafe.receiver, leftMostSafe.name));
    }

    // Recursively convert the node now without the guarded member access.
--- a/packages/compiler/src/expression_parser/ast.ts
+++ b/packages/compiler/src/expression_parser/ast.ts
@ -39,6 +39,13 @@ export class AST {
  }
 }

+export abstract class ASTWithName extends AST {
+  constructor(
+      span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public nameSpan: AbsoluteSourceSpan) {
+    super(span, sourceSpan);
+  }
+}
+
 /**
 * Represents a quoted expression of the form:
 *
@ -101,31 +108,33 @@ export class Conditional extends AST {
  }
 }

-export class PropertyRead extends AST {
+export class PropertyRead extends ASTWithName {
  constructor(
-      span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string) {
-    super(span, sourceSpan);
+      span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan,
+      public receiver: AST, public name: string) {
+    super(span, sourceSpan, nameSpan);
  }
  visit(visitor: AstVisitor, context: any = null): any {
    return visitor.visitPropertyRead(this, context);
  }
 }

-export class PropertyWrite extends AST {
+export class PropertyWrite extends ASTWithName {
  constructor(
-      span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string,
-      public value: AST) {
-    super(span, sourceSpan);
+      span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan,
+      public receiver: AST, public name: string, public value: AST) {
+    super(span, sourceSpan, nameSpan);
  }
  visit(visitor: AstVisitor, context: any = null): any {
    return visitor.visitPropertyWrite(this, context);
  }
 }

-export class SafePropertyRead extends AST {
+export class SafePropertyRead extends ASTWithName {
  constructor(
-      span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string) {
-    super(span, sourceSpan);
+      span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan,
+      public receiver: AST, public name: string) {
+    super(span, sourceSpan, nameSpan);
  }
  visit(visitor: AstVisitor, context: any = null): any {
    return visitor.visitSafePropertyRead(this, context);
@ -152,11 +161,11 @@ export class KeyedWrite extends AST {
  }
 }

-export class BindingPipe extends AST {
+export class BindingPipe extends ASTWithName {
  constructor(
      span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public exp: AST, public name: string,
-      public args: any[], public nameSpan: AbsoluteSourceSpan) {
-    super(span, sourceSpan);
+      public args: any[], nameSpan: AbsoluteSourceSpan) {
+    super(span, sourceSpan, nameSpan);
  }
  visit(visitor: AstVisitor, context: any = null): any {
    return visitor.visitPipe(this, context);
@ -236,22 +245,22 @@ export class NonNullAssert extends AST {
  }
 }

-export class MethodCall extends AST {
+export class MethodCall extends ASTWithName {
  constructor(
-      span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string,
-      public args: any[]) {
-    super(span, sourceSpan);
+      span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan,
+      public receiver: AST, public name: string, public args: any[]) {
+    super(span, sourceSpan, nameSpan);
  }
  visit(visitor: AstVisitor, context: any = null): any {
    return visitor.visitMethodCall(this, context);
  }
 }

-export class SafeMethodCall extends AST {
+export class SafeMethodCall extends ASTWithName {
  constructor(
-      span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public name: string,
-      public args: any[]) {
-    super(span, sourceSpan);
+      span: ParseSpan, sourceSpan: AbsoluteSourceSpan, nameSpan: AbsoluteSourceSpan,
+      public receiver: AST, public name: string, public args: any[]) {
+    super(span, sourceSpan, nameSpan);
  }
  visit(visitor: AstVisitor, context: any = null): any {
    return visitor.visitSafeMethodCall(this, context);
@ -478,26 +487,31 @@ export class AstTransformer implements AstVisitor {
  }

  visitPropertyRead(ast: PropertyRead, context: any): AST {
-    return new PropertyRead(ast.span, ast.sourceSpan, ast.receiver.visit(this), ast.name);
+    return new PropertyRead(
+        ast.span, ast.sourceSpan, ast.nameSpan, ast.receiver.visit(this), ast.name);
  }

  visitPropertyWrite(ast: PropertyWrite, context: any): AST {
    return new PropertyWrite(
-        ast.span, ast.sourceSpan, ast.receiver.visit(this), ast.name, ast.value.visit(this));
+        ast.span, ast.sourceSpan, ast.nameSpan, ast.receiver.visit(this), ast.name,
+        ast.value.visit(this));
  }

  visitSafePropertyRead(ast: SafePropertyRead, context: any): AST {
-    return new SafePropertyRead(ast.span, ast.sourceSpan, ast.receiver.visit(this), ast.name);
+    return new SafePropertyRead(
+        ast.span, ast.sourceSpan, ast.nameSpan, ast.receiver.visit(this), ast.name);
  }

  visitMethodCall(ast: MethodCall, context: any): AST {
    return new MethodCall(
-        ast.span, ast.sourceSpan, ast.receiver.visit(this), ast.name, this.visitAll(ast.args));
+        ast.span, ast.sourceSpan, ast.nameSpan, ast.receiver.visit(this), ast.name,
+        this.visitAll(ast.args));
  }

  visitSafeMethodCall(ast: SafeMethodCall, context: any): AST {
    return new SafeMethodCall(
-        ast.span, ast.sourceSpan, ast.receiver.visit(this), ast.name, this.visitAll(ast.args));
+        ast.span, ast.sourceSpan, ast.nameSpan, ast.receiver.visit(this), ast.name,
+        this.visitAll(ast.args));
  }

  visitFunctionCall(ast: FunctionCall, context: any): AST {
@ -586,7 +600,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor {
  visitPropertyRead(ast: PropertyRead, context: any): AST {
    const receiver = ast.receiver.visit(this);
    if (receiver !== ast.receiver) {
-      return new PropertyRead(ast.span, ast.sourceSpan, receiver, ast.name);
+      return new PropertyRead(ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name);
    }
    return ast;
  }
@ -595,7 +609,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor {
    const receiver = ast.receiver.visit(this);
    const value = ast.value.visit(this);
    if (receiver !== ast.receiver || value !== ast.value) {
-      return new PropertyWrite(ast.span, ast.sourceSpan, receiver, ast.name, value);
+      return new PropertyWrite(ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name, value);
    }
    return ast;
  }
@ -603,7 +617,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor {
  visitSafePropertyRead(ast: SafePropertyRead, context: any): AST {
    const receiver = ast.receiver.visit(this);
    if (receiver !== ast.receiver) {
-      return new SafePropertyRead(ast.span, ast.sourceSpan, receiver, ast.name);
+      return new SafePropertyRead(ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name);
    }
    return ast;
  }
@ -612,7 +626,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor {
    const receiver = ast.receiver.visit(this);
    const args = this.visitAll(ast.args);
    if (receiver !== ast.receiver || args !== ast.args) {
-      return new MethodCall(ast.span, ast.sourceSpan, receiver, ast.name, args);
+      return new MethodCall(ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name, args);
    }
    return ast;
  }
@ -621,7 +635,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor {
    const receiver = ast.receiver.visit(this);
    const args = this.visitAll(ast.args);
    if (receiver !== ast.receiver || args !== ast.args) {
-      return new SafeMethodCall(ast.span, ast.sourceSpan, receiver, ast.name, args);
+      return new SafeMethodCall(ast.span, ast.sourceSpan, ast.nameSpan, receiver, ast.name, args);
    }
    return ast;
  }
--- a/packages/compiler/src/expression_parser/parser.ts
+++ b/packages/compiler/src/expression_parser/parser.ts
@ -305,9 +305,34 @@ export class _ParseAST {
    return this.peek(0);
  }

+  /** Whether all the parser input has been processed. */
+  get atEOF(): boolean {
+    return this.index >= this.tokens.length;
+  }
+
+  /**
+   * Index of the next token to be processed, or the end of the last token if all have been
+   * processed.
+   */
  get inputIndex(): number {
-    return (this.index < this.tokens.length) ? this.next.index + this.offset :
-                                               this.inputLength + this.offset;
+    return this.atEOF ? this.currentEndIndex : this.next.index + this.offset;
+  }
+
+  /**
+   * End index of the last processed token, or the start of the first token if none have been
+   * processed.
+   */
+  get currentEndIndex(): number {
+    if (this.index > 0) {
+      const curToken = this.peek(-1);
+      return curToken.end + this.offset;
+    }
+    // No tokens have been processed yet; return the next token's start or the length of the input
+    // if there is no token.
+    if (this.tokens.length === 0) {
+      return this.inputLength + this.offset;
+    }
+    return this.next.index + this.offset;
  }

  /**
@ -318,12 +343,7 @@ export class _ParseAST {
  }

  span(start: number) {
-    // `end` is either the
-    //   - end index of the current token
-    //   - start of the first token (this can happen e.g. when creating an implicit receiver)
-    const curToken = this.peek(-1);
-    const end = this.index > 0 ? curToken.end + this.offset : this.inputIndex;
-    return new ParseSpan(start, end);
+    return new ParseSpan(start, this.currentEndIndex);
  }

  sourceSpan(start: number): AbsoluteSourceSpan {
@ -730,7 +750,9 @@ export class _ParseAST {

  parseAccessMemberOrMethodCall(receiver: AST, isSafe: boolean = false): AST {
    const start = receiver.span.start;
+    const nameStart = this.inputIndex;
    const id = this.expectIdentifierOrKeyword();
+    const nameSpan = this.sourceSpan(nameStart);

    if (this.consumeOptionalCharacter(chars.$LPAREN)) {
      this.rparensExpected++;
@ -739,8 +761,8 @@ export class _ParseAST {
      this.rparensExpected--;
      const span = this.span(start);
      const sourceSpan = this.sourceSpan(start);
-      return isSafe ? new SafeMethodCall(span, sourceSpan, receiver, id, args) :
-                      new MethodCall(span, sourceSpan, receiver, id, args);
+      return isSafe ? new SafeMethodCall(span, sourceSpan, nameSpan, receiver, id, args) :
+                      new MethodCall(span, sourceSpan, nameSpan, receiver, id, args);

    } else {
      if (isSafe) {
@ -748,7 +770,8 @@ export class _ParseAST {
          this.error('The \'?.\' operator cannot be used in the assignment');
          return new EmptyExpr(this.span(start), this.sourceSpan(start));
        } else {
-          return new SafePropertyRead(this.span(start), this.sourceSpan(start), receiver, id);
+          return new SafePropertyRead(
+              this.span(start), this.sourceSpan(start), nameSpan, receiver, id);
        }
      } else {
        if (this.consumeOptionalOperator('=')) {
@ -758,9 +781,10 @@ export class _ParseAST {
          }

          const value = this.parseConditional();
-          return new PropertyWrite(this.span(start), this.sourceSpan(start), receiver, id, value);
+          return new PropertyWrite(
+              this.span(start), this.sourceSpan(start), nameSpan, receiver, id, value);
        } else {
-          return new PropertyRead(this.span(start), this.sourceSpan(start), receiver, id);
+          return new PropertyRead(this.span(start), this.sourceSpan(start), nameSpan, receiver, id);
        }
      }
    }
--- a/packages/compiler/src/render3/view/template.ts
+++ b/packages/compiler/src/render3/view/template.ts
@ -1428,7 +1428,7 @@ export class ValueConverter extends AstMemoryEfficientTransformer {
    // Allocate one slot for the result plus one slot per pipe argument
    const pureFunctionSlot = this.allocatePureFunctionSlots(2 + pipe.args.length);
    const target = new PropertyRead(
-        pipe.span, pipe.sourceSpan, new ImplicitReceiver(pipe.span, pipe.sourceSpan),
+        pipe.span, pipe.sourceSpan, pipe.nameSpan, new ImplicitReceiver(pipe.span, pipe.sourceSpan),
        slotPseudoLocal);
    const {identifier, isVarLength} = pipeBindingCallInfo(pipe.args);
    this.definePipe(pipe.name, slotPseudoLocal, slot, o.importExpr(identifier));