WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.56k stars 488 forks source link

Allow different text-domains per subdirectory #1650

Open GaryJones opened 5 years ago

GaryJones commented 5 years ago

Is your feature request related to a problem?

In a site-level directory, there may be different directories of custom code - plugins and theme(s). It's not possible to set a text_domain property on the top-level .phpcs.xml.dist, since it wouldn't be correct for some of the directories.

For a single plugin or theme repo, one would do something like this:

<rule ref="WordPress.WP.I18n">
    <properties>
        <property name="text_domain" type="array">
            <!-- This value is the theme text domain. -->
            <element value="my-child-theme"/>
        </property>
    </properties>
</rule>

But that doesn't work for a site repo.

One workaround, is to add a PHPCS config file at the theme or plugin directory level, with the above, and <rule ref="../../.phpcs.xml.dist"/> so it runs all of the other desired checks, but that means having to cd into each directory which has it's own text domain, and run PHPCS. With clients (former and current) that have 20+ plugins, this isn't feasible without further scripting.

Describe the solution you'd like

I don't know if PHPCS exposes the full filepath such that its available to the WPCS Sniff.php, but if so, I'd like to see support for specifying the file path to which the text domain would apply. Something like this in the top-level .phpcs.xml.dist:

<rule ref="WordPress.WP.I18n">
    <properties>
        <property name="text_domain" type="array">
            <element key="themes/my-child-theme" value="my-child-theme"/>
            <element key="plugins/my-plugin" value="my-plugin"/>
            <element key="plugins/my-other-plugin" value="different-text-domain"/>
        </property>
    </properties>
</rule>

In WordPress.WP.I18n, if there was no key, it would run with the current behaviour. If there was a key, then it would check the relative file path against the key, and if that matched, use the value as the text domain.

Additional context (optional)

This "custom property value per file path match" could be generalised into something supported at the PHPCS-level, but that would be harder to implement to satisfy all potential use cases. Just addressing this at the WPCS level seems doable (if the file path is available to compare against).

jrfnl commented 5 years ago

I'm not sure I really understand the use case for this. WPCS is primarily meant to be run on your own code, not on the code of others which you don't maintain. And when it is run (on the code of others), it's normally run for just one plugin or theme (VIP/TRTCS), not for a whole site. Why would you want to do this ?

All the same, I'm not adverse to this feature request. You can get the filename of the current file being scanned via $phpcsFile->getFileName(), so it is doable too.

I would suggest for the key to allow for plain text and regex patterns, similar to what is proposed in #1623.

I think some thought needs to be put into what the behaviour should be for mixed arrays, i.e. some values have keys, some don't. I also think some thought needs to be put into what the behaviour should be when all values are scoped and a file is scanned which doesn't match any of the file patterns.

Well, you get my drift.

I do think this feature request is low prio as there is a (non-scoped) work-around available as the text_domain property is already an array allowing for multiple valid values.

<rule ref="WordPress.WP.I18n">
    <properties>
        <property name="text_domain" type="array">
            <!-- This value is the theme text domain. -->
            <element value="my-child-theme"/>
            <!-- This value is the text domain for your own custom plugin. -->
            <element value="my-plugin"/>
            <!-- This value is the text domain for the parent theme. -->
            <element value="parent-theme"/>
            <!-- This value is the text domain for an external plugin. -->
            <element value="external-plugin"/>
        </property>
    </properties>
</rule>
GaryJones commented 5 years ago

WPCS is primarily meant to be run on your own code, not on the code of others which you don't maintain.

This is exactly the use case I've described. In this case, the repo looks like:

 - themes
   - my-theme
   - my-child-theme
 - plugins
   - my-plugin/
   - your-plugin/
 - client-mu-plugins/
   - my-mu-plugin/
   - your-mu-plugin/

In this case, I want to be able to check text domains for files in my-theme, my-child-theme, my-plugin and my-mu-plugin directories.

While the workaround you described kinda works, it doesn't catch the case when one of the text domains from my code is used in the wrong directory i.e. category.php is copied from my-theme into my-child-theme (to be adapted), but contains internationalised strings which need the text domain updating.

I agree that the exact scoping and behaviour needs working out - I'll have more of a think. The fact that the approach is possible is the starting point for this.

kraftbj commented 3 years ago

Adding an additional use case for this—a monorepo. We'd like to have a single ruleset for all projects within a monorepo, with the exception of the proper textdomain for each one.