Details
-
Improvement
-
Status: Open
-
Low
-
Resolution: Unresolved
-
None
Description
The problem
At the moment we have indentation rules which basically say "4 spaces indentation when you wrap a line, except these cases" and then goes to list 4 common exceptions to this, and a further 2 specific cases where the exceptions do not apply.
Personally I find this very confusing and as an integrator I regularly see these rules mixed-up. In particular, I regularly see a second line of parameters in a function signature indented in line with the parameters in the first line:
public static function something(bool $someparam, bool $someotherparam, int $yetanother,
|
int $countalloftheparameterswehavehere, bool $areyoukiddingme): void {
|
}
|
Per our current coding style the following are acceptable:
public static function something(bool $someparam, bool $someotherparam, int $yetanother,
|
int $countalloftheparameterswehavehere, bool $areyoukiddingme): void {
|
}
|
|
// OR
|
|
public static function something(
|
bool $someparam,
|
bool $someotherparam,
|
int $yetanother,
|
int $countalloftheparameterswehavehere,
|
bool $areyoukiddingme
|
): void {
|
}
|
Equally I also regularly see the function signature rule applied to function calls, which is one of the explicitly named exceptions to the exceptions:
$this->something($someparam, $someotherparam, $yetanother,
|
$countalloftheparameterswehavehere, $areyoukiddingme);
|
Per our current coding style the following are acceptable:
$this->something($someparam, $someotherparam, $yetanother,
|
$countalloftheparameterswehavehere, $areyoukiddingme);
|
// OR
|
$this->something(
|
$someparam,
|
$someotherparam,
|
$yetanother,
|
$countalloftheparameterswehavehere,
|
$areyoukiddingme
|
);
|
Meanwhile PSR-12 has a simple rule for indenting:
Code MUST use an indent of 4 spaces for each indent level, and MUST NOT use tabs for indenting.
https://www.php-fig.org/psr/psr-12/#24-indenting
It also has separate, simple, rules for the various cases which are all basically the same and boil down to these simple points:
- If you split arguments/expressions across multiple lines then you may only have one item per line
- You only indent once
- The open/close braces/brackets must be on their own line.
- Argument lists MAY be split across multiple lines, where each subsequent line is indented once. When doing so, the first item in the list MUST be on the next line, and there MUST be only one argument per line.
- When the argument list is split across multiple lines, the closing parenthesis and opening brace MUST be placed together on their own line with one space between them.
https://www.php-fig.org/psr/psr-12/#45-method-and-function-arguments
- Argument lists MAY be split across multiple lines, where each subsequent line is indented once. When doing so, the first item in the list MUST be on the next line, and there MUST be only one argument per line. A single argument being split across multiple lines (as might be the case with an anonymous function or array) does not constitute splitting the argument list itself.
https://www.php-fig.org/psr/psr-12/#47-method-and-function-calls
- Expressions in parentheses MAY be split across multiple lines, where each subsequent line is indented at least once. When doing so, the first condition MUST be on the next line. The closing parenthesis and opening brace MUST be placed together on their own line with one space between them. Boolean operators between conditions MUST always be at the beginning or at the end of the line, not a mix of both.
https://www.php-fig.org/psr/psr-12/#51-if-elseif-else
https://www.php-fig.org/psr/psr-12/#52-switch-case
https://www.php-fig.org/psr/psr-12/#53-while-do-while
https://www.php-fig.org/psr/psr-12/#54-for
- Argument lists and variable lists MAY be split across multiple lines, where each subsequent line is indented once. When doing so, the first item in the list MUST be on the next line, and there MUST be only one argument or variable per line.
- When the ending list (whether of arguments or variables) is split across multiple lines, the closing parenthesis and opening brace MUST be placed together on their own line with one space between them.
https://www.php-fig.org/psr/psr-12/#7-closures
- Lists of implements and, in the case of interfaces, extends MAY be split across multiple lines, where each subsequent line is indented once. When doing so, the first item in the list MUST be on the next line, and there MUST be only one interface per line.
https://www.php-fig.org/psr/psr-12/#41-extends-and-implements
That is to say that the following are acceptable:
// Wrapping is optional if the line is below the line limit:.
|
public static function something(bool $someparam, bool $someotherparam, int $yetanother): void;
|
|
$this->something($someparam, $someotherparam, $yetanother);
|
|
if ($someparam && $someotherparam && $yetanother) {
|
}
|
|
// When wrapping:
|
// - each param/argument is indented once; and
|
// - only one param/argument per line; and
|
// - the first argument must be on its own line, and not next ot the function name; and
|
// - the last argument must be on its own line, and not with the closing bracket.
|
public static function something(
|
bool $someparam,
|
bool $someotherparam,
|
int $yetanother
|
): void;
|
|
$this->something(
|
$someparam,
|
$someotherparam,
|
$yetanother
|
);
|
|
|
if (
|
$someparam
|
&& $someotherparam
|
&& $yetanother
|
) {
|
}
|
|
|
And the following are not:
// When wrapping, all params/arguments must be on their own line
|
public static function something(bool $someparam, bool $someotherparam, int $yetanother,
|
int $countalloftheparameterswehavehere, bool $areyoukiddingme): void;
|
|
public static function something(bool $someparam, bool $someotherparam, int $yetanother,
|
int $countalloftheparameterswehavehere, bool $areyoukiddingme): void;
|
|
$this->something($someparam, $someotherparam, $yetanother,
|
$countalloftheparameterswehavehere, $areyoukiddingme);
|
|
$this->something($someparam, $someotherparam, $yetanother,
|
$countalloftheparameterswehavehere, $areyoukiddingme);
|
|
if ($someparam && $someotherparam
|
&& $yetanother) {
|
}
|
|
if ($someparam && $someotherparam
|
&& $yetanother) {
|
}
|
|
// Some arguments on the first line
|
// When wrapping, all params/arguments must be on their own line
|
public static function something(bool $someparam,
|
bool $someotherparam,
|
int $yetanother,
|
int $countalloftheparameterswehavehere,
|
bool $areyoukiddingme
|
): void;
|
Proposal
I would propose that we amend the coding style to optionally adopt the use of the PSR-12 styles, with the view to preferring them for new code.
As usual, existing code SHOULD NOT be amended, and it is up to the author as to how this should be adopted when addign or editing code in existing files which use the legacy style.
I would therefore suggest the following three questions be posed:
Question 1
On the question of whether the PSR-12 style of line-wrapping may be allowed:
A: No, the PSR-12 line-wrapping style should be rejected. The existing Moodle line-wrapping should be continued.
B: Yes, the PSR-12 line-wrapping style should be allowed.
Question 2
Should the PSR-12 style be accepted (Option B in the previous question):
On the question of whether the PSR-12 style of line-wrapping should be the preferred style
A: The Moodle style should be the preferred style. PSR-12 style will be accepted.
B: The PSR-12 style should be the preferred style for all new code.
C: The PSR-12 style should be the preferred style for all new, and existing code.
On the question of whether the PSR-12 style of line-wrapping should entirely replace the Moodle style
A: The Moodle style will be allowed in all code.
B: The Moodle style will be allowed only in legacy code where existing files are updated.
C: The Moodle style will be phased out for use in all code. Existing content will not be bulk-updated.
Attachments
Issue Links
- has a non-specific relationship to
-
MDLSITE-2132 RFC: Clarify the line wrapping/indentation rules
-
- Resolved
-