Bug 1613526 - Create a code quality documentation and move the approp…

…riate 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
iaxat · Feb 11, 2020 · 03a0492 · 03a0492
1 parent f8b6e74
commit 03a0492
Show file tree

Hide file tree

Showing 37 changed files with 1,038 additions and 619 deletions.
diff --git a/docs/code-quality/coding-style/coding_style_cpp.rst b/docs/code-quality/coding-style/coding_style_cpp.rst
diff --git a/docs/code-quality/coding-style/coding_style_general.rst 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/...t/docs/coding-style/coding_style_java.rst → ...uality/coding-style/coding_style_java.rst b/...t/docs/coding-style/coding_style_java.rst → ...uality/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
@@ -0,0 +1,150 @@
+=======================
+JavaScript Coding style
+=======================
+
+Coding style
+~~~~~~~~~~~~
+
+`prettier <https://prettier.io/>`_ 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 <https://developer.mozilla.org/docs/Mozilla/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/.../docs/coding-style/coding_style_other.rst → ...ality/coding-style/coding_style_other.rst b/.../docs/coding-style/coding_style_other.rst → ...ality/coding-style/coding_style_other.rst
diff --git a/...docs/coding-style/coding_style_python.rst → ...lity/coding-style/coding_style_python.rst b/...docs/coding-style/coding_style_python.rst → ...lity/coding-style/coding_style_python.rst
diff --git a/...yle/format_cpp_code_with_clang-format.rst → ...yle/format_cpp_code_with_clang-format.rst b/...yle/format_cpp_code_with_clang-format.rst → ...yle/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
@@ -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 → docs/code-quality/index.rst b/tools/clang-tidy/docs/summary.rst → 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 → docs/code-quality/lint/create.rst b/tools/lint/docs/create.rst → 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 → docs/code-quality/lint/index.rst b/tools/lint/docs/index.rst → 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
@@ -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 → docs/code-quality/lint/linters/codespell.rst b/tools/lint/docs/linters/codespell.rst → docs/code-quality/lint/linters/codespell.rst
diff --git a/...s/lint/docs/linters/cpp-virtual-final.rst → ...uality/lint/linters/cpp-virtual-final.rst b/...s/lint/docs/linters/cpp-virtual-final.rst → ...uality/lint/linters/cpp-virtual-final.rst
diff --git a/...nt/docs/linters/eslint-plugin-mozilla.rst → ...ty/lint/linters/eslint-plugin-mozilla.rst b/...nt/docs/linters/eslint-plugin-mozilla.rst → ...ty/lint/linters/eslint-plugin-mozilla.rst
diff --git a/...linters/eslint-plugin-spidermonkey-js.rst → ...linters/eslint-plugin-spidermonkey-js.rst b/...linters/eslint-plugin-spidermonkey-js.rst → ...linters/eslint-plugin-spidermonkey-js.rst
diff --git a/tools/lint/docs/linters/eslint.rst → docs/code-quality/lint/linters/eslint.rst b/tools/lint/docs/linters/eslint.rst → docs/code-quality/lint/linters/eslint.rst
diff --git a/tools/lint/docs/linters/file-perm.rst → docs/code-quality/lint/linters/file-perm.rst b/tools/lint/docs/linters/file-perm.rst → docs/code-quality/lint/linters/file-perm.rst
diff --git a/tools/lint/docs/linters/file-whitespace.rst → ...-quality/lint/linters/file-whitespace.rst b/tools/lint/docs/linters/file-whitespace.rst → ...-quality/lint/linters/file-whitespace.rst
diff --git a/tools/lint/docs/linters/flake8.rst → docs/code-quality/lint/linters/flake8.rst b/tools/lint/docs/linters/flake8.rst → docs/code-quality/lint/linters/flake8.rst
diff --git a/tools/lint/docs/linters/l10n.rst → docs/code-quality/lint/linters/l10n.rst b/tools/lint/docs/linters/l10n.rst → docs/code-quality/lint/linters/l10n.rst
diff --git a/tools/lint/docs/linters/license.rst → docs/code-quality/lint/linters/license.rst b/tools/lint/docs/linters/license.rst → docs/code-quality/lint/linters/license.rst
diff --git a/tools/lint/docs/linters/lintpref.rst → docs/code-quality/lint/linters/lintpref.rst b/tools/lint/docs/linters/lintpref.rst → docs/code-quality/lint/linters/lintpref.rst
diff --git a/...int/docs/linters/mingw-capitalization.rst → ...ity/lint/linters/mingw-capitalization.rst b/...int/docs/linters/mingw-capitalization.rst → ...ity/lint/linters/mingw-capitalization.rst
diff --git a/tools/lint/docs/linters/perfdocs.rst → docs/code-quality/lint/linters/perfdocs.rst b/tools/lint/docs/linters/perfdocs.rst → docs/code-quality/lint/linters/perfdocs.rst
diff --git a/tools/lint/docs/linters/rstlinter.rst → docs/code-quality/lint/linters/rstlinter.rst b/tools/lint/docs/linters/rstlinter.rst → docs/code-quality/lint/linters/rstlinter.rst
diff --git a/tools/lint/docs/linters/rustfmt.rst → docs/code-quality/lint/linters/rustfmt.rst b/tools/lint/docs/linters/rustfmt.rst → docs/code-quality/lint/linters/rustfmt.rst
diff --git a/tools/lint/docs/usage.rst → docs/code-quality/lint/usage.rst b/tools/lint/docs/usage.rst → docs/code-quality/lint/usage.rst
diff --git a/tools/clang-tidy/docs/index.rst → docs/code-quality/static-analysis.rst b/tools/clang-tidy/docs/index.rst → docs/code-quality/static-analysis.rst
@@ -10,7 +10,7 @@ For linting, please see the `linting documentation </tools/lint/>`_.
 Clang-Tidy static analysis
 --------------------------
 
-Our current static-analysis infrastruture is based on
+Our current static-analysis infrastructure is based on
 `clang-tidy <http://clang.llvm.org/extra/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
@@ -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
@@ -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
@@ -185,4 +185,6 @@ DEFINES['top_srcdir'] = TOPSRCDIR
 
 SPHINX_TREES['contributing'] = 'docs/contributing'
 
+SPHINX_TREES['code-quality'] = 'docs/code-quality'
+
 include('build/templates.mozbuild')
Original file line number	Diff line number	Diff line change
Expand Up		@@ -185,4 +185,6 @@ DEFINES['top_srcdir'] = TOPSRCDIR

		SPHINX_TREES['contributing'] = 'docs/contributing'

		SPHINX_TREES['code-quality'] = 'docs/code-quality'

		include('build/templates.mozbuild')