Skip to content
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

test: syntax error if superproperty private name access #1848

Merged

Conversation

jbhoosreddy
Copy link
Contributor

Tracking issues #1343

Summary of changes:

  1. Add test for super property private name early syntax error

@jbhoosreddy
Copy link
Contributor Author

@leobalter @rwaldron @littledan @mkubilayk for review

@jbhoosreddy
Copy link
Contributor Author

@leobalter @rwaldron for review?

@leobalter
Copy link
Member

Rick is off this week due to family obligations. I don't have any time assigned this week to review PRs and I'm pretty busy with other things. We should be able to review this one next week.

@rwaldron
Copy link
Contributor

When I generate these tests, I get the following:

throw "Test262: This statement should not be evaluated.";

class C extends B
{
  method() {
  super.#x();
  }
}

Which will pass by throwing a SyntaxError exception, but it's a false positive:

Uncaught SyntaxError: Invalid private field '#x'

The reason I believe this is a false positive is due this being the same exception that occurs for this:

class C extends B
{
  method() {
  this.#x();
  }
}
Uncaught SyntaxError: Invalid private field '#x'

But then I realized that the case doesn't have any label on this:

class B {
#x() {}
}

So it's not being included.

@rwaldron
Copy link
Contributor

I actually don't think this:

class B {
#x() {}
}

...is even necessary. Since the test is just "expect super.#x() to result in a SyntaxError", there doesn't even need to be a class declaration for B.

I no longer believe that the failure is a false positive.

@rwaldron rwaldron merged commit 7736c00 into tc39:master Oct 15, 2018
@jbhoosreddy
Copy link
Contributor Author

@rwaldron based on your last comment is there something I needed to fix in my PR?

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

Successfully merging this pull request may close these issues.

4 participants