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

Update Formatter.php to avoid error on PHP 8 #19924

Merged
merged 3 commits into from
Aug 21, 2023

Conversation

arollmann
Copy link
Contributor

@arollmann arollmann commented Aug 11, 2023

When upgrading from PHP 7 to PHP 8, I get an error "Unknown named parameter " in a GridView column using this code :

[
	'attribute' => 'id_main',
	'format' => [
		'integer',
		'numberFormatterOptions' => [	\NumberFormatter::MIN_INTEGER_DIGITS => 3],
	],
],

To solve it, we must adapt line 463 in Formatter.php, like this : return call_user_func_array([$this, $method], array_values($params));

Cf. https://php.watch/versions/8.0/named-parameters#named-params-call_user_func_array

Q A
Is bugfix? ✔️
New feature? ✔️/❌
Breaks BC? ✔️/❌
Fixed issues

When upgrading from PHP 7 to PHP 8, I get an error "Unknown named parameter " in a GridView column using this code : 
[
			'attribute' => 'id_main',
			'format' => [
				'integer',
				'numberFormatterOptions' => [									\NumberFormatter::MIN_INTEGER_DIGITS => 3,
				],
			],
		],
To solve it, we must adapt line 463 in Formatter.php, like this : 
return call_user_func_array([$this, $method], array_values($params));
@what-the-diff
Copy link

what-the-diff bot commented Aug 11, 2023

PR Summary

  • Modification in Formatter.php File
    The way arguments are passed to a specific function in the file Formatter.php has been changed. Previously, the data structure $params was used as arguments. Now, we're using the values extracted from $params, making it more straightforward and possibly improving performance.

@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +2.60% 🎉

Comparison is base (e916e9d) 46.29% compared to head (cb9cbe5) 48.89%.

❗ Current head cb9cbe5 differs from pull request most recent head 43e97e0. Consider uploading reports for the commit 43e97e0 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19924      +/-   ##
==========================================
+ Coverage   46.29%   48.89%   +2.60%     
==========================================
  Files         445      445              
  Lines       42810    42790      -20     
==========================================
+ Hits        19818    20923    +1105     
+ Misses      22992    21867    -1125     
Files Changed Coverage Δ
framework/i18n/Formatter.php 63.48% <100.00%> (ø)

... and 32 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@samdark
Copy link
Member

samdark commented Aug 11, 2023

Makes sense. Would you please add a line for CHANGELOG?

@samdark samdark added the php8 label Aug 11, 2023
@samdark samdark added this to the 2.0.49 milestone Aug 11, 2023
@samdark samdark merged commit 4c0a00f into yiisoft:master Aug 21, 2023
47 checks passed
@samdark
Copy link
Member

samdark commented Aug 21, 2023

Thanks. Next time you can just push another file to the same branch. That will update the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants