WordPress / WordPress-Coding-Standards

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

PHPCBF only partially fixes spaces #1128

Closed mikaelz closed 7 years ago

mikaelz commented 7 years ago

With phpcbf --standard=phpcs.xml phpcstest.php some spaces are missing

pastebin of phpcs.xml. installed_paths is at master 1f64b1a0b5b789822d0303436ee4e30e0135e4dc

Before:

function processOnHoldOrder($order) {
    if ('gopaybinder' == $order->get_payment_method() && $days_old > 1) {
        $order->update_status('failed', 'Update by '.JOB_NAME);
        fwrite($log, "Changed status to failed: {$order->id}".PHP_EOL);
    }

    if ('bacs' == $order->get_payment_method() && $days_old >= 14) {
        $order->update_status('failed', 'Update by '.JOB_NAME);
        fwrite($log, "Changed status to failed: {$order->id}".PHP_EOL);
    } elseif ('bacs' == $order->get_payment_method() && $days_old >= 3 && $days_old < 14) {
        if (!empty($mails)) {
            foreach ($mails as $mail) {
                if ($mail->id == 'new_order') {
                    $mail->trigger($order);
                    $order->add_order_note('Email notification sent by '.JOB_NAME, false, true);
                    fwrite($log, "Sent email notification: {$order->id}".PHP_EOL);
                }
            }
        }
    }
}

After:

function processOnHoldOrder( $order) {  // only one space
    if ('gopaybinder' == $order->get_payment_method() && $days_old > 1) {
        $order->update_status( 'failed', 'Update by ' . JOB_NAME );
        fwrite( $log, "Changed status to failed: {$order->id}" . PHP_EOL );
    }

    if ('bacs' == $order->get_payment_method() && $days_old >= 14) { // no spaces
        $order->update_status( 'failed', 'Update by ' . JOB_NAME );
        fwrite( $log, "Changed status to failed: {$order->id}" . PHP_EOL );
    } elseif ('bacs' == $order->get_payment_method() && $days_old >= 3 && $days_old < 14) { // no spaces
        if ( ! empty( $mails )) { // missing space
            foreach ($mails as $mail) { // no spaces
                if ($mail->id == 'new_order') { // no spaces
                    $mail->trigger( $order );
                    $order->add_order_note( 'Email notification sent by ' . JOB_NAME, false, true );
                    fwrite( $log, "Sent email notification: {$order->id}" . PHP_EOL );
                }
            }
        }
    }
}
GaryJones commented 7 years ago

You don't seem to have WordPress-Core in your phpcs.xml, so you're missing Sniffs from the WordPress.Whitespace and Generic.Whitespace categories, for example.

Note, that WordPress-Extra does not include WordPress-Core.

You're also excluding WordPress.WhiteSpace.ControlStructureSpacing.

mikaelz commented 7 years ago

Thank you. Enabling WordPress.WhiteSpace.ControlStructureSpacing solved the issue.

I wrote the xml based on the repo sample, https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/develop/phpcs.xml.dist.sample#L33

jrfnl commented 7 years ago

Note, that WordPress-Extra does not include WordPress-Core

@GaryJones Eh... last time I checked, Extra did include Core: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/develop/WordPress-Extra/ruleset.xml#L66

@mikaelz Ah, yes, excluding sniffs is always a choice, but can have unexpected side-effects if you do.

Glad this is solved now.

jrfnl commented 7 years ago

The take-away here may be that we should comment out the example exclusions in the sample file to avoid people blindly copying them over.

GaryJones commented 7 years ago

Eh... last time I checked, Extra did include Core

Well, that's buried!