-
-
Notifications
You must be signed in to change notification settings - Fork 400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Throw a type error when a non-object is called #561
Conversation
…ther than just objects.
Codecov Report
@@ Coverage Diff @@
## master #561 +/- ##
==========================================
+ Coverage 68.19% 69.30% +1.10%
==========================================
Files 172 172
Lines 10573 10580 +7
==========================================
+ Hits 7210 7332 +122
+ Misses 3363 3248 -115
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! could we have a test for this, maybe something like:
let x = false;
try {
undefined();
} catch(e) {
x = true;
}
and we assert that x == true
Yes, I'll do that. I was wondering what the best way to test exceptions would be. |
I don't like this. I've been thinking about that when I had the same "how to test the exception issue. I concluded that it is much better to do the following: try {
//expression that throws the error here;
} catch(e) {
e.message;
} and then assert that the output of this script is the correct string (the error message). |
That's a good point, and is actually what I ended up doing with the tests as I saw that had been used in the tests file already. |
try {
//expression that throws the error here;
} catch(e) {
e.message;
} This gives us Yes. this this is a step in the right direction, but we still aren't testing what type of error it is, which is the most important part of testing, I think we can improve this by: try {
//expression that throws the error here;
} catch(e) {
e.toString();
} This gives use What do you think?
Maybe this could be done by keeping a span of the source code for function call node and when we error we concat the source span and the message description |
That's a good point. I'll make a seperate PR for that because there are already a few tests that use the previous method, and this one has already been merged. |
This Pull Request fixes #510.
Previously if a value other than an object was called as a function undefined would be returned. Now throws a type error.