Core: No automated way to check Coding Style

[individual-it]

Steps to reproduce

1. write some code for ownCloud
2. open a PR with your changes
3. ask @phil-davis for approval

Expected behaviour

there should not be any code style issues

Actual behaviour

but you could not keep all of https://doc.owncloud.org/server/10.0/developer_manual/general/codingguidelines.html in your head or overlooked missing spaces etc.
so @phil-davis tells you about all your small mistakes you have done

Solution

why not letting the computer check for coding style rules
PHP Code Sniffer could be used for it.
An XML file with the code style standards could be put into the documentation and developers would be able to check their code locally and same tool could be run in CI.

Here is a configuration xml file that should contain all owncloud Standards. To run it install PHP Code Sniffer: pear install PHP_CodeSniffer or apt-get install php-codesniffer, save the XML code into a file and run it with phpcs --standard=./phpcs.xml yourCode.php

<?xml version="1.0"?>
<ruleset name="ownCloud Standard">
 <description>ownCloud coding standard</description>
 <arg name="colors"/>
 <arg value="sp"/>
 <arg name="tab-width" value="4"/>

 <rule ref="PEAR">
  <exclude name="Generic.WhiteSpace.DisallowTabIndent.TabsUsed"/>
  <exclude name="Generic.Commenting.DocComment.ShortNotCapital"/>
  <exclude name="Generic.Commenting.DocComment.SpacingAfter"/>
  <exclude name="Generic.Commenting.DocComment.MissingShort"/>
  <exclude name="Generic.Commenting.DocComment.TagValueIndent"/>
  <exclude name="Generic.Commenting.DocComment.NonParamGroup"/>
  <exclude name="PEAR.Commenting.FileComment.MissingVersion"/>
  <exclude name="PEAR.Commenting.FileComment.MissingCategoryTag"/>
  <exclude name="PEAR.Commenting.FileComment.MissingPackageTag"/>
  <exclude name="PEAR.Commenting.FileComment.MissingLicenseTag"/>
  <exclude name="PEAR.Commenting.FileComment.MissingLinkTag"/>
  <exclude name="PEAR.Commenting.ClassComment.MissingCategoryTag"/>
  <exclude name="PEAR.Commenting.ClassComment.MissingPackageTag"/>
  <exclude name="PEAR.Commenting.ClassComment.MissingAuthorTag"/>
  <exclude name="PEAR.Commenting.ClassComment.MissingLicenseTag"/>
  <exclude name="PEAR.Commenting.ClassComment.MissingLinkTag"/>
  <exclude name="PEAR.Commenting.FunctionComment.MissingParamComment"/>
  <exclude name="PEAR.Commenting.FunctionComment.SpacingAfterParamType"/>
  <exclude name="PEAR.Commenting.FunctionComment.SpacingAfterParamName"/>
  <exclude name="PEAR.NamingConventions.ValidVariableName.PrivateNoUnderscore"/>
  <exclude name="PEAR.NamingConventions.ValidFunctionName.PrivateNoUnderscore"/>
  <exclude name="PEAR.WhiteSpace.ScopeIndent.IncorrectExact"/>
  <exclude name="PEAR.Functions.FunctionDeclaration.BraceOnSameLine"/>
  <exclude name="PEAR.Classes.ClassDeclaration.OpenBraceNewLine"/>
 </rule>

 <rule ref="Squiz.WhiteSpace.OperatorSpacing.NoSpaceBefore"/>
 <rule ref="Squiz.WhiteSpace.OperatorSpacing.NoSpaceAfter"/>
 <rule ref="Generic.WhiteSpace.ScopeIndent"/>
 <rule ref="Squiz.Strings.ConcatenationSpacing">
   <properties>
   <property name="spacing" value="1"/>
   <property name="ignoreNewlines" value="true"/>
  </properties>
 </rule>
 <rule ref="PSR2.ControlStructures.SwitchDeclaration.BreakIndent">
  <exclude name="PSR2.ControlStructures.SwitchDeclaration.SpaceBeforeColonDEFAULT"/>
  <exclude name="PSR2.ControlStructures.SwitchDeclaration.SpaceBeforeColonCASE"/>
 </rule>
 <rule ref="Generic.Functions.OpeningFunctionBraceKernighanRitchie"/>
 <rule ref="Generic.Classes.OpeningBraceSameLine"/>
</ruleset>

[DeepDiver1975]

Pr is welcome

[phil-davis]

Oh, I will be out of a job ;)

Various projects use different format conventions - e.g. PSR-1 is a little different and uses class and function like:

class ClientController extends FormController
{
    public function stuff($param = 1)
    {
        return $param + 1;
    }
}

where the "{" goes by itself on the following line.

Edit: PEAR also defines it like that:
http://pear.php.net/manual/en/standards.classdef.php
http://pear.php.net/manual/en/standards.funcdef.php
but ownCloud is doing:

class ClientController extends FormController {
    public function stuff($param = 1) {
        return $param + 1;
    }
}

hmmm - did PEAR change at some time?

There will be no realistic way to change these sort of standards in a big existing project - it makes a huge code-formatting diff that messes with blame in history and makes backports annoying and...

But I am just mentioning it here in case somebody is really wanting to switch to PSR-1 or...

[individual-it]

PR with a phpcs configuration file https://github.com/owncloud/core/pull/28519
documentation PR https://github.com/owncloud/documentation/pull/3247

[phil-davis]

Note:
On Ubuntu 16.04 LTS:

sudo apt-get install php-codesniffer
phpcs --version
PHP_CodeSniffer version 2.5.1 (stable) by Squiz (http://www.squiz.net)

and version 2.5.1 complains when using this phpcs.xml

sudo pear install PHP_CodeSniffer
phpcs --version
PHP_CodeSniffer version 3.0.2 (stable) by Squiz (http://www.squiz.net)

version 3.0.2 handles the settings in phpcs.xml just fine.

So you need to get phpcs >= v3 from wherever.

[individual-it]

specified the version in the documentation https://github.com/owncloud/documentation/pull/3247

[settermjd]

Closing as the related, implementing PR is now merged. @individual-it really appreciate you taking the time to do this.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.