-
Notifications
You must be signed in to change notification settings - Fork 78
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add ErrorStrategy and ErrorListener to catch and rewrite error messag…
…es to be more helpful (#2136) * Add an errorHandler and an error rewriter to the compiler phase * Some refactoring * Add another specific case and support negative token matching * Fix tests
- Loading branch information
Showing
8 changed files
with
346 additions
and
61 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
109 changes: 109 additions & 0 deletions
109
packages/malloy/src/lang/syntax-errors/custom-error-messages.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| /* | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| import {Parser, Token} from 'antlr4ts'; | ||
|
|
||
| export interface ErrorCase { | ||
| // The rule contexts in which to apply this error case. | ||
| // If no context is provided, this error case will apply to all rules. | ||
| ruleContextOptions?: string[]; | ||
| // This is the symbol that triggered the error. In general, prefer to use | ||
| // this over `offendingToken` when possible. | ||
| offendingSymbol?: number; | ||
| // The value of the current token when this error rewrite should occur. | ||
| // In general, prefer to use offendingSymbol | ||
| currentToken?: number; | ||
|
|
||
| // The tokens preceding the offending token, in the order they occur. | ||
| // Make the token negative to match all other tokens. | ||
| precedingTokenOptions?: number[][]; | ||
|
|
||
| // If provided, at least one of the look ahead sequences would need to match. | ||
| // Make the token negative to match all other tokens. | ||
| lookAheadOptions?: number[][]; | ||
|
|
||
| // The error message to show to the user, instead of whatever was default | ||
| // Supports tokenization: ${currentToken} | ||
| errorMessage: string; | ||
| } | ||
|
|
||
| export const checkCustomErrorMessage = ( | ||
| parser: Parser, | ||
| offendingSymbol: Token | undefined, | ||
| errorCases: ErrorCase[] | ||
| ): string => { | ||
| const currentRuleName = parser.getRuleInvocationStack()[0]; | ||
| const currentToken = parser.currentToken; | ||
|
|
||
| for (const errorCase of errorCases) { | ||
| // Check to see if the initial conditions match | ||
| const isCurrentTokenMatch = | ||
| !errorCase.currentToken || currentToken.type === errorCase.currentToken; | ||
| const isOffendingSymbolMatch = | ||
| !errorCase.offendingSymbol || | ||
| offendingSymbol?.type === errorCase.offendingSymbol; | ||
| const isRuleContextMatch = | ||
| !errorCase.ruleContextOptions || | ||
| errorCase.ruleContextOptions.includes(currentRuleName); | ||
| if (isCurrentTokenMatch && isOffendingSymbolMatch && isRuleContextMatch) { | ||
| // If so, try to check the preceding tokens. | ||
| if (errorCase.precedingTokenOptions) { | ||
| const hasPrecedingTokenMatch = errorCase.precedingTokenOptions.some( | ||
| sequence => checkTokenSequenceMatch(parser, sequence, 'lookback') | ||
| ); | ||
| if (!hasPrecedingTokenMatch) { | ||
| continue; // Continue to check a different error case | ||
| } | ||
| } | ||
| if (errorCase.lookAheadOptions) { | ||
| const hasLookaheadTokenMatch = errorCase.lookAheadOptions.some( | ||
| sequence => checkTokenSequenceMatch(parser, sequence, 'lookahead') | ||
| ); | ||
| if (!hasLookaheadTokenMatch) { | ||
| continue; // Continue to check a different error case | ||
| } | ||
| } | ||
|
|
||
| // If all cases match, return the custom error message | ||
| const message = errorCase.errorMessage.replace( | ||
| '${currentToken}', | ||
| offendingSymbol?.text || currentToken.text || '' | ||
| ); | ||
| return message; | ||
| } | ||
| } | ||
|
|
||
| return ''; | ||
| }; | ||
|
|
||
| const checkTokenSequenceMatch = ( | ||
| parser: Parser, | ||
| sequence: number[], | ||
| direction: 'lookahead' | 'lookback' | ||
| ): boolean => { | ||
| try { | ||
| for (let i = 0; i < sequence.length; i++) { | ||
| // Note: positive lookahead starts at '2' because '1' is the current token. | ||
| const tokenIndex = direction === 'lookahead' ? i + 2 : -1 * (i + 1); | ||
| const streamToken = parser.inputStream.LA(tokenIndex); | ||
|
|
||
| // Note: negative checking is < -1 becuase Token.EOF is -1, but below | ||
| // that we use negatives to indicate "does-not-match" rules. | ||
| if (sequence[i] >= -1 && streamToken !== sequence[i]) { | ||
| return false; | ||
| } | ||
|
|
||
| if (sequence[i] < -1 && streamToken === -1 * sequence[i]) { | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| } catch (ex) { | ||
| // There may not be enough lookback tokens. If so, the case doesn't match. | ||
| return false; | ||
| } | ||
| }; |
50 changes: 50 additions & 0 deletions
50
packages/malloy/src/lang/syntax-errors/malloy-error-strategy.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| /* | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| import {DefaultErrorStrategy, Parser} from 'antlr4ts'; | ||
| import {MalloyParser} from '../lib/Malloy/MalloyParser'; | ||
| import {checkCustomErrorMessage, ErrorCase} from './custom-error-messages'; | ||
|
|
||
| const customErrorCases: ErrorCase[] = [ | ||
| { | ||
| errorMessage: "Missing '{' after 'extend'", | ||
| currentToken: MalloyParser.EXTEND, | ||
| ruleContextOptions: ['sqExpr'], | ||
| lookAheadOptions: [[-MalloyParser.OCURLY]], | ||
| }, | ||
| ]; | ||
|
|
||
| /** | ||
| * Custom error strategy for the Malloy Parser. This strategy attempts to | ||
| * detect known cases where the default error strategy results in an unhelpful | ||
| * parse tree or misleading messages. In any cases not explicitly handled, this | ||
| * custom error strategy will fall back to the default error strategy. | ||
| * | ||
| * For more details, read the documentation in DefaultErrorStrategy.d.ts | ||
| * or reference the superclass at: | ||
| * https://github.com/tunnelvisionlabs/antlr4ts/blob/master/src/DefaultErrorStrategy.ts | ||
| */ | ||
| export class MalloyErrorStrategy extends DefaultErrorStrategy { | ||
| override sync(parser: Parser) { | ||
| const interceptedErrorMessage = checkCustomErrorMessage( | ||
| parser as MalloyParser, | ||
| undefined, | ||
| customErrorCases | ||
| ); | ||
| if (interceptedErrorMessage) { | ||
| parser.notifyErrorListeners( | ||
| interceptedErrorMessage, | ||
| parser.currentToken, | ||
| undefined | ||
| ); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| super.sync(parser); | ||
| } | ||
| } |
Oops, something went wrong.