Skip to content

Commit

Permalink
Fix: methods cannot be used as constructor
Browse files Browse the repository at this point in the history
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 #1299.
  • Loading branch information
andreabergia authored and gbrail committed Jan 9, 2025
1 parent 3b3b5e2 commit bebc345
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 7 deletions.
5 changes: 5 additions & 0 deletions rhino/src/main/java/org/mozilla/javascript/BaseFunction.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
// Only methods have home objects associated with them
throw ScriptRuntime.typeErrorById("msg.not.ctor", getFunctionName());
}

Scriptable result = createObject(cx, scope);
if (result != null) {
Object val = call(cx, scope, result, args);
Expand Down
8 changes: 8 additions & 0 deletions rhino/src/main/java/org/mozilla/javascript/Interpreter.java
Original file line number Diff line number Diff line change
Expand Up @@ -2006,6 +2006,14 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl
if (lhs instanceof InterpretedFunction) {
InterpretedFunction f = (InterpretedFunction) lhs;
if (frame.fnOrScript.securityDomain == f.securityDomain) {
if (cx.getLanguageVersion() >= Context.VERSION_ES6
&& f.getHomeObject() != null) {
// Only methods have home objects associated with
// them
throw ScriptRuntime.typeErrorById(
"msg.not.ctor", f.getFunctionName());
}

Scriptable newInstance =
f.createObject(cx, frame.scope);
CallFrame calleeFrame =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@

package org.mozilla.javascript.tests.harmony;

import org.mozilla.javascript.Context;
import org.mozilla.javascript.drivers.LanguageVersion;
import org.mozilla.javascript.drivers.RhinoTest;
import org.mozilla.javascript.drivers.ScriptTestsBase;

@RhinoTest("testsrc/jstests/harmony/method-definition.js")
@LanguageVersion(Context.VERSION_ES6)
public class MethodDefinitionTest extends ScriptTestsBase {}
5 changes: 4 additions & 1 deletion tests/testsrc/jstests/harmony/method-definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ assertEquals(123, {
}.abc());

// Method is the kind of function, that is non-constructor.
// assertThrows('new (({ a() {} }).a)', TypeError);
assertThrows(function() {
new (({ a() {} }).a)
}, TypeError);
assertNotNull(new (({ notAMethod: function() {} }).notAMethod));

var desc = Object.getOwnPropertyDescriptor({
a() {
Expand Down
10 changes: 4 additions & 6 deletions tests/testsrc/test262.properties
Original file line number Diff line number Diff line change
Expand Up @@ -957,7 +957,7 @@ built-ins/Number 23/335 (6.87%)
S9.3.1_A3_T1_U180E.js {unsupported: [u180e]}
S9.3.1_A3_T2_U180E.js {unsupported: [u180e]}

built-ins/Object 178/3408 (5.22%)
built-ins/Object 177/3408 (5.19%)
assign/assignment-to-readonly-property-of-target-must-throw-a-typeerror-exception.js
assign/not-a-constructor.js
assign/source-own-prop-error.js
Expand Down Expand Up @@ -1530,7 +1530,7 @@ built-ins/Promise 392/631 (62.12%)
resolve-thenable-deferred.js {unsupported: [async]}
resolve-thenable-immed.js {unsupported: [async]}

built-ins/Proxy 76/311 (24.44%)
built-ins/Proxy 73/311 (23.47%)
construct/arguments-realm.js
construct/call-parameters.js
construct/call-parameters-new-target.js
Expand Down Expand Up @@ -3146,7 +3146,7 @@ built-ins/undefined 0/8 (0.0%)

~intl402

language/arguments-object 185/263 (70.34%)
language/arguments-object 184/263 (69.96%)
mapped/mapped-arguments-nonconfigurable-3.js non-strict
mapped/mapped-arguments-nonconfigurable-delete-1.js non-strict
mapped/mapped-arguments-nonconfigurable-delete-2.js non-strict
Expand Down Expand Up @@ -4852,7 +4852,7 @@ language/expressions/new 41/59 (69.49%)

~language/expressions/new.target

language/expressions/object 719/1169 (61.51%)
language/expressions/object 717/1169 (61.33%)
dstr/async-gen-meth-ary-init-iter-close.js {unsupported: [async-iteration, async]}
dstr/async-gen-meth-ary-init-iter-get-err.js {unsupported: [async-iteration]}
dstr/async-gen-meth-ary-init-iter-get-err-array-prototype.js {unsupported: [async-iteration]}
Expand Down Expand Up @@ -5456,7 +5456,6 @@ language/expressions/object 719/1169 (61.51%)
method-definition/gen-yield-spread-arr-multiple.js
method-definition/gen-yield-spread-arr-single.js
method-definition/gen-yield-spread-obj.js
method-definition/generator-invoke-ctor.js
method-definition/generator-invoke-fn-strict.js non-strict
method-definition/generator-length-dflt.js
method-definition/generator-name-prop-string.js
Expand All @@ -5475,7 +5474,6 @@ language/expressions/object 719/1169 (61.51%)
method-definition/meth-eval-var-scope-syntax-err.js non-strict
method-definition/meth-object-destructuring-param-strict-body.js
method-definition/meth-rest-param-strict-body.js
method-definition/name-invoke-ctor.js
method-definition/name-invoke-fn-strict.js non-strict
method-definition/name-length-dflt.js
method-definition/name-name-prop-string.js
Expand Down

0 comments on commit bebc345

Please sign in to comment.