Better error message for modules with lowercase identifiers #2062
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hcarty
reviewed
Jul 9, 2018
src/reason-parser/reason_parser.mly
Outdated
| @@ -1047,6 +1047,9 @@ let record_pat_spread_msg = | |||
| Explanation: you can't collect a subset of a record's field into its own record, since a record needs an explicit declaration and that subset wouldn't have one. | |||
| Solution: you need to pull out each field you want explicitly." | |||
|
|
|||
| let lowercase_module_msg s = | |||
| Printf.sprintf "Module names must be uppercase identifiers. `%s` is not an uppercase identifier." s | |||
There was a problem hiding this comment.
Do we have uppercase identifier defined somewhere? It may be helpful to be explicit in this error message - something along the lines of Module names must start with an uppercase letter. `%s` does not start with an uppercase letter.
EDIT: tweaked wording
|
Yeah no need to say that "foo" isn't uppercased. Thanks! |
|
I'm not sure to what extend we should encode all those error cases into the grammar. There's the cost of tampering with the grammar that might result into shift/reduce conflicts in the future. For example, |
|
Should we revert this one then? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #1974