Add more static class property ordering tests #3013
Conversation
|
XS and engine262 pass both tests. ChakraCore, GraalJS, JSC, SpiderMonkey, and V8 fail. Edit: V8 passes after https://chromium-review.googlesource.com/c/v8/v8/+/2974772 |
TimothyGu
force-pushed
the
name-length-order
branch
from
c04236a
to
a5a55cc
Compare
| $ERROR('Static method should not be executed during definition'); | ||
| } | ||
| static length() { | ||
| $ERROR('Static method should not be executed during definition'); |
There was a problem hiding this comment.
Please don't use $ERROR like this. throw new Test262Error(...) (This is clearly stated in the contributing docs https://github.com/tc39/test262/blob/main/CONTRIBUTING.md#test-environment)
There was a problem hiding this comment.
This is primarily copied from the existing fn-name-static-precedence.js test, which explains the existence of $ERROR. I can fix it up.
There was a problem hiding this comment.
I appreciate the follow up. I just finished eliminating $ERROR from every test file in the project 🤣
|
FWIW, I've updated the linter to check for $ERROR uses. |
|
@TimothyGu please @ mention me when you're ready for another review. |
Since tc39/ecma262#1490, the "length" and "name" properties of a class are defined before any static methods. This is tested by tc39#2057, in test/language/computed-property-names of all places. At the same time, static methods with "name" as the name would overwrite the original property, but retain the original property enumeration order. This was not previously tested. In fact, the overwriting behavior was not tested at all for the "length" property. This commit mends both holes in test coverage.
TimothyGu
force-pushed
the
name-length-order
branch
from
a5a55cc
to
4faf8d2
Compare
|
@rwaldron This is ready for another look. |
Since tc39/ecma262#1490, the "length" and "name" properties of a class are defined before any static methods. This is tested by #2057, in test/language/computed-property-names of all places.
At the same time, static methods with "name" as the name would overwrite the original property, but retain the original property enumeration order. This was not previously tested. In fact, the overwriting behavior was not tested at all for the "length" property.
This commit mends both holes in test coverage.