Page MenuHomePhabricator

Use php strict types in wikimedia maintained php libraries
Open, Needs TriagePublic

Description

Libraries maintenanced by wikimedia should declare strict types to ensure correct types are used on function calls to php internal functions or on function calls within the library.

This is about adding declare( strict_types = 1 ); to all php files of the libraries and enable the Generic.PHP.RequireStrictTypes sniff of the squizlabs/php_codesniffer package (via mediawiki/mediawiki-codesniffer).

Declaring strict types means that PHP will no longer automatically cast wrong scalar variables to the expected type, functions will require the correct type to be called.
Strict typing applies to function calls made from within the file with strict typing enabled, not to the functions declared within that file. If a file without strict typing enabled makes a call to a function that was defined in a file with strict typing, the caller's preference (weak typing) will be respected, and the value will be coerced.

This can catch subtle bugs on runtime (by running tests), that are not catched by phan already (stricter mode possible, see T385060). Libraries with a good code coverage benefits more from this approch for the runtime.

There is a small risk that the library throws TypeError for internal typed functions for untested scenario after a new version is released/deployed, but that is not a breaking change.
There is no performance impact as the type check already is done, but instead of doing a cast, now a TypeError is thrown.
There could be a performance benefit (on language level) on newer php versions that use the type information to remove extra checks (as part of a JIT).

This is indented to be done as a mass bulk change. There is no need to build a release for each of this library as this is only for internal calls and can be released on the usually release plan of the affected library (if any release plan exists).

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
mediawiki/libs/RemexHtmlmaster+106 -0
mediawiki/libs/less.phpmaster+131 -0
mediawiki/libs/Shellboxmaster+86 -0
mediawiki/libs/IDLeDOMmain+558 -0
design/codex-phpmain+133 -0
css-sanitizermaster+253 -0
mediawiki/tools/codesniffermaster+128 -0
mediawiki/libs/Dodomaster+11 -0
mediawiki/libs/IPUtilsmaster+20 -0
mediawiki/libs/Messagemaster+34 -0
mediawiki/libs/ParamValidatormaster+40 -0
mediawiki/libs/IDLeDOMmain+17 -0
purtlemaster+23 -0
mediawiki/libs/php-cssjanusmaster+8 -0
wikipegmaster+15 -0
AhoCorasickmaster+13 -0
mediawiki/libs/LangConvmaster+26 -0
mediawiki/libs/XMPReadermaster+12 -0
cdbmaster+35 -0
mediawiki/libs/Minifymaster+41 -0
mediawiki/libs/JsonCodecmain+38 -0
mediawiki/oauthclient-phpmaster+41 -0
utfnormalmaster+20 -0
wikimedia/lucene-explain-parsermaster+22 -0
CLDRPluralRuleParsermaster+18 -0
mediawiki/libs/RequestTimeoutmaster+16 -0
mediawiki/libs/Servicesmaster+26 -0
mediawiki/libs/CommonPasswordsmaster+7 -0
base-convertmaster+6 -0
mediawiki/libs/WebIDLmain+6 -0
mediawiki/libs/Timestampmaster+14 -0
mediawiki/libs/CloverDiffmaster+16 -0
mediawiki/services/jobrunnermaster+8 -0
mediawiki/libs/Zestmaster+8 -0
performance/excimer-ui-clientmaster+4 -0
mediawiki/libs/UpdateHistorymain+4 -0
performance/excimer-ui-servermaster+5 -0
mediawiki/libs/ScopedCallbackmaster+6 -0
mediawiki/libs/ObjectFactorymaster+8 -0
mediawiki/libs/Bcp47Codemain+8 -0
mediawiki/libs/NormalizedExceptionmaster+7 -0
mediawiki/libs/WaitConditionLoopmaster+9 -0
mediawiki/tools/phpunit-patch-coveragemaster+29 -0
mediawiki/tools/minus-xmaster+5 -0
Show related patches Customize query in gerrit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change #1267944 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[design/codex-php@main] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267944

Change #1264732 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/Bcp47Code@main] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1264732

Change #1267818 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[base-convert@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267818

Change #1267824 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/CommonPasswords@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267824

Should we create sub-tasks for each of the affected libraries that aren't yet compliant, so we can track progress without this task becoming overloaded and hard to read?

Change #1267828 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/ObjectFactory@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267828

Change #1267829 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/RequestTimeout@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267829

Change #1267836 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/UpdateHistory@main] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267836

Change #1267835 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/Services@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267835

Change #1267837 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/WaitConditionLoop@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267837

Change #1267842 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/Timestamp@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267842

Change #1267831 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/ScopedCallback@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267831

Change #1267825 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/php-cssjanus@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267825

Change #1267827 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/NormalizedException@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267827

Change #1267840 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[css-sanitizer@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267840

Change #1267926 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[CLDRPluralRuleParser@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267926

Change #1267928 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/IPUtils@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267928

Change #1267930 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/JsonCodec@main] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267930

Change #1267933 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/Minify@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267933

Change #1267932 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[wikimedia/lucene-explain-parser@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267932

Change #1267931 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/LangConv@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267931

Change #1267937 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[utfnormal@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267937

Change #1267939 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/XMPReader@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267939

Change #1267940 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/Zest@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267940

Change #1267938 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[wikipeg@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267938

Change #1267945 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/IDLeDOM@main] Declare strict types on generated php files

https://gerrit.wikimedia.org/r/1267945

Change #1267942 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/IDLeDOM@main] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267942

Change #1267925 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[cdb@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267925

