Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for Stage 3 Decorator Metadata #3971

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions features.txt
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ regexp-v-flag
# https://github.com/tc39/proposal-decorators
decorators

# Decorator Metadata
# https://github.com/tc39/proposal-decorator-metadata
decorator-metadata

# Duplicate named capturing groups
# https://github.com/tc39/proposal-duplicate-named-capturing-groups
regexp-duplicate-named-groups
Expand Down
19 changes: 19 additions & 0 deletions test/built-ins/Function/prototype/Symbol.metadata.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright (C) 2023 Ron Buckton. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
esid: sec-function.prototype-@@metadata
description: Function.prototype[Symbol.metadata] property descriptor
info: |
The initial value of the @@metadata property is null.
This property has the attributes { [[Writable]]: false, [[Enumerable]]:
false, [[Configurable]]: false }.
includes: [propertyHelper.js]
features: [decorator-metadata]
---*/

verifyProperty(Function.prototype, Symbol.metadata, {
value: null,
writable: false,
enumerable: false,
configurable: false
});
14 changes: 14 additions & 0 deletions test/built-ins/Symbol/metadata/cross-realm.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright (C) 2023 Ron Buckton. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
esid: sec-symbol.metadata
description: Value shared by all realms
info: |
Unless otherwise specified, well-known symbols values are shared by all
realms.
features: [cross-realm, decorator-metadata]
---*/

var OSymbol = $262.createRealm().global.Symbol;

assert.sameValue(Symbol.metadata, OSymbol.metadata);
20 changes: 20 additions & 0 deletions test/built-ins/Symbol/metadata/prop-desc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (C) 2023 Ron Buckton. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
esid: sec-symbol.metadata
description: >
`Symbol.metadata` property descriptor
info: |
This property has the attributes { [[Writable]]: false, [[Enumerable]]:
false, [[Configurable]]: false }.
includes: [propertyHelper.js]
features: [decorator-metadata]
---*/

assert.sameValue(typeof Symbol.metadata, 'symbol');
verifyProperty(Symbol, 'metadata', {
writable: false,
enumerable: false,
configurable: false,
});

Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (C) 2023 Ron Buckton. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
esid: sec-createdecoratorcontextobject
description: >
Property descriptor for metadata property of decorator context object.
info: |
CreateDecoratorContextObject ( kind, name, initializers, decorationState, metadataObj [ , isStatic ] )
[...]
13. Perform ! CreateDataPropertyOrThrow(contextObj, "metadata", metadata).
14. Return contextObj.
includes: [propertyHelper.js]
features: [decorators, decorator-metadata]
---*/

var contextObj;
function dec(_, context) {
contextObj = context;
}

