Add coding-standard rules with PHP-CS-Fixer

[Slamdunk]

• Add coding-standard rules with .php_cs file
• Require PHP-CS-Fixer package on dev
• Run coding-standard checks on the build of the lowest supported PHP version to avoid BC breaks
• Apply coding-standard (PENDING: this will be done only at the and of this PR review to ease the inspection of the config files; rules changes can be seen in the Travis build that intentionally fails)

[alecpl]

Thank you for working on this. We're used to some style rules and we would like to keep them. It is:

1. else (and catch, etc.) in a new line. Prefer else if over elseif.
2. Keep double-quotes for long text, e.g. error messages, sql queries (even if there's no variable inside). Use single quotes for labels (array keys).

[Slamdunk]

else (and catch, etc.) in a new line.

Done. Pay attention that now every control structure is on a new line, so also foreach, while etc.

Prefer else if over elseif.

Done, but strongly discourage this because:

1. elseif and else if are different
2. To be completely explicit, PHP-CS-Fixer changes this:

   if (1 === $var) {
       doThis();
   } else if (2 === $var) {
       doThat();
   } else {
       doOther();
   }

   to this:

   if (1 === $var) {
       doThis();
   } else {
       if (2 === $var) {
           doThat();
       } else {
           doOther();
       }
   }

Keep double-quotes for long text, e.g. error messages, sql queries (even if there's no variable inside). Use single quotes for labels (array keys).

Done.

[alecpl]

I want:

if () {
}
else if {
}
else {
}

From what I see you moved all open brackets to a new line. Also your change to else if is not acceptable, it generates additional indentation level (because of wrapping the code inside else block). Is it not possible to keep else if as we use it right now? I know the difference between elseif and else if and want to keep the code unchanged anyway. If it's not possible we'd rather use elseif.

Thanks again for your work, it's really appreciated.

[Slamdunk]

To be consistent, the fixer handles the newline to all control structures, it's not possible to choose which one yes and which one no.
It is also not possible to achieve, with this tool, what you want for the elseif clause.
I've reverted the configuration to the first commit, except for the quotes: at the time being, you can only choose to use the proposed rules, or to disable at all those rules and missing the automatism of the tool for those code style.

Please note that even if my language is a bit strict, I do not want to impose anything.

I believe it is important to:

1. Have a coding-standard (as this project has)
2. Force coding-standard adoption
3. Let the dirty job to computer: humans should only create and not wasting time with indentations

For this list PHP-CS-Fixer emerged as a common tool in the PHP world, but also chose some conventions over the other (for good reasons most of the times), and the fact that it was born as a Symfony project it skyrocketed those specific conventions thanks to Symfony popularity.

Feel free to accept only what you want; as an OOS maintainer myself, between custom rules and automation, if I must choose one of the two I choose automation.

[alecpl]

If we can do an exception for function line I'm sure we could do for else, but probably this requires some code writing.

I also noted this:

- if ($test['part'] == 'user' || $test['part'] == 'detail') {
+ if ('user' == $test['part'] || 'detail' == $test['part']) {

I really don't like it. Do you know how to disable this?

[Slamdunk]

I really don't like it. Do you know how to disable this?

It's called yoda_style (PHP-CS-Fixer/PHP-CS-Fixer#2446), used to prevent unintentional variable assignment.

Btw I've disabled it.

[alecpl]

Except "else/elseif/catch/etc in a new line" we're almost ok. Last two things:

- 'onchange' => $this->rc->task != 'mail' ? 'rcmail.managesieve_set()' : ''));
+ 'onchange' => $this->rc->task != 'mail' ? 'rcmail.managesieve_set()' : '', ));

Adding comma after the last array item makes sense only if every item of the array is in a separate line, not for "inline" cases.

I'd also disable a rule for this:

- $action_script .= " :seconds " . intval($action['seconds']);
+ $action_script .= " :seconds " . (int) ($action['seconds']);

Performance impact of intval() is negligible on recent PHP versions. I use type casting when I don't have to bother with brackets, but sometimes intval() just looks simpler.

[Slamdunk]

I've disabled modernize_types_casting so now it keeps intval.

Regarding the array comma, the fix is already as you expect. Single inline array are left without ending commas, multiline array must have trailing commas:

$arr = array(1,2,3);
$arr = array(
    1,
    2,
    3,
);

The example you provided is a mixed type:

roundcubemail/plugins/managesieve/lib/Roundcube/rcube_sieve_engine.php

Lines 1276 to 1279 in c26c856

	else {
	$select = new html_select(array('name' => '_set', 'id' => $attrib['id'],
	'onchange' => $this->rc->task != 'mail' ? 'rcmail.managesieve_set()' : ''));

This is not an inline array: the author should have written this like the following:

$select = new html_select(array(
    'name' => '_set',
    'id' => $attrib['id'],
    'onchange' => $this->rc->task != 'mail' ? 'rcmail.managesieve_set()' : '',
));

[dereks]

Thank you for submitting a Pull Request (PR) to the Roundcube GitHub project.

You are receiving this message because your PR has conflicts which need to be resolved. We are trying to catch up on our backlog of old PRs and get them merged in (where appropriate). Therefor, we request the following:

Step 1. Rebase from the latest Roundcube branch master into your PR.

git fetch upstream
git checkout your_feature_branch_name
git rebase upstream/master
git push -f origin your_feature_branch_name

Step 2. Re-test to make sure your new code still works as expected

Step 3. Comment back here once it has been tested and will merge cleanly.

Once this has been done we will treat it like a new Pull Request and consider it for acceptance.

Apologies for the inconvenience. Thank you for contributing to Roundcube!