remote/doc/marionette/CodeStyle.md

   1 # Style guide
   2
   3 Like other projects, we also have some guidelines to keep to the code.
   4 For the overall Marionette project, a few rough rules are:
   5
   6 * Make your code readable and sensible, and don’t try to be
   7   clever.  Prefer simple and easy solutions over more convoluted
   8   and foreign syntax.
   9
  10 * Fixing style violations whilst working on a real change as a
  11   preparatory clean-up step is good, but otherwise avoid useless
  12   code churn for the sake of conforming to the style guide.
  13
  14 * Code is mutable and not written in stone.  Nothing that
  15   is checked in is sacred and we encourage change to make
  16   remote/marionette a pleasant ecosystem to work in.
  17
  18 ## JavaScript
  19
  20 Marionette is written in JavaScript and ships
  21 as part of Firefox.  We have access to all the latest ECMAScript
  22 features currently in development, usually before it ships in the
  23 wild and we try to make use of new features when appropriate,
  24 especially when they move us off legacy internal replacements
  25 (such as Promise.jsm and Task.jsm).
  26
  27 One of the peculiarities of working on JavaScript code that ships as
  28 part of a runtime platform is, that unlike in a regular web document,
  29 we share a single global state with the rest of Firefox.  This means
  30 we have to be responsible and not leak resources unnecessarily.
  31
  32 JS code in Gecko is organised into _modules_ carrying _.js_ or _.jsm_
  33 file extensions.  Depending on the area of Gecko you’re working on,
  34 you may find they have different techniques for exporting symbols,
  35 varying indentation and code style, as well as varying linting
  36 requirements.
  37
  38 To export symbols to other Marionette modules, remember to assign
  39 your exported symbols to the shared global `this`:
  40
  41     const EXPORTED_SYMBOLS = ["PollPromise", "TimedPromise"];
  42
  43 When importing symbols in Marionette code, try to be specific about
  44 what you need:
  45
  46     const { TimedPromise } = ChromeUtils.import(
  47       "chrome://remote/content/marionette/sync.js"
  48     );
  49
  50 We prefer object assignment shorthands when redefining names,
  51 for example when you use functionality from the `Components` global:
  52
  53     const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
  54
  55 When using symbols by their own name, the assignment name can be
  56 omitted:
  57
  58     const {TYPE_ONE_SHOT, TYPE_REPEATING_SLACK} = Ci.nsITimer;
  59
  60 In addition to the default [Mozilla eslint rules], we have [our
  61 own specialisations] that are stricter and enforce more security.
  62 A few notable examples are that we disallow fallthrough `case`
  63 statements unless they are explicitly grouped together:
  64
  65     switch (x) {
  66       case "foo":
  67         doSomething();
  68
  69       case "bar":  // <-- disallowed!
  70         doSomethingElse();
  71         break;
  72
  73       case "baz":
  74       case "bah":  // <-- allowed (-:
  75         doCrazyThings();
  76     }
  77
  78 We disallow the use of `var`, for which we always prefer `let` and
  79 `const` as replacements.  Do be aware that `const` does not mean
  80 that the variable is immutable: just that it cannot be reassigned.
  81 We require all lines to end with semicolons, disallow construction
  82 of plain `new Object()`, require variable names to be camel-cased,
  83 and complain about unused variables.
  84
  85 For purely aesthetic reasons we indent our code with two spaces,
  86 which includes switch-statement `case`s, and limit the maximum
  87 line length to 78 columns.  When you need to wrap a statement to
  88 the next line, the second line is indented with four spaces, like this:
  89
  90     throw new TypeError(
  91         "Expected an element or WindowProxy, " +
  92         pprint`got: ${el}`);
  93
  94 This is not normally something you have to think to deeply about as
  95 it is enforced by the [linter].  The linter also has an automatic
  96 mode that fixes and formats certain classes of style violations.
  97
  98 If you find yourself struggling to fit a long statement on one line,
  99 this is usually an indication that it is too long and should be
 100 split into multiple lines.  This is also a helpful tip to make the
 101 code easier to read.  Assigning transitive values to descriptive
 102 variable names can serve as self-documentation:
 103
 104     let location = event.target.documentURI || event.target.location.href;
 105     log.debug(`Received DOM event ${event.type} for ${location}`);
 106
 107 On the topic of variable naming the opinions are as many as programmers
 108 writing code, but it is often helpful to keep the input and output
 109 arguments to functions descriptive (longer), and let transitive
 110 internal values to be described more succinctly:
 111
 112     /** Prettifies instance of Error and its stacktrace to a string. */
 113     function stringify(error) {
 114       try {
 115         let s = error.toString();
 116         if ("stack" in error) {
 117           s += "\n" + error.stack;
 118         }
 119         return s;
 120       } catch (e) {
 121         return "<unprintable error>";
 122       }
 123     }
 124
 125 When we can, we try to extract the relevant object properties in
 126 the arguments to an event handler or a function:
 127
 128     const responseListener = ({name, target, json, data}) => { … };
 129
 130 Instead of:
 131
 132     const responseListener = msg => {
 133       let name = msg.name;
 134       let target = msg.target;
 135       let json = msg.json;
 136       let data = msg.data;
 137       …
 138     };
 139
 140 All source files should have `"use strict";` as the first directive
 141 so that the file is parsed in [strict mode].
 142
 143 Every source code file that ships as part of the Firefox bundle
 144 must also have a [copying header], such as this:
 145
 146     /* This Source Code Form is subject to the terms of the Mozilla Public
 147      * License, v. 2.0. If a copy of the MPL was not distributed with this file,
 148      * You can obtain one at http://mozilla.org/MPL/2.0/. */
 149
 150 New xpcshell test files _should not_ have a license header as all
 151 new Mozilla tests should be in the [public domain] so that they can
 152 easily be shared with other browser vendors.  We want to re-license
 153 existing tests covered by the [MPL] so that they can be shared.
 154 We very much welcome your help in doing version control archeology
 155 to make this happen!
 156
 157 The practical details of working on the Marionette code is outlined
 158 in [Contributing.md], but generally you do not have to re-build
 159 Firefox when changing code.  Any change to remote/marionette/*.js
 160 will be picked up on restarting Firefox.  The only notable exception
 161 is remote/components/Marionette.jsm, which does require
 162 a re-build.
 163
 164 [strict mode]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Strict_mode
 165 [Mozilla eslint rules]: https://searchfox.org/mozilla-central/source/.eslintrc.js
 166 [our own specialisations]: https://searchfox.org/mozilla-central/source/remote/marionette/.eslintrc.js
 167 [linter]: #linting
 168 [copying header]: https://www.mozilla.org/en-US/MPL/headers/
 169 [public domain]: https://creativecommons.org/publicdomain/zero/1.0/
 170 [MPL]: https://www.mozilla.org/en-US/MPL/2.0/
 171 [Contributing.md]: ./Contributing.md
 172
 173 ## Python
 174
 175 TODO
 176
 177 ## Documentation
 178
 179 We keep our documentation in-tree under [remote/doc/marionette]
 180 and [testing/geckodriver/doc].  Updates and minor changes to
 181 documentation should ideally not be scrutinised to the same degree
 182 as code changes to encourage frequent updates so that the documentation
 183 does not go stale.  To that end, documentation changes with `r=me`
 184 from module peers are permitted.
 185
 186 Use fmt(1) or an equivalent editor specific mechanism (such as Meta-Q
 187 in Emacs) to format paragraphs at a maximum width of 75 columns
 188 with a goal of roughly 65.  This is equivalent to `fmt -w 75 -g 65`,
 189 which happens to be the default on BSD and macOS.
 190
 191 We endeavour to document all _public APIs_ of the Marionette component.
 192 These include public functions—or command implementations—on
 193 the `GeckoDriver` class, as well as all exported symbols from
 194 other modules.  Documentation for non-exported symbols is not required.
 195
 196 [remote/doc/marionette]: https://searchfox.org/mozilla-central/source/remote/marionette/doc
 197 [testing/geckodriver/doc]: https://searchfox.org/mozilla-central/source/testing/geckodriver/doc
 198
 199 ## Linting
 200
 201 Marionette consists mostly of JavaScript (server) and Python (client,
 202 harness, test runner) code.  We lint our code with [mozlint],
 203 which harmonises the output from [eslint] and [ruff].
 204
 205 To run the linter with a sensible output:
 206
 207     % ./mach lint -funix remote/marionette
 208
 209 For certain classes of style violations the eslint linter has
 210 an automatic mode for fixing and formatting your code.  This is
 211 particularly useful to keep to whitespace and indentation rules:
 212
 213     % ./mach eslint --fix remote/marionette
 214
 215 The linter is also run as a try job (shorthand `ES`) which means
 216 any style violations will automatically block a patch from landing
 217 (if using Autoland) or cause your changeset to be backed out (if
 218 landing directly on mozilla-inbound).
 219
 220 If you use git(1) you can [enable automatic linting] before you push
 221 to a remote through a pre-push (or pre-commit) hook.  This will
 222 run the linters on the changed files before a push and abort if
 223 there are any problems.  This is convenient for avoiding a try run
 224 failing due to a stupid linting issue.
 225
 226 [mozlint]: /code-quality/lint/mozlint.rst
 227 [eslint]: /code-quality/lint/linters/eslint.rst
 228 [ruff]: /code-quality/lint/linters/ruff.rst
 229 [enable automatic linting]: /code-quality/lint/usage.rst#using-a-vcs-hook