apblack
commented
4 years ago OK, here is another one where the suggestion is actually spot-on: the problem is a missing close paren, which the suggestion hits.
identifierresolution.grace[429:28-57]: Syntax error: an argument list beginning with a '(' must end with a ')'.
428: }
429: entries.add(serializeVariable (v) in (s)
---------------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Did you mean:
429: entries.add(serializeVariable (v) in (s))In contrast, this one shows the downside. The problem is that I used type as part of a method name, forgetting that it is reserved. The "suggestion" is worse than useless: it actually detracts from the error message, rather than adding to it.
variables.grace[616:19]: Syntax error: a class must have a '{' after the class header.
615:
616: class named(aName) type(dType) annotations (anns) kind (aKind) {
------------------------^
Did you mean:
616: class named(aName) { type(dType) annotations (anns) kind (aKind) {This one also makes garbage suggestions:
identifierresolution.grace[869:17-30]: Syntax error: an expression using two different operators requires parentheses
868: def v = vNameScope.node
869: if (ast.nullNode ≠ v && {v.isMethod}) then {
----------------------^^^^^^^^^^^^^^
Did you mean:
869: if (ast.nullNode ≠ (v && {)v.isMethod}) then {
Did you mean:
869: if (ast.(nullNode ≠ v) && {v.isMethod}) then {It's possible that, with a lot of work, the suggestion system could be improved — for example, by trying to parse the suggested correction, to see if it uses at least valid context-free syntax. If we had a student who wanted to work on this, it would be a fine, self-contained project. I figure, though, that I have more important things to do with my time.
If these suggestions went away, can you conceive of anyone who would miss them?
kjx
KimBruce
A long time ago, I think as the result of a student project, many of the errors generated by the parser were accompanied by "suggestions" of possible fixes. I propose removing these suggestions.
For example:
That example is moderately helpful, in that the suggestions are at least syntactically correct. (The actual problem was my failure to indent the continuation line). Others are less helpful
and
where the suggestion is syntactic garbage.
In response to issue #243, I have already moved the spelling correction suggestions for undeclared identifiers into the error message, so these would not be affected by this proposal.
Why remove them?
The suggestion code is a mess; it is not factored out of the main parser logic (except in a few cases where I have moved it), so it makes the parser module much longer and less maintainable.
The suggestions are often wrong
Students almost always use the IDE, in which case they never see them
Highlighting the location where the parser detected the error (as with
^^^in the above examples is much more useful; more experienced programmers (i.e., me :-) ) never look at the suggestions.The only one that is useful is
but even here, the error message already says what is necessary.