From d7460d45bc282bd45c7ef1b2c8cefcaa5a5f3d01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=BDoljom?= Date: Mon, 1 Apr 2019 11:38:13 +0200 Subject: [PATCH] Add new WPThemeReview.CoreFunctionality.PrefixAllGlobals sniff This new sniff replaces the `WordPress.NamingConventions.PrefixAllGlobals` sniff which it extends. The WPCS sniff checks whether everything - i.e. namespaces, classes, functions, constants, hooks, variables - declared in the global namespace is prefixed with a theme/plugin specific prefix. For variables declared in theme template files, this leads to a lot of errors being thrown. Theme template files are, when using the normal WP flow, included from within a function and are therefore _local_ to that function. With this in mind, for the purposes of the ThemeReview checker, variables declared within theme template files can be ignored. The new WPTRCS native version of the sniff overloads the prefix check for variables only and will bow out if the file being scanned has a typical theme template file name. For all other files, it will fall through to the WPCS native sniff. For all other constructs in theme template files, i.e. namespaces, classes, functions etc, the checks from the WPCS native sniff will still be run. Notes: * The new sniff adds a public `$allowed_folders` property to whitelist files in specific folders of a theme as template files. The `ruleset.xml` file sets this property to a limited set of folders whitelisted by default. The default property value can be overruled and/or extended from a custom ruleset. * Similar to the WPCS FileNameSniff, this sniff does not currently allow for mimetype sublevel only theme template file names, such as `plain.php`. Includes dedicated unit tests. --- .../PrefixAllGlobalsSniff.php | 188 ++++++++++++++++++ .../PrefixAllGlobalsTests/attachment.inc | 7 + .../PrefixAllGlobalsTests/footer_widgets.inc | 7 + .../PrefixAllGlobalsTests/header.inc | 7 + .../page-recent-news.inc | 7 + .../layouts/page_two-columns.inc | 8 + .../partials/post-edit.inc | 8 + .../PrefixAllGlobalsTests/social-share.inc | 7 + .../PrefixAllGlobalsUnitTest.inc | 2 + .../PrefixAllGlobalsUnitTest.php | 69 +++++++ WPThemeReview/ruleset.xml | 13 +- 11 files changed, 321 insertions(+), 2 deletions(-) create mode 100644 WPThemeReview/Sniffs/CoreFunctionality/PrefixAllGlobalsSniff.php create mode 100644 WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/attachment.inc create mode 100644 WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/footer_widgets.inc create mode 100644 WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/header.inc 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 create mode 100644 WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/partials/post-edit.inc create mode 100644 WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/social-share.inc create mode 100644 WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsUnitTest.inc create mode 100644 WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsUnitTest.php diff --git a/WPThemeReview/Sniffs/CoreFunctionality/PrefixAllGlobalsSniff.php b/WPThemeReview/Sniffs/CoreFunctionality/PrefixAllGlobalsSniff.php new file mode 100644 index 00000000..b50716d8 --- /dev/null +++ b/WPThemeReview/Sniffs/CoreFunctionality/PrefixAllGlobalsSniff.php @@ -0,0 +1,188 @@ + 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. + * + * This overloads the parent method to allow for non-prefixed variables to be declared + * in template files as those are included from within a function and therefore would be + * local to that function. + * + * @since 0.2.0 + * + * @param int $stackPtr The position of the current token in the stack. + * + * @return int|void Integer stack pointer to skip forward or void to continue + * normal file processing. + */ + protected function process_variable_assignment( $stackPtr ) { + + // Usage of `strip_quotes` is to ensure `stdin_path` passed by IDEs does not include quotes. + $file = $this->strip_quotes( $this->phpcsFile->getFileName() ); + + $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 ( 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 ); + } + + /** + * Checks if the given file path is located in the $allowed_folders array. + * + * @since 0.2.0 + * + * @param string $path Full path of the sniffed file. + * @return boolean + */ + private function is_from_allowed_folder( $path ) { + if ( empty( $this->allowed_folders ) || ! is_array( $this->allowed_folders ) ) { + return false; + } + + foreach ( $this->allowed_folders as $folder ) { + if ( strrpos( $path, DIRECTORY_SEPARATOR . $folder . DIRECTORY_SEPARATOR ) !== false ) { + return true; + } + } + + return false; + } +} diff --git a/WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/attachment.inc b/WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/attachment.inc new file mode 100644 index 00000000..317016b0 --- /dev/null +++ b/WPThemeReview/Tests/CoreFunctionality/PrefixAllGlobalsTests/attachment.inc @@ -0,0 +1,7 @@ +// phpcs:set WPThemeReview.CoreFunctionality.PrefixAllGlobals prefixes[] my_theme + => + */ + public function getErrorList( $testFile = '' ) { + switch ( $testFile ) { + case 'footer_widgets.inc': + case 'social-share.inc': + return array( + 5 => 1, + ); + + default: + return array(); + } + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() { + return array(); + } + +} diff --git a/WPThemeReview/ruleset.xml b/WPThemeReview/ruleset.xml index a609652b..d1b0e3ba 100644 --- a/WPThemeReview/ruleset.xml +++ b/WPThemeReview/ruleset.xml @@ -127,8 +127,17 @@ - - + + + + + + + + + + +