Skip to content

Commit

Permalink
PrefixAllGlobals: refactor the template file name recognition
Browse files Browse the repository at this point in the history
As it turns out that the regex in the FileName sniff, while good for inspiration, is not actually suitable to be used here directly, we need to solve the template file name recognition within this sniff.

To that end and based on the Theme Handbook, I have:
* Added a `$simple_theme_template_file_names` property with the simple plain file names as per the handbook.
* Added a `COMPLEX_THEME_TEMPLATE_NAME_REGEX` constant with a regex to facilitate recognizing more complex template file names.
* Added the `page-templates` folder name to the list of `$allowed_folders` in the ruleset.
    See: https://developer.wordpress.org/themes/template-files-section/page-template-files/#file-organization-of-page-templates
* Adjusted the logic in the overloaded method to use the new property and constant.
* Adjusted the unit tests to match.
* Added an additional unit test with a file in a subdirectory of an allowed folder.
* Added an additional unit test case file to cover the regex.

Note: similar to the WPCS FileNameSniff, this sniff does not currently allow for mimetype sublevel only file names, such as `plain.php`.
  • Loading branch information
jrfnl committed May 10, 2019
1 parent bff6337 commit 9bc89de
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 8 deletions.
100 changes: 98 additions & 2 deletions WPThemeReview/Sniffs/CoreFunctionality/PrefixAllGlobalsSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
namespace WPThemeReview\Sniffs\CoreFunctionality;

use WordPressCS\WordPress\Sniffs\NamingConventions\PrefixAllGlobalsSniff as WPCSPrefixAllGlobalsSniff;
use WordPressCS\WordPress\Sniffs\Files\FileNameSniff;

