[ES|QL] refactor isEqualType

packages/kbn-esql-validation-autocomplete/index.ts

@@ -49,11 +49,11 @@ export {
   getCommandDefinition,
   getAllCommands,
   getCommandOption,
-  getColumnHit,
+  getColumnByName,
   columnExists,
   shouldBeQuotedText,
   printFunctionSignature,
-  isEqualType,
+  checkArgTypeMatchesDefinition,
   isSourceItem,
   isSettingItem,
   isFunctionItem,

packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts

@@ -18,7 +18,7 @@ import type {
 import type { EditorContext, SuggestionRawDefinition } from './types';
 import {
   columnExists,
-  getColumnHit,
+  getColumnByName,
   getCommandDefinition,
   getCommandOption,
   getFunctionDefinition,
@@ -324,7 +324,9 @@ function workoutBuiltinOptions(
   references: Pick<ReferenceMaps, 'fields' | 'variables'>
 ): { skipAssign: boolean } {
   // skip assign operator if it's a function or an existing field to avoid promoting shadowing
-  return { skipAssign: Boolean(!isColumnItem(nodeArg) || getColumnHit(nodeArg.name, references)) };
+  return {
+    skipAssign: Boolean(!isColumnItem(nodeArg) || getColumnByName(nodeArg.name, references)),
+  };
 }
 
 function areCurrentArgsValid(
@@ -391,7 +393,7 @@ function extractFinalTypeFromArg(
       return arg.literalType;
     }
     if (isColumnItem(arg)) {
-      const hit = getColumnHit(arg.name, references);
+      const hit = getColumnByName(arg.name, references);
       if (hit) {
         return hit.type;
       }

packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts

@@ -219,7 +219,7 @@ function compareLiteralType(argTypes: string, item: ESQLLiteral) {
   return argTypes === item.literalType;
 }
 
-export function getColumnHit(
+export function getColumnByName(
   columnName: string,
   { fields, variables }: Pick<ReferenceMaps, 'fields' | 'variables'>,
   position?: number
@@ -322,8 +322,8 @@ export function getAllArrayTypes(
         types.push(subArg.literalType);
       }
       if (subArg.type === 'column') {
-        const hit = getColumnHit(subArg.name, references);
-        types.push(hit?.type || 'unsupported');
+        const column = getColumnByName(subArg.name, references);
+        types.push(column?.type || 'unsupported');
       }
       if (subArg.type === 'timeInterval') {
         types.push('time_literal');
@@ -363,42 +363,57 @@ export function isValidLiteralOption(arg: ESQLLiteral, argDef: FunctionArgSignat
  * Checks if an AST argument is of the correct type
  * given the definition.
  */
-export function isEqualType(
+export function checkArgTypeMatchesDefinition(
   arg: ESQLSingleAstItem,
   argDef: SignatureArgType,
   references: ReferenceMaps,
   parentCommand?: string,
   nameHit?: string
-) {
-  const argType = 'innerType' in argDef && argDef.innerType ? argDef.innerType : argDef.type;
-  if (argType === 'any') {
-    return true;
+): { matches: true } | { matches: false; expectedType: string; givenType: string } {
+  const expectedArgType =
+    'innerType' in argDef && argDef.innerType ? argDef.innerType : argDef.type;
+  if (expectedArgType === 'any') {
+    return { matches: true };
   }
   if (arg.type === 'literal') {
-    return compareLiteralType(argType, arg);
+    return compareLiteralType(expectedArgType, arg)
+      ? { matches: true }
+      : { matches: false, expectedType: expectedArgType, givenType: arg.literalType };
   }
   if (arg.type === 'function') {
     if (isSupportedFunction(arg.name, parentCommand).supported) {
       const fnDef = buildFunctionLookup().get(arg.name)!;
-      return fnDef.signatures.some((signature) => argType === signature.returnType);
+      // TODO - should filter down signatures based on current args
+      return fnDef.signatures.some((signature) => expectedArgType === signature.returnType)
+        ? { matches: true }
+        : {
+            matches: false,
+            expectedType: expectedArgType,
+            givenType: fnDef.signatures[0].returnType,
+          };
     }
   }
   if (arg.type === 'timeInterval') {
-    return argType === 'time_literal' && inKnownTimeInterval(arg);
+    return expectedArgType === 'time_literal' && inKnownTimeInterval(arg)
+      ? { matches: true }
+      : { matches: false, expectedType: expectedArgType, givenType: arg.type };
   }
   if (arg.type === 'column') {
-    if (argType === 'column') {
+    if (expectedArgType === 'column') {
       // anything goes, so avoid any effort here
-      return true;
+      return { matches: true };
     }
-    const hit = getColumnHit(nameHit ?? arg.name, references);
-    const validHit = hit;
-    if (!validHit) {
-      return false;
+    const column = getColumnByName(nameHit ?? arg.name, references);
+    if (!column) {
+      return { matches: false, expectedType: expectedArgType, givenType: arg.type };
     }
-    const wrappedTypes = Array.isArray(validHit.type) ? validHit.type : [validHit.type];
-    return wrappedTypes.some((ct) => argType === ct);
+    const wrappedTypes = Array.isArray(column.type) ? column.type : [column.type];
+    return wrappedTypes.some((ct) => expectedArgType === ct)
+      ? { matches: true }
+      : { matches: false, expectedType: expectedArgType, givenType: wrappedTypes[0] };
   }
+
+  return { matches: false, expectedType: expectedArgType, givenType: arg.type };
 }
 
 function fuzzySearch(fuzzyName: string, resources: IterableIterator<string>) {

packages/kbn-esql-validation-autocomplete/src/validation/errors.ts

@@ -22,17 +22,19 @@ function getMessageAndTypeFromId<K extends ErrorTypes>({
   // i18n validation wants to the values prop to be declared inline, so need to unpack and redeclare again all props
   switch (messageId) {
     case 'wrongArgumentType':
+      const printType = (type: string) => (type === 'timeInterval' ? 'duration' : type);
+
       return {
         message: i18n.translate(
           'kbn-esql-validation-autocomplete.esql.validation.wrongArgumentType',
           {
             defaultMessage:
-              'Argument of [{name}] must be [{argType}], found value [{value}] type [{givenType}]',
+              'Argument of [{name}] must be [{expectedType}], found value [{value}] type [{givenType}]',
             values: {
               name: out.name,
-              argType: out.argType,
               value: out.value,
-              givenType: out.givenType,
+              expectedType: printType(out.expectedType),
+              givenType: printType(out.givenType),
             },
           }
         ),

packages/kbn-esql-validation-autocomplete/src/validation/helpers.ts

@@ -61,7 +61,7 @@ export function collapseWrongArgumentTypeMessages(
       messageId: 'wrongArgumentType',
       values: {
         name: funcName,
-        argType,
+        expectedType: argType,
         value: `(${getAllArrayValues(arg).join(', ')})`,
         givenType: `(${getAllArrayTypes(arg, parentCommand, references).join(', ')})`,
       },

packages/kbn-esql-validation-autocomplete/src/validation/types.ts

@@ -41,8 +41,8 @@ export interface ValidationErrors {
     message: string;
     type: {
       name: string;
-      argType: string;
       value: string | number | Date;
+      expectedType: string;
       givenType: string;
     };
   };

packages/kbn-esql-validation-autocomplete/src/validation/validation.ts

@@ -29,12 +29,12 @@ import {
 import {
   areFieldAndVariableTypesCompatible,
   extractSingularType,
-  getColumnHit,
+  getColumnByName,
   getCommandDefinition,
   getFunctionDefinition,
   isArrayType,
   isColumnItem,
-  isEqualType,
+  checkArgTypeMatchesDefinition,
   isFunctionItem,
   isLiteralItem,
   isOptionItem,
@@ -100,15 +100,16 @@ function validateFunctionLiteralArg(
       );
     }
 
-    if (!isEqualType(actualArg, argDef, references, parentCommand)) {
+    const matchResult = checkArgTypeMatchesDefinition(actualArg, argDef, references, parentCommand);
+    if (!matchResult.matches) {
       messages.push(
         getMessageFromId({
           messageId: 'wrongArgumentType',
           values: {
             name: astFunction.name,
-            argType: argDef.type,
+            expectedType: matchResult.expectedType,
             value: actualArg.value,
-            givenType: actualArg.literalType,
+            givenType: matchResult.givenType,
           },
           locations: actualArg.location,
         })
@@ -128,15 +129,21 @@ function validateFunctionLiteralArg(
         })
       );
     } else {
-      if (!isEqualType(actualArg, argDef, references, parentCommand)) {
+      const matchResult = checkArgTypeMatchesDefinition(
+        actualArg,
+        argDef,
+        references,
+        parentCommand
+      );
+      if (!matchResult.matches) {
         messages.push(
           getMessageFromId({
             messageId: 'wrongArgumentType',
             values: {
               name: astFunction.name,
-              argType: argDef.type,
+              expectedType: matchResult.expectedType,
               value: actualArg.name,
-              givenType: 'duration',
+              givenType: matchResult.givenType,
             },
             locations: actualArg.location,
           })
@@ -173,15 +180,16 @@ function validateNestedFunctionArg(
         })
       );
     }
-    if (!isEqualType(actualArg, argDef, references, parentCommand)) {
+    const matchResult = checkArgTypeMatchesDefinition(actualArg, argDef, references, parentCommand);
+    if (!matchResult.matches) {
       messages.push(
         getMessageFromId({
           messageId: 'wrongArgumentType',
           values: {
             name: astFunction.name,
-            argType: argDef.type,
             value: printFunctionSignature(actualArg) || actualArg.name,
-            givenType: argFn.signatures[0].returnType,
+            expectedType: matchResult.expectedType,
+            givenType: matchResult.givenType,
           },
           locations: actualArg.location,
         })
@@ -253,17 +261,22 @@ function validateFunctionColumnArg(
           }
           // do not validate any further for now, only count() accepts wildcard as args...
         } else {
-          if (!isEqualType(actualArg, argDef, references, parentCommand, nameHit)) {
-            // guaranteed by the check above
-            const columnHit = getColumnHit(nameHit!, references);
+          const matchResult = checkArgTypeMatchesDefinition(
+            actualArg,
+            argDef,
+            references,
+            parentCommand,
+            nameHit
+          );
+          if (!matchResult.matches) {
             messages.push(
               getMessageFromId({
                 messageId: 'wrongArgumentType',
                 values: {
                   name: astFunction.name,
-                  argType: argDef.type,
                   value: actualArg.name,
-                  givenType: columnHit!.type,
+                  expectedType: matchResult.expectedType,
+                  givenType: matchResult.givenType,
                 },
                 locations: actualArg.location,
               })
@@ -694,7 +707,7 @@ function validateColumnForCommand(
         ({ type, innerType }) => type === 'column' && innerType
       );
       // this should be guaranteed by the columnCheck above
-      const columnRef = getColumnHit(nameHit, references)!;
+      const columnRef = getColumnByName(nameHit, references)!;
 
       if (columnParamsWithInnerTypes.length) {
         const hasSomeWrongInnerTypes = columnParamsWithInnerTypes.every(({ innerType }) => {