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

Implement error reporting in monaco for painless language #84695

Conversation

alisonelizabeth
Copy link
Contributor

@alisonelizabeth alisonelizabeth commented Dec 1, 2020

This PR adds error reporting in monaco for basic syntax errors in the Painless language.

Continuation of work related to #80577.

Implementation

I integrated ANTLR to track syntax errors (and in the future help support contextual autocomplete) in a Painless script. Elasticsearch already has defined lexer and parser rules for the Painless language. For now, these rules have been largely copied from ES to Kibana and reside in the antlr directory with the .g4 file extension (some modifications were needed in order to handle Java-specific code in the grammar). We then use antlr4ts to generate a lexer and a parser in Typescript.

The parser is initialized in parser.ts. I've implemented an ANTLRErrorListener(error_listener.ts) which can collect the errors as ANTLR parses the code. The errors include the position in which the error occurred, which can then be passed to monaco to set error model markers.

I've updated the readme with this information^. There's also more details in the readme around the script used to generate the lexer and parser in TS.

How to review

Anything under the antlr directory includes either the grammar definitions from ES or the generated TS and does not need to be reviewed.

Using Painless Lab (or ingest node pipelines UI or the runtime fields editor), write a script and make a syntax error(s) and verify the monaco editor highlights it accordingly.

Jt56b6mvfN

Future improvements

The current implementation has room for improvement.

  • Better handling of error position. For example, if you forget a closing semi-colon, the error marker appears at the beginning of the next line, rather than at the end of the affected line. Note that this is also off in the result of the painless execute API itself.

Dev-Tools-Elastic

download

  • Better error messages. The error messages returned from ANTLR are sometimes a little hard to parse. It does appear possible to override them (as ES appears to be doing here and here for the execute API), but needs further investigation.

  • Maintenance. As mentioned, the antlr grammar is largely copied from ES. If the grammar is ever modified in ES, the UI team would need to be made aware and also apply the changes. While it's probably not likely the language will change too often or drastically, it is not ideal and prone to be forgotten.

  • Surface syntax error(s) to a form. If the editor is used in the context of a form, it would be helpful to block on submit if there are any syntax errors reported by the editor.

@alisonelizabeth alisonelizabeth added release_note:enhancement painless painless v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.11.0 Feature:Painless Lab Dev tool for learning Painless labels Dec 1, 2020
@alisonelizabeth alisonelizabeth added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:enhancement labels Dec 7, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
enterpriseSearch 1.7MB 1.7MB +60.0B

Distributable file count

id before after diff
default 46929 47703 +774
oss 27560 27574 +14

Page load bundle

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

id before after diff
upgradeAssistant 60.5KB 60.5KB +60.0B
Unknown metric groups

@kbn/ui-shared-deps asset size

id before after diff
kbn-ui-shared-deps.@elastic.js 3.2MB 3.2MB +1.0B
kbn-ui-shared-deps.js 13.5MB 14.0MB ⚠️ +431.1KB
total +431.1KB

History

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

@alisonelizabeth alisonelizabeth marked this pull request as ready for review December 7, 2020 21:03
@alisonelizabeth alisonelizabeth requested a review from a team as a code owner December 7, 2020 21:03
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@cjcenizal
Copy link
Contributor

🚀 This is so slick!

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @alisonelizabeth ! I tested this locally and it is really awesome to finally see in line error reporting.

It's interesting that package bundle sizes seem relatively unaffected 🤔

.toString()
.split('\n');

fileContentRows.unshift('// @ts-nocheck');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Glad to see we can disable TS checks at a file-level!

@alisonelizabeth alisonelizabeth merged commit f8edc51 into elastic:master Dec 8, 2020
@alisonelizabeth alisonelizabeth deleted the monaco/painless_error_reporting branch December 8, 2020 15:07
stu-elastic added a commit to stu-elastic/kibana that referenced this pull request Feb 16, 2022
$('fieldname', <default>) was added in elasticsearch/elastic#80518.

This updated the ANTLR grammar so that the syntax check
passes.

painless_lexer.g4 adapted from elasticsearch/modules/lang-painless/src/main/antlr/PainlessLexer.g4
painless_parser.g4 adapted from elasticsearch/modules/lang-painless/src/main/antlr/PainlessParser.g4

Generated by running `npm run build:antlr4ts` from `packages/kbn-monaco`

Related to elastic#84695
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Painless Lab Dev tool for learning Painless painless painless release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants