Merge pull request #61 from tree-sitter/error-recovery-improvements

More error recovery improvements

project.gyp

@@ -106,6 +106,7 @@
       ],
       'sources': [
         'src/runtime/document.c',
+        'src/runtime/error_costs.c',
         'src/runtime/language.c',
         'src/runtime/lexer.c',
         'src/runtime/node.c',

spec/fixtures/error_corpus/c_errors.txt

@@ -23,20 +23,18 @@ int c;
 ---
 
 (translation_unit
-  (preproc_ifdef
-    (identifier)
-    (ERROR (identifier) (identifier) (comment)))
+  (preproc_ifdef (identifier)
+    (ERROR (identifier) (identifier))
+    (comment))
 
   (declaration (identifier) (identifier))
 
-  (preproc_ifdef
-    (identifier)
+  (preproc_ifdef (identifier)
     (ERROR (storage_class_specifier) (string_literal)))
 
   (declaration (identifier) (identifier))
 
-  (preproc_ifdef
-    (identifier)
+  (preproc_ifdef (identifier)
     (ERROR))
 
   (declaration (identifier) (identifier)))

spec/runtime/parser_spec.cc

@@ -4,6 +4,7 @@
 #include "helpers/spy_input.h"
 #include "helpers/load_language.h"
 #include "helpers/record_alloc.h"
+#include "helpers/point_helpers.h"
 #include "helpers/stderr_logger.h"
 #include "helpers/dedent.h"
 
@@ -168,6 +169,24 @@ describe("Parser", [&]() {
           "(ERROR (program (expression_statement (identifier))) (UNEXPECTED EOF))");
       });
     });
