Merge pull request #2819 from ruby/truncate-errors

Truncate source lines in errors messages when too long
ruby · May 16, 2024 · 6e526a5 · 6e526a5
2 parents 1c8d08b + 72518f5
commit 6e526a5
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 11 deletions.
diff --git a/src/prism.c b/src/prism.c
@@ -21592,6 +21592,7 @@ typedef struct {
 #define PM_COLOR_GRAY "\033[38;5;102m"
 #define PM_COLOR_RED "\033[1;31m"
 #define PM_COLOR_RESET "\033[m"
+#define PM_ERROR_TRUNCATE 30
 
 static inline pm_error_t *
 pm_parser_errors_format_sort(const pm_parser_t *parser, const pm_list_t *error_list, const pm_newline_list_t *newline_list) {
@@ -21646,7 +21647,7 @@ pm_parser_errors_format_sort(const pm_parser_t *parser, const pm_list_t *error_l
 }
 
 static inline void
-pm_parser_errors_format_line(const pm_parser_t *parser, const pm_newline_list_t *newline_list, const char *number_prefix, int32_t line, pm_buffer_t *buffer) {
+pm_parser_errors_format_line(const pm_parser_t *parser, const pm_newline_list_t *newline_list, const char *number_prefix, int32_t line, uint32_t column_start, uint32_t column_end, pm_buffer_t *buffer) {
     int32_t line_delta = line - parser->start_line;
     assert(line_delta >= 0);
 
@@ -21663,9 +21664,25 @@ pm_parser_errors_format_line(const pm_parser_t *parser, const pm_newline_list_t
     }
 
     pm_buffer_append_format(buffer, number_prefix, line);
+
+    // Here we determine if we should truncate the end of the line.
+    bool truncate_end = false;
+    if ((column_end != 0) && ((end - (start + column_end)) >= PM_ERROR_TRUNCATE)) {
+        end = start + column_end + PM_ERROR_TRUNCATE;
+        truncate_end = true;
+    }
+
+    // Here we determine if we should truncate the start of the line.
+    if (column_start >= PM_ERROR_TRUNCATE) {
+        pm_buffer_append_string(buffer, "... ", 4);
+        start += column_start;
+    }
+
     pm_buffer_append_string(buffer, (const char *) start, (size_t) (end - start));
 
-    if (end == parser->end && end[-1] != '\n') {
+    if (truncate_end) {
+        pm_buffer_append_string(buffer, " ...\n", 5);
+    } else if (end == parser->end && end[-1] != '\n') {
         pm_buffer_append_string(buffer, "\n", 1);
     }
 }
@@ -21780,6 +21797,7 @@ pm_parser_errors_format(const pm_parser_t *parser, const pm_list_t *error_list,
     // display the same line twice in case the errors are close enough in the
     // source.
     int32_t last_line = parser->start_line - 1;
+    uint32_t last_column_start = 0;
     const pm_encoding_t *encoding = parser->encoding;
 
     for (size_t index = 0; index < error_list->size; index++) {
@@ -21794,11 +21812,11 @@ pm_parser_errors_format(const pm_parser_t *parser, const pm_list_t *error_list,
                 }
 
                 pm_buffer_append_string(buffer, "  ", 2);
-                pm_parser_errors_format_line(parser, newline_list, error_format.number_prefix, error->line - 2, buffer);
+                pm_parser_errors_format_line(parser, newline_list, error_format.number_prefix, error->line - 2, 0, 0, buffer);
             }
 
             pm_buffer_append_string(buffer, "  ", 2);
-            pm_parser_errors_format_line(parser, newline_list, error_format.number_prefix, error->line - 1, buffer);
+            pm_parser_errors_format_line(parser, newline_list, error_format.number_prefix, error->line - 1, 0, 0, buffer);
         }
 
         // If this is the first error or we're on a new line, then we'll display
@@ -21809,7 +21827,17 @@ pm_parser_errors_format(const pm_parser_t *parser, const pm_list_t *error_list,
             } else {
                 pm_buffer_append_string(buffer, "> ", 2);
             }
-            pm_parser_errors_format_line(parser, newline_list, error_format.number_prefix, error->line, buffer);
+
+            last_column_start = error->column_start;
+
+            // Find the maximum column end of all the errors on this line.
+            uint32_t column_end = error->column_end;
+            for (size_t next_index = index + 1; next_index < error_list->size; next_index++) {
+                if (errors[next_index].line != error->line) break;
+                if (errors[next_index].column_end > column_end) column_end = errors[next_index].column_end;
+            }
+
+            pm_parser_errors_format_line(parser, newline_list, error_format.number_prefix, error->line, error->column_start, column_end, buffer);
         }
 
         const uint8_t *start = &parser->start[newline_list->offsets[error->line - start_line]];
@@ -21828,6 +21856,11 @@ pm_parser_errors_format(const pm_parser_t *parser, const pm_list_t *error_list,
         pm_buffer_append_string(buffer, error_format.blank_prefix, error_format.blank_prefix_length);
 
         size_t column = 0;
+        if (last_column_start >= PM_ERROR_TRUNCATE) {
+            pm_buffer_append_string(buffer, "    ", 4);
+            column = last_column_start;
+        }
+
         while (column < error->column_start) {
             pm_buffer_append_byte(buffer, ' ');
 
@@ -21867,19 +21900,20 @@ pm_parser_errors_format(const pm_parser_t *parser, const pm_list_t *error_list,
 
         if (next_line - last_line > 1) {
             pm_buffer_append_string(buffer, "  ", 2);
-            pm_parser_errors_format_line(parser, newline_list, error_format.number_prefix, ++last_line, buffer);
+            pm_parser_errors_format_line(parser, newline_list, error_format.number_prefix, ++last_line, 0, 0, buffer);
         }
 
         if (next_line - last_line > 1) {
             pm_buffer_append_string(buffer, "  ", 2);
-            pm_parser_errors_format_line(parser, newline_list, error_format.number_prefix, ++last_line, buffer);
+            pm_parser_errors_format_line(parser, newline_list, error_format.number_prefix, ++last_line, 0, 0, buffer);
         }
     }
 
     // Finally, we'll free the array of errors that we allocated.
     xfree(errors);
 }
 
+#undef PM_ERROR_TRUNCATE
 #undef PM_COLOR_GRAY
 #undef PM_COLOR_RED
 #undef PM_COLOR_RESET
diff --git a/test/prism/format_errors_test.rb b/test/prism/format_errors_test.rb
@@ -6,19 +6,53 @@
 
 module Prism
   class FormatErrorsTest < TestCase
-    def test_format_errors
-      assert_equal <<~ERROR, Debug.format_errors("<>", false)
+    def test_basic
+      expected = <<~ERROR
         > 1 | <>
             | ^ unexpected '<', ignoring it
             |  ^ unexpected '>', ignoring it
       ERROR
 
-      assert_equal <<~'ERROR', Debug.format_errors('"%W"\u"', false)
-        > 1 | "%W"\u"
+      assert_equal expected, Debug.format_errors("<>", false)
+    end
+
+    def test_multiple
+      expected = <<~ERROR
+        > 1 | "%W"\\u"
             |     ^ unexpected backslash, ignoring it
             |      ^ unexpected local variable or method, expecting end-of-input
             |        ^ unterminated string meets end of file
       ERROR
+
+      assert_equal expected, Debug.format_errors('"%W"\u"', false)
+    end
+
+    def test_truncate_start
+      expected = <<~ERROR
+        > 1 | ... <>
+            |     ^ unexpected '<', ignoring it
+            |      ^ unexpected '>', ignoring it
+      ERROR
+
+      assert_equal expected, Debug.format_errors("#{" " * 30}<>", false)
+    end
+
+    def test_truncate_end
+      expected = <<~ERROR
+        > 1 | <#{" " * 30} ...
+            | ^ unexpected '<', ignoring it
+      ERROR
+
+      assert_equal expected, Debug.format_errors("<#{" " * 30}a", false)
+    end
+
+    def test_truncate_both
+      expected = <<~ERROR
+        > 1 | ... <#{" " * 30} ...
+            |     ^ unexpected '<', ignoring it
+      ERROR
+
+      assert_equal expected, Debug.format_errors("#{" " * 30}<#{" " * 30}a", false)
     end
   end
 end
diff --git a/test/prism/newline_test.rb b/test/prism/newline_test.rb
@@ -10,6 +10,7 @@ class NewlineTest < TestCase
     filepaths = Dir["*.rb", base: base] - %w[
       encoding_test.rb
       errors_test.rb
+      format_errors_test.rb
       parser_test.rb
       regexp_test.rb
       static_literals_test.rb