-
Notifications
You must be signed in to change notification settings - Fork 83
feat(webui): Add hover messages for ANTLR parsing errors in Monaco single-line editor. #1569
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
base: main
Are you sure you want to change the base?
Changes from all commits
bf8af6a
33f4afe
1b326fd
2b9a0ea
6f83b4d
63d3d45
57edfd4
e26809e
76248cd
cb77dad
8799f54
ec83aef
528f9b5
997e4a3
910c876
7385911
d55cd60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| /** | ||
| * Escapes angle brackets in text for safe display in Monaco editor hover tooltips. | ||
| * Monaco uses Markdown for hover messages, so raw `<` and `>` characters are | ||
| * interpreted as HTML tags and must be escaped to display literals like `<EOF>`. | ||
| * | ||
| * @param text | ||
| * @return Escaped text. | ||
| */ | ||
| const escapeHoverMarkdown = (text: string) => text.replace(/[<>]/g, (ch) => ("<" === ch ? | ||
| "<" : | ||
| ">")); | ||
|
|
||
| export {escapeHoverMarkdown}; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import { | |
| CommonTokenStream, | ||
| ErrorListener, | ||
| Recognizer, | ||
| Token, | ||
| } from "antlr4"; | ||
|
|
||
| import SqlLexer from "./generated/SqlLexer"; | ||
|
|
@@ -17,16 +18,36 @@ import { | |
| class SyntaxError extends Error { | ||
| } | ||
|
|
||
| interface ValidationError { | ||
| line: number; | ||
| column: number; | ||
| message: string; | ||
| startColumn: number; | ||
| endColumn: number; | ||
| } | ||
|
|
||
| class SyntaxErrorListener<TSymbol> extends ErrorListener<TSymbol> { | ||
| // eslint-disable-next-line max-params, class-methods-use-this | ||
| errors: ValidationError[] = []; | ||
|
|
||
| // eslint-disable-next-line max-params | ||
| override syntaxError ( | ||
| _recognizer: Recognizer<TSymbol>, | ||
| _offendingSymbol: TSymbol, | ||
| offendingSymbol: TSymbol, | ||
| line: number, | ||
| column: number, | ||
| msg: string, | ||
| ) { | ||
| throw new SyntaxError(`line ${line}:${column}: ${msg}`); | ||
| const token = offendingSymbol as unknown as Token; | ||
| const startColumn = token.start + 1; | ||
| const endColumn = token.stop + 2; | ||
|
|
||
| this.errors.push({ | ||
| column: column, | ||
| endColumn: endColumn, | ||
| line: line, | ||
| message: msg, | ||
| startColumn: startColumn, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -51,31 +72,60 @@ class UpperCaseCharStream extends CharStream { | |
| } | ||
|
|
||
| /** | ||
| * Creates a SQL parser for a given input string. | ||
| * Helper function to set up parser with error listener. | ||
| * | ||
| * @param input The SQL query string to be parsed. | ||
| * @return The configured SQL parser instance ready to parse the input. | ||
| * @param sqlString | ||
| * @return Object containing parser and error listener | ||
| */ | ||
| const buildParser = (input: string): SqlParser => { | ||
| const setupParser = (sqlString: string) => { | ||
| const syntaxErrorListener = new SyntaxErrorListener(); | ||
| const lexer = new SqlLexer(new UpperCaseCharStream(input)); | ||
| const lexer = new SqlLexer(new UpperCaseCharStream(sqlString)); | ||
| lexer.removeErrorListeners(); | ||
| lexer.addErrorListener(syntaxErrorListener); | ||
| const parser = new SqlParser(new CommonTokenStream(lexer)); | ||
| parser.removeErrorListeners(); | ||
| parser.addErrorListener(syntaxErrorListener); | ||
|
|
||
| return parser; | ||
| return {parser, syntaxErrorListener}; | ||
| }; | ||
|
|
||
| /** | ||
| * Validate a SELECT item list and return any syntax errors found. | ||
| * | ||
| * @param sqlString | ||
| * @return Array of validation errors, empty if valid | ||
| */ | ||
| const validateSelectItemList = (sqlString: string): ValidationError[] => { | ||
| const {parser, syntaxErrorListener} = setupParser(sqlString); | ||
| parser.standaloneSelectItemList(); | ||
|
|
||
| return syntaxErrorListener.errors; | ||
| }; | ||
|
Comment on lines
+98
to
103
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider adding error handling to validation functions. The validation functions don't have try-catch blocks. If the parser encounters an unexpected error (beyond syntax errors), it will propagate to the caller and potentially crash the UI. Consider wrapping validation logic: const validateSelectItemList = (sqlString: string): ValidationError[] => {
+ try {
const {parser, syntaxErrorListener} = setupParser(sqlString);
parser.standaloneSelectItemList();
return syntaxErrorListener.errors;
+ } catch (error) {
+ console.error("Unexpected validation error:", error);
+ return [{
+ line: 1,
+ column: 0,
+ startColumn: 0,
+ endColumn: sqlString.length,
+ message: "Unexpected validation error occurred"
+ }];
+ }
};Apply similar pattern to Also applies to: 111-116, 124-129 🤖 Prompt for AI Agents |
||
|
|
||
| /** | ||
| * Validate a SQL string for syntax errors. | ||
| * Validate a boolean expression and return any syntax errors found. | ||
| * | ||
| * @param sqlString | ||
| * @throws {SyntaxError} with line, column, and message details if a syntax error is found. | ||
| * @return Array of validation errors, empty if valid | ||
| */ | ||
| const validate = (sqlString: string) => { | ||
| buildParser(sqlString).singleStatement(); | ||
| const validateBooleanExpression = (sqlString: string): ValidationError[] => { | ||
| const {parser, syntaxErrorListener} = setupParser(sqlString); | ||
| parser.standaloneBooleanExpression(); | ||
|
|
||
| return syntaxErrorListener.errors; | ||
| }; | ||
|
|
||
| /** | ||
| * Validate a sort item list and return any syntax errors found. | ||
| * | ||
| * @param sqlString | ||
| * @return Array of validation errors, empty if valid | ||
| */ | ||
| const validateSortItemList = (sqlString: string): ValidationError[] => { | ||
| const {parser, syntaxErrorListener} = setupParser(sqlString); | ||
| parser.standaloneSortItemList(); | ||
|
|
||
| return syntaxErrorListener.errors; | ||
| }; | ||
|
|
||
| const MILLISECONDS_PER_SECOND = 1000; | ||
|
|
@@ -93,7 +143,6 @@ const MILLISECONDS_PER_SECOND = 1000; | |
| * @param props.endTimestamp | ||
| * @param props.timestampKey | ||
| * @return | ||
| * @throws {Error} if the constructed SQL string is not valid. | ||
| */ | ||
| const buildSearchQuery = ({ | ||
| selectItemList, | ||
|
|
@@ -120,12 +169,6 @@ AND ${endTimestamp.valueOf() / MILLISECONDS_PER_SECOND}`; | |
| queryString += `\nLIMIT ${limitValue}`; | ||
| } | ||
|
|
||
| try { | ||
| validate(queryString); | ||
| } catch (err: unknown) { | ||
| console.error(`The constructed SQL is not valid: ${queryString}`, err); | ||
| } | ||
|
|
||
| return queryString; | ||
| }; | ||
|
|
||
|
|
@@ -141,7 +184,6 @@ AND ${endTimestamp.valueOf() / MILLISECONDS_PER_SECOND}`; | |
| * @param props.timestampKey | ||
| * @param props.booleanExpression | ||
| * @return | ||
| * @throws {Error} if the constructed SQL string is not valid. | ||
| */ | ||
| const buildTimelineQuery = ({ | ||
| databaseName, | ||
|
|
@@ -189,23 +231,20 @@ LEFT JOIN timestamps ON buckets.idx = timestamps.idx | |
| ORDER BY timestamps.idx | ||
| `; | ||
|
|
||
| try { | ||
| validate(queryString); | ||
| } catch (err: unknown) { | ||
| console.error(`The constructed SQL is not valid: ${queryString}`, err); | ||
| } | ||
|
|
||
| return queryString; | ||
| }; | ||
|
|
||
| export { | ||
| buildSearchQuery, | ||
| buildTimelineQuery, | ||
| SyntaxError, | ||
| validate, | ||
| validateBooleanExpression, | ||
| validateSelectItemList, | ||
| validateSortItemList, | ||
| }; | ||
|
|
||
| export type { | ||
| BuildSearchQueryProps, | ||
| BuildTimelineQueryProps, | ||
| ValidationError, | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify type safety of Token cast and properties.
The cast
offendingSymbol as unknown as Tokenand subsequent access totoken.startandtoken.stopcould fail at runtime if the offending symbol is not actually a Token or lacks these properties. ANTLR4's error listener can receive different symbol types depending on the error context.Consider adding runtime checks:
Alternatively, verify with the ANTLR4 documentation that offendingSymbol is always a Token for lexer/parser errors and document this assumption.
I'll verify this review comment by examining the code context and investigating ANTLR4's error listener specifications.
Length of output: 1806
🌐 Web query:
💡 Result:
offendingSymbol (the Token passed to ANTLRErrorListener.syntaxError) implements org.antlr.v4.runtime.Token and exposes these useful properties/methods:
In an error listener you typically do:
public void syntaxError(Recognizer r, Object offendingSymbol, int line, int pos, String msg, RecognitionException e) {
Token t = (Token) offendingSymbol;
String text = t.getText();
int type = t.getType();
int start = t.getStartIndex();
int stop = t.getStopIndex();
int tokenIndex = t.getTokenIndex();
// ...
}
The listener contract and the meaning of offendingSymbol are documented on the ANTLR error-listener API. [2]
Sources: ANTLR Token API (methods listed above). [1] ANTLR error-listener / BaseErrorListener docs (offendingSymbol description). [2]
I need to verify the JavaScript/TypeScript ANTLR4 Token interface, as the web search returned Java documentation. Let me investigate further.
Length of output: 1108
🏁 Script executed:
Length of output: 3932
🏁 Script executed:
Length of output: 350
🌐 Web query:
💡 Result:
Short answer: In the JS/TypeScript ANTLR runtimes tokens expose startIndex and stopIndex (character offsets); parser contexts expose start and stop Token objects. Use token.startIndex / token.stopIndex for character positions and ctx.start / ctx.stop for the tokens that bracket a rule. [1][2][3]
Sources
Use correct Token property names:
startIndexandstopIndex.The review comment correctly identifies a type safety concern, but the underlying issue is more critical than suggested. In the JavaScript ANTLR4 runtime (version 4.13.2), Token objects expose
startIndexandstopIndexproperties, notstartandstop. The current code will fail at runtime attempting to access undefined properties.Update lines 41–42 to use:
The double cast bypasses TypeScript but does not prevent the runtime failure. No optional chaining fallback is needed if you correct the property names.
🤖 Prompt for AI Agents