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

Sundry improvements of the VIP.DirectDatabaseQuery sniff #400

Merged
merged 5 commits into from
Jun 5, 2015
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Remove old code for preventing dup messages
This code was added long ago
(aa57302), but since then the sniff
has evolved to the point that it is no longer needed. Specifically, the
sniff is much more restrictive about when it flags `$wpdb`, limiting
the errors to a predefined list of methods. One reason that duplicate
messages used to be given on the same line was when you called
`$wpdb->query( $wpdb->prepare( /* … */ ) );`, and both methods was be
flagged. Because the `prepare()` method is no longer flagged, this
problem no longer occurs.

For good measure, I’ve also modified the sniff to return the
`$endOfStatment` pointer, which will tell PHPCS not to sniff any more
code until after the semicolon at the end of the statement. This would
also prevent duplicate errors, and may even provide a small performance
benefit as well.
JDGrimes committed Jun 5, 2015
commit 3bf09ea64437c20a8d60cae905b79df49f9b1695
40 changes: 7 additions & 33 deletions WordPress/Sniffs/VIP/DirectDatabaseQuerySniff.php
Original file line number Diff line number Diff line change
@@ -37,7 +37,7 @@ public function register()
* @param int $stackPtr The position of the current token
* in the stack passed in $tokens.
*
* @return void
* @return int|void
*/
public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
{
@@ -81,19 +81,17 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
$_pos = $stackPtr;
while ( $_pos = $phpcsFile->findNext( array( T_CONSTANT_ENCAPSED_STRING, T_DOUBLE_QUOTED_STRING ), $_pos + 1, $endOfStatement, null, null, true ) ) {
if ( preg_match( '#\b(ALTER|CREATE|DROP)\b#i', $tokens[$_pos]['content'], $matches ) > 0 ) {
$message = 'Attempting a database schema change is highly discouraged.';
$this->add_unique_message( $phpcsFile, 'error', $_pos, $tokens[$_pos]['line'], $message, 'SchemaChange' );
$phpcsFile->addError( 'Attempting a database schema change is highly discouraged.', $_pos, 'SchemaChange' );
}
}

// Flag instance if not whitelisted
if ( ! $whitelisted_db_call ) {
$message = 'Usage of a direct database call is discouraged.';
$this->add_unique_message( $phpcsFile, 'warning', $stackPtr, $tokens[$stackPtr]['line'], $message, 'DirectQuery' );
$phpcsFile->addWarning( 'Usage of a direct database call is discouraged.', $stackPtr, 'DirectQuery' );
}

if ( ! in_array( $method, $this->methods['cachable'] ) ) {
return;
return $endOfStatement;
}

$whitelisted_cache = false;
@@ -126,35 +124,11 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )

if ( ! $cached && ! $whitelisted_cache ) {
$message = 'Usage of a direct database call without caching is prohibited. Use wp_cache_get / wp_cache_set.';
$this->add_unique_message( $phpcsFile, 'error', $stackPtr, $tokens[$stackPtr]['line'], $message, 'NoCaching' );
}

}//end process()

/**
* Add unique message per line
* @param PHP_CodeSniffer_File $phpcsFile
* @param string $type (error|warning)
* @param int $pointer
* @param int $line
* @param string $message
* @param string $code The error code for this message.
* @return void
*/
function add_unique_message( PHP_CodeSniffer_File $phpcsFile, $type, $pointer, $line, $message, $code ) {
$messages = call_user_func( array( $phpcsFile, 'get' . ucfirst( $type . 's' ) ) );
if ( isset( $messages[$line] ) ) {
foreach ( $messages[$line] as $idx => $events ) {
foreach ( $events as $arr ) {
if ( $arr['message'] == $message ) {
return false;
}
}
}
$phpcsFile->addError( $message, $stackPtr, 'NoCaching' );
}

call_user_func( array( $phpcsFile, 'add' . ucfirst( $type ) ), $message, $pointer, $code );
}
return $endOfStatement;

}//end process()

}//end class