Skip to content

Commit

Permalink
Merge pull request #1908 from WordPress/3.0/1583-remove-deprecated-is…
Browse files Browse the repository at this point in the history
…-whitelisted
  • Loading branch information
GaryJones authored Jul 6, 2020
2 parents 8bcbae8 + d8ed55f commit b430993
Show file tree
Hide file tree
Showing 38 changed files with 54 additions and 324 deletions.
10 changes: 0 additions & 10 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,6 @@ Only make a property `public` if that is the intended behaviour.

When you introduce new `public` sniff properties, or your sniff extends a class from which you inherit a `public` property, please don't forget to update the [public properties wiki page](https://github.com/WordPress/WordPress-Coding-Standards/wiki/Customizable-sniff-properties) with the relevant details once your PR has been merged into the `develop` branch.

## Whitelist comments

> **Important**:
> PHPCS 3.2.0 introduced new selective ignore annotations, which can be considered an improved version of the whitelist mechanism which WPCS contains.
>
> Support for the WPCS native whitelist comments has been deprecated in WPCS 2.0.0 and will be removed in WPCS 3.0.0.
>
> With that in mind, (new) sniffs should not introduce new WPCS native whitelist comments.

# Unit Testing

## Pre-requisites
Expand Down
6 changes: 0 additions & 6 deletions .phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,4 @@
<exclude name="PHPCompatibility.Constants.NewConstants.t_yield_fromFound"/>
</rule>

<rule ref="WordPress.NamingConventions.PrefixAllGlobals.DeprecatedWhitelistCommentFound">
<!-- False positive for whitelist comment recognition, but no use fixing this now
as the WPCS native whitelist comments are deprecated anyhow. -->
<exclude-pattern>/WordPress/AbstractClassRestrictionsSniff\.php$</exclude-pattern>
</rule>

