In T224429#5436013, @Simetrical wrote:

We definitely should have a test that checks for this. Any existing cases can be whitelisted for the time being, but it could at least stop new cases from being inadvertently added.

Yes, that'd be great.

Given that sometimes we dynamically create message name strings, one way to do this would be to have PHPUnit tests generate a list of all the messages that are used over the course of the test.

The idea would be good, but:

(Of course, anyone who adds a new message will then have to add a new test that uses the message, but they should be doing that anyway!)

I bet there are tons of messages not used in any test.

Another proposal is to have a smarter alternative to a lame grep that can collect all of the strings passed to wfMessage and the like. However, there's a trade-off between speed (e.g. simple grep like in the task description) and precision (e.g. static analysis, which could also read variable values etc.).
We could also use more precise regexps, without having to rely on static analysis. Of course, this would still fail for legoed/dynamic messages, but IMHO that's good. Adding at least a comment with the possible keys is not only something that devs should be doing anyway, but that we can also enforce brutally, without asking too much (like writing a test, which could end up being a bad test).

I think best would be to require annotating all dynamic message uses (near the place where they are used) with a specially formatted comment. We may need to support patterns like logentry-*-* unless we are okay having these comments somewhere else than near the wfMessage call.

I took a quick look at the ones containing "api".

apisandbox
apisandbox-summary

Used by SpecialApiSandbox via SpecialPage::getDescription() and SpecialPage::outputHeader().

apihelp-summary

Same for SpecialApiHelp. Although in this case I don't know offhand of a way to actually cause it to be used.

BTW, your grep seems to have missed the corresponding "apihelp" message.

apisandbox-api-disabled

Legitimate, missed for removal in rMW225b462a50b5: Drop deprecated EnableAPI and EnableWriteAPI settings. Looks like it was being removed in PS5 but the removal got lost in a rebase in PS7.

api-error-badtoken
api-error-emptypage
api-error-unknownerror

Added originally in rMWd683e249756e: Remove API error message duplication from Translate and UploadWizard into core., which seems self-explanatory. Looks like Translate still uses all three. api-error-badtoken seems to have been removed from UploadWizard in {baa7dadef8}; if the other two were ever used, I can't find it.

In T224429#5437551, @Nikerabbit wrote:

I think best would be to require annotating all dynamic message uses (near the place where they are used) with a specially formatted comment. We may need to support patterns like logentry-*-* unless we are okay having these comments somewhere else than near the wfMessage call.

The unwritten convention is VE code is something along the lines of:

// The following messages are used here:
// * visualeditor-mweditmodewt-popup-body
// * visualeditor-mweditmodeve-popup-body
$content = $( '<p>' ).text( mw.msg( prefix + '-popup-body' ) );

With a well defined format, we could probably write an eslint rule for this.

Change 532818 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/core@master] Follow-up 225b462: Remove 'apisandbox-api-disabled' i18n too

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

Change 532818 merged by jenkins-bot:
[mediawiki/core@master] Follow-up 225b462: Remove 'apisandbox-api-disabled' i18n too

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

In T224429#5443344, @Esanders wrote:

With a well defined format, we could probably write an eslint rule for this.

Is the format you showed simple enough and well defined? I'd imagine it would need a code that find a comment block that matches /the following message.*:/i and then take the stuff after the colon, explode on whitespace and strip lonely * and - to allow multiline lists, and strip trailing , to allow inline lists.

https://codesearch.wmflabs.org/search/?q=(%2F%2F%7C%5C*).%2Bthe%20following%20message&i=fosho&files=(js%7Cphp)%24&repos= confirms this pattern is frequently used and likely without false positives.

Change 532965 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Remove unused localisation messages for old edit toolbar

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

Change 532969 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Remove unused localisation messages 'ascending_abbrev', 'descending_abbrev'

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

Change 532971 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Remove unused localisation message 'wlshowlast'

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

I looked briefly at a few areas I'm familiar with.

Your list has some false positives:

• These messages are used in includes/pager/TablePager.php and documented in code comments there:
  • table_pager_next
  • table_pager_prev
  • table_pager_first
  • table_pager_last
