Feature #21573: Simpler syntax errors

• Status: Open
• Assignee: -
• Target version: -

[kddnewton (Kevin Newton)]

Right now both the prism and parse.y parsers add some context to syntax errors to make it easier to understand where they are coming from. This works well for humans, but is tough for tools (see https://github.com/ruby/prism/issues/3455 for some additional context). I would like to propose a feature that can be enabled via environment variable (or some other mechanism like a configure switch) that makes syntax errors output minimal information (i.e., file:line:col: message).

The diff would be fairly small for prism, below is the entirety of the change:

diff --git a/prism_compile.c b/prism_compile.c
index 578e6f240f..d14c23eb94 100644
--- a/prism_compile.c
+++ b/prism_compile.c
@@ -11018,13 +11018,15 @@ pm_parse_process_error(const pm_parse_result_t *result)
         (parser->error_list.size > 1) ? "s" : ""
     );
 
-    if (valid_utf8) {
+    if (!getenv("SIMPLE_ERRORS") && valid_utf8) {
         pm_parse_errors_format(parser, &parser->error_list, &buffer, highlight, true);
     }
     else {
         for (const pm_diagnostic_t *error = head; error != NULL; error = (const pm_diagnostic_t *) error->node.next) {
             if (error != head) pm_buffer_append_byte(&buffer, '\n');
-            pm_buffer_append_format(&buffer, "%.*s:%" PRIi32 ": %s", (int) pm_string_length(filepath), pm_string_source(filepath), (int32_t) pm_location_line_number(parser, &error->location), error->message);
+
+            pm_line_column_t location = pm_newline_list_line_column(&parser->newline_list, error->location.start, parser->start_line);
+            pm_buffer_append_format(&buffer, "%.*s:%" PRIi32 ":%" PRIu32 ": %s", (int) pm_string_length(filepath), pm_string_source(filepath), location.line, location.column, error->message);
         }
     }

[mame (Yusuke Endoh)]

Before hastily adding modes, we should clarify the problem.

In the proposed patch, the error message will be as follows:

$ echo -e "1 +x 2 do\ndo" | SIMPLE_ERRORS=1 ./miniruby -wc
-:1: warning: '+' after local variable or literal is interpreted as binary operator even though it seems like unary operator
-:1: warning: possibly useless use of + in void context
-:1: warning: possibly useless use of a literal in void context
./miniruby: -:1: syntax errors found (SyntaxError)
-:1: unexpected integer, expecting end-of-input
-:1: unexpected 'do', expecting end-of-input
-:1: unexpected 'do', ignoring it
-:2: unexpected 'do', ignoring it

Will this actually solve the problem?

• The author of the issue states "and counting spaces to ascertain the column)". This suggests a desire to obtain the error column information. The proposed patch does not achieve this goal.
• The author of the issue also states "Ruby can print one syntax error per line". But the proposed patch appears to display -:1: unexpected 'do', expecting end-of-input and -:1: unexpected 'do', ignoring it. Does this achieve the goal?
• The triggering issue was that the interpreter path is displayed since Ruby 3.4. This is simply a Ruby 3.3 incompatibility and should be fixe if possible?

As discussed in the ticket https://github.com/ruby/prism/issues/3455, it seems reasonable for tools like flycheck to switch to Prism.parse_stream(STDIN).errors/warnings.

[kddnewton (Kevin Newton)]

I'm not sure if you ran with my patch enabled, since the patch above explicitly adds column information. When I run it with the patch, I get:

./miniruby: -:1: syntax errors found (SyntaxError)
-:1:5: unexpected integer, expecting end-of-input
-:1:7: unexpected 'do', expecting end-of-input
-:1:7: unexpected 'do', ignoring it
-:2:0: unexpected 'do', ignoring it

To respond to your specific concerns:

• Yes, the desire to obtain error column information is included. The proposed patch does achieve this goal.
• I believe it is one syntax error per line. I understand that there are 3 unexpected 'do', but these are each being interpreted in a different context. I think the error message could be improved, but these are different errors. I believe this does achieve the goal.
• Ruby 3.3 parse.y also changed to include more of the interpreter path, sometimes consolidating multiple errors into a buffer. I can change it to match if you would like, but it is a moving target. My understanding was that the only guarantees on error messages were covered by the tests, which we all pass.

I do think it is reasonable to use Prism directly, but I imagine flycheck isn't the only tool attempting to parse out values from the command line using regexp. I wanted to support that use case (which is detailed in the issue), as opposed to making them invoke Ruby again after the script has already been run (seeing as it might have side effects).