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.

Sign up for GitHub

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 Function.prototype.caller and arguments properties #4355

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add tests for Function.prototype.caller and arguments properties #4355

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1c42cfc
Add tests for Function.prototype.caller and arguments properties
jdorfman Dec 12, 2024
9875dd4
Add tests for Function.prototype.caller and arguments properties
jdorfman Dec 20, 2024
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
18 changes: 18 additions & 0 deletions test/built-ins/Function/prototype/caller-arguments/accessor-properties.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*---
description: Function.prototype caller and arguments properties are accessor properties with ThrowTypeError
esid: sec-function.prototype.caller
info: |
Function instances do not inherit the "caller" and "arguments" accessors
from Function.prototype. The accessors exist only on Function.prototype.
---*/
Comment on lines +1 to +7
Copy link
Contributor

Choose a reason for hiding this comment

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

sec-function.prototype.caller does not appear to be a valid anchor: https://tc39.es/ecma262/#sec-function.prototype.caller

See also CONTRIBUTING.md: Test Case Style.


const callerDesc = Object.getOwnPropertyDescriptor(Function.prototype, "caller");
const argumentsDesc = Object.getOwnPropertyDescriptor(Function.prototype, "arguments");

assert.sameValue(typeof callerDesc.get, "function");
assert.sameValue(typeof callerDesc.set, "function");
assert.sameValue(callerDesc.get, callerDesc.set, "caller getter/setter should be same function");

assert.sameValue(typeof argumentsDesc.get, "function");
assert.sameValue(typeof argumentsDesc.set, "function");
assert.sameValue(argumentsDesc.get, argumentsDesc.set, "arguments getter/setter should be same function");
Copy link
Contributor

Choose a reason for hiding this comment

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

This is already covered by the existing tests in:

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, nice! Then this PR can be largely refactored to modernize those existing tests.

test/built-ins/Function/prototype/restricted-property-arguments.js → test/built-ins/Function/prototype/arguments/prop-desc.js

// Copyright (C) 2024 Justin Dorfman. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.

/*---
esid: sec-addrestrictedfunctionproperties
description: >
  Function.prototype.arguments
info: |
  2. Let _thrower_ be _realm_.[[Intrinsics]].[[%ThrowTypeError%]].
  3. Perform ! DefinePropertyOrThrow(_F_, *"caller"*, PropertyDescriptor { [[Get]]: _thrower_, [[Set]]: _thrower_, [[Enumerable]]: *false*, [[Configurable]]: *true* }).
  4. Perform ! DefinePropertyOrThrow(_F_, *"arguments"*, PropertyDescriptor { [[Get]]: _thrower_, [[Set]]: _thrower_, [[Enumerable]]: *false*, [[Configurable]]: *true* }).
includes: [propertyHelper.js, wellKnownIntrinsicObjects.js]
---*/

verifyProperty(Function.prototype, "arguments", {
  enumerable: false,
  configurable: true
});

var descriptor = Object.getOwnPropertyDescriptor(Function.prototype, "caller");
assert.sameValue(typeof descriptor.get, "function", "Function.prototype.arguments has a getter");
assert.sameValue(typeof descriptor.set, "function", "Function.prototype.arguments has a setter");
assert.sameValue(descriptor.get, descriptor.set, "Function.prototype.arguments getter and setter are identical");

var throwTypeError;
WellKnownIntrinsicObjects.forEach(function(record) {
  if (record.name === "%ThrowTypeError%") {
    throwTypeError = record.value;
  }
});
if (throwTypeError) {
  assert.sameValue(descriptor.set, throwTypeError, "Function.prototype.arguments getter is %ThrowTypeError%");
}
assert.throws(TypeError, function() {
  return Function.prototype.arguments;
});
assert.throws(TypeError, function() {
  Function.prototype.arguments = {};
});

test/built-ins/Function/prototype/restricted-property-caller.js → test/built-ins/Function/prototype/caller/prop-desc.js
(analogous, replacing arguments with caller)

By the way @anba, how are you finding the existing coverage? It's something that I struggle with, especially in the case of test subdirectories that heavily use old patterns.

Copy link
Author

