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.

Details

Related Gerrit Patches:
mediawiki/tools/codesniffer : masterSniff to check assignment expressions in while,if
mediawiki/tools/codesniffer : masterSniff to check assignment in while & if

Event Timeline

Legoktm created this task.Mar 14 2015, 10:22 PM
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.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 14 2015, 10:22 PM

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

TasneemLo set Security to None.

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

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

Legoktm closed this task as Resolved.Oct 23 2015, 1:13 AM

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

Reason:
Superseded by beef13b4b59415b40c5777e3cf4f0c5f63dc8348

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

Physikerwelt added a subscriber: Physikerwelt.EditedOct 24 2015, 4:25 PM

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.

TasneemLo added a comment.EditedOct 25 2015, 3:57 AM

@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 );
}