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

Jump to bottom

Include the source location in Closure names #13550

Merged
merged 14 commits into from
Apr 12, 2024
Merged

Include the source location in Closure names #13550

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
5ccc3b1
Include the source location in Closure names
TimWolla Feb 28, 2024
e07ee9d
Update scripts/dev/bless_tests.php for closure naming
TimWolla Feb 28, 2024
a460100
Adjust existing tests for closure naming
TimWolla Feb 28, 2024
885e088
Adjust tests for closure naming that were not caught locally
TimWolla Feb 28, 2024
9148390
Drop the namespace from closure names
TimWolla Mar 6, 2024
4cc9927
Include filename and line number as separate keys in Closure debug info
TimWolla Mar 15, 2024
9152d69
Fix test
TimWolla Mar 18, 2024
e30ec58
Fix test
TimWolla Mar 18, 2024
f281414
Include the surrounding class and function name in closure names
TimWolla Mar 18, 2024
0558b94
Fix test
TimWolla Mar 18, 2024
b4efdd4
Relax test expecations
TimWolla Apr 12, 2024
369a102
Merge branch 'master' into closure-naming
TimWolla Apr 12, 2024
350c48e
Fix tests after merge
TimWolla Apr 12, 2024
6c920fd
NEWS / UPGRADING
TimWolla Apr 12, 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
4 changes: 2 additions & 2 deletions Zend/tests/arrow_functions/006.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ try {
--EXPECTF--
int(2)
int(10)
{closure}(): Argument #1 ($x) must be of type int, string given, called in %s on line %d
{closure:%s:%d}(): Argument #1 ($x) must be of type int, string given, called in %s on line %d
array(3) {
[0]=>
int(20)
Expand All @@ -41,4 +41,4 @@ array(3) {
[2]=>
int(30)
}
{closure}(): Argument #2 must be of type ?int, string given, called in %s on line %d
{closure:%s:%d}(): Argument #2 must be of type ?int, string given, called in %s on line %d
6 changes: 3 additions & 3 deletions Zend/tests/bug26697.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ spl_autoload_register(function ($name) {
var_dump(class_exists('NotExistingClass'));

?>
--EXPECT--
{closure}(NotExistingClass)
--EXPECTF--
{closure:%s:%d}(NotExistingClass)
bool(false)
{closure}(NotExistingClass), done
{closure:%s:%d}(NotExistingClass), done
bool(false)
8 changes: 4 additions & 4 deletions Zend/tests/bug31102.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ while($test++ < 5)
?>
===DONE===
--EXPECTF--
{closure}(Test1,1)
{closure:%s:%d}(Test1,1)
Caught: Test1::__construct
{closure}(Test2,2)
Caught: {closure}
{closure}(Test3,3)
{closure:%s:%d}(Test2,2)
Caught: {closure:%s:%d}
{closure:%s:%d}(Test3,3)

Fatal error: Uncaught Error: Class "Test3" not found in %s:%d
Stack trace:
Expand Down
4 changes: 2 additions & 2 deletions Zend/tests/bug49908.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ string(3) "Bar"

Fatal error: Uncaught Exception: Bar in %s:%d
Stack trace:
#0 %s(%d): {closure}('Bar')
#1 %s(%d): {closure}('Foo')
#0 %s(%d): {closure:%s:%d}('Bar')
#1 %s(%d): {closure:%s:%d}('Foo')
#2 {main}
thrown in %s on line %d
56 changes: 49 additions & 7 deletions Zend/tests/bug52193.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,43 +35,85 @@ array(0) {
}
array(1) {
[0]=>
object(Closure)#%d (0) {
object(Closure)#%d (3) {
["name"]=>
string(%d) "{closure:%s:%d}"
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need both? I would personally drop name.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just pushed another commit, because I managed to include the parent function name within the closure name, which I believe further improves the feature:

  1. The closure names are even more useful, because they are more specific.
  2. The memory usage will be lower for deep directory hierarchies, because the common filename prefix will not be included in the closure name.
  3. The closure name will remain stable even if the application is deployed into changing directory names for atomic deployments.

I believe this also makes the name useful within the debug output, as it contains additional information over just the file + line.

["file"]=>
string(%d) "%s"
["line"]=>
int(%d)
}
}
int(2)
array(1) {
[0]=>
object(Closure)#%d (1) {
object(Closure)#%d (4) {
["name"]=>
string(%d) "{closure:%s:%d}"
["file"]=>
string(%d) "%s"
["line"]=>
int(%d)
["static"]=>
array(1) {
["h"]=>
&array(1) {
[0]=>
object(Closure)#%d (0) {
object(Closure)#%d (3) {
["name"]=>
string(%d) "{closure:%s:%d}"
["file"]=>
string(%d) "%s"
["line"]=>
int(%d)
}
}
}
}
}
object(Closure)#%d (1) {
object(Closure)#%d (4) {
["name"]=>
string(%d) "{closure:%s:%d}"
["file"]=>
string(%d) "%s"
["line"]=>
int(%d)
["static"]=>
array(1) {
["h"]=>
&array(1) {
[0]=>
object(Closure)#%d (0) {
object(Closure)#%d (3) {
["name"]=>
string(%d) "{closure:%s:%d}"
["file"]=>
string(%d) "%s"
["line"]=>
int(%d)
}
}
}
}
array(1) {
[0]=>
object(Closure)#%d (0) {
object(Closure)#%d (3) {
["name"]=>
string(%d) "{closure:%s:%d}"
["file"]=>
string(%d) "%s"
["line"]=>
int(%d)
}
}
array(2) {
[0]=>
object(Closure)#%d (0) {
object(Closure)#%d (3) {
["name"]=>
string(%d) "{closure:%s:%d}"
["file"]=>
string(%d) "%s"
["line"]=>
int(%d)
}
[1]=>
int(5)
Expand Down
8 changes: 7 additions & 1 deletion Zend/tests/bug60738.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@ trigger_error('Error!');
--EXPECTF--
NULL
Intercepted error!
object(Closure)#1 (0) {
object(Closure)#%d (3) {
["name"]=>
string(%d) "{closure:%s:%d}"
["file"]=>
string(%d) "%s"
["line"]=>
int(%d)
}

