WordPress.Files.FileName.UnderscoresNotAllowed and the template hierarchy

[mboynes]

Currently, this standard will throw an error if a file is encountered whose filename contains an underscore:

 1 | ERROR | [ ] Filename "taxonomy-post_format-post-format-link.php"
   |       |     with underscores found; use
   |       |     taxonomy-post-format-post-format-link.php instead
   |       |     (WordPress.Files.FileName.UnderscoresNotAllowed)

Technically, this is correct. The WordPress coding standards states that "hyphens should separate words" in file names. However, this may conflict with the template hierarchy's requirements, which use post types and taxonomies (as well as page/post slugs, author nicenames, mime types, and more) as part of the file name. For instance, from the docs on the template hierarchy,

In the case of post formats, the taxonomy is ‘post_format’ and the terms are ‘post-format-{format}. i.e. taxonomy-post_format-post-format-link.php for the link post format.

Therefore, the file unavoidably named taxonomy-post_format-post-format-link.php will throw an error as in the example above. I understand that exclusions can be added to the ruleset, but I think the question should still be raised: Should template hierarchy files be excepted from this sniff?

[JDGrimes]

Is it possible to craft a general set of exclusions that can reliably detect template files?

[westonruter]

I think we could whitelist template files based on known patterns. For instance, before reporting an error we could check if it matches the file matches the pattern /taxonomy-.+-post-format-link\.php/, along with the other possible template names that have slugs in them (e.g. single-restaurant_location.php or archive-club_member.php).

[GaryJones]

I've not checked, but is WP clever enough to apply taxonomy-post-format-post-format-link.php to the post_format post type? Not something I've seen before, but might be sanitized in that way.

[GaryJones]

• ~single-.*\.php~
• ~archive-.*\.php~
• ~taxonomy-.*\.php~
• ~embed-.*\.php~

Which gives ~(archive|embed|single|taxonomy)-.*\.php~.

The one that'll through a spanner in the works is {$mimetype}_{$subtype}.php i.e. text/plain attachment could be served via text_plain.php. According to this, the currently registered top-level types are: application, audio, example, image, message, model, multipart, text, video. So that gives us:

• ~(application|audio|example|image|message|model|multipart|text|video)_.*\.php~

And, I think, overall gives an exclusion pattern of:

~((archive|embed|single|taxonomy)-)|((application|audio|example|image|message|model|multipart|text|video)_).*\.php~

If that looks right, I can add the exclusion, though I'm not sure of the best way of adding unit tests for filename sniffs - do I need to create files named after each exclusion, and ensure they don't throw an error?

[JDGrimes]

do I need to create files named after each exclusion, and ensure they don't throw an error?

Yes, I guess so. See https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/PSR2/Tests/Files/ClosingTagUnitTest.php for an example of how tests for multiple files is done.

[jrfnl]

Sorry to butt in, but you might find these closer to what you are actually looking for: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Squiz/Sniffs/Files/FileExtensionSniff.php https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Squiz/Tests/Files/FileExtensionUnitTest.php

[jrfnl]

Problem with the unit tests if that they look for files with the same filename as the sniff but then with UnitTest, so you may need to devise a whole new way of testing this (PHPCompatibility redid the unit testing framework to suit them, but that's not fool-proof either).

[GaryJones]

Pinging @gsherwood, in case this is something that would also/first needs to be solved at the PHPCS level.

[gsherwood]

Pinging @gsherwood, in case this is something that would also/first needs to be solved at the PHPCS level.

If you're pinging me about the unit test filenames, there isn't any way to create random file names and have the sniffs test them. I imagine it would be possible to have sniff files register additional test files that they want to use and have the test runner pick them up, but I really don't have any spare time to look into this at the moment.

Edit: forgot to say: submit a feature request to the project and at least I wont forget about it.

[GaryJones]

PHPCS (develop branch) now supports allowing unit tests to decide which test file names to support. I've not yet looked through the change but the code in another sniff looks simple enough.

It will require PHPCS 2.7.0 though.

[jrfnl]

I've looked through the code and should be possible now to have a subdirectory (easiest to use the normal test base-name for the subdirectory name) with arbitrarily named test files to run against. Could even just glob() the files in the subdirectory to make adding new ones easy.

This will also allow us to start testing for the - currently not covered - rules that files containing a class should start with class- and possibly that files containing template tags should end with -template, though that last one may be harder to determine.

[GaryJones]

I'm not sure I've ever seen a handbook rule that says that template files should named with -template suffix, but let's leave that for a new ticket.

[jrfnl]

It's not template files, but files containing template tags. See https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#naming-conventions last paragraph.

Issue for this is: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/642

[GaryJones]

The handbook specifically limits that rule, to WP core (wp-includes).

[jrfnl]

Absolutely. So needs some careful crafting to limit the check to that.