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

[PHP 8.4] Add mb_ucfirst() and mb_lcfirst() #471

Closed
wants to merge 4 commits into from

Conversation

zonuexe
Copy link
Contributor

@zonuexe zonuexe commented Mar 30, 2024

resolve #470

This implementation is also tested and benchmarked in my repository: https://github.com/zonuexe/polyfill-mb_ulcfirst

@zonuexe zonuexe force-pushed the feature/mb_ulcfirst branch 3 times, most recently from d75bb6b to 86116b5 Compare March 31, 2024 00:01
@zonuexe zonuexe force-pushed the feature/mb_ulcfirst branch 4 times, most recently from 7bfc6b7 to 3cef459 Compare March 31, 2024 00:53
@zonuexe
Copy link
Contributor Author

zonuexe commented Mar 31, 2024

These errors below looks strange:

40) Warning
Incompatible signature for PHP >= 8:
- mb_ucfirst(string $string, ?string $encoding = null): string
+ mb_ucfirst(string $string, ?string $encoding = null): string

41) Warning
Incompatible signature for PHP >= 8:
- mb_lcfirst(string $string, ?string $encoding = null): string
+ mb_lcfirst(string $string, ?string $encoding = null): string

Internal functions up to PHP 8.0 implicitly accepted null as a string. It seems that the type is declared as ?string instead of string for compatibility in polyfills up to php80.

$map = [
'?' => '',
'IDNA_DEFAULT' => \PHP_VERSION_ID >= 80100 ? 'IDNA_DEFAULT' : '0',
'array|string|null $string' => 'array|string $string',
'array|string|null $from_encoding = null' => 'array|string|null $from_encoding = null',
'array|string|null $from_encoding' => 'array|string $from_encoding',
];
if (strtr($polyfillSignature, $map) !== $originalSignature) {
$warnings[] = TestListener::warning("Incompatible signature for PHP >= 8:\n- {$f['name']}$originalSignature\n+ {$f['name']}$polyfillSignature");
}

In the code below, ? is removed to check the difference between the internal function and the polyfill interface.

PHP 8.4 deprecates implicit nullable, so this test should be replaced with more stringent checks.

@@ -105,7 +105,7 @@ public function testDecodeNumericEntity()
$this->assertSame('déjà � â ã', mb_decode_numericentity('déjà � á â', $convmap, 'UTF-8'));

$bogusDecEntities = 'déjà � áá &#&#225&#225 &#225 &#225t';
$this->assertSame('déjà � ââ &#&#225â â ât', mb_decode_numericentity($bogusDecEntities, $convmap, 'UTF-8'));
$this->assertSame('déjà � ââ &#&#225â â ât', p::mb_decode_numericentity($bogusDecEntities, $convmap, 'UTF-8'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test was affected by changes in the native mb_decode_numericentity() function.

https://3v4l.org/fpGFH

It seems that other tests also need to call polyfill methods instead of built-in function.

@Ayesh
Copy link
Contributor

Ayesh commented Mar 31, 2024

Possibly duplicate of #466

@zonuexe
Copy link
Contributor Author

zonuexe commented Mar 31, 2024

@Ayesh Sorry, I missed that PR because the issue didn't exist. It looks like my patch is covered by your implementation.

@zonuexe zonuexe closed this Mar 31, 2024
@Ayesh
Copy link
Contributor

Ayesh commented Mar 31, 2024

Let's put the efforts together :)
If you see anything that we could improve in the other PR, or if you think merging this PR is the way to go, please do comment.

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

Successfully merging this pull request may close these issues.

[PHP 8.4] mb_ucfirst() and mb_lcfirst() polyfills
2 participants