Skip to content

Commit

Permalink
Enable the ESLint no-console rule in parts of the code-base
Browse files Browse the repository at this point in the history
The purpose of these changes is to make it more difficult to accidentally include logging statements, used during development and debugging, when submitting patches for review.

For (almost) all code residing in the `src/` folder we should use our existing helper functions to ensure that all logging can be controlled via the `verbosity` API-option.

For the `test/unit/` respectively `test/integration/` folders we should not need any "normal" logging, but it should be OK to print the *occasional* warning/error message.

Please find additional details about the ESLint rule at https://eslint.org/docs/latest/rules/no-console
  • Loading branch information
Snuffleupagus committed Nov 14, 2024
1 parent 9bf9bbd commit 50f8b0d
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 3 deletions.
8 changes: 8 additions & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,12 @@ export default [
"template-curly-spacing": ["error", "never"],
},
},
{
files: jsFiles("src"),
rules: {
"no-console": "error",
},
},

/* ======================================================================== *\
Test-specific rules
Expand Down Expand Up @@ -354,11 +360,13 @@ export default [
files: jsFiles("test/unit"),
rules: {
"import/no-unresolved": ["error", { ignore: ["pdfjs/"] }],
"no-console": ["error", { allow: ["warn", "error"] }],
},
},
{
files: jsFiles("test/integration"),
rules: {
"no-console": ["error", { allow: ["warn", "error"] }],
"no-restricted-syntax": [
"error",
{
Expand Down
1 change: 1 addition & 0 deletions src/display/display_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ function noContextMenu(e) {

// Deprecated API function -- display regardless of the `verbosity` setting.
function deprecated(details) {
// eslint-disable-next-line no-console
console.log("Deprecated API usage: " + details);
}

Expand Down
2 changes: 1 addition & 1 deletion src/display/editor/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class ImageManager {
}
data.refCounter = 1;
} catch (e) {
console.error(e);
warn(e);
data = null;
}
this.#cache.set(key, data);
Expand Down
1 change: 1 addition & 0 deletions src/pdf.sandbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class Sandbox {
[buf, this._alertOnError]
);
} catch (error) {
// eslint-disable-next-line no-console
console.error(error);
} finally {
if (buf) {
Expand Down
2 changes: 2 additions & 0 deletions src/shared/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -363,13 +363,15 @@ function getVerbosityLevel() {
// end users.
function info(msg) {
if (verbosity >= VerbosityLevel.INFOS) {
// eslint-disable-next-line no-console
console.log(`Info: ${msg}`);
}
}

// Non-fatal warnings.
function warn(msg) {
if (verbosity >= VerbosityLevel.WARNINGS) {
// eslint-disable-next-line no-console
console.log(`Warning: ${msg}`);
}
}
Expand Down
6 changes: 4 additions & 2 deletions test/integration/test_utils.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,11 @@ async function waitForEvent({

const success = await awaitPromise(handle);
if (success === null) {
console.log(`waitForEvent: ${eventName} didn't trigger within the timeout`);
console.warn(
`waitForEvent: ${eventName} didn't trigger within the timeout`
);
} else if (!success) {
console.log(`waitForEvent: ${eventName} triggered, but validation failed`);
console.warn(`waitForEvent: ${eventName} triggered, but validation failed`);
}
}

Expand Down

0 comments on commit 50f8b0d

Please sign in to comment.