Details

Reviewers

Group Reviewers

Maniphest Tasks

T15222: Fix arc unit--everything in PHP 8.2
T15190: PHP 8.1: strlen() and other scalar-only functions do not accept NULL anymore - understand fix strategies
T15199: Introduce PhabricatorEnv::getEnvConfigStr() - that always return a string

Commits

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

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

Branch

arcpatch-D25097_1

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 211
Build 211: arc lint + arc unit

Event Timeline

valerio.bozzolan created this revision.Mar 31 2023, 13:38

Owners added a reviewer: O1: Blessed Committers.Mar 31 2023, 13:38

Herald added subscribers: Cigaryno, Matthew, tobiaswiese, speck. · View Herald TranscriptMar 31 2023, 13:38

valerio.bozzolan requested review of this revision.Mar 31 2023, 13:38

Harbormaster completed remote builds in B209: Diff 435.Mar 31 2023, 13:38

valerio.bozzolan edited the summary of this revision. (Show Details)Mar 31 2023, 13:44

valerio.bozzolan added a task: T15190: PHP 8.1: strlen() and other scalar-only functions do not accept NULL anymore - understand fix strategies.

rebase

Harbormaster completed remote builds in B210: Diff 436.Mar 31 2023, 13:44

valerio.bozzolan added inline comments.Mar 31 2023, 13:47

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.

add newline

Harbormaster completed remote builds in B211: Diff 437.Mar 31 2023, 13:48

valerio.bozzolan edited the summary of this revision. (Show Details)Mar 31 2023, 13:50

valerio.bozzolan added a task: T15222: Fix arc unit--everything in PHP 8.2.

valerio.bozzolan mentioned this in T15187: Fix Arcanist in PHP 8.1+ (testing the Phorge repo).Mar 31 2023, 13:54

valerio.bozzolan mentioned this in Z1: Phorge.

valerio.bozzolan edited the test plan for this revision. (Show Details)Mar 31 2023, 13:57

valerio.bozzolan added inline comments.Mar 31 2023, 14:04

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.

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)

Harbormaster completed remote builds in B212: Diff 438.Mar 31 2023, 20:03

valerio.bozzolan mentioned this in 2023 Week 2.Mar 31 2023, 20:05

Closed by commit rPd25d630fe5dd: PHP 8.2: fixes for strlen() not accepting NULL anymore, part 1. · Explain WhyMar 31 2023, 20:06

This revision was automatically updated to reflect the committed changes.

Restricted Repository Identity added a commit: rPd25d630fe5dd: PHP 8.2: fixes for strlen() not accepting NULL anymore, part 1.

avivey mentioned this in 2023 Week 17.Apr 25 2023, 06:54

valerio.bozzolan mentioned this in T15616: PHP 8.1 "strlen(null)" exception when setting up Phorge locally.Aug 18 2023, 07:51

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

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 437

src/applications/config/check/PhabricatorStorageSetupCheck.php

src/applications/files/engine/PhabricatorS3FileStorageEngine.php

src/applications/files/storage/PhabricatorFile.php

src/applications/metamta/engine/PhabricatorMailEmailEngine.php

src/applications/notification/client/PhabricatorNotificationServerRef.php

src/applications/repository/engine/PhabricatorRepositoryPullEngine.php

src/applications/repository/storage/PhabricatorRepository.php

src/infrastructure/cluster/PhabricatorDatabaseRef.php

src/infrastructure/env/PhabricatorEnv.php

src/infrastructure/log/PhabricatorSSHLog.php

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

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 437

src/applications/config/check/PhabricatorStorageSetupCheck.php

src/applications/files/engine/PhabricatorS3FileStorageEngine.php

src/applications/files/storage/PhabricatorFile.php

src/applications/metamta/engine/PhabricatorMailEmailEngine.php

src/applications/notification/client/PhabricatorNotificationServerRef.php

src/applications/repository/engine/PhabricatorRepositoryPullEngine.php

src/applications/repository/storage/PhabricatorRepository.php

src/infrastructure/cluster/PhabricatorDatabaseRef.php

src/infrastructure/env/PhabricatorEnv.php

src/infrastructure/log/PhabricatorSSHLog.php

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

Revision Contents
Changeset List