Use the ESLint no-restricted-syntax rule to ensure that assert is always called with two arguments

Having `assert` calls without a message string isn't very helpful when debugging, and it turns out that it's easy enough to make use of ESLint to enforce better `assert` call-sites.
In a couple of cases the `assert` calls were changed to "regular" throwing of errors instead, since that seemed more appropriate.

Please find additional details about the ESLint rule at https://eslint.org/docs/rules/no-restricted-syntax
This commit is contained in:
Jonas Jenwald 2020-05-05 12:40:01 +02:00
parent 491904d30a
commit e1f340a0c2
14 changed files with 73 additions and 22 deletions

View File

@ -145,6 +145,10 @@
"no-nested-ternary": "error", "no-nested-ternary": "error",
"no-new-object": "error", "no-new-object": "error",
"no-restricted-syntax": ["error", "no-restricted-syntax": ["error",
{
"selector": "CallExpression[callee.name='assert'][arguments.length!=2]",
"message": "`assert()` must always be invoked with two arguments.",
},
{ {
"selector": "NewExpression[callee.name='Cmd']", "selector": "NewExpression[callee.name='Cmd']",
"message": "Use `Cmd.get()` rather than `new Cmd()`.", "message": "Use `Cmd.get()` rather than `new Cmd()`.",

View File

@ -767,7 +767,15 @@ class PDFDocument {
_getLinearizationPage(pageIndex) { _getLinearizationPage(pageIndex) {
const { catalog, linearization } = this; const { catalog, linearization } = this;
assert(linearization && linearization.pageFirst === pageIndex); if (
typeof PDFJSDev === "undefined" ||
PDFJSDev.test("!PRODUCTION || TESTING")
) {
assert(
linearization && linearization.pageFirst === pageIndex,
"_getLinearizationPage - invalid pageIndex argument."
);
}
const ref = Ref.get(linearization.objectNumberFirst, 0); const ref = Ref.get(linearization.objectNumberFirst, 0);
return this.xref return this.xref

View File

@ -578,7 +578,11 @@ class Catalog {
} }
break; break;
default: default:
assert(typeof value === "boolean"); if (typeof value !== "boolean") {
throw new FormatError(
`viewerPreferences - expected a boolean value for: ${key}`
);
}
prefValue = value; prefValue = value;
} }

View File

@ -216,7 +216,7 @@ class Parser {
} else if (state === 1) { } else if (state === 1) {
state = ch === I ? 2 : 0; state = ch === I ? 2 : 0;
} else { } else {
assert(state === 2); assert(state === 2, "findDefaultInlineStreamEnd - invalid state.");
if (ch === SPACE || ch === LF || ch === CR) { if (ch === SPACE || ch === LF || ch === CR) {
maybeEIPos = stream.pos; maybeEIPos = stream.pos;
// Let's check that the next `n` bytes are ASCII... just to be sure. // Let's check that the next `n` bytes are ASCII... just to be sure.

View File

@ -26,7 +26,10 @@ class PDFWorkerStream {
} }
getFullReader() { getFullReader() {
assert(!this._fullRequestReader); assert(
!this._fullRequestReader,
"PDFWorkerStream.getFullReader can only be called once."
);
this._fullRequestReader = new PDFWorkerStreamReader(this._msgHandler); this._fullRequestReader = new PDFWorkerStreamReader(this._msgHandler);
return this._fullRequestReader; return this._fullRequestReader;
} }

View File

@ -2023,7 +2023,10 @@ class WorkerTransport {
const { messageHandler, loadingTask } = this; const { messageHandler, loadingTask } = this;
messageHandler.on("GetReader", (data, sink) => { messageHandler.on("GetReader", (data, sink) => {
assert(this._networkStream); assert(
this._networkStream,
"GetReader - no `IPDFStream` instance available."
);
this._fullReader = this._networkStream.getFullReader(); this._fullReader = this._networkStream.getFullReader();
this._fullReader.onProgress = evt => { this._fullReader.onProgress = evt => {
this._lastProgress = { this._lastProgress = {
@ -2039,7 +2042,10 @@ class WorkerTransport {
sink.close(); sink.close();
return; return;
} }
assert(isArrayBuffer(value)); assert(
isArrayBuffer(value),
"GetReader - expected an ArrayBuffer."
);
// Enqueue data chunk into sink, and transfer it // Enqueue data chunk into sink, and transfer it
// to other side as `Transferable` object. // to other side as `Transferable` object.
sink.enqueue(new Uint8Array(value), 1, [value]); sink.enqueue(new Uint8Array(value), 1, [value]);
@ -2085,7 +2091,10 @@ class WorkerTransport {
}); });
messageHandler.on("GetRangeReader", (data, sink) => { messageHandler.on("GetRangeReader", (data, sink) => {
assert(this._networkStream); assert(
this._networkStream,
"GetRangeReader - no `IPDFStream` instance available."
);
const rangeReader = this._networkStream.getRangeReader( const rangeReader = this._networkStream.getRangeReader(
data.begin, data.begin,
data.end data.end
@ -2114,7 +2123,10 @@ class WorkerTransport {
sink.close(); sink.close();
return; return;
} }
assert(isArrayBuffer(value)); assert(
isArrayBuffer(value),
"GetRangeReader - expected an ArrayBuffer."
);
sink.enqueue(new Uint8Array(value), 1, [value]); sink.enqueue(new Uint8Array(value), 1, [value]);
}) })
.catch(reason => { .catch(reason => {

View File

@ -65,7 +65,10 @@ class PDFFetchStream {
} }
getFullReader() { getFullReader() {
assert(!this._fullRequestReader); assert(
!this._fullRequestReader,
"PDFFetchStream.getFullReader can only be called once."
);
this._fullRequestReader = new PDFFetchStreamReader(this); this._fullRequestReader = new PDFFetchStreamReader(this);
return this._fullRequestReader; return this._fullRequestReader;
} }

View File

@ -247,7 +247,10 @@ class PDFNetworkStream {
} }
getFullReader() { getFullReader() {
assert(!this._fullRequestReader); assert(
!this._fullRequestReader,
"PDFNetworkStream.getFullReader can only be called once."
);
this._fullRequestReader = new PDFNetworkStreamFullRequestReader( this._fullRequestReader = new PDFNetworkStreamFullRequestReader(
this._manager, this._manager,
this._source this._source

View File

@ -67,7 +67,10 @@ class PDFNodeStream {
} }
getFullReader() { getFullReader() {
assert(!this._fullRequestReader); assert(
!this._fullRequestReader,
"PDFNodeStream.getFullReader can only be called once."
);
this._fullRequestReader = this.isFsUrl this._fullRequestReader = this.isFsUrl
? new PDFNodeStreamFsFullReader(this) ? new PDFNodeStreamFsFullReader(this)
: new PDFNodeStreamFullReader(this); : new PDFNodeStreamFullReader(this);

View File

@ -19,7 +19,10 @@ import { assert, createPromiseCapability } from "../shared/util.js";
/** @implements {IPDFStream} */ /** @implements {IPDFStream} */
class PDFDataTransportStream { class PDFDataTransportStream {
constructor(params, pdfDataRangeTransport) { constructor(params, pdfDataRangeTransport) {
assert(pdfDataRangeTransport); assert(
pdfDataRangeTransport,
'PDFDataTransportStream - missing required "pdfDataRangeTransport" argument.'
);
this._queuedChunks = []; this._queuedChunks = [];
this._progressiveDone = params.progressiveDone || false; this._progressiveDone = params.progressiveDone || false;
@ -73,7 +76,10 @@ class PDFDataTransportStream {
rangeReader._enqueue(buffer); rangeReader._enqueue(buffer);
return true; return true;
}); });
assert(found); assert(
found,
"_onReceiveData - no `PDFDataTransportStreamRangeReader` instance found."
);
} }
} }
@ -111,7 +117,10 @@ class PDFDataTransportStream {
} }
getFullReader() { getFullReader() {
assert(!this._fullRequestReader); assert(
!this._fullRequestReader,
"PDFDataTransportStream.getFullReader can only be called once."
);
const queuedChunks = this._queuedChunks; const queuedChunks = this._queuedChunks;
this._queuedChunks = null; this._queuedChunks = null;
return new PDFDataTransportStreamReader( return new PDFDataTransportStreamReader(

View File

@ -516,7 +516,7 @@ function arrayByteLength(arr) {
if (arr.length !== undefined) { if (arr.length !== undefined) {
return arr.length; return arr.length;
} }
assert(arr.byteLength !== undefined); assert(arr.byteLength !== undefined, "arrayByteLength - invalid argument.");
return arr.byteLength; return arr.byteLength;
} }

View File

@ -21,8 +21,7 @@ import { setPDFNetworkStreamFactory } from "../../src/display/api.js";
// Ensure that this script only runs in Node.js environments. // Ensure that this script only runs in Node.js environments.
if (!isNodeJS) { if (!isNodeJS) {
throw new Error( throw new Error(
"The `gulp unittestcli` command can only be used in " + "The `gulp unittestcli` command can only be used in Node.js environments."
"Node.js environments."
); );
} }

View File

@ -92,8 +92,7 @@ function initializePDFJS(callback) {
if (isNodeJS) { if (isNodeJS) {
throw new Error( throw new Error(
"The `gulp unittest` command cannot be used in " + "The `gulp unittest` command cannot be used in Node.js environments."
"Node.js environments."
); );
} }
// Set the network stream factory for unit-tests. // Set the network stream factory for unit-tests.

View File

@ -14,12 +14,16 @@
*/ */
/* globals __non_webpack_require__ */ /* globals __non_webpack_require__ */
import { AbortException, assert } from "../../src/shared/util.js"; import { AbortException } from "../../src/shared/util.js";
import { isNodeJS } from "../../src/shared/is_node.js"; import { isNodeJS } from "../../src/shared/is_node.js";
import { PDFNodeStream } from "../../src/display/node_stream.js"; import { PDFNodeStream } from "../../src/display/node_stream.js";
// Make sure that we only running this script is Node.js environments. // Ensure that these test only runs in Node.js environments.
assert(isNodeJS); if (!isNodeJS) {
throw new Error(
'The "node_stream" unit-tests can only be run in Node.js environments.'
);
}
const path = __non_webpack_require__("path"); const path = __non_webpack_require__("path");
const url = __non_webpack_require__("url"); const url = __non_webpack_require__("url");