Skip to content

Commit

Permalink
chore: Strictly require jsdoc
Browse files Browse the repository at this point in the history
This enables the eslint rule requiring jsdocs on all class
declarations, function declarations, and methods.

Unfortunately, there are two problems with this:

1. We don't use class _declarations_, we use class _expressions_,
which are not covered by this rule.  So it does not enforce jsdoc at
the class level.
2. We tend to document a class at the class-level, rather than at the
constructor.  But a constructor counts as a method for eslint, so it
requires docs on the constructor.  There is no way to configure it to
make an exception for trivial constructors.

So for all trivial (no-argument) constructors, we add empty jsdocs:
  /** */
  constructor() {

This was quicker and easier than setting up some alternative plugin in
eslint to make an exception for us.

The good news is that this rule caught several undocumented parameters
and places where the jsdoc comment was malformed.  So fixing those
also improves the compiler's ability to enforce types.

Change-Id: Icbc46ed690c94e53d354648a883119524f8fca45
  • Loading branch information
joeyparrish committed Jan 9, 2021
1 parent 6c012aa commit 562a2d5
Show file tree
Hide file tree
Showing 46 changed files with 100 additions and 19 deletions.
25 changes: 19 additions & 6 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,6 @@ module.exports = {
'no-shadow': 'off',
// }}}

// Temporary Google style overrides while we get in compliance with the
// latest style guide {{{
'require-jsdoc': 'off',
// }}}

// "Possible error" rules: {{{
'no-async-promise-executor': 'error',
'no-await-in-loop': 'error',
Expand Down Expand Up @@ -178,6 +173,13 @@ module.exports = {
// verbatim through the compiler.
'markers': ['*', '!'],
}],
'require-jsdoc': ['error', {
'require': {
'FunctionDeclaration': true,
'MethodDefinition': true,
'ClassDeclaration': true,
},
}],
// }}}

// "ECMAScript 6" rules: {{{
Expand Down Expand Up @@ -272,12 +274,23 @@ module.exports = {
],
},
{
'files': ['externs/*', 'externs/shaka/*'],
'rules': {
// Disable rules on useless constructors so we can use ES6 classes in
// externs.
'no-useless-constructor': 'off',
},
'files': ['externs/**/*.js'],
},
{
'rules': {
// JSDoc is not strictly required in externs, tests, and in load.js.
'require-jsdoc': 'off',
},
'files': [
'demo/load.js',
'externs/**/*.js',
'test/**/*.js',
],
},
],
};
7 changes: 7 additions & 0 deletions build/pixelsChanged.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@

const Jimp = require('jimp');

