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