Page MenuHomePhabricator

Assignment in conditions should be avoided
Open, MediumPublic

Description

After r74742, r74743, r74745 there are still 56 assignments in conditionals in MW phase3

Related to that r74727 was found (bug where = had been wrongly used in a comparison)

Details

Reference
bz25517

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:18 PM
bzimport set Reference to bz25517.
bzimport added a subscriber: Unknown Object (MLST).
Reedy created this task.Oct 13 2010, 11:35 PM

I'm not personally against them. Take http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/includes/ResourceLoader.php?view=markup#l324 for instance. I think it's clear that we aren't mistakenly doing assignment rather than comparison.

I'm not against them either, provided the code makes some attempt to show that it's not mistaking = for ==.

Fair enough. I'm fine with avoiding them if we are conforming to an established standard.

(In reply to comment #0)

[..] there are still 56 assignments in conditionals

Got a ack-grep regex to share?

(In reply to comment #5)

Got a ack-grep regex to share?

\(\s*\$[^=\n&|'");]+[^!<>=]=\s*\$[^\n)'";]+\)

Looks usable

Krinkle updated the task description. (Show Details)Dec 10 2014, 2:10 PM
Krinkle added a project: Technical-Debt.
Krinkle set Security to None.
Krinkle removed a subscriber: Unknown Object (MLST).
Krinkle removed a subscriber: Krinkle.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 17 2016, 9:06 PM