		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 Changes in Gerrit:

	Subject	Author	Repo	Branch	Lines +/-
	Add sniff to check if boolean operator on next line inside multiline condition	Aashaka	mediawiki/tools/codesniffer	master	+123 -4

Customize query in gerrit

Related Objects

Mentioned In: T130382: GSoC/Outreachy Proposal for Improving static analysis tools for Mediawiki

Event Timeline

TasneemLo created this task.Oct 26 2015, 4:08 AM

TasneemLo raised the priority of this task from to Needs Triage.

TasneemLo updated the task description. (Show Details)

TasneemLo subscribed.

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 26 2015, 4:08 AM

TasneemLo added a project: MediaWiki-Codesniffer.Oct 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.

TasneemLo unsubscribed.Dec 20 2015, 9:15 AM

Restricted Application added a subscriber: StudiesWorld. · View Herald TranscriptDec 20 2015, 9:15 AM

Aashaka subscribed.Mar 25 2016, 8:36 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

gerritbot added a project: Patch-For-Review.Mar 25 2016, 3:19 PM

@Legoktm : Please review :)

Aashaka mentioned this in T130382: GSoC/Outreachy Proposal for Improving static analysis tools for Mediawiki.Mar 27 2016, 6:24 PM

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

https://gerrit.wikimedia.org/r/#/c/279615/ needs a rebase (says "Cannot Merge")...

In T116561#1759336, @Legoktm wrote:

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

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

Ricordisamoa subscribed.Jan 14 2020, 1:07 PM

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.

Umherirrender unsubscribed.Feb 27 2021, 12:25 PM

Pppery edited projects, added: Patch-Needs-Improvement; removed: Patch-For-Review.Apr 1 2023, 4:12 PM

In T116561#5701530, @thiemowmde wrote:

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

The quoted passage is not in the PHP code conventions page, but the shared/generic code conventions page. It also says:

Individual languages might have more specific rules.

I wrote that because in 2016, our JavaScript started to enforce once "after" line breaks, whereas our PHP code (and prior to that, our JavaScript) code has generally documented and preferred the "before" line breaks.

I realized this week that the reason for JavaScript to change was based on a perceived compatibility rule for "old JavaScript" parsers - parsers that were old in 2009. This fluke happened due to an accidental comeback of a legacy JSLint rule, that JSLint itself has long abandoned, and that we used to disable back when we used JSLint/JSHint. Details of the historical analysis about JavaScript at https://www.mediawiki.org/wiki/Topic:Y87vg2huramcg5w5.

Position of boolean operators inside an if conditionOpen, MediumPublicActions

Description

Details

Related Objects

Event Timeline

Position of boolean operators inside an if condition
Open, MediumPublic
Actions