From 34d436f567e69d36f6d94f6a5abdf8331c2c3385 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 22 Jul 2016 02:10:38 +0200 Subject: [PATCH] Revisit the complete logic of the `DisallowAlternativePHPTags` Sniff. New & improved version. Includes additional unit tests. --- .travis.yml | 1 + .../PHP/DisallowAlternativePHPTagsSniff.php | 178 +++++++++++------- .../DisallowAlternativePHPTagsUnitTest.fixed | 9 - .../DisallowAlternativePHPTagsUnitTest.inc | 11 +- ...sallowAlternativePHPTagsUnitTest.inc.fixed | 18 ++ .../DisallowAlternativePHPTagsUnitTest.php | 63 ++++--- myconfig.ini | 3 + 7 files changed, 179 insertions(+), 104 deletions(-) delete mode 100644 WordPress/Tests/PHP/DisallowAlternativePHPTagsUnitTest.fixed create mode 100644 WordPress/Tests/PHP/DisallowAlternativePHPTagsUnitTest.inc.fixed create mode 100644 myconfig.ini diff --git a/.travis.yml b/.travis.yml index ea05038248..500c1a8546 100644 --- a/.travis.yml +++ b/.travis.yml @@ -36,6 +36,7 @@ matrix: - env: PHPCS_BRANCH=3.0 before_script: + - if [[ $TRAVIS_PHP_VERSION != "7.0" && $TRAVIS_PHP_VERSION != "nightly" && $TRAVIS_PHP_VERSION != "hhvm" ]]; then phpenv config-add myconfig.ini; fi - export PHPCS_DIR=/tmp/phpcs - export PHPCS_BIN=$(if [[ $PHPCS_BRANCH == 3.0 ]]; then echo $PHPCS_DIR/bin/phpcs; else echo $PHPCS_DIR/scripts/phpcs; fi) - mkdir -p $PHPCS_DIR && git clone --depth 1 https://github.com/squizlabs/PHP_CodeSniffer.git -b $PHPCS_BRANCH $PHPCS_DIR diff --git a/WordPress/Sniffs/PHP/DisallowAlternativePHPTagsSniff.php b/WordPress/Sniffs/PHP/DisallowAlternativePHPTagsSniff.php index b2270e3387..e2690c16b6 100644 --- a/WordPress/Sniffs/PHP/DisallowAlternativePHPTagsSniff.php +++ b/WordPress/Sniffs/PHP/DisallowAlternativePHPTagsSniff.php @@ -3,7 +3,7 @@ * WordPress Coding Standard. * * @category PHP - * @package PHP_CodeSniffer + * @package PHP\CodeSniffer\WordPress-Coding-Standards * @link https://make.wordpress.org/core/handbook/best-practices/coding-standards/ */ @@ -15,31 +15,34 @@ * @link https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/580 * * @category PHP - * @package PHP_CodeSniffer + * @package PHP\CodeSniffer\WordPress-Coding-Standards * @author Juliette Reinders Folmer */ class WordPress_Sniffs_PHP_DisallowAlternativePHPTagsSniff implements PHP_CodeSniffer_Sniff { + /** + * Whether ASP tags are enabled or not. + * + * @var bool + */ + private $asp_tags = false; + /** * Returns an array of tokens this test wants to listen for. * * @return array */ public function register() { - $tokens = array( + if ( version_compare( PHP_VERSION, '7.0.0alpha1', '<' ) ) { + $this->asp_tags = (bool) ini_get( 'asp_tags' ); + } + + return array( T_OPEN_TAG, T_OPEN_TAG_WITH_ECHO, + T_INLINE_HTML, ); - $asp_enabled = (boolean) ini_get( 'asp_tags' ); - $short_enabled = (boolean) ini_get( 'short_open_tag' ); - - if ( false === $asp_enabled || ( true === $asp_enabled && false === $short_enabled ) ) { - $tokens[] = T_INLINE_HTML; - } - - return $tokens; - } // end register() /** @@ -52,52 +55,51 @@ public function register() { * @return void */ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) { - $tokens = $phpcsFile->getTokens(); - $openTag = $tokens[ $stackPtr ]; - $asp_enabled = (boolean) ini_get( 'asp_tags' ); - $short_enabled = (boolean) ini_get( 'short_open_tag' ); + $tokens = $phpcsFile->getTokens(); + $openTag = $tokens[ $stackPtr ]; + $content = $openTag['content']; + + if ( '' === trim( $content ) ) { + return; + } if ( T_OPEN_TAG === $openTag['code'] ) { - if ( '<%' === $openTag['content'] ) { - $error = 'ASP style opening tag used; expected "find_closing_tag( $phpcsFile, $tokens, $stackPtr, '%>' ); + if ( '<%' === $content ) { + $error = 'ASP style opening tag used; expected "find_closing_tag( $phpcsFile, $tokens, $stackPtr, '%>' ); + $error_id = 'ASPOpenTagFound'; - if ( false === $closer ) { - $phpcsFile->addError( $error, $stackPtr, 'ASPOpenTagFound', $data ); - } else { - $fix = $phpcsFile->addFixableError( $error, $stackPtr, 'ASPOpenTagFound', $data ); - if ( true === $fix ) { - $this->add_changeset( $phpcsFile, $stackPtr, $closer ); - } - } + } elseif ( false !== strpos( $content, '' ); + $error_id = 'ScriptOpenTagFound'; } - if ( '' ); + if ( isset( $error, $closer, $error_id ) ) { + $data = array( $content ); if ( false === $closer ) { - $phpcsFile->addError( $error, $stackPtr, 'ScriptOpenTagFound', $data ); + $phpcsFile->addError( $error, $stackPtr, $error_id, $data ); } else { - $fix = $phpcsFile->addFixableError( $error, $stackPtr, 'ScriptOpenTagFound', $data ); + $fix = $phpcsFile->addFixableError( $error, $stackPtr, $error_id, $data ); if ( true === $fix ) { - $this->add_changeset( $phpcsFile, $stackPtr, $closer ); + $this->add_changeset( $phpcsFile, $tokens, $stackPtr, $closer ); } } } + + return; } - if ( T_OPEN_TAG_WITH_ECHO === $openTag['code'] && '<%=' === $openTag['content'] ) { + if ( T_OPEN_TAG_WITH_ECHO === $openTag['code'] && '<%=' === $content ) { $error = 'ASP style opening tag used with echo; expected "findNext( PHP_CodeSniffer_Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ) ]; + $nextVar = $phpcsFile->findNext( T_WHITESPACE, ( $stackPtr + 1 ), null, true ); + $snippet = $this->get_snippet( $tokens[ $nextVar ]['content'] ); $data = array( - $nextVar['content'], - $openTag['content'], - $nextVar['content'], + $snippet, + $content, + $snippet, ); $closer = $this->find_closing_tag( $phpcsFile, $tokens, $stackPtr, '%>' ); @@ -107,50 +109,67 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) { } else { $fix = $phpcsFile->addFixableError( $error, $stackPtr, 'ASPShortOpenTagFound', $data ); if ( true === $fix ) { - $this->add_changeset( $phpcsFile, $stackPtr, $closer ); + $this->add_changeset( $phpcsFile, $tokens, $stackPtr, $closer, true ); } } + + return; } - if ( ( true === $asp_enabled && false === $short_enabled ) && ( T_INLINE_HTML === $openTag['code'] && 0 === strpos( $openTag['content'], '<%=' ) ) ) { + // Account for incorrect script open tags. The "(?:]+)?language=[\'"]?php[\'"]?(?:[^>]+)?>)`i', $content, $match ) ) { + $error = 'Script style opening tag used; expected "get_snippet( $content, $match[1] ); + $data = array( $match[1] . $snippet ); - $error = 'Possible use of ASP style short opening tags. Needs manual inspection. Found: %s'; - $data = array( $this->get_snippet( $openTag['content'] ) ); - $phpcsFile->addWarning( $error, $stackPtr, 'MaybeASPShortOpenTagFound', $data ); - } + $phpcsFile->addError( $error, $stackPtr, 'ScriptOpenTagFound', $data ); - if ( false === $asp_enabled && T_INLINE_HTML === $openTag['code'] ) { + return; + } - $data = array( $this->get_snippet( $openTag['content'] ) ); + if ( T_INLINE_HTML === $openTag['code'] && false === $this->asp_tags ) { + if ( false !== strpos( $content, '<%=' ) ) { + $error = 'Possible use of ASP style short opening tags detected. Needs manual inspection. Found: %s'; + $snippet = $this->get_snippet( $content, '<%=' ); + $data = array( '<%=' . $snippet ); - if ( 0 === strpos( $openTag['content'], '<%=' ) ) { - $error = 'Possible use of ASP style short opening tags. Needs manual inspection. Found: %s'; $phpcsFile->addWarning( $error, $stackPtr, 'MaybeASPShortOpenTagFound', $data ); - } elseif ( 0 === strpos( $openTag['content'], '<%' ) ) { - $error = 'Possible use of ASP style opening tags. Needs manual inspection. Found: %s'; + + } elseif ( false !== strpos( $content, '<%' ) ) { + $error = 'Possible use of ASP style opening tags detected. Needs manual inspection. Found: %s'; + $snippet = $this->get_snippet( $content, '<%' ); + $data = array( '<%' . $snippet ); + $phpcsFile->addWarning( $error, $stackPtr, 'MaybeASPOpenTagFound', $data ); - } elseif ( 0 === strpos( $openTag['content'], ' + + + diff --git a/WordPress/Tests/PHP/DisallowAlternativePHPTagsUnitTest.inc.fixed b/WordPress/Tests/PHP/DisallowAlternativePHPTagsUnitTest.inc.fixed new file mode 100644 index 0000000000..2b1c151387 --- /dev/null +++ b/WordPress/Tests/PHP/DisallowAlternativePHPTagsUnitTest.inc.fixed @@ -0,0 +1,18 @@ +
+ +Some content here. + +

