.. redirect-from:: Contributing/Code-Style-Language-Versions .. _CodeStyle: Code style and language versions ================================ .. contents:: Table of Contents :depth: 2 :local: In order to achieve a consistent looking product we will all follow externally (if possible) defined style guidelines for each language. For other things like package layout or documentation layout we will need to come up with our own guidelines, drawing on current, popular styles in use now. Additionally, wherever possible, developers should use integrated tools to allow them to check that these guidelines are followed in their editors. For example, everyone should have a PEP8 checker built into their editor to cut down on review iterations related to style. Also where possible, packages should check style as part of their unit tests to help with the automated detection of style issues (see `ament_lint_auto `__). C - Standard ^^^^^^^^ We will target C99. Style ^^^^^ We will use `Python's PEP7 `__ for our C style guide, with some modifications and additions: * We will target C99, as we do not need to support C89 (as PEP7 recommends) * rationale: among other things it allows us to use both ``//`` and ``/* */`` style comments * rationale: C99 is pretty much ubiquitous now * C++ style ``//`` comments are allowed * (optional) Always place literals on the left-hand side of comparison operators, e.g. ``0 == ret`` instead of ``ret == 0`` * rationale: ``ret == 0`` too easily turns into ``ret = 0`` by accident * optional because when using ``-Wall`` (or equivalent) modern compilers will warn you when this happens All of the following modifications only apply if we are not writing Python modules: * Do not use ``Py_`` as a prefix for everything * instead use a CamelCase version of the package name or other appropriate prefix * The stuff about documentation strings doesn't apply We can use the `pep7 `__ python module for style checking. The editor integration seems slim, we may need to look into automated checking for C in more detail. C++ --- Standard ^^^^^^^^ {DISTRO_TITLE} targets C++17. Style ^^^^^ We will use the `Google C++ Style Guide `__, with some modifications: Line Length ~~~~~~~~~~~ * Our maximum line length is 100 characters. File Extensions ~~~~~~~~~~~~~~~ * Header files should use the .hpp extension. * rationale: Allow tools to determine content of files, C++ or C. * Implementation files should use the .cpp extension. * rationale: Allow tools to determine content of files, C++ or C. Variable Naming ~~~~~~~~~~~~~~~ * For global variables use lowercase with underscores prefixed with ``g_`` * rationale: keep variable naming case consistent across the project * rationale: easy to tell the scope of a variable at a glance * consistency across languages Function and Method Naming ~~~~~~~~~~~~~~~~~~~~~~~~~~ * Google style guide says ``CamelCase``, but the C++ std library's style of ``snake_case`` is also allowed * rationale: ROS 2 core packages currently use ``snake_case`` * reason: either an historical oversight or a personal preference that didn't get checked by the linter * reason for not changing: retroactively changing would be too disruptive * other considerations: * ``cpplint.py`` does not check this case (hard to enforce other than with review) * ``snake_case`` can result in more consistency across languages * specific guidance: * for existing projects, prefer the existing style * for new projects, either is acceptable, but a preference for matching related existing projects is advised * final decision is always developer discretion * special cases like function pointers, callable types, etc. may require bending the rules * Note that classes should still use ``CamelCase`` by default Access Control ~~~~~~~~~~~~~~ * Drop requirement for all class members to be private and therefore require accessors * rationale: this is overly constraining for user API design * we should prefer private members, only making them public when they are needed * we should consider using accessors before choosing to allow direct member access * we should have a good reason for allowing direct member access, other than because it is convenient for us Exceptions ~~~~~~~~~~ * Exceptions are allowed * rationale: this is a new codebase, so the legacy argument doesn't apply to us * rationale: for user-facing API's it is more idiomatic C++ to have exceptions * Exceptions in destructors should be explicitly avoided * We should consider avoiding Exceptions if we intend to wrap the resulting API in C * rationale: it will make it easier to wrap in C * rationale: most of our dependencies in code we intend to wrap in C do not use exceptions anyways Function-like Objects ~~~~~~~~~~~~~~~~~~~~~ * No restrictions on Lambda's or ``std::function`` or ``std::bind`` Boost ~~~~~ * Boost should be avoided unless absolutely required. Comments and Doc Comments ~~~~~~~~~~~~~~~~~~~~~~~~~ * Use ``///`` and ``/** */`` comments for *documentation* purposes and ``//`` style comments for notes and general comments * Class and Function comments should use ``///`` and ``/** */`` style comments * rationale: these are recommended for Doxygen and Sphinx in C/C++ * rationale: mixing ``/* */`` and ``//`` is convenient for block commenting out code which contains comments * Descriptions of how the code works or notes within classes and functions should use ``//`` style comments Pointer Syntax Alignment ~~~~~~~~~~~~~~~~~~~~~~~~ * Use ``char * c;`` instead of ``char* c;`` or ``char *c;`` because of this scenario ``char* c, *d, *e;`` Class Privacy Keywords ~~~~~~~~~~~~~~~~~~~~~~ * Do not put 1 space before ``public:``, ``private:``, or ``protected:``, it is more consistent for all indentions to be a multiple of 2 * rationale: most editors don't like indentions which are not a multiple of the (soft) tab size * Use zero spaces before ``public:``, ``private:``, or ``protected:``, or 2 spaces * If you use 2 spaces before, indent other class statements by 2 additional spaces * Prefer zero spaces, i.e. ``public:``, ``private:``, or ``protected:`` in the same column as the class Nested Templates ~~~~~~~~~~~~~~~~ * Never add whitespace to nested templates * Prefer ``set>`` (C++11 feature) to ``set >`` or ``set< list >`` Always Use Braces ~~~~~~~~~~~~~~~~~ * Always use braces following ``if``, ``else``, ``do``, ``while``, and ``for``, even when the body is a single line. * rationale: less opportunity for visual ambiguity and for complications due to use of macros in the body Open Versus Cuddled Braces ~~~~~~~~~~~~~~~~~~~~~~~~~~ * Use open braces for ``function``, ``class``, ``enum``, and ``struct`` definitions, but cuddle braces on ``if``, ``else``, ``while``, ``for``, etc... * Exception: when an ``if`` (or ``while``, etc.) condition is long enough to require line-wrapping, then use an open brace (i.e., don't cuddle). * When a function call cannot fit on one line, wrap at the open parenthesis (not in between arguments) and start them on the next line with a 2-space indent. Continue with the 2-space indent on subsequent lines for more arguments. (Note that the `Google style guide `__ is internally contradictory on this point.) * Same goes for ``if`` (and ``while``, etc.) conditions that are too long to fit on one line. Examples ~~~~~~~~ This is OK: .. code-block:: c++ int main(int argc, char **argv) { if (condition) { return 0; } else { return 1; } } if (this && that || both) { ... } // Long condition; open brace if ( this && that || both && this && that || both && this && that || both && this && that) { ... } // Short function call call_func(foo, bar); // Long function call; wrap at the open parenthesis call_func( foo, bar, foo, bar, foo, bar, foo, bar, foo, bar, foo, bar, foo, bar, foo, bar, foo, bar, foo, bar, foo, bar, foo, bar, foo, bar, foo, bar, foo, bar, foo, bar, foo, bar, foo, bar); // Very long function argument; separate it for readability call_func( bang, fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo, bar, bat); This is **not** OK: .. code-block:: c++ int main(int argc, char **argv) { return 0; } if (this && that || both) { ... } Use open braces rather than excessive indention, e.g. for distinguishing constructor code from constructor initializer lists This is OK: .. code-block:: c++ ReturnType LongClassName::ReallyReallyReallyLongFunctionName( Type par_name1, // 2 space indent Type par_name2, Type par_name3) { DoSomething(); // 2 space indent ... } MyClass::MyClass(int var) : some_var_(var), some_other_var_(var + 1) { ... DoSomething(); ... } This is **not** OK, even weird (the google way?): .. code-block:: c++ ReturnType LongClassName::ReallyReallyReallyLongFunctionName( Type par_name1, // 4 space indent Type par_name2, Type par_name3) { DoSomething(); // 2 space indent ... } MyClass::MyClass(int var) : some_var_(var), // 4 space indent some_other_var_(var + 1) { // lined up ... DoSomething(); ... } Linters ~~~~~~~ We check these styles with a combination of Google's `cpplint.py `__ and `uncrustify `__. We provide command line tools with custom configurations: * `ament_clang_format `__: `configuration `__ * `ament_cpplint `__ * `ament_uncrustify `__: `configuration `__ Some formatters such as ament_uncrustify and ament_clang_format support ``--reformat`` options to apply changes in place. We also run other tools to detect and eliminate as many warnings as possible. Here's a non-exhaustive list of additional things we try to do on all of our packages: * use compiler flags like ``-Wall -Wextra -Wpedantic`` * run static code analysis like ``cppcheck``, which we have integrated in `ament_cppcheck `__. Python ------ Version ^^^^^^^ We will target Python 3 for our development. Style ^^^^^ We will use the `PEP8 guidelines `_ for code format. We chose the following more precise rule where PEP 8 leaves some freedom: * `We allow up to 100 characters per line (fifth paragraph) `_. * `We pick single quotes over double quotes as long as no escaping is necessary `_. * `We prefer hanging indents for continuation lines `_. Tools like the ``(ament_)pycodestyle`` Python package should be used in unit-test and/or editor integration for checking Python code style. The pycodestyle configuration used in the linter is `here `__. Integration with editors: * atom: https://atom.io/packages/linter-pycodestyle * emacs: https://www.emacswiki.org/emacs/PythonProgrammingInEmacs * Sublime Text: https://sublime.wbond.net/packages/SublimeLinter-flake8 * vim: https://github.com/nvie/vim-flake8 CMake ----- Version ^^^^^^^ We will target CMake 3.8. Style ^^^^^ Since there is not an existing CMake style guide we will define our own: * Use lowercase command names (``find_package``, not ``FIND_PACKAGE``). * Use ``snake_case`` identifiers (variables, functions, macros). * Use empty ``else()`` and ``end...()`` commands. * No whitespace before ``(``\ 's. * Use two spaces of indention, do not use tabs. * Do not use aligned indentation for parameters of multi-line macro invocations. Use two spaces only. * Prefer functions with ``set(PARENT_SCOPE)`` to macros. * When using macros prefix local variables with ``_`` or a reasonable prefix. Markdown / reStructured Text / docblocks ---------------------------------------- Style ^^^^^ The following rules to format text is intended to increase readability as well as versioning. * *[.md, .rst only]* Each section title should be preceded by one empty line and succeeded by one empty line. * Rationale: It expedites to get an overview about the structure when screening the document. * *[.rst only]* In reStructured Text the headings should follow the hierarchy described in the `Sphinx style guide `__: * ``#`` with overline (only once, used for the document title) * ``*`` with overline * ``=`` * ``-`` * ``^`` * ``"`` * Rationale: A consistent hierarchy expedites getting an idea about the nesting level when screening the document. * *[.md only]* In Markdown the headings should follow the ATX-style described in the `Markdown syntax documentation `__ * ATX-style headers use 1-6 hash characters (``#``) at the start of the line to denote header levels 1-6. * A space between the hashes and the header title should be used (such as ``# Heading 1``) to make it easier to visually separate them. * Justification for the ATX-style preference comes from the `Google Markdown style guide `__ * Rationale: ATX-style headers are easier to search and maintain, and make the first two header levels consistent with the other levels. * *[any]* Each sentence must start on a new line. * Rationale: For longer paragraphs a single change in the beginning makes the diff unreadable since it carries forward through the whole paragraph. * *[any]* Each sentence can optionally be wrapped to keep each line short. * *[any]* The lines should not have any trailing white spaces. * *[.md, .rst only]* A code block must be preceded and succeeded by an empty line. * Rationale: Whitespace is significant only directly before and directly after fenced code blocks. Following these instructions will ensure that highlighting works properly and consistently. * *[.md, .rst only]* A code block should specify a syntax (e.g. ``bash``).