/**
* Verify that everything defined in the global namespace is prefixed with a theme specific prefix.
Expand All @@ -23,10 +22,41 @@
* @link https://github.com/WPTRT/WPThemeReview/issues/201
* @link https://github.com/WPTRT/WPThemeReview/issues/200
*
* The sniff does not currently allow for mimetype sublevel only file names,
* such as `plain.php`.
*
* @since 0.2.0
*/
class PrefixAllGlobalsSniff extends WPCSPrefixAllGlobalsSniff {

/**
* Regex to recognize more complex typical theme template file names.
*
* @link https://developer.wordpress.org/themes/basics/template-hierarchy/#single-post
* @link https://developer.wordpress.org/themes/basics/template-hierarchy/#custom-taxonomies
* @link https://developer.wordpress.org/themes/basics/template-hierarchy/#custom-post-types
* @link https://developer.wordpress.org/themes/basics/template-hierarchy/#embeds
* @link https://developer.wordpress.org/themes/basics/template-hierarchy/#attachment
* @link https://developer.wordpress.org/themes/template-files-section/partial-and-miscellaneous-template-files/#content-slug-php
* @link https://wphierarchy.com/
* @link https://en.wikipedia.org/wiki/Media_type#Naming
*
* @since 0.2.0
*
* @var string
*/
const COMPLEX_THEME_TEMPLATE_NAME_REGEX = '`
^ # Anchor to the beginning of the string.
(?:
# Template file prefixes with subtype.
(?:archive|author|category|content|embed|page|single|tag|taxonomy)
-[^\.]+ # These need to be followed by a dash and some chars.
|
(?:application|audio|example|font|image|message|model|multipart|text|video) #Top-level mime-types
(?:_[^\.]+)? # Optionally followed by an underscore and a sub-type.
)\.php$ # End in .php and anchor to the end of the string.
`Dx';

/**
* The list of allowed folders to check the file path against.
*
Expand All @@ -39,6 +69,59 @@ class PrefixAllGlobalsSniff extends WPCSPrefixAllGlobalsSniff {
*/
public $allowed_folders = [];

/**
* List of plain template file names.
*
* @link https://developer.wordpress.org/themes/basics/template-hierarchy/
* @link https://developer.wordpress.org/themes/template-files-section/partial-and-miscellaneous-template-files/#content-slug-php
* @link https://wphierarchy.com/
* @link https://en.wikipedia.org/wiki/Media_type#Naming
*
* @since 0.2.0
*
* @var array
*/
protected $simple_theme_template_file_names = [
// Plain primary template file names.
'404.php' => true,
'archive.php' => true,
'home.php' => true,
'index.php' => true,
'page.php' => true,
'search.php' => true,
'single.php' => true,
'singular.php' => true,

// Plain secondary template file names.
'attachment.php' => true,
'author.php' => true,
'category.php' => true,
'date.php' => true,
'embed.php' => true,
'front-page.php' => true,
'single-post.php' => true,
'tag.php' => true,
'taxonomy.php' => true,

// Plain partial and miscellaneous template file names.
'comments.php' => true,
'footer.php' => true,
'header.php' => true,
'sidebar.php' => true,

// Top-leve mime types.
'application.php' => true,
'audio.php' => true,
'example.php' => true,
'font.php' => true,
'image.php' => true,
'message.php' => true,
'model.php' => true,
'multipart.php' => true,
'text.php' => true,
'video.php' => true,
];

/**
* Check that defined global variables are prefixed.
*
Expand All @@ -60,11 +143,24 @@ protected function process_variable_assignment( $stackPtr ) {

$fileName = basename( $file );

if ( \defined( '\PHP_CODESNIFFER_IN_TESTS' ) ) {
$fileName = str_replace( '.inc', '.php', $fileName );
}

// Don't process in case of a template file or folder.
if ( preg_match( FileNameSniff::THEME_EXCEPTIONS_REGEX, $fileName ) === 1 || $this->is_from_allowed_folder( $file ) ) {
if ( isset( $this->simple_theme_template_file_names[ $fileName ] ) === true ) {
return;
}

if ( preg_match( self::COMPLEX_THEME_TEMPLATE_NAME_REGEX, $fileName ) === 1 ) {
return;
}

if ( $this->is_from_allowed_folder( $file ) ) {
return;
}

// Not a typical template file name, defer to the prefix checking in the parent sniff.
return parent::process_variable_assignment( $stackPtr );
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
<?php

$my_theme_var = 123; // OK, prefixed.
$var = 'Value'; // Error. Not in a template file.
$var = 'Value'; // OK, template file.

// phpcs:set WPThemeReview.CoreFunctionality.PrefixAllGlobals prefixes[]
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// phpcs:set WPThemeReview.CoreFunctionality.PrefixAllGlobals prefixes[] my_theme
<?php

$my_theme_var = 123; // OK, prefixed.
$var = 'Value'; // OK, template file.

// phpcs:set WPThemeReview.CoreFunctionality.PrefixAllGlobals prefixes[]
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// phpcs:set WPThemeReview.CoreFunctionality.PrefixAllGlobals prefixes[] my_theme
// phpcs:set WPThemeReview.CoreFunctionality.PrefixAllGlobals allowed_folders[] template-parts,templates,partials,page-templates
<?php
$my_theme_var = 123; // OK, prefixed.
$var = 'Value'; // OK, file in allowed folder.

// phpcs:set WPThemeReview.CoreFunctionality.PrefixAllGlobals prefixes[]
// phpcs:set WPThemeReview.CoreFunctionality.PrefixAllGlobals allowed_folders[]
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// phpcs:set WPThemeReview.CoreFunctionality.PrefixAllGlobals prefixes[] my_theme
// phpcs:set WPThemeReview.CoreFunctionality.PrefixAllGlobals allowed_folders[] template-parts,templates,partials
// phpcs:set WPThemeReview.CoreFunctionality.PrefixAllGlobals allowed_folders[] template-parts,templates,partials,page-templates
<?php
$my_theme_var = 123; // OK, prefixed.
$var = 'Value'; // OK, template file.
$var = 'Value'; // OK, file in allowed folder.

// phpcs:set WPThemeReview.CoreFunctionality.PrefixAllGlobals prefixes[]
// phpcs:set WPThemeReview.CoreFunctionality.PrefixAllGlobals allowed_folders[]
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,7 @@ protected function getTestFiles( $testFileBase ) {
*/
public function getErrorList( $testFile = '' ) {
switch ( $testFile ) {
case 'attachment.inc':
// Template file - all OK, fall through to the default case.
case 'footer_widgets.inc':
case 'header.inc':
case 'social-share.inc':
return array(
5 => 1,
Expand Down
1 change: 1 addition & 0 deletions WPThemeReview/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@
<element value="template-parts"/>
<element value="templates"/>
<element value="partials"/>
<element value="page-templates"/>
</property>
</properties>
</rule>
Expand Down

0 comments on commit 9bc89de

Please sign in to comment.