Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] [ES|QL] Comment parsing and pretty-printing (#192173) #194117

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

kibanamachine
Copy link
Contributor

Backport

This will backport the following commits from main to 8.x:

Questions ?

Please refer to the Backport tool documentation

## Summary

TL;DR

- Adds ability to parse out comments from source to AST.
- Adds ability for every AST node to have *decoration*—comments,
which can be attached from left, top, and right from the node.
- Implements routine which attached comments to AST nodes.
- In `BasicPrettyPrinter` adds support only for *left* and *right*
comment printing, as the basic printer prints only on one line.
- In `WrappingPrettyPrinter` adds support for all comment printing for
all AST nodes.
- Introduces a `Query` object and `query` AST node, which represent
thole query—the root node, list of commands.
- The ES|QL AST example plugin now displays the pretty-printed text
version.

### Comments

This PR introduced an optional `formatting` field for all AST nodes. In
the `formatting` field one can specify comment decorations from
different sides of a node.

When parsing, once can now specify the `{ withComments: true }` option,
which will collect all comments from the source while parsing using the
`collectDecorations` routine. It will then also call the
`attachDecorations`, which walks the AST and assigns each comment to
some AST node.

Further, traversal and pretty-print API have been updated to work with
comments:

- The `Walker` has been updated to be able to walk all comments from the
AST.
- The `BasicPrettyPrinter` adds support only for *left* and *right*
inline comment printing, as the basic printer prints only on one line.
- The `WrappingPrettyPrinter` adds support for all comment printing for
all AST nodes. It switches to line-break printing mode if it detects
there are comments with line breaks (those could be multi-line comments,
or single line comments—single line comments are always followed
by a line break). It also correctly inserts punctuation, when an AST
node is surrounded by comments.

### Parsing utils

All parsing utils have been moved to the `/parser` sub-folder.

Files in the `/parser` folder have been renamed as per Kibana convention
to reflect what is inside the file. For example, the `EsqlErrorListener`
class is in a file named `esql_error_listener.ts`.

A `Query` class and `ESQLAstQueryExpression` AST nodes have been
introduced. They represent the result of a full query parse. (Before
that, the AST root was just an array of command nodes, now the AST root
is represented by the `ESQLAstQueryExpression` node.)

### Builder

I have started the implementation of the `Builder` static class in the
`/builder` folder. It is simply a collection of stateless AST node
factories—functions which construct AST nodes.

Some of the `Builder` methods are already used by the parser, more will
follow. We will also use the `Builder` in upcoming [*Mutation
API*](elastic#191812).

### ES|QL Example Plugin

This PR sets up Storybook and implements few Storybook stories for the
ES|QL AST example plugin, run it with:

```
yarn storybook esql_ast_inspector
```

This PR updates the *ES|QL AST Explorer* example plugin. Start Kibana
with example plugins enabled:

```
yarn start --run-examples
```

And navigate to
[`/app/esql_ast_inspector`](http://localhost:5601/app/esql_ast_inspector)
to see the new example plugin UI.

![esql-ast-explorer](https://github.com/user-attachments/assets/8ded91ea-1b60-4514-8cf5-c8a4066a3a12)

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Stratoula Kalafateli <[email protected]>
(cherry picked from commit 2217337)
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/esql-ast 139 208 +69

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dashboard 490.7KB 490.6KB -16.0B
dataVisualizer 726.6KB 726.5KB -64.0B
esql 177.1KB 177.0KB -47.0B
infra 1.6MB 1.6MB -16.0B
investigateApp 466.7KB 466.7KB -32.0B
lens 1.5MB 1.5MB -32.0B
maps 3.0MB 3.0MB -48.0B
observabilityAIAssistantApp 154.4KB 154.3KB -32.0B
securitySolution 20.5MB 20.5MB -80.0B
stackAlerts 75.7KB 75.7KB -32.0B
unifiedHistogram 67.3KB 67.2KB -16.0B
total -415.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
@kbn/esql-ast 20 34 +14

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
discover 49.1KB 49.0KB -64.0B
kbnUiSharedDeps-srcJs 3.4MB 3.4MB +16.6KB
total +16.6KB
Unknown metric groups

API count

id before after diff
@kbn/esql-ast 177 266 +89

ESLint disabled in files

id before after diff
@kbn/esql-ast 1 2 +1

References to deprecated APIs

id before after diff
@kbn/esql-utils 0 8 +8
@kbn/securitysolution-utils 0 1 +1
securitySolution 457 459 +2
total +11

Total ESLint disabled count

id before after diff
@kbn/esql-ast 3 4 +1

History

  • 💚 Build #237301 succeeded 5cedfb32e09936a2786f161f1674e00fccf173b5

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @vadimkibana

@kibanamachine kibanamachine merged commit 2fd6817 into elastic:8.x Sep 26, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants