Fix: methods cannot be used as constructor #1774
Conversation
|
This makes sense to me, but I wonder how much existing code it could break and whether we should put it behind a "VERSION_ES6" or a "VERSION_ECMASCRIPT" flag just in case -- then again, since it didn't break any of MozillaSuiteTest perhaps it's not a big problem. |
There was a problem hiding this comment.
I'm willing to be convinced on this, but I think that since this breaks things that may have worked before, we should check that language version is >= VERSION_ES6 before throwing this exception. I'm not sure that we will put this in 1.8.0, but can merge it soon after.
The spec says that invoking a method as a constructor is not valid,
but invoking normal functions is. This is, for example, `node`:
```js
> o = { method() {}, notAMethod: function() {} }
{ method: [Function: method], notAMethod: [Function: notAMethod] }
> new o.method()
Uncaught TypeError: o.method is not a constructor
> new o.notAMethod()
notAMethod {}
```
This PR implements the same behavior in Rhino, and fixes mozilla#1299.
andreabergia
force-pushed
the
method-constructor
branch
from
b462d96 to
c80888d
Compare
Rebased onto master and added a check on the language version. |
| @@ -408,6 +408,11 @@ public Object call(Context cx, Scriptable scope, Scriptable thisObj, Object[] ar | |||
|
|
|||
| @Override | |||
| public Scriptable construct(Context cx, Scriptable scope, Object[] args) { | |||
| if (cx.getLanguageVersion() >= Context.VERSION_ES6 && this.getHomeObject() != null) { | |||
There was a problem hiding this comment.
usually we do not use this (only in setters when it is required)
There was a problem hiding this comment.
Oh yeah, make sense. I guess I'm writing too much rust outside work recently :D
|
Looks good to me -- I agree about the excessive use of "this" but I'd rather move forward ;-) Thanks! |
The spec says that invoking a method as a constructor is not valid, but invoking normal functions is. This is, for example,
node:This PR implements the same behavior in Rhino, and fixes #1299.