Fixes I18NSniff for multi-line gettext calls

[sdnunca]

Currently, there is an issue with the PHPCS Sniff that validates translator comments.
Translator comments usually need to be placed on the line above the gettext call.

However, in the case of multi-line translation function calls, the comment needs to be placed between the line of the function call and the first string.

Here's an example of possible comment locations:

This PR changes the sniff implementation to also take into account the multi-line calls edge case.

[jrfnl]

However, in the case of multi-line translation function calls, the comment needs to be placed between the line of the function call and the first string.

@sdnunca What do you base this on ?

The sniff implementation as-is is based on the PO specs + the implementation of the most commonly used tools. None of which agree with your statement.

[sdnunca]

@jrfnl Thanks for looking into this so quickly. I've encountered this extracting comments with xgettext, as you can see in the screenshot above.

If the call is multi line and the comment is on the line above, the text is not extracted.

[jrfnl]

Related #742

[sdnunca]

I'm not sure if this is particular to gettext but the use-case is also mentioned in their documentation: :
https://www.gnu.org/software/gettext/manual/gettext.html#Operation-mode

[jrfnl]

Ok, interesting. Not sure if that was changed between the original implementation and now, but never mind that.

For this PR to be considered, tests will need to be run with all commonly used tooling to extract translatable strings, like makepot, POEdit, the grunt tooling commonly used, the tooling used by WP Core itself etc.

Someone will need to create an overview of how each of those tools handle the various situations highlighted in the screenshot above. Once that is done, it can be evaluated whether this PR would improve or break compatibility with the various tools.

Only if it would turn out that it would improve compatibility, a full code review of this PR will be done.

@sdnunca Please don't take this push-back the wrong way. I appreciate your initiative and interest in improving WPCS.
However, as WPCS is used by so many projects, we really need to be diligent about changes like this which could break existing integrations.

It might have been better to initially open an issue about this to discuss the problem instead of opening a PR.

[sdnunca]

@jrfnl Again, thank you so much for taking the time to look into this.

It might have been better to initially open an issue about this to discuss the problem instead of opening a PR.

I'll definitely keep that in mind for the future :) . I'll further investigate and get back with my findings on compatibility.

[sdnunca]

Here's the result of my testing.

List of tools checked:

• wp i18n make-pot - also used by WordPress.org
• POEdit (and any other tool based on gettext, such as Loco Translate)
• grunt tooling → grunt-wp-i18n → node-wp-i18n → makepot.php

Test file:

<?php
/*
 * Plugin Name: Test plugin
 * Description: Test description
 * Text Domain: test-plugin
 */

/* translators: number of monkeys, location 1. */
__( 'There are %1$d monkeys in the %2$s 1'
); // Ok, the string is on the same line as the gettext call

__( /* translators: number of monkeys, location 2.*/
	'There are %1$d monkeys in the %2$s 2'
); // OK, comment is on the same line as the gettext call

/* translators: number of monkeys, location 3. */
_n(
	'There are %1$d monkeys in the %2$s 3',
	'There are %1$d monkeys in the %2$s 3'
); // Bad, comment will not be extracted

_n(
/* translators: number of monkeys, location 4. */
	'There are %1$d monkeys in the %2$s 4',
	'There are %1$d monkeys in the %2$s 4'
); // Ok, comment will be extracted

function test_plugin_init() {
    load_plugin_textdomain( 'test-plugin', false, dirname( plugin_basename( __FILE__ ) ) . '/languages' ); 
}
add_action( 'plugins_loaded', 'test_plugin_init' );

1. wp i18n make-pot (also used by core)

Command:

Result:

Conclusion: All locations are extracted by wp i18n make-pot

2. POEdit

Results:

Locations #1, #2, #4 are successfully extracted ✅.

#3 is NOT ❌.

Conclusion: The current implementation is not working with POEdit.

3. Grunt tooling

Testing grunt-wp-i18n

Results:

Locations #2 and #4 are NOT extracted ❌.

Conclusion: grunt-wp-i18n uses the old makepot.php implementation which is no longer used by WordPress.org

Conclusions

As a conclusion for the whole experiment, we will lose compatibility with makepot.php implementation from above and gain compatibility with xgettext based tools.

grunt-wp-i18n is planning to switch to wp i18n make-pot however those efforts have been in progress for a while.

Please let me know if there are any other tools you feel need testing.

[sdnunca]

Hey @jrfnl ,
Just a quick ping :), whenever you get the chance I'd really appreciate your thoughts on the results above.