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
F3033509: D25104.1740884019.diff
Sat, Mar 1, 02:53
F3004954: D25104.1740505711.diff
Mon, Feb 24, 17:48
F3002061: D25104.1740445236.diff
Mon, Feb 24, 01:00
F3002060: D25104.1740445234.diff
Mon, Feb 24, 01:00
F3002059: D25104.1740445234.diff
Mon, Feb 24, 01:00
F3002058: D25104.1740445233.diff
Mon, Feb 24, 01:00
F3002057: D25104.1740445232.diff
Mon, Feb 24, 01:00
F2996804: D25104.1740386302.diff
Sun, Feb 23, 08:38

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.