Improve rulesIf evaluation

package.json

@@ -24,6 +24,7 @@
     "fetch-schema": "curl https://gitlab.com/gitlab-org/gitlab/-/raw/master/app/assets/javascripts/editor/schema/ci.json -sf > src/schema.json"
   },
   "dependencies": {
+    "@jsep-plugin/regex": "^1.0.4",
     "ajv": "8.x.x",
     "axios": "1.x.x",
     "base64url": "3.x.x",
@@ -37,6 +38,7 @@
     "fs-extra": "11.x.x",
     "globby": "16.x.x",
     "js-yaml": "4.x.x",
+    "jsep": "^1.4.0",
     "jsonpointer": "5.x.x",
     "micromatch": "4.x.x",
     "object-traversal": "1.x.x",

src/utils.ts

@@ -1,6 +1,8 @@
 import "./global.js";
 import {RE2JS} from "re2js";
 import chalk from "chalk-template";
+import jsep from "jsep";
+import jsepRegex from "@jsep-plugin/regex";
 import {Job, JobRule, Need, Service} from "./job.js";
 import {needsComplex} from "./data-expander.js";
 import fs from "fs-extra";
@@ -16,6 +18,10 @@ import {AxiosRequestConfig} from "axios";
 import path from "node:path";
 import {Argv} from "./argv.js";
 
+jsep.plugins.register(jsepRegex); // /pattern/flags literals
+jsep.addBinaryOp("=~", 10); // regex match operator
+jsep.addBinaryOp("!~", 10); // regex non-match operator
+
 type RuleResultOpt = {
     argv: Argv;
     cwd: string;
@@ -213,10 +219,39 @@ export class Utils {
         return {when, allowFailure, variables: ruleVariable, needs: ruleNeeds};
     }
 
+    // Reconstruct the source string for an atomic (non-logical) jsep node.
+    // Identifiers keep their name ($VAR), literals use their raw form so that
+    // strings retain quotes and regexes retain slashes and flags.
+    private static _nodeToAtom (node: jsep.Expression): string {
+        switch (node.type) {
+            case "Identifier": return (node as jsep.Identifier).name;
+            case "Literal": return (node as jsep.Literal).raw;
+            case "BinaryExpression": {
+                const n = node as jsep.BinaryExpression;
+                return `${Utils._nodeToAtom(n.left)} ${n.operator} ${Utils._nodeToAtom(n.right)}`;
+            }
+            default: throw new Error(`Unsupported expression node type: ${(node as any).type}`);
+        }
+    }
+
+    static stripQuotes (str: string) {
+        if (str.length < 2) return str;
+        const first = str[0];
+        const last = str[str.length - 1];
+        if ((first === "\"" && last === "\"") || (first === "'" && last === "'")) {
+            return str.slice(1, -1);
+        }
+        return str;
+    }
+
     static evaluateRuleIf (ruleIf: string | undefined, envs: {[key: string]: string}): boolean {
         if (ruleIf === undefined) return true;
         assert(!/\$\{\w+\}/.test(ruleIf), chalk`rules:rule if invalid expression syntax: {blueBright ${ruleIf}}\nuse {green $VAR} not {red \${VAR\}} in rules:if`);
         let evalStr = ruleIf;
+        evalStr = this.expandTextWith(evalStr, {
+            unescape: JSON.stringify("$"),
+            variable: (name) => JSON.stringify(envs[name] ?? null).replaceAll("\\\\", "\\"),
+        }); // replace all $VAR by their values
 
         const flagsToBinary = (flags: string): number => {
             let binary = 0;
@@ -231,97 +266,100 @@ export class Utils {
             }
             return binary;
         };
-
-        // Expand all variables
-        evalStr = this.expandTextWith(evalStr, {
-            unescape: JSON.stringify("$"),
-            variable: (name) => JSON.stringify(envs[name] ?? null).replaceAll("\\\\", "\\"),
-        });
-        const expandedEvalStr = evalStr;
-
-        // Scenario when RHS is a <regex>
-        // https://regexr.com/85sjo
-        const pattern1 = /\s*(?<operator>(?:=~)|(?:!~))\s*\/(?<rhs>.*?[^\\])\/(?<flags>[igmsuy]*)(\s|$|\))/g;
-        evalStr = evalStr.replaceAll(pattern1, (_, operator, rhs, flags, remainingTokens) => {
-            let _operator;
-            switch (operator) {
-                case "=~":
-                    _operator = "!=";
-                    break;
-                case "!~":
-                    _operator = "==";
-                    break;
-                default:
-                    throw operator;
+        // jsep parses ruleIf into an AST, handling &&, ||, () and operator precedence.
+        const walk = (node: jsep.Expression): boolean => {
+            if (node.type === "BinaryExpression") {
+                const n = node as jsep.BinaryExpression;
+                if (n.operator === "&&") return walk(n.left) && walk(n.right);
+                if (n.operator === "||") return walk(n.left) || walk(n.right);
+                if (n.operator === "=~" || n.operator === "!~") {
+                    assert(n.left.type === "Literal", `Not a Literal: ${JSON.stringify(n.left)}`);
+                    assert(n.right.type === "Literal", `Not a Literal: ${JSON.stringify(n.right)}`);
+                    const leftStr = n.left as jsep.Literal;
+                    const rightStr = n.right as jsep.Literal;
+                    if (leftStr.value === null)
+                        return n.operator === "!~"; // null =~ /p/ → false; null !~ /p/ → true
+                    if (rightStr.value === null)
+                        return false;
+                    let regexStr: string = rightStr.raw;
+                    regexStr = this.stripQuotes(regexStr);
+
+                    const regex = /\/(?<pattern>.*)\/(?<flags>[igmsuy]*)/;
+                    const _rhs = regexStr.replace(regex, (_: string, pattern: string, flags: string) => {
+                        const flagsBinary = flagsToBinary(flags);
+                        return `RE2JS.compile(${JSON.stringify(pattern)}, ${flagsBinary})`;
+                    });
+
+                    const assertMsg = [
+                        "RHS (${rhs}) must be a regex pattern. Do not rely on this behavior!",
+                        "Refer to https://docs.gitlab.com/ee/ci/jobs/job_rules.html#unexpected-behavior-from-regular-expression-matching-with- for more info...",
+                    ];
+                    assert(_rhs !== regexStr, assertMsg.join("\n"));
+
+                    const _operator = n.operator === "=~" ? "!=" : "=="; // =~ -> !=; !~ -> ==
+
+                    const evalStr = `${leftStr.raw}.matchRE2JS(${_rhs}) ${_operator} null`;
+
+                    let res;
+                    try {
+                        (globalThis as any).RE2JS = RE2JS;
+                        res = (0, eval)(evalStr); // indirect eval
+                        delete (globalThis as any).RE2JS;
+                    } catch (error) {
+                        console.error(error);
+                        const assertMsg = [
+                            "Error attempting to evaluate the following rules:",
+                            "  rules:",
+                            `    - if: '${Utils._nodeToAtom(node)}'`,
+                            "as",
+                            "```javascript",
+                            `${evalStr}`,
+                            "```",
+                        ];
+                        assert(false, assertMsg.join("\n"));
+                    }
+                    return Boolean(res);
+                }
             }
-            const _rhs = JSON.stringify(rhs); // JSON.stringify for escaping `"`
-            const containsNonEscapedSlash = /(?<!\\)\//.test(_rhs);
-            const assertMsg = [
-                "Error attempting to evaluate the following rules:",
-                "  rules:",
-                `    - if: '${expandedEvalStr}'`,
-                "as rhs contains unescaped quote",
-            ];
-            assert(!containsNonEscapedSlash, assertMsg.join("\n"));
-            const flagsBinary = flagsToBinary(flags);
-            return `.matchRE2JS(RE2JS.compile(${_rhs}, ${flagsBinary})) ${_operator} null${remainingTokens}`;
-        });
-
-        // Scenario when RHS is surrounded by single/double-quotes
-        // https://regexr.com/85t0g
-        const pattern2 = /\s*(?<operator>=~|!~)\s*(["'])(?<rhs>(?:\\.|[^\\])*?)\2/g;
-        evalStr = evalStr.replaceAll(pattern2, (_, operator, __, rhs) => {
-            let _operator;
-            switch (operator) {
-                case "=~":
-                    _operator = "!=";
-                    break;
-                case "!~":
-                    _operator = "==";
-                    break;
-                default:
-                    throw operator;
+            const atom = Utils._nodeToAtom(node);
+            let res;
+            try {
+                (globalThis as any).RE2JS = RE2JS;
+                res = (0, eval)(atom);
+                delete (globalThis as any).RE2JS;
+            } catch {
+                assert(false, [
+                    "Error attempting to evaluate the following rules:",
+                    "  rules:",
+                    `    - if: '${ruleIf}'`,
+                    "as",
+                    "```javascript",
+                    `${atom}`,
+                    "```",
+                ].join("\n"));
             }
+            return Boolean(res);
+        };
 
-            const assertMsg = [
-                "RHS (${rhs}) must be a regex pattern. Do not rely on this behavior!",
-                "Refer to https://docs.gitlab.com/ee/ci/jobs/job_rules.html#unexpected-behavior-from-regular-expression-matching-with- for more info...",
-            ];
-            assert((/\/(.*)\/(\w*)/.test(rhs)), assertMsg.join("\n"));
-
-            const regex = /\/(?<pattern>.*)\/(?<flags>[igmsuy]*)/;
-            const _rhs = rhs.replace(regex, (_: string, pattern: string, flags: string) => {
-                const flagsBinary = flagsToBinary(flags);
-                return `RE2JS.compile("${pattern}", ${flagsBinary})`;
-            });
-            return `.matchRE2JS(${_rhs}) ${_operator} null`;
-        });
-
-        evalStr = evalStr.replaceAll(/null.matchRE2JS\(.+?\)\s*!=\s*null/g, "false");
-        evalStr = evalStr.replaceAll(/null.matchRE2JS\(.+?\)\s*==\s*null/g, "true");
-
-        evalStr = evalStr.trim();
-
-        let res;
+        let ast;
         try {
-            (globalThis as any).RE2JS = RE2JS;
-            res = (0, eval)(evalStr); // indirect eval
-            delete (globalThis as any).RE2JS;
+            ast = jsep(evalStr);
         } catch {
             const assertMsg = [
                 "Error attempting to evaluate the following rules:",
                 "  rules:",
-                `    - if: '${expandedEvalStr}'`,
+                `    - if: '${ruleIf}'`,
                 "as",
                 "```javascript",
                 `${evalStr}`,
                 "```",
             ];
             assert(false, assertMsg.join("\n"));
         }
-        return Boolean(res);
+        return walk(ast!);
     }
 
+
     static evaluateRuleExist (cwd: string, ruleExists: string[] | {paths: string[]} | undefined): boolean {
         if (ruleExists === undefined) return true;