Notice: Error! in %s on line %d
8 changes: 7 additions & 1 deletion Zend/tests/bug60738_variation.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@ throw new Exception('Exception!');
?>
--EXPECTF--
NULL
object(Closure)#1 (0) {
object(Closure)#%d (3) {
["name"]=>
string(%d) "{closure:%s:%d}"
["file"]=>
string(%d) "%s"
["line"]=>
int(%d)
}

Fatal error: Uncaught Exception: Exception! in %s:%d
Expand Down
2 changes: 1 addition & 1 deletion Zend/tests/bug60909_1.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ require 'notfound.php';
error(require(notfound.php): Failed to open stream: %s)
Fatal error: Uncaught Exception: Foo in %sbug60909_1.php:5
Stack trace:
#0 %sbug60909_1.php(8): {closure}(2, 'require(notfoun...', '%s', 8)
#0 %s(%d): {closure:%s:%d}(2, 'require(notfoun...', '%s', 8)
#1 %sbug60909_1.php(8): require()
#2 {main}
thrown in %sbug60909_1.php on line 5
Expand Down
2 changes: 1 addition & 1 deletion Zend/tests/bug61273.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ call_user_func_array(function(&$a) {}, $args);
echo strval("okey");
?>
--EXPECTF--
Warning: {closure}(): Argument #1 ($a) must be passed by reference, value given in %s on line %d
Warning: {closure:%s:%d}(): Argument #1 ($a) must be passed by reference, value given in %s on line %d
okey
8 changes: 6 additions & 2 deletions Zend/tests/bug61767.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,18 @@ Error handler called (Undefined variable $undefined)

Fatal error: Uncaught ErrorException: Undefined variable $undefined in %sbug61767.php:%d
Stack trace:
#0 %sbug61767.php(%d): {closure}(%s, 'Undefined varia...', '%s', %d)
#0 %s(%d): {closure:%s:%d}(2, 'Undefined varia...', '%s', 13)
#1 {main}
thrown in %sbug61767.php on line %d
Shutting down
Array
(
[type] => 1
[message] => %a
[message] => Uncaught ErrorException: Undefined variable $undefined in %s:%d
Stack trace:
#0 %s(%d): {closure:%s:%d}(2, 'Undefined varia...', '%s', 13)
#1 {main}
thrown
[file] => %sbug61767.php
[line] => %d
)
2 changes: 1 addition & 1 deletion Zend/tests/bug64960.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Deprecated: Creation of dynamic property Exception::$_trace is deprecated in %s

Fatal error: Uncaught Exception in %sbug64960.php:19
Stack trace:
#0 [internal function]: {closure}(8, 'ob_end_clean():...', '%s', 9)
#0 [internal function]: {closure:%s:%d}(8, 'ob_end_clean():...', '%s', 9)
#1 %sbug64960.php(9): ob_end_clean()
#2 [internal function]: ExceptionHandler->__invoke(Object(Exception))
#3 {main}
Expand Down
12 changes: 6 additions & 6 deletions Zend/tests/bug67856.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ var_dump(array_reduce($array, function(&$a, &$b) {
}, 0));
?>
--EXPECTF--
Warning: {closure}(): Argument #1 ($a) must be passed by reference, value given in %s on line %d
Warning: {closure:%s:%d}(): Argument #1 ($a) must be passed by reference, value given in %s on line %d

Warning: {closure}(): Argument #2 ($b) must be passed by reference, value given in %s on line %d
Warning: {closure:%s:%d}(): Argument #2 ($b) must be passed by reference, value given in %s on line %d

Warning: {closure}(): Argument #1 ($a) must be passed by reference, value given in %s on line %d
Warning: {closure:%s:%d}(): Argument #1 ($a) must be passed by reference, value given in %s on line %d

