[ES|QL] Validation and autocomplete code improvements

[drewdaemon]

That the validation and autocomplete engines work today is a feat in and of itself. However, there is a lot of room for simplification and understandability improvements.

[drewdaemon]

Separate AST traversal from validation code

Our current validation routine is essentially a top-down AST traversal that runs various node-type-specific validation routines. For example, there’s a routine for function arguments, and one for commands, and one for options, etc.
Right now, it’s essentially a set of calls between these routines. For example, the command routine calls the function routine, passes down some context, and then the function routine may call itself if there’s a nested function before finally checking the args, etc.
There’s a messages array that gets passed by reference through the call stack and collects errors and warnings that are generated.
What if, instead, we separated the validation from the traversal logic and ended up with a single-node-centric validation pattern.
const messages = [];

walk(ast, (node) => {
  if (isCommand(node)) {
    /* validate command */
    messages.push(...validateCommand(node));
  }
  if (isFunction(node)) {
    /* validate function */
    messages.push(...validateFunction(node));
  }
  ...
}

return messages;

This might require that the AST have bidirectional references since you might have to “reach up” to the parent of a node to perform a proper validation.

But, I think this could be a great separation of concerns (separating tree traversal and checking against definitions). I also have a feeling that it would promote type specificity since the node-type checks will happen at the top level and there will be no casting or checking within the subroutines….

[drewdaemon]

Handle literal suggestions uniformly and declaratively

Right now we have two ways to suggest literal values. One is the literalOptions property that lives on a functions parameter definition. The other is getCompatibleLiterals in packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts

Does this make sense? I would vote for literal suggestions always to be defined in the definition of a parameter. If they need to be generated based on dynamic context, the logic could be housed in a callback attached to the parameter definition, just like we do with custom validation.

At the moment, getCompatibleLiterals makes me nervous. It does checks based on both command name and parameter type and feels like it will be difficult to maintain/grow.

[drewdaemon]

Improve names

• isEqualType -> checkArgTypeMatchesDefinition
• getColumnHit -> getColumnByName
• literal -> constant
• literalOptions -> acceptedConstants
• builtin function -> operation

[drewdaemon]

Revisit expression autocomplete logic

#181916 (comment)

[drewdaemon]

Evaluate AST structure for ease of traversal

Does it make sense to handle arrays like this? type ESQLAstItem = ESQLSingleAstItem | ESQLAstItem[];

Does it make sense to add bidirectional references between parents and children?

Does it make sense to define parent-child relationships as args or should they simply be generic?

[drewdaemon]

Constant-only vs literal types

Right now, we have two ways to say "this parameter shouldn't accept fields." One is via the constantOnly flag, and the other is via time_literal and chrono_literal types which are supposed to be treated as if constantOnly were set to true by default. Are we okay with having two ways to do one thing?

[drewdaemon]

Evaluate handling of array types

The way validation and the AST handle array types is a blindspot for me (Drew) and makes me nervous.

E.g. | eval 1 in (1, 2, 3)