Coding conventions v2
Table of Contents
-
Coding conventions
- Default conventions
- Language
- Code documentation
- Syntax
- Date and time management
- Database
- Security
- Translations management
- Error Management
- Git and Contribution process
- Git
Chamilo strives to follow PSR-1, [PSR-2] (https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md) and the Symfony Coding Standards, the Twig Coding Standards and (since v2) VueJS and TailwindCSS coding standards (to be documented later).
PSR-2 does not set a guideline for the properties names such as $StudlyCaps, $camelCase, or $under_score. We are using $camelCase for all new code (and progressively replacing other styles) since 2016, in order to have reasonable code consistancy.
The following rules define additional elements, not defined in PSR-2 (which includes PSR-1, which includes PSR-0).
Chamilo v2 and following do not support PHP 7.* anymore, so any syntax generating warnings, notices or errors should be avoided.
For new functions and methods, strict filtering of parameters should be applied. For legacy functions and methods, converting to strict filtering is desired, but should be done with care as they might be called from unsuspected places and still use a high level of variability in their parameters.
- Use English for comments and variables/function names.
The code, comments and developer instructions you write should be in English (preferrably UK English), no matter what your native language is. It's great if manuals are available in many languages, but code with comments and variables in English, French and Spanish is hard to understand. For feature requests and bug reports, English, French and Spanish are accepted, although English is preferred.
- Document your code using PHPDoc or one-liners (
/* Abcd efgh */or// Abcd efghare both valid) - Add licensing terms link to all new files
For many reasons, it is important to write good documentation. All page- or block-level code documentation should be generated respecting PHPDoc. Adding comments with ======= or ///////// separators is a deprecated syntax and is not accepted for new comments. Please remove these on sight.
All PHP files in the Chamilo code should always show licensing terms reference on the second line, like so:
<?php
/* For licensing terms, see /license.txt */`
/**
* Some PHPDoc file-level block
*/
All code in Chamilo must be written and shared under the GNU/GPLv3+ license or a compatible license (excluding the case of including external libraries which already have their own, compatible, license). No software that has a license incompatible with GNU/GPLv3+ will be accepted in the Chamilo code.
With the inclusion of entities and their relationships, it is now common to have objects of a certain class being instanciated in a way that prevents other developers to quickly identify which child class is being instanciated. This is true with resources, between other object types, in Chamilo.
To improve readability, developers making use of such classes where, for example, the IDE cannot guess which subclass is being instanciated, MUST add either a PHPDoc @var line on top of the instanciation, or (if the instance can be of several different child classes) indicate which children classes it might be instanciating at that point in the code. Below are two (fake) examples of what we mean:
/* @var DocumentResource $document */
$document = new ResourceTool('document');
/**
* $tool below can be of class DocumentTool, AssignmentTool, WikiTool or TestTool, depending on $toolType
*/
$tool = new ResourceTool($toolType);
Please use:
require 'main/inc/global.inc.php';
or
require __DIR__.'/../../main/inc/global.inc.php';
don't use:
require('main/inc/global.inc.php');
Explanation: include and require are language constructs, not functions. Although they do work with parenthesis, adding the parenthesis actually makes PHP believe it is a function, and as such go through the additional process of trying to find the function reference.
When you get a chance to safely fix these in some script you are modifying, please do so.
An include in PHP is the same as a require, except that require is stricter, meaning that if you try to include a file that does not exist, include will generate a warning message, while require will generate a fatal error.
Unless you want to try out inclusion of some files, and you actually catch the warning messages so you can deal with them, there's no reason at all to be using include. include obfuscates problems in your code, preventing you from realizing that some of the libraries you are trying to use are not actually there. Sometime later, side-effects might appear because the file you are supposed to be including is not actually included but you don't know it.
Don't use include or include_once, use require or require_once instead!
- Do not use
<?
Use the correct, complete php tags: start with and end with ?> (except at the end of a file). Do not use short tags like <?, <?= or ASP-style tags <%, <%= as these rely on php.ini configuration settings short_open_tag and asp_tags. These settings can cause errors when they are off and Chamilo code assumes that they are set to on.
The php.ini comments advise not to use short tags or ASP-style tags:
NOTE: Using short tags should be avoided when developing applications or
libraries that are meant for redistribution, or deployment on PHP
servers which are not under your control, because short tags may not
be supported on the target server. For portable, redistributable code,
be sure not to use short tags.
- Do not using file-level closing PHP tags
?>
As per PSR-2, we ask not to use the closing ?> tag at the end of a script. This is due to the fact that, if there are too many blank lines at the end of a script (more than one), after the closing tag, then your script will trigger the sending of HTTP headers to the user, preventing the system to send the appropriate redirect headers or other headers bound to the session (sending of cookies).
For example,
<?php
// Comment.
$var = 123;
?>
This code will send headers to the user although we are not at the end of the PHP process (there are still more scripts to be loaded in this chain), and consequently trigger the apparition of strange warning message, and the loss of the user's session. If you intentionally omit the closing tag (which in turn could trigger problems for parsing the PHP code through XML, but who does that?), then you ensure nothing will send blank lines to the user's browser, because the blank lines are considered as part of the PHP code and, as such, are ignored.
<?php
// Comment.
$var = 123;
Note that PHP itself will automatically ignore one blank line after the closing PHP tag. Just one.
Adding a blank line between lines of code creates paragraphs of code and helps to visualize it. You don't want to read books that have a thousand pages filled with line after line and no breaks; nobody wants to read code like that either.
Also, using spaces around operators and after comma's can help to make the code more readable. However, most people write their function brackets without a space between the function name and the brackets:
function something($param1, $param2)
{
if ($param1 and $param2) {
// Code
}
return $result;
}
When configuring your PHP editor, make sure it replaces all tabulations by 4 blank spaces. In PHPEclipse, this can be done by following the sequence: Window -> Preferences -> PHPEclipse -> PHP -> Appearance -> Display Tab Width: 4 and PHPEclipse -> PHP -> Typing -> Insert spaces for tab. In PHPStorm, this can be done by following the sequence: File -> Settings -> Editor -> Code Style -> PHP -> Tabs and Indents -> Tab size: 4 (and make sure "Use tab character" is unchecked. This ensures files are seen the same way under all editors (including terminal-based editors).
For all uses of binary operators (=, +, etc), these should be surrounded by spaces, with the exception of the concatenation operator (.) which must not be surrounded by spaces.
// Correct syntax
$a = 1 + 2;
$string = "Hello "."world";
If you have to use variables, use lower cased and underscored variable names:
{% set foo = 'foo' %}
{% set foo_bar = 'foo' %}
See http://twig.sensiolabs.org/doc/coding_standards.html
- Use
generic-css-class-names, NOTGenericCSSClass-Names
All CSS classes and IDs should be expressed in lowercase letters separated by hyphen (-).
echo '<div class="generic-class other-class-less-generic">'.get_lang('Term').'</div>';
When using a CSS style specific to a tool, use the name of the tool as a prefix.
echo '<div class="tool-name-something tool-name-something-else generic-modifier">'.get_lang('Term').'</div>';
JavaScript and CSS must be added in the header.
// Documentation incomplete here...
For classes and methods, put your opening brackets and closing brackets on an empty line, and at the same indentation
function something($a)
{
for ($i = 0; $i < $a; $i++) {
echo "$a <br />";
}
}
This gives a very clear view of where a loop starts and where it finishes. Some code editors even improve this visibility by tracing a vertical line between the opening and the closing bracket.
Please note that control structures should have their opening brackets on the same line.
Do NOT use:
if ($hello) echo 'goodbye';
Use:
if ($hello) {
echo 'goodbye';
}
Using a condition on a single line is not a good idea, and [PSR-2] (https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md) kind of implicitly forbids it: "Opening braces for control structures MUST go on the same line, and closing braces MUST go on the next line after the body.", so the logic in Chamilo is to always define the body of a condition on a separate line.
To remain flexible and more quickly readable, the code MUST always include brackets, and should not include single-line conditional statements.
This helps delimiting clearly the boundaries of the condition even if the indentation is broken, without requiring the 1-second brain loop to figure out if that's really the single statement included and whether it should be more indented or not.
Try to use Chamilo's internal functions (API) as much as possible.
// Documentation incomplete here...
// Documentation incomplete here...
New variables must be expressed in $camelCase (starting with lower case and using a capital letter for each new word).
Although you might find a lot of $lower-case-variables, these are remains of an older coding convention. Wherever you are completely sure about the use of a specific variable expressed in lower-case, you are invited to change it to camel case in a separate commit prefixed with the "Minor - Coding conventions" message.
Language variables now use Gettext format, so they represent an "ID" which is identical or close to the full string translation in English, between quotes. In the past, language strings were represented as CamelCase Strings. Not anymore.
CONSTANTS names MUST be in all-upper-case letters.
These coding conventions have to be applied only for new code (e. g. code written after Feb 17, 2010) that uses dates and times:
- Any date and time stored in the database must use the SQL DATETIME format
- Before storing any DATETIME in the database, you MUST call the function
api_get_utc_datetime($time)with the date and time you want to store in the database. This function will return you a string under the format Y-m-d H:i:s that you can store directly in the database. - Before displaying any datetime, you must call the function
api_get_local_time($time, $to_timezone = null, $from_timezone = null)with the datetime you want to convert ($from_timezonecan usually be left empty as it is stored in UTC in the database).
To know more about date and time management in Chamilo, click on Date and time management
Since Chamilo 2.0, all database tables must be defined through an entity. These can be found in the src/[Some]Bundle/Entity/ directories.
To avoid using the automated naming of indexes by Doctrine, we ask all developers to name their indexes. Adding indexes to a table with a name is done the following way:
However the name of the index has to follow a certain set of rules (not like "course" in this example):
- start with 'idx_'
- contain a shortened version of the table name (ex: 'cannattach_')
- contain a shortened version of the indexed row(s) (ex: 'cid')
For Foreign Keys, Doctrine doesn't support naming at the moment, so we let Doctrine manage the naming. This can cause some differences in the database structure as reflected by php bin/doctrine.php orm:schema-tool:update --dump-sql
// Documentation incomplete here...
// Documentation incomplete here...
// Documentation incomplete here...
// Documentation incomplete here...
// Documentation incomplete here...
// Documentation incomplete here...
Defining repositories (referenced in the entities section above) must be done in a directory parallel to Entities:
- /src/UserBundle/Entity
- /src/UserBundle/Repository
Backticks ( ` ) are a very bad MySQL-only habit. Don't use backticks at all. This will improve your respect of the SQL standard and make you find meaningful names for your tables and your fields, that will avoid them being misunderstood for a MySQL reserved keyword.
When copy-pasting some code from phpMyAdmin, make sure you remove all backticks.
- Use Chamilo's date functions, do NOT use MySQL's
NOW()(ever)
time() is a PHP function that generates a UNIX timestamp (the number of seconds from 1/1/1970 00:00:00 until now).
NOW() is a MySQL function that generates a timestamp to the required format to fill the field it is put in.
Combining the two functions can lead to time matching problems, because time() depends on Apache and NOW() depends on MySQL server. These two programs can have different times (they can be on different servers or simply have a different configuration).
Because the process in Chamilo is based on PHP interpretation, time() must be used instead of NOW(), even when simply filling database logs fields. This will ensure that time-based values are all based on the same time source, and will make reliable statistical queries possible.
- Do NOT use the useless MySQL syntax
INT (11). Only useINT.
INT (11) is a markup created by phpMyAdmin in the automated table creation query generation, but is actually useless in the sense of PHP + MySQL. To be truly portable, declare INT fields as INT, not INT (11).
This is a quote from http://dev.mysql.com/doc/refman/5.0/en/numeric-types.html:
The display width does not constrain the range of values that can be stored in the column, nor the number of digits that are displayed for values having a width exceeding that specified for the column. For example, a column specified as SMALLINT(3) has the usual SMALLINT range of -32768 to 32767, and values outside the range allowed by three characters are displayed using more than three characters.
When declaring a primary or foreign key, please use the "UNSIGNED" qualifier. Although few of the current fields have been declared as unsigned, there is no use for the negative side of integers.
Unsigned integers, in MySQL, go from 0 to 4,294,967,295 (which is a considerably safe primary key allowance in Chamilo)
When declaring foreign keys in the database, the relation is considered implicit, and should be using the name "id" in the table where it acts as a primary key, and "[origintable]_id" in the table where it is used as a foreign key. The element type must be the same.
In the different implementations of SQL, some words happen to be reserved for specific use, like uid for internal IDs in Oracle or "time" in PostgreSQL. Although Chamilo 1 doesn't support anything else than MySQL, we are definitely working on having support for other database management systems. This means that we have to avoid using reserved keywords, even if they are reserved only in another database system. If you have doubts, you can check the words on this page: http://www.petefreitag.com/tools/sql_reserved_words_checker/?word=time
An easy trick to avoid using reserved keywords by mistake is to simply put the name of the table (or any short version of it) as a prefix to the field.
An essential part of not using reserved keywords is to avoid using the special MySQL character "backtick" () which doesn't work in any other database management system (not portable), and is specially made to allow you to use reserved keywords in MySQL. We don't want that, so we don't want to see any in a SQL query. Backticks are generally added because you made a copy-paste from phpMyAdmin. Avoid that. Care about your queries!
We do our best effort to make Chamilo the most secure LMS around. This is kind of acknowledged by the number of CVEs and the time to fix. To maintain a high standard of security in Chamilo, we need all developers to be on track as to what is needed to maintain it that high. The following sections try o give you a short but important overview of what we expect from you.
It is mandatory to always make sure the data is filtered properly before being put in the database.
With PHP 7.0 and 7.1, the principle of "Type declaration" was added to PHP. Chamilo 1.11.14 and above depend on a minimum PHP version of 7.2, so Type declaration, which is a superior filtering mechanism (more strict than most others) should be used whenever possible in the development of Chamilo.
By using type filtering at the function/method declaration level, we delegate filtering at the script level and block any non-matching variable from going any further (the function/method will just not execute if the type doesn't match). For new functions/methods, please always use Type declaration.
In other cases, or where Type declaration is not practical, please always filter your data inside the function which is calling the database query. This way, you ensure your database is safe, no matter where the variables came from.
For strings, use
$filteredString = Database::escape_string($unfilteredString);For integer values (or other numerical values) use the corresponding converter/type caster:
$filteredInt = (int) $unfilteredInt;
$filteredFloat = (float) $unfilteredFloat;You can find more information on Doctrine security here: https://www.doctrine-project.org/projects/doctrine-orm/en/2.6/reference/security.html
Type declaration is a required practice wherever possible (wherever we use functions/methods). But this section's content is still accurate, even with type declaration enabled, as some strings might match the string type but still include hacks (as strings).
XSS attacks are always based on using JavaScript to the user. What is less known is that a user can be directed to your site with a URL that includes JavaScript. In this case, the JavaScript can be executed with increased access due to the user's session. Although the effects are often damaging the user's experience rather than the website's safety, it is best to avoid the problem as much as possible.
A Security class is provided inside Chamilo 1.8.x which allows for filtering of several kinds of attacks. We ask you to make use of Security::remove_XSS() on any content that might contain JavaScript to show to the user. This includes messages generated from third party sources.
For example:
echo $_POST['username'];
should be escaped like so:
echo Security::remove_XSS($_POST['username']);
Displayable text MUST be filtered just before displaying it, because we don't know exactly where we will show it, so maybe escaping JS and HTML code doesn't make sense because we are going to write the data to a file that will never execute in a browser. So the database might actually contain data that is tainted with XSS hacks. No worries. The important thing is to make sure that when it is printed for a browser to view, XSS is properly escaped.
In Chamilo 2.0 and subsequent versions, Twig (the templating system) has the ability to auto-filter XSS. However, because we consider we should not depend on this mechanism to have secure filtering, we expect this feature to be turned off and the variables to be escaped before sending to the TPL.
Cross-Site Request Forgery is also a mechanism by which JavaScript is abusing the user's browser. It is generally triggered from another website to benefit from the "open-session" effect that a user generates by staying "connected" to several sites at any one time. For example, a hackers site might use the user's session to connect to Chamilo and post spam content inside a course's description.
To avoid this, always make sure your forms are protected by a mechanism of controlled submission. The Security class can help you do that, particularly Security::get_token() and Security::check_token().
Once Chamilo 2.0 is in release candidate, we will move to another platform at https://translation.chamilo.org, but the translation workflow will then be different from the one below.
A language term should always be as expressive as possible. Don't try to join several strings into one. Something like: get_lang('Dear ').$username.get_lang('WelcomeToPortal').get_lang('WeHopeYouHaveFun').get_lang('BestRegards').$teamname; will not work out in languages with different grammatical rules like Japanese, Arabic, Quechua and many more.
Instead, use a context-full string like this "Dear %s, welcome to this portal. We hope you'll have fun learning through the courses made available to you. Best regards, %s".
The %s will then be replaced in the call to get_lang(): sprintf(get_lang('GeneralWelcomeToPortal'),$username, $teamname);
This is because, in other languages, the order of the sentences may not be exactly the same. You might make an introduction as "Yannick San E" (as compared with "Dear Mr Yannick"), instead of what could be possible with a single get_lang('Dear'), which would force you to use "E San Yannick", which doesn't mean anything in Japanese (it would be the equivalent of "Yannick Mr Dear" in English)
When using Twig, instead of preparing the translated variable inside PHP and for Twig, you should use the Twig filter "get_lang" and give the appropriate variable (without the $ sign), like this:
<div class="salutation">{{ "Hello" | get_lang }}</div>This will automatically translate the message to the current language.
If, as suggested above, you use a composed language variable (using %s signs), then you can call the get_lang filter as well, but you need an extra step (and you need the replacement of "%s" to have been assigned to a Twig variable:
<div class="salutation">{{ "HelloX" | get_lang | format(show_user_info.user_info.complete_name) }}</div>Multiple replacements can be obtained through the use of multiple parameters separated by commas in the format() call.
All debugging messages should be kept locally, unless they represent a long-term debugging session (spanning several days). The error_log('debug message'); method is preferred to the echo 'debug message'; method because it doesn't break the user display, even if forgotten in the code. To check the results of error_log(), check your Apache's error log. Under Ubuntu, you can have a live view of the debug console using the following command pattern:
sudo tail -f /var/log/apache2/yoursite-error.log
When using the @ symbol to obfuscate errors, make sure you check $php_errormsg (only available if php.ini track_errors setting is on) to deal with the error. In this specific case, adding a call to error_log() is an excellent idea, so that the error message is not "lost".
See [http://www.php.net/manual/en/reserved.variables.phperrormsg.php the php documentation page] for more info about this variable.
For example:
$res = @rmdir('/var/www/chamilo/archive/ABC');
if ($res === false) {
error_log(__FILE__.' line '.__LINE__.': '.(ini_get('track_errors') != false ? $php_errormsg : 'error not recorded because track_errors is off in your php.ini'));
}We assume that any code you add to Chamilo will not generate Fatal nor Warning errors. You can try that by putting your portal in "test mode" in the configuration page. Notice-level error messages are accepted (but frowned upon) when submitting new code. They are expected to disappear by when the code is merged into Chamilo.
Although most external libraries are included through Composer now, this documentation still serves for those (extremely rare) libraries unknown to Composer.
When integrating an external library, plugin or any piece of code maintained by a third party, make sure the first integration commit is the integration of the original version of the external code. Only commit the adaptation to the Chamilo code after that first "original" commit. This way, it will be much easier, later on, to detect the customization that has been made to integrate the external library.
A natural consequence of that (but that might not be obvious for everybody) is that you should always send one separate commit to include a new library. It should not be sent together with a Chamilo code change in one group commit.
Please avoid sending the code examples or unit tests that generally come with these libraries. They will take space and possibly introduce security flaws in the Chamilo code.
Starting in 1.10.0, Chamilo LMS uses Composer for the inclusion of PHP libraries and JS libraries, respectively. If the library you are including is available through Composer, you are required to use it (and update composer.json) as a source rather than inserting it into Git directly.
A commit should be something "atomic", that you can send as one piece and that you can remove in one piece without affecting the rest too much. If fixing a small bug, a commit could be a few lines of changes in one or several files.
If, during the bugfix, you find code that is not developed in respect with these coding conventions, your code style changes must go into a separate commit, preferrably starting with the keyword "Minor - " so reviewers know this commit doesn't affect the code logic (and cannot affect the code logic).
When sending changes in coding style (for example increasing the indentation for a whole function, a whole class or a whole file, please make sure you send a commit with a "Minor: code style" message separately, so comparing the code between an initial file and another version with the modified code is not too difficult.
-
Home
- Tools and sessions
- Quiz: Importing
- Releases
- Community support strategy
- Translation management
- How to report issues
- Development
- Integration