From 1e3d688a325ae0c6780c52e2bb6888535850ed5e Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Thu, 2 Apr 2026 15:08:03 +0200 Subject: [PATCH] Add an eslint plugin for using MathClamp when it's possible --- eslint.config.mjs | 4 + external/eslint_plugins/prefer-math-clamp.mjs | 118 ++++++++++++++++++ src/core/postscript/js_evaluator.js | 5 +- src/shared/math_clamp.js | 1 + web/internal/split_view.js | 9 +- 5 files changed, 128 insertions(+), 9 deletions(-) create mode 100644 external/eslint_plugins/prefer-math-clamp.mjs diff --git a/eslint.config.mjs b/eslint.config.mjs index b948b4562..2f4461b8b 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -5,6 +5,7 @@ import jasmine from "eslint-plugin-jasmine"; import json from "@eslint/json"; import noUnsanitized from "eslint-plugin-no-unsanitized"; import perfectionist from "eslint-plugin-perfectionist"; +import preferMathClamp from "./external/eslint_plugins/prefer-math-clamp.mjs"; import prettierRecommended from "eslint-plugin-prettier/recommended"; import unicorn from "eslint-plugin-unicorn"; @@ -74,6 +75,7 @@ export default [ json, "no-unsanitized": noUnsanitized, perfectionist, + "prefer-math-clamp": preferMathClamp, unicorn, }, @@ -179,6 +181,8 @@ export default [ "unicorn/prefer-ternary": ["error", "only-single-line"], "unicorn/throw-new-error": "error", + "prefer-math-clamp/prefer-math-clamp": "error", + // Possible errors "for-direction": "error", "getter-return": "error", diff --git a/external/eslint_plugins/prefer-math-clamp.mjs b/external/eslint_plugins/prefer-math-clamp.mjs new file mode 100644 index 000000000..989de21dd --- /dev/null +++ b/external/eslint_plugins/prefer-math-clamp.mjs @@ -0,0 +1,118 @@ +/** + * ESLint rule to prefer `MathClamp(v, min, max)` over nested + * `Math.min(Math.max(...), ...)` / `Math.max(Math.min(...), ...)` patterns. + * + * Detected patterns and their fixes: + * Math.min(Math.max(A, B), C) → MathClamp(A, B, C) + * Math.min(C, Math.max(A, B)) → MathClamp(A, B, C) + * Math.max(Math.min(A, B), C) → MathClamp(A, C, B) + * Math.max(C, Math.min(A, B)) → MathClamp(A, C, B) + */ + +function isMathCall(node, method) { + return ( + node.type === "CallExpression" && + node.callee.type === "MemberExpression" && + !node.callee.computed && + node.callee.object.type === "Identifier" && + node.callee.object.name === "Math" && + node.callee.property.type === "Identifier" && + node.callee.property.name === method && + node.arguments.length === 2 && + node.arguments.every(a => a.type !== "SpreadElement") + ); +} + +// Returns true if node is a Math.min or Math.max call. +function isMathMinMax(node) { + return isMathCall(node, "min") || isMathCall(node, "max"); +} + +const preferMathClampRule = { + meta: { + type: "suggestion", + fixable: "code", + docs: { + description: + "Prefer MathClamp(v, min, max) over nested Math.min/Math.max", + }, + messages: { + useClamp: + "Use MathClamp(v, min, max) instead of nested Math.min/Math.max.", + }, + schema: [], + }, + create(context) { + const src = context.sourceCode ?? context.getSourceCode(); + + return { + CallExpression(node) { + // Pattern: Math.min(Math.max(A, B), C) or Math.min(C, Math.max(A, B)). + // Fix as MathClamp(A, B, C) where A,B are inner args, C is outer arg. + if (isMathCall(node, "min")) { + const [arg0, arg1] = node.arguments; + let outerArg, innerNode; + + // Math.max(Math.min(A, B), Math.min(C, D)) isn't a clamp pattern, so + // require the outer arg to not be a min/max call. + if (isMathCall(arg0, "max") && !isMathMinMax(arg1)) { + innerNode = arg0; + outerArg = arg1; + } else if (isMathCall(arg1, "max") && !isMathMinMax(arg0)) { + innerNode = arg1; + outerArg = arg0; + } else { + return; + } + + const v = src.getText(innerNode.arguments[0]); + const min = src.getText(innerNode.arguments[1]); + const max = src.getText(outerArg); + + context.report({ + node, + messageId: "useClamp", + fix(fixer) { + return fixer.replaceText(node, `MathClamp(${v}, ${min}, ${max})`); + }, + }); + } + + // Pattern: Math.max(Math.min(A, B), C) or Math.max(C, Math.min(A, B)). + // Fix as MathClamp(A, C, B) where A,B are inner args, C is outer arg. + if (isMathCall(node, "max")) { + const [arg0, arg1] = node.arguments; + let outerArg, innerNode; + + if (isMathCall(arg0, "min") && !isMathMinMax(arg1)) { + innerNode = arg0; + outerArg = arg1; + } else if (isMathCall(arg1, "min") && !isMathMinMax(arg0)) { + innerNode = arg1; + outerArg = arg0; + } else { + return; + } + + const v = src.getText(innerNode.arguments[0]); + const max = src.getText(innerNode.arguments[1]); + const min = src.getText(outerArg); + + context.report({ + node, + messageId: "useClamp", + fix(fixer) { + return fixer.replaceText(node, `MathClamp(${v}, ${min}, ${max})`); + }, + }); + } + }, + }; + }, +}; + +export default { + rules: { + "prefer-math-clamp": preferMathClampRule, + }, +}; diff --git a/src/core/postscript/js_evaluator.js b/src/core/postscript/js_evaluator.js index 50436a90a..05126ad02 100644 --- a/src/core/postscript/js_evaluator.js +++ b/src/core/postscript/js_evaluator.js @@ -793,10 +793,7 @@ class PSStackBasedInterpreter { const base = this.#sp - nOut; for (let i = 0; i < nOut; i++) { const v = base + i >= 0 ? this.#stack[base + i] : 0; - dest[destOffset + i] = Math.max( - range[i * 2], - Math.min(range[i * 2 + 1], v) - ); + dest[destOffset + i] = MathClamp(range[i * 2 + 1], range[i * 2], v); } }; } diff --git a/src/shared/math_clamp.js b/src/shared/math_clamp.js index bd8a8f061..20b8071a8 100644 --- a/src/shared/math_clamp.js +++ b/src/shared/math_clamp.js @@ -16,6 +16,7 @@ // TODO: Replace all occurrences of this function, and remove the file, once // https://github.com/tc39/proposal-math-clamp/ is generally available. function MathClamp(v, min, max) { + // eslint-disable-next-line prefer-math-clamp/prefer-math-clamp return Math.min(Math.max(v, min), max); } diff --git a/web/internal/split_view.js b/web/internal/split_view.js index 92b2410c4..901919046 100644 --- a/web/internal/split_view.js +++ b/web/internal/split_view.js @@ -13,6 +13,8 @@ * limitations under the License. */ +import { MathClamp } from "pdfjs/shared/math_clamp.js"; + /** * Wraps two elements with a drag-to-resize handle between them. * @@ -105,12 +107,9 @@ class SplitView { return 0; } if (total <= this.#minSize * 2) { - return Math.min(total, Math.max(0, requestedFirst)); + return MathClamp(0, requestedFirst, total); } - return Math.max( - this.#minSize, - Math.min(total - this.#minSize, requestedFirst) - ); + return MathClamp(total - this.#minSize, this.#minSize, requestedFirst); } #resize(newFirst) {