search

WordPress / WordPress-Coding-Standards

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

allow assignment in condition for fgetcsv #1705

Closed kkmuffme closed 1 year ago

kkmuffme commented 5 years ago

Bug Description

fgetcsv reads the file line by line, thus we need to assign the data to a variable in the condition like:

while ( ( $data = fgetcsv( $handle, 0, ',' ) ) !== false ) {

However this triggers: WordPress.CodeAnalysis.AssignmentInCondition.Found Variable assignment found within a condition. Did you mean to do a comparison?

Minimal Code Snippet

while ( ( $data = fgetcsv( $handle, 0, ',' ) ) !== false ) {
    echo $data;
}

Tested Against develop branch?

jrfnl commented 5 years ago

@kkmuffme IMO, this is not an issue.

  1. The sniff actually has a separate error code for while conditions so if there are good reasons for it, you can selectively exclude that specific error message.
  2. This code can be rewritten to a do-while construct without assignment in condition.
kkmuffme commented 5 years ago

@jrfnl 1) thanks I am aware I can disable a rule, however for fgetcsv it ALWAYS has to be disabled in a while loop. 2) I think this cannot be rewritten without making the code longer & ugly. Bc in a do while loop the reading would happen after the first iteration, thus be empty & not the same thing.

jrfnl commented 5 years ago

The handbook now contains the rule that assignments in conditions are not allowed.

It's perfectly fine if you don't want to comply and you clearly know how to exclude a rule, but that is not a reason to adjust the sniff.

kkmuffme commented 5 years ago

So there is no valid way to process files/csv with fgetcsv, if you want to adhere to the WPCS? Isn't this clearly a bug?

jrfnl commented 5 years ago

@kkmuffme There are perfectly valid ways to do this, such as the do ... while I suggested, which may be a little more verbose, but will work perfectly fine. In other words: this is not a bug.