Skip to content
This repository has been archived by the owner on May 19, 2018. It is now read-only.

Only allow declares inside declare module #73

Merged
merged 2 commits into from
Jul 29, 2016
Merged

Only allow declares inside declare module #73

merged 2 commits into from
Jul 29, 2016

Conversation

danez
Copy link
Member

@danez danez commented Jul 10, 2016

This is the followup of #71

This now checks for the term declare being used inside declare module. Previously everything was allowed instead of declare. https://astexplorer.net/#/Y6hVjJkQ2m

@@ -0,0 +1,3 @@
{
"throws": "Unexpected token (2:2)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to have a better error message here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like: "Only declare statements are allowed inside declare module {}"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah something like that. Could even give an example (we haven't really done that)

I've been liking elm's errors/messages http://elm-lang.org/blog/compilers-as-assistants

@codecov-io
Copy link

codecov-io commented Jul 28, 2016

Current coverage is 96.91% (diff: 100%)

Merging #73 into master will not change coverage

@@             master        #73   diff @@
==========================================
  Files            19         19          
  Lines          2922       2922          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           2832       2832          
  Misses           90         90          
  Partials          0          0          

Powered by Codecov. Last update f576865...3db8b0c

@danez
Copy link
Member Author

danez commented Jul 28, 2016

I changed the error message to be a little bit nicer.

pp.unexpected = function (pos) {
this.raise(pos != null ? pos : this.state.start, "Unexpected token");
pp.unexpected = function (pos, message = "Unexpected token") {
this.raise(pos != null ? pos : this.state.start, message);
Copy link
Member

@hzoo hzoo Jul 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I like being able to change the error message. Unexpected token is not fun to see (my example is function a() { await b; }) without async in front of function

@hzoo
Copy link
Member

hzoo commented Jul 28, 2016

k 👍 , @jeffmo

@gabelevi
Copy link
Contributor

👍 this looks correct to me. Everything inside of a declare module declaration does start with declare.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants