Difference between revisions of "Development/Coding standard"

From AtoM wiki
(empty())
(Update name of .php-cs-fixer.dist.php file)
 
(14 intermediate revisions by 4 users not shown)
Line 2: Line 2:
 
[[Main Page]] > [[Development]] > Development/Coding standard
 
[[Main Page]] > [[Development]] > Development/Coding standard
  
On this page you will find an overview of some of the coding practices we follow when developing AtoM, and which we look for as part of our [[Development/Code review|Code review]] process. The AtoM project is based on the symfony (1.4) framework, which is written in PHP. We've adopted the following standards, in addition to [http://trac.symfony-project.org/wiki/HowToContributeToSymfony#CodingStandards symfony's coding standard] and best practices for PHP development.  
+
AtoM uses PHP CS Fixer to check and auto-format the PHP code following the [https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/2.18/doc/ruleSets/PhpCsFixer.rst @PhpCsFixer ruleset], a highly opinionated extension of the [https://symfony.com/doc/current/contributing/code/standards.html#symfony-coding-standards-in-detail Symfony Coding Standards] and the [https://www.php-fig.org/psr/psr-12/ PSR-12 coding style specification].
  
In general, we recommend that anyone wanting to work with AtoM code familiarize themselves with git, the PHP Symfony 1.x framework, Twitter Bootstrap (2.3), and Elasticsearch. You should also be comfortable working in the command-line! Below are a few basic resources to get you started:
+
The PHP CS Fixer tool is included in the AtoM project's Composer dependencies for development.  PHP CS Fixer's configuration is tracked as part of the project's source code (see [https://github.com/artefactual/atom/blob/qa/2.x/.php-cs-fixer.dist.php .php-cs-fixer.dist.php]) and it's used in the [https://github.com/artefactual/atom/actions/workflows/syntax-checks.yml Continuous Integration process] to check the code on every pull request and commit merged to the <code>stable/**</code> and <code>qa/**</code> branches.
  
* Git Book: http://www.git-scm.com/book
+
Contributors to the AtoM project should run PHP CS Fixer locally to ensure their modifications meet the coding standards. There are a number of options for running PHP CS Fixer on your code: [https://github.com/FriendsOfPHP/PHP-CS-Fixer#editor-integration PHP CS Fixer's README file] links to plugins for several popular code editors (e.g. VS Studio Code, atom.io, Sublime text), you can configure a git [https://itnext.io/learning-to-add-git-hook-tasks-php-cs-fixer-41f34d99aa8a pre-commit hook to run PHP CS Fixer when commiting changes], or PHP CS Fixer  can be [https://github.com/FriendsOfPHP/PHP-CS-Fixer#usage run manually].
* Symfony 1.x documentation: http://symfony.com/legacy/doc
 
* Elasticsearch documentation: http://www.elasticsearch.org/guide/
 
* Unix/Linux command cheat sheet: http://fosswire.com/post/2007/08/unixlinux-command-cheat-sheet/
 
  
<admonition type="seealso">
+
== Coding Standards that are not handled by PHP CS Fixer ==
* [[Resources/Code repository|Code repository]]
 
* [[Development/Code review|Code review]]
 
</admonition>
 
  
 +
While PHP CS Fixer covers a lot of rules to meet the coding standard, there are a few cases that need to be handled manually, especially for multi-line statements. [https://www.php-fig.org/psr/psr-12/#23-lines The PSR-12 standard] doesn't impose a hard limit on line length, but it recommends a soft limit of 120 characters, and to keep lines under 80 characters where possible. However, the following cases need to be manually edited at the moment.
  
== Variable names ==
+
=== Multi-line method calls ===
  
We prefer, whenever possible, using camelCase variables:
+
Due to an issue formatting the Symfony templates, the <code>ensure_fully_multiline</code> option of the <code>method_argument_space</code> rule is currently disabled. Nevertheless, as noted in [https://www.php-fig.org/psr/psr-12/#47-method-and-function-calls the PSR-12 standard]:
  
<pre>
+
<blockquote>
$myVariable = 5; // Preferred
+
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.
$my_variable = 5; // Not preferred
+
</blockquote>
</pre>
 
 
 
There are a lot of variations in what is considered to be camelCase - we prefer starting with a lower-case letter.  
 
 
 
<admonition type="seealso">
 
* https://en.wikipedia.org/wiki/CamelCase
 
</admonition>
 
 
 
 
 
== Braces ==
 
 
 
Always use braces, even when not necessary:
 
  
 
<pre>
 
<pre>
if ($x) // Preferred
+
<?php
{
 
  myFunc();
 
}
 
  
if ($x) // Not preferred
+
$foo->bar(
  myFunc();
+
    $longArgument,
 +
    $longerArgument,
 +
    $muchLongerArgument
 +
);
 
</pre>
 
</pre>
  
Also, always ensure braces are on their own lines:
+
'''N.B.''': AtoM argument lists SHOULD be split into multiple lines, according to the rules above, to keep the line length under 80 characters.
<pre>
 
if ($x) // Preferred
 
{
 
  //...
 
}
 
  
if ($x) { // Not preferred
+
=== Multi-line control structures ===
  // ...
 
}
 
  
</pre>
+
[https://www.php-fig.org/psr/psr-12/#5-control-structures The PSR-12 standard on control structures] is not fully covered by PHP CS Fixer currently, but control structure formatting follows the same pattern as the method call formatting, with the additional rule:
  
 
+
<blockquote>
== PHP open tags ==
+
Boolean operators between conditions MUST always be at the beginning or at the end of the line, not a mix of both.
 
+
</blockquote>
Never use PHP short open tag (<code><?</code>); instead, please use the full tag  <code><?php</code> instead.
 
 
 
To ensure you remember this during development, you can disable short open tags support in your PHP environment (short_open_tag directive in your php.ini file).
 
 
 
== count() vs. sizeof() ==
 
 
 
Always use <code>count()</code>. Also see below for using <code>count()</code> to test for empty arrays.
 
 
 
 
 
== empty() ==
 
 
 
Never use <code>empty()</code> for checking whether an array value is empty. Instead always use <code>0 < count()</code>. This is because our arrays may be objects implementing the ArrayAccess interface, in which case  <code>empty()</code> may not behave as expected.
 
 
 
See: http://www.symfony-project.org/book/dev/07-Inside-the-View-Layer#Escaping%20Arrays%20and%20Objects
 
 
 
An advantage of using <code>count() > 0</code> over <code>empty()</code> is that <code>empty()</code> cannot be called using variable functions.
 
 
 
See: http://php.net/empty
 
 
 
<code>empty()</code> ''is a language construct and doing so is a syntax error.''
 
 
 
== Templates ==
 
 
 
Don't use trailing semicolons in templates unless required. Generally the trailing semicolon is implied by the PHP close tag. The trailing semicolon is only required after [http://www.symfony-project.org/book/1_1/04-The-Basics-of-Page-Creation#Adding%20a%20Template alternative PHP syntax] end control statements (endif, endforeach, endwhile, etc.)
 
  
 
<pre>
 
<pre>
<?php echo $someVar ?> // Good, semicolon is implied by the PHP close tag
+
<?php
<?php echo $someVar; ?> // Unnecessary
 
<?php endif; ?> // Necessary
 
</pre>
 
  
 
+
if (
== Constructors ==
+
    $expr1
 
+
    && $expr2
Don't use empty empty constructor argument lists, since they're not required,
+
) {
 
+
    // if body
<pre>
+
} elseif (
$foo = new Foo; // Good, no argument list
+
    $expr3
$foo = new Foo(); // Unnecessary
+
    && $expr4
$foo = new Foo($bar, ...); // Necessary
+
) {
 +
    // elseif body
 +
}
 
</pre>
 
</pre>
  
When it is useful to have a constructor method, either because you can chain methods but not constructors (`create...()->aaa()->bbb()` works, but `new Foo->aaa()->bbb()` does not), or in order to support constructor overloading (`createFromFoo($foo)` and `createFromBar($bar)`), or because the actual constructor must have protected visibility, the constructor method should be named like `create...()`. It shouldn't be named like `new...()` because `new` is a reserved word in PHP
+
'''N.B.''' AtoM boolean operators MUST come at the beginning of the line, as shown above, when boolean statements are split over multiple lines.
  
 +
=== Multi-line assignments ===
  
== Return statements ==
+
The PSR-12 standard doesn't specify how to format multi-line assignments, nor does the @PhpCsFixer ruleset. However, the AtoM standard formats multi-line assignments thus:
  
As per symfony standard, add an empty line before any return statements *except* where it would leave an empty line after an opening curly brace (e.g. a one line function). Empty lines should *never* follow an open curly brace or precede a close curly brace
+
* The assignment statement starts on the same line as the assigned variable or <code>return</code> keyword.
 +
* Subsequent operators come at the start of the following lines, with indentation.
  
 
+
For example:
== Whitespace ==
 
 
 
Use one empty line between functions.
 
 
 
 
 
== is_null() ==
 
 
 
Unlike the symfony coding standard, use <code>null === $foo</code> instead of <code>is_null($foo)</code>. This is because there's no equivalent to <code>is_null()</code> for "false === $foo" or "0 === $foo", and we prefer to use consistent syntax for "null === $foo", "false === $foo", and "0 === $foo"
 
 
 
symfony has since adopted this convention, in symfony >= 1.3
 
 
 
 
 
== $options ==
 
 
 
Instead of functions with long lists of optional arguments, we prefer passing these arguments in an $options array,
 
 
 
<pre>
 
public function foo($required1, $required2, array $options = array()) // Preferred
 
public function foo($required1, $required2, $option1 = null, $option2 = null, ...) // Not preferred
 
</pre>
 
 
 
This saves having to pass several null arguments when you want to specify only the last option, which is prone to getting the number of arguments in the function signature wrong, and accidentally specifying the wrong option
 
 
 
It's interesting how symfony mixes associative and ordered options/parameters; see for example: <code>sfLogger::listenToEvent()</code>
 
 
 
In the function body, set default values for optional arguments with array addition,
 
 
 
<pre>
 
$options += array('option1' => 'foo', 'option2' => 'bar');
 
</pre>
 
 
 
 
 
== Links ==
 
 
 
Don't use [http://www.symfony-project.org/book/1_2/09-Links-and-the-Routing-System#How%20It%20Works symfony internal URIs]. Instead, always use an array of parameters for generating URIs. This is because it's easy to confuse internal and external URIs, and internal URIs add yet another URI format to remember
 
 
 
<pre>
 
link_to('link text', array('module' => user', 'action' => 'login')); // Good
 
link_to('link text', 'user/login'); // Bad
 
link_to('link text', '/login/'); // Incorrect
 
</pre>
 
 
 
 
 
== substr() ==
 
 
 
Use <code>substr()</code> to check if a string begins or ends with another string. It's possible to check if a string begins with another string with <code>strncmp()</code>, but <code>strncmp()</code> isn't able to check if a string [http://bugs.php.net/bug.php?id=36944 ends with another string] and we prefer to use a consistent pattern for both. Also, the return value of<code>strncmp()</code> (< 0 if str1 is less than str2 ; > 0 if str1 is greater than str2 , and 0 if they are equal) is confusing.
 
  
 
<pre>
 
<pre>
if (substr($string, 5) == 'HTTP_') // Good
+
<?php
if (substr($string, -8) == '_wrapper') // Good
 
if (!strncmp($string, 'HTTP_', 5)) // Awkward
 
if (!strncmp($string, '_wrapper', -8)) // Not supported
 
</pre>
 
  
 +
$foo = $condition
 +
    ? 'true value'
 +
    : 'false value';
  
== javascript ==
+
return $conditionOne
 +
    && $conditionTwo
 +
    && $conditionThree;
  
Never use the JavaScript pseudo protocol. Always use standard JavaScript events for triggering behavior. Because the "a" tag must have an "href" attribute, use href="#". This is standard practice in YUI (see the tree view) and symfony (see the debug toolbar). A possible advantage of href="#" is that it could possibly be used to [http://developer.yahoo.com/yui/history/ support the back button].
+
$sum = $sumOne
 +
    + $sumTwo
 +
    + 123;
  
<pre>
+
return $stringOne
<!-- Good -->
+
    .$stringTwo
<a class="foo" href="#">
+
    .' extra content';
[...]
 
$('.foo').click(foo);
 
  
<!-- Bad -->
+
$bar = 'Long string
<a href="javascript:foo()">
+
    where whitespace
 +
    is not an issue';
 
</pre>
 
</pre>
  
 
+
'''N.B.''' statements SHOULD NOT be broken over multiple lines unless it is necessary to keep lines under the recommended 80 character line length.
== Editorial style guide ==
 
 
 
https://infrastructure.drupal.org/drupal.org-style-guide/editorial.html
 
 
 
http://drupal.org/about/authoring
 
 
 
Consider gathering coding standard and editorial style guide into separate sections or pages?
 
 
 
Use serial commas
 
 
 
* "Add" - name of buttons for adding new resources, adding new links, title of "add" pages, should eventually be the URL component
 
* "Create" - name of buttons to submit the "add" form, fluid API method (FooObject::create())
 
* "New" - PHP reserved word (new FooObject)
 
* "Load"? "Find"? - fluid API method?
 
 
 
amos user manual guidelines page
 
 
 
 
 
 
 
  
 
* [[#Development/Coding standard|Back to top]]
 
* [[#Development/Coding standard|Back to top]]
 
 
  
 
[[Category:Development documentation]]
 
[[Category:Development documentation]]

Latest revision as of 10:46, 26 July 2021

Main Page > Development > Development/Coding standard

AtoM uses PHP CS Fixer to check and auto-format the PHP code following the @PhpCsFixer ruleset, a highly opinionated extension of the Symfony Coding Standards and the PSR-12 coding style specification.

The PHP CS Fixer tool is included in the AtoM project's Composer dependencies for development. PHP CS Fixer's configuration is tracked as part of the project's source code (see .php-cs-fixer.dist.php) and it's used in the Continuous Integration process to check the code on every pull request and commit merged to the stable/** and qa/** branches.

Contributors to the AtoM project should run PHP CS Fixer locally to ensure their modifications meet the coding standards. There are a number of options for running PHP CS Fixer on your code: PHP CS Fixer's README file links to plugins for several popular code editors (e.g. VS Studio Code, atom.io, Sublime text), you can configure a git pre-commit hook to run PHP CS Fixer when commiting changes, or PHP CS Fixer can be run manually.

Coding Standards that are not handled by PHP CS Fixer

While PHP CS Fixer covers a lot of rules to meet the coding standard, there are a few cases that need to be handled manually, especially for multi-line statements. The PSR-12 standard doesn't impose a hard limit on line length, but it recommends a soft limit of 120 characters, and to keep lines under 80 characters where possible. However, the following cases need to be manually edited at the moment.

Multi-line method calls

Due to an issue formatting the Symfony templates, the ensure_fully_multiline option of the method_argument_space rule is currently disabled. Nevertheless, as noted in the PSR-12 standard:

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.

<?php

$foo->bar(
    $longArgument,
    $longerArgument,
    $muchLongerArgument
);

N.B.: AtoM argument lists SHOULD be split into multiple lines, according to the rules above, to keep the line length under 80 characters.

Multi-line control structures

The PSR-12 standard on control structures is not fully covered by PHP CS Fixer currently, but control structure formatting follows the same pattern as the method call formatting, with the additional rule:

Boolean operators between conditions MUST always be at the beginning or at the end of the line, not a mix of both.

<?php

if (
    $expr1
    && $expr2
) {
    // if body
} elseif (
    $expr3
    && $expr4
) {
    // elseif body
}

N.B. AtoM boolean operators MUST come at the beginning of the line, as shown above, when boolean statements are split over multiple lines.

Multi-line assignments

The PSR-12 standard doesn't specify how to format multi-line assignments, nor does the @PhpCsFixer ruleset. However, the AtoM standard formats multi-line assignments thus:

  • The assignment statement starts on the same line as the assigned variable or return keyword.
  • Subsequent operators come at the start of the following lines, with indentation.

For example:

<?php

$foo = $condition
    ? 'true value'
    : 'false value';

return $conditionOne
    && $conditionTwo
    && $conditionThree;

$sum = $sumOne
    + $sumTwo
    + 123;

return $stringOne
    .$stringTwo
    .' extra content';

$bar = 'Long string
    where whitespace
    is not an issue';

N.B. statements SHOULD NOT be broken over multiple lines unless it is necessary to keep lines under the recommended 80 character line length.