Details

Reviewers

avivey

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15188: arc lint: "Function utf8_decode() is deprecated" in PHP 8.2

Commits

rARC08dfffd5caf7: Replace function utf8_decode() - deprecated since PHP 8.2

Summary

The function utf8_decode() was a shortcut to convert strings
encoded from UTF-8 to ISO-8859-1 ("Latin 1").

This function was deprecated since PHP 8.2 and will be dropped
in PHP 9:

https://wiki.php.net/rfc/remove_utf8_decode_and_utf8_encode

As mentioned in the RFC, if a $string is a valid UTF-8 string,
so this could be used to count the number of code points:

strlen(utf8_decode($string))

It works because any unmappable code point is replaced with the
single byte '?' in the output. But, the correct native approach
should be this one:

mb_strlen($string, 'UTF-8');

Also, another good approach is this one:

iconv_strlen($string, 'UTF-8')

Note that mb_strlen() was introduced in PHP 4, so, there
are no compatibility issues in using that.

Note that the mbstring extension is already required in the installation
documentation, so this should not change anything for any person.

https://we.phorge.it/T15188

https://wiki.php.net/rfc/remove_utf8_decode_and_utf8_encode

https://www.php.net/manual/en/function.utf8-decode

https://www.php.net/manual/en/function.mb-convert-encoding.php

https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#utf8decodeencodetombconvertencodingrector

Closes T15188

Test Plan

I was able to execute "arc lint" from PHP 8.2
I was able to execute this "arc diff" from PHP 8.2
With this patch you can still run "arc lint" with your local version

Diff Detail

Repository

rARC Arcanist

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

valerio.bozzolan created this revision.Mar 24 2023, 14:31

Owners added a reviewer: O1: Blessed Committers.Mar 24 2023, 14:31

Herald added subscribers: Cigaryno, Matthew, tobiaswiese, speck. · View Herald TranscriptMar 24 2023, 14:31

valerio.bozzolan requested review of this revision.Mar 24 2023, 14:31

Harbormaster completed remote builds in B171: Diff 383.Mar 24 2023, 14:31

valerio.bozzolan edited the summary of this revision. (Show Details)Mar 24 2023, 14:32

valerio.bozzolan added a task: T15188: arc lint: "Function utf8_decode() is deprecated" in PHP 8.2.

valerio.bozzolan mentioned this in T15187: Fix Arcanist in PHP 8.1+ (testing the Phorge repo).Mar 24 2023, 14:39

valerio.bozzolan edited the test plan for this revision. (Show Details)Mar 24 2023, 14:56

add inline comment

Harbormaster completed remote builds in B172: Diff 384.Mar 24 2023, 15:01

fix inline comment

Harbormaster completed remote builds in B173: Diff 385.Mar 24 2023, 15:15

simplification

Harbormaster completed remote builds in B174: Diff 386.Mar 24 2023, 15:24

adopt mb_strlen() that is optimized to do exactly this if you tell that you want an UTF-8

Harbormaster completed remote builds in B175: Diff 387.Mar 24 2023, 15:36

valerio.bozzolan edited the summary of this revision. (Show Details)Mar 24 2023, 15:45

valerio.bozzolan edited the summary of this revision. (Show Details)Mar 24 2023, 15:47

valerio.bozzolan edited the test plan for this revision. (Show Details)

Interestingly, this modification also brings a performance improvement on calculating the length of multiple strings.

Example test:

<?php

// the Arabic (Hello) string below is: 59 bytes and 32 characters
$string = "السلام علیکم ورحمة الله وبرکاته!";

$t = 100000;

$start_time = microtime(TRUE);
for($i=0; $i <$t; $i++) {
        $n = mb_strlen($string, 'UTF-8');
}
$end_time = microtime(TRUE);
echo $end_time - $start_time . "\n";

$start_time = microtime(TRUE);
for($i=0; $i <$t; $i++) {
        $n = strlen(utf8_decode($string) );
}
$end_time = microtime(TRUE);
echo $end_time - $start_time . "\n";

On my computer I get:

# new way (shorter is better):
0.042040109634399

# old way:
0.060499906539917

This means less CPU cycles and, for environmental lovers, this also means less waste of resources in long terms. Anyway, a single burp for me is probably able to cancel this environmental benefit. So I will try to restrain myself.

valerio.bozzolan mentioned this in Z1: Phorge.Mar 24 2023, 16:20

valerio.bozzolan edited the test plan for this revision. (Show Details)

avivey accepted this revision.Mar 25 2023, 09:47

This revision is now accepted and ready to land.Mar 25 2023, 09:47

remote: This push was rejected by Herald push rule H8.
remote:     Change: commit/
remote:       Rule: Guard Arcanist Repo with Blessed Committers
remote:     Reason: Commit is not approved by Blessed Committers
remote: Transcript: https://we.phorge.it/herald/transcript/4967/

https://we.phorge.it/herald/transcript/4967/

In D25092#2806, @valerio.bozzolan wrote:

remote: This push was rejected by Herald push rule H8.
remote:     Change: commit/
remote:       Rule: Guard Arcanist Repo with Blessed Committers
remote:     Reason: Commit is not approved by Blessed Committers
remote: Transcript: https://we.phorge.it/herald/transcript/4967/

https://we.phorge.it/herald/transcript/4967/

There's too many negatives in that conditions. I'm going to need some paper...