Page MenuHomePhabricator

Position of boolean operators inside an if condition
Open, Needs TriagePublic

Description

While reading the code of SpaceyParanthesisSniff.php in CodeSniffer - I found the following snippet at Line 76 :

		if ( ( $nextToken['code'] === T_WHITESPACE &&
				strpos( $nextToken['content'], "\n" ) === false
				&& $nextToken['content'] != ' ' )
			|| ( $nextToken['code'] !== T_CLOSE_PARENTHESIS && $nextToken['code'] !== T_WHITESPACE ) ) {

I was wondering if there is a style guide for where the boolean operator (&&) should be kept - in the beginning of the next line or the end of the current line.

If there is a rule (couldn't find one in https://www.mediawiki.org/wiki/CC/PHP) - maybe there should be a sniff for this ?

Could also be extended for all operators - for example a long string - should the concatenation (dot .) operator be at the end of the line or at the beginning of the new line

$a = "a very long string which spans " .
    "multiple lines.";

OR

$a = "a very long string which spans "
    . "multiple lines.";

Details

Related Gerrit Patches:

Event Timeline

TasneemLo raised the priority of this task from to Needs Triage.
TasneemLo updated the task description. (Show Details)
TasneemLo added a subscriber: TasneemLo.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 26 2015, 4:08 AM
TasneemLo set Security to None.
TasneemLo updated the task description. (Show Details)Oct 27 2015, 7:15 AM

I think you're looking for https://www.mediawiki.org/wiki/Manual:Coding_conventions#Line_continuation ? The operator should be at the beginning of the next line.

Restricted Application added a subscriber: StudiesWorld. · View Herald TranscriptDec 20 2015, 9:15 AM
Aashaka claimed this task.Mar 25 2016, 11:06 AM

Change 279615 had a related patch set uploaded (by Aashaka):
Add sniff to check if boolean operator on same line inside 'if condition'

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

@Legoktm : Please review :)

@Aashaka: No need to ping about patch review in Phabricator two minutes after adding a patch. Thanks! :)

I understand. At that time, I had forgotten that Reviewer-bot asked reviewers to do patch review too.

https://www.mediawiki.org/wiki/Manual:Coding_conventions#Line_continuation has changed. It now suggests using operator at the end of the previous line. I have changed my patch accordingly!

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 19 2016, 1:28 PM

Any updates/reviews on this?

I think you're looking for https://www.mediawiki.org/wiki/Manual:Coding_conventions#Line_continuation ? The operator should be at the beginning of the next line.

I agree that that is better for readability. Although it seems the page has been changed since to instead prefer the operator to be at the end of the previous line..

The new PSR12.ControlStructures.BooleanOperatorPlacement from release 3.5.0 sounds a bit like this

https://github.com/squizlabs/PHP_CodeSniffer/releases

Added PSR12.ControlStructures.BooleanOperatorPlacement sniff
Enforces that boolean operators between conditions are consistently at the start or end of the line

Krinkle updated the task description. (Show Details)Sep 27 2019, 4:28 PM
Aklapper removed Aashaka as the assignee of this task.Nov 29 2019, 8:36 AM

@Aashaka: Thanks again for the patch. I am resetting the assignee of this task because there has not been progress lately (please correct me if I am wrong!).
Resetting the assignee avoids the impression that somebody is already working on this task. It also allows others to potentially work towards fixing this task.
Please claim this task again when you plan to work on it (via Add Action...Assign / Claim in the dropdown menu) - it would be welcome! Thanks for your understanding!

Personally, I don't really like the idea of having a strict rule for this. Not only because it's a matter of taste. From my experience the code is sometimes more readable when all boolean operators are at the beginning of the line, and sometimes when they are at the end of the line. In rare edge-cases this might even vary within a single file, sometimes even within a single if.

https://www.mediawiki.org/wiki/Manual:Coding_conventions#Line_continuation currently reflects this by saying "The operator separating the two lines should be placed consistently (always at the end or always at the start of the line)."

If I really, really need to pick a side, I vote for "always at the beginning".