mozilla / rhino

Rhino is an open-source implementation of JavaScript written entirely in Java
https://rhino.github.io
Other
4.11k stars 848 forks source link

Support ES2019 Function.toString() revision #1300

Open p-bakker opened 1 year ago

p-bakker commented 1 year ago

See:

Some examples: Currently:

=> Math.pow
function pow() { [native code for Math.pow, arity=2] }

Expected:

=> Math.pow.toString()
function pow() { [native code] }

Currently:

=> (function(){}).bind({}).toString()
function () {
    [native code, arity=0]
}

Expected:

=> (function(){}).bind({}).toString()
function () { [native code] }

The spec revision also details that the .toString() of user-defined function inside JavaScript must return the source exactly as authored, which I'm not sure we're compliant with

p-bakker commented 1 year ago

The part about .toString() returning the source code as authored might be fixed by #1188

tuchida commented 1 month ago

It would be possible to handle this by removing the code that adds arity= to the result of Function.prototype.toString. However, since #939(#326) is not implemented, the harness for test262 does not work.

// nativeFunctionMatcher.js
  const eatWhitespace = () => {
    while (pos < source.length) {
      const c = source[pos]; // `c` does not change. There are three similar codes.
      if (isWhitespace(c) || isNewline(c)) {
        pos += 1;
        continue;
      }

This works by replacing const with let.

Besides native code, getter/setter test was failing.

p-bakker commented 1 month ago

But besides the tests for this in test262 failing, code in Rhino needs to be fixed still to make it functionally work?

tuchida commented 1 month ago

As for arity=, the following codes needs to be removed. https://github.com/mozilla/rhino/blob/955af1565bd440b4954a7cbd3db4278b669302e7/rhino/src/main/java/org/mozilla/javascript/BaseFunction.java#L464-L466 https://github.com/mozilla/rhino/blob/955af1565bd440b4954a7cbd3db4278b669302e7/rhino/src/main/java/org/mozilla/javascript/IdFunctionObject.java#L115-L117

I have not looked too closely at the getter/setter.

tuchida commented 1 month ago

This issue is not yet fixed.

p-bakker commented 1 month ago

To clarify: #1520 fixed the .toString() behaviour on user-authored JavaScript functions, but not yet on native (Java) methods

tuchida commented 1 month ago

What should the Java method be?

https://262.ecma-international.org/15.0/index.html#sec-function.prototype.tostring

  1. If func is an Object and IsCallable(func) is true, return an implementation-defined String source code representation of func. The representation must have the syntax of a NativeFunction.

I think this sentence expects [native code]. Now Rhino returns a string like this.

js> java.math.BigInteger.valueOf.toString()
function valueOf() {/*
java.math.BigInteger valueOf(long)
*/}

In GraalJS, this is what returned.

> Java.type('java.math.BigInteger').valueOf.toString()
function valueOf() { [native code] }
p-bakker commented 1 month ago

[native code] is correct afaik

rbri commented 1 month ago

switching back to only [native code] will be great - have done some adjustments for the change in HtmlUnit to be browser compatible (https://github.com/HtmlUnit/htmlunit/commit/0c18c7709e83d8147117b5b459c92139e207bbdb)

Hopefully i can remove more code and only insert the line break that ff has in the output...