void @dec class C {};
assert.sameValue(typeof contextObj.metadata, "object");
verifyProperty(contextObj, 'metadata', {
enumerable: false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By my reading of the spec, this seems like it should be true? In CreateDecoratorContextObject we create the property with CreateDataPropertyOrThrow, which calls CreateDataProperty, which creates enumerable properties.

writable: true,
configurable: true,
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing this test could additionally do that it doesn't currently do: make sure the private/kind metadata for each invocation of the decorator matches the type of thing that it decorates. Currently the test passes if each private/kind combination is hit once, but an implementation could switch two of them (e.g., private accessors are reported as public and vice versa) and the test would still pass.

(May or may not be worth it.)

Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Copyright (C) 2023 Ron Buckton. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
esid: sec-createdecoratorcontextobject
description: >
Property descriptor for metadata property of decorator context object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test doesn't seem to be about the property descriptor, maybe this was accidentally copied from another test?

info: |
CreateDecoratorContextObject ( kind, name, initializers, decorationState, metadataObj [ , isStatic ] )
[...]
13. Perform ! CreateDataPropertyOrThrow(contextObj, "metadata", metadata).
14. Return contextObj.
includes: [deepEqual.js]
features: [decorators, decorator-metadata]
---*/

var kinds = {
rbuckton marked this conversation as resolved.
Show resolved Hide resolved
__proto__: null,
"class": false,
"public method": false,
"public getter": false,
"public setter": false,
"public field": false,
"public accessor": false,
"private method": false,
"private getter": false,
"private setter": false,
"private field": false,
"private accessor": false,
};
function dec(_, context) {
const key = `${context.private ? "private" : "public"} ${context.kind}`;
kinds[key] = typeof context.metadata === "object";
}

void @dec class C {
@dec method() {}
@dec get getter() {}
@dec set setter(x) {}
@dec field;
@dec accessor accessor;
@dec #method() {}
@dec get #getter() {}
@dec set #setter(x) {}
@dec #field;
@dec accessor #accessor;
};

assert.deepEqual(kinds, {
"class": true,
"public method": true,
"public getter": true,
"public setter": true,
"public field": true,
"public accessor": true,
"private method": true,
"private getter": true,
"private setter": true,
"private field": true,
"private accessor": true,
});
Comment on lines +48 to +60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're trying to get rid of assert.deepEqual (#3476). Maybe assert.compareArray(Object.entries(kinds), [...]) would work here.

Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright (C) 2023 Ron Buckton. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.

/*---
esid: sec-runtime-semantics-classdefinitionevaluation
description: >
Metadata is only attached when a class or class element is decorated.
info: |
ClassTail : ClassHeritage_opt { ClassBody_opt }
21. Let hasDecorators be false.
22. If decorators is not empty, set hasDecorators to true.
[...]
25. For each ClassElement e of elements, do
[...]
e. If element is a ClassElementDefinition Record, then
i. If e.[[Decorators]] is not empty, set hasDecorators to true.
[...]
[...]
29. Let metadataObj be empty.
30. If hasDecorators is true, then
a. If ClassHeritage is present, [...]
b. Else, let metadataParent be null.
c. Set metadataObj to OrdinaryObjectCreate(metadataParent).
[...]
41. If metadataObj is not empty, then
a. Let setMetadataResult be Completion(DefinePropertyOrThrow(F, @@metadata, PropertyDescriptor {
[[Value]]: metadataObj, [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true })).
i. If _setMetadataResult_ is an abrupt completion, then
1. Set the running execution context's PrivateEnvironment to _outerPrivateEnvironment_.
2. Return ? _setMetadataResult_.
[...]
features: [decorators, decorator-metadata]
---*/

function dec() {}

let C1 = @dec class C1 {};
assert.sameValue(typeof C1[Symbol.metadata], "object");
assert.notSameValue(C1[Symbol.metadata], null);

let C2 = class C2 { @dec method() {} };
assert.sameValue(typeof C2[Symbol.metadata], "object");
assert.notSameValue(C2[Symbol.metadata], null);

let C3 = class C3 {};
assert.sameValue(C3[Symbol.metadata], null); // inherited from Function.prototype[Symbol.metadata]
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright (C) 2023 Ron Buckton. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.

/*---
esid: sec-runtime-semantics-classdefinitionevaluation
description: >
Metadata on a derived class inherits from the metadata of the declared super class.
info: |
ClassTail : ClassHeritage_opt { ClassBody_opt }
21. Let hasDecorators be false.
22. If decorators is not empty, set hasDecorators to true.
[...]
25. For each ClassElement e of elements, do
[...]
e. If element is a ClassElementDefinition Record, then
i. If e.[[Decorators]] is not empty, set hasDecorators to true.
[...]
[...]
29. Let metadataObj be empty.
30. If hasDecorators is true, then
a. If ClassHeritage is present, [...]
b. Else, let metadataParent be null.
c. Set metadataObj to OrdinaryObjectCreate(metadataParent).
[...]
41. If metadataObj is not empty, then
a. Let setMetadataResult be Completion(DefinePropertyOrThrow(F, @@metadata, PropertyDescriptor {
[[Value]]: metadataObj, [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true })).
i. If _setMetadataResult_ is an abrupt completion, then
1. Set the running execution context's PrivateEnvironment to _outerPrivateEnvironment_.
2. Return ? _setMetadataResult_.
[...]
features: [decorators, decorator-metadata]
---*/

function dec() {}

class UndecoratedBase {}

let Base = @dec class Base {};
const baseMetadata = Base[Symbol.metadata];

let Derived = @dec class Derived extends Base { };
Object.setPrototypeOf(Derived, UndecoratedBase);

const derivedMetadata = Derived[Symbol.metadata];
assert.sameValue(Object.getPrototypeOf(derivedMetadata), baseMetadata);
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright (C) 2023 Ron Buckton. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.

/*---
esid: sec-runtime-semantics-classdefinitionevaluation
description: >
Metadata on a derived class inherits from the metadata of the declared super class.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion, the description could be a bit more specific here, more like the title of the test: maybe something like "Metadata on a derived class inherits from the original metadata of the declared super class, unaffected by overwriting"

info: |
ClassTail : ClassHeritage_opt { ClassBody_opt }
21. Let hasDecorators be false.
22. If decorators is not empty, set hasDecorators to true.
[...]
25. For each ClassElement e of elements, do
[...]
e. If element is a ClassElementDefinition Record, then
i. If e.[[Decorators]] is not empty, set hasDecorators to true.
[...]
[...]
29. Let metadataObj be empty.
30. If hasDecorators is true, then
a. If ClassHeritage is present, [...]
b. Else, let metadataParent be null.
c. Set metadataObj to OrdinaryObjectCreate(metadataParent).
[...]
41. If metadataObj is not empty, then
a. Let setMetadataResult be Completion(DefinePropertyOrThrow(F, @@metadata, PropertyDescriptor {
[[Value]]: metadataObj, [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true })).
i. If _setMetadataResult_ is an abrupt completion, then
1. Set the running execution context's PrivateEnvironment to _outerPrivateEnvironment_.
2. Return ? _setMetadataResult_.
[...]
features: [decorators, decorator-metadata]
---*/

function dec() {}

class UndecoratedBase {}

let Base = @dec class Base {};
const baseMetadata = Base[Symbol.metadata];

let Derived = @dec class Derived extends Base { };
Base[Symbol.metadata] = {};

const derivedMetadata = Derived[Symbol.metadata];
assert.sameValue(Object.getPrototypeOf(derivedMetadata), baseMetadata);
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright (C) 2023 Ron Buckton. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.

/*---
esid: sec-runtime-semantics-classdefinitionevaluation
description: >
Metadata is only attached when a class or class element is decorated.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description seems copied from somewhere else, maybe something like "Metadata inherits from the metadata of the super class"?

info: |
ClassTail : ClassHeritage_opt { ClassBody_opt }
21. Let hasDecorators be false.
22. If decorators is not empty, set hasDecorators to true.
[...]
25. For each ClassElement e of elements, do
[...]
e. If element is a ClassElementDefinition Record, then
i. If e.[[Decorators]] is not empty, set hasDecorators to true.
[...]
[...]
29. Let metadataObj be empty.
30. If hasDecorators is true, then
a. If ClassHeritage is present, [...]
b. Else, let metadataParent be null.
c. Set metadataObj to OrdinaryObjectCreate(metadataParent).
[...]
41. If metadataObj is not empty, then
a. Let setMetadataResult be Completion(DefinePropertyOrThrow(F, @@metadata, PropertyDescriptor {
[[Value]]: metadataObj, [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true })).
i. If _setMetadataResult_ is an abrupt completion, then
1. Set the running execution context's PrivateEnvironment to _outerPrivateEnvironment_.
2. Return ? _setMetadataResult_.
[...]
features: [decorators, decorator-metadata]
---*/

function dec() {}

let Base = @dec class Base {};
const baseMetadata = Base[Symbol.metadata];
assert.sameValue(Object.getPrototypeOf(baseMetadata), null);

let Derived = @dec class Derived extends Base { };
const derivedMetadata = Derived[Symbol.metadata];
assert.sameValue(Object.getPrototypeOf(derivedMetadata), baseMetadata);
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright (C) 2023 Ron Buckton. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.

/*---
esid: sec-runtime-semantics-classdefinitionevaluation
description: >
The metadata object is a regular, extensible JS object.
info: |
ClassTail : ClassHeritage_opt { ClassBody_opt }
21. Let hasDecorators be false.
22. If decorators is not empty, set hasDecorators to true.
[...]
25. For each ClassElement e of elements, do
[...]
e. If element is a ClassElementDefinition Record, then
i. If e.[[Decorators]] is not empty, set hasDecorators to true.
[...]
[...]
29. Let metadataObj be empty.
30. If hasDecorators is true, then
a. If ClassHeritage is present, [...]
b. Else, let metadataParent be null.
c. Set metadataObj to OrdinaryObjectCreate(metadataParent).
[...]
features: [decorators, decorator-metadata]
---*/

function dec() {}

let C = @dec class C {};
const metadata = C[Symbol.metadata];
assert(Object.isExtensible(metadata));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert(Object.isExtensible(metadata));
assert(Object.isExtensible(metadata), "Metadata is not frozen");

(I suggest using assertion messages at least in plain assert(), otherwise if it fails you get "Expected false to be true" or something like that)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we additionally verify that the object is a plain object, something like this?

assert.sameValue(Object.getPrototypeOf(metadata), Object.prototype, "Metadata's prototype is Object.prototype");

Loading