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
added check to disallow super.private variable access and test case added #10472
added check to disallow super.private variable access and test case added #10472
Conversation
vivek12345
commented
Sep 20, 2019
•
edited by nicolo-ribaudo
edited by nicolo-ribaudo
Q | A |
---|---|
Fixed Issues? | Fixes #10470 |
Patch: Bug Fix? | Yes |
Major: Breaking Change? | No |
Minor: New Feature? | No |
Tests Added + Pass? | Yes |
Documentation PR Link | |
Any Dependency Changes? | No |
License | MIT |
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/11629/ |
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.
Travis CI is failing, but for a good reason. We test @babel/parser
with Test262 (the official ECMAScript tests) and with the official flow tests.
Since we aren't 100% spec compliant yet, we have a whitelist file of tests which are allowed to fail.
This PR made some tests which were previously failing pass, so they should be removed from the whitelist.
You can run make bootstrap-flow
and make bootstrap-test262
to download the test suites, and make test-flow-update-whitelist
and make test-test262-update-whitelist
to update them.
) { | ||
this.raise( | ||
startPos, | ||
"super is not allowed to be called on a private identifier of a class", |
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.
Well, in this case super
is not being called (it's not super(#x)
).
What about Private fields can't be accessed on super?
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.
Yes that message makes more sense. I will make the changes accordingly and also update the tests. Thank you
@nicolo-ribaudo when I am doing
I get the following error:
Any help here? |
Could you try running |
That did the trick. Thank you 👍 |
…ses from whitelist of flow and test 262 suite
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.
Thanks!