</ruleset>
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
+ [Using PHPCS and WPCS from within your IDE](#using-phpcs-and-wpcs-from-within-your-ide)
* [Running your code through WPCS automatically using CI tools](#running-your-code-through-wpcs-automatically-using-ci-tools)
+ [Travis CI](#travis-ci)
* [Fixing errors or whitelisting them](#fixing-errors-or-whitelisting-them)
* [Fixing errors or ignoring them](#fixing-errors-or-ignoring-them)
+ [Tools shipped with WPCS](#tools-shipped-with-wpcs)
* [Contributing](#contributing)
* [License](#license)
Expand Down Expand Up @@ -267,7 +267,7 @@ script:
More examples and advice about integrating PHPCS in your Travis build tests can be found here: https://github.com/jrfnl/make-phpcs-work-for-you/tree/master/travis-examples
## Fixing errors or whitelisting them
## Fixing errors or ignoring them
You can find information on how to deal with some of the more frequent issues in the [wiki](https://github.com/WordPress/WordPress-Coding-Standards/wiki).
Expand Down
107 changes: 0 additions & 107 deletions WordPress/Sniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -1149,113 +1149,6 @@ protected function get_wp_version_from_cl() {
}
}

/**
* Find whitelisting comment.
*
* Comment must be at the end of the line or at the end of the statement
* and must use // format.
* It can be prefixed or suffixed with anything e.g. "foobar" will match:
* ... // foobar okay
* ... // WPCS: foobar whitelist.
*
* There is an exception, and that is when PHP is being interspersed with HTML.
* In that case, the comment should always come at the end of the statement (right
* before the closing tag, ?>). For example:
*
* <input type="text" id="<?php echo $id; // XSS OK ?>" />
*
* @since 0.4.0
* @since 0.14.0 Whitelist comments at the end of the statement are now also accepted.
*
* @deprecated 2.0.0 Use the PHPCS native `phpcs:ignore` annotations instead.
*
* @param string $comment Comment to find.
* @param integer $stackPtr The position of the current token in the stack passed
* in $tokens.
*
* @return boolean True if whitelisting comment was found, false otherwise.
*/
protected function has_whitelist_comment( $comment, $stackPtr ) {

// Respect the PHPCS 3.x --ignore-annotations setting.
if ( true === PHPCSHelper::ignore_annotations( $this->phpcsFile ) ) {
return false;
}

static $thrown_notices = array();

$deprecation_notice = 'Using the WPCS native whitelist comments is deprecated. Please use the PHPCS native "phpcs:ignore Standard.Category.SniffName.ErrorCode" annotations instead. Found: %s';
$deprecation_code = 'DeprecatedWhitelistCommentFound';
$filename = $this->phpcsFile->getFileName();

$regex = '#\b' . preg_quote( $comment, '#' ) . '\b#i';

// There is a findEndOfStatement() method, but it considers more tokens than
// we need to consider here.
$end_of_statement = $this->phpcsFile->findNext( array( \T_CLOSE_TAG, \T_SEMICOLON ), $stackPtr );

if ( false !== $end_of_statement ) {
// If the statement was ended by a semicolon, check if there is a whitelist comment directly after it.
if ( \T_SEMICOLON === $this->tokens[ $end_of_statement ]['code'] ) {
$lastPtr = $this->phpcsFile->findNext( \T_WHITESPACE, ( $end_of_statement + 1 ), null, true );
} elseif ( \T_CLOSE_TAG === $this->tokens[ $end_of_statement ]['code'] ) {
// If the semicolon was left out and it was terminated by an ending tag, we need to look backwards.
$lastPtr = $this->phpcsFile->findPrevious( \T_WHITESPACE, ( $end_of_statement - 1 ), null, true );
}

if ( ( \T_COMMENT === $this->tokens[ $lastPtr ]['code']
|| ( isset( Tokens::$phpcsCommentTokens[ $this->tokens[ $lastPtr ]['code'] ] )
&& \T_PHPCS_SET !== $this->tokens[ $lastPtr ]['code'] ) )
&& $this->tokens[ $lastPtr ]['line'] === $this->tokens[ $end_of_statement ]['line']
&& preg_match( $regex, $this->tokens[ $lastPtr ]['content'] ) === 1
) {
if ( isset( $thrown_notices[ $filename ][ $lastPtr ] ) === false
&& isset( Tokens::$phpcsCommentTokens[ $this->tokens[ $lastPtr ]['code'] ] ) === false
) {
$this->phpcsFile->addWarning(
$deprecation_notice,
$lastPtr,
$deprecation_code,
array( $this->tokens[ $lastPtr ]['content'] )
);

$thrown_notices[ $filename ][ $lastPtr ] = true;
}

return true;
}
}

// No whitelist comment found so far. Check at the end of the stackPtr line.
// Note: a T_COMMENT includes the new line character, so may be the last token on the line!
$end_of_line = $this->get_last_ptr_on_line( $stackPtr );
$lastPtr = $this->phpcsFile->findPrevious( \T_WHITESPACE, $end_of_line, null, true );

if ( ( \T_COMMENT === $this->tokens[ $lastPtr ]['code']
|| ( isset( Tokens::$phpcsCommentTokens[ $this->tokens[ $lastPtr ]['code'] ] )
&& \T_PHPCS_SET !== $this->tokens[ $lastPtr ]['code'] ) )
&& $this->tokens[ $lastPtr ]['line'] === $this->tokens[ $stackPtr ]['line']
&& preg_match( $regex, $this->tokens[ $lastPtr ]['content'] ) === 1
) {
if ( isset( $thrown_notices[ $filename ][ $lastPtr ] ) === false
&& isset( Tokens::$phpcsCommentTokens[ $this->tokens[ $lastPtr ]['code'] ] ) === false
) {
$this->phpcsFile->addWarning(
$deprecation_notice,
$lastPtr,
$deprecation_code,
array( $this->tokens[ $lastPtr ]['content'] )
);

$thrown_notices[ $filename ][ $lastPtr ] = true;
}

return true;
}

return false;
}

/**
* Check if a token is used within a unit test.
*
Expand Down
11 changes: 1 addition & 10 deletions WordPress/Sniffs/DB/PreparedSQLPlaceholdersSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -445,11 +445,6 @@ public function process_token( $stackPtr ) {
return;
}

$count_diff_whitelisted = $this->has_whitelist_comment(
'PreparedSQLPlaceholders replacement count',
$stackPtr
);

if ( 0 === $total_placeholders ) {
if ( 1 === $total_parameters ) {
if ( false === $variable_found && false === $sql_wildcard_found ) {
Expand All @@ -465,7 +460,7 @@ public function process_token( $stackPtr ) {
'UnnecessaryPrepare'
);
}
} elseif ( false === $count_diff_whitelisted && 0 === $valid_in_clauses['uses_in'] ) {
} elseif ( 0 === $valid_in_clauses['uses_in'] ) {
$this->phpcsFile->addWarning(
'Replacement variables found, but no valid placeholders found in the query.',
$i,
Expand All @@ -486,10 +481,6 @@ public function process_token( $stackPtr ) {
return;
}

if ( true === $count_diff_whitelisted ) {
return;
}

$replacements = $parameters;
array_shift( $replacements ); // Remove the query.

Expand Down
4 changes: 0 additions & 4 deletions WordPress/Sniffs/DB/PreparedSQLSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,6 @@ public function process_token( $stackPtr ) {
return;
}

if ( $this->has_whitelist_comment( 'unprepared SQL', $stackPtr ) ) {
return;
}

for ( $this->i; $this->i < $this->end; $this->i++ ) {

if ( isset( $this->ignored_tokens[ $this->tokens[ $this->i ]['code'] ] ) ) {
Expand Down
21 changes: 0 additions & 21 deletions WordPress/Sniffs/DB/SlowDBQuerySniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,27 +47,6 @@ public function getGroups() {
);
}

/**
* Processes this test, when one of its tokens is encountered.
*
* @since 0.10.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.
*/
public function process_token( $stackPtr ) {

if ( $this->has_whitelist_comment( 'slow query', $stackPtr ) ) {
return;
} elseif ( $this->has_whitelist_comment( 'tax_query', $stackPtr ) ) {
return;
}

return parent::process_token( $stackPtr );
}

/**
* Callback to process each confirmed key, to check value.
* This must be extended to add the logic to check assignment value.
Expand Down
12 changes: 0 additions & 12 deletions WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -263,18 +263,6 @@ public function getGroups() {
* normal file processing.
*/
public function process_token( $stackPtr ) {
/*
* Allow for whitelisting.
*
* Generally speaking a theme/plugin should *only* execute their own hooks, but there may be a
* good reason to execute a core hook.
*
* Similarly, newer PHP or WP functions or constants may need to be emulated for continued support
* of older PHP and WP versions.
*/
if ( $this->has_whitelist_comment( 'prefix', $stackPtr ) ) {
return;
}

// Allow overruling the prefixes set in a ruleset via the command line.
$cl_prefixes = trim( PHPCSHelper::get_config_data( 'prefixes' ) );
Expand Down
6 changes: 2 additions & 4 deletions WordPress/Sniffs/PHP/StrictComparisonsSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,8 @@ public function register() {
*/
public function process_token( $stackPtr ) {

if ( ! $this->has_whitelist_comment( 'loose comparison', $stackPtr ) ) {
$error = 'Found: ' . $this->tokens[ $stackPtr ]['content'] . '. Use strict comparisons (=== or !==).';
$this->phpcsFile->addWarning( $error, $stackPtr, 'LooseComparison' );
}
$error = 'Found: ' . $this->tokens[ $stackPtr ]['content'] . '. Use strict comparisons (=== or !==).';
$this->phpcsFile->addWarning( $error, $stackPtr, 'LooseComparison' );
}

}
5 changes: 0 additions & 5 deletions WordPress/Sniffs/Security/EscapeOutputSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,6 @@ public function process_token( $stackPtr ) {
}
}

// Checking for the ignore comment, ex: //xss ok.
if ( $this->has_whitelist_comment( 'xss', $stackPtr ) ) {
return;
}

if ( isset( $this->unsafePrintingFunctions[ $function ] ) ) {
$error = $this->phpcsFile->addError(
"All output should be run through an escaping function (like %s), found '%s'.",
Expand Down
4 changes: 0 additions & 4 deletions WordPress/Sniffs/Security/NonceVerificationSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,6 @@ public function process_token( $stackPtr ) {
return;
}

if ( $this->has_whitelist_comment( 'CSRF', $stackPtr ) ) {
return;
}

if ( $this->is_assignment( $stackPtr ) ) {
return;
}
Expand Down
4 changes: 0 additions & 4 deletions WordPress/Sniffs/Security/ValidatedSanitizedInputSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,6 @@ function ( $symbol ) {
);
}

if ( $this->has_whitelist_comment( 'sanitization', $stackPtr ) ) {
return;
}

// If this variable is being tested with one of the `is_..()` functions, sanitization isn't needed.
if ( $this->is_in_type_test( $stackPtr ) ) {
return;
Expand Down
5 changes: 0 additions & 5 deletions WordPress/Sniffs/WP/CapitalPDangitSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,6 @@ public function register() {
* normal file processing.
*/
public function process_token( $stackPtr ) {

if ( $this->has_whitelist_comment( 'spelling', $stackPtr ) ) {
return;
}

/*
* Ignore tokens within an array definition as this is a false positive in 80% of all cases.
*
Expand Down
25 changes: 2 additions & 23 deletions WordPress/Sniffs/WP/GlobalVariablesOverrideSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,6 @@ protected function process_list_assignment( $stackPtr ) {
*/
protected function process_variable_assignment( $stackPtr, $in_list = false ) {

if ( $this->has_whitelist_comment( 'override', $stackPtr ) === true ) {
return;
}

$token = $this->tokens[ $stackPtr ];
$var_name = substr( $token['content'], 1 ); // Strip the dollar sign.
$data = array();
Expand Down Expand Up @@ -410,34 +406,17 @@ protected function process_global_statement( $stackPtr, $in_function_scope ) {
}

if ( true === $this->is_assignment( $ptr ) ) {
$this->maybe_add_error( $ptr );
$this->add_error( $ptr );
continue;
}

// Check if this is a variable assignment within a `foreach()` declaration.
if ( $this->is_foreach_as( $ptr ) === true ) {
$this->maybe_add_error( $ptr );
$this->add_error( $ptr );
}
}
}

/**
* Add the error if there is no whitelist comment present.
*
* @since 0.11.0
* @since 1.1.0 - Visibility changed from public to protected.
* - Check for being in a test class moved to the process_token() method.
*
* @param int $stackPtr The position of the token to throw the error for.
*
* @return void
*/
protected function maybe_add_error( $stackPtr ) {
if ( $this->has_whitelist_comment( 'override', $stackPtr ) === false ) {
$this->add_error( $stackPtr );
}
}

/**
* Add the error.
*
Expand Down
2 changes: 1 addition & 1 deletion WordPress/Sniffs/WhiteSpace/PrecisionAlignmentSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public function process_token( $stackPtr ) {
break;
}

if ( $spaces > 0 && ! $this->has_whitelist_comment( 'precision alignment', $i ) ) {
if ( $spaces > 0 ) {
$this->phpcsFile->addWarning(
'Found precision alignment of %s spaces.',
$i,
Expand Down
6 changes: 3 additions & 3 deletions WordPress/Tests/DB/PreparedSQLPlaceholdersUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ $sql = $wpdb->prepare( "SELECT * FROM $wpdb->users WHERE id = %d AND user_login

$replacements = [1, "admin", $variable];
$sql = $wpdb->prepare( "SELECT * FROM $wpdb->users WHERE id = %d AND user_login = %s", $replacements ); // Bad.
$sql = $wpdb->prepare( "SELECT * FROM $wpdb->users WHERE id = %d AND user_login = %s", $replacements ); // WPCS: PreparedSQLPlaceholders replacement count OK.
$sql = $wpdb->prepare( "SELECT * FROM $wpdb->users WHERE id = %d AND user_login = %s", $replacements ); // Bad - old-style ignore comment. WPCS: PreparedSQLPlaceholders replacement count OK.

// Valid test case as found in WP core /wp-admin/includes/export.php
$esses = array_fill( 0, count($post_types), '%s' );
$where = $wpdb->prepare( "{$wpdb->posts}.post_type IN (" . implode( ',', $esses ) . ')', $post_types ); // Warning.
// Testing that whitelist comment work for this mismatch too.
$where = $wpdb->prepare( "{$wpdb->posts}.post_type IN (" . implode( ',', $esses ) . ')', $post_types ); // WPCS: PreparedSQLPlaceholders replacement count OK.
// Testing that ignore comment works for this mismatch too.
$where = $wpdb->prepare( "{$wpdb->posts}.post_type IN (" . implode( ',', $esses ) . ')', $post_types ); // Bad - old-style ignore comment. WPCS: PreparedSQLPlaceholders replacement count OK.

/*
* Test correctly recognizing queries using IN in combination with dynamic placeholder creation.
Expand Down
4 changes: 2 additions & 2 deletions WordPress/Tests/DB/PreparedSQLPlaceholdersUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ public function getWarningList() {
57 => 1,
58 => 1,
61 => 1,
62 => 1, // Whitelist comment deprecation warning.
62 => 1, // Old-style WPCS ignore comments are no longer supported.
66 => 1,
68 => 1, // Whitelist comment deprecation warning.
68 => 1, // Old-style WPCS ignore comments are no longer supported.
126 => 1,
139 => 1,
160 => 2,
Expand Down
Loading

0 comments on commit b430993

Please sign in to comment.