From e560bef51fdff622d51eed7d8eda932e81a45525 Mon Sep 17 00:00:00 2001 From: dej611 Date: Fri, 30 Sep 2022 09:11:19 +0200 Subject: [PATCH 01/25] :sparkles: Introduce new comparison functions --- .../src/functions/comparison/eq.js | 36 +++++++++++++++++++ .../src/functions/comparison/gt.js | 36 +++++++++++++++++++ .../src/functions/comparison/gte.js | 29 +++++++++++++++ .../src/functions/comparison/ifelse.js | 30 ++++++++++++++++ .../src/functions/comparison/index.js | 16 +++++++++ .../src/functions/comparison/lt.js | 36 +++++++++++++++++++ .../src/functions/comparison/lte.js | 29 +++++++++++++++ packages/kbn-tinymath/src/functions/index.js | 7 ++++ 8 files changed, 219 insertions(+) create mode 100644 packages/kbn-tinymath/src/functions/comparison/eq.js create mode 100644 packages/kbn-tinymath/src/functions/comparison/gt.js create mode 100644 packages/kbn-tinymath/src/functions/comparison/gte.js create mode 100644 packages/kbn-tinymath/src/functions/comparison/ifelse.js create mode 100644 packages/kbn-tinymath/src/functions/comparison/index.js create mode 100644 packages/kbn-tinymath/src/functions/comparison/lt.js create mode 100644 packages/kbn-tinymath/src/functions/comparison/lte.js diff --git a/packages/kbn-tinymath/src/functions/comparison/eq.js b/packages/kbn-tinymath/src/functions/comparison/eq.js new file mode 100644 index 0000000000000..9692e856d99fb --- /dev/null +++ b/packages/kbn-tinymath/src/functions/comparison/eq.js @@ -0,0 +1,36 @@ +/* + * 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. + */ + +/** + * Performs an equality comparison between two values. + * @param {number|number[]} a a number or an array of numbers + * @param {number|number[]} b a number or an array of numbers + * @return {boolean} Returns true if `a` and `b` are equal, false otherwise. Returns an array with the equality comparison of each element if `a` is an array. + * @throws `'Array length mismatch'` if `args` contains arrays of different lengths + * @example + * eq(1, 1) // returns true + * eq(1, 2) // returns false + * eq([1, 2], 1) // returns [true, false] + * eq([1, 2], [1, 2]) // returns [true, true] + */ + +module.exports = { eq }; + +function eq(a, b) { + if (Array.isArray(a)) { + if (!Array.isArray(b)) { + return a.every((v) => v === b); + } + if (a.length !== b.length) { + throw new Error('Array length mismatch'); + } + return a.every((v, i) => v === b[i]); + } + + return a === b; +} diff --git a/packages/kbn-tinymath/src/functions/comparison/gt.js b/packages/kbn-tinymath/src/functions/comparison/gt.js new file mode 100644 index 0000000000000..0eee8a17e7cd9 --- /dev/null +++ b/packages/kbn-tinymath/src/functions/comparison/gt.js @@ -0,0 +1,36 @@ +/* + * 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. + */ + +/** + * Performs a greater than comparison between two values. + * @param {number|number[]} a a number or an array of numbers + * @param {number|number[]} b a number or an array of numbers + * @return {boolean} Returns true if `a` is greater than `b`, false otherwise. Returns an array with the greater than comparison of each element if `a` is an array. + * @throws `'Array length mismatch'` if `args` contains arrays of different lengths + * @example + * gt(1, 1) // returns false + * gt(2, 1) // returns true + * gt([1, 2], 1) // returns [true, false] + * gt([1, 2], [2, 1]) // returns [false, true] + */ + +module.exports = { gt }; + +function gt(a, b) { + if (Array.isArray(a)) { + if (!Array.isArray(b)) { + return a.every((v) => v > b); + } + if (a.length !== b.length) { + throw new Error('Array length mismatch'); + } + return a.every((v, i) => v > b[i]); + } + + return a > b; +} diff --git a/packages/kbn-tinymath/src/functions/comparison/gte.js b/packages/kbn-tinymath/src/functions/comparison/gte.js new file mode 100644 index 0000000000000..4ba67f441d669 --- /dev/null +++ b/packages/kbn-tinymath/src/functions/comparison/gte.js @@ -0,0 +1,29 @@ +/* + * 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. + */ + +const { eq } = require('./eq'); +const { gt } = require('./gt'); + +/** + * Performs a greater than or equal comparison between two values. + * @param {number|number[]} a a number or an array of numbers + * @param {number|number[]} b a number or an array of numbers + * @return {boolean} Returns true if `a` is greater than or equal to `b`, false otherwise. Returns an array with the greater than or equal comparison of each element if `a` is an array. + * @throws `'Array length mismatch'` if `args` contains arrays of different lengths + * @example + * gte(1, 1) // returns true + * gte(1, 2) // returns false + * gte([1, 2], 2) // returns [false, true] + * gte([1, 2], [1, 1]) // returns [true, true] + */ + +module.exports = { gte }; + +function gte(a, b) { + return eq(a, b) || gt(a, b); +} diff --git a/packages/kbn-tinymath/src/functions/comparison/ifelse.js b/packages/kbn-tinymath/src/functions/comparison/ifelse.js new file mode 100644 index 0000000000000..0e8be7245d19a --- /dev/null +++ b/packages/kbn-tinymath/src/functions/comparison/ifelse.js @@ -0,0 +1,30 @@ +/* + * 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. + */ + +/** + * Evaluates the a conditional argument and returns one of the two values based on that. + * @param {(boolean)} cond a boolean value + * @param {(any|any[])} a a value or an array of any values + * @param {(any|any[])} b a value or an array of any values + * @return {(any|any[])} if the value of cond is truthy, return `a`, otherwise return `b`. + * @example + * ifelse( 5 > 6, 1, 0) // returns 0 + * ifelse( 1 == 1, [1, 2, 3], 5) // returns [1, 2, 3] + * ifelse( 1 < 2, [1, 2, 3], [2, 3, 4]) // returns [1, 2, 3] + */ + +module.exports = { ifelse }; + +function ifelse(cond, whenTrue, whenFalse) { + if (typeof cond !== 'boolean') { + throw Error('Condition clause is of the wrong type'); + } + return cond ? whenTrue : whenFalse; +} + +ifelse.skipNumberValidation = true; diff --git a/packages/kbn-tinymath/src/functions/comparison/index.js b/packages/kbn-tinymath/src/functions/comparison/index.js new file mode 100644 index 0000000000000..b20cc39014a51 --- /dev/null +++ b/packages/kbn-tinymath/src/functions/comparison/index.js @@ -0,0 +1,16 @@ +/* + * 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. + */ + +const { eq } = require('./eq'); +const { lt } = require('./lt'); +const { gt } = require('./gt'); +const { lte } = require('./lte'); +const { gte } = require('./gte'); +const { ifelse } = require('./ifelse'); + +module.exports = { eq, lt, gt, lte, gte, ifelse }; diff --git a/packages/kbn-tinymath/src/functions/comparison/lt.js b/packages/kbn-tinymath/src/functions/comparison/lt.js new file mode 100644 index 0000000000000..edfce00724088 --- /dev/null +++ b/packages/kbn-tinymath/src/functions/comparison/lt.js @@ -0,0 +1,36 @@ +/* + * 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. + */ + +/** + * Performs a lower than comparison between two values. + * @param {number|number[]} a a number or an array of numbers + * @param {number|number[]} b a number or an array of numbers + * @return {boolean} Returns true if `a` is lower than `b`, false otherwise. Returns an array with the lower than comparison of each element if `a` is an array. + * @throws `'Array length mismatch'` if `args` contains arrays of different lengths + * @example + * lt(1, 1) // returns false + * lt(1, 2) // returns true + * lt([1, 2], 2) // returns [true, false] + * lt([1, 2], [1, 2]) // returns [false, false] + */ + +module.exports = { lt }; + +function lt(a, b) { + if (Array.isArray(a)) { + if (!Array.isArray(b)) { + return a.every((v) => v < b); + } + if (a.length !== b.length) { + throw new Error('Array length mismatch'); + } + return a.every((v, i) => v < b[i]); + } + + return a < b; +} diff --git a/packages/kbn-tinymath/src/functions/comparison/lte.js b/packages/kbn-tinymath/src/functions/comparison/lte.js new file mode 100644 index 0000000000000..47ab7f024ef4c --- /dev/null +++ b/packages/kbn-tinymath/src/functions/comparison/lte.js @@ -0,0 +1,29 @@ +/* + * 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. + */ + +const { eq } = require('./eq'); +const { lt } = require('./lt'); + +/** + * Performs a lower than or equal comparison between two values. + * @param {number|number[]} a a number or an array of numbers + * @param {number|number[]} b a number or an array of numbers + * @return {boolean} Returns true if `a` is lower than or equal to `b`, false otherwise. Returns an array with the lower than or equal comparison of each element if `a` is an array. + * @throws `'Array length mismatch'` if `args` contains arrays of different lengths + * @example + * lte(1, 1) // returns true + * lte(1, 2) // returns true + * lte([1, 2], 2) // returns [true, true] + * lte([1, 2], [1, 1]) // returns [true, false] + */ + +module.exports = { lte }; + +function lte(a, b) { + return eq(a, b) || lt(a, b); +} diff --git a/packages/kbn-tinymath/src/functions/index.js b/packages/kbn-tinymath/src/functions/index.js index eb58a4d56b569..bc3443718cb82 100644 --- a/packages/kbn-tinymath/src/functions/index.js +++ b/packages/kbn-tinymath/src/functions/index.js @@ -44,6 +44,7 @@ const { subtract } = require('./subtract'); const { sum } = require('./sum'); const { tan } = require('./tan'); const { unique } = require('./unique'); +const { eq, lt, gt, lte, gte, ifelse } = require('./comparison'); module.exports = { functions: { @@ -61,6 +62,7 @@ module.exports = { first, fix, floor, + ifelse, last, log, log10, @@ -85,5 +87,10 @@ module.exports = { sum, tan, unique, + eq, + lt, + gt, + lte, + gte, }, }; From 6a78199216d1bb09e82c14d27458c2c2c2653e31 Mon Sep 17 00:00:00 2001 From: dej611 Date: Fri, 30 Sep 2022 09:12:37 +0200 Subject: [PATCH 02/25] :sparkles: Introduce new comparison symbols into grammar --- packages/kbn-tinymath/grammar/grammar.peggy | 69 ++++++++++++++------- 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/packages/kbn-tinymath/grammar/grammar.peggy b/packages/kbn-tinymath/grammar/grammar.peggy index 414bc2fa11cb7..8ed9224eaff6a 100644 --- a/packages/kbn-tinymath/grammar/grammar.peggy +++ b/packages/kbn-tinymath/grammar/grammar.peggy @@ -11,6 +11,32 @@ max: location.end.offset } } + + const symbolsToFn = { + '+': 'add', '-': 'subtract', + '*': 'multiply', '/': 'divide', + '<': 'lt', '>': 'gt', '==': 'eq', + '<=': 'lte', '>=': 'gte', + } + + // Shared function for AST operations + function parseSymbol(left, rest){ + const topLevel = rest.reduce((acc, [name, right]) => ({ + type: 'function', + name: symbolsToFn[name], + args: [acc, right], + }), left); + if (typeof topLevel === 'object') { + topLevel.location = simpleLocation(location()); + topLevel.text = text(); + } + return topLevel; + } + + // op is always defined, while eq can be null for gt and lt cases + function getComparisonSymbol([op, eq]){ + return symbolsToFn[op+(eq || '')]; + } } start @@ -71,38 +97,35 @@ Variable // expressions Expression + = Comparison + / MathOperation + +MathOperation = AddSubtract + / MultiplyDivide + / Factor + +Comparison + = _ left:MathOperation op:(('>' / '<')('=')? / '=''=') right:MathOperation _ { + return { + type: 'function', + name: getComparisonSymbol(op), + args: [left, right], + location: simpleLocation(location()), + text: text() + }; + } AddSubtract = _ left:MultiplyDivide rest:(('+' / '-') MultiplyDivide)+ _ { - const topLevel = rest.reduce((acc, curr) => ({ - type: 'function', - name: curr[0] === '+' ? 'add' : 'subtract', - args: [acc, curr[1]], - }), left); - if (typeof topLevel === 'object') { - topLevel.location = simpleLocation(location()); - topLevel.text = text(); - } - return topLevel; + return parseSymbol(left, rest, {'+': 'add', '-': 'subtract'}); } - / MultiplyDivide MultiplyDivide = _ left:Factor rest:(('*' / '/') Factor)* _ { - const topLevel = rest.reduce((acc, curr) => ({ - type: 'function', - name: curr[0] === '*' ? 'multiply' : 'divide', - args: [acc, curr[1]], - }), left); - if (typeof topLevel === 'object') { - topLevel.location = simpleLocation(location()); - topLevel.text = text(); - } - return topLevel; + return parseSymbol(left, rest, {'*': 'multiply', '/': 'divide'}); } - / Factor - + Factor = Group / Function From 96e27ccf36c9ada66b61c753639c95157d73fa41 Mon Sep 17 00:00:00 2001 From: dej611 Date: Fri, 30 Sep 2022 09:25:20 +0200 Subject: [PATCH 03/25] :wrench: Introduce new tinymath functions --- .../operations/definitions/formula/util.ts | 156 +++++++++++++++++- 1 file changed, 155 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts index 29293d598d521..574ce62d65b12 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts @@ -109,10 +109,12 @@ function getTypeI18n(type: string) { if (type === 'string') { return i18n.translate('xpack.lens.formula.string', { defaultMessage: 'string' }); } + if (type === 'boolean') { + return i18n.translate('xpack.lens.formula.boolean', { defaultMessage: 'boolean' }); + } return ''; } -// Todo: i18n everything here export const tinymathFunctions: Record< string, { @@ -125,6 +127,9 @@ export const tinymathFunctions: Record< }>; // Help is in Markdown format help: string; + // When omitted defaults to "number". + // Used for comparison functions return type + outputType?: string; } > = { add: { @@ -527,6 +532,155 @@ Example: Find the minimum between two fields averages `, }), }, + lt: { + positionalArguments: [ + { + name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), + type: getTypeI18n('number'), + }, + { + name: i18n.translate('xpack.lens.formula.right', { defaultMessage: 'right' }), + type: getTypeI18n('number'), + }, + ], + outputType: getTypeI18n('boolean'), + help: i18n.translate('xpack.lens.formula.ltFunction.markdown', { + defaultMessage: ` +Performs a lower than comparison between two values. +To be used as condition for \`ifelse\` comparison function. +Also works with \`<\` symbol. + +Example: Compare two fields average and return the lowest +\`ifelse(average(memory) < average(bytes), average(memory), average(bytes))\` + +Example: \`ifelse(lt(average(bytes), 1000), 1, 0)\` + `, + }), + }, + gt: { + positionalArguments: [ + { + name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), + type: getTypeI18n('number'), + }, + { + name: i18n.translate('xpack.lens.formula.right', { defaultMessage: 'right' }), + type: getTypeI18n('number'), + }, + ], + outputType: getTypeI18n('boolean'), + help: i18n.translate('xpack.lens.formula.gtFunction.markdown', { + defaultMessage: ` +Performs a greater than comparison between two values. +To be used as condition for \`ifelse\` comparison function. +Also works with \`>\` symbol. + +Example: Compare two fields average and return the highest +\`ifelse(average(memory) > average(bytes), average(memory), average(bytes))\` + +Example: \`ifelse(gt(average(bytes), 1000), 1, 0)\` + `, + }), + }, + eq: { + positionalArguments: [ + { + name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), + type: getTypeI18n('number'), + }, + { + name: i18n.translate('xpack.lens.formula.right', { defaultMessage: 'right' }), + type: getTypeI18n('number'), + }, + ], + outputType: getTypeI18n('boolean'), + help: i18n.translate('xpack.lens.formula.eqFunction.markdown', { + defaultMessage: ` +Performs an equality comparison between two values. +To be used as condition for \`ifelse\` comparison function. +Also works with \`==\` symbol. + +Example: Returns 1 if two sums of a field match exactly the same value +\`ifelse(sum(revenues) == sum(costs), min(revenues), min(costs))\` + +Example: \`ifelse(eq(sum(revenues), sum(costs)), 0, 1)\` + `, + }), + }, + lte: { + positionalArguments: [ + { + name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), + type: getTypeI18n('number'), + }, + { + name: i18n.translate('xpack.lens.formula.right', { defaultMessage: 'right' }), + type: getTypeI18n('number'), + }, + ], + outputType: getTypeI18n('boolean'), + help: i18n.translate('xpack.lens.formula.lteFunction.markdown', { + defaultMessage: ` +Performs a lower than or equal comparison between two values. +To be used as condition for \`ifelse\` comparison function. +Also works with \`<=\` symbol. + +Example: Compare two fields average and return the lowest +\`ifelse(average(memory) <= average(bytes), average(memory), average(bytes))\` + +Example: \`ifelse(lte(average(bytes), 1000), 1, 0)\` + `, + }), + }, + gte: { + positionalArguments: [ + { + name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), + type: getTypeI18n('number'), + }, + { + name: i18n.translate('xpack.lens.formula.right', { defaultMessage: 'right' }), + type: getTypeI18n('number'), + }, + ], + outputType: getTypeI18n('boolean'), + help: i18n.translate('xpack.lens.formula.gteFunction.markdown', { + defaultMessage: ` +Performs a greater than comparison between two values. +To be used as condition for \`ifelse\` comparison function. +Also works with \`>\` symbol. + +Example: Compare two fields average and return the highest +\`ifelse(average(memory) >= average(bytes), average(memory), average(bytes))\` + +Example: \`ifelse(gte(average(bytes), 1000), 1, 0)\` + `, + }), + }, + ifelse: { + positionalArguments: [ + { + name: i18n.translate('xpack.lens.formula.condition', { defaultMessage: 'condition' }), + type: getTypeI18n('boolean'), + }, + { + name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), + type: getTypeI18n('number'), + }, + { + name: i18n.translate('xpack.lens.formula.right', { defaultMessage: 'right' }), + type: getTypeI18n('number'), + }, + ], + help: i18n.translate('xpack.lens.formula.ifElseFunction.markdown', { + defaultMessage: ` +Returns a value depending on whether the element of condition is true or false. + +Example: Compare two fields average and return the highest +\`ifelse(average(memory) >= average(bytes), average(memory), average(bytes))\` + `, + }), + }, }; export function isMathNode(node: TinymathAST | string) { From c74cf676976ccabbdf1b7c2acec8fc135ccd8711 Mon Sep 17 00:00:00 2001 From: dej611 Date: Fri, 30 Sep 2022 11:46:53 +0200 Subject: [PATCH 04/25] :sparkles: Add comparison fn validation to formula --- .../formula/editor/formula_help.tsx | 158 +++++++++++------- .../operations/definitions/formula/util.ts | 61 +++++-- .../definitions/formula/validation.ts | 67 +++++++- 3 files changed, 207 insertions(+), 79 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/editor/formula_help.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/editor/formula_help.tsx index 0a1cf746c19f8..e250edab5a445 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/editor/formula_help.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/editor/formula_help.tsx @@ -21,6 +21,7 @@ import { EuiSpacer, } from '@elastic/eui'; import { Markdown } from '@kbn/kibana-react-plugin/public'; +import { groupBy } from 'lodash'; import type { IndexPattern } from '../../../../../types'; import { tinymathFunctions } from '../util'; import { getPossibleFunctions } from './math_completion'; @@ -193,31 +194,40 @@ max(system.network.in.bytes, reducedTimeRange="30m") items: [], }); - const availableFunctions = getPossibleFunctions(indexPattern); + const { + elasticsearch: esFunction, + calculation: calculationFunction, + math: mathOperations, + comparison: comparisonOperations, + } = useMemo( + () => + groupBy(getPossibleFunctions(indexPattern), (key) => { + if (key in operationDefinitionMap) { + return operationDefinitionMap[key].documentation?.section; + } + if (key in tinymathFunctions) { + return tinymathFunctions[key].section; + } + }), + [operationDefinitionMap, indexPattern] + ); // Es aggs helpGroups[2].items.push( - ...availableFunctions - .filter( - (key) => - key in operationDefinitionMap && - operationDefinitionMap[key].documentation?.section === 'elasticsearch' - ) - .sort() - .map((key) => ({ - label: key, - description: ( - <> -

