test: let phan see Translate instead of looking away from it - #468
Merged
Conversation
Three files told phan to ignore whole classes of issue because Translate is not installed where phan runs, and a file-wide suppression is wider than the hole it covers. Naming Translate in the phan config closes the hole instead, and needs no workflow change: quibble-action resolves what to clone from the first source that yields anything, and with no `requires.extensions` in extension.json that source is this directory list. It is how Gadgets already gets there. Measured, not assumed, by running Wikimedia's own phan image over the tree the wikven image carries (MediaWiki 1.46.0, Translate 2025-07-31): * without Translate in the config and with no suppressions: 15 issues, all of them in these three files; * with Translate: none. The two halves belong in one commit. Resolve Translate while the suppressions stay and phan reports `UnusedPluginFileSuppression` for what is now dead. `checkTranslations.php` is not named in #459 and carries the same two suppressions for the same reason, so it goes with them. What the suppression cost is narrower than #459 says, and worth stating accurately. A typo'd method on a class phan can see is `PhanUndeclaredMethod`, which was never suppressed. What `PhanUndeclaredClassMethod` hid is any call on a class phan cannot see at all. Planting one in `buildTranslations.php` and running both ways: reported now, and under the old header silent, with phan noting only that the *other three* suppressions were unused. The one that mattered still counted as used, because the bug it was hiding was using it. Closes #459. --- _Generated by [Claude Code](https://claude.ai/code/session_935f02d1)_ Co-authored-by: Claude <noreply@anthropic.com>
lens0021
force-pushed
the
claude/phan-sees-translate
branch
from
August 18, 2026 14:25
524a39d to
82d3cde
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three files told phan to ignore whole classes of issue because Translate is not resolved where phan runs. A file-wide suppression is wider than the hole it covers, so the hole gets closed instead: naming Translate in the phan config resolves the symbols, and all three files end up carrying no suppression at all.
Measured, not assumed
Ran Wikimedia's own phan image over the tree the wikven image carries (MediaWiki 1.46.0, Translate 2025-07-31), which is the same layout CI builds:
The 15 are exactly the shapes named in #459:
PhanUndeclaredClassMethod,PhanUndeclaredClassConstant,PhanUndeclaredConstant(TRANSLATE_FUZZY,NS_TRANSLATIONS) andPhanUndeclaredClassCatch.The two halves belong together
Resolve Translate while the suppressions stay and phan reports
UnusedPluginFileSuppressionfor what has become dead. So this could not have been split without CI going red in between.It also moved what CI installs, which is why #475 goes first
quibble-action resolves what to clone and install from the first source that answers: the
dependenciesinput, thenrequires.extensionsin extension.json, then the phan config's directory list. wikven named no extensions inrequires, so the phan config was the source in use — that is how Gadgets was already being cloned, and it means naming Translate here hands it to the installer too. MediaWiki refuses to install Translate without UniversalLanguageSelector, and thecoveragejob failed on exactly that:One file was answering two questions that have different answers. phan wants the directories whose symbols this code reaches for; the wiki wants whatever will not load without it. Translate's own phan config draws the same line — it names AbuseFilter, AdminLinks, cldr, Echo, Elastica, Scribunto and TranslationNotifications, the optional integrations it reaches for, and does not name ULS, its one hard
requires.extensionsdependency.#475 separates them: the install list is declared in
quibble.yml, where CI can be asked about it, and this file goes back to being about phan. It landed first, and this pull request went green on it without changing.Correcting #459 on what the suppression cost
The issue says a typo'd method name in
buildTranslations.phpwent unreported. Not quite: a typo on a class phan can see isPhanUndeclaredMethod, which was never suppressed. WhatPhanUndeclaredClassMethodhid is a call on a class phan cannot see at all.Demonstrated by planting one and running it both ways. With the suppressions gone:
With the old header back and the same bug in place, phan says nothing about it, and reports only that the other three suppressions are unused. The one that mattered still counted as used, because the bug it was hiding was using it. That is the failure mode worth naming: a stale suppression is kept alive by exactly what it hides.
Scope
maintenance/checkTranslations.phpis not named in #459 and carries the same two suppressions for the same reason, so it goes with them.SifterSearch, which #459 expected to be the hard half, turns out not to need anything:
Indexerdeliberately does not implementSifterSearchIndexPageHook(see its class comment, from #458), and the only mention left is a@seein a docblock, which phan does not resolve. Nothing here references a SifterSearch symbol.Checked outside CI
The unit suite, on this branch:
OK (185 tests, 281 assertions). PHPUnit 9.6.34 fromphar.phpunit.de, since MediaWiki's dev dependencies would not install while codeload was answering 429. Worth checking because #458 taught it: a docblock is not always inert, andtestValidCoversis the test that noticed last time.Not reproducible locally: the REL1_46 and master phan runs as CI does them. This machine has no php-ast, so phan ran inside Wikimedia's phan image against one MediaWiki version, and CI covered the other two.
Closes #459.