After https://static-codereview.wikimedia.org/MediaWiki/74742.html, https://static-codereview.wikimedia.org/MediaWiki/74743.html, https://static-codereview.wikimedia.org/MediaWiki/74745.html there are still 56 assignments in conditionals in MW phase3
Related to that https://static-codereview.wikimedia.org/MediaWiki/74727.html was found (bug where = had been wrongly used in a comparison)
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | None | T27517 Assignment in conditions should be avoided | |||
| Open | Feature | None | T27407 Make SearchResultSet, and hence, subclasses, implement Iterator |
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 ==.
http://www.mediawiki.org/wiki/Manual:Coding_conventions#Assignment_expressions
Turns out it's in the coding conventions...
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
This has been enforced for a while via the Generic.CodeAnalysis.AssignmentInCondition.Found rule in CodeSniffer.