Page MenuHomePhabricator

Add sniff for "if/while ( $a = foo() )" constructs in phpcs
Closed, ResolvedPublic

Description

Per CC/PHP:

if ( $a = foo() ) {
    bar();
}

should not be used.

Same with:

$res = $dbr->query( 'SELECT * FROM some_table' );
while ( $row = $dbr->fetchObject( $res ) ) {
    showRow( $row );
}

A check for that should be implemented. Should also check if a sniff for this has been written already and just needs to be added to our config.

Event Timeline

Legoktm raised the priority of this task from to Needs Triage.
Legoktm updated the task description. (Show Details)
Legoktm added a project: MediaWiki-Codesniffer.
Legoktm added a subscriber: Legoktm.

Change 196873 had a related patch set uploaded (by Sumit):
Sniff to check assignment expressions in while,if

https://gerrit.wikimedia.org/r/196873

Sumit, may I attempt this ? or will you be fixing the patch ?

Change 237902 had a related patch set uploaded (by TasneemLo):
Sniff to check assignment in while & if

https://gerrit.wikimedia.org/r/237902

Change 237902 merged by jenkins-bot:
Sniff to check assignment in while & if

https://gerrit.wikimedia.org/r/237902

Change 196873 abandoned by Legoktm:
Sniff to check assignment expressions in while,if

Reason:
Superseded by beef13b4b59415b40c5777e3cf4f0c5f63dc8348

https://gerrit.wikimedia.org/r/196873

I'm wondering how to avoid assignment in while loops completely
see for example http://php.net/manual/en/function.fgetcsv.php

While I this new feature certainly discovered a bug
https://gerrit.wikimedia.org/r/#/c/248626/1/includes/ImportCsv.php
I think the new version is at least correct.

@Physikerwelt
I think this might be more appropriate and easy to read :

$line = fgetcsv( $csv_file );
if ( $line === NULL ) {
    // throw error for invalid handle
}
while ( $line !== false ) {
    // process the csv line here

    $line = fgetcsv( $csv_file );
}