chore: revise ESLint built-in configuration (balena-io#1149)

There are a lot of new rules since the last time I revised the ESLint
rules documentation.

I've updated the main `.eslintrc.yml` to include some newer additions,
plus I added another ESLint configuration file inside `tests`, so we can
add some stricted rules to the production code while relaxing them for
the test suite (due to the fact that Mocha is not very ES6 friendly and
Angular tests require a bit of dark magic to setup).

This is a summary of the most important changes:

- Disallow "magic numbers"

These should now be extracted to constants, which forces us to think of
a good name for them, and thus make the code more self-documenting (I
had to Google up the meaning of some existing magic numbers, so I guess
this will be great for readability purposes).

- Require consistent `return` statements

Some functions relied on JavaScript relaxed casting mechanism to work,
which now have explicit return values. This flag also helped me detect
some promises that were not being returned, and therefore risked not
being caught by the exception handlers in case of errors.

- Disallow redefining function arguments

Immutability makes functions easier to reason about.

- Enforce JavaScript string templates instead of string concatenation

We were heavily mixing boths across the codebase.

There are some extra rules that I tweaked, however most of codebase
changes in this commit are related to the rules mentioned above.

Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

.eslintrc.yml

@@ -9,9 +9,6 @@ rules:
 
   # Possible Errors
 
-  comma-dangle:
-    - error
-    - never
   no-cond-assign:
     - error
   no-console:
@@ -59,6 +56,8 @@ rules:
     - error
   no-sparse-arrays:
     - error
+  no-template-curly-in-string:
+    - error
   no-unexpected-multiline:
     - error
   no-unreachable:
@@ -93,8 +92,12 @@ rules:
     - error
   block-scoped-var:
     - error
+  class-methods-use-this:
+    - error
   complexity:
     - off
+  consistent-return:
+    - error
   curly:
     - error
   default-case:
@@ -136,6 +139,8 @@ rules:
     - error
   no-floating-decimal:
     - error
+  no-global-assign:
+    - error
   no-implicit-coercion:
     - error
   no-implicit-globals:
@@ -150,6 +155,8 @@ rules:
     - error
   no-loop-func:
     - error
+  no-magic-numbers:
+    - error
   no-multi-spaces:
     - error
   no-multi-str:
@@ -166,12 +173,19 @@ rules:
     - error
   no-octal-escape:
     - error
+  no-param-reassign:
+    - error
   no-proto:
     - error
   no-redeclare:
     - error
+  no-restricted-properties:
+    - error
+    - property: __proto__
   no-return-assign:
     - error
+  no-return-await:
+    - error
   no-script-url:
     - error
   no-self-assign:
@@ -184,6 +198,8 @@ rules:
     - error
   no-unmodified-loop-condition:
     - error
+  no-unused-expressions:
+    - error
   no-unused-labels:
     - error
   no-useless-call:
@@ -216,12 +232,18 @@ rules:
 
   # Variables
 
+  init-declarations:
+    - error
+    - always
   no-catch-shadow:
     - error
   no-delete-var:
     - error
   no-label-var:
     - error
+  no-restricted-globals:
+    - error
+    - event
   no-shadow:
     - error
   no-shadow-restricted-names:
@@ -230,6 +252,8 @@ rules:
     - error
   no-undef-init:
     - error
+  no-undefined:
+    - error
   no-unused-vars:
     - error
   no-use-before-define:
@@ -268,6 +292,13 @@ rules:
     - 1tbs
   camelcase:
     - error
+  capitalized-comments:
+    - error
+    - always
+    - ignoreConsecutiveComments: true
+  comma-dangle:
+    - error
+    - never
   comma-spacing:
     - error
     - before: false
@@ -283,6 +314,12 @@ rules:
     - self
   eol-last:
     - error
+  func-call-spacing:
+    - error
+    - never
+  func-name-matching:
+    - error
+    - always
   func-names:
     - error
     - never
@@ -291,6 +328,11 @@ rules:
     - expression
   id-blacklist:
     - error
+  id-length:
+    - error
+    - min: 2
+      exceptions:
+        - "_"
   indent:
     - error
     - 2
@@ -304,6 +346,12 @@ rules:
     - error
     - before: true
       after: true
+  line-comment-position:
+    - error
+    - position: above
+  linebreak-style:
+    - error
+    - unix
   lines-around-comment:
     - error
     - beforeBlockComment: true
@@ -316,20 +364,30 @@ rules:
       allowObjectEnd: false
       allowArrayStart: true
       allowArrayEnd: false
+  lines-around-directive:
+    - error
+    - always
   max-len:
     - error
     - code: 130
       comments: 150
       ignoreComments: false
       ignoreTrailingComments: false
       ignoreUrls: true
+  max-params:
+    - off
   max-statements-per-line:
     - error
     - max: 1
+  multiline-ternary:
+    - error
+    - never
   new-cap:
     - error
   new-parens:
     - error
+  newline-per-chained-call:
+    - off
   no-array-constructor:
     - error
   no-bitwise:
@@ -338,10 +396,14 @@ rules:
     - error
   no-inline-comments:
     - error
+  no-lonely-if:
+    - error
   no-mixed-operators:
     - error
   no-mixed-spaces-and-tabs:
     - error
+  no-multi-assign:
+    - error
   no-multiple-empty-lines:
     - error
     - max: 1
@@ -355,8 +417,14 @@ rules:
     - error
   no-plusplus:
     - error
+  no-restricted-syntax:
+    - error
+    - WithStatement
+    - ForInStatement
   no-spaced-func:
     - error
+  no-tabs:
+    - error
   no-trailing-spaces:
     - error
   no-underscore-dangle:
@@ -374,6 +442,9 @@ rules:
     - always
   object-property-newline:
     - error
+  one-var-declaration-per-line:
+    - error
+    - always
   one-var:
     - error
     - never
@@ -383,6 +454,9 @@ rules:
   operator-linebreak:
     - error
     - before
+  padded-blocks:
+    - error
+    - classes: always
   quote-props:
     - error
     - as-needed
@@ -395,6 +469,7 @@ rules:
         FunctionDeclaration: true
         ClassDeclaration: true
         MethodDefinition: true
+        ArrowFunctionExpression: true
   semi:
     - error
     - always
@@ -412,9 +487,16 @@ rules:
     - never
   space-infix-ops:
     - error
+  space-unary-ops:
+    - error
+    - words: true
+      nonwords: false
   spaced-comment:
     - error
     - always
+  template-tag-spacing:
+    - error
+    - always
   unicode-bom:
     - error
 
@@ -458,19 +540,33 @@ rules:
     - error
   no-var:
     - error
+  object-shorthand:
+    - error
+    - always
   prefer-const:
     - error
   prefer-reflect:
     - error
   prefer-spread:
     - error
+  prefer-numeric-literals:
+    - error
+  prefer-rest-params:
+    - error
+  prefer-template:
+    - error
+  prefer-arrow-callback:
+    - error
+    - allowNamedFunctions: false
   require-yield:
     - error
   rest-spread-spacing:
     - error
   template-curly-spacing:
     - error
     - never
+  symbol-description:
+    - error
   yield-star-spacing:
     - error
     - before: true

lib/child-writer/cli.js

@@ -47,7 +47,8 @@ exports.getBooleanArgumentForm = (argumentName, value) => {
       return '--no-';
     }
 
-    if (_.size(argumentName) === 1) {
+    const SHORT_OPTION_LENGTH = 1;
+    if (_.size(argumentName) === SHORT_OPTION_LENGTH) {
       return '-';
     }
 

lib/child-writer/index.js

@@ -61,7 +61,7 @@ exports.write = (image, drive, options) => {
 
   const argv = cli.getArguments({
     entryPoint: rendererUtils.getApplicationEntryPoint(),
-    image: image,
+    image,
     device: drive.device,
     validateWriteOnSuccess: options.validateWriteOnSuccess,
     unmountOnSuccess: options.unmountOnSuccess
@@ -77,6 +77,14 @@ exports.write = (image, drive, options) => {
   ipc.config.silent = true;
   ipc.serve();
 
+  /**
+   * @summary Safely terminate the IPC server
+   * @function
+   * @private
+   *
+   * @example
+   * terminateServer();
+   */
   const terminateServer = () => {
 
     // Turns out we need to destroy all sockets for
@@ -90,14 +98,24 @@ exports.write = (image, drive, options) => {
     ipc.server.stop();
   };
 
+  /**
+   * @summary Emit an error to the client
+   * @function
+   * @private
+   *
+   * @param {Error} error - error
+   *
+   * @example
+   * emitError(new Error('foo bar'));
+   */
   const emitError = (error) => {
     terminateServer();
     emitter.emit('error', error);
   };
 
   ipc.server.on('error', emitError);
   ipc.server.on('message', (data) => {
-    let message;
+    let message = null;
 
     try {
       message = robot.parseMessage(data);
@@ -112,7 +130,7 @@ exports.write = (image, drive, options) => {
       return emitError(robot.recomposeErrorMessage(message));
     }
 
-    emitter.emit(robot.getCommand(message), robot.getData(message));
+    return emitter.emit(robot.getCommand(message), robot.getData(message));
   });
 
   ipc.server.on('start', () => {
@@ -144,9 +162,13 @@ exports.write = (image, drive, options) => {
         });
       }
 
-      if (code !== EXIT_CODES.SUCCESS && code !== EXIT_CODES.VALIDATION_ERROR) {
-        return emitError(new Error(`Child process exited with error code: ${code}`));
+      // We shouldn't emit the `done` event manually here
+      // since the writer process will take care of it.
+      if (code === EXIT_CODES.SUCCESS || code === EXIT_CODES.VALIDATION_ERROR) {
+        return null;
       }
+
+      return emitError(new Error(`Child process exited with error code: ${code}`));
     });
   });
 

lib/child-writer/renderer-utils.js

@@ -40,9 +40,12 @@ exports.getApplicationEntryPoint = () => {
     return path.join(process.resourcesPath, 'app.asar');
   }
 
+  const ENTRY_POINT_ARGV_INDEX = 1;
+  const relativeEntryPoint = electron.remote.process.argv[ENTRY_POINT_ARGV_INDEX];
+
   // On GNU/Linux, `pkexec` resolves relative paths
   // from `/root`, therefore we pass an absolute path,
   // in order to be on the safe side.
-  return path.join(CONSTANTS.PROJECT_ROOT, electron.remote.process.argv[1]);
+  return path.join(CONSTANTS.PROJECT_ROOT, relativeEntryPoint);
 
 };