10up / phpcs-composer

Official 10up PHPCS rules.
MIT License
48 stars 8 forks source link

10up-Default standard has missing DB related rules from WordPress #40

Closed pabamato closed 1 year ago

pabamato commented 1 year ago

Describe the bug

While doing a code audit for a plugin I found that the 10up-Default standard is not flagging some relevant findings for querying the DB which are being flagged if the WordPress CS is used. For these cases, I would always prefer 10up-Default to flag those so I can manually review the code/function.

Other relevant issues not being flagged are related to caching DB call results and possible slow queries when using meta_query

I’ve tried WordPress-VIP-Go and these are being captured since the ruleset is importing some of the WordPress.DB.. rules ( see ruleset )

On Wordpress-Core, some of the DB-related rules are imported by adding the reference (see ruleset) but others rules are only available on the WordPress base rule set which is not imported by 10up-Default. I believe those relevant rules should be available on the 10up-Default standard but maybe there is a historical reason to not include those ?

Expected output I would expect 10up-Default to include and flag possible performance issues included in the following rules:

Steps to Reproduce

Run phpcs using 10up-Default CS on the following file and compare output aginast WordPress CS. ./vendor/bin/phpcs ./warning.php --standard="WordPress" --report=full -s ./vendor/bin/phpcs ./warning.php --standard="10up-Default" --report=full -s

<?php // phpcs:ignore Squiz.Commenting.FileComment.Missing
global $wpdb;

$table_name = Database::$table_name;

if ( false === $achievements ) {
    $achievements = $wpdb->get_results(
        $wpdb->prepare(
            'SELECT `user_id`, `post_id`, `points`, COUNT(*) c FROM %s WHERE `user_id` = %d GROUP BY `post_id`',
            array(
                $table_name,
                $atts['user_id'],
            )
        )
    );
}

$mysqli = new mysqli( 'localhost', 'my_user', 'my_password', 'my_db' );

$query_args = array(
    'post_type'      => 'post',
    'post_status'    => 'publish',
    'order'          => 'ASC',
    'posts_per_page' => 800,
    'meta_query'     => array(
        '0'        => array(
            'key'     => 'key1',
            'value'   => 'on',
            'compare' => '=',
        ),
        '1'        => array(
            'key'     => 'key2',
            'value'   => 'off',
            'compare' => 'exp_eq',
        ),
        'relation' => 'AND',
    ),
);

$the_query = new WP_Query( $query_args );

if ( $the_query->have_posts() ) {
    while ( $the_query->have_posts() ) {
        $the_query->the_post();
    }
    /* Restore original Post Data */
    wp_reset_postdata();
} else {
    echo esc_html__( 'Nothing found', 'text-domain' );
}

Screenshots, screen recording, code snippet

Monosnap 2023-04-17 12-27-00 test-phpcs 2023-04-17 12-28-42

Environment information

PHP 8.0+

WordPress information

No response

Code of Conduct

darylldoyle commented 1 year ago

@pabamato it looks like this was already merged, so I'm going to close this issue