From 9bc89de9bdc7af674882cb74fee31ae11775c34f Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 10 May 2019 03:54:05 +0200 Subject: [PATCH] PrefixAllGlobals: refactor the template file name recognition 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`. --- .../PrefixAllGlobalsSniff.php | 100 +++++++++++++++++- .../PrefixAllGlobalsTests/header.inc | 2 +- .../page-recent-news.inc | 7 ++ .../layouts/page_two-columns.inc | 8 ++ .../partials/post-edit.inc | 4 +- .../PrefixAllGlobalsUnitTest.php | 3 - WPThemeReview/ruleset.xml | 1 + 7 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/page-recent-news.inc create mode 100644 WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/page-templates/layouts/page_two-columns.inc diff --git a/WPThemeReview/Sniffs/CoreFunctionality/PrefixAllGlobalsSniff.php b/WPThemeReview/Sniffs/CoreFunctionality/PrefixAllGlobalsSniff.php index 38a4c1a2..b50716d8 100644 --- a/WPThemeReview/Sniffs/CoreFunctionality/PrefixAllGlobalsSniff.php +++ b/WPThemeReview/Sniffs/CoreFunctionality/PrefixAllGlobalsSniff.php @@ -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. @@ -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. * @@ -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. * @@ -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 ); } diff --git a/WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/header.inc b/WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/header.inc index efeb89d9..317016b0 100644 --- a/WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/header.inc +++ b/WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/header.inc @@ -2,6 +2,6 @@ 1, diff --git a/WPThemeReview/ruleset.xml b/WPThemeReview/ruleset.xml index 6853198a..d1b0e3ba 100644 --- a/WPThemeReview/ruleset.xml +++ b/WPThemeReview/ruleset.xml @@ -134,6 +134,7 @@ +