Page MenuHomePhabricator

Add sniff for checking that "else" is on same line as previous closing brace
Closed, ResolvedPublic

Description

As per Manual:CC/PHP, an elseif or else should be on the same line as the previous closing brace. We should have a sniff that checks this.

Details

Related Gerrit Patches:
mediawiki/tools/codesniffer : masterAdd IfElseStructureSniff to handle else structures

Event Timeline

polybuildr raised the priority of this task from to Needs Triage.
polybuildr updated the task description. (Show Details)
polybuildr added a subscriber: polybuildr.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 3 2015, 9:09 PM

Also, Manual:CC/PHP doesn't seem to explicitly say this right now. However, a quick grep in mw-core/includes shows:

$ grep "} else" -R | wc -l
5186
$ grep -E "^\s*else" -R | wc -l
60

so the page should probably say this explicitly. Editing page.

TasneemLo added a subscriber: TasneemLo.EditedSep 11 2015, 5:39 PM

This seems to already be there in phpcs

I get
Expected "} else {\n"; found "}\nelse {\n"
and
Expected "} elseif (...) {\n"; found "}\nelseif (...) {\n"

It is there in PEAR.Sniffs.ControlStructures.ControlSignatureSniff

EDIT :
Sorry, this wasnt being run with the MW ruleset.

Change 237733 had a related patch set uploaded (by TasneemLo):
Add ControlSignatureSniff to handle else & elseif

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

Could you assign this to me please ?

TasneemLo set Security to None.

Change 237733 merged by jenkins-bot:
Add IfElseStructureSniff to handle else structures

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

Legoktm closed this task as Resolved.Oct 7 2015, 7:12 AM
Legoktm added a subscriber: Legoktm.