Merge pull request #21030 from calixteman/eslint_math_clamp

Add an eslint plugin for using MathClamp when it's possible

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",

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,
+  },
+};

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);
       }
     };
   }

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);
 }
 

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) {