From 355f42f27a6069077c2522097306fa5b9bdba2e0 Mon Sep 17 00:00:00 2001 From: Tim Date: Fri, 17 May 2019 17:33:27 -0400 Subject: [PATCH 1/6] add body_class/language_attributes sniffs --- WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php diff --git a/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php b/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php new file mode 100644 index 00000000..e69de29b From f6dc4f2c65eaba7536b810937fcda30045704876 Mon Sep 17 00:00:00 2001 From: Tim Date: Fri, 17 May 2019 17:38:13 -0400 Subject: [PATCH 2/6] add test cases --- WPThemeReview/Tests/Templates/RequiredFunctionUnitTest.inc | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 WPThemeReview/Tests/Templates/RequiredFunctionUnitTest.inc diff --git a/WPThemeReview/Tests/Templates/RequiredFunctionUnitTest.inc b/WPThemeReview/Tests/Templates/RequiredFunctionUnitTest.inc new file mode 100644 index 00000000..e69de29b From f521b850cb1b11311a422af1c5b9c7c3eac2ed1b Mon Sep 17 00:00:00 2001 From: Tim Date: Fri, 17 May 2019 17:51:47 -0400 Subject: [PATCH 3/6] add required function unit test --- .../Templates/RequiredFunctionUnitTest.php | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 WPThemeReview/Tests/Templates/RequiredFunctionUnitTest.php diff --git a/WPThemeReview/Tests/Templates/RequiredFunctionUnitTest.php b/WPThemeReview/Tests/Templates/RequiredFunctionUnitTest.php new file mode 100644 index 00000000..765c49e3 --- /dev/null +++ b/WPThemeReview/Tests/Templates/RequiredFunctionUnitTest.php @@ -0,0 +1,52 @@ + => + */ + public function getErrorList() { + return array( + 7 => 1, + 8 => 1, + 10 => 1, + 11 => 1, + 13 => 1, + 22 => 1, + 23 => 1, + 24 => 1, + 25 => 1, + 26 => 1, + 27 => 1, + 38 => 1, + ); + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() { + return array(); + } + +} From 6c4c1bc3794c5c67f140ad31e933854e646be2da Mon Sep 17 00:00:00 2001 From: Tim Date: Fri, 17 May 2019 17:56:08 -0400 Subject: [PATCH 4/6] add sniff for lang attributes and body class --- .../Templates/RequiredFunctionSniff.php | 229 ++++++++++++++++++ .../Templates/RequiredFunctionUnitTest.inc | 53 ++++ 2 files changed, 282 insertions(+) diff --git a/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php b/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php index e69de29b..93d45e0e 100644 --- a/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php +++ b/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php @@ -0,0 +1,229 @@ + tag. + * + * @link https://make.wordpress.org/themes/handbook/review/required/#templates + * + * @since 0.1.0 + */ +class RequiredFunctionSniff implements Sniff { + + /** + * Sniff Settings + * + * @var array + */ + public $tagsConfig = array( + 'body' => array( + 'function' => 'body_class', + 'attribute' => 'class', + ), + 'html' => array( + 'function' => 'language_attributes', + 'attribute' => 'lang', + ), + ); + + /** + * Supported Tokenizers + * + * Currently this sniff is only useful in PHP as the required + * functions to call are done in PHP. In testing various + * themes - some had inline comments including ``, and + * were tokenized as T_INLINE_HTML throwing some false positives. + * + * @var array + */ + public $supportedTokenizers = array( 'PHP' ); + + /** + * Tag being searched. + * + * @var array + */ + protected $tag; + + /** + * Returns an array of tokens this test wants to listen for. + * + * @return array + */ + public function register() { + return Tokens::$textStringTokens; + } + + /** + * Processes this test, when one of its tokens is encountered. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The PHP_CodeSniffer file where the + * token was found. + * @param int $stackPtr The position of the current token + * in the stack. + * + * @return void + */ + public function process( File $phpcsFile, $stackPtr ) { + + $tokens = $phpcsFile->getTokens(); + $content = $this->clean_str( $tokens[ $stackPtr ]['content'] ); + $filename = $phpcsFile->getFileName(); + + // Set to false if it is the first time this sniff is run on a file. + if ( ! isset( $this->tag[ $filename ] ) ) { + $this->tag[ $filename ] = false; + } + + // Skip on empty. + if ( '' === $content ) { + return; + } + + // Set tag class property. + foreach ( $this->tagsConfig as $tag => $settings ) { + // HTML case should be insensitive + if ( false !== stripos( $content, '<' . $tag ) ) { + $this->tag[ $filename ] = $this->tagsConfig[ $tag ]; + $this->tag[ $filename ]['tag'] = $tag; + break; + } + } + + // Skip if not a tag. + if ( false === $this->tag[ $filename ] ) { + return; + } + + // Set vars used for reference. + $tagName = $this->tag[ $filename ]['tag']; + $tagFn = $this->tag[ $filename ]['function']; + $tagAttr = $this->tag[ $filename ]['attribute']; + $pascal = str_replace( '_', '', ucwords( $tagFn, '_' ) ); + $nextPtr = $stackPtr; + $foundFunction = false; + $foundAttribute = false; + $foundEnd = false; + + do { + $nextPtrContent = $this->clean_str( $tokens[ $nextPtr ]['content'] ); + $nextPtrCode = $tokens[ $nextPtr ]['code']; + + // Check for attribute not allowed. + if ( + false === $foundAttribute && + in_array( $nextPtrCode, Tokens::$textStringTokens, true ) && + false !== stripos( $nextPtrContent, $tagAttr . '=' ) + ) { + $foundAttribute =true; + } + + // Check for required function call. + if ( + false === $foundFunction && + in_array( $nextPtrCode, Tokens::$functionNameTokens, true ) && + false !== strpos( $nextPtrContent, $tagFn ) + ) { + + // Check next non-whitespace token for opening parens. + $next = $phpcsFile->findNext( Tokens::$emptyTokens, ( $nextPtr + 1 ), null, true ); + + if ( ! $next || ! isset( $tokens[ $next ] ) ) { + break; // Nothing left. + } + + // Verify function( $param = 'optional' ) type. + if ( 'PHPCS_T_OPEN_PARENTHESIS' === $tokens[ $next ]['code'] ) { + + // Skip over contents to closing parens in stack. + if ( isset( $tokens[ $next ]['parenthesis_closer'] ) ) { + $nextPtr = $tokens[ $next ]['parenthesis_closer']; + $foundFunction = true; + } + } + + continue; + } + + // Check for searched tag matched closing bracket. + if ( + in_array( $nextPtrCode, Tokens::$textStringTokens, true ) && + '>' === substr( $nextPtrContent, -1 ) + ) { + + $this->tag[ $filename ] = false; + $foundEnd = true; + break; + } + + // Increment stack to next non-whitespace token. + $next = $phpcsFile->findNext( Tokens::$emptyTokens, ( $nextPtr + 1 ), null, true ); + + if ( ! $next || ! isset( $tokens[ $next ] ) ) { + break; // Short circuit loop as there's not anything left. + } + + $nextPtr = $next; + + } while ( false === $foundEnd ); // Loop until matched closing bracket is found for searched tag. + + // Required function not found. + if ( false === $foundFunction ) { + $phpcsFile->addError( + "Themes must call {$tagFn}() inside <{$tagName}> tags.", + $stackPtr, + "RequiredFunction{$pascal}" + ); + + return; + } + + // Atrribute is not allowed. + if ( true === $foundAttribute ) { + $phpcsFile->addError( + "Attribute '{$tagAttr}' is not allowed on <{$tagName}> tags. Themes must call {$tagFn}() instead.", + $stackPtr, + "DisallowedAttribute{$pascal}" + ); + + return; + } + } + + /** + * Cleans string for parsing. + * + * This cleans whitespace chars and single/double quotes + * from string. Primary used to check T_CONSTANT_ENCAPSED_STRING + * and T_DOUBLE_QUOTED_STRING for closing HTML brackets. This is + * because < and > are valid attribute values, and a strpos wouldn't + * be enough. + * + * Strips: + * ' ' : Whitespace + * '"' : double quote + * ''' : single quote + * '\t' : tab + * '\n' : newline + * '\r' : carriage return + * '\0' : NUL-byte + * '\x0B': vertical tab + * + * @return string Cleaned string. + */ + private function clean_str( $str ) { + return trim( $str, " \"\'\t\n\r\0\x0B" ); + } +} diff --git a/WPThemeReview/Tests/Templates/RequiredFunctionUnitTest.inc b/WPThemeReview/Tests/Templates/RequiredFunctionUnitTest.inc index e69de29b..363d8c4d 100644 --- a/WPThemeReview/Tests/Templates/RequiredFunctionUnitTest.inc +++ b/WPThemeReview/Tests/Templates/RequiredFunctionUnitTest.inc @@ -0,0 +1,53 @@ +'; // Bad. +echo ''; // Bad. +?> + + lang="en-US"> +'; +?> +'; +?> + lang="en-US"> +'; // Bad. ?> + +'; // Bad. ?> +'; // Bad. ?> +'; // Bad. ?> +> + class="html-class"> +'; // Ok. ?> +'; // Ok. ?> +> + + + + ... + +EOT; + +// Test weird but valid heredoc style. +$html = << + + ... +EOT; +?> From 322688341a9137365d97e67e1a2b648613e634c8 Mon Sep 17 00:00:00 2001 From: Tim Date: Fri, 17 May 2019 18:30:15 -0400 Subject: [PATCH 5/6] pass build --- .../Sniffs/Templates/RequiredFunctionSniff.php | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php b/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php index 93d45e0e..3a724912 100644 --- a/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php +++ b/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php @@ -94,7 +94,8 @@ public function process( File $phpcsFile, $stackPtr ) { // Set tag class property. foreach ( $this->tagsConfig as $tag => $settings ) { - // HTML case should be insensitive + + // HTML case should be insensitive. if ( false !== stripos( $content, '<' . $tag ) ) { $this->tag[ $filename ] = $this->tagsConfig[ $tag ]; $this->tag[ $filename ]['tag'] = $tag; @@ -111,7 +112,7 @@ public function process( File $phpcsFile, $stackPtr ) { $tagName = $this->tag[ $filename ]['tag']; $tagFn = $this->tag[ $filename ]['function']; $tagAttr = $this->tag[ $filename ]['attribute']; - $pascal = str_replace( '_', '', ucwords( $tagFn, '_' ) ); + $pascal = str_replace( ' ', '', ucwords( str_replace( '_', ' ', $tagFn ) ) ); $nextPtr = $stackPtr; $foundFunction = false; $foundAttribute = false; @@ -119,7 +120,7 @@ public function process( File $phpcsFile, $stackPtr ) { do { $nextPtrContent = $this->clean_str( $tokens[ $nextPtr ]['content'] ); - $nextPtrCode = $tokens[ $nextPtr ]['code']; + $nextPtrCode = $tokens[ $nextPtr ]['code']; // Check for attribute not allowed. if ( @@ -127,7 +128,7 @@ public function process( File $phpcsFile, $stackPtr ) { in_array( $nextPtrCode, Tokens::$textStringTokens, true ) && false !== stripos( $nextPtrContent, $tagAttr . '=' ) ) { - $foundAttribute =true; + $foundAttribute = true; } // Check for required function call. @@ -149,7 +150,7 @@ public function process( File $phpcsFile, $stackPtr ) { // Skip over contents to closing parens in stack. if ( isset( $tokens[ $next ]['parenthesis_closer'] ) ) { - $nextPtr = $tokens[ $next ]['parenthesis_closer']; + $nextPtr = $tokens[ $next ]['parenthesis_closer']; $foundFunction = true; } } @@ -162,9 +163,8 @@ public function process( File $phpcsFile, $stackPtr ) { in_array( $nextPtrCode, Tokens::$textStringTokens, true ) && '>' === substr( $nextPtrContent, -1 ) ) { - $this->tag[ $filename ] = false; - $foundEnd = true; + $foundEnd = true; break; } @@ -221,6 +221,8 @@ public function process( File $phpcsFile, $stackPtr ) { * '\0' : NUL-byte * '\x0B': vertical tab * + * @param string $str String to clean. + * * @return string Cleaned string. */ private function clean_str( $str ) { From c19ea43f874a9de1f321b3e107052c19074e618e Mon Sep 17 00:00:00 2001 From: Tim Elsass Date: Tue, 21 May 2019 01:30:03 -0400 Subject: [PATCH 6/6] code review updates --- .../Sniffs/Templates/RequiredFunctionSniff.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php b/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php index 3a724912..4ba38245 100644 --- a/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php +++ b/WPThemeReview/Sniffs/Templates/RequiredFunctionSniff.php @@ -14,11 +14,13 @@ use PHP_CodeSniffer\Util\Tokens; /** - * Ensures that body_class() is called if theme adds a tag. + * Ensures functions are called within HTML tags. + * + * Ex: > * * @link https://make.wordpress.org/themes/handbook/review/required/#templates * - * @since 0.1.0 + * @since 0.2.0 */ class RequiredFunctionSniff implements Sniff { @@ -125,7 +127,7 @@ public function process( File $phpcsFile, $stackPtr ) { // Check for attribute not allowed. if ( false === $foundAttribute && - in_array( $nextPtrCode, Tokens::$textStringTokens, true ) && + isset( Tokens::$textStringTokens[ $nextPtrCode ] ) && false !== stripos( $nextPtrContent, $tagAttr . '=' ) ) { $foundAttribute = true; @@ -134,7 +136,7 @@ public function process( File $phpcsFile, $stackPtr ) { // Check for required function call. if ( false === $foundFunction && - in_array( $nextPtrCode, Tokens::$functionNameTokens, true ) && + isset( Tokens::$functionNameTokens[ $nextPtrCode ] ) && false !== strpos( $nextPtrContent, $tagFn ) ) { @@ -160,7 +162,7 @@ public function process( File $phpcsFile, $stackPtr ) { // Check for searched tag matched closing bracket. if ( - in_array( $nextPtrCode, Tokens::$textStringTokens, true ) && + isset( Tokens::$textStringTokens[ $nextPtrCode ] ) && '>' === substr( $nextPtrContent, -1 ) ) { $this->tag[ $filename ] = false;