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(…), and getXxx, 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.

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.