Add sniff to check function prefixes and such

[jrfnl]

I'm thinking of developing a sniff to verify that all global functions, classes, global variables and global constants are prefixed. [Edit]: and hook names ;-)

Obviously this sniff would be irrelevant for core, but I believe it would be a good addition for Extra.

For the sniff to work, a property would need to be set with the expected prefix. I imagine allowing this property to be set both from a custom ruleset as well as from the command-line, similar to how we currently do so for the text_domain in the I18n sniff.

I imagine this sniff would be interesting for both theme as well as plugin devs + the theme review team. /cc @grappler

Opinions ?

[dingo-d]

For the theme review team, the prefixing could be set as the text domain of the theme. In that case you could pull that information out from style.css or even functions.php file (load_theme_textdomain).

In that case we would have to make that a rule in the handbook about prefixing things.

[jrfnl]

the prefixing could be set as the text domain of the theme

@dingo-d I'd been thinking about that and while this may be possible to "demand" from a theme review perspective, the two don't necessarily need to be the same.

Just to use a popular plugin as an example: the text_domain for Yoast SEO is wordpress-seo (also note the dash which would not be allowed in a prefix) and the function prefixes they mostly use are yoast and wpseo.

So for now, I'd like to keep these properties separate.

In that case you could pull that information out from style.css or even functions.php file (load_theme_textdomain).

That's something for the theme check plugin / theme review command line tool, not for the sniff.

In that case we would have to make that a rule in the handbook about prefixing things.

I think this wouldn't be a bad thing. Maybe put it on the agenda for the next theme review board meeting ?

[dingo-d]

@jrfnl I know that text domains don't have to be made from the single line, but I was thinking that maybe, for the themes specifically, we could request that they are the same. That would make things easier for new theme check plugin to use.

In the case of a text domain with hyphens, these could all be converted to underscores (_) and used as a prefix.

I'll ask in the slack channel and see if this can be added to the agenda for next week :)

[grappler]

I have been thinking about this previously.

As a default we can use the root directory name and convert it to underscores. For example my-theme would be my_theme. It would be good to be able to overwrite it (or perhaps extend) with multiple prefixes as the theme or plugin may be using libraries.

It would be interesting to make sure that namespaces are respected and would not be false positives.

For themes I would mark it as an Warning where the function needs to be verified that the function prefix is unique.

[jrfnl]

As a default we can use the root directory name and convert it to underscores. For example my-theme would be my_theme.

That may be possible to do for the Theme Check plugin, but is outside of the scope of the sniff. The sniff should work independently of the directory structure used by the person running the sniff.

Also, especially so for plugins, the directory name may be very long which would create unwieldy function/variable etc names, so it is quite common to use an acronym as a prefix instead. See the above example regarding Yoast SEO.

It would be good to be able to overwrite it (or perhaps extend) with multiple prefixes as the theme or plugin may be using libraries.

Multiple prefixes is already sorted and are being taken into account.

It would be interesting to make sure that namespaces are respected and would not be false positives.

Already taken into account (though will give false positives when run on PHP 5.2 as namespaces are not recognized).

For themes I would mark it as an Warning where the function needs to be verified that the function prefix is unique.

I think it should be an error. Verifying that the function prefix is unique in comparison to other themes/plugins is something which is outside of the ability of PHPCS/WPCS and outside of the scope of this sniff.

[jrfnl]

The sniff for this has now been pulled in PR #907

[justintadlock]

I was just about to point out the need for multiple prefixes due to bundling libraries in plugins/themes. Looks like you got that covered.

I like the direction of this.

[grappler]

Would it make sense to check for prefixing the following items or a new issue and sniff would be better?

• Action/Filter hooks.
• Script handles.
• Style handles.
• Image size names.

[jrfnl]

Action/Filter hooks.

Hooks are already covered by the sniff in the open PR.

The other things might be possible to add a later stage, however, it would need to be very clearly defined what should be checked for in those cases, so please follow up with some code example, reference links etc.

[dingo-d]

I was thinking while fixing issues in my submitted theme - in JS file, if we wrap our code in a closure, inside or outside of $(document).ready(), wouldn't that make our code 'impervious' from any possible prefixing issues? I'm not that good in JS, but if I'm correct, iife's are safe from any outside influence. So in theory, prefixing wouldn't be needed there, right?

[jrfnl]

@dingo-d For JS, I'm pretty sure that's correct, but only if all code is wrapped in a closure.

Just so you know: the current sniff handles only PHP files, not JS files. If you'd like to suggest a similar sniff for JS files and/or adjusting the current sniff to also handle JS, I'd suggest you open a new issue for that (as this one is closed).

[dingo-d]

Yeah, can't really think of anything like that off the top of my head atm. Probably a custom lint that should be made. I'll keep this in mind, as I'll be learning more about JS side of WordPress (REST API) in the future.