Page MenuHomePhabricator

Position of boolean operators inside an if condition
Open, MediumPublic

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.";

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.

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.

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

@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!

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

@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".

Krinkle triaged this task as Medium priority.Jul 16 2020, 10:22 PM
Krinkle moved this task from Untriaged to Accepted rule changes on the MediaWiki-Codesniffer board.