Bug 142840

Summary: ES6 Classes: Extends should accept an expression without parenthesis
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: JavaScriptCoreAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, joepeck, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 140491    
Attachments:
Description Flags
[PATCH] Proposed Fix
none
[PATCH] Proposed Fix rniwa: review+

Description Joseph Pecoraro 2015-03-18 14:38:57 PDT
* TEST:
var namespace = {};
namespace.A = class A { constructor() { console.log("a"); } };
namespace.B = class B extends namespace.A { constructor() { super(); console.log("b"); } };
new namespace.B();

* EXPECTED
Logs for "a" and "b"

* ACTUAL
SyntaxError: Unexpected token '.'. Expected opening '{' at the start of a class body.

* NOTES
- Workaround: "class B extends (namespace.A) { ... }"
- Works in Chrome 43. I don't know how to test Firefox with Classes.
Comment 1 Joseph Pecoraro 2015-03-18 14:40:33 PDT
I'll take a quick look. The spec says AssignmentExpression and the Parser is doing a PrimaryExpression.
Comment 2 Joseph Pecoraro 2015-03-18 14:57:32 PDT
Actually, this is a LeftHandSideExpression, according to the latest spec:

  ClassHeritage[Yield] :
    extends LeftHandSideExpression[?Yield]
Comment 3 Joseph Pecoraro 2015-03-18 15:21:38 PDT
Created attachment 248972 [details]
[PATCH] Proposed Fix

Reference:
https://people.mozilla.org/~jorendorff/es6-draft.html#sec-class-definitions
Comment 4 Joseph Pecoraro 2015-03-18 15:22:38 PDT
Comment on attachment 248972 [details]
[PATCH] Proposed Fix

Oops, I forgot to include expected results.
Comment 5 Joseph Pecoraro 2015-03-18 15:24:36 PDT
Created attachment 248973 [details]
[PATCH] Proposed Fix
Comment 6 Ryosuke Niwa 2015-03-18 15:59:36 PDT
Comment on attachment 248973 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=248973&action=review

> LayoutTests/js/script-tests/class-syntax-extends.js:42
> +shouldThrow('x = 1; c = class extends ++x { };');

Could you add an empty constructor here and the rest of test cases so that this test doesn't depend on the support for default constructor?
Comment 7 Joseph Pecoraro 2015-03-18 20:29:00 PDT
(In reply to comment #6)
> Comment on attachment 248973 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248973&action=review
> 
> > LayoutTests/js/script-tests/class-syntax-extends.js:42
> > +shouldThrow('x = 1; c = class extends ++x { };');
> 
> Could you add an empty constructor here and the rest of test cases so that
> this test doesn't depend on the support for default constructor?

I did this for all the tests, and added a few that had the default constructor as well, to test all cases.
Comment 8 Joseph Pecoraro 2015-03-18 20:29:07 PDT
http://trac.webkit.org/changeset/181724