Computed expressions respect left-to-right associativity and operator precedence

packages/perspective-viewer/package.json

@@ -36,7 +36,7 @@
         "build": "npm-run-all --silent build:babel build:webpack:cjs build:webpack:umd",
         "watch": "webpack --color --watch --config src/config/view.config.js",
         "test:build": "cpx \"test/html/*\" dist/umd && cpx \"test/csv/*\" dist/umd && cpx \"test/css/*\" dist/umd",
-        "test:run": "jest --rootDir=. --config=../perspective-test/jest.config.js --silent --color 2>&1",
+        "test:run": "jest --rootDir=. --config=../perspective-test/jest.config.js --color 2>&1",
         "test": "npm-run-all test:build test:run",
         "clean": "rimraf dist",
         "clean:screenshots": "rimraf \"screenshots/**/*.@(failed|diff).png\"",
@@ -70,6 +70,7 @@
     },
     "devDependencies": {
         "@finos/perspective-test": "^0.5.0",
-        "@finos/perspective-webpack-plugin": "^0.5.0"
+        "@finos/perspective-webpack-plugin": "^0.5.0",
+        "jsverify": "^0.8.4"
     }
 }

packages/perspective-viewer/src/js/computed_expressions/lexer.js

@@ -459,7 +459,7 @@ export const clean_tokens = function(tokens) {
     const cleaned_tokens = [];
 
     for (const token of tokens) {
-        if (token.tokenType.name !== "whitespace") {
+        if (!tokenMatcher(token, Whitespace)) {
             cleaned_tokens.push(token);
         }
     }

packages/perspective-viewer/src/js/computed_expressions/parser.js

@@ -9,54 +9,119 @@
 import {CstParser} from "chevrotain";
 import {PerspectiveParserErrorMessage} from "./error";
 
+/**
+ * A Chevrotain expression parser that produces a concrete syntax tree
+ * representing every single parsed token.
+ */
 export class ComputedExpressionColumnParser extends CstParser {
     constructor(vocabulary) {
         super(vocabulary, {
             errorMessageProvider: PerspectiveParserErrorMessage
         });
 
+        /**
+         * The overarching rule - required so that one single array of
+         * parsed computed columns can be maintained across multiple operator,
+         * functional, and parenthetical expressions.
+         */
         this.RULE("SuperExpression", () => {
             this.SUBRULE(this.Expression);
         });
 
+        /**
+         * The `base rule` for all expressions, except that the state of the
+         * computed column array can be changed between each invocation of
+         * this rule.
+         */
         this.RULE("Expression", () => {
-            this.OR(
-                [
-                    {
-                        ALT: () => {
-                            this.SUBRULE(this.OperatorComputedColumn);
-                        }
-                    },
-                    {
-                        ALT: () => {
-                            this.SUBRULE(this.FunctionComputedColumn);
-                        }
-                    }
-                ],
-                {
-                    ERR_MSG: "Expected an expression of the form `x + y` or `func(x)`."
-                }
-            );
+            this.SUBRULE(this.OperatorComputedColumn);
         });
 
+        /**
+         * A computed column in `x + y` notation. Because it appears earlier,
+         * it has lower precedence compared to the rules that are to follow.
+         */
         this.RULE("OperatorComputedColumn", () => {
-            this.SUBRULE(this.ColumnName, {LABEL: "left"});
-            this.AT_LEAST_ONE(() => {
+            this.SUBRULE(this.AdditionOperatorComputedColumn, {LABEL: "left"});
+
+            // 0...n operators and right-hand expressions are available here.
+            // Though a single column name is syntactically valid, it does
+            // not actually generate any computed columns. However, this
+            // rule must allow for 0...n and not 1...n as it allows for
+            // a function-only expression (`sqrt("a")`) without a right
+            // hand side of the expression.
+            this.MANY(() => {
                 this.SUBRULE(this.Operator);
-                this.SUBRULE2(this.ColumnName, {LABEL: "right"});
+                this.SUBRULE2(this.AdditionOperatorComputedColumn, {LABEL: "right"});
+                this.OPTION(() => {
+                    this.SUBRULE(this.As, {LABEL: "as"});
+                });
             });
-            this.OPTION(() => {
-                this.SUBRULE(this.As, {LABEL: "as"});
+        });
+
+        /**
+         * A computed column in `x + y` or `x - y` notation. To maintain a
+         * notion of operator precedence, different rules must be created for
+         * add/subtract and multiply/divide operators, even if the actual
+         * evaluator logic is the same.
+         */
+        this.RULE("AdditionOperatorComputedColumn", () => {
+            this.SUBRULE(this.MultiplicationOperatorComputedColumn, {LABEL: "left"});
+            this.MANY(() => {
+                this.SUBRULE(this.AdditionOperator);
+                this.SUBRULE2(this.MultiplicationOperatorComputedColumn, {LABEL: "right"});
+                this.OPTION(() => {
+                    this.SUBRULE(this.As, {LABEL: "as"});
+                });
             });
         });
 
+        /**
+         * A computed column in `x * y` or `x / y` notation. Because it is
+         * defined after the addition and generic operators, it is evaluated
+         * before the addition/generic operators - hence satisfying precedence.
+         */
+        this.RULE("MultiplicationOperatorComputedColumn", () => {
+            this.SUBRULE(this.ExponentOperatorComputedColumn, {LABEL: "left"});
+            this.MANY(() => {
+                this.SUBRULE(this.MultiplicationOperator);
+                this.SUBRULE2(this.ExponentOperatorComputedColumn, {LABEL: "right"});
+                this.OPTION(() => {
+                    this.SUBRULE(this.As, {LABEL: "as"});
+                });
+            });
+        });
+
+        /**
+         * A computed column in `x ^ y` notation. Exponents are evaluated before
+         * multiplication/division and addition/subtraction, so it is defined
+         * after those rules to give itself precedence.
+         */
+        this.RULE("ExponentOperatorComputedColumn", () => {
+            this.SUBRULE(this.ColumnName, {LABEL: "left"});
+            this.MANY(() => {
+                this.SUBRULE(this.ExponentOperator);
+                this.SUBRULE2(this.ColumnName, {LABEL: "right"});
+                this.OPTION(() => {
+                    this.SUBRULE(this.As, {LABEL: "as"});
+                });
+            });
+        });
+
+        /**
+         * A computed column in `f(x)` notation. It is evaluated before all
+         * operator computed columns.
+         */
         this.RULE("FunctionComputedColumn", () => {
             this.SUBRULE(this.Function);
             this.CONSUME(vocabulary["leftParen"]);
+
             this.AT_LEAST_ONE_SEP({
                 SEP: vocabulary["comma"],
                 DEF: () => {
-                    this.SUBRULE(this.ColumnName);
+                    // Allow for arbitary expressions inside functions without
+                    // use of parentheses.
+                    this.SUBRULE(this.Expression, {LABEL: "param"});
                 }
             });
             this.CONSUME(vocabulary["rightParen"]);
@@ -65,21 +130,6 @@ export class ComputedExpressionColumnParser extends CstParser {
             });
         });
 
-        this.RULE("ColumnName", () => {
-            this.OR([{ALT: () => this.SUBRULE(this.ParentheticalExpression)}, {ALT: () => this.CONSUME(vocabulary["columnName"])}], {
-                ERR_MSG: "Expected a column name (wrapped in double quotes) or a parenthesis-wrapped expression."
-            });
-        });
-
-        this.RULE("TerminalColumnName", () => {
-            this.CONSUME(vocabulary["columnName"]);
-        });
-
-        this.RULE("As", () => {
-            this.CONSUME(vocabulary["as"]);
-            this.SUBRULE(this.TerminalColumnName);
-        });
-
         this.RULE("Function", () => {
             this.OR([
                 {ALT: () => this.CONSUME(vocabulary["sqrt"])},
@@ -112,13 +162,25 @@ export class ComputedExpressionColumnParser extends CstParser {
             ]);
         });
 
+        /**
+         * Consume an addition or subtraction symbol. Rules for operators with
+         * defined precedence rules are separated from the general
+         * `Operator` rule.
+         */
+        this.RULE("AdditionOperator", () => {
+            this.OR([{ALT: () => this.CONSUME(vocabulary["add"])}, {ALT: () => this.CONSUME(vocabulary["subtract"])}]);
+        });
+
+        this.RULE("MultiplicationOperator", () => {
+            this.OR([{ALT: () => this.CONSUME(vocabulary["multiply"])}, {ALT: () => this.CONSUME(vocabulary["divide"])}]);
+        });
+
+        this.RULE("ExponentOperator", () => {
+            this.CONSUME(vocabulary["pow"]);
+        });
+
         this.RULE("Operator", () => {
             this.OR([
-                {ALT: () => this.CONSUME(vocabulary["add"])},
-                {ALT: () => this.CONSUME(vocabulary["subtract"])},
-                {ALT: () => this.CONSUME(vocabulary["multiply"])},
-                {ALT: () => this.CONSUME(vocabulary["divide"])},
-                {ALT: () => this.CONSUME(vocabulary["pow"])},
                 {ALT: () => this.CONSUME(vocabulary["percent_of"])},
                 {ALT: () => this.CONSUME(vocabulary["equals"])},
                 {ALT: () => this.CONSUME(vocabulary["not_equals"])},
@@ -128,9 +190,42 @@ export class ComputedExpressionColumnParser extends CstParser {
             ]);
         });
 
+        /**
+         * A special rule for column names used as alias after `as` to prevent
+         * further evaluation of possible expressions.
+         */
+        this.RULE("TerminalColumnName", () => {
+            this.CONSUME(vocabulary["columnName"]);
+        });
+
+        /**
+         * A rule for aliasing computed columns - placed at the top so that it
+         * is evaluated after everything else.
+         *
+         * TODO: make AS left evaluative by default: an expression like
+         * x + y + z as "abc" currently breaks to abc + z, when it should be
+         * x + abc.
+         */
+        this.RULE("As", () => {
+            this.CONSUME(vocabulary["as"]);
+            this.SUBRULE(this.TerminalColumnName);
+        });
+
+        /**
+         * A column name, which can evaluate to a parenthetical expression,
+         * a functional column, or a literal column name - a string
+         * wrapped in double or single quotes.
+         */
+        this.RULE("ColumnName", () => {
+            this.OR([{ALT: () => this.SUBRULE(this.ParentheticalExpression)}, {ALT: () => this.SUBRULE(this.FunctionComputedColumn)}, {ALT: () => this.CONSUME(vocabulary["columnName"])}], {
+                ERR_MSG: "Expected a column name (wrapped in double quotes) or a parenthesis-wrapped expression."
+            });
+        });
+
         /**
          * The rule for parenthetical expressions, which consume parentheses
-         * and resolve to this.Expression.
+         * and resolve to this.Expression. Because it is lowest in the
+         * tree, it is evaluated before everything else.
          */
         this.RULE("ParentheticalExpression", () => {
             this.CONSUME(vocabulary["leftParen"]);