Skip to content

Add phan and update mediawiki-codesniffer to v48 #465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
Abijeet wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add phan and update mediawiki-codesniffer to v48 #465

Abijeet wants to merge 4 commits into from

Conversation

@Abijeet
Copy link
Member

@Abijeet Abijeet commented Dec 1, 2025 •
edited
Loading

This PR adds the following commits:

Minimum PHP version has been updated to v8.1
Had to drop support for PHP 8.5 to fix CI errors:

 /home/runner/work/language-data/language-data/vendor/phan/phan/src/Phan/CodeBase.php:590 [8192] Method SplObjectStorage::attach() is deprecated since 8.5, use method SplObjectStorage::offsetSet() instead
(Phan 5.5.1 crashed)
More details:
#2: Phan\CodeBase->addInternalFunctionsByNames() called at [/home/runner/work/language-data/language-data/vendor/phan/phan/src/Phan/CodeBase.php:243] Args: [["clone", "exit", "die", "zend_version", "func_num_args", "func_get_arg", "func_get_args", "strlen", "strcmp", "strncmp", ... 2328 more element(s)]]
#3: Phan\CodeBase->__construct() called at [/home/runner/work/language-data/language-data/vendor/phan/phan/src/codebase.php:16] Args: [["InternalIterator", "Exception", "ErrorException", "Error", "CompileError", "ParseError", "TypeError", "ArgumentCountError", "ValueError", "ArithmeticError", ... 429 more element(s)], ["Traversable", "IteratorAggregate", "Iterator", "Serializable", "ArrayAccess", "Countable", "Stringable", "Throwable", "UnitEnum", "BackedEnum", ... 64 more element(s)], ["Phan\\PluginV3\\IssueEmitter"], ["true", "false", "null", "E_ERROR", "E_WARNING", "E_PARSE", "E_NOTICE", "E_CORE_ERROR", "E_CORE_WARNING", "E_COMPILE_ERROR", ... 3569 more element(s)], ["clone", "exit", "die", "zend_version", "func_num_args", "func_get_arg", "func_get_args", "strlen", "strcmp", "strncmp", ... 2328 more element(s)]]

See: https://github.com/wikimedia/language-data/actions/runs/19819690893/job/56779101738?pr=465

Bug: https://phabricator.wikimedia.org/T410000

@Abijeet Abijeet requested review from reedy and wangombe-g December 1, 2025 10:27
@Abijeet Abijeet self-assigned this Dec 1, 2025
@Abijeet Abijeet added the chore label Dec 1, 2025
@Abijeet
Copy link
Member Author

Abijeet commented Dec 1, 2025

Please Rebase and merge this PR into the main branch.

@Abijeet Abijeet requested a review from hueitan December 1, 2025 11:05
@jdforrester
Copy link
Member

jdforrester commented Dec 1, 2025

You shouldn't merge this until Phan 6.x which is PHP 8.5 compatible, as you have found.

reedy
reedy reviewed Dec 1, 2025
View reviewed changes
reedy
reedy reviewed Dec 1, 2025
View reviewed changes
src/util/ulsdata2json.php
print "Reading langdb.yaml...\n";
$yamlLangdb = file_get_contents( DATA_DIRECTORY . '/langdb.yaml' );
// spyc_load is from mustangostang/spyc. Only needed during development.
// @phan-suppress-next-line PhanUndeclaredFunction
Copy link
Member

@reedy reedy Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be a sign that this script doesn't actually want to be in src/

jdforrester
jdforrester requested changes Dec 2, 2025
View reviewed changes
Copy link
Member

@jdforrester jdforrester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said, please back out the Phan changes until 8.5 is working, rather than regress the state.

@Abijeet
Copy link
Member Author

Abijeet commented Dec 3, 2025

You shouldn't merge this until Phan 6.x which is PHP 8.5 compatible, as you have found.

I'd prefer to include Phan now and remove PHP 8.5 compatibility to keep it inline with WMF usage. We can update to Phan 6.x and PHP 8.5 when upstream work is done.

@jdforrester
Copy link
Member

jdforrester commented Dec 4, 2025

You shouldn't merge this until Phan 6.x which is PHP 8.5 compatible, as you have found.

I'd prefer to include Phan now and remove PHP 8.5 compatibility to keep it inline with WMF usage. We can update to Phan 6.x and PHP 8.5 when upstream work is done.

Wikimedia CI will enforce PHP 8.5 soon. So this repo now won't be able to be a library in vendor until Phan 6 is released.

@Abijeet
Copy link
Member Author

Abijeet commented Dec 17, 2025

I submitted: #473 for the non-phan related changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reedy reedy reedy left review comments

@jdforrester jdforrester jdforrester requested changes

@wangombe-g wangombe-g Awaiting requested review from wangombe-g

@hueitan hueitan Awaiting requested review from hueitan

Requested changes must be addressed to merge this pull request.

Assignees

@Abijeet Abijeet

Labels

chore

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Abijeet @jdforrester @reedy @hueitan