- {key}({operationDefinitionMap[key].documentation?.signature}) -

- - {operationDefinitionMap[key].documentation?.description ? ( - - ) : null} - - ), - })) + ...esFunction.sort().map((key) => ({ + label: key, + description: ( + <> +

+ {key}({operationDefinitionMap[key].documentation?.signature}) +

+ + {operationDefinitionMap[key].documentation?.description ? ( + + ) : null} + + ), + })) ); helpGroups.push({ @@ -236,31 +246,24 @@ max(system.network.in.bytes, reducedTimeRange="30m") // Calculations aggs helpGroups[3].items.push( - ...availableFunctions - .filter( - (key) => - key in operationDefinitionMap && - operationDefinitionMap[key].documentation?.section === 'calculation' - ) - .sort() - .map((key) => ({ - label: key, - description: ( - <> -

- {key}({operationDefinitionMap[key].documentation?.signature}) -

- - {operationDefinitionMap[key].documentation?.description ? ( - - ) : null} - - ), - checked: - selectedFunction === `${key}: ${operationDefinitionMap[key].displayName}` - ? ('on' as const) - : undefined, - })) + ...calculationFunction.sort().map((key) => ({ + label: key, + description: ( + <> +

+ {key}({operationDefinitionMap[key].documentation?.signature}) +

+ + {operationDefinitionMap[key].documentation?.description ? ( + + ) : null} + + ), + checked: + selectedFunction === `${key}: ${operationDefinitionMap[key].displayName}` + ? ('on' as const) + : undefined, + })) ); helpGroups.push({ @@ -274,22 +277,55 @@ max(system.network.in.bytes, reducedTimeRange="30m") items: [], }); - const tinymathFns = useMemo(() => { - return getPossibleFunctions(indexPattern) - .filter((key) => key in tinymathFunctions) - .sort() - .map((key) => { - const [description, examples] = tinymathFunctions[key].help.split(`\`\`\``); - return { - label: key, - description: description.replace(/\n/g, '\n\n'), - examples: examples ? `\`\`\`${examples}\`\`\`` : '', - }; - }); - }, [indexPattern]); + const mathFns = useMemo(() => { + return mathOperations.sort().map((key) => { + const [description, examples] = tinymathFunctions[key].help.split(`\`\`\``); + return { + label: key, + description: description.replace(/\n/g, '\n\n'), + examples: examples ? `\`\`\`${examples}\`\`\`` : '', + }; + }); + }, [mathOperations]); helpGroups[4].items.push( - ...tinymathFns.map(({ label, description, examples }) => { + ...mathFns.map(({ label, description, examples }) => { + return { + label, + description: ( + <> +

{getFunctionSignatureLabel(label, operationDefinitionMap)}

+ + + + ), + }; + }) + ); + + helpGroups.push({ + label: i18n.translate('xpack.lens.formulaDocumentation.comparisonSection', { + defaultMessage: 'Comparison', + }), + description: i18n.translate('xpack.lens.formulaDocumentation.comparisonSectionDescription', { + defaultMessage: 'These functions are used to perform value comparison.', + }), + items: [], + }); + + const comparisonFns = useMemo(() => { + return comparisonOperations.sort().map((key) => { + const [description, examples] = tinymathFunctions[key].help.split(`\`\`\``); + return { + label: key, + description: description.replace(/\n/g, '\n\n'), + examples: examples ? `\`\`\`${examples}\`\`\`` : '', + }; + }); + }, [comparisonOperations]); + + helpGroups[5].items.push( + ...comparisonFns.map(({ label, description, examples }) => { return { label, description: ( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts index 574ce62d65b12..3626b364ecf23 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts @@ -118,6 +118,7 @@ function getTypeI18n(type: string) { export const tinymathFunctions: Record< string, { + section: 'math' | 'comparison'; positionalArguments: Array<{ name: string; optional?: boolean; @@ -133,6 +134,7 @@ export const tinymathFunctions: Record< } > = { add: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), @@ -159,6 +161,7 @@ Example: Offset count by a static value }), }, subtract: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), @@ -180,6 +183,7 @@ Example: Calculate the range of a field }), }, multiply: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), @@ -204,6 +208,7 @@ Example: Calculate price after constant tax rate }), }, divide: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), @@ -227,6 +232,7 @@ Example: \`divide(sum(bytes), 2)\` }), }, abs: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -242,6 +248,7 @@ Example: Calculate average distance to sea level \`abs(average(altitude))\` }), }, cbrt: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -258,6 +265,7 @@ Example: Calculate side length from volume }), }, ceil: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -274,6 +282,7 @@ Example: Round up price to the next dollar }), }, clamp: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -306,6 +315,7 @@ clamp( }), }, cube: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -322,6 +332,7 @@ Example: Calculate volume from side length }), }, exp: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -339,6 +350,7 @@ Example: Calculate the natural exponential function }), }, fix: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -355,6 +367,7 @@ Example: Rounding towards zero }), }, floor: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -371,6 +384,7 @@ Example: Round down a price }), }, log: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -396,6 +410,7 @@ log(sum(bytes), 2) }), }, mod: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -416,6 +431,7 @@ Example: Calculate last three digits of a value }), }, pow: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -436,6 +452,7 @@ Example: Calculate volume based on side length }), }, round: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -461,6 +478,7 @@ round(sum(bytes), 2) }), }, sqrt: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -477,6 +495,7 @@ Example: Calculate side length based on area }), }, square: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.value', { defaultMessage: 'value' }), @@ -493,6 +512,7 @@ Example: Calculate area based on side length }), }, pick_max: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), @@ -513,6 +533,7 @@ Example: Find the maximum between two fields averages }), }, pick_min: { + section: 'math', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), @@ -533,6 +554,7 @@ Example: Find the minimum between two fields averages }), }, lt: { + section: 'comparison', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), @@ -550,14 +572,15 @@ Performs a lower than comparison between two values. To be used as condition for \`ifelse\` comparison function. Also works with \`<\` symbol. -Example: Compare two fields average and return the lowest -\`ifelse(average(memory) < average(bytes), average(memory), average(bytes))\` +Example: Returns true if the average of bytes is lower than the average amount of memory +\`average(bytes) <= average(memory)\` -Example: \`ifelse(lt(average(bytes), 1000), 1, 0)\` +Example: \`lt(average(bytes), 1000)\` `, }), }, gt: { + section: 'comparison', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), @@ -575,14 +598,15 @@ Performs a greater than comparison between two values. To be used as condition for \`ifelse\` comparison function. Also works with \`>\` symbol. -Example: Compare two fields average and return the highest -\`ifelse(average(memory) > average(bytes), average(memory), average(bytes))\` +Example: Returns true if the average of bytes is greater than the average amount of memory +\`average(bytes) > average(memory)\` -Example: \`ifelse(gt(average(bytes), 1000), 1, 0)\` +Example: \`gt(average(bytes), 1000)\` `, }), }, eq: { + section: 'comparison', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), @@ -600,14 +624,15 @@ Performs an equality comparison between two values. To be used as condition for \`ifelse\` comparison function. Also works with \`==\` symbol. -Example: Returns 1 if two sums of a field match exactly the same value -\`ifelse(sum(revenues) == sum(costs), min(revenues), min(costs))\` +Example: Returns true if the average of bytes is exactly the same amount of average memory +\`average(bytes) == average(memory)\` -Example: \`ifelse(eq(sum(revenues), sum(costs)), 0, 1)\` +Example: \`eq(sum(bytes), 1000000)\` `, }), }, lte: { + section: 'comparison', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), @@ -625,14 +650,15 @@ Performs a lower than or equal comparison between two values. To be used as condition for \`ifelse\` comparison function. Also works with \`<=\` symbol. -Example: Compare two fields average and return the lowest -\`ifelse(average(memory) <= average(bytes), average(memory), average(bytes))\` +Example: Returns true if the average of bytes is lower than or equal to the average amount of memory +\`average(bytes) <= average(memory)\` -Example: \`ifelse(lte(average(bytes), 1000), 1, 0)\` +Example: \`lte(average(bytes), 1000)\` `, }), }, gte: { + section: 'comparison', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.left', { defaultMessage: 'left' }), @@ -648,16 +674,17 @@ Example: \`ifelse(lte(average(bytes), 1000), 1, 0)\` defaultMessage: ` Performs a greater than comparison between two values. To be used as condition for \`ifelse\` comparison function. -Also works with \`>\` symbol. +Also works with \`>=\` symbol. -Example: Compare two fields average and return the highest -\`ifelse(average(memory) >= average(bytes), average(memory), average(bytes))\` +Example: Returns true if the average of bytes is greater than or equal to the average amount of memory +\`average(bytes) >= average(memory)\` -Example: \`ifelse(gte(average(bytes), 1000), 1, 0)\` +Example: \`gte(average(bytes), 1000)\` `, }), }, ifelse: { + section: 'comparison', positionalArguments: [ { name: i18n.translate('xpack.lens.formula.condition', { defaultMessage: 'condition' }), @@ -677,7 +704,7 @@ Example: \`ifelse(gte(average(bytes), 1000), 1, 0)\` Returns a value depending on whether the element of condition is true or false. Example: Compare two fields average and return the highest -\`ifelse(average(memory) >= average(bytes), average(memory), average(bytes))\` +\`ifelse( average(memory) >= average(bytes), average(memory), average(bytes))\` `, }), }, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts index a7ffc57a2361e..5e0fe3a1d2676 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts @@ -45,6 +45,10 @@ interface ValidationErrors { message: string; type: { operation: string; params: string }; }; + wrongTypeArgument: { + message: string; + type: { operation: string; name: string; type: string; expectedType: string }; + }; wrongFirstArgument: { message: string; type: { operation: string; type: string; argument: string | number }; @@ -260,6 +264,18 @@ function getMessageFromId({ values: { operation: out.operation, params: out.params }, }); break; + case 'wrongTypeArgument': + message = i18n.translate('xpack.lens.indexPattern.formulaExpressionWrongTypeArgument', { + defaultMessage: + 'The {name} argument for the operation {operation} in the Formula is of the wrong type: {expectedType} instead of {type}', + values: { + operation: out.operation, + name: out.name, + type: out.type, + expectedType: out.expectedType, + }, + }); + break; case 'duplicateArgument': message = i18n.translate('xpack.lens.indexPattern.formulaOperationDuplicateParams', { defaultMessage: @@ -404,6 +420,7 @@ export function runASTValidation( ) { return [ ...checkMissingVariableOrFunctions(ast, layer, indexPattern, operations), + ...checkTopNodeReturnType(ast), ...runFullASTValidation(ast, layer, indexPattern, operations, currentColumn), ]; } @@ -621,6 +638,27 @@ function validateNameArguments( return errors; } +const DEFAULT_RETURN_TYPE = 'number'; +function checkTopNodeReturnType(ast: TinymathAST): ErrorWrapper[] { + if ( + isObject(ast) && + ast.type === 'function' && + ast.text && + (tinymathFunctions[ast.name]?.outputType || DEFAULT_RETURN_TYPE) !== DEFAULT_RETURN_TYPE + ) { + return [ + getMessageFromId({ + messageId: 'wrongReturnedType', + values: { + text: ast.text, + }, + locations: getNodeLocation(ast), + }), + ]; + } + return []; +} + function runFullASTValidation( ast: TinymathAST, layer: IndexPatternLayer, @@ -978,7 +1016,34 @@ export function validateMathNodes(root: TinymathAST, missingVariableSet: Set { + const arg = node.args[index]; + if (arg != null) { + if (!isObject(arg)) { + return typeof arg !== type; + } + if (arg.type === 'function' && tinymathFunctions[arg.name]) { + const { outputType = 'number' } = tinymathFunctions[arg.name]; + return outputType !== type; + } + } + }); + if (wrongTypeArgumentIndex > -1) { + errors.push( + getMessageFromId({ + messageId: 'wrongTypeArgument', + values: { + operation: node.name, + name: positionalArguments[wrongTypeArgumentIndex].name, + type: typeof node.args[wrongTypeArgumentIndex], + expectedType: positionalArguments[wrongTypeArgumentIndex].type || '', + }, + locations: node.location ? [node.location] : [], + }) + ); + } + + // no need to iterate all the arguments, one field is enough to trigger the error const hasFieldAsArgument = positionalArguments.some((requirements, index) => { const arg = node.args[index]; if (arg != null && typeof arg !== 'number') { From 5784a158c16ec3d1618da7bc3a215af9299a7871 Mon Sep 17 00:00:00 2001 From: dej611 Date: Fri, 30 Sep 2022 11:47:54 +0200 Subject: [PATCH 05/25] :recycle: Some type refactoring --- .../formula/editor/formula_editor.tsx | 4 +- .../formula/editor/math_completion.ts | 6 +- .../definitions/formula/formula.tsx | 4 +- .../operations/definitions/formula/parse.ts | 7 ++- .../operations/definitions/formula/util.ts | 6 +- .../definitions/formula/validation.ts | 58 ++++++++++--------- 6 files changed, 48 insertions(+), 37 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/editor/formula_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/editor/formula_editor.tsx index d742b0497a85c..b7fc648986ed7 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/editor/formula_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/editor/formula_editor.tsx @@ -48,7 +48,7 @@ import { MemoizedFormulaHelp } from './formula_help'; import './formula.scss'; import { FormulaIndexPatternColumn } from '../formula'; import { insertOrReplaceFormulaColumn } from '../parse'; -import { filterByVisibleOperation } from '../util'; +import { filterByVisibleOperation, nonNullable } from '../util'; import { getColumnTimeShiftWarnings, getDateHistogramInterval } from '../../../../time_shift_utils'; function tableHasData( @@ -363,7 +363,7 @@ export function FormulaEditor({ } return newWarnings; }) - .filter((marker) => marker); + .filter(nonNullable); setWarnings(markers.map(({ severity, message }) => ({ severity, message }))); monaco.editor.setModelMarkers(editorModel.current, 'LENS', markers); } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/editor/math_completion.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/editor/math_completion.ts index 94b8dcc2a9681..de103466c00e0 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/editor/math_completion.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/editor/math_completion.ts @@ -24,7 +24,7 @@ import type { DataViewsPublicPluginStart } from '@kbn/data-views-plugin/public'; import { parseTimeShift } from '@kbn/data-plugin/common'; import type { IndexPattern } from '../../../../../types'; import { memoizedGetAvailableOperationsByMetadata } from '../../../operations'; -import { tinymathFunctions, groupArgsByType, unquotedStringRegex } from '../util'; +import { tinymathFunctions, groupArgsByType, unquotedStringRegex, nonNullable } from '../util'; import type { GenericOperationDefinition } from '../..'; import { getFunctionSignatureLabel, getHelpTextContent } from './formula_help'; import { hasFunctionFieldArgument } from '../validation'; @@ -78,7 +78,7 @@ export function getInfoAtZeroIndexedPosition( if (ast.type === 'function') { const [match] = ast.args .map((arg) => getInfoAtZeroIndexedPosition(arg, zeroIndexedPosition, ast)) - .filter((a) => a); + .filter(nonNullable); if (match) { return match; } else if (ast.location) { @@ -297,7 +297,7 @@ function getArgumentSuggestions( const fields = validOperation.operations .filter((op) => op.operationType === operation.type) .map((op) => ('field' in op ? op.field : undefined)) - .filter((field) => field); + .filter(nonNullable); const fieldArg = ast.args[0]; const location = typeof fieldArg !== 'string' && (fieldArg as TinymathVariable).location; let range: monaco.IRange | undefined; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.tsx index be5a3f30282f9..63dfb0ebdb054 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.tsx @@ -13,7 +13,7 @@ import { runASTValidation, tryToParse } from './validation'; import { WrappedFormulaEditor } from './editor'; import { insertOrReplaceFormulaColumn } from './parse'; import { generateFormula } from './generate'; -import { filterByVisibleOperation } from './util'; +import { filterByVisibleOperation, nonNullable } from './util'; import { getManagedColumnsFrom } from '../../layer_helpers'; import { getFilter, isColumnFormatted } from '../helpers'; @@ -85,7 +85,7 @@ export const formulaOperation: OperationDefinition marker); + .filter(nonNullable); const hasBuckets = layer.columnOrder.some((colId) => layer.columns[colId].isBucketed); const hasOtherMetrics = layer.columnOrder.some((colId) => { const col = layer.columns[colId]; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/parse.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/parse.ts index 5c9a277070936..c7cacb511c014 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/parse.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/parse.ts @@ -25,6 +25,7 @@ import { getOperationParams, groupArgsByType, mergeWithGlobalFilter, + nonNullable, } from './util'; import { FormulaIndexPatternColumn, isFormulaIndexPatternColumn } from './formula'; import { getColumnOrder } from '../../layer_helpers'; @@ -88,9 +89,9 @@ function extractColumns( const nodeOperation = operations[node.name]; if (!nodeOperation) { // it's a regular math node - const consumedArgs = node.args - .map(parseNode) - .filter((n) => typeof n !== 'undefined' && n !== null) as Array; + const consumedArgs = node.args.map(parseNode).filter(nonNullable) as Array< + number | TinymathVariable + >; return { ...node, args: consumedArgs, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts index 3626b364ecf23..711cccf0a4b51 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts @@ -710,6 +710,10 @@ Example: Compare two fields average and return the highest }, }; +export function nonNullable(v: T): v is NonNullable { + return v != null; +} + export function isMathNode(node: TinymathAST | string) { return isObject(node) && node.type === 'function' && tinymathFunctions[node.name]; } @@ -719,7 +723,7 @@ export function findMathNodes(root: TinymathAST | string): TinymathFunction[] { if (!isObject(node) || node.type !== 'function' || !isMathNode(node)) { return []; } - return [node, ...node.args.flatMap(flattenMathNodes)].filter(Boolean); + return [node, ...node.args.flatMap(flattenMathNodes)].filter(nonNullable); } return flattenMathNodes(root); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts index 5e0fe3a1d2676..11f7c87c68f7c 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts @@ -19,6 +19,7 @@ import { getValueOrName, groupArgsByType, isMathNode, + nonNullable, tinymathFunctions, } from './util'; @@ -113,6 +114,10 @@ export interface ErrorWrapper { severity?: 'error' | 'warning'; } +function getNodeLocation(node: TinymathFunction): TinymathLocation[] { + return [node.location].filter(nonNullable); +} + export function isParsingError(message: string) { return message.includes('Failed to parse expression'); } @@ -122,7 +127,7 @@ function findFunctionNodes(root: TinymathAST | string): TinymathFunction[] { if (!isObject(node) || node.type !== 'function') { return []; } - return [node, ...node.args.flatMap(flattenFunctionNodes)].filter(Boolean); + return [node, ...node.args.flatMap(flattenFunctionNodes)].filter(nonNullable); } return flattenFunctionNodes(root); @@ -136,14 +141,15 @@ export function hasInvalidOperations( return { // avoid duplicates names: Array.from(new Set(nodes.map(({ name }) => name))), - locations: nodes.map(({ location }) => location).filter((a) => a) as TinymathLocation[], + locations: nodes.map(({ location }) => location).filter(nonNullable), }; } export const getRawQueryValidationError = (text: string, operations: Record) => { // try to extract the query context here const singleLine = text.split('\n').join(''); - const allArgs = singleLine.split(',').filter((args) => /(kql|lucene)/.test(args)); + const languagesRegexp = /(kql|lucene)/; + const allArgs = singleLine.split(',').filter((args) => languagesRegexp.test(args)); // check for the presence of a valid ES operation const containsOneValidOperation = Object.keys(operations).some((operation) => singleLine.includes(operation) @@ -157,7 +163,7 @@ export const getRawQueryValidationError = (text: string, operations: Record - arg.split('count').filter((subArg) => /(kql|lucene)/.test(subArg)) + arg.split('count').filter((subArg) => languagesRegexp.test(subArg)) ); const [kqlQueries, luceneQueries] = partition(flattenArgs, (arg) => /kql/.test(arg)); const errors = []; @@ -565,7 +571,7 @@ function validateFiltersArguments( innerType, outerType, }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } @@ -591,7 +597,7 @@ function validateNameArguments( operation: node.name, params: missingParams.map(({ name }) => name).join(', '), }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } @@ -604,7 +610,7 @@ function validateNameArguments( operation: node.name, params: wrongTypeParams.map(({ name }) => name).join(', '), }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } @@ -617,7 +623,7 @@ function validateNameArguments( operation: node.name, params: duplicateParams.join(', '), }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } @@ -631,7 +637,7 @@ function validateNameArguments( getMessageFromId({ messageId: 'tooManyQueries', values: {}, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } @@ -702,7 +708,7 @@ function runFullASTValidation( }), argument: `math operation`, }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } else { @@ -721,7 +727,7 @@ function runFullASTValidation( defaultMessage: 'no field', }), }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } @@ -754,7 +760,7 @@ function runFullASTValidation( values: { operation: node.name, }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } else { @@ -798,7 +804,7 @@ function runFullASTValidation( defaultMessage: 'no operation', }), }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } @@ -824,7 +830,7 @@ function runFullASTValidation( values: { operation: node.name, }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } else { @@ -999,7 +1005,7 @@ export function validateMathNodes(root: TinymathAST, missingVariableSet: Set name).join(', '), }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } @@ -1100,7 +1106,7 @@ export function validateMathNodes(root: TinymathAST, missingVariableSet: Set text).join(', '), }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } @@ -1150,7 +1156,7 @@ function validateFieldArguments( values: { text: node.text ?? `${node.name}(${getValueOrName(firstArg)})`, }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } @@ -1166,7 +1172,7 @@ function validateFieldArguments( defaultMessage: 'field', }), }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } @@ -1198,7 +1204,7 @@ function validateFunctionArguments( defaultMessage: 'metric', }), }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } else { @@ -1211,7 +1217,7 @@ function validateFunctionArguments( supported: requiredFunctions, text: (esOperations as TinymathFunction[]).map(({ text }) => text).join(', '), }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } @@ -1229,7 +1235,7 @@ function validateFunctionArguments( type, text: (mathOperations as TinymathFunction[]).map(({ text }) => text).join(', '), }, - locations: node.location ? [node.location] : [], + locations: getNodeLocation(node), }) ); } From 3944ae2aaf1ee8c2f4e7fca173ca98da80fdfd56 Mon Sep 17 00:00:00 2001 From: dej611 Date: Fri, 30 Sep 2022 12:56:58 +0200 Subject: [PATCH 06/25] :pencil2: Fix wrong error message --- .../operations/definitions/formula/validation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts index 11f7c87c68f7c..b08a246f730a9 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts @@ -273,7 +273,7 @@ function getMessageFromId({ case 'wrongTypeArgument': message = i18n.translate('xpack.lens.indexPattern.formulaExpressionWrongTypeArgument', { defaultMessage: - 'The {name} argument for the operation {operation} in the Formula is of the wrong type: {expectedType} instead of {type}', + 'The {name} argument for the operation {operation} in the Formula is of the wrong type: {type} instead of {expectedType}', values: { operation: out.operation, name: out.name, From 5d4df237c60e5a96144f7d70ce7f896683e3189c Mon Sep 17 00:00:00 2001 From: dej611 Date: Fri, 30 Sep 2022 18:57:40 +0200 Subject: [PATCH 07/25] :white_check_mark: Add more formula unit tests --- .../definitions/formula/formula.test.tsx | 93 ++++++++++++++++--- .../definitions/formula/formula.tsx | 3 +- .../definitions/formula/validation.ts | 68 +++++++------- 3 files changed, 115 insertions(+), 49 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx index e68bb0d3142ef..14836e5854f65 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx @@ -1496,7 +1496,7 @@ invalid: " ).toEqual([ `The return value type of the operation ${ errorFormula ?? formula - } is not supported in Formula.`, + } is not supported in Formula`, ]); } }); @@ -1507,8 +1507,14 @@ invalid: " // * field passed // * missing argument const errors = [ - (operation: string) => - `The first argument for ${operation} should be a operation name. Found ()`, + (operation: string) => { + const required = tinymathFunctions[operation].positionalArguments.filter( + ({ optional }) => !optional + ); + return `The operation ${operation} in the Formula is missing ${ + required.length + } arguments: ${required.map(({ name }) => name).join(', ')}`; + }, (operation: string) => `The operation ${operation} has too many arguments`, (operation: string) => `The operation ${operation} does not accept any field as argument`, (operation: string) => { @@ -1523,9 +1529,11 @@ invalid: " .join(', ')}`; }, ]; + + const mathFns = Object.keys(tinymathFunctions); // we'll try to map all of these here in this test - for (const fn of Object.keys(tinymathFunctions)) { - it(`returns an error for the math functions available: ${fn}`, () => { + for (const fn of mathFns) { + it(`[${fn}] returns an error for the math functions available`, () => { const nArgs = tinymathFunctions[fn].positionalArguments; // start with the first 3 types const formulas = [ @@ -1535,14 +1543,22 @@ invalid: " `${fn}(${Array(nArgs.length).fill('bytes').join(', ')})`, ]; // add the fourth check only for those functions with more than 1 arg required + // and check that this first argument is of type number const enableFourthCheck = nArgs.filter( ({ optional, alternativeWhenMissing }) => !optional && !alternativeWhenMissing - ).length > 1; + ).length > 1 && nArgs[0]?.type === 'number'; if (enableFourthCheck) { formulas.push(`${fn}(1)`); } - formulas.forEach((formula, i) => { + const finalFormulas = formulas.map((text) => { + if (tinymathFunctions[fn].outputType !== 'boolean') { + return text; + } + // for comparison functions wrap the existing formula within the ifelse function + return `ifelse(${text}, 1, 0)`; + }); + finalFormulas.forEach((formula, i) => { expect( formulaOperation.getErrorMessage!( getNewLayerWithFormula(formula), @@ -1550,16 +1566,67 @@ invalid: " indexPattern, operationDefinitionMap ) - ).toEqual([errors[i](fn)]); + ).toContain(errors[i](fn)); }); }); } + // comparison tests + for (const fn of mathFns.filter((name) => tinymathFunctions[name].section === 'comparison')) { + if (tinymathFunctions[fn].outputType === 'boolean') { + it(`[${fn}] returns an error about unsupported return type`, () => { + expect( + formulaOperation.getErrorMessage!( + getNewLayerWithFormula(`${fn}(1, 2)`), + 'col1', + indexPattern, + operationDefinitionMap + ) + ).toEqual([ + `The return value type of the operation ${fn}(1, 2) is not supported in Formula`, + ]); + }); + + it(`[${fn}] returns an error about unsupported return type and when partial arguments are passed`, () => { + const formulas = [`${fn}()`, `${fn}(1)`]; + formulas.forEach((formula, nArg) => { + const expectedCount = tinymathFunctions[fn].positionalArguments.length - nArg; + const expectedArgs = ['left', 'right'].slice(nArg).join(', '); + expect( + formulaOperation.getErrorMessage!( + getNewLayerWithFormula(formula), + 'col1', + indexPattern, + operationDefinitionMap + ) + ).toEqual([ + `The return value type of the operation ${formula} is not supported in Formula`, + `The operation ${fn} in the Formula is missing ${expectedCount} arguments: ${expectedArgs}`, + ]); + }); + }); + } else { + it(`[${fn}] returns an error if the argument is of the wrong type`, () => { + const [firstArg] = tinymathFunctions[fn].positionalArguments; + expect( + formulaOperation.getErrorMessage!( + getNewLayerWithFormula(`${fn}(1, 2, 3)`), + 'col1', + indexPattern, + operationDefinitionMap + ) + ).toEqual([ + `The ${firstArg.name} argument for the operation ${fn} in the Formula is of the wrong type: number instead of ${firstArg.type}`, + ]); + }); + } + } + it('returns an error suggesting to use an alternative function', () => { const formulas = [`clamp(1)`, 'clamp(1, 5)']; const errorsWithSuggestions = [ - 'The operation clamp in the Formula is missing the min argument: use the pick_max operation instead.', - 'The operation clamp in the Formula is missing the max argument: use the pick_min operation instead.', + 'The operation clamp in the Formula is missing the min argument: use the pick_max operation instead', + 'The operation clamp in the Formula is missing the max argument: use the pick_min operation instead', ]; formulas.forEach((formula, i) => { expect( @@ -1598,7 +1665,7 @@ invalid: " operationDefinitionMap ) ).toEqual([ - `The Formula filter of type "lucene" is not compatible with the inner filter of type "kql" from the ${operation} operation.`, + `The Formula filter of type "lucene" is not compatible with the inner filter of type "kql" from the ${operation} operation`, ]); } }); @@ -1618,8 +1685,8 @@ invalid: " operationDefinitionMap ) ).toEqual([ - `The Formula filter of type "lucene" is not compatible with the inner filter of type "kql" from the count operation.`, - `The Formula filter of type "lucene" is not compatible with the inner filter of type "kql" from the sum operation.`, + `The Formula filter of type "lucene" is not compatible with the inner filter of type "kql" from the count operation`, + `The Formula filter of type "lucene" is not compatible with the inner filter of type "kql" from the sum operation`, ]); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.tsx index 63dfb0ebdb054..5bd23d5369707 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.tsx @@ -72,7 +72,8 @@ export const formulaOperation: OperationDefinition message); + // remove duplicates + return Array.from(new Set(errors.map(({ message }) => message))); } const managedColumns = getManagedColumnsFrom(columnId, layer.columns); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts index b08a246f730a9..28fc67442b03d 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts @@ -354,21 +354,20 @@ function getMessageFromId({ break; case 'wrongReturnedType': message = i18n.translate('xpack.lens.indexPattern.formulaOperationWrongReturnedType', { - defaultMessage: - 'The return value type of the operation {text} is not supported in Formula.', + defaultMessage: 'The return value type of the operation {text} is not supported in Formula', values: { text: out.text }, }); break; case 'filtersTypeConflict': message = i18n.translate('xpack.lens.indexPattern.formulaOperationFiltersTypeConflicts', { defaultMessage: - 'The Formula filter of type "{outerType}" is not compatible with the inner filter of type "{innerType}" from the {operation} operation.', + 'The Formula filter of type "{outerType}" is not compatible with the inner filter of type "{innerType}" from the {operation} operation', values: { operation: out.operation, outerType: out.outerType, innerType: out.innerType }, }); break; case 'useAlternativeFunction': message = i18n.translate('xpack.lens.indexPattern.formulaUseAlternative', { - defaultMessage: `The operation {operation} in the Formula is missing the {params} argument: use the {alternativeFn} operation instead.`, + defaultMessage: `The operation {operation} in the Formula is missing the {params} argument: use the {alternativeFn} operation instead`, values: { operation: out.operation, params: out.params, alternativeFn: out.alternativeFn }, }); break; @@ -995,15 +994,16 @@ export function validateMathNodes(root: TinymathAST, missingVariableSet: Set { const { positionalArguments } = tinymathFunctions[node.name]; + const mandatoryArguments = positionalArguments.filter(({ optional }) => !optional); if (!node.args.length) { // we can stop here return errors.push( getMessageFromId({ - messageId: 'wrongFirstArgument', + messageId: 'missingMathArgument', values: { operation: node.name, - type: 'operation', - argument: `()`, + count: mandatoryArguments.length, + params: mandatoryArguments.map(({ name }) => name).join(', '), }, locations: getNodeLocation(node), }) @@ -1022,33 +1022,6 @@ export function validateMathNodes(root: TinymathAST, missingVariableSet: Set { - const arg = node.args[index]; - if (arg != null) { - if (!isObject(arg)) { - return typeof arg !== type; - } - if (arg.type === 'function' && tinymathFunctions[arg.name]) { - const { outputType = 'number' } = tinymathFunctions[arg.name]; - return outputType !== type; - } - } - }); - if (wrongTypeArgumentIndex > -1) { - errors.push( - getMessageFromId({ - messageId: 'wrongTypeArgument', - values: { - operation: node.name, - name: positionalArguments[wrongTypeArgumentIndex].name, - type: typeof node.args[wrongTypeArgumentIndex], - expectedType: positionalArguments[wrongTypeArgumentIndex].type || '', - }, - locations: getNodeLocation(node), - }) - ); - } - // no need to iterate all the arguments, one field is enough to trigger the error const hasFieldAsArgument = positionalArguments.some((requirements, index) => { const arg = node.args[index]; @@ -1068,7 +1041,6 @@ export function validateMathNodes(root: TinymathAST, missingVariableSet: Set !optional); // if there is only 1 mandatory arg, this is already handled by the wrongFirstArgument check if (mandatoryArguments.length > 1 && node.args.length < mandatoryArguments.length) { const missingArgs = mandatoryArguments.filter((_, i) => node.args[i] == null); @@ -1111,6 +1083,32 @@ export function validateMathNodes(root: TinymathAST, missingVariableSet: Set { + const arg = node.args[index]; + if (arg != null) { + if (!isObject(arg)) { + return typeof arg !== type; + } + if (arg.type === 'function' && tinymathFunctions[arg.name]) { + const { outputType = 'number' } = tinymathFunctions[arg.name]; + return outputType !== type; + } + } + }); + if (wrongTypeArgumentIndex > -1) { + errors.push( + getMessageFromId({ + messageId: 'wrongTypeArgument', + values: { + operation: node.name, + name: positionalArguments[wrongTypeArgumentIndex].name, + type: typeof node.args[wrongTypeArgumentIndex], + expectedType: positionalArguments[wrongTypeArgumentIndex].type || '', + }, + locations: getNodeLocation(node), + }) + ); + } }); return errors; } From 66289e6ebf604ac0f4e5faa2d939a232e054c60f Mon Sep 17 00:00:00 2001 From: dej611 Date: Fri, 30 Sep 2022 19:06:38 +0200 Subject: [PATCH 08/25] :white_check_mark: Add more tests --- .../definitions/formula/formula.test.tsx | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx index 14836e5854f65..ec7885c228b24 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx @@ -1441,6 +1441,23 @@ invalid: " ).toEqual(undefined); }); + it('returns no error if the formula contains comparison operator within the ifelse operation', () => { + const formulas = [ + ...['lt', 'gt', 'lte', 'gte', 'eq'].map((op) => `${op}(5, 1)`), + ...['<', '>', '==', '>=', '<='].map((symbol) => `5 ${symbol} 1`), + ]; + for (const formula of formulas) { + expect( + formulaOperation.getErrorMessage!( + getNewLayerWithFormula(`ifelse(${formula}, 1, 5)`), + 'col1', + indexPattern, + operationDefinitionMap + ) + ).toEqual(undefined); + } + }); + it('returns no error if a math operation is passed to fullReference operations', () => { const formulas = [ 'derivative(7+1)', @@ -1484,6 +1501,8 @@ invalid: " { formula: 'last_value(dest)' }, { formula: 'terms(dest)' }, { formula: 'moving_average(last_value(dest), window=7)', errorFormula: 'last_value(dest)' }, + ...['lt', 'gt', 'lte', 'gte', 'eq'].map((op) => ({ formula: `${op}(5, 1)` })), + ...['<', '>', '==', '>=', '<='].map((symbol) => ({ formula: `5 ${symbol} 1` })), ]; for (const { formula, errorFormula } of formulas) { expect( @@ -1574,19 +1593,6 @@ invalid: " // comparison tests for (const fn of mathFns.filter((name) => tinymathFunctions[name].section === 'comparison')) { if (tinymathFunctions[fn].outputType === 'boolean') { - it(`[${fn}] returns an error about unsupported return type`, () => { - expect( - formulaOperation.getErrorMessage!( - getNewLayerWithFormula(`${fn}(1, 2)`), - 'col1', - indexPattern, - operationDefinitionMap - ) - ).toEqual([ - `The return value type of the operation ${fn}(1, 2) is not supported in Formula`, - ]); - }); - it(`[${fn}] returns an error about unsupported return type and when partial arguments are passed`, () => { const formulas = [`${fn}()`, `${fn}(1)`]; formulas.forEach((formula, nArg) => { From 12dcfc24553722d91f37e41a5d91f67e3745aea6 Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 3 Oct 2022 09:22:51 +0200 Subject: [PATCH 09/25] :white_check_mark: Fix tsvb test --- .../server/lib/vis_data/response_processors/series/math.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/vis_types/timeseries/server/lib/vis_data/response_processors/series/math.test.js b/src/plugins/vis_types/timeseries/server/lib/vis_data/response_processors/series/math.test.js index 3ff2ee0cba353..6d2a435a20b68 100644 --- a/src/plugins/vis_types/timeseries/server/lib/vis_data/response_processors/series/math.test.js +++ b/src/plugins/vis_types/timeseries/server/lib/vis_data/response_processors/series/math.test.js @@ -253,7 +253,7 @@ describe('math(resp, panel, series)', () => { )(await mathAgg(resp, panel, series)((results) => results))([]); } catch (e) { expect(e.message).toEqual( - 'Failed to parse expression. Expected "*", "+", "-", "/", end of input, or whitespace but "(" found.' + 'Failed to parse expression. Expected "*", "+", "-", "/", "<", "=", ">", end of input, or whitespace but "(" found.' ); } }); From 9cee5123ec7ed3033bf18e1cc17417346095c31f Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 3 Oct 2022 11:13:00 +0200 Subject: [PATCH 10/25] :bug: Fix issue with divide by 0 --- packages/kbn-tinymath/src/functions/divide.js | 7 ++++++- packages/kbn-tinymath/test/functions/divide.test.js | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/kbn-tinymath/src/functions/divide.js b/packages/kbn-tinymath/src/functions/divide.js index 3f323944c886b..2759126633acc 100644 --- a/packages/kbn-tinymath/src/functions/divide.js +++ b/packages/kbn-tinymath/src/functions/divide.js @@ -30,7 +30,12 @@ function divide(a, b) { return val / b[i]; }); } - if (Array.isArray(b)) return b.map((b) => a / b); + if (Array.isArray(b)) { + return b.map((bi) => { + if (bi === 0) throw new Error('Cannot divide by 0'); + return a / bi; + }); + } if (b === 0) throw new Error('Cannot divide by 0'); if (Array.isArray(a)) return a.map((a) => a / b); return a / b; diff --git a/packages/kbn-tinymath/test/functions/divide.test.js b/packages/kbn-tinymath/test/functions/divide.test.js index 00af7b1430e58..85851e78df8dd 100644 --- a/packages/kbn-tinymath/test/functions/divide.test.js +++ b/packages/kbn-tinymath/test/functions/divide.test.js @@ -29,4 +29,11 @@ describe('Divide', () => { it('array length mismatch', () => { expect(() => divide([1, 2], [3])).toThrow('Array length mismatch'); }); + + it('divide by 0', () => { + expect(() => divide([1, 2], 0)).toThrow('Cannot divide by 0'); + expect(() => divide(1, 0)).toThrow('Cannot divide by 0'); + expect(() => divide([1, 2], [0, 0])).toThrow('Cannot divide by 0'); + expect(() => divide(1, [1, 0])).toThrow('Cannot divide by 0'); + }); }); From 11442e46eaf82fd52d33eac5d1f94ed1a2afd540 Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 3 Oct 2022 11:41:17 +0200 Subject: [PATCH 11/25] :pencil2: Update testing command --- packages/kbn-tinymath/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kbn-tinymath/README.md b/packages/kbn-tinymath/README.md index 1094c4286c851..939afa432f816 100644 --- a/packages/kbn-tinymath/README.md +++ b/packages/kbn-tinymath/README.md @@ -68,5 +68,5 @@ This package is rebuilt when running `yarn kbn bootstrap`, but can also be build using `yarn build` from the `packages/kbn-tinymath` directory. ### Running tests -To test `@kbn/tinymath` from Kibana, run `yarn run jest --watch packages/kbn-tinymath` from +To test `@kbn/tinymath` from Kibana, run `node scripts/jest --config packages/kbn-tinymath/jest.config.js` from the top level of Kibana. From 5dc906c4ad2dd79c404594d1484cefc0e63fb315 Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 3 Oct 2022 11:58:04 +0200 Subject: [PATCH 12/25] :pencil2: Add some more testing info --- packages/kbn-tinymath/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/kbn-tinymath/README.md b/packages/kbn-tinymath/README.md index 939afa432f816..3db95cef9adb0 100644 --- a/packages/kbn-tinymath/README.md +++ b/packages/kbn-tinymath/README.md @@ -66,7 +66,10 @@ parse('1 + random()') This package is rebuilt when running `yarn kbn bootstrap`, but can also be build directly using `yarn build` from the `packages/kbn-tinymath` directory. + ### Running tests To test `@kbn/tinymath` from Kibana, run `node scripts/jest --config packages/kbn-tinymath/jest.config.js` from the top level of Kibana. + +To test grammar changes it is required to run a build task before the test suite. From c22a3d585e4423f53dc9c447f4a65586a26986ea Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 3 Oct 2022 12:20:26 +0200 Subject: [PATCH 13/25] :sparkles: Improved grammar to handle edge cases --- packages/kbn-tinymath/grammar/grammar.peggy | 27 +++++++++++++++------ 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/packages/kbn-tinymath/grammar/grammar.peggy b/packages/kbn-tinymath/grammar/grammar.peggy index 8ed9224eaff6a..7eb5f6c5f82d6 100644 --- a/packages/kbn-tinymath/grammar/grammar.peggy +++ b/packages/kbn-tinymath/grammar/grammar.peggy @@ -96,15 +96,15 @@ Variable // expressions +// An Expression can be of 3 different types: +// * a Comparison operation, which can contain recursive MathOperations inside +// * a MathOperation, which can contain other MathOperations, but not Comparison types +// * an ExpressionGroup, which is a generic Grouping that contains also Comparison operations (i.e. ( 5 > 1)) Expression = Comparison / MathOperation - -MathOperation - = AddSubtract - / MultiplyDivide - / Factor - + / ExpressionGroup + Comparison = _ left:MathOperation op:(('>' / '<')('=')? / '=''=') right:MathOperation _ { return { @@ -116,6 +116,11 @@ Comparison }; } +MathOperation + = AddSubtract + / MultiplyDivide + / Factor + AddSubtract = _ left:MultiplyDivide rest:(('+' / '-') MultiplyDivide)+ _ { return parseSymbol(left, rest, {'+': 'add', '-': 'subtract'}); @@ -125,13 +130,21 @@ MultiplyDivide = _ left:Factor rest:(('*' / '/') Factor)* _ { return parseSymbol(left, rest, {'*': 'multiply', '/': 'divide'}); } - + Factor = Group / Function / Literal +// Because of the new Comparison syntax it is required a new Group type +// the previous Group has been renamed into ExpressionGroup while +// a new Group type has been defined to exclude the Comparison type from it Group + = _ '(' _ expr:MathOperation _ ')' _ { + return expr + } + +ExpressionGroup = _ '(' _ expr:Expression _ ')' _ { return expr } From 6f0874c96326a0b5c6fa1cb40be41df196f0298c Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 3 Oct 2022 12:21:56 +0200 Subject: [PATCH 14/25] :white_check_mark: Improve comparison code + unit tests --- .../src/functions/comparison/eq.js | 4 + .../src/functions/comparison/gt.js | 4 + .../src/functions/comparison/ifelse.js | 13 ++- .../src/functions/comparison/lt.js | 4 + .../test/functions/comparison/eq.test.js | 42 +++++++++ .../test/functions/comparison/gt.test.js | 42 +++++++++ .../test/functions/comparison/gte.test.js | 59 +++++++++++++ .../test/functions/comparison/ifelse.test.js | 33 +++++++ .../test/functions/comparison/lt.test.js | 42 +++++++++ .../test/functions/comparison/lte.test.js | 59 +++++++++++++ packages/kbn-tinymath/test/library.test.js | 85 +++++++++++++++++++ 11 files changed, 385 insertions(+), 2 deletions(-) create mode 100644 packages/kbn-tinymath/test/functions/comparison/eq.test.js create mode 100644 packages/kbn-tinymath/test/functions/comparison/gt.test.js create mode 100644 packages/kbn-tinymath/test/functions/comparison/gte.test.js create mode 100644 packages/kbn-tinymath/test/functions/comparison/ifelse.test.js create mode 100644 packages/kbn-tinymath/test/functions/comparison/lt.test.js create mode 100644 packages/kbn-tinymath/test/functions/comparison/lte.test.js diff --git a/packages/kbn-tinymath/src/functions/comparison/eq.js b/packages/kbn-tinymath/src/functions/comparison/eq.js index 9692e856d99fb..8834587f92a8d 100644 --- a/packages/kbn-tinymath/src/functions/comparison/eq.js +++ b/packages/kbn-tinymath/src/functions/comparison/eq.js @@ -11,6 +11,7 @@ * @param {number|number[]} a a number or an array of numbers * @param {number|number[]} b a number or an array of numbers * @return {boolean} Returns true if `a` and `b` are equal, false otherwise. Returns an array with the equality comparison of each element if `a` is an array. + * @throws `'Missing b value'` if `b` is not provided * @throws `'Array length mismatch'` if `args` contains arrays of different lengths * @example * eq(1, 1) // returns true @@ -22,6 +23,9 @@ module.exports = { eq }; function eq(a, b) { + if (b == null) { + throw new Error('Missing b value'); + } if (Array.isArray(a)) { if (!Array.isArray(b)) { return a.every((v) => v === b); diff --git a/packages/kbn-tinymath/src/functions/comparison/gt.js b/packages/kbn-tinymath/src/functions/comparison/gt.js index 0eee8a17e7cd9..a4996c60a59bd 100644 --- a/packages/kbn-tinymath/src/functions/comparison/gt.js +++ b/packages/kbn-tinymath/src/functions/comparison/gt.js @@ -11,6 +11,7 @@ * @param {number|number[]} a a number or an array of numbers * @param {number|number[]} b a number or an array of numbers * @return {boolean} Returns true if `a` is greater than `b`, false otherwise. Returns an array with the greater than comparison of each element if `a` is an array. + * @throws `'Missing b value'` if `b` is not provided * @throws `'Array length mismatch'` if `args` contains arrays of different lengths * @example * gt(1, 1) // returns false @@ -22,6 +23,9 @@ module.exports = { gt }; function gt(a, b) { + if (b == null) { + throw new Error('Missing b value'); + } if (Array.isArray(a)) { if (!Array.isArray(b)) { return a.every((v) => v > b); diff --git a/packages/kbn-tinymath/src/functions/comparison/ifelse.js b/packages/kbn-tinymath/src/functions/comparison/ifelse.js index 0e8be7245d19a..4a37bdd8ffeaf 100644 --- a/packages/kbn-tinymath/src/functions/comparison/ifelse.js +++ b/packages/kbn-tinymath/src/functions/comparison/ifelse.js @@ -12,6 +12,9 @@ * @param {(any|any[])} a a value or an array of any values * @param {(any|any[])} b a value or an array of any values * @return {(any|any[])} if the value of cond is truthy, return `a`, otherwise return `b`. + * @throws `'Condition clause is of the wrong type'` if the `cond` provided is not of boolean type + * @throws `'Missing a value'` if `a` is not provided + * @throws `'Missing b value'` if `b` is not provided * @example * ifelse( 5 > 6, 1, 0) // returns 0 * ifelse( 1 == 1, [1, 2, 3], 5) // returns [1, 2, 3] @@ -20,11 +23,17 @@ module.exports = { ifelse }; -function ifelse(cond, whenTrue, whenFalse) { +function ifelse(cond, a, b) { if (typeof cond !== 'boolean') { throw Error('Condition clause is of the wrong type'); } - return cond ? whenTrue : whenFalse; + if (a == null) { + throw new Error('Missing a value'); + } + if (b == null) { + throw new Error('Missing b value'); + } + return cond ? a : b; } ifelse.skipNumberValidation = true; diff --git a/packages/kbn-tinymath/src/functions/comparison/lt.js b/packages/kbn-tinymath/src/functions/comparison/lt.js index edfce00724088..29ef49631cf4c 100644 --- a/packages/kbn-tinymath/src/functions/comparison/lt.js +++ b/packages/kbn-tinymath/src/functions/comparison/lt.js @@ -11,6 +11,7 @@ * @param {number|number[]} a a number or an array of numbers * @param {number|number[]} b a number or an array of numbers * @return {boolean} Returns true if `a` is lower than `b`, false otherwise. Returns an array with the lower than comparison of each element if `a` is an array. + * @throws `'Missing b value'` if `b` is not provided * @throws `'Array length mismatch'` if `args` contains arrays of different lengths * @example * lt(1, 1) // returns false @@ -22,6 +23,9 @@ module.exports = { lt }; function lt(a, b) { + if (b == null) { + throw new Error('Missing b value'); + } if (Array.isArray(a)) { if (!Array.isArray(b)) { return a.every((v) => v < b); diff --git a/packages/kbn-tinymath/test/functions/comparison/eq.test.js b/packages/kbn-tinymath/test/functions/comparison/eq.test.js new file mode 100644 index 0000000000000..eb84479a3708a --- /dev/null +++ b/packages/kbn-tinymath/test/functions/comparison/eq.test.js @@ -0,0 +1,42 @@ +/* + * 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. + */ + +const { eq } = require('../../../src/functions/comparison/eq'); + +describe('Eq', () => { + it('numbers', () => { + expect(eq(-10, -10)).toBeTruthy(); + expect(eq(10, 10)).toBeTruthy(); + expect(eq(0, 0)).toBeTruthy(); + }); + + it('arrays', () => { + // Should pass + expect(eq([-1], -1)).toBeTruthy(); + expect(eq([-1], [-1])).toBeTruthy(); + expect(eq([-1, -1], -1)).toBeTruthy(); + expect(eq([-1, -1], [-1, -1])).toBeTruthy(); + + // Should not pass + expect(eq([-1], 0)).toBeFalsy(); + expect(eq([-1], [0])).toBeFalsy(); + expect(eq([-1, -1], 0)).toBeFalsy(); + expect(eq([-1, -1], [0, 0])).toBeFalsy(); + expect(eq([-1, -1], [-1, 0])).toBeFalsy(); + }); + + it('missing args', () => { + expect(() => eq()).toThrow(); + expect(() => eq(-10)).toThrow(); + expect(() => eq([])).toThrow(); + }); + + it('empty arrays', () => { + expect(eq([], [])).toBeTruthy(); + }); +}); diff --git a/packages/kbn-tinymath/test/functions/comparison/gt.test.js b/packages/kbn-tinymath/test/functions/comparison/gt.test.js new file mode 100644 index 0000000000000..a6a7173d35043 --- /dev/null +++ b/packages/kbn-tinymath/test/functions/comparison/gt.test.js @@ -0,0 +1,42 @@ +/* + * 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. + */ + +const { gt } = require('../../../src/functions/comparison/gt'); + +describe('Gt', () => { + it('missing args', () => { + expect(() => gt()).toThrow(); + expect(() => gt(-10)).toThrow(); + expect(() => gt([])).toThrow(); + }); + + it('empty arrays', () => { + expect(gt([], [])).toBeTruthy(); + }); + + it('numbers', () => { + expect(gt(-10, -20)).toBeTruthy(); + expect(gt(10, 0)).toBeTruthy(); + expect(gt(0, -1)).toBeTruthy(); + }); + + it('arrays', () => { + // Should pass + expect(gt([-1], -2)).toBeTruthy(); + expect(gt([-1], [-2])).toBeTruthy(); + expect(gt([-1, -1], -2)).toBeTruthy(); + expect(gt([-1, -1], [-2, -2])).toBeTruthy(); + + // Should not pass + expect(gt([-1], 2)).toBeFalsy(); + expect(gt([-1], [2])).toBeFalsy(); + expect(gt([-1, -1], 2)).toBeFalsy(); + expect(gt([-1, -1], [2, 2])).toBeFalsy(); + expect(gt([-1, -1], [-2, 2])).toBeFalsy(); + }); +}); diff --git a/packages/kbn-tinymath/test/functions/comparison/gte.test.js b/packages/kbn-tinymath/test/functions/comparison/gte.test.js new file mode 100644 index 0000000000000..fde1294a5a0fc --- /dev/null +++ b/packages/kbn-tinymath/test/functions/comparison/gte.test.js @@ -0,0 +1,59 @@ +/* + * 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. + */ + +const { gte } = require('../../../src/functions/comparison/gte'); + +describe('Gte', () => { + it('missing args', () => { + expect(() => gte()).toThrow(); + expect(() => gte(-10)).toThrow(); + expect(() => gte([])).toThrow(); + }); + + it('empty arrays', () => { + expect(gte([], [])).toBeTruthy(); + }); + + describe('eq values', () => { + it('numbers', () => { + expect(gte(-10, -10)).toBeTruthy(); + expect(gte(10, 10)).toBeTruthy(); + expect(gte(0, 0)).toBeTruthy(); + }); + + it('arrays', () => { + expect(gte([-1], -1)).toBeTruthy(); + expect(gte([-1], [-1])).toBeTruthy(); + expect(gte([-1, -1], -1)).toBeTruthy(); + expect(gte([-1, -1], [-1, -1])).toBeTruthy(); + }); + }); + + describe('gt values', () => { + it('numbers', () => { + expect(gte(-10, -20)).toBeTruthy(); + expect(gte(10, 0)).toBeTruthy(); + expect(gte(0, -1)).toBeTruthy(); + }); + + it('arrays', () => { + // Should pass + expect(gte([-1], -2)).toBeTruthy(); + expect(gte([-1], [-2])).toBeTruthy(); + expect(gte([-1, -1], -2)).toBeTruthy(); + expect(gte([-1, -1], [-2, -2])).toBeTruthy(); + + // Should not pass + expect(gte([-1], 2)).toBeFalsy(); + expect(gte([-1], [2])).toBeFalsy(); + expect(gte([-1, -1], 2)).toBeFalsy(); + expect(gte([-1, -1], [2, 2])).toBeFalsy(); + expect(gte([-1, -1], [-2, 2])).toBeFalsy(); + }); + }); +}); diff --git a/packages/kbn-tinymath/test/functions/comparison/ifelse.test.js b/packages/kbn-tinymath/test/functions/comparison/ifelse.test.js new file mode 100644 index 0000000000000..efd098e26292a --- /dev/null +++ b/packages/kbn-tinymath/test/functions/comparison/ifelse.test.js @@ -0,0 +1,33 @@ +/* + * 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. + */ + +const { ifelse } = require('../../../src/functions/comparison/ifelse'); + +describe('Ifelse', () => { + it('should basically work', () => { + expect(ifelse(true, 1, 0)).toEqual(1); + expect(ifelse(false, 1, 0)).toEqual(0); + expect(ifelse(1 > 0, 1, 0)).toEqual(1); + expect(ifelse(1 < 0, 1, 0)).toEqual(0); + }); + + it('should throw if cond is not of boolean type', () => { + expect(() => ifelse(5, 1, 0)).toThrow('Condition clause is of the wrong type'); + expect(() => ifelse(null, 1, 0)).toThrow('Condition clause is of the wrong type'); + expect(() => ifelse(undefined, 1, 0)).toThrow('Condition clause is of the wrong type'); + expect(() => ifelse(0, 1, 0)).toThrow('Condition clause is of the wrong type'); + }); + + it('missing args', () => { + expect(() => ifelse()).toThrow(); + expect(() => ifelse(-10)).toThrow(); + expect(() => ifelse([])).toThrow(); + expect(() => ifelse(true)).toThrow(); + expect(() => ifelse(true, 1)).toThrow(); + }); +}); diff --git a/packages/kbn-tinymath/test/functions/comparison/lt.test.js b/packages/kbn-tinymath/test/functions/comparison/lt.test.js new file mode 100644 index 0000000000000..cbd56a4ac674e --- /dev/null +++ b/packages/kbn-tinymath/test/functions/comparison/lt.test.js @@ -0,0 +1,42 @@ +/* + * 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. + */ + +const { lt } = require('../../../src/functions/comparison/lt'); + +describe('Lt', () => { + it('missing args', () => { + expect(() => lt()).toThrow(); + expect(() => lt(-10)).toThrow(); + expect(() => lt([])).toThrow(); + }); + + it('empty arrays', () => { + expect(lt([], [])).toBeTruthy(); + }); + + it('numbers', () => { + expect(lt(-10, -2)).toBeTruthy(); + expect(lt(10, 20)).toBeTruthy(); + expect(lt(0, 1)).toBeTruthy(); + }); + + it('arrays', () => { + // Should pass + expect(lt([-1], 0)).toBeTruthy(); + expect(lt([-1], [0])).toBeTruthy(); + expect(lt([-1, -1], 0)).toBeTruthy(); + expect(lt([-1, -1], [0, 0])).toBeTruthy(); + + // Should not pass + expect(lt([-1], -2)).toBeFalsy(); + expect(lt([-1], [-2])).toBeFalsy(); + expect(lt([-1, -1], -2)).toBeFalsy(); + expect(lt([-1, -1], [-2, -2])).toBeFalsy(); + expect(lt([-1, -1], [-2, 2])).toBeFalsy(); + }); +}); diff --git a/packages/kbn-tinymath/test/functions/comparison/lte.test.js b/packages/kbn-tinymath/test/functions/comparison/lte.test.js new file mode 100644 index 0000000000000..5c6f4a4b5eaa7 --- /dev/null +++ b/packages/kbn-tinymath/test/functions/comparison/lte.test.js @@ -0,0 +1,59 @@ +/* + * 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. + */ + +const { lte } = require('../../../src/functions/comparison/lte'); + +describe('Lte', () => { + it('missing args', () => { + expect(() => lte()).toThrow(); + expect(() => lte(-10)).toThrow(); + expect(() => lte([])).toThrow(); + }); + + it('empty arrays', () => { + expect(lte([], [])).toBeTruthy(); + }); + + describe('eq values', () => { + it('numbers', () => { + expect(lte(-10, -10)).toBeTruthy(); + expect(lte(10, 10)).toBeTruthy(); + expect(lte(0, 0)).toBeTruthy(); + }); + + it('arrays', () => { + expect(lte([-1], -1)).toBeTruthy(); + expect(lte([-1], [-1])).toBeTruthy(); + expect(lte([-1, -1], -1)).toBeTruthy(); + expect(lte([-1, -1], [-1, -1])).toBeTruthy(); + }); + }); + + describe('lt values', () => { + it('numbers', () => { + expect(lte(-10, -2)).toBeTruthy(); + expect(lte(10, 20)).toBeTruthy(); + expect(lte(0, 1)).toBeTruthy(); + }); + + it('arrays', () => { + // Should pass + expect(lte([-1], 0)).toBeTruthy(); + expect(lte([-1], [0])).toBeTruthy(); + expect(lte([-1, -1], 0)).toBeTruthy(); + expect(lte([-1, -1], [0, 0])).toBeTruthy(); + + // Should not pass + expect(lte([-1], -2)).toBeFalsy(); + expect(lte([-1], [-2])).toBeFalsy(); + expect(lte([-1, -1], -2)).toBeFalsy(); + expect(lte([-1, -1], [-2, -2])).toBeFalsy(); + expect(lte([-1, -1], [-2, 2])).toBeFalsy(); + }); + }); +}); diff --git a/packages/kbn-tinymath/test/library.test.js b/packages/kbn-tinymath/test/library.test.js index 9d87919c4f1ac..ab05bb3334a03 100644 --- a/packages/kbn-tinymath/test/library.test.js +++ b/packages/kbn-tinymath/test/library.test.js @@ -68,6 +68,91 @@ describe('Parser', () => { location: { min: 0, max: 13 }, }); }); + + describe('Comparison', () => { + it('should throw for non valid comparison symbols', () => { + const symbols = ['<>', '><', '===', '>>', '<<']; + for (const symbol of symbols) { + expect(() => parse(`5 ${symbol} 1`)).toThrow(); + } + }); + describe.each` + symbol | fn + ${'<'} | ${'lt'} + ${'>'} | ${'gt'} + ${'=='} | ${'eq'} + ${'>='} | ${'gte'} + ${'<='} | ${'lte'} + `('Symbol "$symbol" ( $fn )', ({ symbol, fn }) => { + it(`should parse comparison symbol: "$symbol"`, () => { + expect(parse(`5 ${symbol} 1`)).toEqual({ + name: fn, + type: 'function', + args: [5, 1], + text: `5 ${symbol} 1`, + location: { min: 0, max: 4 + symbol.length }, + }); + expect(parse(`a ${symbol} b`)).toEqual({ + name: fn, + type: 'function', + args: [variableEqual('a'), variableEqual('b')], + text: `a ${symbol} b`, + location: { min: 0, max: 4 + symbol.length }, + }); + }); + + it.each` + expression + ${`1 + (1 >${symbol} 1)`} + ${`(1 ${symbol} 1) + 1`} + ${`((1 ${symbol} 1) + 1)`} + ${`((1 ${symbol} 1) + (1 ${symbol} 1))`} + ${`((1 ${symbol} 1) + ( ${symbol} 1))`} + ${` ${symbol} 1`} + ${`1 ${symbol} `} + ${`a + (b >${symbol} c)`} + ${`(a ${symbol} b) + c`} + ${`((a ${symbol} b) + c)`} + ${`((a ${symbol} b) + (c ${symbol} d))`} + ${`((a ${symbol} b) + ( ${symbol} c))`} + ${` ${symbol} a`} + ${`a ${symbol} `} + `( + 'should throw for invalid expression with comparison arguments: $expression', + ({ expression }) => { + expect(() => parse(expression)).toThrow(); + } + ); + + it.each` + expression + ${`1 ${symbol} 1 ${symbol} 1`} + ${`(1 ${symbol} 1) ${symbol} 1`} + ${`1 ${symbol} (1 ${symbol} 1)`} + ${`a ${symbol} b ${symbol} c`} + ${`(a ${symbol} b) ${symbol} c`} + ${`a ${symbol} (b ${symbol} c)`} + `('should throw for cascading comparison operators: $expression', ({ expression }) => { + expect(() => parse(expression)).toThrow(); + }); + + it.each` + expression + ${`1 ${symbol} 1`} + ${`(1 ${symbol} 1)`} + ${`((1 ${symbol} 1))`} + ${`((1 + 1) ${symbol} 1)`} + ${`1 + 1 ${symbol} 1 * 1`} + ${`a ${symbol} b`} + ${`(a ${symbol} b)`} + ${`((a ${symbol} b))`} + ${`((a + b) ${symbol} c)`} + ${`a + b ${symbol} c * d`} + `('should parse comparison expressions: $expression', ({ expression }) => { + expect(() => parse(expression)).not.toThrow(); + }); + }); + }); }); describe('Variables', () => { From 00bfae4fa4220f6c096f9b23bbab8f0081847970 Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 3 Oct 2022 13:54:24 +0200 Subject: [PATCH 15/25] :white_check_mark: Fix test --- packages/kbn-tinymath/test/library.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kbn-tinymath/test/library.test.js b/packages/kbn-tinymath/test/library.test.js index ab05bb3334a03..054d78fc60adb 100644 --- a/packages/kbn-tinymath/test/library.test.js +++ b/packages/kbn-tinymath/test/library.test.js @@ -103,14 +103,14 @@ describe('Parser', () => { it.each` expression - ${`1 + (1 >${symbol} 1)`} + ${`1 + (1 ${symbol} 1)`} ${`(1 ${symbol} 1) + 1`} ${`((1 ${symbol} 1) + 1)`} ${`((1 ${symbol} 1) + (1 ${symbol} 1))`} ${`((1 ${symbol} 1) + ( ${symbol} 1))`} ${` ${symbol} 1`} ${`1 ${symbol} `} - ${`a + (b >${symbol} c)`} + ${`a + (b ${symbol} c)`} ${`(a ${symbol} b) + c`} ${`((a ${symbol} b) + c)`} ${`((a ${symbol} b) + (c ${symbol} d))`} From ab1f5ad291b82ce9684560a6657683db1c5bdbb9 Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 3 Oct 2022 15:39:50 +0200 Subject: [PATCH 16/25] :pencil2: Update documentation with latest functions --- packages/kbn-tinymath/docs/functions.md | 137 ++++++++++++++++++ packages/kbn-tinymath/src/functions/abs.js | 4 +- packages/kbn-tinymath/src/functions/add.js | 3 +- packages/kbn-tinymath/src/functions/cbrt.js | 4 +- packages/kbn-tinymath/src/functions/ceil.js | 4 +- packages/kbn-tinymath/src/functions/clamp.js | 4 +- .../src/functions/comparison/eq.js | 3 +- .../src/functions/comparison/gt.js | 3 +- .../src/functions/comparison/gte.js | 3 +- .../src/functions/comparison/ifelse.js | 3 +- .../src/functions/comparison/lt.js | 3 +- .../src/functions/comparison/lte.js | 3 +- packages/kbn-tinymath/src/functions/cos.js | 3 +- packages/kbn-tinymath/src/functions/count.js | 4 +- packages/kbn-tinymath/src/functions/cube.js | 3 +- .../kbn-tinymath/src/functions/degtorad.js | 3 +- packages/kbn-tinymath/src/functions/divide.js | 3 +- packages/kbn-tinymath/src/functions/exp.js | 3 +- packages/kbn-tinymath/src/functions/first.js | 4 +- packages/kbn-tinymath/src/functions/fix.js | 4 +- packages/kbn-tinymath/src/functions/floor.js | 4 +- packages/kbn-tinymath/src/functions/last.js | 3 +- .../src/functions/lib/transpose.js | 1 + packages/kbn-tinymath/src/functions/log.js | 3 +- packages/kbn-tinymath/src/functions/log10.js | 3 +- packages/kbn-tinymath/src/functions/max.js | 3 +- packages/kbn-tinymath/src/functions/mean.js | 3 +- packages/kbn-tinymath/src/functions/median.js | 3 +- packages/kbn-tinymath/src/functions/min.js | 3 +- packages/kbn-tinymath/src/functions/mod.js | 3 +- packages/kbn-tinymath/src/functions/mode.js | 3 +- .../kbn-tinymath/src/functions/multiply.js | 3 +- packages/kbn-tinymath/src/functions/pi.js | 3 +- packages/kbn-tinymath/src/functions/pow.js | 3 +- .../kbn-tinymath/src/functions/radtodeg.js | 3 +- packages/kbn-tinymath/src/functions/random.js | 4 +- packages/kbn-tinymath/src/functions/range.js | 3 +- packages/kbn-tinymath/src/functions/round.js | 9 +- packages/kbn-tinymath/src/functions/sin.js | 3 +- packages/kbn-tinymath/src/functions/size.js | 3 +- packages/kbn-tinymath/src/functions/sqrt.js | 3 +- packages/kbn-tinymath/src/functions/square.js | 3 +- .../kbn-tinymath/src/functions/subtract.js | 3 +- packages/kbn-tinymath/src/functions/sum.js | 3 +- packages/kbn-tinymath/src/functions/tan.js | 3 +- packages/kbn-tinymath/src/functions/unique.js | 3 +- 46 files changed, 194 insertions(+), 91 deletions(-) diff --git a/packages/kbn-tinymath/docs/functions.md b/packages/kbn-tinymath/docs/functions.md index 0c7460a8189dd..21af3365cd045 100644 --- a/packages/kbn-tinymath/docs/functions.md +++ b/packages/kbn-tinymath/docs/functions.md @@ -96,6 +96,143 @@ clamp(35, 10, [20, 30, 40, 50]) // returns [20, 30, 35, 35] clamp([1, 9], 3, [4, 5]) // returns [clamp([1, 3, 4]), clamp([9, 3, 5])] = [3, 5] ``` *** +## _eq(_ _a_, _b_ _)_ +Performs an equality comparison between two values. + + +| Param | Type | Description | +| --- | --- | --- | +| a | number \| Array.<number> | a number or an array of numbers | +| b | number \| Array.<number> | a number or an array of numbers | + +**Returns**: boolean - Returns true if `a` and `b` are equal, false otherwise. Returns an array with the equality comparison of each element if `a` is an array. +**Throws**: + +- `'Missing b value'` if `b` is not provided +- `'Array length mismatch'` if `args` contains arrays of different lengths + +**Example** +```js +eq(1, 1) // returns true +eq(1, 2) // returns false +eq([1, 2], 1) // returns [true, false] +eq([1, 2], [1, 2]) // returns [true, true] +``` +*** +## _gt(_ _a_, _b_ _)_ +Performs a greater than comparison between two values. + + +| Param | Type | Description | +| --- | --- | --- | +| a | number \| Array.<number> | a number or an array of numbers | +| b | number \| Array.<number> | a number or an array of numbers | + +**Returns**: boolean - Returns true if `a` is greater than `b`, false otherwise. Returns an array with the greater than comparison of each element if `a` is an array. +**Throws**: + +- `'Missing b value'` if `b` is not provided +- `'Array length mismatch'` if `args` contains arrays of different lengths + +**Example** +```js +gt(1, 1) // returns false +gt(2, 1) // returns true +gt([1, 2], 1) // returns [true, false] +gt([1, 2], [2, 1]) // returns [false, true] +``` +*** +## _gte(_ _a_, _b_ _)_ +Performs a greater than or equal comparison between two values. + + +| Param | Type | Description | +| --- | --- | --- | +| a | number \| Array.<number> | a number or an array of numbers | +| b | number \| Array.<number> | a number or an array of numbers | + +**Returns**: boolean - Returns true if `a` is greater than or equal to `b`, false otherwise. Returns an array with the greater than or equal comparison of each element if `a` is an array. +**Throws**: + +- `'Array length mismatch'` if `args` contains arrays of different lengths + +**Example** +```js +gte(1, 1) // returns true +gte(1, 2) // returns false +gte([1, 2], 2) // returns [false, true] +gte([1, 2], [1, 1]) // returns [true, true] +``` +*** +## _ifelse(_ _cond_, _a_, _b_ _)_ +Evaluates the a conditional argument and returns one of the two values based on that. + + +| Param | Type | Description | +| --- | --- | --- | +| cond | boolean | a boolean value | +| a | any \| Array.<any> | a value or an array of any values | +| b | any \| Array.<any> | a value or an array of any values | + +**Returns**: any \| Array.<any> - if the value of cond is truthy, return `a`, otherwise return `b`. +**Throws**: + +- `'Condition clause is of the wrong type'` if the `cond` provided is not of boolean type +- `'Missing a value'` if `a` is not provided +- `'Missing b value'` if `b` is not provided + +**Example** +```js +ifelse( 5 > 6, 1, 0) // returns 0 +ifelse( 1 == 1, [1, 2, 3], 5) // returns [1, 2, 3] +ifelse( 1 < 2, [1, 2, 3], [2, 3, 4]) // returns [1, 2, 3] +``` +*** +## _lt(_ _a_, _b_ _)_ +Performs a lower than comparison between two values. + + +| Param | Type | Description | +| --- | --- | --- | +| a | number \| Array.<number> | a number or an array of numbers | +| b | number \| Array.<number> | a number or an array of numbers | + +**Returns**: boolean - Returns true if `a` is lower than `b`, false otherwise. Returns an array with the lower than comparison of each element if `a` is an array. +**Throws**: + +- `'Missing b value'` if `b` is not provided +- `'Array length mismatch'` if `args` contains arrays of different lengths + +**Example** +```js +lt(1, 1) // returns false +lt(1, 2) // returns true +lt([1, 2], 2) // returns [true, false] +lt([1, 2], [1, 2]) // returns [false, false] +``` +*** +## _lte(_ _a_, _b_ _)_ +Performs a lower than or equal comparison between two values. + + +| Param | Type | Description | +| --- | --- | --- | +| a | number \| Array.<number> | a number or an array of numbers | +| b | number \| Array.<number> | a number or an array of numbers | + +**Returns**: boolean - Returns true if `a` is lower than or equal to `b`, false otherwise. Returns an array with the lower than or equal comparison of each element if `a` is an array. +**Throws**: + +- `'Array length mismatch'` if `args` contains arrays of different lengths + +**Example** +```js +lte(1, 1) // returns true +lte(1, 2) // returns true +lte([1, 2], 2) // returns [true, true] +lte([1, 2], [1, 1]) // returns [true, false] +``` +*** ## _cos(_ _a_ _)_ Calculates the the cosine of a number. For arrays, the function will be applied index-wise to each element. diff --git a/packages/kbn-tinymath/src/functions/abs.js b/packages/kbn-tinymath/src/functions/abs.js index aa3c808d89afd..46cf65837621b 100644 --- a/packages/kbn-tinymath/src/functions/abs.js +++ b/packages/kbn-tinymath/src/functions/abs.js @@ -17,11 +17,11 @@ * abs([-1 , -2, 3, -4]) // returns [1, 2, 3, 4] */ -module.exports = { abs }; - function abs(a) { if (Array.isArray(a)) { return a.map((a) => Math.abs(a)); } return Math.abs(a); } + +module.exports = { abs }; diff --git a/packages/kbn-tinymath/src/functions/add.js b/packages/kbn-tinymath/src/functions/add.js index 184c619c94534..b1abb23d99f6e 100644 --- a/packages/kbn-tinymath/src/functions/add.js +++ b/packages/kbn-tinymath/src/functions/add.js @@ -17,8 +17,6 @@ * add([1, 2], 3, [4, 5], 6) // returns [(1 + 3 + 4 + 6), (2 + 3 + 5 + 6)] = [14, 16] */ -module.exports = { add }; - function add(...args) { if (args.length === 1) { if (Array.isArray(args[0])) return args[0].reduce((result, current) => result + current); @@ -35,3 +33,4 @@ function add(...args) { return result + current; }); } +module.exports = { add }; diff --git a/packages/kbn-tinymath/src/functions/cbrt.js b/packages/kbn-tinymath/src/functions/cbrt.js index 1c5f75a724b5e..39ea7ffb33c1e 100644 --- a/packages/kbn-tinymath/src/functions/cbrt.js +++ b/packages/kbn-tinymath/src/functions/cbrt.js @@ -17,11 +17,11 @@ * cbrt([27, 64, 125]) // returns [3, 4, 5] */ -module.exports = { cbrt }; - function cbrt(a) { if (Array.isArray(a)) { return a.map((a) => Math.cbrt(a)); } return Math.cbrt(a); } + +module.exports = { cbrt }; diff --git a/packages/kbn-tinymath/src/functions/ceil.js b/packages/kbn-tinymath/src/functions/ceil.js index fe1ca21e4aed1..3dbfbcabaa224 100644 --- a/packages/kbn-tinymath/src/functions/ceil.js +++ b/packages/kbn-tinymath/src/functions/ceil.js @@ -17,11 +17,11 @@ * ceil([1.1, 2.2, 3.3]) // returns [2, 3, 4] */ -module.exports = { ceil }; - function ceil(a) { if (Array.isArray(a)) { return a.map((a) => Math.ceil(a)); } return Math.ceil(a); } + +module.exports = { ceil }; diff --git a/packages/kbn-tinymath/src/functions/clamp.js b/packages/kbn-tinymath/src/functions/clamp.js index 29c190aa8f921..a8563895abec1 100644 --- a/packages/kbn-tinymath/src/functions/clamp.js +++ b/packages/kbn-tinymath/src/functions/clamp.js @@ -30,8 +30,6 @@ const findClamp = (a, min, max) => { * clamp([1, 9], 3, [4, 5]) // returns [clamp([1, 3, 4]), clamp([9, 3, 5])] = [3, 5] */ -module.exports = { clamp }; - function clamp(a, min, max) { if (max === null) throw new Error("Missing maximum value. You may want to use the 'min' function instead"); @@ -73,3 +71,5 @@ function clamp(a, min, max) { return findClamp(a, min, max); } + +module.exports = { clamp }; diff --git a/packages/kbn-tinymath/src/functions/comparison/eq.js b/packages/kbn-tinymath/src/functions/comparison/eq.js index 8834587f92a8d..dc1e7f027c44f 100644 --- a/packages/kbn-tinymath/src/functions/comparison/eq.js +++ b/packages/kbn-tinymath/src/functions/comparison/eq.js @@ -20,8 +20,6 @@ * eq([1, 2], [1, 2]) // returns [true, true] */ -module.exports = { eq }; - function eq(a, b) { if (b == null) { throw new Error('Missing b value'); @@ -38,3 +36,4 @@ function eq(a, b) { return a === b; } +module.exports = { eq }; diff --git a/packages/kbn-tinymath/src/functions/comparison/gt.js b/packages/kbn-tinymath/src/functions/comparison/gt.js index a4996c60a59bd..1fc1a0cac083a 100644 --- a/packages/kbn-tinymath/src/functions/comparison/gt.js +++ b/packages/kbn-tinymath/src/functions/comparison/gt.js @@ -20,8 +20,6 @@ * gt([1, 2], [2, 1]) // returns [false, true] */ -module.exports = { gt }; - function gt(a, b) { if (b == null) { throw new Error('Missing b value'); @@ -38,3 +36,4 @@ function gt(a, b) { return a > b; } +module.exports = { gt }; diff --git a/packages/kbn-tinymath/src/functions/comparison/gte.js b/packages/kbn-tinymath/src/functions/comparison/gte.js index 4ba67f441d669..2d70145e90f9e 100644 --- a/packages/kbn-tinymath/src/functions/comparison/gte.js +++ b/packages/kbn-tinymath/src/functions/comparison/gte.js @@ -22,8 +22,7 @@ const { gt } = require('./gt'); * gte([1, 2], [1, 1]) // returns [true, true] */ -module.exports = { gte }; - function gte(a, b) { return eq(a, b) || gt(a, b); } +module.exports = { gte }; diff --git a/packages/kbn-tinymath/src/functions/comparison/ifelse.js b/packages/kbn-tinymath/src/functions/comparison/ifelse.js index 4a37bdd8ffeaf..810296711636b 100644 --- a/packages/kbn-tinymath/src/functions/comparison/ifelse.js +++ b/packages/kbn-tinymath/src/functions/comparison/ifelse.js @@ -21,8 +21,6 @@ * ifelse( 1 < 2, [1, 2, 3], [2, 3, 4]) // returns [1, 2, 3] */ -module.exports = { ifelse }; - function ifelse(cond, a, b) { if (typeof cond !== 'boolean') { throw Error('Condition clause is of the wrong type'); @@ -37,3 +35,4 @@ function ifelse(cond, a, b) { } ifelse.skipNumberValidation = true; +module.exports = { ifelse }; diff --git a/packages/kbn-tinymath/src/functions/comparison/lt.js b/packages/kbn-tinymath/src/functions/comparison/lt.js index 29ef49631cf4c..fb7a444d1af7a 100644 --- a/packages/kbn-tinymath/src/functions/comparison/lt.js +++ b/packages/kbn-tinymath/src/functions/comparison/lt.js @@ -20,8 +20,6 @@ * lt([1, 2], [1, 2]) // returns [false, false] */ -module.exports = { lt }; - function lt(a, b) { if (b == null) { throw new Error('Missing b value'); @@ -38,3 +36,4 @@ function lt(a, b) { return a < b; } +module.exports = { lt }; diff --git a/packages/kbn-tinymath/src/functions/comparison/lte.js b/packages/kbn-tinymath/src/functions/comparison/lte.js index 47ab7f024ef4c..36aceb11f3bd7 100644 --- a/packages/kbn-tinymath/src/functions/comparison/lte.js +++ b/packages/kbn-tinymath/src/functions/comparison/lte.js @@ -22,8 +22,7 @@ const { lt } = require('./lt'); * lte([1, 2], [1, 1]) // returns [true, false] */ -module.exports = { lte }; - function lte(a, b) { return eq(a, b) || lt(a, b); } +module.exports = { lte }; diff --git a/packages/kbn-tinymath/src/functions/cos.js b/packages/kbn-tinymath/src/functions/cos.js index 590cc2aee06b9..590f2bf5bd5e0 100644 --- a/packages/kbn-tinymath/src/functions/cos.js +++ b/packages/kbn-tinymath/src/functions/cos.js @@ -16,11 +16,10 @@ * cos([0, 1.5707963267948966]) // returns [1, 6.123233995736766e-17] */ -module.exports = { cos }; - function cos(a) { if (Array.isArray(a)) { return a.map((a) => Math.cos(a)); } return Math.cos(a); } +module.exports = { cos }; diff --git a/packages/kbn-tinymath/src/functions/count.js b/packages/kbn-tinymath/src/functions/count.js index aa01e1c25835a..ecddb623180f2 100644 --- a/packages/kbn-tinymath/src/functions/count.js +++ b/packages/kbn-tinymath/src/functions/count.js @@ -19,10 +19,10 @@ const { size } = require('./size'); * count(100) // returns 1 */ -module.exports = { count }; - function count(a) { return size(a); } count.skipNumberValidation = true; + +module.exports = { count }; diff --git a/packages/kbn-tinymath/src/functions/cube.js b/packages/kbn-tinymath/src/functions/cube.js index 4d6f8cbea6374..92fe42326f056 100644 --- a/packages/kbn-tinymath/src/functions/cube.js +++ b/packages/kbn-tinymath/src/functions/cube.js @@ -18,8 +18,7 @@ const { pow } = require('./pow'); * cube([3, 4, 5]) // returns [27, 64, 125] */ -module.exports = { cube }; - function cube(a) { return pow(a, 3); } +module.exports = { cube }; diff --git a/packages/kbn-tinymath/src/functions/degtorad.js b/packages/kbn-tinymath/src/functions/degtorad.js index e50365b0beabc..ce6f53ac29c27 100644 --- a/packages/kbn-tinymath/src/functions/degtorad.js +++ b/packages/kbn-tinymath/src/functions/degtorad.js @@ -16,11 +16,10 @@ * degtorad([0, 90, 180, 360]) // returns [0, 1.5707963267948966, 3.141592653589793, 6.283185307179586] */ -module.exports = { degtorad }; - function degtorad(a) { if (Array.isArray(a)) { return a.map((a) => (a * Math.PI) / 180); } return (a * Math.PI) / 180; } +module.exports = { degtorad }; diff --git a/packages/kbn-tinymath/src/functions/divide.js b/packages/kbn-tinymath/src/functions/divide.js index 2759126633acc..b572217f3ce7d 100644 --- a/packages/kbn-tinymath/src/functions/divide.js +++ b/packages/kbn-tinymath/src/functions/divide.js @@ -20,8 +20,6 @@ * divide([14, 42, 65, 108], [2, 7, 5, 12]) // returns [7, 6, 13, 9] */ -module.exports = { divide }; - function divide(a, b) { if (Array.isArray(a) && Array.isArray(b)) { if (a.length !== b.length) throw new Error('Array length mismatch'); @@ -40,3 +38,4 @@ function divide(a, b) { if (Array.isArray(a)) return a.map((a) => a / b); return a / b; } +module.exports = { divide }; diff --git a/packages/kbn-tinymath/src/functions/exp.js b/packages/kbn-tinymath/src/functions/exp.js index 79669b9dccedd..cf89785e507c1 100644 --- a/packages/kbn-tinymath/src/functions/exp.js +++ b/packages/kbn-tinymath/src/functions/exp.js @@ -16,11 +16,10 @@ * exp([1, 2, 3]) // returns [e^1, e^2, e^3] = [2.718281828459045, 7.3890560989306495, 20.085536923187668] */ -module.exports = { exp }; - function exp(a) { if (Array.isArray(a)) { return a.map((a) => Math.exp(a)); } return Math.exp(a); } +module.exports = { exp }; diff --git a/packages/kbn-tinymath/src/functions/first.js b/packages/kbn-tinymath/src/functions/first.js index 894cb38e69f47..d7970ac0abb71 100644 --- a/packages/kbn-tinymath/src/functions/first.js +++ b/packages/kbn-tinymath/src/functions/first.js @@ -16,8 +16,6 @@ * first([1, 2, 3]) // returns 1 */ -module.exports = { first }; - function first(a) { if (Array.isArray(a)) { return a[0]; @@ -26,3 +24,5 @@ function first(a) { } first.skipNumberValidation = true; + +module.exports = { first }; diff --git a/packages/kbn-tinymath/src/functions/fix.js b/packages/kbn-tinymath/src/functions/fix.js index 1d297327fd47b..3088a949add77 100644 --- a/packages/kbn-tinymath/src/functions/fix.js +++ b/packages/kbn-tinymath/src/functions/fix.js @@ -24,11 +24,11 @@ const fixer = (a) => { * fix([1.8, 2.9, -3.7, -4.6]) // returns [1, 2, -3, -4] */ -module.exports = { fix }; - function fix(a) { if (Array.isArray(a)) { return a.map((a) => fixer(a)); } return fixer(a); } + +module.exports = { fix }; diff --git a/packages/kbn-tinymath/src/functions/floor.js b/packages/kbn-tinymath/src/functions/floor.js index 5d27228d7cdf9..e13f64dc665ae 100644 --- a/packages/kbn-tinymath/src/functions/floor.js +++ b/packages/kbn-tinymath/src/functions/floor.js @@ -17,11 +17,11 @@ * floor([1.7, 2.8, 3.9]) // returns [1, 2, 3] */ -module.exports = { floor }; - function floor(a) { if (Array.isArray(a)) { return a.map((a) => Math.floor(a)); } return Math.floor(a); } + +module.exports = { floor }; diff --git a/packages/kbn-tinymath/src/functions/last.js b/packages/kbn-tinymath/src/functions/last.js index 63ba3bd1cfc35..7cdd5fe9cdde6 100644 --- a/packages/kbn-tinymath/src/functions/last.js +++ b/packages/kbn-tinymath/src/functions/last.js @@ -16,8 +16,6 @@ * last([1, 2, 3]) // returns 3 */ -module.exports = { last }; - function last(a) { if (Array.isArray(a)) { return a[a.length - 1]; @@ -26,3 +24,4 @@ function last(a) { } last.skipNumberValidation = true; +module.exports = { last }; diff --git a/packages/kbn-tinymath/src/functions/lib/transpose.js b/packages/kbn-tinymath/src/functions/lib/transpose.js index 9637971cec7cf..9e4d673fdf57a 100644 --- a/packages/kbn-tinymath/src/functions/lib/transpose.js +++ b/packages/kbn-tinymath/src/functions/lib/transpose.js @@ -8,6 +8,7 @@ /** * Transposes a 2D array, i.e. turns the rows into columns and vice versa. Scalar values are also included in the transpose. + * @private * @param {any[][]} args an array or an array that contains arrays * @param {number} index index of the first array element in args * @return {any[][]} transpose of args diff --git a/packages/kbn-tinymath/src/functions/log.js b/packages/kbn-tinymath/src/functions/log.js index 083c9cdef2dc0..06043eca5384c 100644 --- a/packages/kbn-tinymath/src/functions/log.js +++ b/packages/kbn-tinymath/src/functions/log.js @@ -22,8 +22,6 @@ const changeOfBase = (a, b) => Math.log(a) / Math.log(b); * log([2, 4, 8, 16, 32], 2) // returns [1, 2, 3, 4, 5] */ -module.exports = { log }; - function log(a, b = Math.E) { if (b <= 0) throw new Error('Base out of range'); @@ -36,3 +34,4 @@ function log(a, b = Math.E) { if (a < 0) throw new Error('Must be greater than 0'); return changeOfBase(a, b); } +module.exports = { log }; diff --git a/packages/kbn-tinymath/src/functions/log10.js b/packages/kbn-tinymath/src/functions/log10.js index 68c2ffb2bbd30..09f7182e70375 100644 --- a/packages/kbn-tinymath/src/functions/log10.js +++ b/packages/kbn-tinymath/src/functions/log10.js @@ -20,8 +20,7 @@ const { log } = require('./log'); * log([10, 100, 1000, 10000, 100000]) // returns [1, 2, 3, 4, 5] */ -module.exports = { log10 }; - function log10(a) { return log(a, 10); } +module.exports = { log10 }; diff --git a/packages/kbn-tinymath/src/functions/max.js b/packages/kbn-tinymath/src/functions/max.js index 4b6bb2bd27d5e..46e6fec0f6989 100644 --- a/packages/kbn-tinymath/src/functions/max.js +++ b/packages/kbn-tinymath/src/functions/max.js @@ -17,8 +17,6 @@ * max([1, 9], 4, [3, 5]) // returns [max([1, 4, 3]), max([9, 4, 5])] = [4, 9] */ -module.exports = { max }; - function max(...args) { if (args.length === 1) { if (Array.isArray(args[0])) @@ -36,3 +34,4 @@ function max(...args) { return Math.max(result, current); }); } +module.exports = { max }; diff --git a/packages/kbn-tinymath/src/functions/mean.js b/packages/kbn-tinymath/src/functions/mean.js index 441b90cff2ac3..995c54b3b1ea0 100644 --- a/packages/kbn-tinymath/src/functions/mean.js +++ b/packages/kbn-tinymath/src/functions/mean.js @@ -19,8 +19,6 @@ const { add } = require('./add'); * mean([1, 9], 5, [3, 4]) // returns [mean([1, 5, 3]), mean([9, 5, 4])] = [3, 6] */ -module.exports = { mean }; - function mean(...args) { if (args.length === 1) { if (Array.isArray(args[0])) return add(args[0]) / args[0].length; @@ -34,3 +32,4 @@ function mean(...args) { return sum / args.length; } +module.exports = { mean }; diff --git a/packages/kbn-tinymath/src/functions/median.js b/packages/kbn-tinymath/src/functions/median.js index 7c47158a81dda..facac3d367750 100644 --- a/packages/kbn-tinymath/src/functions/median.js +++ b/packages/kbn-tinymath/src/functions/median.js @@ -33,8 +33,6 @@ const findMedian = (a) => { * median([1, 9], 2, 4, [3, 5]) // returns [median([1, 2, 4, 3]), median([9, 2, 4, 5])] = [2.5, 4.5] */ -module.exports = { median }; - function median(...args) { if (args.length === 1) { if (Array.isArray(args[0])) return findMedian(args[0]); @@ -48,3 +46,4 @@ function median(...args) { } return findMedian(args); } +module.exports = { median }; diff --git a/packages/kbn-tinymath/src/functions/min.js b/packages/kbn-tinymath/src/functions/min.js index 356ccecc4b6f2..8f9ecd7c25c28 100644 --- a/packages/kbn-tinymath/src/functions/min.js +++ b/packages/kbn-tinymath/src/functions/min.js @@ -17,8 +17,6 @@ * min([1, 9], 4, [3, 5]) // returns [min([1, 4, 3]), min([9, 4, 5])] = [1, 4] */ -module.exports = { min }; - function min(...args) { if (args.length === 1) { if (Array.isArray(args[0])) @@ -36,3 +34,4 @@ function min(...args) { return Math.min(result, current); }); } +module.exports = { min }; diff --git a/packages/kbn-tinymath/src/functions/mod.js b/packages/kbn-tinymath/src/functions/mod.js index 7f6a4fffca829..bb945772f41c9 100644 --- a/packages/kbn-tinymath/src/functions/mod.js +++ b/packages/kbn-tinymath/src/functions/mod.js @@ -20,8 +20,6 @@ * mod([14, 42, 65, 108], [5, 4, 14, 2]) // returns [5, 2, 9, 0] */ -module.exports = { mod }; - function mod(a, b) { if (Array.isArray(a) && Array.isArray(b)) { if (a.length !== b.length) throw new Error('Array length mismatch'); @@ -35,3 +33,4 @@ function mod(a, b) { if (Array.isArray(a)) return a.map((a) => a % b); return a % b; } +module.exports = { mod }; diff --git a/packages/kbn-tinymath/src/functions/mode.js b/packages/kbn-tinymath/src/functions/mode.js index 0836c1c939f44..99f0086de0a78 100644 --- a/packages/kbn-tinymath/src/functions/mode.js +++ b/packages/kbn-tinymath/src/functions/mode.js @@ -40,8 +40,6 @@ const findMode = (a) => { * mode([1, 9], 1, 4, [3, 5]) // returns [mode([1, 1, 4, 3]), mode([9, 1, 4, 5])] = [[1], [4, 5, 9]] */ -module.exports = { mode }; - function mode(...args) { if (args.length === 1) { if (Array.isArray(args[0])) return findMode(args[0]); @@ -55,3 +53,4 @@ function mode(...args) { } return findMode(args); } +module.exports = { mode }; diff --git a/packages/kbn-tinymath/src/functions/multiply.js b/packages/kbn-tinymath/src/functions/multiply.js index d7c9c75ee3647..d9d6c751bdaf7 100644 --- a/packages/kbn-tinymath/src/functions/multiply.js +++ b/packages/kbn-tinymath/src/functions/multiply.js @@ -19,8 +19,6 @@ * multiply([1, 2, 3, 4], [2, 7, 5, 12]) // returns [2, 14, 15, 48] */ -module.exports = { multiply }; - function multiply(...args) { return args.reduce((result, current) => { if (Array.isArray(result) && Array.isArray(current)) { @@ -32,3 +30,4 @@ function multiply(...args) { return result * current; }); } +module.exports = { multiply }; diff --git a/packages/kbn-tinymath/src/functions/pi.js b/packages/kbn-tinymath/src/functions/pi.js index 9f0b74292524c..dacb0ea5ea4ed 100644 --- a/packages/kbn-tinymath/src/functions/pi.js +++ b/packages/kbn-tinymath/src/functions/pi.js @@ -14,8 +14,7 @@ * pi() // 3.141592653589793 */ -module.exports = { pi }; - function pi() { return Math.PI; } +module.exports = { pi }; diff --git a/packages/kbn-tinymath/src/functions/pow.js b/packages/kbn-tinymath/src/functions/pow.js index 2c600cb7f47aa..e2be268d3c623 100644 --- a/packages/kbn-tinymath/src/functions/pow.js +++ b/packages/kbn-tinymath/src/functions/pow.js @@ -17,8 +17,6 @@ * pow([1, 2, 3], 4) // returns [1, 16, 81] */ -module.exports = { pow }; - function pow(a, b) { if (b == null) throw new Error('Missing exponent'); if (Array.isArray(a)) { @@ -26,3 +24,4 @@ function pow(a, b) { } return Math.pow(a, b); } +module.exports = { pow }; diff --git a/packages/kbn-tinymath/src/functions/radtodeg.js b/packages/kbn-tinymath/src/functions/radtodeg.js index 6cfd40841ba55..733147334a637 100644 --- a/packages/kbn-tinymath/src/functions/radtodeg.js +++ b/packages/kbn-tinymath/src/functions/radtodeg.js @@ -16,11 +16,10 @@ * radtodeg([0, 1.5707963267948966, 3.141592653589793, 6.283185307179586]) // returns [0, 90, 180, 360] */ -module.exports = { radtodeg }; - function radtodeg(a) { if (Array.isArray(a)) { return a.map((a) => (a * 180) / Math.PI); } return (a * 180) / Math.PI; } +module.exports = { radtodeg }; diff --git a/packages/kbn-tinymath/src/functions/random.js b/packages/kbn-tinymath/src/functions/random.js index 799f6515d4ca5..be2dd2c773322 100644 --- a/packages/kbn-tinymath/src/functions/random.js +++ b/packages/kbn-tinymath/src/functions/random.js @@ -18,8 +18,6 @@ * random(-10,10) // returns a random number between -10 (inclusive) and 10 (exclusive) */ -module.exports = { random }; - function random(a, b) { if (a == null) return Math.random(); @@ -33,3 +31,5 @@ function random(a, b) { if (a > b) throw new Error(`Min is greater than max`); return Math.random() * (b - a) + a; } + +module.exports = { random }; diff --git a/packages/kbn-tinymath/src/functions/range.js b/packages/kbn-tinymath/src/functions/range.js index 571b179b75b65..0b2a05dfb8ae4 100644 --- a/packages/kbn-tinymath/src/functions/range.js +++ b/packages/kbn-tinymath/src/functions/range.js @@ -21,8 +21,7 @@ const { subtract } = require('./subtract'); * range([1, 9], 4, [3, 5]) // returns [range([1, 4, 3]), range([9, 4, 5])] = [3, 5] */ -module.exports = { range }; - function range(...args) { return subtract(max(...args), min(...args)); } +module.exports = { range }; diff --git a/packages/kbn-tinymath/src/functions/round.js b/packages/kbn-tinymath/src/functions/round.js index 9befb64ca5d45..8a98b2ec555b4 100644 --- a/packages/kbn-tinymath/src/functions/round.js +++ b/packages/kbn-tinymath/src/functions/round.js @@ -22,11 +22,10 @@ const rounder = (a, b) => Math.round(a * Math.pow(10, b)) / Math.pow(10, b); * round([2.9234, 5.1234, 3.5234, 4.49234324], 2) // returns [2.92, 5.12, 3.52, 4.49] */ -module.exports = { round }; - -function round(a, b = 0) { +function round(a, b) { if (Array.isArray(a)) { - return a.map((a) => rounder(a, b)); + return a.map((a) => rounder(a, b || 0)); } - return rounder(a, b); + return rounder(a, b || 0); } +module.exports = { round }; diff --git a/packages/kbn-tinymath/src/functions/sin.js b/packages/kbn-tinymath/src/functions/sin.js index 591c799ff3ebe..6a2801623283e 100644 --- a/packages/kbn-tinymath/src/functions/sin.js +++ b/packages/kbn-tinymath/src/functions/sin.js @@ -16,11 +16,10 @@ * sin([0, 1.5707963267948966]) // returns [0, 1] */ -module.exports = { sin }; - function sin(a) { if (Array.isArray(a)) { return a.map((a) => Math.sin(a)); } return Math.sin(a); } +module.exports = { sin }; diff --git a/packages/kbn-tinymath/src/functions/size.js b/packages/kbn-tinymath/src/functions/size.js index fb16bcb905f96..f862ee33ed92a 100644 --- a/packages/kbn-tinymath/src/functions/size.js +++ b/packages/kbn-tinymath/src/functions/size.js @@ -17,11 +17,10 @@ * size(100) // returns 1 */ -module.exports = { size }; - function size(a) { if (Array.isArray(a)) return a.length; throw new Error('Must pass an array'); } size.skipNumberValidation = true; +module.exports = { size }; diff --git a/packages/kbn-tinymath/src/functions/sqrt.js b/packages/kbn-tinymath/src/functions/sqrt.js index 19aeaab964d03..fa666d6a4b0ba 100644 --- a/packages/kbn-tinymath/src/functions/sqrt.js +++ b/packages/kbn-tinymath/src/functions/sqrt.js @@ -17,8 +17,6 @@ * sqrt([9, 16, 25]) // returns [3, 4, 5] */ -module.exports = { sqrt }; - function sqrt(a) { if (Array.isArray(a)) { return a.map((a) => { @@ -30,3 +28,4 @@ function sqrt(a) { if (a < 0) throw new Error('Unable find the square root of a negative number'); return Math.sqrt(a); } +module.exports = { sqrt }; diff --git a/packages/kbn-tinymath/src/functions/square.js b/packages/kbn-tinymath/src/functions/square.js index 5c285eaee3209..58acd1c7f5e33 100644 --- a/packages/kbn-tinymath/src/functions/square.js +++ b/packages/kbn-tinymath/src/functions/square.js @@ -18,8 +18,7 @@ const { pow } = require('./pow'); * square([3, 4, 5]) // returns [9, 16, 25] */ -module.exports = { square }; - function square(a) { return pow(a, 2); } +module.exports = { square }; diff --git a/packages/kbn-tinymath/src/functions/subtract.js b/packages/kbn-tinymath/src/functions/subtract.js index becc267ca51bb..fe2b5ae754456 100644 --- a/packages/kbn-tinymath/src/functions/subtract.js +++ b/packages/kbn-tinymath/src/functions/subtract.js @@ -19,8 +19,6 @@ * subtract([14, 42, 65, 108], [2, 7, 5, 12]) // returns [12, 35, 52, 96] */ -module.exports = { subtract }; - function subtract(a, b) { if (Array.isArray(a) && Array.isArray(b)) { if (a.length !== b.length) throw new Error('Array length mismatch'); @@ -30,3 +28,4 @@ function subtract(a, b) { if (Array.isArray(b)) return b.map((b) => a - b); return a - b; } +module.exports = { subtract }; diff --git a/packages/kbn-tinymath/src/functions/sum.js b/packages/kbn-tinymath/src/functions/sum.js index d9a8f4e531010..fb5bca8abde00 100644 --- a/packages/kbn-tinymath/src/functions/sum.js +++ b/packages/kbn-tinymath/src/functions/sum.js @@ -20,8 +20,6 @@ const findSum = (total, current) => total + current; * sum([10, 20, 30, 40], 10, [1, 2, 3], 22) // returns sum(10, 20, 30, 40, 10, 1, 2, 3, 22) = 138 */ -module.exports = { sum }; - function sum(...args) { return args.reduce((total, current) => { if (Array.isArray(current)) { @@ -30,3 +28,4 @@ function sum(...args) { return total + current; }, 0); } +module.exports = { sum }; diff --git a/packages/kbn-tinymath/src/functions/tan.js b/packages/kbn-tinymath/src/functions/tan.js index 7f045acce0f38..4c980ee3fd272 100644 --- a/packages/kbn-tinymath/src/functions/tan.js +++ b/packages/kbn-tinymath/src/functions/tan.js @@ -16,11 +16,10 @@ * tan([0, 1, -1]) // returns [0, 1.5574077246549023, -1.5574077246549023] */ -module.exports = { tan }; - function tan(a) { if (Array.isArray(a)) { return a.map((a) => Math.tan(a)); } return Math.tan(a); } +module.exports = { tan }; diff --git a/packages/kbn-tinymath/src/functions/unique.js b/packages/kbn-tinymath/src/functions/unique.js index 5220b4ad79adf..77153c036822d 100644 --- a/packages/kbn-tinymath/src/functions/unique.js +++ b/packages/kbn-tinymath/src/functions/unique.js @@ -18,8 +18,6 @@ * unique([1, 2, 3, 4, 2, 2, 2, 3, 4, 2, 4, 5, 2, 1, 4, 2]) // returns 5 */ -module.exports = { unique }; - function unique(a) { if (Array.isArray(a)) { return a.filter((val, i) => a.indexOf(val) === i).length; @@ -28,3 +26,4 @@ function unique(a) { } unique.skipNumberValidation = true; +module.exports = { unique }; From f4f48f26d0fec898489f14e10385ee7c38e075a6 Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 5 Oct 2022 12:31:49 +0200 Subject: [PATCH 17/25] :ok_hand: Integrate feedback --- packages/kbn-tinymath/docs/functions.md | 6 +++--- packages/kbn-tinymath/src/functions/comparison/ifelse.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/kbn-tinymath/docs/functions.md b/packages/kbn-tinymath/docs/functions.md index 21af3365cd045..23d267ec975f5 100644 --- a/packages/kbn-tinymath/docs/functions.md +++ b/packages/kbn-tinymath/docs/functions.md @@ -183,9 +183,9 @@ Evaluates the a conditional argument and returns one of the two values based on **Example** ```js -ifelse( 5 > 6, 1, 0) // returns 0 -ifelse( 1 == 1, [1, 2, 3], 5) // returns [1, 2, 3] -ifelse( 1 < 2, [1, 2, 3], [2, 3, 4]) // returns [1, 2, 3] +ifelse(5 > 6, 1, 0) // returns 0 +ifelse(1 == 1, [1, 2, 3], 5) // returns [1, 2, 3] +ifelse(1 < 2, [1, 2, 3], [2, 3, 4]) // returns [1, 2, 3] ``` *** ## _lt(_ _a_, _b_ _)_ diff --git a/packages/kbn-tinymath/src/functions/comparison/ifelse.js b/packages/kbn-tinymath/src/functions/comparison/ifelse.js index 810296711636b..7f9c212d6029d 100644 --- a/packages/kbn-tinymath/src/functions/comparison/ifelse.js +++ b/packages/kbn-tinymath/src/functions/comparison/ifelse.js @@ -16,9 +16,9 @@ * @throws `'Missing a value'` if `a` is not provided * @throws `'Missing b value'` if `b` is not provided * @example - * ifelse( 5 > 6, 1, 0) // returns 0 - * ifelse( 1 == 1, [1, 2, 3], 5) // returns [1, 2, 3] - * ifelse( 1 < 2, [1, 2, 3], [2, 3, 4]) // returns [1, 2, 3] + * ifelse(5 > 6, 1, 0) // returns 0 + * ifelse(1 == 1, [1, 2, 3], 5) // returns [1, 2, 3] + * ifelse(1 < 2, [1, 2, 3], [2, 3, 4]) // returns [1, 2, 3] */ function ifelse(cond, a, b) { From 0bb5e4ddccf3900b76bdfc7c70fa3ff7b683d852 Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 5 Oct 2022 12:35:27 +0200 Subject: [PATCH 18/25] :ok_hand: Integrate more feedback --- packages/kbn-tinymath/src/functions/round.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/kbn-tinymath/src/functions/round.js b/packages/kbn-tinymath/src/functions/round.js index 8a98b2ec555b4..88d45fda42b5f 100644 --- a/packages/kbn-tinymath/src/functions/round.js +++ b/packages/kbn-tinymath/src/functions/round.js @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -const rounder = (a, b) => Math.round(a * Math.pow(10, b)) / Math.pow(10, b); +const rounder = (a, b = 0) => Math.round(a * Math.pow(10, b)) / Math.pow(10, b); /** * Rounds a number towards the nearest integer by default or decimal place if specified. For arrays, the function will be applied index-wise to each element. @@ -24,8 +24,8 @@ const rounder = (a, b) => Math.round(a * Math.pow(10, b)) / Math.pow(10, b); function round(a, b) { if (Array.isArray(a)) { - return a.map((a) => rounder(a, b || 0)); + return a.map((a) => rounder(a, b)); } - return rounder(a, b || 0); + return rounder(a, b); } module.exports = { round }; From 125ecddfc4aec1b7b80335dcf81ac7342af31e23 Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 5 Oct 2022 15:23:05 +0200 Subject: [PATCH 19/25] :ok_hand: Update doc --- .../operations/definitions/formula/util.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts index 3b4f598bc0437..b0cedee4bc36b 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts @@ -724,8 +724,8 @@ Example: \`gte(average(bytes), 1000)\` defaultMessage: ` Returns a value depending on whether the element of condition is true or false. -Example: Compare two fields average and return the highest -\`ifelse( average(memory) >= average(bytes), average(memory), average(bytes))\` +Example: Average revenue per customer but in some cases customer id is not provided which counts as additional customer +\`sum(total)/(cardinality(customer_id) + ifelse(count() > count(filter=customer_id:*), 1, 0)\` `, }), }, From f26e9b7ed4328ea273d3ff98e582a557837b232c Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 5 Oct 2022 15:23:30 +0200 Subject: [PATCH 20/25] :bug: Fix bug with function return type check --- .../definitions/formula/formula.test.tsx | 48 ++++++++++++++++- .../definitions/formula/validation.ts | 53 +++++++++++++------ 2 files changed, 84 insertions(+), 17 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx index ec7885c228b24..84b728e9feb43 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx @@ -1612,7 +1612,53 @@ invalid: " }); }); } else { - it(`[${fn}] returns an error if the argument is of the wrong type`, () => { + const indexReverseMap = { + cond: [0], + left: [1], + right: [2], + all: [0, 1, 2], + }; + it.each` + cond | left | right | expectedFail + ${'1'} | ${'2'} | ${'3'} | ${'cond'} + ${'1 > 1'} | ${'2 > 2'} | ${'3'} | ${'left'} + ${'1 > 1'} | ${'2'} | ${'3 > 3'} | ${'right'} + ${'1'} | ${'2 > 2'} | ${'3 > 3'} | ${'all'} + ${'count()'} | ${'average(bytes)'} | ${'average(bytes)'} | ${'cond'} + ${'count() > 1'} | ${'average(bytes) > 2'} | ${'average(bytes)'} | ${'left'} + ${'count() > 1'} | ${'average(bytes)'} | ${'average(bytes) > 3'} | ${'right'} + ${'count()'} | ${'average(bytes) > 2'} | ${'average(bytes) > 3'} | ${'all'} + `( + `[${fn}] returns an error if $expectedFail argument is/are of the wrong type: ${fn}($cond, $left, $right)`, + ({ + cond, + left, + right, + expectedFail, + }: { + cond: string; + left: string; + right: string; + expectedFail: keyof typeof indexReverseMap; + }) => { + const argsSorted = [cond, left, right]; + expect( + formulaOperation.getErrorMessage!( + getNewLayerWithFormula(`${fn}(${cond}, ${left}, ${right})`), + 'col1', + indexPattern, + operationDefinitionMap + ) + ).toEqual( + indexReverseMap[expectedFail].map((i) => { + const arg = tinymathFunctions[fn].positionalArguments[i]; + const passedValue = />/.test(argsSorted[i]) ? 'boolean' : 'number'; + return `The ${arg.name} argument for the operation ${fn} in the Formula is of the wrong type: ${passedValue} instead of ${arg.type}`; + }) + ); + } + ); + it(`[${fn}] returns an error if the condition argument is of the wrong type`, () => { const [firstArg] = tinymathFunctions[fn].positionalArguments; expect( formulaOperation.getErrorMessage!( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts index 28fc67442b03d..b0c5fa58e41be 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/validation.ts @@ -118,6 +118,22 @@ function getNodeLocation(node: TinymathFunction): TinymathLocation[] { return [node.location].filter(nonNullable); } +function getArgumentType(arg: TinymathAST, operations: Record) { + if (!isObject(arg)) { + return typeof arg; + } + if (arg.type === 'function') { + if (tinymathFunctions[arg.name]) { + return tinymathFunctions[arg.name].outputType ?? 'number'; + } + // Assume it's a number for now + if (operations[arg.name]) { + return 'number'; + } + } + // leave for now other argument types +} + export function isParsingError(message: string) { return message.includes('Failed to parse expression'); } @@ -688,7 +704,7 @@ function runFullASTValidation( const [firstArg] = node?.args || []; if (!nodeOperation) { - errors.push(...validateMathNodes(node, missingVariablesSet)); + errors.push(...validateMathNodes(node, missingVariablesSet, operations)); // carry on with the validation for all the functions within the math operation if (functions?.length) { return errors.concat(functions.flatMap((fn) => validateNode(fn))); @@ -785,7 +801,8 @@ function runFullASTValidation( // In general this should be handled down the Esaggs route rather than here const isFirstArgumentNotValid = Boolean( !isArgumentValidType(firstArg, 'function') || - (isMathNode(firstArg) && validateMathNodes(firstArg, missingVariablesSet).length) + (isMathNode(firstArg) && + validateMathNodes(firstArg, missingVariablesSet, operations).length) ); // First field has a special handling if (isFirstArgumentNotValid) { @@ -989,7 +1006,11 @@ export function isArgumentValidType(arg: TinymathAST | string, type: TinymathNod return isObject(arg) && arg.type === type; } -export function validateMathNodes(root: TinymathAST, missingVariableSet: Set) { +export function validateMathNodes( + root: TinymathAST, + missingVariableSet: Set, + operations: Record +) { const mathNodes = findMathNodes(root); const errors: ErrorWrapper[] = []; mathNodes.forEach((node: TinymathFunction) => { @@ -1083,26 +1104,26 @@ export function validateMathNodes(root: TinymathAST, missingVariableSet: Set { - const arg = node.args[index]; - if (arg != null) { - if (!isObject(arg)) { - return typeof arg !== type; - } - if (arg.type === 'function' && tinymathFunctions[arg.name]) { - const { outputType = 'number' } = tinymathFunctions[arg.name]; - return outputType !== type; + const wrongTypeArgumentIndexes = positionalArguments + .map(({ type }, index) => { + const arg = node.args[index]; + if (arg != null) { + const argType = getArgumentType(arg, operations); + if (argType && argType !== type) { + return index; + } } - } - }); - if (wrongTypeArgumentIndex > -1) { + }) + .filter(nonNullable); + for (const wrongTypeArgumentIndex of wrongTypeArgumentIndexes) { + const arg = node.args[wrongTypeArgumentIndex]; errors.push( getMessageFromId({ messageId: 'wrongTypeArgument', values: { operation: node.name, name: positionalArguments[wrongTypeArgumentIndex].name, - type: typeof node.args[wrongTypeArgumentIndex], + type: getArgumentType(arg, operations) || 'number', expectedType: positionalArguments[wrongTypeArgumentIndex].type || '', }, locations: getNodeLocation(node), From 3abc4f46b4d5993f5348d4262dd4c56a71428923 Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 5 Oct 2022 15:29:05 +0200 Subject: [PATCH 21/25] :fire: remove duplicate test --- .../operations/definitions/formula/formula.test.tsx | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx index 84b728e9feb43..1b27e22195bb6 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/formula.test.tsx @@ -1658,19 +1658,6 @@ invalid: " ); } ); - it(`[${fn}] returns an error if the condition argument is of the wrong type`, () => { - const [firstArg] = tinymathFunctions[fn].positionalArguments; - expect( - formulaOperation.getErrorMessage!( - getNewLayerWithFormula(`${fn}(1, 2, 3)`), - 'col1', - indexPattern, - operationDefinitionMap - ) - ).toEqual([ - `The ${firstArg.name} argument for the operation ${fn} in the Formula is of the wrong type: number instead of ${firstArg.type}`, - ]); - }); } } From 4a7e738cd83b1f64464bc899acc5d7808b534dfa Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 5 Oct 2022 14:43:04 +0000 Subject: [PATCH 22/25] [CI] Auto-commit changed files from 'node scripts/build_plugin_list_docs' --- docs/developer/plugin-list.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/plugin-list.asciidoc b/docs/developer/plugin-list.asciidoc index 407261c6f1d7e..4853d859b371a 100644 --- a/docs/developer/plugin-list.asciidoc +++ b/docs/developer/plugin-list.asciidoc @@ -71,7 +71,7 @@ as uiSettings within the code. |{kib-repo}blob/{branch}/src/plugins/data_views/README.mdx[dataViews] |The data views API provides a consistent method of structuring and formatting documents -and field lists across the various Kibana apps. Its typically used in conjunction with +and field lists across the various Kibana apps. It's typically used in conjunction with for composing queries. From 0a95ab46fed02757e91b3b22bfd26e3f0a5d77b1 Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Tue, 11 Oct 2022 14:39:29 +0200 Subject: [PATCH 23/25] Update x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts --- .../operations/definitions/formula/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts index c993830185ccb..92bbc292c5ff5 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts @@ -730,7 +730,7 @@ Example: \`gte(average(bytes), 1000)\` Returns a value depending on whether the element of condition is true or false. Example: Average revenue per customer but in some cases customer id is not provided which counts as additional customer -\`sum(total)/(cardinality(customer_id) + ifelse(count() > count(filter=customer_id:*), 1, 0)\` +\`sum(total)/(cardinality(customer_id) + ifelse(count() > count(filter=customer_id:*), 1, 0))\` `, }), }, From 67a6e1ada9d081fed31d87e44566e95bb477327e Mon Sep 17 00:00:00 2001 From: dej611 Date: Tue, 11 Oct 2022 14:59:44 +0200 Subject: [PATCH 24/25] :pencil2: Fixes formula --- .../operations/definitions/formula/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts index 92bbc292c5ff5..4c310b5efa5d3 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/formula/util.ts @@ -730,7 +730,7 @@ Example: \`gte(average(bytes), 1000)\` Returns a value depending on whether the element of condition is true or false. Example: Average revenue per customer but in some cases customer id is not provided which counts as additional customer -\`sum(total)/(cardinality(customer_id) + ifelse(count() > count(filter=customer_id:*), 1, 0))\` +\`sum(total)/(unique_count(customer_id) + ifelse( count() > count(kql='customer_id:*'), 1, 0))\` `, }), }, From 3eb20bc4fbae1648caa53bf8c08dcd5c365ca9b4 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 11 Oct 2022 14:13:08 +0000 Subject: [PATCH 25/25] [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' --- .../privileges/kibana/feature_table/feature_table.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/feature_table/feature_table.tsx b/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/feature_table/feature_table.tsx index f7d480b3b5617..505dd8e70024e 100644 --- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/feature_table/feature_table.tsx +++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/feature_table/feature_table.tsx @@ -7,11 +7,10 @@ import './feature_table.scss'; +import type { EuiAccordionProps, EuiButtonGroupOptionProps } from '@elastic/eui'; import { EuiAccordion, - EuiAccordionProps, EuiButtonGroup, - EuiButtonGroupOptionProps, EuiCallOut, EuiFlexGroup, EuiFlexItem,