syntax icon indicating copy to clipboard operation
syntax copied to clipboard

Provide specific error for uppercase typedef

Open jchavarri opened this issue 5 years ago • 2 comments

Fixes #122. It works, but it is probably still far to be the right fix 😅 Any feedback is appreciated.

jchavarri avatar Sep 05 '20 23:09 jchavarri

Looking into this PR now. I'm thinking about ways to handle this in a more generic way.

IwanKaramazow avatar Sep 26 '20 15:09 IwanKaramazow

After having given it some thought, this looks like the correct solution. I was thinking that we should solve it in a more “generic” way, but that turned out to be magic. One of the goals of the new syntax is to keep the implementation of the parser as simple as possible. Your intuition was good. Thanks for helping out here.

A couple of suggestions:

  1. Can we change the parameter ?customError into ~msg? It would be good if the caller of parseValuePath needed to pass an error message. This way we “force” good error messages everywhere =D Can we pass an appropriate error message everywhere? Should this just be a simple string?
  2. Would it be possible to add test cases for every possible situation where parseValuePath is called with an uppercased ident? We can dump them all in one file tests/parsing/errors/other/ if you want.
  3. Unrelated, but should we rename let rec aux to let rec parseValuePathRest or let rec parseValuePathTail to make it more clear inside parseValuePath? What do you think?

IwanKaramazow avatar Sep 29 '20 06:09 IwanKaramazow

The rescript-lang/syntax repo is obsolete and will be archived soon. If this issue is still relevant, please reopen in the compiler repo (https://github.com/rescript-lang/rescript-compiler) or comment here to ask for it to be moved. Thank you for your contributions.

stale[bot] avatar May 28 '23 08:05 stale[bot]