Improves error message for missing or invalid arguments for rangemap command#817
Improves error message for missing or invalid arguments for rangemap command#817Abigael-JT wants to merge 2 commits into
Conversation
elliVM
left a comment
There was a problem hiding this comment.
While we do fix some of the exception cases from parser in this change.
It seems to me that the visitor should not be responsible for handling parser syntax exceptions at all. It should interpret the parse tree with the assumption that the tree is structurally correct.
I think this exposes a common scenario in our current design where the pth_03 parser provides a broken or incomplete tree to the visitor. Result is that these parser failures leak into the visitors and further on into the exception messages.
Parser exception messages don't really make any sense to the end user as pointed out in the issue description.
No easy solution for this here but I think it is worth raising the problem.
See my suggestion for refactoring the pth_03 rule and if it makes sense to you please create an issue for it.
|
|
||
| @Override | ||
| public Node visitT_rangemap_attrnParameter(DPLParser.T_rangemap_attrnParameterContext ctx) { | ||
| final String key = ctx.stringType().getText(); |
There was a problem hiding this comment.
We are currently building separate tokens for GET_NUMBER_LEFT and GET_NUMBER_RIGHT which leads to partial parse states that force defensive handling here.
It could be worth considering refactoring the pth_03 parser grammar to represent
range : NUMBER '-' NUMBER.
Then extraction becomes easy here
left = rangeCtx.NUMBER(0).getText()
right = rangeCtx.NUMBER(1).getText()
// parse to double
// ensure start <= end
There was a problem hiding this comment.
That's a valid point, I have raised a ticket for this in PTH_03 teragrep/pth_03#120
Improves error message for missing or invalid arguments for rangemap command
Description
Checklists
Testing
General
Assertions
Testing Data
Statements
Java
Other
Code Quality