WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.56k stars 487 forks source link

Allow prepared queries to be passed in via a variable #1331

Open johnbillion opened 6 years ago

johnbillion commented 6 years ago

Given the following code, a WordPress.WP.PreparedSQL.NotPrepared error is raised:

$query = $wpdb->prepare( "
    SELECT ID
    FROM {$wpdb->posts}
    WHERE post_type = %s
", $post_type );
$all_post_ids = $wpdb->get_col( $query );

The error is raised because the prepared query is passed in via a variable instead of prepare() being called directly inside get_col().

Is there a way that this format can be supported in WPCS?

JDGrimes commented 6 years ago

Duplicate of https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/469?

Sephster commented 6 years ago

I would like to see this rectified as well. The code being written is valid and conforms to the standard. This is a false detection and it would be preferable to fix this in the rules instead of peppering source code with exceptions for PHPCS.

pento commented 5 years ago

Looks like this is a problem for Core, too: I'm finding a lot of instances of this (perfectly valid) pattern.

dingo-d commented 5 years ago

Just thinking out loud here, in case code like above is found, the sniff should look what are the previous tokens (up to some point which can be right above the found issue, or even passed from another file in which case it's impossible to determine if the query is prepared or not) in the file, and if one of them matches prepare then this would be ok (just thinking out loud about it doesn't sound ok to me xD)? We should also see if the same variable passed to unprepared query contains the prepare statement...

Is this even possible (sounds highly unlikely)? Wouldn't it be easier to just refactor the offending code to include prepared statement?

@pento what is the number of such occurrences in the core?

pento commented 5 years ago

There are currently 116 WordPress.DB.PreparedSQL.NotPrepared errors in Core: I haven't manually checked each of them yet, but all the ones I've spot checked have been valid.

It's possible to refactor the simple examples to be $wpdb->query( $wpdb->prepare( ... ) ), but there are a lot of instances where the query is built up through a bunch of different conditions (eg, the various WP_Query classes).

3ynm commented 4 years ago

Same problem here. A prepared statement should not be required on get_var, get_results nor get_col, instead detected only when directly interpolating variables on a string.

eddr commented 2 weeks ago

Hi Any news? Important for basic coding..