Custom expression: friendlier message on failing tokenization (#15787)

* Custom expression: friendlier message on failing tokenization

This covers cases such as:
* invalid characters
* unterminated quoted string
* unterminated bracket/field reference

* Ensure that tokenizerError is initialized properly

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

@@ -328,8 +328,7 @@ export function tokenize(expression) {
 }
 
 // e.g. "COUNTIF(([Total]-[Tax] <5" returns 2 (missing parentheses)
-export function countMatchingParentheses(expression) {
-  const { tokens } = tokenize(expression);
+export function countMatchingParentheses(tokens) {
   const isOpen = t => t.op === OPERATOR.OpenParenthesis;
   const isClose = t => t.op === OPERATOR.CloseParenthesis;
   const count = (c, token) =>

frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx

@@ -8,7 +8,10 @@ import cx from "classnames";
 
 import { format } from "metabase/lib/expressions/format";
 import { processSource } from "metabase/lib/expressions/process";
-import { countMatchingParentheses } from "metabase/lib/expressions/tokenizer";
+import {
+  tokenize,
+  countMatchingParentheses,
+} from "metabase/lib/expressions/tokenizer";
 import MetabaseSettings from "metabase/lib/settings";
 import colors from "metabase/lib/colors";
 
@@ -143,10 +146,16 @@ export default class ExpressionEditorTextfield extends React.Component {
               source,
               ...this._getParserOptions(newProps),
             })
-          : { expression: null, compileError: null, syntaxTree: null };
+          : {
+              expression: null,
+              tokenizerError: [],
+              compileError: null,
+              syntaxTree: null,
+            };
       this.setState({
         source,
         expression,
+        tokenizeError: [],
         compileError,
         syntaxTree,
         suggestions: [],
@@ -241,7 +250,7 @@ export default class ExpressionEditorTextfield extends React.Component {
     this.clearSuggestions();
     const { tokenizerError, compileError } = this.state;
     const displayError =
-      tokenizerError.length > 0 ? tokenizerError : compileError;
+      tokenizerError.length > 0 ? _.first(tokenizerError) : compileError;
     this.setState({ displayError });
 
     // whenever our input blurs we push the updated expression to our parent if valid
@@ -314,8 +323,8 @@ export default class ExpressionEditorTextfield extends React.Component {
     const showSuggestions =
       !hasSelection && !(isValid && isAtEnd && !endsWithWhitespace);
 
-    const tokenizerError = [];
-    const mismatchedParentheses = countMatchingParentheses(source);
+    const { tokens, errors: tokenizerError } = tokenize(source);
+    const mismatchedParentheses = countMatchingParentheses(tokens);
     const mismatchedError =
       mismatchedParentheses === 1
         ? t`Expecting a closing parenthesis`

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

@@ -135,11 +135,12 @@ describe("metabase/lib/expressions/tokenizer", () => {
   });
 
   it("should count matching parentheses", () => {
-    expect(countMatchingParentheses("()")).toEqual(0);
-    expect(countMatchingParentheses("(")).toEqual(1);
-    expect(countMatchingParentheses(")")).toEqual(-1);
-    expect(countMatchingParentheses("(A+(")).toEqual(2);
-    expect(countMatchingParentheses("SUMIF(")).toEqual(1);
-    expect(countMatchingParentheses("COUNTIF(Deal))")).toEqual(-1);
+    const count = expr => countMatchingParentheses(tokenize(expr).tokens);
+    expect(count("()")).toEqual(0);
+    expect(count("(")).toEqual(1);
+    expect(count(")")).toEqual(-1);
+    expect(count("(A+(")).toEqual(2);
+    expect(count("SUMIF(")).toEqual(1);
+    expect(count("COUNTIF(Deal))")).toEqual(-1);
   });
 });

frontend/test/metabase/scenarios/question/notebook.cy.spec.js

@@ -848,6 +848,44 @@ describe("scenarios > question > notebook", () => {
         cy.contains(/^Expecting an opening parenthesis/i);
       });
     });
+
+    it("should catch invalid characters", () => {
+      openProductsTable({ mode: "notebook" });
+      cy.findByText("Custom column").click();
+      popover().within(() => {
+        cy.get("[contenteditable='true']").type("[Price] / #");
+        cy.findByPlaceholderText("Something nice and descriptive")
+          .click()
+          .type("Massive Discount");
+        cy.contains(/^Invalid character: #/i);
+      });
+    });
+
+    it("should catch unterminated string literals", () => {
+      openProductsTable({ mode: "notebook" });
+      cy.findByText("Filter").click();
+      cy.findByText("Custom Expression").click();
+      cy.get("[contenteditable='true']")
+        .click()
+        .clear()
+        .type('[Category] = "widget', { delay: 50 });
+      cy.findAllByRole("button", { name: "Done" })
+        .should("not.be.disabled")
+        .click();
+      cy.findByText("Unterminated quoted string");
+    });
+
+    it("should catch unterminated field reference", () => {
+      openProductsTable({ mode: "notebook" });
+      cy.findByText("Custom column").click();
+      popover().within(() => {
+        cy.get("[contenteditable='true']").type("[Price / 2");
+        cy.findByPlaceholderText("Something nice and descriptive")
+          .click()
+          .type("Massive Discount");
+        cy.contains(/^Unterminated bracket identifier/i);
+      });
+    });
   });
 });