ESlint JSdoc (#2148)

* Install eslint-plugin-jsdoc

* Initial rules setup

* Update lists

* Use @returns rather than @return

* Remove error throwing code which is never executed

* Fix issues relating to @throws

* General error fixes

* Update Display type documentation

* Various doc fixes

* Fix invalid tuple syntax

* Doc updates

* Remove unused

* Doc updates

* Enable jsdoc/require-returns

* Update rules

* Update remaining rules

6 files changed, 58 insertions, 33 deletions

diff --git a/ext/js/app/frontend.js b/ext/js/app/frontend.js
index 634408d9..f3b925c3 100644
--- a/ext/js/app/frontend.js
+++ b/ext/js/app/frontend.js
@@ -27,7 +27,7 @@
 class Frontend {
     /**
      * Creates a new instance.
-     * @param {object} details
+     * @param {object} details Details about how to set up the instance.
      * @param {string} details.pageType The type of page, one of 'web', 'popup', or 'search'.
      * @param {PopupFactory} details.popupFactory A PopupFactory instance to use for generating popups.
      * @param {number} details.depth The nesting depth value of the popup.
diff --git a/ext/js/app/popup-factory.js b/ext/js/app/popup-factory.js
index bef687ff..112fa963 100644
--- a/ext/js/app/popup-factory.js
+++ b/ext/js/app/popup-factory.js
@@ -63,7 +63,7 @@ class PopupFactory {
 
     /**
      * Gets or creates a popup based on a set of parameters
-     * @param {object} details
+     * @param {object} details Details about how to acquire the popup.
      * @param {?number} [details.frameId] The ID of the frame that should host the popup.
      * @param {?string} [details.id] A specific ID used to find an existing popup, or to assign to the new popup.
      * @param {?string} [details.parentPopupId] The ID of the parent popup.
diff --git a/ext/js/app/popup-proxy.js b/ext/js/app/popup-proxy.js
index 2bf81d0a..77d87520 100644
--- a/ext/js/app/popup-proxy.js
+++ b/ext/js/app/popup-proxy.js
@@ -22,7 +22,7 @@
 class PopupProxy extends EventDispatcher {
     /**
      * Creates a new instance.
-     * @param {object} details
+     * @param {object} details Details about how to set up the instance.
      * @param {string} details.id The ID of the popup.
      * @param {number} details.depth The depth of the popup.
      * @param {number} details.frameId The ID of the host frame.
@@ -66,10 +66,10 @@ class PopupProxy extends EventDispatcher {
 
     /**
      * Attempts to set the parent popup.
-     * @param {Popup} value
-     * @throws Throws an error, since this class doesn't support a direct parent.
+     * @param {Popup} _value The parent to assign.
+     * @throws {Error} Throws an error, since this class doesn't support a direct parent.
      */
-    set parent(value) {
+    set parent(_value) {
         throw new Error('Not supported on PopupProxy');
     }
 
@@ -84,10 +84,10 @@ class PopupProxy extends EventDispatcher {
 
     /**
      * Attempts to set the child popup.
-     * @param {Popup} value
-     * @throws Throws an error, since this class doesn't support children.
+     * @param {Popup} _child The child to assign.
+     * @throws {Error} Throws an error, since this class doesn't support children.
      */
-    set child(value) {
+    set child(_child) {
         throw new Error('Not supported on PopupProxy');
     }
 
@@ -233,7 +233,6 @@ class PopupProxy extends EventDispatcher {
 
     /**
      * Returns whether or not the popup is currently visible, synchronously.
-     * @returns {boolean} `true` if the popup is visible, `false` otherwise.
      * @throws An exception is thrown for `PopupProxy` since it cannot synchronously detect visibility.
      */
     isVisibleSync() {
diff --git a/ext/js/app/popup-window.js b/ext/js/app/popup-window.js
index 6a63a9cb..19753906 100644
--- a/ext/js/app/popup-window.js
+++ b/ext/js/app/popup-window.js
@@ -21,7 +21,7 @@
 class PopupWindow extends EventDispatcher {
     /**
      * Creates a new instance.
-     * @param {object} details
+     * @param {object} details Details about how to set up the instance.
      * @param {string} details.id The ID of the popup.
      * @param {number} details.depth The depth of the popup.
      * @param {number} details.frameId The ID of the host frame.
@@ -53,10 +53,10 @@ class PopupWindow extends EventDispatcher {
     /**
      * The parent of the popup, which is always `null` for `PopupWindow` instances,
      * since any potential parent popups are in a different frame.
-     * @param {Popup} value
-     * @type {Popup}
+     * @param {Popup} _value The parent to assign.
+     * @throws {Error} Throws an error, since this class doesn't support children.
      */
-    set parent(value) {
+    set parent(_value) {
         throw new Error('Not supported on PopupWindow');
     }
 
@@ -71,10 +71,10 @@ class PopupWindow extends EventDispatcher {
 
     /**
      * Attempts to set the child popup.
-     * @param {Popup} value
+     * @param {Popup} _value The child to assign.
      * @throws Throws an error, since this class doesn't support children.
      */
-    set child(value) {
+    set child(_value) {
         throw new Error('Not supported on PopupWindow');
     }
 
@@ -168,7 +168,7 @@ class PopupWindow extends EventDispatcher {
 
     /**
      * Shows and updates the positioning and content of the popup.
-     * @param {Popup.ContentDetails} details Settings for the outer popup.
+     * @param {Popup.ContentDetails} _details Settings for the outer popup.
      * @param {Display.ContentDetails} displayDetails The details parameter passed to `Display.setContent`.
      * @returns {Promise<void>}
      */
@@ -180,6 +180,7 @@ class PopupWindow extends EventDispatcher {
     /**
      * Sets the custom styles for the popup content.
      * @param {string} css The CSS rules.
+     * @returns {Promise<void>}
      */
     setCustomCss(css) {
         return this._invoke(false, 'Display.setCustomCss', {id: this._id, css});
@@ -187,6 +188,7 @@ class PopupWindow extends EventDispatcher {
 
     /**
      * Stops the audio auto-play timer, if one has started.
+     * @returns {Promise<void>}
      */
     clearAutoPlayTimer() {
         return this._invoke(false, 'Display.clearAutoPlayTimer', {id: this._id});
@@ -194,7 +196,7 @@ class PopupWindow extends EventDispatcher {
 
     /**
      * Sets the scaling factor of the popup content.
-     * @param {number} scale The scaling factor.
+     * @param {number} _scale The scaling factor.
      */
     setContentScale(_scale) {
         // NOP
@@ -202,7 +204,6 @@ class PopupWindow extends EventDispatcher {
 
     /**
      * Returns whether or not the popup is currently visible, synchronously.
-     * @returns {boolean} `true` if the popup is visible, `false` otherwise.
      * @throws An exception is thrown for `PopupWindow` since it cannot synchronously detect visibility.
      */
     isVisibleSync() {
diff --git a/ext/js/app/popup.js b/ext/js/app/popup.js
index e3ac4dc3..9ca2165d 100644
--- a/ext/js/app/popup.js
+++ b/ext/js/app/popup.js
@@ -66,7 +66,7 @@ class Popup extends EventDispatcher {
 
     /**
      * Creates a new instance.
-     * @param {object} details
+     * @param {object} details The details used to construct the new instance.
      * @param {string} details.id The ID of the popup.
      * @param {number} details.depth The depth of the popup.
      * @param {number} details.frameId The ID of the host frame.
@@ -673,9 +673,11 @@ class Popup extends EventDispatcher {
     }
 
     /**
-     * @param {Rect[]} sourceRects
-     * @param {string} writingMode
-     * @returns {SizeRect}
+     * Computes the position where the popup should be placed relative to the source content.
+     * @param {Rect[]} sourceRects The rectangles of the source content.
+     * @param {string} writingMode The CSS writing mode of the source text.
+     * @param {Rect} viewport The viewport that the popup can be placed within.
+     * @returns {SizeRect} The calculated rectangle for where to position the popup.
      */
     _getPosition(sourceRects, writingMode, viewport) {
         const scale = this._contentScale;
@@ -720,7 +722,15 @@ class Popup extends EventDispatcher {
     }
 
     /**
-     * @returns {SizeRect}
+     * Computes the position where the popup should be placed for horizontal text.
+     * @param {Rect} sourceRect The rectangle of the source content.
+     * @param {number} frameWidth The preferred width of the frame.
+     * @param {number} frameHeight The preferred height of the frame.
+     * @param {Rect} viewport The viewport that the frame can be placed within.
+     * @param {number} horizontalOffset The horizontal offset from the source rect that the popup will be placed.
+     * @param {number} verticalOffset The vertical offset from the source rect that the popup will be placed.
+     * @param {boolean} preferBelow Whether or not the popup is preferred to be placed below the source content.
+     * @returns {SizeRect} The calculated rectangle for where to position the popup.
      */
     _getPositionForHorizontalText(sourceRect, frameWidth, frameHeight, viewport, horizontalOffset, verticalOffset, preferBelow) {
         const [left, width, after] = this._getConstrainedPosition(
@@ -743,7 +753,15 @@ class Popup extends EventDispatcher {
     }
 
     /**
-     * @returns {SizeRect}
+     * Computes the position where the popup should be placed for vertical text.
+     * @param {Rect} sourceRect The rectangle of the source content.
+     * @param {number} frameWidth The preferred width of the frame.
+     * @param {number} frameHeight The preferred height of the frame.
+     * @param {Rect} viewport The viewport that the frame can be placed within.
+     * @param {number} horizontalOffset The horizontal offset from the source rect that the popup will be placed.
+     * @param {number} verticalOffset The vertical offset from the source rect that the popup will be placed.
+     * @param {boolean} preferRight Whether or not the popup is preferred to be placed to the right of the source content.
+     * @returns {SizeRect} The calculated rectangle for where to position the popup.
      */
     _getPositionForVerticalText(sourceRect, frameWidth, frameHeight, viewport, horizontalOffset, verticalOffset, preferRight) {
         const [left, width, after] = this._getConstrainedPositionBinary(
@@ -824,6 +842,11 @@ class Popup extends EventDispatcher {
         return [position, size, after];
     }
 
+    /**
+     * Gets the visual viewport.
+     * @param {boolean} useVisualViewport Whether or not the `window.visualViewport` should be used.
+     * @returns {Rect} The rectangle of the visual viewport.
+     */
     _getViewport(useVisualViewport) {
         const visualViewport = window.visualViewport;
         if (visualViewport !== null && typeof visualViewport === 'object') {
@@ -887,8 +910,9 @@ class Popup extends EventDispatcher {
     }
 
     /**
-     * @param {Rect[]} sourceRects
-     * @returns {Rect}
+     * Computes the bounding rectangle for a set of rectangles.
+     * @param {Rect[]} sourceRects An array of rectangles.
+     * @returns {Rect} The bounding rectangle for all of the source rectangles.
      */
     _getBoundingSourceRect(sourceRects) {
         switch (sourceRects.length) {
@@ -907,10 +931,11 @@ class Popup extends EventDispatcher {
     }
 
     /**
-     * @param {SizeRect} sizeRect
-     * @param {Rect[]} sourceRects
-     * @param {number} ignoreIndex
-     * @returns {boolean}
+     * Checks whether or not a rectangle is overlapping any other rectangles.
+     * @param {SizeRect} sizeRect The rectangles to check for overlaps.
+     * @param {Rect[]} sourceRects The list of rectangles to compare against.
+     * @param {number} ignoreIndex The index of an item in `sourceRects` to ignore.
+     * @returns {boolean} `true` if `sizeRect` overlaps any one of `sourceRects`, excluding `sourceRects[ignoreIndex]`; `false` otherwise.
      */
     _isOverlapping(sizeRect, sourceRects, ignoreIndex) {
         const {left, top} = sizeRect;
diff --git a/ext/js/app/theme-controller.js b/ext/js/app/theme-controller.js
index 4ee82c52..6a95e2d7 100644
--- a/ext/js/app/theme-controller.js
+++ b/ext/js/app/theme-controller.js
@@ -174,7 +174,7 @@ class ThemeController {
 
     /**
      * Adds the value of a CSS color to an accumulation target.
-     * @param {[number, number, number]} target The target color buffer to accumulate into.
+     * @param {number[]} target The target color buffer to accumulate into, as an array of [r, g, b].
      * @param {string|*} cssColor The CSS color value to add to the target. If this value is not a string,
      *   the target will not be modified.
      */
@@ -196,7 +196,7 @@ class ThemeController {
     /**
      * Decomposes a CSS color string into its RGBA values.
      * @param {string} cssColor The color value to decompose. This value is expected to be in the form RGB(r, g, b) or RGBA(r, g, b, a).
-     * @returns {?[number, number, number, number]} The color and alpha values. The color component values range from [0, 255], and the alpha ranges from [0, 1].
+     * @returns {?number[]} The color and alpha values as [r, g, b, a]. The color component values range from [0, 255], and the alpha ranges from [0, 1].
      */
     _getColorInfo(cssColor) {
         const m = /^\s*rgba?\s*\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*(?:,\s*([\d.]+)\s*)?\)\s*$/.exec(cssColor);