[Expressions] Add support of comments (#122457)

* Add comments support to the expressions grammar
* Add typings to the interpreter parser
* Add expressions comments highlighting
* Update canvas to preserve original expression formatting
* Update documentation to cover comments

docs/canvas/canvas-expression-lifecycle.asciidoc

@@ -14,6 +14,7 @@ To use demo dataset available in Canvas to produce a table, run the following ex
 
 [source,text]
 ----
+/* Simple demo table */
 filters
 | demodata
 | table
@@ -24,6 +25,7 @@ This expression starts out with the <<filters_fn, filters>> function, which prov
 
 The filtered <<demodata_fn, demo data>> becomes the _context_ of the next function, <<table_fn, table>>, which creates a table visualization from this data set. The <<table_fn, table>> function isn’t strictly required, but by being explicit, you have the option of providing arguments to control things like the font used in the table. The output of the <<table_fn, table>> function becomes the _context_ of the <<render_fn, render>> function. Like the <<table_fn, table>>, the <<render_fn, render>> function isn’t required either, but it allows access to other arguments, such as styling the border of the element or injecting custom CSS.
 
+It is possible to add comments to the expression by starting them with a `//` sequence or by using `/*` and `*/` to enclose multi-line comments.
 
 [[canvas-function-arguments]]
 === Function arguments

docs/developer/plugin-list.asciidoc

@@ -140,6 +140,9 @@ All the arguments to expression functions need to be serializable, as well as in
 Expression functions should try to stay 'pure'. This makes functions easy to reuse and also 
 make it possible to serialize the whole chain as well as output at every step of execution.
 
+It is possible to add comments to expressions by starting them with a `//` sequence
+or by using `/*` and `*/` to enclose multi-line comments.
+
 Expressions power visualizations in Dashboard and Lens, as well as, every
 *element* in Canvas is backed by an expression.
 

packages/kbn-interpreter/grammar/grammar.peggy

@@ -23,7 +23,7 @@ start
   = expression
 
 expression
-  = space? first:function? rest:('|' space? fn:function { return fn; })* {
+  = blank? first:function? rest:('|' blank? fn:function { return fn; })* {
     return addMeta({
       type: 'expression',
       chain: first ? [first].concat(rest) : []
@@ -44,7 +44,7 @@ function "function"
 /* ----- Arguments ----- */
 
 argument_assignment
-  = name:identifier space? '=' space? value:argument {
+  = name:identifier blank? '=' blank? value:argument {
     return { name, value };
   }
   / value:argument {
@@ -58,7 +58,7 @@ argument
   }
 
 arg_list
-  = args:(space arg:argument_assignment { return arg; })* space? {
+  = args:(blank arg:argument_assignment { return arg; })* blank? {
     return args.reduce((accumulator, { name, value }) => ({
       ...accumulator,
       [name]: (accumulator[name] || []).concat(value)
@@ -82,7 +82,7 @@ phrase
 
 unquoted_string_or_number
   // Make sure we're not matching the beginning of a search
-  = string:unquoted+ { // this also matches nulls, booleans, and numbers
+  = !comment string:unquoted+ { // this also matches nulls, booleans, and numbers
     var result = string.join('');
     // Sort of hacky, but PEG doesn't have backtracking so
     // a null/boolean/number rule is hard to read, and performs worse
@@ -93,8 +93,20 @@ unquoted_string_or_number
     return Number(result);
   }
 
+blank
+  = (space / comment)+
+
 space
-  = [\ \t\r\n]+
+  = [\ \t\r\n]
+
+comment
+  = inline_comment / multiline_comment
+
+inline_comment
+  = "//" [^\n]*
+
+multiline_comment
+  = "/*" (!"*/" .)* "*/"?
 
 unquoted
   = "\\" sequence:([\"'(){}<>\[\]$`|=\ \t\n\r] / "\\") { return sequence; }

packages/kbn-interpreter/src/common/index.ts

@@ -6,14 +6,26 @@
  * Side Public License, v 1.
  */
 
-export type { Ast, ExpressionFunctionAST } from './lib/ast';
-export { fromExpression, toExpression, safeElementFromExpression } from './lib/ast';
+export type {
+  Ast,
+  AstArgument,
+  AstFunction,
+  AstNode,
+  AstWithMeta,
+  AstArgumentWithMeta,
+  AstFunctionWithMeta,
+} from './lib/ast';
+export {
+  fromExpression,
+  isAst,
+  isAstWithMeta,
+  toExpression,
+  safeElementFromExpression,
+} from './lib/ast';
 export { Fn } from './lib/fn';
 export { getType } from './lib/get_type';
 export { castProvider } from './lib/cast';
-// @ts-expect-error
-// @internal
-export { parse } from '../../grammar';
+export { parse } from './lib/parse';
 export { getByAlias } from './lib/get_by_alias';
 export { Registry } from './lib/registry';
 export { addRegistries, register, registryFactory } from './registries';

packages/kbn-interpreter/src/common/lib/ast.ts

@@ -1,159 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License
- * 2.0 and the Server Side Public License, v 1; you may not use this file except
- * in compliance with, at your election, the Elastic License 2.0 or the Server
- * Side Public License, v 1.
- */
-
-import { getType } from './get_type';
-// @ts-expect-error
-import { parse } from '../../../grammar';
-
-export type ExpressionArgAST = string | boolean | number | Ast;
-
-export interface ExpressionFunctionAST {
-  type: 'function';
-  function: string;
-  arguments: {
-    [key: string]: ExpressionArgAST[];
-  };
-}
-
-export interface Ast {
-  /** @internal */
-  function: any;
-  /** @internal */
-  arguments: any;
-  type: 'expression';
-  chain: ExpressionFunctionAST[];
-  /** @internal */
-  replace(regExp: RegExp, s: string): string;
-}
-
-function getArgumentString(arg: Ast, argKey: string | undefined, level = 0) {
-  const type = getType(arg);
-
-  // eslint-disable-next-line @typescript-eslint/no-shadow
-  function maybeArgKey(argKey: string | null | undefined, argString: string) {
-    return argKey == null || argKey === '_' ? argString : `${argKey}=${argString}`;
-  }
-
-  if (type === 'string') {
-    // correctly (re)escape double quotes
-    const escapedArg = arg.replace(/[\\"]/g, '\\$&'); // $& means the whole matched string
-    return maybeArgKey(argKey, `"${escapedArg}"`);
-  }
-
-  if (type === 'boolean' || type === 'null' || type === 'number') {
-    // use values directly
-    return maybeArgKey(argKey, `${arg}`);
-  }
-
-  if (type === 'expression') {
-    // build subexpressions
-    return maybeArgKey(argKey, `{${getExpression(arg.chain, level + 1)}}`);
-  }
-
-  // unknown type, throw with type value
-  throw new Error(`Invalid argument type in AST: ${type}`);
-}
-
-function getExpressionArgs(block: Ast, level = 0) {
-  const args = block.arguments;
-  const hasValidArgs = typeof args === 'object' && args != null && !Array.isArray(args);
-
-  if (!hasValidArgs) throw new Error('Arguments can only be an object');
-
-  const argKeys = Object.keys(args);
-  const MAX_LINE_LENGTH = 80; // length before wrapping arguments
-  return argKeys.map((argKey) =>
-    args[argKey].reduce((acc: any, arg: any) => {
-      const argString = getArgumentString(arg, argKey, level);
-      const lineLength = acc.split('\n').pop().length;
-
-      // if arg values are too long, move it to the next line
-      if (level === 0 && lineLength + argString.length > MAX_LINE_LENGTH) {
-        return `${acc}\n  ${argString}`;
-      }
-
-      // append arg values to existing arg values
-      if (lineLength > 0) return `${acc} ${argString}`;
-
-      // start the accumulator with the first arg value
-      return argString;
-    }, '')
-  );
-}
-
-function fnWithArgs(fnName: any, args: any[]) {
-  if (!args || args.length === 0) return fnName;
-  return `${fnName} ${args.join(' ')}`;
-}
-
-function getExpression(chain: any[], level = 0) {
-  if (!chain) throw new Error('Expressions must contain a chain');
-
-  // break new functions onto new lines if we're not in a nested/sub-expression
-  const separator = level > 0 ? ' | ' : '\n| ';
-
-  return chain
-    .map((chainObj) => {
-      const type = getType(chainObj);
-
-      if (type === 'function') {
-        const fn = chainObj.function;
-        if (!fn || fn.length === 0) throw new Error('Functions must have a function name');
-
-        const expArgs = getExpressionArgs(chainObj, level);
-
-        return fnWithArgs(fn, expArgs);
-      }
-    }, [])
-    .join(separator);
-}
-
-export function fromExpression(expression: string, type = 'expression'): Ast {
-  try {
-    return parse(String(expression), { startRule: type });
-  } catch (e) {
-    throw new Error(`Unable to parse expression: ${e.message}`);
-  }
-}
-
-// TODO: OMG This is so bad, we need to talk about the right way to handle bad expressions since some are element based and others not
-export function safeElementFromExpression(expression: string) {
-  try {
-    return fromExpression(expression);
-  } catch (e) {
-    return fromExpression(
-      `markdown
-"## Crud.
-Canvas could not parse this element's expression. I am so sorry this error isn't more useful. I promise it will be soon.
-
-Thanks for understanding,
-#### Management
-"`
-    );
-  }
-}
-
-// TODO: Respect the user's existing formatting
-export function toExpression(astObj: Ast, type = 'expression'): string {
-  if (type === 'argument') {
-    // @ts-ignore
-    return getArgumentString(astObj);
-  }
-
-  const validType = ['expression', 'function'].includes(getType(astObj));
-  if (!validType) throw new Error('Expression must be an expression or argument function');
-
-  if (getType(astObj) === 'expression') {
-    if (!Array.isArray(astObj.chain)) throw new Error('Expressions must contain a chain');
-
-    return getExpression(astObj.chain);
-  }
-
-  const expArgs = getExpressionArgs(astObj);
-  return fnWithArgs(astObj.function, expArgs);
-}

packages/kbn-interpreter/src/common/lib/ast/ast.ts

@@ -0,0 +1,64 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+export type AstNode = Ast | AstFunction | AstArgument;
+
+// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
+export type Ast = {
+  type: 'expression';
+  chain: AstFunction[];
+};
+
+// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
+export type AstFunction = {
+  type: 'function';
+  function: string;
+  arguments: Record<string, AstArgument[]>;
+};
+
+export type AstArgument = string | boolean | number | Ast;
+
+interface WithMeta<T> {
+  start: number;
+  end: number;
+  text: string;
+  node: T;
+}
+
+type Replace<T, R> = Pick<T, Exclude<keyof T, keyof R>> & R;
+
+type WrapAstArgumentWithMeta<T> = T extends Ast ? AstWithMeta : WithMeta<T>;
+export type AstArgumentWithMeta = WrapAstArgumentWithMeta<AstArgument>;
+
+export type AstFunctionWithMeta = WithMeta<
+  Replace<
+    AstFunction,
+    {
+      arguments: {
+        [key: string]: AstArgumentWithMeta[];
+      };
+    }
+  >
+>;
+
+export type AstWithMeta = WithMeta<
+  Replace<
+    Ast,
+    {
+      chain: AstFunctionWithMeta[];
+    }
+  >
+>;
+
+export function isAstWithMeta(value: any): value is AstWithMeta {
+  return typeof value?.node === 'object';
+}
+
+export function isAst(value: any): value is Ast {
+  return typeof value === 'object' && value?.type === 'expression';
+}

packages/kbn-interpreter/src/common/lib/ast/compare.ts

@@ -0,0 +1,85 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { forEach, xor, zip } from 'lodash';
+import { parse } from '../parse';
+import type {
+  Ast,
+  AstArgument,
+  AstArgumentWithMeta,
+  AstWithMeta,
+  AstFunction,
+  AstFunctionWithMeta,
+} from './ast';
+import { isAst, isAstWithMeta } from './ast';
+
+export interface ValueChange {
+  type: 'value';
+  source: AstArgumentWithMeta;
+  target: AstArgument;
+}
+
+export type Change = ValueChange;
+
+export function isValueChange(value: any): value is ValueChange {
+  return value?.type === 'value';
+}
+
+export function compare(expression: string, ast: Ast): Change[] {
+  const astWithMeta = parse(expression, { addMeta: true });
+  const queue = [[astWithMeta, ast]] as Array<[typeof astWithMeta, typeof ast]>;
+  const changes = [] as Change[];
+
+  function compareExpression(source: AstWithMeta, target: Ast) {
+    zip(source.node.chain, target.chain).forEach(([fnWithMeta, fn]) => {
+      if (!fnWithMeta || !fn || fnWithMeta?.node.function !== fn?.function) {
+        throw Error('Expression changes are not supported.');
+      }
+
+      compareFunction(fnWithMeta, fn);
+    });
+  }
+
+  function compareFunction(source: AstFunctionWithMeta, target: AstFunction) {
+    if (xor(Object.keys(source.node.arguments), Object.keys(target.arguments)).length) {
+      throw Error('Function changes are not supported.');
+    }
+
+    forEach(source.node.arguments, (valuesWithMeta, argument) => {
+      const values = target.arguments[argument];
+
+      compareArgument(valuesWithMeta, values);
+    });
+  }
+
+  function compareArgument(source: AstArgumentWithMeta[], target: AstArgument[]) {
+    if (source.length !== target.length) {
+      throw Error('Arguments changes are not supported.');
+    }
+
+    zip(source, target).forEach(([valueWithMeta, value]) => compareValue(valueWithMeta!, value!));
+  }
+
+  function compareValue(source: AstArgumentWithMeta, target: AstArgument) {
+    if (isAstWithMeta(source) && isAst(target)) {
+      compareExpression(source, target);
+
+      return;
+    }
+
+    if (source.node !== target) {
+      changes.push({ type: 'value', source, target });
+    }
+  }
+
+  while (queue.length) {
+    compareExpression(...queue.shift()!);
+  }
+
+  return changes;
+}

packages/kbn-interpreter/src/common/lib/ast/from_expression.test.js

@@ -7,9 +7,9 @@
  */
 
 import { fromExpression } from '@kbn/interpreter';
-import { getType } from './get_type';
+import { getType } from '../get_type';
 
-describe('ast fromExpression', () => {
+describe('fromExpression', () => {
   describe('invalid expression', () => {
     it('throws with invalid expression', () => {
       const check = () => fromExpression('wat!');

packages/kbn-interpreter/src/common/lib/ast/from_expression.ts

@@ -0,0 +1,18 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import type { Ast } from './ast';
+import { parse } from '../parse';
+
+export function fromExpression(expression: string, type = 'expression'): Ast {
+  try {
+    return parse(String(expression), { startRule: type });
+  } catch (e) {
+    throw new Error(`Unable to parse expression: ${e.message}`);
+  }
+}

packages/kbn-interpreter/src/common/lib/ast/index.ts

@@ -0,0 +1,12 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+export * from './ast';
+export * from './from_expression';
+export * from './safe_element_from_expression';
+export * from './to_expression';

packages/kbn-interpreter/src/common/lib/ast/patch.ts

@@ -0,0 +1,47 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import type { Ast } from './ast';
+import { isAstWithMeta } from './ast';
+import type { Change, ValueChange } from './compare';
+import { compare, isValueChange } from './compare';
+import { toExpression } from './to_expression';
+
+export function patch(expression: string, ast: Ast): string {
+  let result = '';
+  let position = 0;
+
+  function apply(change: Change) {
+    if (isValueChange(change)) {
+      return void patchValue(change);
+    }
+
+    throw new Error('Cannot apply patch for the change.');
+  }
+
+  function patchValue(change: ValueChange) {
+    if (isAstWithMeta(change.source)) {
+      throw new Error('Patching sub-expressions is not supported.');
+    }
+
+    result += `${expression.substring(position, change.source.start)}${toExpression(
+      change.target,
+      'argument'
+    )}`;
+
+    position = change.source.end;
+  }
+
+  compare(expression, ast)
+    .sort(({ source: source1 }, { source: source2 }) => source1.start - source2.start)
+    .forEach(apply);
+
+  result += expression.substring(position);
+
+  return result;
+}

packages/kbn-interpreter/src/common/lib/ast/safe_element_from_expression.ts

@@ -0,0 +1,26 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { fromExpression } from './from_expression';
+
+// TODO: OMG This is so bad, we need to talk about the right way to handle bad expressions since some are element based and others not
+export function safeElementFromExpression(expression: string) {
+  try {
+    return fromExpression(expression);
+  } catch (e) {
+    return fromExpression(
+      `markdown
+"## Crud.
+Canvas could not parse this element's expression. I am so sorry this error isn't more useful. I promise it will be soon.
+
+Thanks for understanding,
+#### Management
+"`
+    );
+  }
+}

packages/kbn-interpreter/src/common/lib/ast/to_expression.test.js

@@ -7,8 +7,9 @@
  */
 
 import { toExpression } from '@kbn/interpreter';
+import { cloneDeep, set, unset } from 'lodash';
 
-describe('ast toExpression', () => {
+describe('toExpression', () => {
   describe('single expression', () => {
     it('throws if no type included', () => {
       const errMsg = 'Objects must have a type property';
@@ -616,4 +617,110 @@ describe('ast toExpression', () => {
       expect(expression).toBe('both named="example" another="item" "one" "two" "three"');
     });
   });
+
+  describe('patch expression', () => {
+    const expression = 'f1 a=1 a=2 b=1 b={f2 c=1 c=2 | f3 d=1 d=2} | f4 e=1 e=2';
+    const ast = {
+      type: 'expression',
+      chain: [
+        {
+          type: 'function',
+          function: 'f1',
+          arguments: {
+            a: [1, 2],
+            b: [
+              1,
+              {
+                type: 'expression',
+                chain: [
+                  {
+                    type: 'function',
+                    function: 'f2',
+                    arguments: { c: ['a', 'b'] },
+                  },
+                  {
+                    type: 'function',
+                    function: 'f3',
+                    arguments: { d: [1, 2] },
+                  },
+                ],
+              },
+            ],
+          },
+        },
+        {
+          type: 'function',
+          function: 'f4',
+          arguments: {
+            e: [1, 2],
+          },
+        },
+      ],
+    };
+
+    it.each([
+      [
+        expression,
+        'f1 a="updated" a=2 b=1 b={f2 c="a" c="b" | f3 d=1 d=2} | f4 e=1 e=2',
+        set(cloneDeep(ast), 'chain.0.arguments.a.0', 'updated'),
+      ],
+      [
+        expression,
+        'f1 a=1 a=2 b=1 b={f2 c="updated" c="b" | f3 d=1 d=2} | f4 e=1 e=2',
+        set(cloneDeep(ast), 'chain.0.arguments.b.1.chain.0.arguments.c.0', 'updated'),
+      ],
+      [
+        expression,
+        'f1 a={updated} a=2 b=1 b={f2 c="a" c="b" | f3 d=1 d=2} | f4 e=1 e=2',
+        set(cloneDeep(ast), 'chain.0.arguments.a.0', {
+          type: 'expression',
+          chain: [
+            {
+              type: 'function',
+              function: 'updated',
+              arguments: {},
+            },
+          ],
+        }),
+      ],
+      [
+        '/* comment */ f1 a /* comment */ =1',
+        '/* comment */ f1 a /* comment */ =2',
+        {
+          type: 'expression',
+          chain: [
+            {
+              type: 'function',
+              function: 'f1',
+              arguments: {
+                a: [2],
+              },
+            },
+          ],
+        },
+      ],
+    ])('should patch "%s" to become "%s"', (source, expected, ast) => {
+      expect(toExpression(ast, { source })).toBe(expected);
+    });
+
+    it.each([
+      [
+        expression,
+        set(cloneDeep(ast), 'chain.2', {
+          type: 'function',
+          function: 'f5',
+          arguments: {},
+        }),
+      ],
+      [expression, unset(cloneDeep(ast), 'chain.1')],
+      [expression, set(cloneDeep(ast), 'chain.0.function', 'updated')],
+      [expression, set(cloneDeep(ast), 'chain.0.arguments.c', [1])],
+      [expression, unset(cloneDeep(ast), 'chain.0.arguments.b')],
+      [expression, unset(cloneDeep(ast), 'chain.0.arguments.b.1')],
+      [expression, set(cloneDeep(ast), 'chain.0.arguments.b.2', 3)],
+      [expression, set(cloneDeep(ast), 'chain.0.arguments.b.1', 2)],
+    ])('should fail on patching expression', (source, ast) => {
+      expect(() => toExpression(ast, { source })).toThrowError();
+    });
+  });
 });

packages/kbn-interpreter/src/common/lib/ast/to_expression.ts

@@ -0,0 +1,145 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { getType } from '../get_type';
+import type { Ast, AstArgument, AstFunction, AstNode } from './ast';
+import { isAst } from './ast';
+import { patch } from './patch';
+
+interface Options {
+  /**
+   * Node type.
+   */
+  type?: 'argument' | 'expression' | 'function';
+
+  /**
+   * Original expression to apply the new AST to.
+   * At the moment, only arguments values changes are supported.
+   */
+  source?: string;
+}
+
+function getArgumentString(arg: AstArgument, argKey?: string, level = 0): string {
+  const type = getType(arg);
+
+  // eslint-disable-next-line @typescript-eslint/no-shadow
+  function maybeArgKey(argKey: string | null | undefined, argString: string) {
+    return argKey == null || argKey === '_' ? argString : `${argKey}=${argString}`;
+  }
+
+  if (type === 'string') {
+    // correctly (re)escape double quotes
+    const escapedArg = (arg as string).replace(/[\\"]/g, '\\$&'); // $& means the whole matched string
+    return maybeArgKey(argKey, `"${escapedArg}"`);
+  }
+
+  if (type === 'boolean' || type === 'null' || type === 'number') {
+    // use values directly
+    return maybeArgKey(argKey, `${arg}`);
+  }
+
+  if (type === 'expression') {
+    // build subexpressions
+    return maybeArgKey(argKey, `{${getExpression((arg as Ast).chain, level + 1)}}`);
+  }
+
+  // unknown type, throw with type value
+  throw new Error(`Invalid argument type in AST: ${type}`);
+}
+
+function getExpressionArgs({ arguments: args }: AstFunction, level = 0) {
+  if (args == null || typeof args !== 'object' || Array.isArray(args)) {
+    throw new Error('Arguments can only be an object');
+  }
+
+  const argKeys = Object.keys(args);
+  const MAX_LINE_LENGTH = 80; // length before wrapping arguments
+  return argKeys.map((argKey) =>
+    args[argKey].reduce((acc: string, arg) => {
+      const argString = getArgumentString(arg, argKey, level);
+      const lineLength = acc.split('\n').pop()!.length;
+
+      // if arg values are too long, move it to the next line
+      if (level === 0 && lineLength + argString.length > MAX_LINE_LENGTH) {
+        return `${acc}\n  ${argString}`;
+      }
+
+      // append arg values to existing arg values
+      if (lineLength > 0) {
+        return `${acc} ${argString}`;
+      }
+
+      // start the accumulator with the first arg value
+      return argString;
+    }, '')
+  );
+}
+
+function fnWithArgs(fnName: string, args: unknown[]) {
+  return `${fnName} ${args?.join(' ') ?? ''}`.trim();
+}
+
+function getExpression(chain: AstFunction[], level = 0) {
+  if (!chain) {
+    throw new Error('Expressions must contain a chain');
+  }
+
+  // break new functions onto new lines if we're not in a nested/sub-expression
+  const separator = level > 0 ? ' | ' : '\n| ';
+
+  return chain
+    .map((item) => {
+      const type = getType(item);
+
+      if (type !== 'function') {
+        return;
+      }
+
+      const { function: fn } = item;
+      if (!fn) {
+        throw new Error('Functions must have a function name');
+      }
+
+      const expressionArgs = getExpressionArgs(item, level);
+
+      return fnWithArgs(fn, expressionArgs);
+    })
+    .join(separator);
+}
+
+export function toExpression(ast: AstNode, options: string | Options = 'expression'): string {
+  const { type, source } = typeof options === 'string' ? ({ type: options } as Options) : options;
+
+  if (source && isAst(ast)) {
+    return patch(source, ast);
+  }
+
+  if (type === 'argument') {
+    return getArgumentString(ast as AstArgument);
+  }
+
+  const nodeType = getType(ast);
+
+  if (nodeType === 'expression') {
+    const { chain } = ast as Ast;
+    if (!Array.isArray(chain)) {
+      throw new Error('Expressions must contain a chain');
+    }
+
+    return getExpression(chain);
+  }
+
+  if (nodeType === 'function') {
+    const { function: fn } = ast as AstFunction;
+    const args = getExpressionArgs(ast as AstFunction);
+
+    return fnWithArgs(fn, args);
+  }
+
+  throw new Error('Expression must be an expression or argument function');
+}

packages/kbn-interpreter/src/common/lib/grammar.d.ts

@@ -0,0 +1,11 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+declare module '*/grammar' {
+  export const parse: import('./parse').Parse;
+}

packages/kbn-interpreter/src/common/lib/parse.ts

@@ -0,0 +1,27 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import type { Ast, AstWithMeta } from './ast';
+import { parse } from '../../../grammar';
+
+interface Options {
+  startRule?: string;
+}
+
+interface OptionsWithMeta extends Options {
+  addMeta: true;
+}
+
+export interface Parse {
+  (input: string, options?: Options): Ast;
+  (input: string, options: OptionsWithMeta): AstWithMeta;
+}
+
+const typedParse = parse;
+
+export { typedParse as parse };

src/plugins/expressions/README.asciidoc

@@ -10,6 +10,9 @@ All the arguments to expression functions need to be serializable, as well as in
 Expression functions should try to stay 'pure'. This makes functions easy to reuse and also 
 make it possible to serialize the whole chain as well as output at every step of execution.
 
+It is possible to add comments to expressions by starting them with a `//` sequence
+or by using `/*` and `*/` to enclose multi-line comments.
+
 Expressions power visualizations in Dashboard and Lens, as well as, every
 *element* in Canvas is backed by an expression.
 
@@ -30,7 +33,8 @@ filters
   query="SELECT COUNT(timestamp) as total_errors
     FROM kibana_sample_data_logs
     WHERE tags LIKE '%warning%' OR tags LIKE '%error%'"
-| math "total_errors"
+| math "total_errors" // take "total_errors" column
+/* Represent as a number over a label */
 | metric "TOTAL ISSUES"
   metricFont={font family="'Open Sans', Helvetica, Arial, sans-serif" size=48 align="left" color="#FFFFFF" weight="normal" underline=false italic=false}
   labelFont={font family="'Open Sans', Helvetica, Arial, sans-serif" size=30 align="left" color="#FFFFFF" weight="lighter" underline=false italic=false}

src/plugins/expressions/common/ast/types.ts

@@ -6,6 +6,7 @@
  * Side Public License, v 1.
  */
 
+import type { Ast, AstFunction } from '@kbn/interpreter';
 import { ExpressionValue, ExpressionValueError } from '../expression_types';
 
 export type ExpressionAstNode =
@@ -13,14 +14,11 @@ export type ExpressionAstNode =
   | ExpressionAstFunction
   | ExpressionAstArgument;
 
-export type ExpressionAstExpression = {
-  type: 'expression';
+export type ExpressionAstExpression = Omit<Ast, 'chain'> & {
   chain: ExpressionAstFunction[];
 };
 
-export type ExpressionAstFunction = {
-  type: 'function';
-  function: string;
+export type ExpressionAstFunction = Omit<AstFunction, 'arguments'> & {
   arguments: Record<string, ExpressionAstArgument[]>;
 
   /**

src/plugins/presentation_util/public/components/expression_input/autocomplete.ts

@@ -7,12 +7,10 @@
  */
 
 import { uniq } from 'lodash';
-// @ts-expect-error untyped library
+import type { AstWithMeta, AstArgumentWithMeta } from '@kbn/interpreter';
+import { isAstWithMeta } from '@kbn/interpreter';
 import { parse } from '@kbn/interpreter';
 import {
-  ExpressionAstExpression,
-  ExpressionAstFunction,
-  ExpressionAstArgument,
   ExpressionFunction,
   ExpressionFunctionParameter,
   getByAlias,
@@ -40,7 +38,7 @@ interface ValueSuggestion extends BaseSuggestion {
 }
 
 interface FnArgAtPosition {
-  ast: ExpressionASTWithMeta;
+  ast: AstWithMeta;
   fnIndex: number;
 
   argName?: string;
@@ -57,45 +55,6 @@ interface FnArgAtPosition {
   contextFn?: string | null;
 }
 
-// If you parse an expression with the "addMeta" option it completely
-// changes the type of returned object.  The following types
-// enhance the existing AST types with the appropriate meta information
-interface ASTMetaInformation<T> {
-  start: number;
-  end: number;
-  text: string;
-  node: T;
-}
-
-// Wraps ExpressionArg with meta or replace ExpressionAstExpression with ExpressionASTWithMeta
-type WrapExpressionArgWithMeta<T> = T extends ExpressionAstExpression
-  ? ExpressionASTWithMeta
-  : ASTMetaInformation<T>;
-
-type ExpressionArgASTWithMeta = WrapExpressionArgWithMeta<ExpressionAstArgument>;
-
-type Modify<T, R> = Pick<T, Exclude<keyof T, keyof R>> & R;
-
-// Wrap ExpressionFunctionAST with meta and modify arguments to be wrapped with meta
-type ExpressionFunctionASTWithMeta = Modify<
-  ExpressionAstFunction,
-  {
-    arguments: {
-      [key: string]: ExpressionArgASTWithMeta[];
-    };
-  }
->;
-
-// Wrap ExpressionFunctionAST with meta and modify chain to be wrapped with meta
-type ExpressionASTWithMeta = ASTMetaInformation<
-  Modify<
-    ExpressionAstExpression,
-    {
-      chain: Array<ASTMetaInformation<ExpressionFunctionASTWithMeta>>;
-    }
-  >
->;
-
 export interface FunctionSuggestion extends BaseSuggestion {
   type: 'function';
   fnDef: ExpressionFunction;
@@ -103,13 +62,6 @@ export interface FunctionSuggestion extends BaseSuggestion {
 
 export type AutocompleteSuggestion = FunctionSuggestion | ArgSuggestion | ValueSuggestion;
 
-// Typeguard for checking if ExpressionArg is a new expression
-function isExpression(
-  maybeExpression: ExpressionArgASTWithMeta
-): maybeExpression is ExpressionASTWithMeta {
-  return typeof maybeExpression.node === 'object';
-}
-
 /**
  * Generates the AST with the given expression and then returns the function and argument definitions
  * at the given position in the expression, if there are any.
@@ -120,9 +72,9 @@ export function getFnArgDefAtPosition(
   position: number
 ) {
   try {
-    const ast: ExpressionASTWithMeta = parse(expression, {
+    const ast: AstWithMeta = parse(expression, {
       addMeta: true,
-    }) as ExpressionASTWithMeta;
+    }) as AstWithMeta;
 
     const { ast: newAst, fnIndex, argName, argStart, argEnd } = getFnArgAtPosition(ast, position);
     const fn = newAst.node.chain[fnIndex].node;
@@ -153,7 +105,7 @@ export function getAutocompleteSuggestions(
 ): AutocompleteSuggestion[] {
   const text = expression.substr(0, position) + MARKER + expression.substr(position);
   try {
-    const ast = parse(text, { addMeta: true }) as ExpressionASTWithMeta;
+    const ast = parse(text, { addMeta: true }) as AstWithMeta;
     const {
       ast: newAst,
       fnIndex,
@@ -206,7 +158,7 @@ export function getAutocompleteSuggestions(
     The context function for the first expression in the chain is `math`, since it's the parent's previous
     item. The context function for `formatnumber` is the return of `math "divide(value, 2)"`.
 */
-function getFnArgAtPosition(ast: ExpressionASTWithMeta, position: number): FnArgAtPosition {
+function getFnArgAtPosition(ast: AstWithMeta, position: number): FnArgAtPosition {
   const fnIndex = ast.node.chain.findIndex((fn) => fn.start <= position && position <= fn.end);
   const fn = ast.node.chain[fnIndex];
   for (const [argName, argValues] of Object.entries(fn.node.arguments)) {
@@ -222,7 +174,7 @@ function getFnArgAtPosition(ast: ExpressionASTWithMeta, position: number): FnArg
 
         // If the arg value is an expression, expand our start and end position
         // to include the opening and closing braces
-        if (value.node !== null && isExpression(value)) {
+        if (value.node !== null && isAstWithMeta(value)) {
           argStart--;
           argEnd++;
         }
@@ -233,7 +185,7 @@ function getFnArgAtPosition(ast: ExpressionASTWithMeta, position: number): FnArg
         // argument name (`font=` for example), recurse within the expression
         if (
           value.node !== null &&
-          isExpression(value) &&
+          isAstWithMeta(value) &&
           (argName === '_' || !(argStart <= position && position <= argStart + argName.length + 1))
         ) {
           const result = getFnArgAtPosition(value, position);
@@ -265,7 +217,7 @@ function getFnArgAtPosition(ast: ExpressionASTWithMeta, position: number): FnArg
 
 function getFnNameSuggestions(
   specs: ExpressionFunction[],
-  ast: ExpressionASTWithMeta,
+  ast: AstWithMeta,
   fnIndex: number
 ): FunctionSuggestion[] {
   // Filter the list of functions by the text at the marker
@@ -299,7 +251,7 @@ function getFnNameSuggestions(
 
 function getSubFnNameSuggestions(
   specs: ExpressionFunction[],
-  ast: ExpressionASTWithMeta,
+  ast: AstWithMeta,
   fnIndex: number,
   parentFn: string,
   parentFnArgName: string,
@@ -391,7 +343,7 @@ function getScore(
 
 function getArgNameSuggestions(
   specs: ExpressionFunction[],
-  ast: ExpressionASTWithMeta,
+  ast: AstWithMeta,
   fnIndex: number,
   argName: string,
   argIndex: number
@@ -407,7 +359,7 @@ function getArgNameSuggestions(
   const { start, end } = fn.arguments[argName][argIndex];
 
   // Filter the list of args by those which aren't already present (unless they allow multi)
-  const argEntries = Object.entries(fn.arguments).map<[string, ExpressionArgASTWithMeta[]]>(
+  const argEntries = Object.entries(fn.arguments).map<[string, AstArgumentWithMeta[]]>(
     ([name, values]) => {
       return [name, values.filter((value) => !value.text.includes(MARKER))];
     }
@@ -442,7 +394,7 @@ function getArgNameSuggestions(
 
 function getArgValueSuggestions(
   specs: ExpressionFunction[],
-  ast: ExpressionASTWithMeta,
+  ast: AstWithMeta,
   fnIndex: number,
   argName: string,
   argIndex: number

src/plugins/presentation_util/public/components/expression_input/language.ts

@@ -72,6 +72,9 @@ const expressionsLanguage: ExpressionsLanguage = {
       [/'/, 'string', '@string_single'],
 
       [/@symbols/, 'delimiter'],
+
+      [/\/\*/, 'comment', '@multiline_comment'],
+      [/\/\/.*$/, 'comment'],
     ],
 
     string_double: [
@@ -93,6 +96,12 @@ const expressionsLanguage: ExpressionsLanguage = {
       [/\}/, 'delimiter.bracket', '@pop'],
       { include: 'common' },
     ],
+
+    multiline_comment: [
+      [/[^\/*]+/, 'comment'],
+      ['\\*/', 'comment', '@pop'],
+      [/[\/*]/, 'comment'],
+    ],
   },
 };
 

x-pack/plugins/canvas/public/components/workpad_filters/hooks/use_canvas_filters.ts

@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { ExpressionFunctionAST, fromExpression } from '@kbn/interpreter';
+import { AstFunction, fromExpression } from '@kbn/interpreter';
 import { shallowEqual, useSelector } from 'react-redux';
 import { State } from '../../../../types';
 import { getFiltersByFilterExpressions } from '../../../lib/filter';
@@ -14,7 +14,7 @@ import { useFiltersService } from '../../../services';
 
 const extractExpressionAST = (filters: string[]) => fromExpression(filters.join(' | '));
 
-export function useCanvasFilters(filterExprsToGroupBy: ExpressionFunctionAST[] = []) {
+export function useCanvasFilters(filterExprsToGroupBy: AstFunction[] = []) {
   const filtersService = useFiltersService();
   const filterExpressions = useSelector(
     (state: State) => filtersService.getFilters(state),

x-pack/plugins/canvas/public/lib/filter.ts

@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { Ast, ExpressionFunctionAST, fromExpression, toExpression } from '@kbn/interpreter';
+import { Ast, AstFunction, fromExpression, toExpression } from '@kbn/interpreter';
 import { flowRight, get, groupBy } from 'lodash';
 import {
   Filter as FilterType,
@@ -63,7 +63,7 @@ export const groupFiltersBy = (filters: FilterType[], groupByField: FilterField)
   }));
 };
 
-const excludeFiltersByGroups = (filters: Ast[], filterExprAst: ExpressionFunctionAST) => {
+const excludeFiltersByGroups = (filters: Ast[], filterExprAst: AstFunction) => {
   const groupsToExclude = filterExprAst.arguments.group ?? [];
   const removeUngrouped = filterExprAst.arguments.ungrouped?.[0] ?? false;
   return filters.filter((filter) => {
@@ -85,7 +85,7 @@ const excludeFiltersByGroups = (filters: Ast[], filterExprAst: ExpressionFunctio
 
 const includeFiltersByGroups = (
   filters: Ast[],
-  filterExprAst: ExpressionFunctionAST,
+  filterExprAst: AstFunction,
   ignoreUngroupedIfGroups: boolean = false
 ) => {
   const groupsToInclude = filterExprAst.arguments.group ?? [];
@@ -109,7 +109,7 @@ const includeFiltersByGroups = (
 
 export const getFiltersByFilterExpressions = (
   filters: string[],
-  filterExprsAsts: ExpressionFunctionAST[]
+  filterExprsAsts: AstFunction[]
 ) => {
   const filtersAst = filters.map((filter) => fromExpression(filter));
   const matchedFiltersAst = filterExprsAsts.reduce((includedFilters, filter) => {

x-pack/plugins/canvas/public/lib/filter_adapters.test.ts

@@ -5,11 +5,11 @@
  * 2.0.
  */
 
-import { ExpressionFunctionAST } from '@kbn/interpreter';
+import { AstFunction } from '@kbn/interpreter';
 import { adaptCanvasFilter } from './filter_adapters';
 
 describe('adaptCanvasFilter', () => {
-  const filterAST: ExpressionFunctionAST = {
+  const filterAST: AstFunction = {
     type: 'function',
     function: 'exactly',
     arguments: {

x-pack/plugins/canvas/public/lib/filter_adapters.ts

@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { ExpressionFunctionAST } from '@kbn/interpreter';
+import type { AstFunction } from '@kbn/interpreter';
 import { identity } from 'lodash';
 import { ExpressionAstArgument, Filter, FilterType } from '../../types';
 
@@ -23,7 +23,7 @@ const argToValue = (
 
 const convertFunctionToFilterType = (func: string) => functionToFilter[func] ?? FilterType.exactly;
 
-const collectArgs = (args: ExpressionFunctionAST['arguments']) => {
+const collectArgs = (args: AstFunction['arguments']) => {
   const argsKeys = Object.keys(args);
 
   if (!argsKeys.length) {
@@ -36,7 +36,7 @@ const collectArgs = (args: ExpressionFunctionAST['arguments']) => {
   );
 };
 
-export function adaptCanvasFilter(filter: ExpressionFunctionAST): Filter {
+export function adaptCanvasFilter(filter: AstFunction): Filter {
   const { function: type, arguments: args } = filter;
   const { column, filterGroup, value: valueArg, type: typeArg, ...rest } = args ?? {};
   return {

x-pack/plugins/canvas/server/collectors/collector_helpers.ts

@@ -10,19 +10,13 @@
  * @param cb: callback to do something with a function that has been found
  */
 
-import {
-  ExpressionAstExpression,
-  ExpressionAstNode,
-} from '../../../../../src/plugins/expressions/common';
-
-function isExpression(
-  maybeExpression: ExpressionAstNode
-): maybeExpression is ExpressionAstExpression {
-  return typeof maybeExpression === 'object' && maybeExpression.type === 'expression';
-}
+import { isAst } from '@kbn/interpreter';
+import { ExpressionAstNode } from '../../../../../src/plugins/expressions/common';
 
 export function collectFns(ast: ExpressionAstNode, cb: (functionName: string) => void) {
-  if (!isExpression(ast)) return;
+  if (!isAst(ast)) {
+    return;
+  }
 
   ast.chain.forEach(({ function: cFunction, arguments: cArguments }) => {
     cb(cFunction);

x-pack/plugins/canvas/server/saved_objects/workpad_references.ts

@@ -31,7 +31,10 @@ export const extractReferences = (
         }))
       );
 
-      return { ...element, expression: toExpression(extract.state) };
+      return {
+        ...element,
+        expression: toExpression(extract.state, { source: element.expression }),
+      };
     });
 
     return { ...page, elements };
@@ -59,7 +62,7 @@ export const injectReferences = (
         referencesForElement
       );
 
-      return { ...element, expression: toExpression(injectedAst) };
+      return { ...element, expression: toExpression(injectedAst, { source: element.expression }) };
     });
 
     return { ...page, elements };

x-pack/plugins/lens/public/editor_frame_service/editor_frame/expression_helpers.ts

@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { Ast, fromExpression, ExpressionFunctionAST } from '@kbn/interpreter';
+import { Ast, AstFunction, fromExpression } from '@kbn/interpreter';
 import { DatasourceStates } from '../../state_management';
 import { Visualization, DatasourcePublicAPI, DatasourceMap } from '../../types';
 
@@ -35,7 +35,7 @@ export function prependDatasourceExpression(
     ([layerId, expr]) => [layerId, typeof expr === 'string' ? fromExpression(expr) : expr]
   );
 
-  const datafetchExpression: ExpressionFunctionAST = {
+  const datafetchExpression: AstFunction = {
     type: 'function',
     function: 'lens_merge_tables',
     arguments: {

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.ts

@@ -6,7 +6,7 @@
  */
 
 import { i18n } from '@kbn/i18n';
-import type { ExpressionFunctionAST } from '@kbn/interpreter';
+import type { AstFunction } from '@kbn/interpreter';
 import memoizeOne from 'memoize-one';
 import { LayerType, layerTypes } from '../../../../../common';
 import type { TimeScaleUnit } from '../../../../../common/expressions';
@@ -143,7 +143,7 @@ export function dateBasedOperationToExpression(
   columnId: string,
   functionName: string,
   additionalArgs: Record<string, unknown[]> = {}
-): ExpressionFunctionAST[] {
+): AstFunction[] {
   const currentColumn = layer.columns[columnId] as unknown as ReferenceBasedIndexPatternColumn;
   const buckets = layer.columnOrder.filter((colId) => layer.columns[colId].isBucketed);
   const dateColumnIndex = buckets.findIndex(
@@ -174,7 +174,7 @@ export function optionallHistogramBasedOperationToExpression(
   columnId: string,
   functionName: string,
   additionalArgs: Record<string, unknown[]> = {}
-): ExpressionFunctionAST[] {
+): AstFunction[] {
   const currentColumn = layer.columns[columnId] as unknown as ReferenceBasedIndexPatternColumn;
   const buckets = layer.columnOrder.filter((colId) => layer.columns[colId].isBucketed);
   const nonHistogramColumns = buckets.filter(

x-pack/plugins/lens/server/migrations/saved_object_migrations.ts

@@ -6,7 +6,7 @@
  */
 
 import { cloneDeep, mergeWith } from 'lodash';
-import { fromExpression, toExpression, Ast, ExpressionFunctionAST } from '@kbn/interpreter';
+import { fromExpression, toExpression, Ast, AstFunction } from '@kbn/interpreter';
 import {
   SavedObjectMigrationMap,
   SavedObjectMigrationFn,
@@ -141,7 +141,7 @@ const removeLensAutoDate: SavedObjectMigrationFn<LensDocShapePre710, LensDocShap
   }
   try {
     const ast = fromExpression(expression);
-    const newChain: ExpressionFunctionAST[] = ast.chain.map((topNode) => {
+    const newChain: AstFunction[] = ast.chain.map((topNode) => {
       if (topNode.function !== 'lens_merge_tables') {
         return topNode;
       }
@@ -202,7 +202,7 @@ const addTimeFieldToEsaggs: SavedObjectMigrationFn<LensDocShapePre710, LensDocSh
 
   try {
     const ast = fromExpression(expression);
-    const newChain: ExpressionFunctionAST[] = ast.chain.map((topNode) => {
+    const newChain: AstFunction[] = ast.chain.map((topNode) => {
       if (topNode.function !== 'lens_merge_tables') {
         return topNode;
       }