From f9328d20210082a02b307dcd8856ac47f997c989 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 11 Feb 2020 09:11:44 +0000 Subject: [PATCH] Bug 1613526 - Create a code quality documentation and move the appropriate docs r=ahal This for a few reasons: * The summary becomes the landing page for code quality: https://firefox-source-docs.mozilla.org/tools/static-analysis/summary.html * I don't think we need a full code quality category * Closer to the source-code-doc * All the files at the same place Differential Revision: https://phabricator.services.mozilla.com/D61767 --HG-- rename : tools/lint/docs/coding-style/coding_style_java.rst => docs/code-quality/coding-style/coding_style_java.rst rename : tools/lint/docs/coding-style/coding_style_other.rst => docs/code-quality/coding-style/coding_style_other.rst rename : tools/lint/docs/coding-style/coding_style_python.rst => docs/code-quality/coding-style/coding_style_python.rst rename : tools/lint/docs/coding-style/format_cpp_code_with_clang-format.rst => docs/code-quality/coding-style/format_cpp_code_with_clang-format.rst rename : tools/clang-tidy/docs/summary.rst => docs/code-quality/index.rst rename : tools/lint/docs/create.rst => docs/code-quality/lint/create.rst rename : tools/lint/docs/index.rst => docs/code-quality/lint/index.rst rename : tools/lint/docs/index.rst => docs/code-quality/lint/lint.rst rename : tools/lint/docs/linters/codespell.rst => docs/code-quality/lint/linters/codespell.rst rename : tools/lint/docs/linters/cpp-virtual-final.rst => docs/code-quality/lint/linters/cpp-virtual-final.rst rename : tools/lint/docs/linters/eslint-plugin-mozilla.rst => docs/code-quality/lint/linters/eslint-plugin-mozilla.rst rename : tools/lint/docs/linters/eslint-plugin-spidermonkey-js.rst => docs/code-quality/lint/linters/eslint-plugin-spidermonkey-js.rst rename : tools/lint/docs/linters/eslint.rst => docs/code-quality/lint/linters/eslint.rst rename : tools/lint/docs/linters/file-perm.rst => docs/code-quality/lint/linters/file-perm.rst rename : tools/lint/docs/linters/file-whitespace.rst => docs/code-quality/lint/linters/file-whitespace.rst rename : tools/lint/docs/linters/flake8.rst => docs/code-quality/lint/linters/flake8.rst rename : tools/lint/docs/linters/l10n.rst => docs/code-quality/lint/linters/l10n.rst rename : tools/lint/docs/linters/license.rst => docs/code-quality/lint/linters/license.rst rename : tools/lint/docs/linters/lintpref.rst => docs/code-quality/lint/linters/lintpref.rst rename : tools/lint/docs/linters/mingw-capitalization.rst => docs/code-quality/lint/linters/mingw-capitalization.rst rename : tools/lint/docs/linters/perfdocs.rst => docs/code-quality/lint/linters/perfdocs.rst rename : tools/lint/docs/linters/rstlinter.rst => docs/code-quality/lint/linters/rstlinter.rst rename : tools/lint/docs/linters/rustfmt.rst => docs/code-quality/lint/linters/rustfmt.rst rename : tools/lint/docs/usage.rst => docs/code-quality/lint/usage.rst rename : tools/clang-tidy/docs/index.rst => docs/code-quality/static-analysis.rst extra : moz-landing-system : lando --- .../coding-style/coding_style_cpp.rst | 808 ++++++++++++++++++ .../coding-style/coding_style_general.rst | 18 + .../coding-style/coding_style_java.rst | 0 .../coding-style/coding_style_js.rst | 150 ++++ .../coding-style/coding_style_other.rst | 0 .../coding-style/coding_style_python.rst | 0 .../format_cpp_code_with_clang-format.rst | 0 docs/code-quality/coding-style/index.rst | 12 + .../code-quality/index.rst | 14 +- .../code-quality/lint}/create.rst | 2 +- .../docs => docs/code-quality/lint}/index.rst | 10 - docs/code-quality/lint/lint.rst | 31 + .../code-quality/lint}/linters/codespell.rst | 0 .../lint}/linters/cpp-virtual-final.rst | 0 .../lint}/linters/eslint-plugin-mozilla.rst | 0 .../linters/eslint-plugin-spidermonkey-js.rst | 0 .../code-quality/lint}/linters/eslint.rst | 0 .../code-quality/lint}/linters/file-perm.rst | 0 .../lint}/linters/file-whitespace.rst | 0 .../code-quality/lint}/linters/flake8.rst | 0 .../code-quality/lint}/linters/l10n.rst | 0 .../code-quality/lint}/linters/license.rst | 0 .../code-quality/lint}/linters/lintpref.rst | 0 .../lint}/linters/mingw-capitalization.rst | 0 .../code-quality/lint}/linters/perfdocs.rst | 0 .../code-quality/lint}/linters/rstlinter.rst | 0 .../code-quality/lint}/linters/rustfmt.rst | 0 .../docs => docs/code-quality/lint}/usage.rst | 0 .../code-quality/static-analysis.rst | 2 +- docs/config.yml | 7 +- docs/index.rst | 6 - moz.build | 2 + tools/lint/docs/Makefile | 192 ----- .../coding-style/coding_style_general.rst | 18 - tools/lint/docs/conf.py | 118 --- tools/lint/docs/make.bat | 263 ------ tools/moz.build | 4 - 37 files changed, 1038 insertions(+), 619 deletions(-) create mode 100644 docs/code-quality/coding-style/coding_style_cpp.rst create mode 100644 docs/code-quality/coding-style/coding_style_general.rst rename {tools/lint/docs => docs/code-quality}/coding-style/coding_style_java.rst (100%) create mode 100644 docs/code-quality/coding-style/coding_style_js.rst rename {tools/lint/docs => docs/code-quality}/coding-style/coding_style_other.rst (100%) rename {tools/lint/docs => docs/code-quality}/coding-style/coding_style_python.rst (100%) rename {tools/lint/docs => docs/code-quality}/coding-style/format_cpp_code_with_clang-format.rst (100%) create mode 100644 docs/code-quality/coding-style/index.rst rename tools/clang-tidy/docs/summary.rst => docs/code-quality/index.rst (94%) rename {tools/lint/docs => docs/code-quality/lint}/create.rst (98%) rename {tools/lint/docs => docs/code-quality/lint}/index.rst (94%) create mode 100644 docs/code-quality/lint/lint.rst rename {tools/lint/docs => docs/code-quality/lint}/linters/codespell.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/cpp-virtual-final.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/eslint-plugin-mozilla.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/eslint-plugin-spidermonkey-js.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/eslint.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/file-perm.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/file-whitespace.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/flake8.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/l10n.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/license.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/lintpref.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/mingw-capitalization.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/perfdocs.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/rstlinter.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/linters/rustfmt.rst (100%) rename {tools/lint/docs => docs/code-quality/lint}/usage.rst (100%) rename tools/clang-tidy/docs/index.rst => docs/code-quality/static-analysis.rst (99%) delete mode 100644 tools/lint/docs/Makefile delete mode 100644 tools/lint/docs/conf.py delete mode 100644 tools/lint/docs/make.bat diff --git a/docs/code-quality/coding-style/coding_style_cpp.rst b/docs/code-quality/coding-style/coding_style_cpp.rst new file mode 100644 index 000000000000..54ae65a7d090 --- /dev/null +++ b/docs/code-quality/coding-style/coding_style_cpp.rst @@ -0,0 +1,808 @@ +================ +C++ Coding style +================ + + +This document attempts to explain the basic styles and patterns used in +the Mozilla codebase. New code should try to conform to these standards, +so it is as easy to maintain as existing code. There are exceptions, but +it's still important to know the rules! + +This article is particularly for those new to the Mozilla codebase, and +in the process of getting their code reviewed. Before requesting a +review, please read over this document, making sure that your code +conforms to recommendations. + +.. container:: blockIndicator warning + + Firefox code base uses the `Google Coding style for C++ + code `__ + + +Formatting code +--------------- + +Formatting is done automatically via clang-format, and controlled via in-tree +configuration files. See :ref:`Formatting C++ Code With clang-format` +for more information. + +Unix-style linebreaks (``\n``), not Windows-style (``\r\n``). You can +convert patches, with DOS newlines to Unix via the ``dos2unix`` utility, +or your favorite text editor. + +Static analysis +--------------- + +Several of the rules in the Google C++ coding styles and the additions mentioned below +can be checked via clang-tidy (some rules are from the upstream clang-tidy, some are +provided via a mozilla-specific plugin). Some of these checks also allow fixes to +be automatically applied. + +``mach static-analysis`` provides a convenient way to run these checks. For example, +for the check called ``google-readability-braces-around-statements``, you can run: + +.. code-block:: shell + + ./mach static-analysis check --checks="-*,google-readability-braces-around-statements" --fix + +It may be necessary to reformat the files after automatically applying fixes, see +:ref:`Formatting C++ Code With clang-format`. + +Additional rules +---------------- + +*The norms in this section should be followed for new code. For existing code, +use the prevailing style in a file or module, ask the owner if you are +in another team's codebase or it's not clear what style to use.* + + + + +Control structures +~~~~~~~~~~~~~~~~~~ + +Always brace controlled statements, even a single-line consequent of +``if else else``. This is redundant, typically, but it avoids dangling +else bugs, so it's safer at scale than fine-tuning. + +Examples: + +.. code-block:: cpp + + if (...) { + } else if (...) { + } else { + } + + while (...) { + } + + do { + } while (...); + + for (...; ...; ...) { + } + + switch (...) { + case 1: { + // When you need to declare a variable in a switch, put the block in braces. + int var; + break; + } + case 2: + ... + break; + default: + break; + } + +``else`` should only ever be followed by ``{`` or ``if``; i.e., other +control keywords are not allowed and should be placed inside braces. + +.. note:: + + For this rule, clang-tidy provides the ``google-readability-braces-around-statements`` + check with autofixes. + + +C++ namespaces +~~~~~~~~~~~~~~ + +Mozilla project C++ declarations should be in the ``mozilla`` +namespace. Modules should avoid adding nested namespaces under +``mozilla``, unless they are meant to contain names which have a high +probability of colliding with other names in the code base. For example, +``Point``, ``Path``, etc. Such symbols can be put under +module-specific namespaces, under ``mozilla``, with short +all-lowercase names. Other global namespaces besides ``mozilla`` are +not allowed. + +No ``using`` directives are allowed in header files, except inside class +definitions or functions. (We don't want to pollute the global scope of +compilation units that use the header file.) + +.. note:: + + For parts of this rule, clang-tidy provides the ``google-global-names-in-headers`` + check. It only detects ``using namespace`` directives in the global namespace. + + +``using namespace ...;`` is only allowed in ``.cpp`` files after all +``#include``\ s. Prefer to wrap code in ``namespace ... { ... };`` +instead, if possible. ``using namespace ...;``\ should always specify +the fully qualified namespace. That is, to use ``Foo::Bar`` do not +write ``using namespace Foo; using namespace Bar;``, write +``using namespace Foo::Bar;`` + + +Anonymous namespaces +~~~~~~~~~~~~~~~~~~~~ + +We prefer using ``static``, instead of anonymous C++ namespaces. This may +change once there is better debugger support (especially on Windows) for +placing breakpoints, etc. on code in anonymous namespaces. You may still +use anonymous namespaces for things that can't be hidden with ``static``, +such as types, or certain objects which need to be passed to template +functions. + + +C++ classes +~~~~~~~~~~~~ + +.. code-block:: cpp + + namespace mozilla { + + class MyClass : public A + { + ... + }; + + class MyClass + : public X + , public Y + { + public: + MyClass(int aVar, int aVar2) + : mVar(aVar) + , mVar2(aVar2) + { + ... + } + + // Special member functions, like constructors, that have default bodies + // should use '= default' annotation instead. + MyClass() = default; + + // Unless it's a copy or move constructor or you have a specific reason to allow + // implicit conversions, mark all single-argument constructors explicit. + explicit MyClass(OtherClass aArg) + { + ... + } + + // This constructor can also take a single argument, so it also needs to be marked + // explicit. + explicit MyClass(OtherClass aArg, AnotherClass aArg2 = AnotherClass()) + { + ... + } + + int LargerFunction() + { + ... + ... + } + + private: + int mVar; + }; + + } // namespace mozilla + +Define classes using the style given above. + +.. note:: + + For the rule on ``= default``, clang-tidy provides the ``modernize-use-default`` + check with autofixes. + + For the rule on explicit constructors and conversion operators, clang-tidy + provides the ``mozilla-implicit-constructor`` check. + +Existing classes in the global namespace are named with a short prefix +(For example, ``ns``) as a pseudo-namespace. + + +Methods and functions +~~~~~~~~~~~~~~~~~~~~~ + + +C/C++ +^^^^^ + +In C/C++, method names should use ``UpperCamelCase``. + +Getters that never fail, and never return null, are named ``Foo()``, +while all other getters use ``GetFoo()``. Getters can return an object +value, via a ``Foo** aResult`` outparam (typical for an XPCOM getter), +or as an ``already_AddRefed`` (typical for a WebIDL getter, +possibly with an ``ErrorResult& rv`` parameter), or occasionally as a +``Foo*`` (typical for an internal getter for an object with a known +lifetime). See `the bug 223255 `_ +for more information. + +XPCOM getters always return primitive values via an outparam, while +other getters normally use a return value. + +Method declarations must use, at most, one of the following keywords: +``virtual``, ``override``, or ``final``. Use ``virtual`` to declare +virtual methods, which do not override a base class method with the same +signature. Use ``override`` to declare virtual methods which do +override a base class method, with the same signature, but can be +further overridden in derived classes. Use ``final`` to declare virtual +methods which do override a base class method, with the same signature, +but can NOT be further overridden in the derived classes. This should +help the person reading the code fully understand what the declaration +is doing, without needing to further examine base classes. + +.. note:: + + For the rule on ``virtual/override/final``, clang-tidy provides the + ``modernize-use-override`` check with autofixes. + + +Operators +~~~~~~~~~ + +The unary keyword operator ``sizeof``, should have its operand parenthesized +even if it is an expression; e.g. ``int8_t arr[64]; memset(arr, 42, sizeof(arr));``. + + +Literals +~~~~~~~~ + +Use ``\uXXXX`` unicode escapes for non-ASCII characters. The character +set for XUL, DTD, script, and properties files is UTF-8, which is not easily +readable. + + +Prefixes +~~~~~~~~ + +Follow these naming prefix conventions: + + +Variable prefixes +^^^^^^^^^^^^^^^^^ + +- k=constant (e.g. ``kNC_child``). Not all code uses this style; some + uses ``ALL_CAPS`` for constants. +- g=global (e.g. ``gPrefService``) +- a=argument (e.g. ``aCount``) +- C++ Specific Prefixes + + - s=static member (e.g. ``sPrefChecked``) + - m=member (e.g. ``mLength``) + - e=enum variants (e.g. ``enum Foo { eBar, eBaz }``). Enum classes + should use ``CamelCase`` instead (e.g. + ``enum class Foo { Bar, Baz }``). + + +Global functions/macros/etc +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +- Macros begin with ``MOZ_``, and are all caps (e.g. + ``MOZ_WOW_GOODNESS``). Note that older code uses the ``NS_`` prefix; + while these aren't being changed, you should only use ``MOZ_`` for + new macros. The only exception is if you're creating a new macro, + which is part of a set of related macros still using the old ``NS_`` + prefix. Then you should be consistent with the existing macros. + + +Error Variables +^^^^^^^^^^^^^^^ + +- Local variables that are assigned ``nsresult`` result codes should be named ``rv`` + (i.e., e.g., not ``res``, not ``result``, not ``foo``). `rv` should not be + used for bool or other result types. +- Local variables that are assigned ``bool`` result codes should be named `ok`. + + +C/C++ practices +--------------- + +- **Have you checked for compiler warnings?** Warnings often point to + real bugs. `Many of them `__ + are enabled by default in the build system. +- In C++ code, use ``nullptr`` for pointers. In C code, using ``NULL`` + or ``0`` is allowed. + +.. note:: + + For the C++ rule, clang-tidy provides the ``modernize-use-nullptr`` check + with autofixes. + +- Don't use ``PRBool`` and ``PRPackedBool`` in C++, use ``bool`` + instead. +- For checking if a ``std`` container has no items, don't use + ``size()``, instead use ``empty()``. +- When testing a pointer, use ``(!myPtr)`` or ``(myPtr)``; + don't use ``myPtr != nullptr`` or ``myPtr == nullptr``. +- Do not compare ``x == true`` or ``x == false``. Use ``(x)`` or + ``(!x)`` instead. ``if (x == true)`` may have semantics different from + ``if (x)``! + +.. note:: + + clang-tidy provides the ``readability-simplify-boolean-expr`` check + with autofixes that checks for these and some other boolean expressions + that can be simplified. + +- In general, initialize variables with ``nsFoo aFoo = bFoo,`` and not + ``nsFoo aFoo(bFoo)``. + + - For constructors, initialize member variables with : ``nsFoo + aFoo(bFoo)`` syntax. + +- To avoid warnings created by variables used only in debug builds, use + the + `DebugOnly `__ + helper when declaring them. +- You should `use the static preference + API `__ for + working with preferences. +- One-argument constructors, that are not copy or move constructors, + should generally be marked explicit. Exceptions should be annotated + with ``MOZ_IMPLICIT``. +- Use ``char32_t`` as the return type or argument type of a method that + returns or takes as argument a single Unicode scalar value. (Don't + use UTF-32 strings, though.) +- Forward-declare classes in your header files, instead of including + them, whenever possible. For example, if you have an interface with a + ``void DoSomething(nsIContent* aContent)`` function, forward-declare + with ``class nsIContent;`` instead of ``#include "nsIContent.h"`` +- Include guards are named per the Google coding style and should not + include a leading ``MOZ_`` or ``MOZILLA_``. For example + ``dom/media/foo.h`` would use the guard ``DOM_MEDIA_FOO_H_``. + + + + +COM and pointers +---------------- + +- Use ``nsCOMPtr<>`` + If you don't know how to use it, start looking in the code for + examples. The general rule, is that the very act of typing + ``NS_RELEASE`` should be a signal to you to question your code: + "Should I be using ``nsCOMPtr`` here?". Generally the only valid use + of ``NS_RELEASE`` is when you are storing refcounted pointers in a + long-lived datastructure. +- Declare new XPCOM interfaces using `XPIDL `__, so they + will be scriptable. +- Use `nsCOMPtr `__ for strong references, and + `nsWeakPtr `__ for weak references. +- Don't use ``QueryInterface`` directly. Use ``CallQueryInterface`` or + ``do_QueryInterface`` instead. +- Use `Contract + IDs `__, + instead of CIDs with ``do_CreateInstance``/``do_GetService``. +- Use pointers, instead of references for function out parameters, even + for primitive types. + + +IDL +--- + + +Use leading-lowercase, or "interCaps" +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When defining a method or attribute in IDL, the first letter should be +lowercase, and each following word should be capitalized. For example: + +.. code-block:: cpp + + long updateStatusBar(); + + +Use attributes wherever possible +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Whenever you are retrieving or setting a single value, without any +context, you should use attributes. Don't use two methods when you could +use an attribute. Using attributes logically connects the getting and +setting of a value, and makes scripted code look cleaner. + +This example has too many methods: + +.. code-block:: cpp + + interface nsIFoo : nsISupports + { + long getLength(); + void setLength(in long length); + long getColor(); + }; + +The code below will generate the exact same C++ signature, but is more +script-friendly. + +.. code-block:: cpp + + interface nsIFoo : nsISupports + { + attribute long length; + readonly attribute long color; + }; + + +Use Java-style constants +~~~~~~~~~~~~~~~~~~~~~~~~ + +When defining scriptable constants in IDL, the name should be all +uppercase, with underscores between words: + +.. code-block:: cpp + + const long ERROR_UNDEFINED_VARIABLE = 1; + + +See also +~~~~~~~~ + +For details on interface development, as well as more detailed style +guides, see the `Interface development +guide `__. + + +Error handling +-------------- + + +Check for errors early and often +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Every time you make a call into an XPCOM function, you should check for +an error condition. You need to do this even if you know that call will +never fail. Why? + +- Someone may change the callee in the future to return a failure + condition. +- The object in question may live on another thread, another process, + or possibly even another machine. The proxy could have failed to make + your call in the first place. + +Also, when you make a new function which is failable (i.e. it will +return a ``nsresult`` or a ``bool`` that may indicate an error), you should +explicitly mark the return value should always be checked. For example: + +:: + + // for IDL. + [must_use] nsISupports + create(); + + // for C++, add this in *declaration*, do not add it again in implementation. + MOZ_MUST_USE nsresult + DoSomething(); + +There are some exceptions: + +- Predicates or getters, which return ``bool`` or ``nsresult``. +- IPC method implementation (For example, ``bool RecvSomeMessage()``). +- Most callers will check the output parameter, see below. + +.. code-block:: cpp + + nsresult + SomeMap::GetValue(const nsString& key, nsString& value); + +If most callers need to check the output value first, then adding +``MOZ_MUST_USE`` might be too verbose. In this case, change the return value +to void might be a reasonable choice. + +There is also a static analysis attribute ``MOZ_MUST_USE_TYPE``, which can +be added to class declarations, to ensure that those declarations are +always used when they are returned. + + +Use the NS_WARN_IF macro when errors are unexpected. +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The ``NS_WARN_IF`` macro can be used to issue a console warning, in debug +builds if the condition fails. This should only be used when the failure +is unexpected and cannot be caused by normal web content. + +If you are writing code which wants to issue warnings when methods fail, +please either use ``NS_WARNING`` directly, or use the new ``NS_WARN_IF`` macro. + +.. code-block:: cpp + + if (NS_WARN_IF(somethingthatshouldbefalse)) { + return NS_ERROR_INVALID_ARG; + } + + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + +Previously, the ``NS_ENSURE_*`` macros were used for this purpose, but +those macros hide return statements, and should not be used in new code. +(This coding style rule isn't generally agreed, so use of ``NS_ENSURE_*`` +can be valid.) + + +Return from errors immediately +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +In most cases, your knee-jerk reaction should be to return from the +current function, when an error condition occurs. Don't do this: + +.. code-block:: cpp + + rv = foo->Call1(); + if (NS_SUCCEEDED(rv)) { + rv = foo->Call2(); + if (NS_SUCCEEDED(rv)) { + rv = foo->Call3(); + } + } + return rv; + +Instead, do this: + +.. code-block:: cpp + + rv = foo->Call1(); + if (NS_FAILED(rv)) { + return rv; + } + + rv = foo->Call2(); + if (NS_FAILED(rv)) { + return rv; + } + + rv = foo->Call3(); + if (NS_FAILED(rv)) { + return rv; + } + +Why? Error handling should not obfuscate the logic of the code. The +author's intent, in the first example, was to make 3 calls in +succession. Wrapping the calls in nested if() statements, instead +obscured the most likely behavior of the code. + +Consider a more complicated example to hide a bug: + +.. code-block:: cpp + + bool val; + rv = foo->GetBooleanValue(&val); + if (NS_SUCCEEDED(rv) && val) { + foo->Call1(); + } else { + foo->Call2(); + } + +The intent of the author, may have been, that ``foo->Call2()`` would only +happen when val had a false value. In fact, ``foo->Call2()`` will also be +called, when ``foo->GetBooleanValue(&val)`` fails. This may, or may not, +have been the author's intent. It is not clear from this code. Here is +an updated version: + +.. code-block:: cpp + + bool val; + rv = foo->GetBooleanValue(&val); + if (NS_FAILED(rv)) { + return rv; + } + if (val) { + foo->Call1(); + } else { + foo->Call2(); + } + +In this example, the author's intent is clear, and an error condition +avoids both calls to ``foo->Call1()`` and ``foo->Call2();`` + +*Possible exceptions:* Sometimes it is not fatal if a call fails. For +instance, if you are notifying a series of observers that an event has +fired, it might be trivial that one of these notifications failed: + +.. code-block:: cpp + + for (size_t i = 0; i < length; ++i) { + // we don't care if any individual observer fails + observers[i]->Observe(foo, bar, baz); + } + +Another possibility, is you are not sure if a component exists or is +installed, and you wish to continue normally, if the component is not +found. + +.. code-block:: cpp + + nsCOMPtr service = do_CreateInstance(NS_MYSERVICE_CID, &rv); + // if the service is installed, then we'll use it. + if (NS_SUCCEEDED(rv)) { + // non-fatal if this fails too, ignore this error. + service->DoSomething(); + + // this is important, handle this error! + rv = service->DoSomethingImportant(); + if (NS_FAILED(rv)) { + return rv; + } + } + + // continue normally whether or not the service exists. + + +Strings +------- + +- String arguments to functions should be declared as ``nsAString``. +- Use ``EmptyString()`` and ``EmptyCString()`` instead of + ``NS_LITERAL_STRING("")`` or ``nsAutoString empty;``. +- Use ``str.IsEmpty()`` instead of ``str.Length() == 0``. +- Use ``str.Truncate()`` instead of ``str.SetLength(0)`` or + ``str.Assign(EmptyString())``. +- For constant strings, use ``NS_LITERAL_STRING("...")`` instead of + ``NS_ConvertASCIItoUCS2("...")``, ``AssignWithConversion("...")``, + ``EqualsWithConversion("...")``, or ``nsAutoString()`` +- To compare a string with a literal, use ``.EqualsLiteral("...")``. +- Don't use functions from ``ctype.h`` (``isdigit()``, ``isalpha()``, + etc.) or from ``strings.h`` (``strcasecmp()``, ``strncasecmp()``). + These are locale-sensitive, which makes them inappropriate for + processing protocol text. At the same time, they are too limited to + work properly for processing natural-language text. Use the + alternatives in ``mozilla/TextUtils.h`` and in ``nsUnicharUtils.h`` + in place of ``ctype.h``. In place of ``strings.h``, prefer the + ``nsStringComparator`` facilities for comparing strings or if you + have to work with zero-terminated strings, use ``nsCRT.h`` for + ASCII-case-insensitive comparison. + + +Use the ``Auto`` form of strings for local values +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When declaring a local, short-lived ``nsString`` class, always use +``nsAutoString`` or ``nsAutoCString``. These pre-allocate a 64-byte +buffer on the stack, and avoid fragmenting the heap. Don't do this: + +.. code-block:: cpp + + nsresult + foo() + { + nsCString bar; + .. + } + +instead: + +.. code-block:: cpp + + nsresult + foo() + { + nsAutoCString bar; + .. + } + + +Be wary of leaking values from non-XPCOM functions that return char\* or PRUnichar\* +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +It is an easy trap to return an allocated string, from an internal +helper function, and then using that function inline in your code, +without freeing the value. Consider this code: + +.. code-block:: cpp + + static char* + GetStringValue() + { + .. + return resultString.ToNewCString(); + } + + .. + WarnUser(GetStringValue()); + +In the above example, ``WarnUser`` will get the string allocated from +``resultString.ToNewCString()`` and throw away the pointer. The +resulting value is never freed. Instead, either use the string classes, +to make sure your string is automatically freed when it goes out of +scope, or make sure that your string is freed. + +Automatic cleanup: + +.. code-block:: cpp + + static void + GetStringValue(nsAWritableCString& aResult) + { + .. + aResult.Assign("resulting string"); + } + + .. + nsAutoCString warning; + GetStringValue(warning); + WarnUser(warning.get()); + +Free the string manually: + +.. code-block:: cpp + + static char* + GetStringValue() + { + .. + return resultString.ToNewCString(); + } + + .. + char* warning = GetStringValue(); + WarnUser(warning); + nsMemory::Free(warning); + + +Use MOZ_UTF16() or NS_LITERAL_STRING() to avoid runtime string conversion +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +It is very common to need to assign the value of a literal string, such +as ``"Some String"``, into a unicode buffer. Instead of using ``nsString``'s +``AssignLiteral`` and ``AppendLiteral``, use ``NS_LITERAL_STRING()`` +instead. On most platforms, this will force the compiler to compile in a +raw unicode string, and assign it directly. + +Incorrect: + +.. code-block:: cpp + + nsAutoString warning; + warning.AssignLiteral("danger will robinson!"); + ... + foo->SetStringValue(warning); + ... + bar->SetUnicodeValue(warning.get()); + +Correct: + +.. code-block:: cpp + + NS_NAMED_LITERAL_STRING(warning, "danger will robinson!"); + ... + // if you'll be using the 'warning' string, you can still use it as before: + foo->SetStringValue(warning); + ... + bar->SetUnicodeValue(warning.get()); + + // alternatively, use the wide string directly: + foo->SetStringValue(NS_LITERAL_STRING("danger will robinson!")); + ... + bar->SetUnicodeValue(MOZ_UTF16("danger will robinson!")); + +.. note:: + + Note: Named literal strings cannot yet be static. + + +Usage of PR_(MAX|MIN|ABS|ROUNDUP) macro calls +--------------------------------------------- + +Use the standard-library functions (``std::max``), instead of +``PR_(MAX|MIN|ABS|ROUNDUP)``. + +Use ``mozilla::Abs`` instead of ``PR_ABS``. All ``PR_ABS`` calls in C++ code have +been replaced with ``mozilla::Abs`` calls, in `bug +847480 `__. All new +code in ``Firefox/core/toolkit`` needs to ``#include "nsAlgorithm.h"`` and +use the ``NS_foo`` variants instead of ``PR_foo``, or +``#include "mozilla/MathAlgorithms.h"`` for ``mozilla::Abs``. diff --git a/docs/code-quality/coding-style/coding_style_general.rst b/docs/code-quality/coding-style/coding_style_general.rst new file mode 100644 index 000000000000..dfa2269ca11d --- /dev/null +++ b/docs/code-quality/coding-style/coding_style_general.rst @@ -0,0 +1,18 @@ + +Mode line +~~~~~~~~~ + +Files should have Emacs and vim mode line comments as the first two +lines of the file, which should set ``indent-tabs-mode`` to ``nil``. For new +files, use the following, specifying two-space indentation: + +.. code-block:: cpp + + /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ + /* vim: set ts=8 sts=2 et sw=2 tw=80: */ + /* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +Be sure to use the correct ``Mode`` in the first line, don't use ``C++`` in +JavaScript files. diff --git a/tools/lint/docs/coding-style/coding_style_java.rst b/docs/code-quality/coding-style/coding_style_java.rst similarity index 100% rename from tools/lint/docs/coding-style/coding_style_java.rst rename to docs/code-quality/coding-style/coding_style_java.rst diff --git a/docs/code-quality/coding-style/coding_style_js.rst b/docs/code-quality/coding-style/coding_style_js.rst new file mode 100644 index 000000000000..46ec1dd8a6aa --- /dev/null +++ b/docs/code-quality/coding-style/coding_style_js.rst @@ -0,0 +1,150 @@ +======================= +JavaScript Coding style +======================= + +Coding style +~~~~~~~~~~~~ + +`prettier `_ is the tool used to reformat the JavaScript code. + + +Methods and functions +~~~~~~~~~~~~~~~~~~~~~ + +In JavaScript, functions should use camelCase, but should not capitalize +the first letter. Methods should not use the named function expression +syntax, because our tools understand method names: + +.. code-block:: cpp + + doSomething: function (aFoo, aBar) { + ... + } + +In-line functions should have spaces around braces, except before commas +or semicolons: + +.. code-block:: cpp + + function valueObject(aValue) { return { value: aValue }; } + + +JavaScript objects +~~~~~~~~~~~~~~~~~~ + +.. code-block:: cpp + + var foo = { prop1: "value1" }; + + var bar = { + prop1: "value1", + prop2: "value2" + }; + +Constructors for objects should be capitalized and use Pascal Case: + +.. code-block:: cpp + + function ObjectConstructor() { + this.foo = "bar"; + } + + +Operators +~~~~~~~~~ + +In JavaScript, overlong expressions not joined by ``&&`` and +``||`` should break so the operator starts on the second line and +starting in the same column as the beginning of the expression in the +first line. This applies to ``?:``, binary arithmetic operators +including ``+``, and member-of operators. Rationale: an operator at the +front of the continuation line makes for faster visual scanning, as +there is no need to read to the end of line. Also there exists a +context-sensitive keyword hazard in JavaScript; see {{bug(442099, "bug", +19)}}, which can be avoided by putting . at the start of a continuation +line, in long member expression. + +In JavaScript, ``==`` is preferred to ``===``. + +Unary keyword operators, such as ``typeof``, should have their operand +parenthesized; e.g. ``typeof("foo") == "string"``. + +Literals +~~~~~~~~ + +Double-quoted strings (e.g. ``"foo"``) are preferred to single-quoted +strings (e.g. ``'foo'``), in JavaScript, except to avoid escaping +embedded double quotes, or when assigning inline event handlers. + + +Prefixes +~~~~~~~~ + +- k=constant (e.g. ``kNC_child``). Not all code uses this style; some + uses ``ALL_CAPS`` for constants. +- g=global (e.g. ``gPrefService``) +- a=argument (e.g. ``aCount``) + +- JavaScript Specific Prefixes + + - \_=member (variable or function) (e.g. ``_length`` or + ``_setType(aType)``) + - k=enumeration value (e.g. ``const kDisplayModeNormal = 0``) + - on=event handler (e.g. ``function onLoad()``) + - Convenience constants for interface names should be prefixed with + ``nsI``: + + .. code-block:: javascript + + const nsISupports = Components.interfaces.nsISupports; + const nsIWBN = Components.interfaces.nsIWebBrowserNavigation; + + + +Other advices +~~~~~~~~~~~~~ + +- Make sure you are aware of the `JavaScript + Tips `__. +- Do not compare ``x == true`` or ``x == false``. Use ``(x)`` or + ``(!x)`` instead. ``x == true``, is certainly different from if + ``(x)``! Compare objects to ``null``, numbers to ``0`` or strings to + ``""``, if there is chance for confusion. +- Make sure that your code doesn't generate any strict JavaScript + warnings, such as: + + - Duplicate variable declaration. + - Mixing ``return;`` with ``return value;`` + - Undeclared variables or members. If you are unsure if an array + value exists, compare the index to the array's length. If you are + unsure if an object member exists, use ``"name"`` in ``aObject``, + or if you are expecting a particular type you may use + ``typeof(aObject.name) == "function"`` (or whichever type you are + expecting). + +- Use ``['value1, value2']`` to create a JavaScript array in preference + to using + ``new {{JSxRef("Array", "Array", "Syntax", 1)}}(value1, value2)`` + which can be confusing, as ``new Array(length)`` will actually create + a physically empty array with the given logical length, while + ``[value]`` will always create a 1-element array. You cannot actually + guarantee to be able to preallocate memory for an array. +- Use ``{ member: value, ... }`` to create a JavaScript object; a + useful advantage over ``new {{JSxRef("Object", "Object", "", 1)}}()`` + is the ability to create initial properties and use extended + JavaScript syntax, to define getters and setters. +- If having defined a constructor you need to assign default + properties, it is preferred to assign an object literal to the + prototype property. +- Use regular expressions, but use wisely. For instance, to check that + ``aString`` is not completely whitespace use + ``/\S/.{{JSxRef("RegExp.test", "test(aString)", "", 1)}}``. Only use + {{JSxRef("String.search", "aString.search()")}} if you need to know + the position of the result, or {{JSxRef("String.match", + "aString.match()")}} if you need to collect matching substrings + (delimited by parentheses in the regular expression). Regular + expressions are less useful if the match is unknown in advance, or to + extract substrings in known positions in the string. For instance, + {{JSxRef("String.slice", "aString.slice(-1)")}} returns the last + letter in ``aString``, or the empty string if ``aString`` is empty. + diff --git a/tools/lint/docs/coding-style/coding_style_other.rst b/docs/code-quality/coding-style/coding_style_other.rst similarity index 100% rename from tools/lint/docs/coding-style/coding_style_other.rst rename to docs/code-quality/coding-style/coding_style_other.rst diff --git a/tools/lint/docs/coding-style/coding_style_python.rst b/docs/code-quality/coding-style/coding_style_python.rst similarity index 100% rename from tools/lint/docs/coding-style/coding_style_python.rst rename to docs/code-quality/coding-style/coding_style_python.rst diff --git a/tools/lint/docs/coding-style/format_cpp_code_with_clang-format.rst b/docs/code-quality/coding-style/format_cpp_code_with_clang-format.rst similarity index 100% rename from tools/lint/docs/coding-style/format_cpp_code_with_clang-format.rst rename to docs/code-quality/coding-style/format_cpp_code_with_clang-format.rst diff --git a/docs/code-quality/coding-style/index.rst b/docs/code-quality/coding-style/index.rst new file mode 100644 index 000000000000..0b72218256cc --- /dev/null +++ b/docs/code-quality/coding-style/index.rst @@ -0,0 +1,12 @@ +Coding style +============ + +Firefox code is using different programming languages. +For each language, we are enforcing a specific coding style. + +.. toctree:: + :maxdepth: 2 + :glob: + + * + diff --git a/tools/clang-tidy/docs/summary.rst b/docs/code-quality/index.rst similarity index 94% rename from tools/clang-tidy/docs/summary.rst rename to docs/code-quality/index.rst index 8c13fd19dae5..fe26db813700 100644 --- a/tools/clang-tidy/docs/summary.rst +++ b/docs/code-quality/index.rst @@ -1,10 +1,19 @@ -Static analyzers and linters -============================ +Code quality +============ Because Firefox is a complex piece of software, a lot of tools are executed to identify issues at development phase. In this document, we try to list these all tools. + +.. toctree:: + :maxdepth: 1 + :glob: + + static-analysis.rst + lint/index.rst + coding-style/index.rst + .. list-table:: C/C++ :widths: 25 25 25 20 :header-rows: 1 @@ -112,4 +121,3 @@ In this document, we try to list these all tools. - - - https://github.com/facebook/infer - diff --git a/tools/lint/docs/create.rst b/docs/code-quality/lint/create.rst similarity index 98% rename from tools/lint/docs/create.rst rename to docs/code-quality/lint/create.rst index 06c8b6bc6260..170f0362dc00 100644 --- a/tools/lint/docs/create.rst +++ b/docs/code-quality/lint/create.rst @@ -10,7 +10,7 @@ For a linter to be integrated into the mozilla-central tree, it needs to have: * A ``./mach lint`` interface * Running ``./mach lint`` command must pass (note, linters can be disabled for individual directories) * Taskcluster/Treeherder integration -* In tree documentation (under ``tools/lint/docs``) to give a basic summary, links and any other useful information +* In tree documentation (under ``docs/code-quality/lint``) to give a basic summary, links and any other useful information * Unit tests (under ``tools/lint/test``) to make sure that the linter works as expected and we don't regress. The review group in Phabricator is ``#linter-reviewers``. diff --git a/tools/lint/docs/index.rst b/docs/code-quality/lint/index.rst similarity index 94% rename from tools/lint/docs/index.rst rename to docs/code-quality/lint/index.rst index 1817448e3d24..9ea3f70c7049 100644 --- a/tools/lint/docs/index.rst +++ b/docs/code-quality/lint/index.rst @@ -29,13 +29,3 @@ like mach, phabricator and taskcluster. usage create linters/* - -Coding style -============ - -.. toctree:: - :maxdepth: 1 - :glob: - - coding-style/* - diff --git a/docs/code-quality/lint/lint.rst b/docs/code-quality/lint/lint.rst new file mode 100644 index 000000000000..9ea3f70c7049 --- /dev/null +++ b/docs/code-quality/lint/lint.rst @@ -0,0 +1,31 @@ +Linting +======= + +Linters are used in mozilla-central to help enforce coding style and avoid bad practices. Due to the +wide variety of languages in use, this is not always an easy task. In addition, linters should be runnable from editors, from the command line, from review tools +and from continuous integration. It's easy to see how the complexity of running all of these +different kinds of linters in all of these different places could quickly balloon out of control. + +``Mozlint`` is a library that accomplishes several goals: + +1. It provides a standard method for adding new linters to the tree, which can be as easy as + defining a config object in a ``.yml`` file. This helps keep lint related code localized, and + prevents different teams from coming up with their own unique lint implementations. +2. It provides a streamlined interface for running all linters at once. Instead of running N + different lint commands to test your patch, a single ``mach lint`` command will automatically run + all applicable linters. This means there is a single API surface that other tools can use to + invoke linters. +3. With a simple taskcluster configuration, Mozlint provides an easy way to execute all these jobs + at review phase. + +``Mozlint`` isn't designed to be used directly by end users. Instead, it can be consumed by things +like mach, phabricator and taskcluster. + +.. toctree:: + :caption: Linting User Guide + :maxdepth: 1 + :glob: + + usage + create + linters/* diff --git a/tools/lint/docs/linters/codespell.rst b/docs/code-quality/lint/linters/codespell.rst similarity index 100% rename from tools/lint/docs/linters/codespell.rst rename to docs/code-quality/lint/linters/codespell.rst diff --git a/tools/lint/docs/linters/cpp-virtual-final.rst b/docs/code-quality/lint/linters/cpp-virtual-final.rst similarity index 100% rename from tools/lint/docs/linters/cpp-virtual-final.rst rename to docs/code-quality/lint/linters/cpp-virtual-final.rst diff --git a/tools/lint/docs/linters/eslint-plugin-mozilla.rst b/docs/code-quality/lint/linters/eslint-plugin-mozilla.rst similarity index 100% rename from tools/lint/docs/linters/eslint-plugin-mozilla.rst rename to docs/code-quality/lint/linters/eslint-plugin-mozilla.rst diff --git a/tools/lint/docs/linters/eslint-plugin-spidermonkey-js.rst b/docs/code-quality/lint/linters/eslint-plugin-spidermonkey-js.rst similarity index 100% rename from tools/lint/docs/linters/eslint-plugin-spidermonkey-js.rst rename to docs/code-quality/lint/linters/eslint-plugin-spidermonkey-js.rst diff --git a/tools/lint/docs/linters/eslint.rst b/docs/code-quality/lint/linters/eslint.rst similarity index 100% rename from tools/lint/docs/linters/eslint.rst rename to docs/code-quality/lint/linters/eslint.rst diff --git a/tools/lint/docs/linters/file-perm.rst b/docs/code-quality/lint/linters/file-perm.rst similarity index 100% rename from tools/lint/docs/linters/file-perm.rst rename to docs/code-quality/lint/linters/file-perm.rst diff --git a/tools/lint/docs/linters/file-whitespace.rst b/docs/code-quality/lint/linters/file-whitespace.rst similarity index 100% rename from tools/lint/docs/linters/file-whitespace.rst rename to docs/code-quality/lint/linters/file-whitespace.rst diff --git a/tools/lint/docs/linters/flake8.rst b/docs/code-quality/lint/linters/flake8.rst similarity index 100% rename from tools/lint/docs/linters/flake8.rst rename to docs/code-quality/lint/linters/flake8.rst diff --git a/tools/lint/docs/linters/l10n.rst b/docs/code-quality/lint/linters/l10n.rst similarity index 100% rename from tools/lint/docs/linters/l10n.rst rename to docs/code-quality/lint/linters/l10n.rst diff --git a/tools/lint/docs/linters/license.rst b/docs/code-quality/lint/linters/license.rst similarity index 100% rename from tools/lint/docs/linters/license.rst rename to docs/code-quality/lint/linters/license.rst diff --git a/tools/lint/docs/linters/lintpref.rst b/docs/code-quality/lint/linters/lintpref.rst similarity index 100% rename from tools/lint/docs/linters/lintpref.rst rename to docs/code-quality/lint/linters/lintpref.rst diff --git a/tools/lint/docs/linters/mingw-capitalization.rst b/docs/code-quality/lint/linters/mingw-capitalization.rst similarity index 100% rename from tools/lint/docs/linters/mingw-capitalization.rst rename to docs/code-quality/lint/linters/mingw-capitalization.rst diff --git a/tools/lint/docs/linters/perfdocs.rst b/docs/code-quality/lint/linters/perfdocs.rst similarity index 100% rename from tools/lint/docs/linters/perfdocs.rst rename to docs/code-quality/lint/linters/perfdocs.rst diff --git a/tools/lint/docs/linters/rstlinter.rst b/docs/code-quality/lint/linters/rstlinter.rst similarity index 100% rename from tools/lint/docs/linters/rstlinter.rst rename to docs/code-quality/lint/linters/rstlinter.rst diff --git a/tools/lint/docs/linters/rustfmt.rst b/docs/code-quality/lint/linters/rustfmt.rst similarity index 100% rename from tools/lint/docs/linters/rustfmt.rst rename to docs/code-quality/lint/linters/rustfmt.rst diff --git a/tools/lint/docs/usage.rst b/docs/code-quality/lint/usage.rst similarity index 100% rename from tools/lint/docs/usage.rst rename to docs/code-quality/lint/usage.rst diff --git a/tools/clang-tidy/docs/index.rst b/docs/code-quality/static-analysis.rst similarity index 99% rename from tools/clang-tidy/docs/index.rst rename to docs/code-quality/static-analysis.rst index 4bd07f060f4e..9b1fb0bcd95c 100644 --- a/tools/clang-tidy/docs/index.rst +++ b/docs/code-quality/static-analysis.rst @@ -10,7 +10,7 @@ For linting, please see the `linting documentation `_. Clang-Tidy static analysis -------------------------- -Our current static-analysis infrastruture is based on +Our current static-analysis infrastructure is based on `clang-tidy `__. It uses checkers in order to assert different programming errors present in the code. The checkers that we use are split into 3 categories: diff --git a/docs/config.yml b/docs/config.yml index 14f299a32e1d..a6b1bf401854 100644 --- a/docs/config.yml +++ b/docs/config.yml @@ -18,6 +18,7 @@ categories: - remote - services/common/services - uriloader + - code-quality build_doc: - mach - tools/try @@ -29,9 +30,6 @@ categories: - testing/geckodriver - web-platform - tools/fuzzing - code_quality_doc: - - tools/lint - - tools/static-analysis l10n_doc: - intl - l10n @@ -57,3 +55,6 @@ redirects: tools/docs/index.html: tools/moztreedocs/index.html tools/docs/contribute/how_to_contribute_firefox.html: contributing/how_to_contribute_firefox.html tools/docs/contribute/directory_structure.html: contributing/directory_structure.html + tools/lint: code-quality/lint + tools/lint/coding-style: code-quality/coding-style + tools/static-analysis/index.html: code-quality/static-analysis.html diff --git a/docs/index.rst b/docs/index.rst index 63c96a21003c..bd2836f61855 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -39,12 +39,6 @@ Mozilla Source Tree Documentation {python_doc} -.. toctree:: - :caption: Code quality - :maxdepth: 1 - - {code_quality_doc} - Indices and tables ================== diff --git a/moz.build b/moz.build index 015a6ca43651..6c3788799c15 100644 --- a/moz.build +++ b/moz.build @@ -185,4 +185,6 @@ DEFINES['top_srcdir'] = TOPSRCDIR SPHINX_TREES['contributing'] = 'docs/contributing' +SPHINX_TREES['code-quality'] = 'docs/code-quality' + include('build/templates.mozbuild') diff --git a/tools/lint/docs/Makefile b/tools/lint/docs/Makefile deleted file mode 100644 index 1d97fa065b6d..000000000000 --- a/tools/lint/docs/Makefile +++ /dev/null @@ -1,192 +0,0 @@ -# Makefile for Sphinx documentation -# - -# You can set these variables from the command line. -SPHINXOPTS = -SPHINXBUILD = sphinx-build -PAPER = -BUILDDIR = _build - -# User-friendly check for sphinx-build -ifeq ($(shell which $(SPHINXBUILD) >/dev/null 2>&1; echo $$?), 1) -$(error The '$(SPHINXBUILD)' command was not found. Make sure you have Sphinx installed, then set the SPHINXBUILD environment variable to point to the full path of the '$(SPHINXBUILD)' executable. Alternatively you can add the directory with the executable to your PATH. If you don't have Sphinx installed, grab it from http://sphinx-doc.org/) -endif - -# Internal variables. -PAPEROPT_a4 = -D latex_paper_size=a4 -PAPEROPT_letter = -D latex_paper_size=letter -ALLSPHINXOPTS = -d $(BUILDDIR)/doctrees $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) . -# the i18n builder cannot share the environment and doctrees with the others -I18NSPHINXOPTS = $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) . - -.PHONY: help clean html dirhtml singlehtml pickle json htmlhelp qthelp devhelp epub latex latexpdf text man changes linkcheck doctest coverage gettext - -help: - @echo "Please use \`make ' where is one of" - @echo " html to make standalone HTML files" - @echo " dirhtml to make HTML files named index.html in directories" - @echo " singlehtml to make a single large HTML file" - @echo " pickle to make pickle files" - @echo " json to make JSON files" - @echo " htmlhelp to make HTML files and a HTML help project" - @echo " qthelp to make HTML files and a qthelp project" - @echo " applehelp to make an Apple Help Book" - @echo " devhelp to make HTML files and a Devhelp project" - @echo " epub to make an epub" - @echo " latex to make LaTeX files, you can set PAPER=a4 or PAPER=letter" - @echo " latexpdf to make LaTeX files and run them through pdflatex" - @echo " latexpdfja to make LaTeX files and run them through platex/dvipdfmx" - @echo " text to make text files" - @echo " man to make manual pages" - @echo " texinfo to make Texinfo files" - @echo " info to make Texinfo files and run them through makeinfo" - @echo " gettext to make PO message catalogs" - @echo " changes to make an overview of all changed/added/deprecated items" - @echo " xml to make Docutils-native XML files" - @echo " pseudoxml to make pseudoxml-XML files for display purposes" - @echo " linkcheck to check all external links for integrity" - @echo " doctest to run all doctests embedded in the documentation (if enabled)" - @echo " coverage to run coverage check of the documentation (if enabled)" - -clean: - rm -rf $(BUILDDIR)/* - -html: - $(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(BUILDDIR)/html - @echo - @echo "Build finished. The HTML pages are in $(BUILDDIR)/html." - -dirhtml: - $(SPHINXBUILD) -b dirhtml $(ALLSPHINXOPTS) $(BUILDDIR)/dirhtml - @echo - @echo "Build finished. The HTML pages are in $(BUILDDIR)/dirhtml." - -singlehtml: - $(SPHINXBUILD) -b singlehtml $(ALLSPHINXOPTS) $(BUILDDIR)/singlehtml - @echo - @echo "Build finished. The HTML page is in $(BUILDDIR)/singlehtml." - -pickle: - $(SPHINXBUILD) -b pickle $(ALLSPHINXOPTS) $(BUILDDIR)/pickle - @echo - @echo "Build finished; now you can process the pickle files." - -json: - $(SPHINXBUILD) -b json $(ALLSPHINXOPTS) $(BUILDDIR)/json - @echo - @echo "Build finished; now you can process the JSON files." - -htmlhelp: - $(SPHINXBUILD) -b htmlhelp $(ALLSPHINXOPTS) $(BUILDDIR)/htmlhelp - @echo - @echo "Build finished; now you can run HTML Help Workshop with the" \ - ".hhp project file in $(BUILDDIR)/htmlhelp." - -qthelp: - $(SPHINXBUILD) -b qthelp $(ALLSPHINXOPTS) $(BUILDDIR)/qthelp - @echo - @echo "Build finished; now you can run "qcollectiongenerator" with the" \ - ".qhcp project file in $(BUILDDIR)/qthelp, like this:" - @echo "# qcollectiongenerator $(BUILDDIR)/qthelp/mozlint.qhcp" - @echo "To view the help file:" - @echo "# assistant -collectionFile $(BUILDDIR)/qthelp/mozlint.qhc" - -applehelp: - $(SPHINXBUILD) -b applehelp $(ALLSPHINXOPTS) $(BUILDDIR)/applehelp - @echo - @echo "Build finished. The help book is in $(BUILDDIR)/applehelp." - @echo "N.B. You won't be able to view it unless you put it in" \ - "~/Library/Documentation/Help or install it in your application" \ - "bundle." - -devhelp: - $(SPHINXBUILD) -b devhelp $(ALLSPHINXOPTS) $(BUILDDIR)/devhelp - @echo - @echo "Build finished." - @echo "To view the help file:" - @echo "# mkdir -p $$HOME/.local/share/devhelp/mozlint" - @echo "# ln -s $(BUILDDIR)/devhelp $$HOME/.local/share/devhelp/mozlint" - @echo "# devhelp" - -epub: - $(SPHINXBUILD) -b epub $(ALLSPHINXOPTS) $(BUILDDIR)/epub - @echo - @echo "Build finished. The epub file is in $(BUILDDIR)/epub." - -latex: - $(SPHINXBUILD) -b latex $(ALLSPHINXOPTS) $(BUILDDIR)/latex - @echo - @echo "Build finished; the LaTeX files are in $(BUILDDIR)/latex." - @echo "Run \`make' in that directory to run these through (pdf)latex" \ - "(use \`make latexpdf' here to do that automatically)." - -latexpdf: - $(SPHINXBUILD) -b latex $(ALLSPHINXOPTS) $(BUILDDIR)/latex - @echo "Running LaTeX files through pdflatex..." - $(MAKE) -C $(BUILDDIR)/latex all-pdf - @echo "pdflatex finished; the PDF files are in $(BUILDDIR)/latex." - -latexpdfja: - $(SPHINXBUILD) -b latex $(ALLSPHINXOPTS) $(BUILDDIR)/latex - @echo "Running LaTeX files through platex and dvipdfmx..." - $(MAKE) -C $(BUILDDIR)/latex all-pdf-ja - @echo "pdflatex finished; the PDF files are in $(BUILDDIR)/latex." - -text: - $(SPHINXBUILD) -b text $(ALLSPHINXOPTS) $(BUILDDIR)/text - @echo - @echo "Build finished. The text files are in $(BUILDDIR)/text." - -man: - $(SPHINXBUILD) -b man $(ALLSPHINXOPTS) $(BUILDDIR)/man - @echo - @echo "Build finished. The manual pages are in $(BUILDDIR)/man." - -texinfo: - $(SPHINXBUILD) -b texinfo $(ALLSPHINXOPTS) $(BUILDDIR)/texinfo - @echo - @echo "Build finished. The Texinfo files are in $(BUILDDIR)/texinfo." - @echo "Run \`make' in that directory to run these through makeinfo" \ - "(use \`make info' here to do that automatically)." - -info: - $(SPHINXBUILD) -b texinfo $(ALLSPHINXOPTS) $(BUILDDIR)/texinfo - @echo "Running Texinfo files through makeinfo..." - make -C $(BUILDDIR)/texinfo info - @echo "makeinfo finished; the Info files are in $(BUILDDIR)/texinfo." - -gettext: - $(SPHINXBUILD) -b gettext $(I18NSPHINXOPTS) $(BUILDDIR)/locale - @echo - @echo "Build finished. The message catalogs are in $(BUILDDIR)/locale." - -changes: - $(SPHINXBUILD) -b changes $(ALLSPHINXOPTS) $(BUILDDIR)/changes - @echo - @echo "The overview file is in $(BUILDDIR)/changes." - -linkcheck: - $(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) $(BUILDDIR)/linkcheck - @echo - @echo "Link check complete; look for any errors in the above output " \ - "or in $(BUILDDIR)/linkcheck/output.txt." - -doctest: - $(SPHINXBUILD) -b doctest $(ALLSPHINXOPTS) $(BUILDDIR)/doctest - @echo "Testing of doctests in the sources finished, look at the " \ - "results in $(BUILDDIR)/doctest/output.txt." - -coverage: - $(SPHINXBUILD) -b coverage $(ALLSPHINXOPTS) $(BUILDDIR)/coverage - @echo "Testing of coverage in the sources finished, look at the " \ - "results in $(BUILDDIR)/coverage/python.txt." - -xml: - $(SPHINXBUILD) -b xml $(ALLSPHINXOPTS) $(BUILDDIR)/xml - @echo - @echo "Build finished. The XML files are in $(BUILDDIR)/xml." - -pseudoxml: - $(SPHINXBUILD) -b pseudoxml $(ALLSPHINXOPTS) $(BUILDDIR)/pseudoxml - @echo - @echo "Build finished. The pseudo-XML files are in $(BUILDDIR)/pseudoxml." diff --git a/tools/lint/docs/coding-style/coding_style_general.rst b/tools/lint/docs/coding-style/coding_style_general.rst index 220e9d9449d5..dfa2269ca11d 100644 --- a/tools/lint/docs/coding-style/coding_style_general.rst +++ b/tools/lint/docs/coding-style/coding_style_general.rst @@ -1,21 +1,3 @@ -================================ -Coding style - General practices -================================ - -- Don't put an ``else`` right after a ``return`` (or a ``break``). - Delete the ``else``, it's unnecessary and increases indentation - level. -- Don't leave debug ``printf``\ s or ``dump``\ s lying around. -- Use `JavaDoc-style - comments `__. -- When fixing a problem, check to see if the problem occurs elsewhere - in the same file, and fix it everywhere if possible. -- End the file with a newline. Make sure your patches don't contain - files with the text "no newline at end of file" in them. -- Declare local variables as near to their use as possible. -- For new files, be sure to use the right `license - boilerplate `__, per our - `license policy `__. Mode line ~~~~~~~~~ diff --git a/tools/lint/docs/conf.py b/tools/lint/docs/conf.py deleted file mode 100644 index 6ed45832f0d8..000000000000 --- a/tools/lint/docs/conf.py +++ /dev/null @@ -1,118 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this file, -# You can obtain one at http://mozilla.org/MPL/2.0/. - -# -*- coding: utf-8 -*- -# -# mozlint documentation build configuration file, created by -# sphinx-quickstart on Fri Nov 27 17:38:49 2015. -# -# This file is execfile()d with the current directory set to its -# containing dir. - -import os - -# -- General configuration ------------------------------------------------ - -# Add any Sphinx extension module names here, as strings. They can be -# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom -# ones. -extensions = [ - 'sphinx.ext.autodoc', - 'sphinx.ext.intersphinx', -] - -# Add any paths that contain templates here, relative to this directory. -templates_path = ['_templates'] - -# The suffix(es) of source filenames. -# You can specify multiple suffix as a list of string: -# source_suffix = ['.rst', '.md'] -source_suffix = '.rst' - -# The master toctree document. -master_doc = 'index' - -# General information about the project. -project = u'mozlint' -copyright = u'2015-Today, Mozilla' -author = u'Andrew Halberstadt and others' - -# The short X.Y version. -version = '0.2.0' -# The full version, including alpha/beta/rc tags. -release = '0.2.0' - -# This is also used if you do content translation via gettext catalogs. -# Usually you set "language" from the command line for these cases. -language = None - -# List of patterns, relative to source directory, that match files and -# directories to ignore when looking for source files. -exclude_patterns = ['_build'] - -# The name of the Pygments (syntax highlighting) style to use. -pygments_style = 'sphinx' - -# If true, `todo` and `todoList` produce output, else they produce nothing. -todo_include_todos = False - - -# -- Options for HTML output ---------------------------------------------- - -# The theme to use for HTML and HTML Help pages. See the documentation for -# a list of builtin themes. -html_theme = 'default' - -if os.environ.get('READTHEDOCS', None) != 'True': - try: - import sphinx_rtd_theme - html_theme = 'sphinx_rtd_theme' - html_theme_path = [sphinx_rtd_theme.get_html_theme_path()] - except ImportError: - pass - -# Add any paths that contain custom static files (such as style sheets) here, -# relative to this directory. They are copied after the builtin static files, -# so a file named "default.css" will overwrite the builtin "default.css". -html_static_path = ['_static'] - -# Output file base name for HTML help builder. -htmlhelp_basename = 'mozlintdoc' - -# -- Options for LaTeX output --------------------------------------------- - -latex_elements = {} - -# Grouping the document tree into LaTeX files. List of tuples -# (source start file, target name, title, -# author, documentclass [howto, manual, or own class]). -latex_documents = [ - (master_doc, 'mozlint.tex', u'mozlint Documentation', - u'Andrew Halberstadt', 'manual'), -] - -# -- Options for manual page output --------------------------------------- - -# One entry per manual page. List of tuples -# (source start file, name, description, authors, manual section). -man_pages = [ - (master_doc, 'mozlint', u'mozlint Documentation', - [author], 1) -] - -# -- Options for Texinfo output ------------------------------------------- - -# Grouping the document tree into Texinfo files. List of tuples -# (source start file, target name, title, author, -# dir menu entry, description, category) -texinfo_documents = [ - (master_doc, 'mozlint', u'mozlint Documentation', - author, 'mozlint', 'One line description of project.', - 'Miscellaneous'), -] - -# Example configuration for intersphinx: refer to the Python standard library. -intersphinx_mapping = {'https://docs.python.org/': None} - -html_show_copyright = False diff --git a/tools/lint/docs/make.bat b/tools/lint/docs/make.bat deleted file mode 100644 index be5489633baf..000000000000 --- a/tools/lint/docs/make.bat +++ /dev/null @@ -1,263 +0,0 @@ -@ECHO OFF - -REM Command file for Sphinx documentation - -if "%SPHINXBUILD%" == "" ( - set SPHINXBUILD=sphinx-build -) -set BUILDDIR=_build -set ALLSPHINXOPTS=-d %BUILDDIR%/doctrees %SPHINXOPTS% . -set I18NSPHINXOPTS=%SPHINXOPTS% . -if NOT "%PAPER%" == "" ( - set ALLSPHINXOPTS=-D latex_paper_size=%PAPER% %ALLSPHINXOPTS% - set I18NSPHINXOPTS=-D latex_paper_size=%PAPER% %I18NSPHINXOPTS% -) - -if "%1" == "" goto help - -if "%1" == "help" ( - :help - echo.Please use `make ^` where ^ is one of - echo. html to make standalone HTML files - echo. dirhtml to make HTML files named index.html in directories - echo. singlehtml to make a single large HTML file - echo. pickle to make pickle files - echo. json to make JSON files - echo. htmlhelp to make HTML files and a HTML help project - echo. qthelp to make HTML files and a qthelp project - echo. devhelp to make HTML files and a Devhelp project - echo. epub to make an epub - echo. latex to make LaTeX files, you can set PAPER=a4 or PAPER=letter - echo. text to make text files - echo. man to make manual pages - echo. texinfo to make Texinfo files - echo. gettext to make PO message catalogs - echo. changes to make an overview over all changed/added/deprecated items - echo. xml to make Docutils-native XML files - echo. pseudoxml to make pseudoxml-XML files for display purposes - echo. linkcheck to check all external links for integrity - echo. doctest to run all doctests embedded in the documentation if enabled - echo. coverage to run coverage check of the documentation if enabled - goto end -) - -if "%1" == "clean" ( - for /d %%i in (%BUILDDIR%\*) do rmdir /q /s %%i - del /q /s %BUILDDIR%\* - goto end -) - - -REM Check if sphinx-build is available and fallback to Python version if any -%SPHINXBUILD% 2> nul -if errorlevel 9009 goto sphinx_python -goto sphinx_ok - -:sphinx_python - -set SPHINXBUILD=python -m sphinx.__init__ -%SPHINXBUILD% 2> nul -if errorlevel 9009 ( - echo. - echo.The 'sphinx-build' command was not found. Make sure you have Sphinx - echo.installed, then set the SPHINXBUILD environment variable to point - echo.to the full path of the 'sphinx-build' executable. Alternatively you - echo.may add the Sphinx directory to PATH. - echo. - echo.If you don't have Sphinx installed, grab it from - echo.http://sphinx-doc.org/ - exit /b 1 -) - -:sphinx_ok - - -if "%1" == "html" ( - %SPHINXBUILD% -b html %ALLSPHINXOPTS% %BUILDDIR%/html - if errorlevel 1 exit /b 1 - echo. - echo.Build finished. The HTML pages are in %BUILDDIR%/html. - goto end -) - -if "%1" == "dirhtml" ( - %SPHINXBUILD% -b dirhtml %ALLSPHINXOPTS% %BUILDDIR%/dirhtml - if errorlevel 1 exit /b 1 - echo. - echo.Build finished. The HTML pages are in %BUILDDIR%/dirhtml. - goto end -) - -if "%1" == "singlehtml" ( - %SPHINXBUILD% -b singlehtml %ALLSPHINXOPTS% %BUILDDIR%/singlehtml - if errorlevel 1 exit /b 1 - echo. - echo.Build finished. The HTML pages are in %BUILDDIR%/singlehtml. - goto end -) - -if "%1" == "pickle" ( - %SPHINXBUILD% -b pickle %ALLSPHINXOPTS% %BUILDDIR%/pickle - if errorlevel 1 exit /b 1 - echo. - echo.Build finished; now you can process the pickle files. - goto end -) - -if "%1" == "json" ( - %SPHINXBUILD% -b json %ALLSPHINXOPTS% %BUILDDIR%/json - if errorlevel 1 exit /b 1 - echo. - echo.Build finished; now you can process the JSON files. - goto end -) - -if "%1" == "htmlhelp" ( - %SPHINXBUILD% -b htmlhelp %ALLSPHINXOPTS% %BUILDDIR%/htmlhelp - if errorlevel 1 exit /b 1 - echo. - echo.Build finished; now you can run HTML Help Workshop with the ^ -.hhp project file in %BUILDDIR%/htmlhelp. - goto end -) - -if "%1" == "qthelp" ( - %SPHINXBUILD% -b qthelp %ALLSPHINXOPTS% %BUILDDIR%/qthelp - if errorlevel 1 exit /b 1 - echo. - echo.Build finished; now you can run "qcollectiongenerator" with the ^ -.qhcp project file in %BUILDDIR%/qthelp, like this: - echo.^> qcollectiongenerator %BUILDDIR%\qthelp\mozlint.qhcp - echo.To view the help file: - echo.^> assistant -collectionFile %BUILDDIR%\qthelp\mozlint.ghc - goto end -) - -if "%1" == "devhelp" ( - %SPHINXBUILD% -b devhelp %ALLSPHINXOPTS% %BUILDDIR%/devhelp - if errorlevel 1 exit /b 1 - echo. - echo.Build finished. - goto end -) - -if "%1" == "epub" ( - %SPHINXBUILD% -b epub %ALLSPHINXOPTS% %BUILDDIR%/epub - if errorlevel 1 exit /b 1 - echo. - echo.Build finished. The epub file is in %BUILDDIR%/epub. - goto end -) - -if "%1" == "latex" ( - %SPHINXBUILD% -b latex %ALLSPHINXOPTS% %BUILDDIR%/latex - if errorlevel 1 exit /b 1 - echo. - echo.Build finished; the LaTeX files are in %BUILDDIR%/latex. - goto end -) - -if "%1" == "latexpdf" ( - %SPHINXBUILD% -b latex %ALLSPHINXOPTS% %BUILDDIR%/latex - cd %BUILDDIR%/latex - make all-pdf - cd %~dp0 - echo. - echo.Build finished; the PDF files are in %BUILDDIR%/latex. - goto end -) - -if "%1" == "latexpdfja" ( - %SPHINXBUILD% -b latex %ALLSPHINXOPTS% %BUILDDIR%/latex - cd %BUILDDIR%/latex - make all-pdf-ja - cd %~dp0 - echo. - echo.Build finished; the PDF files are in %BUILDDIR%/latex. - goto end -) - -if "%1" == "text" ( - %SPHINXBUILD% -b text %ALLSPHINXOPTS% %BUILDDIR%/text - if errorlevel 1 exit /b 1 - echo. - echo.Build finished. The text files are in %BUILDDIR%/text. - goto end -) - -if "%1" == "man" ( - %SPHINXBUILD% -b man %ALLSPHINXOPTS% %BUILDDIR%/man - if errorlevel 1 exit /b 1 - echo. - echo.Build finished. The manual pages are in %BUILDDIR%/man. - goto end -) - -if "%1" == "texinfo" ( - %SPHINXBUILD% -b texinfo %ALLSPHINXOPTS% %BUILDDIR%/texinfo - if errorlevel 1 exit /b 1 - echo. - echo.Build finished. The Texinfo files are in %BUILDDIR%/texinfo. - goto end -) - -if "%1" == "gettext" ( - %SPHINXBUILD% -b gettext %I18NSPHINXOPTS% %BUILDDIR%/locale - if errorlevel 1 exit /b 1 - echo. - echo.Build finished. The message catalogs are in %BUILDDIR%/locale. - goto end -) - -if "%1" == "changes" ( - %SPHINXBUILD% -b changes %ALLSPHINXOPTS% %BUILDDIR%/changes - if errorlevel 1 exit /b 1 - echo. - echo.The overview file is in %BUILDDIR%/changes. - goto end -) - -if "%1" == "linkcheck" ( - %SPHINXBUILD% -b linkcheck %ALLSPHINXOPTS% %BUILDDIR%/linkcheck - if errorlevel 1 exit /b 1 - echo. - echo.Link check complete; look for any errors in the above output ^ -or in %BUILDDIR%/linkcheck/output.txt. - goto end -) - -if "%1" == "doctest" ( - %SPHINXBUILD% -b doctest %ALLSPHINXOPTS% %BUILDDIR%/doctest - if errorlevel 1 exit /b 1 - echo. - echo.Testing of doctests in the sources finished, look at the ^ -results in %BUILDDIR%/doctest/output.txt. - goto end -) - -if "%1" == "coverage" ( - %SPHINXBUILD% -b coverage %ALLSPHINXOPTS% %BUILDDIR%/coverage - if errorlevel 1 exit /b 1 - echo. - echo.Testing of coverage in the sources finished, look at the ^ -results in %BUILDDIR%/coverage/python.txt. - goto end -) - -if "%1" == "xml" ( - %SPHINXBUILD% -b xml %ALLSPHINXOPTS% %BUILDDIR%/xml - if errorlevel 1 exit /b 1 - echo. - echo.Build finished. The XML files are in %BUILDDIR%/xml. - goto end -) - -if "%1" == "pseudoxml" ( - %SPHINXBUILD% -b pseudoxml %ALLSPHINXOPTS% %BUILDDIR%/pseudoxml - if errorlevel 1 exit /b 1 - echo. - echo.Build finished. The pseudo-XML files are in %BUILDDIR%/pseudoxml. - goto end -) - -:end diff --git a/tools/moz.build b/tools/moz.build index df24e66783dc..752fd8cb2a2d 100644 --- a/tools/moz.build +++ b/tools/moz.build @@ -53,8 +53,6 @@ with Files("update-packaging/**"): with Files("update-verify/**"): BUG_COMPONENT = ("Release Engineering", "Release Automation: Updates") -SPHINX_TREES['lint'] = 'lint/docs' - SPHINX_TREES['moztreedocs'] = 'moztreedocs/docs' with Files('lint/docs/**'): @@ -62,8 +60,6 @@ with Files('lint/docs/**'): SPHINX_TREES['try'] = 'tryselect/docs' -SPHINX_TREES['static-analysis'] = 'clang-tidy/docs' - SPHINX_TREES['fuzzing'] = 'fuzzing/docs' with Files('tryselect/docs/**'):