Change 926679 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/tools/phan@master] [WIP] Add plugin to detect dynamic message keys

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

There are two main things that would have to be decided before implementing the sniff.

First: what kind of documentation comment should it be looking for? The eslint version simply looks for a message containing a list (lines starting with "*"). Another option would be to force one or more predefined headers (e.g., "the following messages are used here:") and then not care about the rest of the comment. I think each option has its pros and cons: the list version would be more consistent with JS and allow more flexibility; the predefined header version would work better when there can be many short messages for which a comma-separated list would suffice, it would guarantee consistent documentation, and it would allow us to optimize the plugin (phan doesn't read comments by default, so we need to tokenize the whole file to find them, which could be skipped altogether if the file doesn't contain the predefined header).

Second: do we want to allow the comment in a parent node, or should it be immediately before the call node? This doesn't make any difference for simple calls, but consider code like this:

// L1
$html = Html::openElement( 'table' ) .
    Html::element(
        'tr',
        [],
        Html::element(
            'td',
            [],
            // L2
            wfMessage( $msg )
        )
    ) .
    Html::closeElement( 'table );

should we require that the comment be at L2, or should we also accept it if it's at L1? Accepting L1 gives the developer more flexibility, and is what the eslint version does. However, the implementation would be slightly more expensive (need to iterate over the parent nodes). Also, and most importantly, if the statement is really long (like building some complex HTML table this way), the comment and the call would be far apart, which would make the comment less useful. On top of that, things would get worse if the long statement contains more than one call with dynamic messages.

In T338091#8900888, @Daimona wrote:

There are two main things that would have to be decided before implementing the sniff.

First: what kind of documentation comment should it be looking for? The eslint version simply looks for a message containing a list (lines starting with "*"). Another option would be to force one or more predefined headers (e.g., "the following messages are used here:") and then not care about the rest of the comment. I think each option has its pros and cons: the list version would be more consistent with JS and allow more flexibility; the predefined header version would work better when there can be many short messages for which a comma-separated list would suffice, it would guarantee consistent documentation, and it would allow us to optimize the plugin (phan doesn't read comments by default, so we need to tokenize the whole file to find them, which could be skipped altogether if the file doesn't contain the predefined header).

Happy for it solely to assert that a list exists.

Second: do we want to allow the comment in a parent node, or should it be immediately before the call node? This doesn't make any difference for simple calls, but consider code like this:

// L1
$html = Html::openElement( 'table' ) .
    Html::element(
        'tr',
        [],
        Html::element(
            'td',
            [],
            // L2
            wfMessage( $msg )
        )
    ) .
    Html::closeElement( 'table );

should we require that the comment be at L2, or should we also accept it if it's at L1? Accepting L1 gives the developer more flexibility, and is what the eslint version does. However, the implementation would be slightly more expensive (need to iterate over the parent nodes). Also, and most importantly, if the statement is really long (like building some complex HTML table this way), the comment and the call would be far apart, which would make the comment less useful. On top of that, things would get worse if the long statement contains more than one call with dynamic messages.

Accepting L1 would be great, yes.

I think just to get the ball rolling, we could:

• Require a list (i.e, at least two lines starting with asterisks, regardless of content)
• Only allow comments on the immediately preceding line. Otherwise it could get messy if you have multiple wfMessage-like calls in the same statement.

In T338091#11879434, @Daimona wrote:

• Require a list (i.e, at least two lines starting with asterisks, regardless of content)

Actually, I would rather go back to my initial implementation of requiring a fixed string introducing the list ("The following messages are used here:", so that these dynamic usages are easier to find and catalog in future (e.g. if we need it in for T224429).