• These messages are used in includes/specialpage/ChangesListSpecialPage.php and documented in code comments there:
  • wlshowhideminor
  • wlshowhidebots
  • wlshowhideliu
  • wlshowhideanons
  • wlshowhidepatr
  • wlshowhidemine
  • wlshowhidecategorization

In T224429#5444638, @Nikerabbit wrote:

In T224429#5443344, @Esanders wrote:

With a well defined format, we could probably write an eslint rule for this.

Is the format you showed simple enough and well defined? I'd imagine it would need a code that find a comment block that matches /the following message.*:/i and then take the stuff after the colon, explode on whitespace and strip lonely * and - to allow multiline lists, and strip trailing , to allow inline lists.

https://codesearch.wmflabs.org/search/?q=(%2F%2F%7C%5C*).%2Bthe%20following%20message&i=fosho&files=(js%7Cphp)%24&repos= confirms this pattern is frequently used and likely without false positives.

Perhaps, although we may want to make it more obviously structured, by using a magic word before the list (e.g. // messages-keys-used) or in it (e.g. // * MSG:visualeditor-mweditmodewt-popup-body). This would make it more obvious the devs that the format should not be changed, and would make it a bit easier to parse.

Change 532969 merged by jenkins-bot:
[mediawiki/core@master] Remove unused localisation messages 'ascending_abbrev', 'descending_abbrev'

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

Change 532971 merged by jenkins-bot:
[mediawiki/core@master] Remove unused localisation message 'wlshowlast'

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

Change 532965 merged by jenkins-bot:
[mediawiki/core@master] Remove unused localisation messages for old edit toolbar

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

T235502 creates an eslint plugin to ensure that message building code in JS is commented with at least 2 'msg:' items. As this is not a runtime analysis we can't know exactly what messages are being used, but ensuring some documentation should be an improvement.

I was thinking about this again today. I'd love to have something similar to the eslint plugin for PHP code. It wouldn't have to be perfect: for a start, just report every message key who doesn't appear when grepping the codebase, regardless of where it actually appears. This will likely have many false negatives, but I don't think it will have (many) false positive, as long as we require that every message be documented in a comment like in JS (and no need to enforce a specific syntax just yet). It could be a non-voting job and later be made voting. Better than nothing, I'd say.

I looked at it from a different perspective. I checked every "unused" i18n message in git history to find the last seen in code. This resulted in ~80 messages possibly being used explicitly up to some point. That commit helps figure out if the message became unused or became implicit (and where) so you can improve grep-ability easily.

Running the code for this took a whole day 😅

Change 774005 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/core@master] Drop limitreport-ppgeneratednodes message

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

Change 774006 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/core@master] Drop djvu_no_xml i18n message

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

Change 774007 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/core@master] Drop two unused Enhanced RC i18n messages

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

Change 774006 merged by jenkins-bot:

[mediawiki/core@master] Drop djvu_no_xml i18n message

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

Change 774007 merged by jenkins-bot:

[mediawiki/core@master] Drop two unused Enhanced RC i18n messages

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

Change 776296 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/core@master] Drop unused directorynotreadableerror message

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

Change 776297 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/core@master] Remove unsued youhavenewmessagesmulti and newtalkseparator messages

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

Change 776296 merged by jenkins-bot:

[mediawiki/core@master] Drop unused directorynotreadableerror message

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

Change 776297 merged by jenkins-bot:

[mediawiki/core@master] Remove unsued youhavenewmessagesmulti and newtalkseparator messages

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

Hi @Tgr There are 24 unused i18n messages found that were referenced last in 854a462dc0aef59e1c26c057792b8a9214449af5 (removing pre-auth manager code). See T224429#7809087. Can you take a look and remove the ones that you're sure not needed anymore?

Change 778677 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/core@master] Remove unused i18n message of removed CacheHelper classes

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

Change 778677 merged by jenkins-bot:

[mediawiki/core@master] Remove unused i18n message of removed CacheHelper classes

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

In T224429#7843721, @Ladsgroup wrote:

Hi @Tgr There are 24 unused i18n messages found that were referenced last in 854a462dc0aef59e1c26c057792b8a9214449af5 (removing pre-auth manager code). See T224429#7809087. Can you take a look and remove the ones that you're sure not needed anymore?

