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

3.0: Remove deprecated Sniff::has_whitelist_comment() method and all references to it #1908

Merged
merged 4 commits into from
Jul 6, 2020
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
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