Reflect: Implement Reflect.ownKeys()

[duonglaiquang]

Overview

This PR does the following:

• Proposes an implementation for Reflect.ownKeys()

• WARNING: DO NOT MERGE. DOES NOT COMPILE. See following point.

• For this implementation to compile, Rhino's ScriptableObject.java needs a new method (e.g. getAllIdsIncludingSymbols()) that allows retrieval of all properties (enumerable or not not) including both strings and symbols:

  public Object[] getAllIdsIncludingSymbols() {
      return getIds(true, true);
  }

Test case

(This test case is also implemented in ReflectTest.java.)

var obj = {
  [Symbol.for('foo')]: 0,
  "str": 0,
  773: 0,
  "55": 0,
  0: 0,
  "-1": 0,
  8: 0,
  "6": 8,
  [Symbol.for('bar')]: 0,
  "str2": 0,
};

// Chrome:  [ "0", "6", "8", "55", "773", "str", "-1", "str2", Symbol("foo"), Symbol("bar") ]
// HtmlUnit: [ "-1", "0", "6", "8", "55", "773", "str", "str2", "Symbol(foo)", "Symbol(bar)" ]
console.log(Reflect.ownKeys(obj));

Points of note

About the javadoc

The javadoc for ownKeys() is very basic because:

• We aren't sure if licensing allows us to copy mdn docs' words or whether we need to reword to our own words
• Perhaps writing fully descriptive API docs in the javadoc of these methods is not all that useful since they're JS methods, and a basic javadoc is easier to produce

Regarding the ordering of property keys returned

Notice -1 is in the wrong place.:

• Chrome: [ "0", "6", "8", "55", "773", "str", "-1", "str2", Symbol("foo"), Symbol("bar") ]
• HtmlUnit: [ "-1", "0", "6", "8", "55", "773", "str", "str2", "Symbol(foo)", "Symbol(bar)" ]

The specs for Reflect.ownKeys() provide clear description for the ordering of these:

1. Non-negative integer indexes in increasing numeric order (but as strings)
2. Other string keys in the order of property creation
3. Symbol keys in the order of property creation.

We cannot accommodate this for -1 and other negative integers because Rhino does not preserve the property creation order of negative integer keys. This appears to be a bug in Rhino:

• Rhino DOES preserve the property creation order of non-integer keys adhering to specs
• Rhino DOES orders "positive integer" keys in canonical order adhering to specs
• Rhino DOES NOT preserve the property creation order of "negative integer" since it seems to be incorrectly treating these as an integer index even though they're not in the defined range of 0 <= i <= F [1]

[1] https://262.ecma-international.org/13.0/#sec-object-type

Regarding the initialization of the test case

var obj = {
  [Symbol.for('foo')]: 0,
  "str": 0,
  773: 0,
  ...
};

HtmlUnit throws invalid property id error on the [Symbol.for('foo')] part which is a computed property name. The object is instead initialized in ReflectTest.java as such:

var obj = {};
obj[Symbol.for('foo')] = 0;
obj['str'] = 0;
obj[773] = 0;
obj["55"] = 0;
obj[0] = 0;
obj['-1'] = 0;
obj[8] = 0;
obj["6"] = 0;
obj[Symbol.for('bar')] = 0;
obj['str2'] = 0;

[rbri]

Hi @duonglaiquang,
i guess this requires some time. Maybe i can move the whole Reflect impl to Rhino....
Lets see.

[rbri]

Looks like i can do a (more or less complete) impl in Rhino... hopefully i can finish it over the weekend.

[atnak]

fyi, we're looking into an issue where Reflect.ownKeys.toString() throws TypeError: Cannot find default value for object and suspect there might need to be something added to FunctionObject. Perhaps the situation will change when the implementation is moved to Rhino.

[rbri]

Will add at least your testcase to the HtmlUnit test suite after the Impl is in Rhino/core-js. Hope then we are on the save side

[rbri]

Step 1: mozilla/rhino#1324

[rbri]

Reflect impl is now in core-js (will be moved later to Rhino)

• construct still missing
• negative index values handling still open

Next step: provide PR for Rhino for negative index handling (have already some working code)

[rbri]

HtmlUnit/htmlunit-rhino-fork@e6c4c79

Because i fear there is no chance for this in Rhino (is a bit of a hack and not backward compatible) i did it only here