Something seems wrong with the script (git log --grep instead of git log -S?), those messages aren't removed in that patch.

It was git log -S, Are you saying it should be git log --grep? I will double check it in the weekend.

@Tgr Are you sure they are not deleted in that patch? for example. createacct-submit was in includes/templates/Usercreate.php and the whole file got deleted in that patch (without re-adding):

amir@amir-ThinkPad-P1-Gen-3:~/core$ git diff 854a462dc0aef59e1c26c057792b8a9214449af5^ 854a462dc0aef59e1c26c057792b8a9214449af5 | grep -i createacct-submit
-					$this->getMsg( $this->data['loggedin'] ? 'createacct-another-submit' : 'createacct-submit' ),

Unless you made it dynamic loading.

createacct-submit is used on https://en.wikipedia.org/wiki/Special:CreateAccount?uselang=qqx right now. The message key comes from here: https://gerrit.wikimedia.org/g/mediawiki/core/+/7ab4f666be6dd31ed552fc4575c7b99571407857/includes/specialpage/LoginSignupSpecialPage.php#869 and it's indeed constructed dynamically. It's probably similar for many other messages used on login/signup pages.

Change 774005 merged by jenkins-bot:

[mediawiki/core@master] Drop limitreport-ppgeneratednodes message

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

Change 805788 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/core@master] LoginSignupSpecialPage: Document used message keys

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

Change 805788 merged by jenkins-bot:

[mediawiki/core@master] LoginSignupSpecialPage: Document used message keys

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

Change 805878 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/core@master] SkinTemplate: Document more nstab- message keys

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

Change 805878 merged by jenkins-bot:

[mediawiki/core@master] SkinTemplate: Document more nstab- message keys

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

I'm working on a phan plugin to detect dyamic message usage in PHP, see subtask. There are a few details that need to be hashed out in T338091#8900888. Suggestions are welcome :)

I wrote a script to search the current codebase for existing messages, either in the code or documentation:

https://gerrit.wikimedia.org/r/c/VisualEditor/VisualEditor/+/971262

It can be run on any extension with a bit of tweaking. It would be nice to make it generic and configurable, then it can be part of CI.

Also for missing messages, checking if they ever appeared in the git history, as done by above, would be useful.

Example output:

Change 532072 abandoned by Ladsgroup:

[mediawiki/core@master] Remove mediastatistics i18n message

Reason:

The i18n is still needed

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

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

[mediawiki/core@master] Remove unused message filehist-filesize

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

Change 1005616 merged by jenkins-bot:

[mediawiki/core@master] Remove unused message filehist-filesize

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

oldpassword seems unused since https://gerrit.wikimedia.org/r/c/mediawiki/core/+/280945

Change #1049937 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/core@master] Remove unused message 'createaccount-hook-aborted'

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

Change #1049997 had a related patch set uploaded (by Raimond Spekking; author: Raimond Spekking):

[translatewiki@master] [Core] Remove a deleted key

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

Change #1049997 merged by jenkins-bot:

[translatewiki@master] [Core] Remove a deleted key

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

Change #1049937 merged by jenkins-bot:

[mediawiki/core@master] Remove unused message 'createaccount-hook-aborted'

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

Change #1112681 had a related patch set uploaded (by Winston Sung; author: Winston Sung):

[mediawiki/core@master] i18n: Remove unused system message "prefs-labs"

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

Change #1112681 merged by jenkins-bot:

[mediawiki/core@master] i18n: Remove unused system message "prefs-labs"

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

Change #1160291 had a related patch set uploaded (by ToluAyo; author: ToluAyo):

[translatewiki@master] Remove specialpages-note-cached and sitenotice_id from MediaWiki.yaml

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

Change #1160291 merged by jenkins-bot:

[translatewiki@master] Remove specialpages-note-cached and sitenotice_id from MediaWiki.yaml

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

Change #1160361 had a related patch set uploaded (by ToluAyo; author: ToluAyo):

[translatewiki@master] Remove unused messages from MediaWiki.yaml

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

Change #1160361 merged by jenkins-bot:

[translatewiki@master] Remove unused messages from MediaWiki.yaml

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