+
+    describe("when there are extra tokens at the end of the viable prefix", [&]() {
+      it("does not include them in the error node", [&]() {
+        ts_document_set_language(document, get_test_language("javascript"));
+        set_text(
+          "var x;\n"
+          "\n"
+          "if\n"
+          "\n"
+          "var y;"
+        );
+
+        TSNode error = ts_node_named_child(root, 1);
+        AssertThat(ts_node_type(error, document), Equals("ERROR"));
+        AssertThat(ts_node_start_point(error), Equals<TSPoint>({2, 0}));
+        AssertThat(ts_node_end_point(error), Equals<TSPoint>({2, 2}));
+      });
+    });
   });
 
   describe("handling extra tokens", [&]() {

src/runtime/array.h

@@ -46,6 +46,9 @@ extern "C" {
   (array_grow((self), (self)->size + 1), \
    (self)->contents[(self)->size++] = (element))
 
+#define array_push_all(self, other)                                       \
+  array_splice((self), (self)->size, 0, (other)->size, (other)->contents)
+
 #define array_splice(self, index, old_count, new_count, new_elements)          \
   array__splice((VoidArray *)(self), array__elem_size(self), index, old_count, \
                 new_count, (new_elements))

src/runtime/error_costs.c

@@ -0,0 +1,28 @@
+#include "runtime/error_costs.h"
+#include <math.h>
+
+static inline unsigned error_status__min_cost(ErrorStatus status) {
+  return status.cost + ERROR_COST_PER_SKIPPED_TREE * status.count * status.count;
+}
+
+static inline unsigned error_status__max_cost(ErrorStatus status) {
+  return status.cost +
+         ERROR_COST_PER_SKIPPED_TREE * status.count * status.count +
+         (6 * ERROR_COST_PER_SKIPPED_TREE * status.count +
+          12 * ERROR_COST_PER_SKIPPED_TREE) /
+           (1 + status.push_count / 2);
+}
+
+int error_status_compare(ErrorStatus a, ErrorStatus b) {
+  if ((a.count + 1 < b.count) || (a.count < b.count && a.cost <= b.cost))
+    return -1;
+  if ((a.count > b.count + 1) || (b.count < a.count && b.cost <= a.cost))
+    return 1;
+
+  if (error_status__max_cost(a) < error_status__min_cost(b))
+    return -1;
+  if (error_status__min_cost(a) > error_status__max_cost(b))
+    return 1;
+
+  return 0;
+}

src/runtime/error_costs.h

@@ -1,41 +1,25 @@
 #ifndef RUNTIME_ERROR_COSTS_H_
 #define RUNTIME_ERROR_COSTS_H_
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 #define ERROR_STATE 0
 #define ERROR_COST_PER_SKIPPED_TREE 10
 #define ERROR_COST_PER_SKIPPED_LINE 3
 #define ERROR_COST_PER_SKIPPED_CHAR 0
 
 typedef struct {
-  unsigned cost;
   unsigned count;
+  unsigned cost;
   unsigned push_count;
 } ErrorStatus;
 
-static inline unsigned error_status_min_cost(ErrorStatus status) {
-  return status.cost + ERROR_COST_PER_SKIPPED_TREE * status.count * status.count;
-}
-
-static inline unsigned error_status_max_cost(ErrorStatus status) {
-  return status.cost +
-         ERROR_COST_PER_SKIPPED_TREE * status.count * status.count +
-         (6 * ERROR_COST_PER_SKIPPED_TREE * status.count +
-          12 * ERROR_COST_PER_SKIPPED_TREE) /
-           (1 + status.push_count / 2);
-}
-
-static inline int error_status_compare(ErrorStatus a, ErrorStatus b) {
-  if ((a.count + 1 < b.count) || (a.count < b.count && a.cost <= b.cost))
-    return -1;
-  if ((a.count > b.count + 1) || (b.count < a.count && b.cost <= a.cost))
-    return 1;
-
-  if (error_status_max_cost(a) < error_status_min_cost(b))
-    return -1;
-  if (error_status_min_cost(a) > error_status_max_cost(b))
-    return 1;
-
-  return 0;
+int error_status_compare(ErrorStatus a, ErrorStatus b);
+
+#ifdef __cplusplus
 }
+#endif
 
 #endif

src/runtime/language.c

@@ -2,8 +2,8 @@
 #include "runtime/tree.h"
 #include "runtime/error_costs.h"
 
-static const TSParseAction ERROR_SHIFT_EXTRA = {
-  .type = TSParseActionTypeShift, .extra = true,
+static const TSParseAction SHIFT_ERROR = {
+  .type = TSParseActionTypeShift, .params = {.to_state = ERROR_STATE}
 };
 
 void ts_language_table_entry(const TSLanguage *self, TSStateId state,
@@ -14,7 +14,7 @@ void ts_language_table_entry(const TSLanguage *self, TSStateId state,
       result->action_count = 1;
       result->is_reusable = false;
       result->depends_on_lookahead = false;
-      result->actions = &ERROR_SHIFT_EXTRA;
+      result->actions = &SHIFT_ERROR;
       return;
     }
     action_index = 0;

src/runtime/parser.c

@@ -755,8 +755,12 @@ static bool parser__repair_error(Parser *self, StackSlice slice,
   }
 
   TreeArray skipped_children = ts_tree_array_remove_last_n(&children, skip_count);
+  TreeArray trailing_extras = ts_tree_array_remove_trailing_extras(&skipped_children);
   Tree *error = ts_tree_make_error_node(&skipped_children);
   array_push(&children, error);
+  array_push_all(&children, &trailing_extras);
+  trailing_extras.size = 0;
+  array_delete(&trailing_extras);
 
   for (uint32_t i = 0; i < slice.trees.size; i++)
     array_push(&children, slice.trees.contents[i]);

src/runtime/tree.c

@@ -84,6 +84,21 @@ TreeArray ts_tree_array_remove_last_n(TreeArray *self, uint32_t remove_count) {
   return result;
 }
 
+TreeArray ts_tree_array_remove_trailing_extras(TreeArray *self) {
+  TreeArray result = array_new();
+
+  uint32_t i = self->size - 1;
+  for (; i + 1 > 0; i--) {
+    Tree *child = self->contents[i];
+    if (!child->extra) break;
+    array_push(&result, child);
+  }
+
+  self->size = i + 1;
+  array_reverse(&result);
+  return result;
+}
+
 Tree *ts_tree_make_error(Length size, Length padding, char lookahead_char) {
   Tree *result = ts_tree_make_leaf(ts_builtin_sym_error, padding, size,
                                      (TSSymbolMetadata){

src/runtime/tree.h

@@ -69,6 +69,7 @@ bool ts_tree_array_copy(TreeArray, TreeArray *);
 void ts_tree_array_delete(TreeArray *);
 uint32_t ts_tree_array_essential_count(const TreeArray *);
 TreeArray ts_tree_array_remove_last_n(TreeArray *, uint32_t);
+TreeArray ts_tree_array_remove_trailing_extras(TreeArray *);
 
 Tree *ts_tree_make_leaf(TSSymbol, Length, Length, TSSymbolMetadata);
 Tree *ts_tree_make_node(TSSymbol, uint32_t, Tree **, TSSymbolMetadata);