/**
* Compare two images and output the number of changed pixels. Uses the same
* node module as the comparisons done in the tests through Karma.
*
* @param {string} oldPath
* @param {string} newPath
*/
async function main(oldPath, newPath) {
const oldImage = await Jimp.read(oldPath);
const newImage = await Jimp.read(newPath);
Expand Down
1 change: 1 addition & 0 deletions demo/cast_receiver/receiver_app.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ goog.require('ShakaDemoAssetInfo');
* @suppress {missingProvide}
*/
class ShakaReceiverApp {
/** */
constructor() {
/** @private {HTMLVideoElement} */
this.video_ = null;
Expand Down
1 change: 1 addition & 0 deletions demo/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ goog.require('shakaDemo.Utils');
* configuration, etc).
*/
shakaDemo.Main = class {
/** */
constructor() {
/** @private {HTMLVideoElement} */
this.video_ = null;
Expand Down
1 change: 1 addition & 0 deletions lib/abr/ewma_bandwidth_estimator.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ goog.require('shaka.abr.Ewma');
*
*/
shaka.abr.EwmaBandwidthEstimator = class {
/** */
constructor() {
/**
* A fast-moving average.
Expand Down
3 changes: 2 additions & 1 deletion lib/abr/simple_abr_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ goog.require('shaka.util.StreamUtils');
* @export
*/
shaka.abr.SimpleAbrManager = class {
/** */
constructor() {
/** @private {?shaka.extern.AbrManager.SwitchCallback} */
this.switch_ = null;
Expand Down Expand Up @@ -284,7 +285,7 @@ shaka.abr.SimpleAbrManager = class {
}


/*
/**
* @private
*/
getDefaultBandwidth_() {
Expand Down
1 change: 1 addition & 0 deletions lib/ads/ad_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ goog.require('shaka.util.FakeEventTarget');
* @export
*/
shaka.ads.AdManager = class extends shaka.util.FakeEventTarget {
/** */
constructor() {
super();
/** @private {shaka.ads.ClientSideAdManager} */
Expand Down
1 change: 1 addition & 0 deletions lib/ads/ads_stats.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ goog.provide('shaka.ads.AdsStats');
* @final
*/
shaka.ads.AdsStats = class {
/** */
constructor() {
/** @private {!Array.<number>} */
this.loadTimes_ = [];
Expand Down
1 change: 1 addition & 0 deletions lib/cea/cea708_window.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ shaka.cea.Cea708Window = class {
this.italics_ = italics;
}

/** Reset the pen to 0,0 with default styling. */
resetPen() {
this.row_ = 0;
this.col_ = 0;
Expand Down
1 change: 1 addition & 0 deletions lib/cea/cea_decoder.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ goog.requireType('shaka.cea.ICaptionDecoder.ClosedCaption');
* @implements {shaka.cea.ICaptionDecoder}
*/
shaka.cea.CeaDecoder = class {
/** */
constructor() {
/**
* An array of CEA-608 closed caption data extracted for decoding.
Expand Down
7 changes: 7 additions & 0 deletions lib/cea/cea_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,13 @@ shaka.cea.CeaUtils = class {
};

shaka.cea.CeaUtils.StyledChar = class {
/**
* @param {string} character
* @param {boolean} underline
* @param {boolean} italics
* @param {string} backgroundColor
* @param {string} textColor
*/
constructor(character, underline, italics, backgroundColor, textColor) {
/**
* @private {!string}
Expand Down
17 changes: 11 additions & 6 deletions lib/cea/dtvcc_packet_builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@ goog.require('shaka.util.Error');
goog.requireType('shaka.cea.Cea708Service');


// CEA-708 DTVCC Packet Builder.
// Builds packets based on Figure 5 CCP State Table in 5.2 of CEA-708-E.
// Initially, there is no packet. When a DTVCC_PACKET_START payload is received,
// a packet begins construction. The packet is considered "built" once all bytes
// indicated in the header are read, and ignored if a new packet starts building
// before the current packet is finished being built.
/**
* CEA-708 DTVCC Packet Builder.
* Builds packets based on Figure 5 CCP State Table in 5.2 of CEA-708-E.
* Initially, there is no packet. When a DTVCC_PACKET_START payload is received,
* a packet begins construction. The packet is considered "built" once all bytes
* indicated in the header are read, and ignored if a new packet starts building
* before the current packet is finished being built.
*/
shaka.cea.DtvccPacketBuilder = class {
/** */
constructor() {
/**
* An array containing built DTVCC packets that are ready to be processed.
Expand Down Expand Up @@ -83,10 +86,12 @@ shaka.cea.DtvccPacketBuilder = class {
return this.builtPackets_;
}

/** Clear built packets. */
clearBuiltPackets() {
this.builtPackets_ = [];
}

/** Clear built packets and packets in progress. */
clear() {
this.builtPackets_ = [];
this.currentPacketBeingBuilt_ = [];
Expand Down
1 change: 1 addition & 0 deletions lib/cea/mp4_cea_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ goog.require('shaka.util.Mp4BoxParsers');
* @implements {shaka.cea.ICeaParser}
*/
shaka.cea.Mp4CeaParser = class {
/** */
constructor() {
/**
* SEI data processor.
Expand Down
2 changes: 1 addition & 1 deletion lib/hls/hls_classes.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ shaka.hls.Tag = class {
return attribute.value;
}

/*
/**
* Set the name of the tag. Used only for Preload hinted MAP tag.
* @param {string} name
*/
Expand Down
1 change: 1 addition & 0 deletions lib/hls/manifest_text_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ goog.require('shaka.util.TextParser');
* HlS manifest text parser.
*/
shaka.hls.ManifestTextParser = class {
/** */
constructor() {
/** @private {number} */
this.globalId_ = 0;
Expand Down
1 change: 1 addition & 0 deletions lib/media/closed_caption_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ shaka.media.IClosedCaptionParser = class {
* @final
*/
shaka.media.ClosedCaptionParser = class {
/** */
constructor() {
/**
* MP4 Parser to extract closed caption packets from H.264 video.
Expand Down
5 changes: 4 additions & 1 deletion lib/media/region_timeline.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ goog.require('shaka.util.Timer');
* @final
*/
shaka.media.RegionTimeline = class {
/**
* @param {!function():{start: number, end: number}} getSeekRange
*/
constructor(getSeekRange) {
/** @private {function(shaka.extern.TimelineRegionInfo)} */
this.onAddRegion_ = (region) => {};
Expand Down Expand Up @@ -73,7 +76,7 @@ shaka.media.RegionTimeline = class {
}
}

/*
/**
* @private
*/
filterBySeekRange_() {
Expand Down
1 change: 1 addition & 0 deletions lib/media/segment_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,7 @@ shaka.media.SegmentIterator = class {
* @export
*/
shaka.media.MetaSegmentIndex = class extends shaka.media.SegmentIndex {
/** */
constructor() {
super([]);

Expand Down
1 change: 1 addition & 0 deletions lib/media/transmuxer.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ goog.require('shaka.dependencies');
* @implements {shaka.util.IDestroyable}
*/
shaka.media.Transmuxer = class {
/** */
constructor() {
/** @private {?muxjs} */
this.muxjs_ = shaka.dependencies.muxjs();
Expand Down
1 change: 1 addition & 0 deletions lib/offline/download_progress_estimator.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ goog.provide('shaka.offline.DownloadProgressEstimator');
* @final
*/
shaka.offline.DownloadProgressEstimator = class {
/** */
constructor() {
/**
* This is the sum of all estimates passed to |open|. This is used as the
Expand Down
1 change: 1 addition & 0 deletions lib/offline/indexeddb/storage_mechanism.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ goog.require('shaka.util.Platform');
* @implements {shaka.extern.StorageMechanism}
*/
shaka.offline.indexeddb.StorageMechanism = class {
/** */
constructor() {
/** @private {IDBDatabase} */
this.db_ = null;
Expand Down
1 change: 1 addition & 0 deletions lib/offline/offline_manifest_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ goog.require('shaka.util.Error');
* @implements {shaka.extern.ManifestParser}
*/
shaka.offline.OfflineManifestParser = class {
/** */
constructor() {
/** @private {shaka.offline.OfflineUri} */
this.uri_ = null;
Expand Down
1 change: 1 addition & 0 deletions lib/offline/storage_muxer.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ shaka.offline.StorageCellHandle;
* @export
*/
shaka.offline.StorageMuxer = class {
/** */
constructor() {
/**
* A key in this map is the name given when registering a StorageMechanism.
Expand Down
1 change: 1 addition & 0 deletions lib/offline/stream_bandwidth_estimator.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ goog.requireType('shaka.media.SegmentReference');
* will have some influence over the progress of the download.
*/
shaka.offline.StreamBandwidthEstimator = class {
/** */
constructor() {
/** @private {!Object.<number, number>} */
this.estimateByStreamId_ = {};
Expand Down
3 changes: 3 additions & 0 deletions lib/polyfill/orientation.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ shaka.polyfill.Orientation = class {

shaka.polyfill.Orientation.FakeOrientation =
class extends shaka.util.FakeEventTarget {
/** */
constructor() {
super();

Expand All @@ -87,6 +88,7 @@ class extends shaka.util.FakeEventTarget {
this.angle = 0;
}

/** Dispatch a 'change' event. */
dispatchChangeEvent() {
const event = new shaka.util.FakeEvent('change', {});
this.dispatchEvent(event);
Expand Down Expand Up @@ -150,6 +152,7 @@ class extends shaka.util.FakeEventTarget {
return Promise.reject(unsupportedKeySystemError);
}

/** Unlock the screen orientation. */
unlock() {
// screen.unlockOrientation has a return value, but
// screen.orientation.unlock does not. So ignore the return value.
Expand Down
1 change: 1 addition & 0 deletions lib/polyfill/patchedmediakeys_apple.js
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,7 @@ class extends shaka.util.FakeEventTarget {
* @implements {MediaKeyStatusMap}
*/
shaka.polyfill.PatchedMediaKeysApple.MediaKeyStatusMap = class {
/** */
constructor() {
/**
* @type {number}
Expand Down
1 change: 1 addition & 0 deletions lib/polyfill/patchedmediakeys_ms.js
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ class extends shaka.util.FakeEventTarget {
* @implements {MediaKeyStatusMap}
*/
shaka.polyfill.PatchedMediaKeysMs.MediaKeyStatusMap = class {
/** */
constructor() {
/**
* @type {number}
Expand Down
2 changes: 2 additions & 0 deletions lib/polyfill/patchedmediakeys_nop.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ shaka.polyfill.PatchedMediaKeysNop = class {
* @implements {MediaKeys}
*/
shaka.polyfill.PatchedMediaKeysNop.MediaKeys = class {
/** */
constructor() {
throw new TypeError('Illegal constructor.');
}
Expand All @@ -111,6 +112,7 @@ shaka.polyfill.PatchedMediaKeysNop.MediaKeys = class {
* @implements {MediaKeySystemAccess}
*/
shaka.polyfill.PatchedMediaKeysNop.MediaKeySystemAccess = class {
/** */
constructor() {
/** @override */
this.keySystem = ''; // For the compiler.
Expand Down
1 change: 1 addition & 0 deletions lib/polyfill/patchedmediakeys_webkit.js
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,7 @@ class extends shaka.util.FakeEventTarget {
* @implements {MediaKeyStatusMap}
*/
shaka.polyfill.PatchedMediaKeysWebkit.MediaKeyStatusMap = class {
/** */
constructor() {
/**
* @type {number}
Expand Down
1 change: 1 addition & 0 deletions lib/text/cue.js
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ shaka.text.Cue.textDecoration = {
* @export
*/
shaka.text.CueRegion = class {
/** */
constructor() {
const CueRegion = shaka.text.CueRegion;

Expand Down
1 change: 1 addition & 0 deletions lib/text/mp4_ttml_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ goog.require('shaka.util.Mp4Parser');
* @export
*/
shaka.text.Mp4TtmlParser = class {
/** */
constructor() {
/**
* @type {!shaka.extern.TextParser}
Expand Down
1 change: 1 addition & 0 deletions lib/text/mp4_vtt_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ goog.require('shaka.util.TextParser');
* @export
*/
shaka.text.Mp4VttParser = class {
/** */
constructor() {
/**
* The current time scale used by the VTT parser.
Expand Down
1 change: 1 addition & 0 deletions lib/text/srt_text_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ goog.require('shaka.util.StringUtils');
* @export
*/
shaka.text.SrtTextParser = class {
/** */
constructor() {
/**
* @type {!shaka.extern.TextParser}
Expand Down
1 change: 1 addition & 0 deletions lib/util/event_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ goog.require('shaka.util.MultiMap');
* @export
*/
shaka.util.EventManager = class {
/** */
constructor() {
/**
* Maps an event type to an array of event bindings.
Expand Down
Loading

0 comments on commit 562a2d5

Please sign in to comment.