Warning: {closure}(): Argument #2 ($b) must be passed by reference, value given in %s on line %d
Warning: {closure:%s:%d}(): Argument #2 ($b) must be passed by reference, value given in %s on line %d

Warning: {closure}(): Argument #1 ($a) must be passed by reference, value given in %s on line %d
Warning: {closure:%s:%d}(): Argument #1 ($a) must be passed by reference, value given in %s on line %d

Warning: {closure}(): Argument #2 ($b) must be passed by reference, value given in %s on line %d
Warning: {closure:%s:%d}(): Argument #2 ($b) must be passed by reference, value given in %s on line %d
int(6)
16 changes: 14 additions & 2 deletions Zend/tests/bug70321.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,23 @@ var_dump($foo->bar->onBaz);
--EXPECTF--
array(1) {
[0]=>
object(Closure)#%d (0) {
object(Closure)#%d (3) {
["name"]=>
string(%d) "{closure:%s:%d}"
["file"]=>
string(%d) "%s"
["line"]=>
int(%d)
}
}
array(1) {
[0]=>
object(Closure)#%d (0) {
object(Closure)#%d (3) {
["name"]=>
string(%d) "{closure:%s:%d}"
["file"]=>
string(%d) "%s"
["line"]=>
int(%d)
}
}
2 changes: 1 addition & 1 deletion Zend/tests/bug72101.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ $foo->bar($a, $b, $c);
--EXPECTF--
Fatal error: Uncaught Error: Class "DoesNotExists" not found in %s:%d
Stack trace:
#0 %sbug72101.php(%d): {closure}(2, 'MethodCallbackB...', '%s', 8)
#0 %s(%d): {closure:%s:%d}(2, 'MethodCallbackB...', '%s', 8)
#1 %sbug72101.php(%d): PHPUnit_Framework_MockObject_Stub_ReturnCallback->invoke(Object(PHPUnit_Framework_MockObject_Invocation_Static))
#2 %sbug72101.php(%d): PHPUnit_Framework_MockObject_Matcher->invoked(Object(PHPUnit_Framework_MockObject_Invocation_Static))
#3 %sbug72101.php(%d): PHPUnit_Framework_MockObject_InvocationMocker->invoke(Object(PHPUnit_Framework_MockObject_Invocation_Static))
Expand Down
4 changes: 2 additions & 2 deletions Zend/tests/bug74164.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ set_error_handler(function ($type, $msg) {
call_user_func(function (array &$ref) {var_dump("xxx");}, 'not_an_array_variable');
?>
--EXPECTF--
Fatal error: Uncaught Exception: Foo\{closure}(): Argument #1 ($ref) must be passed by reference, value given in %s:%d
Fatal error: Uncaught Exception: {closure:%s:%d}(): Argument #1 ($ref) must be passed by reference, value given in %s:%d
Stack trace:
#0 [internal function]: Foo\{closure}(%s)
#0 [internal function]: {closure:%s:%d}(2, '%s', '%s', 9)
#1 %sbug74164.php(%d): call_user_func(%s)
#2 {main}
thrown in %sbug74164.php on line %d
2 changes: 1 addition & 1 deletion Zend/tests/bug75079_2.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ int(123)

Fatal error: Uncaught Error: Cannot access private property Foo::$bar in %s:%d
Stack trace:
#0 %s(%d): A->{closure}()
#0 %s(%d): A->{closure:%s:%d}()
#1 {main}
thrown in %s on line %d
12 changes: 9 additions & 3 deletions Zend/tests/bug75290.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ var_dump((new ReflectionFunction('sin'))->getClosure());
var_dump(function ($someThing) {});

?>
--EXPECT--
object(Closure)#2 (2) {
--EXPECTF--
object(Closure)#%d (2) {
["function"]=>
string(3) "sin"
["parameter"]=>
Expand All @@ -18,7 +18,13 @@ object(Closure)#2 (2) {
string(10) "<required>"
}
}
object(Closure)#2 (1) {
object(Closure)#%d (4) {
["name"]=>
string(%d) "{closure:%s:%d}"
["file"]=>
string(%d) "%s"
["line"]=>
int(%d)
["parameter"]=>
array(1) {
["$someThing"]=>
Expand Down
2 changes: 1 addition & 1 deletion Zend/tests/bug76534.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ $y = &$x["2bar"];
--EXPECTF--
Fatal error: Uncaught Exception: Illegal string offset "2bar" in %s:%d
Stack trace:
#0 %sbug76534.php(%d): {closure}(2, 'Illegal string ...', '%s', %d)
#0 %s(%d): {closure:%s:%d}(2, 'Illegal string ...', '%s', 7)
#1 {main}
thrown in %sbug76534.php on line %d
2 changes: 1 addition & 1 deletion Zend/tests/bug79657.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ get(...loop());
Fatal error: Uncaught Exception in %s:%d
Stack trace:
#0 %s(%d): throwException()
#1 %s(%d): {closure}()
#1 %s(%d): {closure:%s:%d}()
#2 %s(%d): loop()
#3 {main}
thrown in %s on line %d
Loading
Loading