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