reasonml / reason

Simple, fast & type safe code that leverages the JavaScript & OCaml ecosystems
http://reasonml.github.io
MIT License
10.12k stars 428 forks source link

Confusing error location with extra closing paren in sequence (False alarm) #2588

Closed jordwalke closed 4 years ago

jordwalke commented 4 years ago

EDIT: Closing as this appears to be fixed and I was just using an old Reason version

In reason 3.6+ I get the error location of the ); which is pretty close to ideal. Greatly improved!

Line 11, characters 6-7:
Error: Unclosed "{" (opened line 1, column 23)

And in recovery mode (what merlin uses in IDE) I get the same:

%merlin.syntax-error
"\027[1mLine 11, characters 6-7\027[0m:\n\027[1;31mError\027[0m: Unclosed \"{\" (opened line 1, column 23)\n\n";

Original message below

An extra closing parenthesis leads to an error that has less precise location information than possible - in version Reason 3.5.2 @ e6229f9a

  let name = (t: t) => {
    let andThen =
      return(
       Mod.getThing(
          t.mv,
        ),
     );
   (campaign, cb) =>
     andThen(fn =>
       cb(Mod.setName(fn, camp)))
     );
 };

output:

# File "product-hack-only/data-model-query/AdsQuery.re", line 8, characters 4-205:
# Error: SyntaxError in block

Line 8 is the let andThen line and the error is produced in non-recovery mode (which in theory should give more precise location information).

The problem is the extra closing paren on the line cb(Mod.setName(friendlyName, campaign)))

This could probably be detected in the sequence parsing where we see the ); - a free floating closing paren in a sequence can probably easily have its own specific error message.

In recovery mode, the error location is omitted entirely. This is the only case where I can find this happening so far with the latest menhir recovery:

%merlin.syntax-error
"A syntax error occurred. Help us improve this message: https://github.com/facebook/reason/blob/master/src/README.md#add-a-menhir-error-message";