Choose a reason for hiding this comment

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

@gibson042 I added the refactored tests. For the other files I'm not sure what to do. Should I still address the issues or will they be removed?

Copy link
Author

Choose a reason for hiding this comment

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

@gibson042 @anba Should I still address the other reviews/suggestions?

13 changes: 13 additions & 0 deletions test/built-ins/Function/prototype/caller-arguments/cross-realm-behavior.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*---
description: Function.prototype caller and arguments properties use the same ThrowTypeError across realms
features: [cross-realm]
---*/

const otherRealm = $262.createRealm();
const otherFunctionProto = otherRealm.evaluate('Function.prototype');

const mainCallerDesc = Object.getOwnPropertyDescriptor(Function.prototype, "caller");
const otherCallerDesc = Object.getOwnPropertyDescriptor(otherFunctionProto, "caller");

assert.sameValue(mainCallerDesc.get, otherCallerDesc.get, "caller getter should be same across realms");
assert.sameValue(mainCallerDesc.set, otherCallerDesc.set, "caller setter should be same across realms");
Copy link
Contributor

Choose a reason for hiding this comment

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

This is incorrect: %ThrowTypeError% is defined once per realm, cf. https://tc39.es/ecma262/#sec-%throwtypeerror%.

See also https://github.com/tc39/test262/blob/main/test/built-ins/ThrowTypeError/unique-per-realm-function-proto.js.

12 changes: 12 additions & 0 deletions test/built-ins/Function/prototype/caller-arguments/module-context.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*---
description: Function properties behave consistently in module context
flags: [module]
---*/

function normalFunc() {}
function strictFunc() { }
Copy link
Contributor

Choose a reason for hiding this comment

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

Functions are implicitly in strict-mode when defined in modules.


assert(!Object.hasOwnProperty.call(normalFunc, "caller"), "normal function should not have caller");
assert(!Object.hasOwnProperty.call(strictFunc, "caller"), "strict function should not have caller");
assert(!Object.hasOwnProperty.call(normalFunc, "arguments"), "normal function should not have arguments");
assert(!Object.hasOwnProperty.call(strictFunc, "arguments"), "strict function should not have arguments");
18 changes: 18 additions & 0 deletions test/built-ins/Function/prototype/caller-arguments/strict-vs-nonstrict.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*---
description: Function properties behavior in strict vs non-strict contexts
flags: [noStrict]
---*/

function nonStrictFunc() {
return nonStrictFunc.caller;
}

function strictFunc() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing "use strict" to make strictFunc actually a strict-mode function.

return strictFunc.caller;
}

assert(!Object.hasOwnProperty.call(nonStrictFunc, "caller"), "non-strict function should not have own caller property");
assert(!Object.hasOwnProperty.call(strictFunc, "caller"), "strict function should not have own caller property");
Copy link
Contributor

Choose a reason for hiding this comment

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

The strict-mode case is already covered by:

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should modernize that as well, introducing a helper like verifyNoRestrictedFunctionProperties and using it at e.g.

  • test/language/expressions/{arrow-function,async-arrow-function,async-function,async-generator,class,function,generators}/{restricted-properties,restricted-properties-module}.js (covering expressions like var f = function(){}; and generalizations, in script and module code)
  • test/language/statements/{async-function,async-generator,class,function,generators}/restricted-properties.js (covering statements like function f(){} and generalizations, in script and module code)
  • test/language/statements/class/definition/{methods-restricted-properties,methods-restricted-properties-module}.js (covering methods like var m = new (class { m(){} })().m, in script and module code)
    • likewise somewhere for generalizations—class static methods, object literal methods, and syntactic get/set accessors
  • test/built-ins/{AsyncFunction,AsyncGeneratorFunction,Function,GeneratorFunction}/instance-restricted-properties.js (each covering both $Function() and $Function('"use strict"'), or alternatively split into a pair of files)
  • test/built-ins/Function/prototype/bind/instance-restricted-properties.js

and removing all other restricted-properties files.


assert.throws(TypeError, () => nonStrictFunc(), "accessing caller should throw");
assert.throws(TypeError, () => strictFunc(), "accessing caller should throw in strict mode");