Tuleap coding standards¶
Code formatting¶
As Tuleap is mainly written in PHP, we use the PSR standards:
Rule of thumb: All new classes MUST respect PSR-2
Internal conventions¶
- Use an indent of 4 spaces, with no tabs. This helps to avoid problems with diffs, patches, git history…
- It is recommended to keep lines at approximately 85-100 characters long for better code readability.
- methodsInCamelCase()
- $variables_in_snake_case
- constants in UPPER_CASE
- public methods documented (at least @return statement)
- class documented (
I'm responsible of…
) - All added code should follow PSR-2. Existing code should be converted to PSR-2 in a dedicated commit in order to not clutter the review of your functional change.
- No trailing whitespaces
- In DataAccessObject, convention is to name
searchXxx()
the methods that returns a set of rows (eg.searchProjectsUserIsAdmin(…)
, andgetXxx
,isXxx
,hasXxx
for other cases (eg.doesUserHavePermission(…)
).
Note
Contributions SHOULD NOT add/fix features AND fix coding standard of a legacy file in the same review. The code WONT be accepted. If your eyes are bleeding, conform to coding standard in a dedicated review, then contribute your change.
This is especially true for refactoring, where the goal is to improve a part of the code. Extracted crappy code to a dedicated file does not need to be refactored, in order to ease the review (You may need to use one of ignore capabilities of phpcs in order to pass coding standards check). Contributor has to focus his mind on one task at a time.
Remember: refactoring is here to improve the existing code without breaking functionality.
Copyright & license¶
All source code files (php, js, bash, …) must contain a page-level docblock at the top of each file. This header includes your copyright and a reference to the license GPLv2+ of the script.
/** * Copyright (c) <You>, <Year>. All rights reserved * * This file is a part of Tuleap. * * Tuleap is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or * (at your option) any later version. * * Tuleap is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * * You should have received a copy of the GNU General Public License * along with Tuleap. If not, see <http://www.gnu.org/licenses/ */
Adapt the copyright line to your situation.
Good quality code¶
Tuleap is a big (+600k LOC) and old (16 years) software and has probably an example of every existing bad designs that existed during those 16 years.
Yet, it’s not a fatality and we are on the way to, slowly and carefully clean things up. On our road toward a Clean Code, some motto might help you to make your design choices:
- Test your code; TDD (Test Driven Development) should be the default.
- Follow SOLID design principles.
- Don’t contribute STUPID code.
We also strongly suggest that you familiarize yourself with Code Smells as it might pop up during code reviews.
Resources¶
A couple of documents worth to read when you consider contributing to Tuleap:
Tuleap principles¶
Output something / templating system¶
All new code must output content based on Mustache templates. The code is typically organized in 3 files:
- The template
- The presenter
- The calling code (in a Controller for instance)
Example of template:
<h1>Hello</h1> <p>Welcome to {{ my_title }}</p> <!-- For readability, please note : --> <!-- * the spaces between {{, variable name and }} --> <!-- * the use of snake_case for variables -->
Example of Presenter
class Presenter { /** @var string */ public $my_title; public function __construct() { $this->my_title = "My title"; } }
Example of calling code:
$renderer = TemplateRendererFactory::build()->getRenderer('/path/to/template/directory'); // Output content directly (to the browser for instance) $renderer->renderToPage('template_name', new Presenter()); // Return the content for futur reuse $string = $renderer->renderToString('template_name', new Presenter());
Note
For existing code, it’s acceptable to output content with “echo” to keep consistency.
Escaping¶
You should rely on Mustache {{ }}
notation to benefit from automatic escaping.
If you need to put light formatting in you localised string, then you should escape beforehand and use {{{ }}}
notation. As it produces a code that is less auditable (reviewer has to manually check if injections are not possible), the convention is to prefix the variable with purified_
and manually purify the variable in the presenter.
class Presenter { public $purified_description; public function __construct() { $this->purified_description = Codendi_HTMLPurifier::instance()->purify( $GLOBALS['Language']->getText('key1', 'key2', 'https://example.com'), CODENDI_PURIFIER_LIGHT ); } } // .tab file: // key1 key2 This is the <b>description</b> you can put <a href="$1">light formatting</a> // .mustache file: // <p>{{{ purified_description }}}</p>
Secure forms against CSRF¶
All state-changing actions MUST be protected against CSRF vulnerabilities. In order to do that, a specific token must be added to your forms and verified before the execution of the action.
Example:
Controller.php:
namespace Tuleap/CsrfExample; use CSRFSynchronizerToken; use TemplateRendererFactory; class Controller { public function display() { $csrf_token = CSRFSynchronizerToken(CSRF_EXAMPLE_BASE_URL . '/do_things'); $presenter = new Presenter($csrf_token); $renderer = TemplateRendererFactory::build()->getRenderer(CSRF_EXAMPLE_TEMPLATE_DIR); $renderer->renderToPage('csrf-example', $presenter); } public function process() { $csrf_token = CSRFSynchronizerToken(CSRF_EXAMPLE_BASE_URL . '/do_things'); $csrf_token->check(); do_things(); } }
Presenter.php:
namespace Tuleap/CsrfExample; use CSRFSynchronizerToken; class Presenter { /** * @var CSRFSynchronizerToken */ public $csrf_token; public function __construct(CSRFSynchronizerToken $csrf_token) { $this->csrf_token = $csrf_token; } }
csrf-example.mustache:
<form method="post"> {{# csrf_token }} {{> csrf_token_input }} {{/ csrf_token }} <input type="submit"> </form>
Note
For existing code rendering HTML without using templates, it can be acceptable to use the fetchHTMLInput method of the CSRFSynchronizerToken class.
Secure DB against SQL injections¶
All code related to database MUST rely on prepared statements to pass parameters to a SQL query.
Example of DataAccessObject:
namespace Tuleap\Git; use Tuleap\DB\DataAccessObject; use ParagonIE\EasyDB\EasyStatement; class RepositoryDao extends DataAccessObject { public function searchByName($project_id, $name) { $sql = 'SELECT * FROM plugin_git_repositories WHERE project_id = ? AND name = ?'; return $this->getDB()->run($sql, $project_id, $name); } public function searchByProjectIDs(array $project_ids) { $project_ids_in_condition = EasyStatement::open()->in('?*', $project_ids); $sql = 'SELECT * FROM plugin_git_repositories WHERE project_id IN ($project_ids_in_condition)'; return $this->getDB()->safeQuery($sql, $project_ids_in_condition->values()); } }
Note
You might find existing code using the \DataAccessObject
class or db_*()
functions,
in that case you will need to use the dedicated escaping methods (\DataAccessObject::quoteSmart
,
\DataAccessObject::escapeInt
, db_es
and db_ei
). The usage of these deprecated
interfaces should be avoided.