Page MenuHomePhorge

PHP 8.2: fixes for strlen() not accepting NULL anymore, part 1
ClosedPublic

Authored by valerio.bozzolan on Mar 31 2023, 13:38.
Tags
None
Referenced Files
F3604808: D25104.1745231000.diff
Sun, Apr 20, 10:23
F3604509: D25104.1745207093.diff
Sun, Apr 20, 03:44
F3576972: D25104.1745074787.diff
Fri, Apr 18, 14:59
F3529836: D25104.1744868645.diff
Wed, Apr 16, 05:44
F3375127: D25104.1744303785.diff
Wed, Apr 9, 16:49
F3374593: D25104.1744292523.diff
Wed, Apr 9, 13:42
F3374089: D25104.1744283210.diff
Wed, Apr 9, 11:06
F3372584: D25104.1744250395.diff
Wed, Apr 9, 01:59

Details

Summary

This change avoids some unnecessary uses of the strlen() function,
actually fixing some deprecation warnings in PHP 8.2.

In short, this is the suggested universal replace:

-if(strlen($v))
+if(phutil_nonempty_string($v))

And, if you know PHP, this is also another adoptable replace, but
only for cases where you are sure that the string "0" is not useful:

-if(strlen($v))
+if($v))

As usual the optimal solution depends on the contest.

Other similar patches will probably follow.

Closes T15222
Ref T15190

Test Plan
  • for the first time in my life, with this change, the unit tests are passed in PHP 8.2
  • check with your big eyes that there are no obvious typos

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

src/infrastructure/cluster/PhabricatorDatabaseRef.php
232–233

This check can be adopted since a port "0" has no sense.

src/infrastructure/env/PhabricatorEnv.php
435–436

This check can be adopted since a domain "0" has no sense.

458–459

This check can be adopted since a domain "0" has no sense.

748–754

This simplified check can be adopted since a domain "0" has no sense.

796–802

This simplified check can be adopted since a protocol "0" has no sense.

814–821

This simplified check can be adopted since a protocol "0" has no sense.

src/applications/metamta/engine/PhabricatorMailEmailEngine.php
510–513

Yep I confirm that I know PHP and I know what is going on up here. I confirm that the value "0" does not make any sense in this string and that is why this simple if is sufficient.

530–532

Yep I confirm that I know PHP and I know what is going on up here. I confirm that the value "0" does not make any sense in this string and that is why this simple if is sufficient.

src/infrastructure/env/PhabricatorEnv.php
728–729

Yep I confirm that I know PHP and I know what is going on up here. I confirm that the value "0" does not make any sense in this string and that is why this simple if is sufficient.

avivey subscribed.

I'm not convinced that a host named 0 is invalid, but fine.

This revision is now accepted and ready to land.Mar 31 2023, 19:12
  • allow $host with very weird names (zero ihih)
This revision was automatically updated to reflect the committed changes.