Some text and some more text

+ +

Some text and some more text

+ + + + +
diff --git a/WordPress/Tests/PHP/DisallowAlternativePHPTagsUnitTest.php b/WordPress/Tests/PHP/DisallowAlternativePHPTagsUnitTest.php index f27083a7ca..f68c4a94de 100644 --- a/WordPress/Tests/PHP/DisallowAlternativePHPTagsUnitTest.php +++ b/WordPress/Tests/PHP/DisallowAlternativePHPTagsUnitTest.php @@ -3,7 +3,7 @@ * Unit test class for WordPress Coding Standard. * * @category PHP - * @package PHP_CodeSniffer + * @package PHP\CodeSniffer\WordPress-Coding-Standards * @link https://make.wordpress.org/core/handbook/best-practices/coding-standards/ */ @@ -14,11 +14,38 @@ * coding standard. Expected errors and warnings are stored in this class. * * @category PHP - * @package PHP_CodeSniffer + * @package PHP\CodeSniffer\WordPress-Coding-Standards * @author Juliette Reinders Folmer */ class WordPress_Tests_PHP_DisallowAlternativePHPTagsUnitTest extends AbstractSniffUnitTest { + /** + * Whether ASP tags are enabled or not. + * + * @var bool + */ + private $asp_tags = false; + + /** + * Get the ini values only once. + */ + protected function setUp() { + parent::setUp(); + + if ( version_compare( PHP_VERSION, '7.0.0alpha1', '<' ) ) { + $this->asp_tags = (bool) ini_get( 'asp_tags' ); + } + } + + /** + * Skip this test on HHVM. + * + * @return bool Whether to skip this test. + */ + protected function shouldSkipTest() { + return defined( 'HHVM_VERSION' ); + } + /** * Returns the lines where errors should occur. * @@ -28,22 +55,22 @@ class WordPress_Tests_PHP_DisallowAlternativePHPTagsUnitTest extends AbstractSni * @return array */ public function getErrorList() { - $asp_enabled = (boolean) ini_get( 'asp_tags' ); - $short_enabled = (boolean) ini_get( 'short_open_tag' ); - $errors = array( - 6 => 1, + 8 => 1, + 11 => 1, + 12 => 1, + 15 => 1, ); - if ( true === $asp_enabled ) { + if ( true === $this->asp_tags ) { $errors[4] = 1; - } - if ( true === $asp_enabled && ( true === $short_enabled || defined( 'HHVM_VERSION' ) === true ) ) { $errors[5] = 1; + $errors[6] = 1; + $errors[7] = 1; } return $errors; - } // end getErrorList() + } /** * Returns the lines where warnings should occur. @@ -54,24 +81,18 @@ public function getErrorList() { * @return array */ public function getWarningList() { - $asp_enabled = (boolean) ini_get( 'asp_tags' ); - $short_enabled = (boolean) ini_get( 'short_open_tag' ); - $warnings = array(); - if ( false === $asp_enabled ) { + if ( false === $this->asp_tags ) { $warnings = array( 4 => 1, 5 => 1, - ); - } elseif ( false === $short_enabled && false === defined( 'HHVM_VERSION' ) ) { - $warnings = array( - 5 => 1, + 6 => 1, + 7 => 1, ); } return $warnings; + } - } // end getWarningList() - -} // end class +} // End class. diff --git a/myconfig.ini b/myconfig.ini new file mode 100644 index 0000000000..8c5f68f520 --- /dev/null +++ b/myconfig.ini @@ -0,0 +1,3 @@ +; Allow ASP-style <% %> tags. +; http://php.net/asp-tags +asp_tags = On