Add a barebone type checker for the expression parser (#14226)

* Add a barebone type checker for the expression parser

* Add more context and other review feedback

frontend/src/metabase/lib/expressions/parser.js

@@ -25,6 +25,7 @@ import {
 } from "./lexer";
 
 import { isExpressionType, getFunctionArgType } from ".";
+import { typeCheck } from "./typechecker";
 
 export class ExpressionParser extends CstParser {
   constructor(config = {}) {
@@ -457,6 +458,7 @@ export function parse({
   if (parserErrors.length > 0 && !recover) {
     throw parserErrors;
   }
+  typeCheck(cst, startRule || "expression");
   const parserRecovered = !!(cst && parserErrors.length > 0);
 
   return {

frontend/src/metabase/lib/expressions/typechecker.js

+import { ExpressionVisitor } from "./visitor";
+
+export function typeCheck(cst, rootType) {
+  class TypeChecker extends ExpressionVisitor {
+    constructor() {
+      super();
+      this.typeStack = [rootType];
+    }
+
+    expression(ctx) {
+      this.typeStack.unshift(rootType);
+      const result = super.expression(ctx);
+      this.typeStack.shift();
+      return result;
+    }
+    aggregation(ctx) {
+      this.typeStack.unshift("aggregation");
+      const result = super.aggregation(ctx);
+      this.typeStack.shift();
+      return result;
+    }
+    boolean(ctx) {
+      this.typeStack.unshift("boolean");
+      const result = super.boolean(ctx);
+      this.typeStack.shift();
+      return result;
+    }
+
+    // TODO check for matching argument signature
+    functionExpression(ctx) {
+      const args = ctx.arguments || [];
+      return args.map(arg => {
+        this.typeStack.unshift("expression");
+        const result = this.visit(arg);
+        this.typeStack.unshift();
+        return result;
+      });
+    }
+
+    metricExpression(ctx) {
+      const type = this.typeStack[0];
+      if (type !== "aggregation" && type !== "expression") {
+        throw new Error("Incorrect type for metric");
+      }
+      return super.metricExpression(ctx);
+    }
+
+    segmentExpression(ctx) {
+      const type = this.typeStack[0];
+      if (type !== "boolean") {
+        throw new Error("Incorrect type for segment");
+      }
+      return super.segmentExpression(ctx);
+    }
+
+    dimensionExpression(ctx) {
+      const type = this.typeStack[0];
+      if (type === "boolean" || type === "aggregation") {
+        throw new Error("Incorrect type for dimension");
+      }
+      return super.dimensionExpression(ctx);
+    }
+
+    booleanExpression(ctx) {
+      const type = ctx.operands.length > 1 ? rootType : this.typeStack[0];
+      this.typeStack.unshift(type);
+      const result = super.booleanExpression(ctx);
+      this.typeStack.shift();
+      return result;
+    }
+    comparisonExpression(ctx) {
+      this.typeStack.unshift("expression");
+      const result = super.comparisonExpression(ctx);
+      this.typeStack.shift();
+      return result;
+    }
+    booleanUnaryExpression(ctx) {
+      const type = ctx.operands.length > 1 ? rootType : this.typeStack[0];
+      this.typeStack.unshift(type);
+      const result = super.booleanUnaryExpression(ctx);
+      this.typeStack.shift();
+      return result;
+    }
+  }
+  new TypeChecker().visit(cst);
+}

frontend/test/metabase/lib/expressions/typechecker.unit.spec.js

+import { parse } from "metabase/lib/expressions/parser";
+import { ExpressionVisitor } from "metabase/lib/expressions/visitor";
+import { parseIdentifierString } from "metabase/lib/expressions/index";
+
+// Since the type checking is inserted as the last stage in the expression parser,
+// the whole tests must continue to pass (i.e. none of them should thrown
+// an exception) to assert that type checker works correctly.
+
+describe("type-checker", () => {
+  function parseSource(source, startRule) {
+    let cst = null;
+    try {
+      cst = parse({ source, tokenVector: null, startRule }).cst;
+    } catch (e) {
+      let err = e;
+      if (err.length && err.length > 0) {
+        err = err[0];
+        if (typeof err.message === "string") {
+          err = err.message;
+        }
+      }
+      throw err;
+    }
+    return cst;
+  }
+
+  function collect(source, startRule) {
+    class Collector extends ExpressionVisitor {
+      constructor() {
+        super();
+        this.metrics = [];
+        this.segments = [];
+        this.dimensions = [];
+      }
+      identifier(ctx) {
+        return ctx.Identifier[0].image;
+      }
+      identifierString(ctx) {
+        return parseIdentifierString(ctx.IdentifierString[0].image);
+      }
+      metricExpression(ctx) {
+        this.metrics.push(this.visit(ctx.metricName));
+      }
+      segmentExpression(ctx) {
+        this.segments.push(this.visit(ctx.segmentName));
+      }
+      dimensionExpression(ctx) {
+        this.dimensions.push(this.visit(ctx.dimensionName));
+      }
+    }
+    const tree = parseSource(source, startRule);
+    const collector = new Collector();
+    collector.visit(tree);
+    return collector;
+  }
+
+  describe("for an expression", () => {
+    function expr(source) {
+      return collect(source, "expression");
+    }
+    it("should resolve dimensions correctly", () => {
+      expect(expr("[Price]+[Tax]").dimensions).toEqual(["Price", "Tax"]);
+      expect(expr("ABS([Discount])").dimensions).toEqual(["Discount"]);
+      expect(expr("CASE([Deal],10,20)").dimensions).toEqual([]);
+    });
+
+    it("should resolve segments correctly", () => {
+      expect(expr("[Price]+[Tax]").segments).toEqual([]);
+      expect(expr("ABS([Discount])").segments).toEqual([]);
+      expect(expr("CASE([Deal],10,20)").segments).toEqual(["Deal"]);
+    });
+
+    it("should resolve dimensions and segments correctly", () => {
+      expect(expr("[X]+CASE([Y],4,5)").dimensions).toEqual(["X"]);
+      expect(expr("[X]+CASE([Y],4,5)").segments).toEqual(["Y"]);
+    });
+  });
+
+  describe("for a filter", () => {
+    function filter(source) {
+      return collect(source, "boolean");
+    }
+    it("should resolve segments correctly", () => {
+      expect(filter("[Clearance]").segments).toEqual(["Clearance"]);
+      expect(filter("NOT [Deal]").segments).toEqual(["Deal"]);
+      expect(filter("NOT NOT [Deal]").segments).toEqual(["Deal"]);
+      expect(filter("P > 3").segments).toEqual([]);
+      expect(filter("R<1 AND [S]>4").segments).toEqual([]);
+      expect(filter("Between([BIG],3,7)").segments).toEqual([]);
+      expect(filter("Contains([GI],'Joe')").segments).toEqual([]);
+    });
+
+    it("should resolve dimensions correctly", () => {
+      expect(filter("[Clearance]").dimensions).toEqual([]);
+      expect(filter("NOT [Deal]").dimensions).toEqual([]);
+      expect(filter("NOT NOT [Deal]").dimensions).toEqual([]);
+      expect(filter("P > 3").dimensions).toEqual(["P"]);
+      expect(filter("R<1 AND [S]>4").dimensions).toEqual(["R", "S"]);
+      expect(filter("Between([BIG],3,7)").dimensions).toEqual(["BIG"]);
+      expect(filter("Contains([GI],'Joe')").dimensions).toEqual(["GI"]);
+    });
+
+    it("should resolve dimensions and segments correctly", () => {
+      expect(filter("[A] OR [B]>0").segments).toEqual(["A"]);
+      expect(filter("[A] OR [B]>0").dimensions).toEqual(["B"]);
+      expect(filter("[X]=4 AND NOT [Y]").segments).toEqual(["Y"]);
+      expect(filter("[X]=4 AND NOT [Y]").dimensions).toEqual(["X"]);
+      expect(filter("T OR Between([R],0,9)").segments).toEqual(["T"]);
+      expect(filter("T OR Between([R],0,9)").dimensions).toEqual(["R"]);
+    });
+  });
+
+  describe("for an aggregation", () => {
+    function aggregation(source) {
+      return collect(source, "aggregation");
+    }
+    it("should resolve dimensions correctly", () => {
+      expect(aggregation("Sum([Discount])").dimensions).toEqual(["Discount"]);
+      expect(aggregation("5-Average([Rating])").dimensions).toEqual(["Rating"]);
+      expect(aggregation("Share(contains([P],'Q'))").dimensions).toEqual(["P"]);
+      expect(aggregation("CountIf([Tax]>13)").dimensions).toEqual(["Tax"]);
+      expect(aggregation("Sum([Total]*2)").dimensions).toEqual(["Total"]);
+      expect(aggregation("[Total]").dimensions).toEqual([]);
+    });
+
+    it("should resolve metrics correctly", () => {
+      expect(aggregation("Sum([Discount])").metrics).toEqual([]);
+      expect(aggregation("5-Average([Rating])").metrics).toEqual([]);
+      expect(aggregation("Share(contains([P],'Q'))").metrics).toEqual([]);
+      expect(aggregation("CountIf([Tax]>13)").metrics).toEqual([]);
+      expect(aggregation("Sum([Total]*2)").metrics).toEqual([]);
+      expect(aggregation("[Total]").metrics).toEqual(["Total"]);
+    });
+
+    it("should resolve dimensions and metrics correctly", () => {
+      expect(aggregation("[X]+Sum([Y])").dimensions).toEqual(["Y"]);
+      expect(aggregation("[X]+Sum([Y])").metrics).toEqual(["X"]);
+    });
+  });
+});