Change #1267941 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/Shellbox@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267941

Change #1267943 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/RemexHtml@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267943

Change #1267954 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/tools/minus-x@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267954

Change #1267957 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/CloverDiff@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267957

Change #1267961 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/tools/codesniffer@master] chore: Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267961

Change #1267969 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[AhoCorasick@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267969

Change #1267970 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[performance/excimer-ui-client@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267970

Change #1267971 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[performance/excimer-ui-server@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267971

Change #1267972 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/Message@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267972

Change #1267973 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/ParamValidator@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267973

Change #1267974 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/tools/phpunit-patch-coverage@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267974

Change #1267975 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/WebIDL@main] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267975

Change #1267977 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/services/jobrunner@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267977

Change #1267986 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/Dodo@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267986

Change #1267988 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/libs/less.php@master] chore: Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267988

Change #1267934 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[purtle@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267934

Change #1267976 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/oauthclient-php@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267976

Should we create sub-tasks for each of the affected libraries that aren't yet compliant, so we can track progress without this task becoming overloaded and hard to read?

It is a long list of affected libraries, maybe the libraries with active development should get or needs a sub-task to have the correct projects and on the correct board for visibility, not sure.

Change #1267954 merged by jenkins-bot:

[mediawiki/tools/minus-x@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267954

Reedy renamed this task from Use php strict types in wikimedia maintenanced php libraries to Use php strict types in wikimedia maintained php libraries.Wed, Apr 8, 6:33 PM

Change #1267974 merged by jenkins-bot:

[mediawiki/tools/phpunit-patch-coverage@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267974

Change #1267837 merged by jenkins-bot:

[mediawiki/libs/WaitConditionLoop@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267837

Change #1267827 merged by jenkins-bot:

[mediawiki/libs/NormalizedException@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267827

Change #1264732 merged by jenkins-bot:

[mediawiki/libs/Bcp47Code@main] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1264732

Change #1267828 merged by jenkins-bot:

[mediawiki/libs/ObjectFactory@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267828

Change #1267836 merged by jenkins-bot:

[mediawiki/libs/UpdateHistory@main] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267836

Change #1267831 merged by jenkins-bot:

[mediawiki/libs/ScopedCallback@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267831

Change #1267971 merged by jenkins-bot:

[performance/excimer-ui-server@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267971

Change #1267970 merged by jenkins-bot:

[performance/excimer-ui-client@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267970

Change #1267940 merged by jenkins-bot:

[mediawiki/libs/Zest@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267940

Change #1267977 merged by jenkins-bot:

[mediawiki/services/jobrunner@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267977

Change #1267957 merged by jenkins-bot:

[mediawiki/libs/CloverDiff@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267957

Change #1267842 merged by jenkins-bot:

[mediawiki/libs/Timestamp@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267842

Change #1267975 merged by jenkins-bot:

[mediawiki/libs/WebIDL@main] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267975

Change #1267818 merged by jenkins-bot:

[base-convert@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267818

Change #1267824 merged by jenkins-bot:

[mediawiki/libs/CommonPasswords@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267824

Change #1267835 merged by jenkins-bot:

[mediawiki/libs/Services@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267835

Change #1267829 merged by jenkins-bot:

[mediawiki/libs/RequestTimeout@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267829

Change #1267926 merged by jenkins-bot:

[CLDRPluralRuleParser@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267926

Change #1267937 merged by jenkins-bot:

[utfnormal@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267937

Change #1267976 merged by jenkins-bot:

[mediawiki/oauthclient-php@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267976

Change #1267930 merged by jenkins-bot:

[mediawiki/libs/JsonCodec@main] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267930

Change #1267933 merged by jenkins-bot:

[mediawiki/libs/Minify@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267933

Change #1267925 merged by jenkins-bot:

[cdb@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267925

Change #1267939 merged by jenkins-bot:

[mediawiki/libs/XMPReader@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267939

Change #1267931 merged by jenkins-bot:

[mediawiki/libs/LangConv@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267931

Change #1267932 merged by jenkins-bot:

[wikimedia/lucene-explain-parser@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267932

Change #1267969 merged by jenkins-bot:

[AhoCorasick@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267969

Change #1267972 merged by jenkins-bot:

[mediawiki/libs/Message@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267972

Change #1267938 merged by jenkins-bot:

[wikipeg@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267938

Change #1267825 merged by jenkins-bot:

[mediawiki/libs/php-cssjanus@master] Declare strict types on all php files

https://gerrit.wikimedia.org/r/1267825

Skipped libraries are:

Not libraries, but non-MediaWiki code with php on gerrit:

The sniff should be added to the cookiecutter repo (for libraries?), to ensure new libraries using this as default (ref T330014: Create a new codesniffer ruleset to make usage for libraries easier)

php code on github could be handled also as part of this task:

  • composer-merge-plugin
  • fundraising related repos
  • ...

php code on gitlab could be handled also as part of this task:

  • metrics-platform
  • ...

Skipped libraries are:

  • php-session-serializer, maybe obsolete, but reference not found

wikimedia/php-session-serializer==3.0.2 is an active Composer dependency for MediaWiki.

Skipped libraries are:

  • php-session-serializer, maybe obsolete, but reference not found

wikimedia/php-session-serializer==3.0.2 is an active Composer dependency for MediaWiki.

I have seen T404636: Deprecate PHPSessionHandler and $wgPHPSessionHandling and assumed the library is not needed any more in the near future as PhpSessionSerializer is only used in deprecated PHPSessionHandler, so treating the library as obsolete for this task.