Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHPCS 3.x compat: remove php 5.2 work arounds and PHP 5.2 specific code #1050

Merged
merged 3 commits into from
Jul 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 12 additions & 39 deletions WordPress/Sniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -2184,59 +2184,32 @@ public function get_declared_namespace_name( $stackPtr ) {
/**
* Check if a content string contains a specific html open tag.
*
* {@internal For PHP 5.3+ this is straightforward, just check if $content
* contains the tag.
* PHP 5.2 however, creates a separate token for `<s` when used in inline HTML,
* so in that case we need to check that the next token starts with the rest
* of the tag.
* I.e. PHP 5.2 tokenizes the inline HTML `text <span>text</span> text` as:
* - T_INLINE_HTML 'text'
* - T_INLINE_HTML '<s'
* - T_INLINE_HTML 'pan>text</span> text'
*
* We don't need to worry about checking the rest of the content of the next
* token as sniffs using this function will be sniffing for all text string
* tokens, so the next token will be passed to the sniff in the next iteration
* and checked then.
* Similarly, no need to check content before the '<s' as the bug will break up the
* inline html to several string tokens if it plays up.}}
*
* @link https://bugs.php.net/bug.php?id=48446
*
* @since 0.11.0
* @since 0.13.0 No longer allows for the PHP 5.2 bug for which the function was
* originally created.
* @since 0.13.0 The $stackPtr parameter is now optional. Either that or the
* $content parameter has to be passed.
*
* @param string $tag_name The name of the HTML tag without brackets. So if
* searching for '<span...', this would be 'span'.
* @param int $stackPtr The position of the current token in the token stack.
* @param int $stackPtr Optional. The position of the current token in the
* token stack.
* This parameter needs to be passed if no $content is
* passed.
* @param string $content Optionally, the current content string, might be a
* substring of the original string.
* Defaults to `false` to distinguish between a passed
* empty string and not passing the $content string.
*
* @return bool True if the string contains an <tag_name> open tag, false otherwise.
*/
public function has_html_open_tag( $tag_name, $stackPtr, $content = false ) {
if ( false === $content ) {
public function has_html_open_tag( $tag_name, $stackPtr = null, $content = false ) {
if ( false === $content && isset( $stackPtr ) ) {
$content = $this->tokens[ $stackPtr ]['content'];
}

// Check for the open tag in normal string tokens and T_INLINE_HTML for PHP 5.3+.
if ( 's' !== $tag_name[0] || PHP_VERSION_ID >= 50300 || T_INLINE_HTML !== $this->tokens[ $stackPtr ]['code'] ) {
if ( false !== strpos( $content, '<' . $tag_name ) ) {
return true;
}
} elseif ( '<s' === $content ) {
// Ok, we might be coming across the token parser issue. Check the next token.
$next_ptr = ( $stackPtr + 1 );
$rest_tag_name = substr( $tag_name, 1 );

if ( ! empty( $rest_tag_name )
&& isset( $this->tokens[ $next_ptr ] )
&& T_INLINE_HTML === $this->tokens[ $next_ptr ]['code']
&& 0 === strpos( $this->tokens[ $next_ptr ]['content'], $rest_tag_name )
) {
return true;
}
if ( ! empty( $content ) && false !== strpos( $content, '<' . $tag_name ) ) {
return true;
}

return false;
Expand Down
4 changes: 3 additions & 1 deletion WordPress/Sniffs/VIP/ValidatedSanitizedInputSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ public function process_token( $stackPtr ) {
|| T_HEREDOC === $this->tokens[ $stackPtr ]['code']
) {
$interpolated_variables = array_map(
create_function( '$symbol', 'return "$" . $symbol;' ), // Replace with closure when 5.3 is minimum requirement for PHPCS.
function ( $symbol ) {
return '$' . $symbol;
},
$this->get_interpolated_variables( $this->tokens[ $stackPtr ]['content'] )
);
foreach ( array_intersect( $interpolated_variables, $superglobals ) as $bad_variable ) {
Expand Down
4 changes: 3 additions & 1 deletion WordPress/Sniffs/WP/PreparedSQLSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ public function process_token( $stackPtr ) {

$bad_variables = array_filter(
$this->get_interpolated_variables( $this->tokens[ $this->i ]['content'] ),
create_function( '$symbol', 'return ( $symbol !== "wpdb" );' ) // Replace this with closure once 5.3 is minimum requirement.
function ( $symbol ) {
return ( 'wpdb' !== $symbol );
}
);

foreach ( $bad_variables as $bad_variable ) {
Expand Down
17 changes: 0 additions & 17 deletions WordPress/Tests/Classes/ClassInstantiationUnitTest.2.inc

This file was deleted.

17 changes: 0 additions & 17 deletions WordPress/Tests/Classes/ClassInstantiationUnitTest.2.inc.fixed

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,11 @@ $b = & new Foobar();
// Currently not accounted for by the sniff, i.e. false negatives.
$a = new $$varHoldingClassName;
$a = new ${$varHoldingClassName};

// Namespaced classes: OK.
$a = new \MyClass();
$a = new \MyNamespace\MyClass();

// Namespaced classes: Bad.
$a = new \MyClass;
$a = new \MyNamespace\MyClass;
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,11 @@ $b = & new Foobar();
// Currently not accounted for by the sniff, i.e. false negatives.
$a = new $$varHoldingClassName;
$a = new ${$varHoldingClassName};

// Namespaced classes: OK.
$a = new \MyClass();
$a = new \MyNamespace\MyClass();

// Namespaced classes: Bad.
$a = new \MyClass();
$a = new \MyNamespace\MyClass();
27 changes: 3 additions & 24 deletions WordPress/Tests/Classes/ClassInstantiationUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,6 @@
*/
class ClassInstantiationUnitTest extends AbstractSniffUnitTest {

/**
* Get a list of all test files to check.
*
* @param string $testFileBase The base path that the unit tests files will have.
*
* @return string[]
*/
protected function getTestFiles( $testFileBase ) {
$testFiles = parent::getTestFiles( $testFileBase );

if ( PHP_VERSION_ID < 50300 ) {
$testFiles = array_diff( $testFiles, array( $testFileBase . '2.inc' ) );
}

return $testFiles;
}

/**
* Returns the lines where errors should occur.
*
Expand All @@ -47,7 +30,7 @@ protected function getTestFiles( $testFileBase ) {
public function getErrorList( $testFile = '' ) {

switch ( $testFile ) {
case 'ClassInstantiationUnitTest.1.inc':
case 'ClassInstantiationUnitTest.inc':
return array(
37 => 1,
38 => 1,
Expand All @@ -70,12 +53,8 @@ public function getErrorList( $testFile = '' ) {
80 => 1,
84 => 1,
85 => 1,
);

case 'ClassInstantiationUnitTest.2.inc':
return array(
16 => 1,
17 => 1,
97 => 1,
98 => 1,
);

case 'ClassInstantiationUnitTest.js':
Expand Down
9 changes: 0 additions & 9 deletions WordPress/Tests/DB/RestrictedClassesUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,6 @@ protected function tearDown() {
parent::tearDown();
}

/**
* Skip this test on PHP 5.2 as otherwise testing the namespace resolving would fail.
*
* @return bool Whether to skip this test.
*/
protected function shouldSkipTest() {
return ( PHP_VERSION_ID < 50300 );
}

/**
* Returns the lines where errors should occur.
*
Expand Down
5 changes: 0 additions & 5 deletions WordPress/Tests/Files/FileNameUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,6 @@ protected function getTestFiles( $testFileBase ) {
$sep = DIRECTORY_SEPARATOR;
$test_files = glob( dirname( $testFileBase ) . $sep . 'FileNameUnitTests{' . $sep . ',' . $sep . '*' . $sep . '}*.inc', GLOB_BRACE );

// Adjust the expected results array for PHP 5.2 as PHP 5.2 does not recognize namespaces.
if ( PHP_VERSION_ID < 50300 ) {
$this->expected_results['test-sample-phpunit6.inc'] = 1;
}

if ( ! empty( $test_files ) ) {
return $test_files;
}
Expand Down
21 changes: 1 addition & 20 deletions WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,6 @@ public function getErrorList( $testFile = 'PrefixAllGlobalsUnitTest.inc' ) {
40 => 1,
90 => 1,
91 => 1,
// Scoped.
149 => ( PHP_VERSION_ID >= 50300 ) ? 0 : 1, // PHPCS on PHP 5.2 does not recognize namespaces.
151 => ( PHP_VERSION_ID >= 50300 ) ? 0 : 1, // PHPCS on PHP 5.2 does not recognize namespaces.
153 => ( PHP_VERSION_ID >= 50300 ) ? 0 : 1, // PHPCS on PHP 5.2 does not recognize namespaces.
154 => ( PHP_VERSION_ID >= 50300 ) ? 0 : 1, // PHPCS on PHP 5.2 does not recognize namespaces.
155 => ( PHP_VERSION_ID >= 50300 ) ? 0 : 1, // PHPCS on PHP 5.2 does not recognize namespaces.
// Backfills.
225 => ( function_exists( '\mb_strpos' ) ) ? 0 : 1,
230 => ( function_exists( '\array_column' ) ) ? 0 : 1,
Expand All @@ -66,20 +60,7 @@ public function getErrorList( $testFile = 'PrefixAllGlobalsUnitTest.inc' ) {
);

case 'PrefixAllGlobalsUnitTest.1.inc':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case could simply fall through to the default. No need for a separate return array().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Fixed

// Namespaced - all OK.
if ( PHP_VERSION_ID >= 50300 ) {
return array();
}

// PHPCS on PHP 5.2 does not recognize namespaces.
return array(
9 => 1,
11 => 1,
13 => 1,
14 => 1,
15 => 1,
);

// Namespaced - all OK, fall through to the default case.
default:
return array();

Expand Down
8 changes: 4 additions & 4 deletions WordPress/Tests/VIP/AdminBarRemovalUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ public function getErrorList( $testFile = '' ) {
21 => 1,
26 => 1,
32 => 1,
56 => ( PHP_VERSION_ID >= 50300 ) ? 1 : 0, // PHPCS on PHP 5.2 does not recognize T_NOWDOC.
57 => ( PHP_VERSION_ID >= 50300 ) ? 1 : 0, // PHPCS on PHP 5.2 does not recognize T_NOWDOC.
58 => ( PHP_VERSION_ID >= 50300 ) ? 1 : 0, // PHPCS on PHP 5.2 does not recognize T_NOWDOC.
56 => 1,
57 => 1,
58 => 1,
68 => 1,
69 => 1,
70 => 1,
Expand All @@ -60,7 +60,7 @@ public function getErrorList( $testFile = '' ) {

case 'AdminBarRemovalUnitTest.css':
return array(
15 => 1,
15 => 1,
16 => 1,
17 => 1,
22 => 1,
Expand Down
2 changes: 1 addition & 1 deletion WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public function getErrorList() {
190 => 1,
250 => 1,
257 => 1,
274 => ( PHP_VERSION_ID >= 50300 ) ? 1 : 0, // PHPCS on PHP 5.2 does not recognize T_NOWDOC.
274 => 1,
);

}
Expand Down
7 changes: 3 additions & 4 deletions WordPress/Tests/VIP/PluginMenuSlugUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@ class PluginMenuSlugUnitTest extends AbstractSniffUnitTest {
*/
public function getErrorList() {
return array(
3 => 1,
5 => 1,
9 => 2,
14 => ( PHP_VERSION_ID < 50300 ) ? 1 : 0, // PHPCS on PHP 5.2 does not recognize T_NS_SEPARATOR.
3 => 1,
5 => 1,
9 => 2,
);

}
Expand Down
1 change: 0 additions & 1 deletion WordPress/Tests/VIP/RestrictedFunctionsUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ public function getErrorList() {
return array(
3 => 1,
17 => 1,
30 => ( PHP_VERSION_ID >= 50300 ) ? 0 : 1,
32 => 1,
34 => 1,
36 => 1,
Expand Down
10 changes: 0 additions & 10 deletions WordPress/Tests/WP/CapitalPDangitUnitTest.1.inc

This file was deleted.

6 changes: 6 additions & 0 deletions WordPress/Tests/WP/CapitalPDangitUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,9 @@ echo 'This is an explanation about wordpress.'; // WPCS: spelling ok.

<?php // POT filename should be ignored. ?>
wordpress.pot

<?php
// Bad.
$text = <<<'EOD'
This is an explanation about word-press.
EOD;
6 changes: 6 additions & 0 deletions WordPress/Tests/WP/CapitalPDangitUnitTest.inc.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,9 @@ echo 'This is an explanation about wordpress.'; // WPCS: spelling ok.

<?php // POT filename should be ignored. ?>
wordpress.pot

<?php
// Bad.
$text = <<<'EOD'
This is an explanation about WordPress.
EOD;
Loading