From 383996641f2f16a992f16abd7783fe7d04953126 Mon Sep 17 00:00:00 2001 From: James Fry Date: Thu, 8 Oct 2026 12:37:19 -0600 Subject: [PATCH 1/3] Keep the functions compile() serialises out of coverage instrumentation toRegExp() and resolveAllowedConstant() are embedded in the code compile() generates through Function#toString(). When Istanbul instruments them, the text holds calls to coverage counters (`cov_xxx()`) that do not exist where the compiled code later runs, so four tests failed under `jest --coverage` (already the case on main). Shipped code is not affected: nothing instruments lib/. `istanbul ignore next` leaves the two functions as they are written. Co-Authored-By: Claude Sonnet 5.5 --- src/Provider/ConstantFunctionProvider.js | 1 + src/lib/pregPattern.js | 1 + 2 files changed, 2 insertions(+) diff --git a/src/Provider/ConstantFunctionProvider.js b/src/Provider/ConstantFunctionProvider.js index dcdc529..f46827e 100644 --- a/src/Provider/ConstantFunctionProvider.js +++ b/src/Provider/ConstantFunctionProvider.js @@ -10,6 +10,7 @@ import LogicException from "../LogicException"; * * Only own properties are followed, so inherited members ("constructor", "__proto__", ...) can never be reached. */ +/* istanbul ignore next: serialised with Function#toString(), so it must not hold the counters of a coverage run */ function resolveAllowedConstant(root, name, regexpSource, label) { if (typeof name !== 'string') { throw new Error(label + ' name must be a string.'); diff --git a/src/lib/pregPattern.js b/src/lib/pregPattern.js index 5315a1f..1b59bb0 100644 --- a/src/lib/pregPattern.js +++ b/src/lib/pregPattern.js @@ -15,6 +15,7 @@ import SyntaxError from "../SyntaxError"; * @returns {RegExp} * @throws {Error} with a message starting with "Regexp ... is not valid" */ +/* istanbul ignore next: serialised with Function#toString(), so it must not hold the counters of a coverage run */ export function toRegExp(pattern) { var invalid = function (reason) { var e = new Error('Regexp "' + pattern + '" passed to "matches" is not valid: ' + reason); From 26ef69424019c30f02d12465564e0ef7f70360bd Mon Sep 17 00:00:00 2001 From: James Fry Date: Thu, 8 Oct 2026 12:37:20 -0600 Subject: [PATCH 2/3] Resolve the sprintf-js Dependabot alert by moving js-yaml to 4.x Dependabot alert #89 (GHSA-hp3w-g68c-fv3c, DoS through unbounded precision in sprintf-js <= 1.1.3) has no patched release: 1.1.3 is the latest. The package is only reached by the test tooling, through jest > @jest/core > @jest/transform > babel-plugin-istanbul > @istanbuljs/load-nyc-config > js-yaml 3.x > argparse 1.x > sprintf-js, and js-yaml 4 (argparse 2) no longer depends on it. load-nyc-config only calls `require('js-yaml').load(...)` to read a .nycrc.yaml, which has the same meaning in js-yaml 4. Checked: the tests, a coverage run, loading a .nycrc.yaml through load-nyc-config, and both builds. The lockfile loses sprintf-js and esprima and changes only js-yaml and argparse. Nothing that ships is affected (`npm audit --omit=dev` was clean). Not addressed: braces <= 3.0.3 (GHSA-vfj7-8cjw-p6xm), which is also unpatched and comes from jest's own micromatch. GitHub auto-dismissed that alert (#88). Co-Authored-By: Claude Sonnet 5.5 --- package-lock.json | 51 +++++++++++++++++------------------------------ package.json | 3 +++ 2 files changed, 21 insertions(+), 33 deletions(-) diff --git a/package-lock.json b/package-lock.json index 3ad422c..250c082 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3609,14 +3609,11 @@ } }, "node_modules/argparse": { - "version": "1.0.10", - "resolved": "https://registry.npmjs.org/argparse/-/argparse-1.0.10.tgz", - "integrity": "sha512-o5Roy6tNG4SL/FOkCAN6RzjiakZS25RLYFrcMttJqbdd8BWrnA+fGz57iN5Pb06pvBGvl5gQ0B48dJlslXvoTg==", + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/argparse/-/argparse-2.0.1.tgz", + "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==", "dev": true, - "license": "MIT", - "dependencies": { - "sprintf-js": "~1.0.2" - } + "license": "Python-2.0" }, "node_modules/babel-jest": { "version": "30.2.0", @@ -4231,20 +4228,6 @@ "node": ">=8" } }, - "node_modules/esprima": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/esprima/-/esprima-4.0.1.tgz", - "integrity": "sha512-eGuFFw7Upda+g4p+QHvnW0RyTX/SVeJBDM/gCtMARO0cLuT2HcEKnTPvhjV6aGeqrCB/sbNop0Kszm0jsaWU4A==", - "dev": true, - "license": "BSD-2-Clause", - "bin": { - "esparse": "bin/esparse.js", - "esvalidate": "bin/esvalidate.js" - }, - "engines": { - "node": ">=4" - } - }, "node_modules/estree-walker": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/estree-walker/-/estree-walker-2.0.2.tgz", @@ -5618,14 +5601,23 @@ "license": "MIT" }, "node_modules/js-yaml": { - "version": "3.15.2", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.15.2.tgz", - "integrity": "sha512-6EuL879VkRA+1Cz578mKMiKvjPNEuk6+r1JaFzoSWejZmtf7xWbIyw1e3KkxlkzTIt9Taw6JBhEppG7utc1P+w==", + "version": "4.3.2", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.2.tgz", + "integrity": "sha512-SFNOvSJ+Dgf/9An904Yx+CgSlIPCkIpao4qo51lpee25TIRejdH3rhR4EZMGoNx3/TP3O+wzWuiTFl4sqbltzA==", "dev": true, + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/puzrin" + }, + { + "type": "github", + "url": "https://github.com/sponsors/nodeca" + } + ], "license": "MIT", "dependencies": { - "argparse": "^1.0.7", - "esprima": "^4.0.0" + "argparse": "^2.0.1" }, "bin": { "js-yaml": "bin/js-yaml.js" @@ -6424,13 +6416,6 @@ "source-map": "^0.6.0" } }, - "node_modules/sprintf-js": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/sprintf-js/-/sprintf-js-1.0.3.tgz", - "integrity": "sha512-D9cPgkvLlV3t3IzL0D0YLvGA9Ahk4PcvVwUbN0dSGr1aP0Nrt4AEnTUbuGvquEC0mA64Gqt1fzirlRs5ibXx8g==", - "dev": true, - "license": "BSD-3-Clause" - }, "node_modules/stack-utils": { "version": "2.0.6", "resolved": "https://registry.npmjs.org/stack-utils/-/stack-utils-2.0.6.tgz", diff --git a/package.json b/package.json index f8aac56..ec0289a 100644 --- a/package.json +++ b/package.json @@ -48,5 +48,8 @@ "@rollup/plugin-terser": "^1.0.0", "jest": "^30.0.5", "rollup": "^4.0.0" + }, + "overrides": { + "js-yaml": "^4.1.0" } } From 2e518ac80b7fa0e787434c34333a3a8280fd9f7b Mon Sep 17 00:00:00 2001 From: James Fry Date: Thu, 8 Oct 2026 12:37:47 -0600 Subject: [PATCH 3/3] Add CI for pull requests Until now the tests only ran when a release was published, so a pull request had no checks at all. The CI workflow runs on pull requests and on pushes to main: - Tests on Node 20, 22 and 24. - Tests with coverage: instrumented sources have to work too (this is what caught the functions compile() serialises with Function#toString()). - Build and smoke test of what is published, lib/ (Babel) and the minified browser bundle (scripts/smoke-build.cjs): the serialised functions only work while the build leaves them self-contained. - TypeScript definitions (`tsc --strict`; typescript is now a devDependency). - The PHP generated by dumpCompiled() evaluated by Symfony's own CompiledExpressionLanguage, against a Symfony commit pinned in the workflow, with the PHP of the runner. - `npm audit --omit=dev --audit-level=high`: the runtime dependencies only, as the development tools have advisories that no release fixes yet. - A final `CI` job aggregating the others, to require in branch protection. New npm scripts: test:coverage, test:types, test:build. The README's Contributing section lists them. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/ci.yml | 155 +++++++++++++++++++++++++++++++++++++++ README.md | 11 ++- package-lock.json | 17 ++++- package.json | 8 +- scripts/smoke-build.cjs | 115 +++++++++++++++++++++++++++++ 5 files changed, 302 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 scripts/smoke-build.cjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..dd79e1d --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,155 @@ +name: CI + +on: + pull_request: + push: + branches: [main] + workflow_dispatch: + +permissions: + contents: read + +# a new push to a pull request replaces the run in progress +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +env: + # The commit of symfony/expression-language the PHP dump is checked against. It is pinned so that a change upstream cannot turn + # this workflow red by itself: move it on purpose (and regenerate the fixtures, see "Contributing" in the README). + SYMFONY_EXPRESSION_LANGUAGE_SHA: 4ac922a1172448c08599fa275c4965e0418ff664 + +jobs: + test: + name: Tests (Node ${{ matrix.node }}) + runs-on: ubuntu-latest + timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + node: [20, 22, 24] + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: ${{ matrix.node }} + cache: npm + + - run: npm ci --no-audit --no-fund + + - run: npm test + + coverage: + # Tests run again with the sources instrumented: this catches code that breaks when it is (the functions that compile() + # serialises with Function#toString(), for instance, must not hold the counters of a coverage run). + name: Tests with coverage + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + + - run: npm ci --no-audit --no-fund + + - run: npm run test:coverage -- --coverageReporters=text-summary --coverageReporters=text + + build: + # The sources are tested above; this runs what is published: the Babel build (lib/) and the minified browser bundle (dist/). + name: Build and smoke-test the bundles + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + + - run: npm ci --no-audit --no-fund + + - run: npm run build:all + + - run: npm run test:build + + types: + name: TypeScript definitions + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + + - run: npm ci --no-audit --no-fund + + - run: npm run test:types + + php: + # The PHP that dumpCompiled() generates has to be accepted by Symfony's own CompiledExpressionLanguage, and behave like the file + # Symfony dumps itself. Needs the PHP of the runner and a checkout of the component (the Lexer, Parser and Nodes: no Composer). + name: PHP dump against Symfony's loader + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + + - run: npm ci --no-audit --no-fund + + - name: Fetch Symfony's ExpressionLanguage at the pinned commit + run: | + git init --quiet "$RUNNER_TEMP/symfony-expression-language" + cd "$RUNNER_TEMP/symfony-expression-language" + git remote add origin https://github.com/symfony/expression-language.git + git fetch --quiet --depth 1 origin "$SYMFONY_EXPRESSION_LANGUAGE_SHA" + git checkout --quiet FETCH_HEAD + test "$(git rev-parse HEAD)" = "$SYMFONY_EXPRESSION_LANGUAGE_SHA" + + - name: Make sure PHP is there + run: | + command -v php >/dev/null || { sudo apt-get update -qq && sudo apt-get install -y -qq php-cli; } + php --version + + - name: Evaluate the dumped PHP with Symfony's loader + run: npm test -- CompiledExpressionLanguage + env: + SYMFONY_EXPRESSION_LANGUAGE_PATH: ${{ runner.temp }}/symfony-expression-language + + audit: + # Only what is installed by the people who use the package. The development tools are not in scope here: they have advisories + # that no release fixes yet (see the Dependabot alerts), and they never reach a user. + name: Audit of the runtime dependencies + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + + - run: npm audit --omit=dev --audit-level=high + + ci: + # The one check to require in the branch protection rules: the names of the matrix jobs above can change. + name: CI + if: ${{ always() }} + needs: [test, coverage, build, types, php, audit] + runs-on: ubuntu-latest + steps: + - name: Every job succeeded + run: test "${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }}" = "false" diff --git a/README.md b/README.md index a19eb9a..e6fb25a 100644 --- a/README.md +++ b/README.md @@ -568,7 +568,16 @@ JavaScript and PHP are different languages, so a few things cannot be identical ## Contributing -Run the tests with `npm test`. +Run the tests with `npm test`. The pull request checks ([ci.yml](.github/workflows/ci.yml)) run, and you can run locally: + +| Command | Checks | +|---|---| +| `npm test` | the unit tests, on the sources | +| `npm run test:coverage` | the same tests with the sources instrumented (code that `compile()` serialises with `Function#toString()` must stay out of it: see the `istanbul ignore` hints) | +| `npm run test:types` | the TypeScript definitions (`types/test-types.ts`, `tsc --strict`) | +| `npm run build:all && npm run test:build` | the Babel build and the minified browser bundle, which are what gets published | + +The `CI` workflow also audits the runtime dependencies (`npm audit --omit=dev`) and evaluates the PHP that `dumpCompiled()` generates with Symfony's own loader, against a Symfony commit pinned in `SYMFONY_EXPRESSION_LANGUAGE_SHA`: move it on purpose, together with the fixtures. `src/__tests__/SymfonyParity.test.js` replays `src/__tests__/fixtures/symfony-corpus.json` and compares it with `src/__tests__/fixtures/symfony-parity.json`, which holds what Symfony's own code answers. After adding expressions to diff --git a/package-lock.json b/package-lock.json index 250c082..a5836bc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,7 +24,8 @@ "@rollup/plugin-node-resolve": "^16.0.0", "@rollup/plugin-terser": "^1.0.0", "jest": "^30.0.5", - "rollup": "^4.0.0" + "rollup": "^4.0.0", + "typescript": "~5.6.3" } }, "node_modules/@babel/cli": { @@ -6642,6 +6643,20 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/typescript": { + "version": "5.6.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.6.3.tgz", + "integrity": "sha512-hjcS1mhfuyi4WW8IWtjP7brDrG2cuDZukyrYrSauoXGNgx0S7zceP07adYkJycEr56BOUTNPzbInooiN3fn1qw==", + "dev": true, + "license": "Apache-2.0", + "bin": { + "tsc": "bin/tsc", + "tsserver": "bin/tsserver" + }, + "engines": { + "node": ">=14.17" + } + }, "node_modules/undici-types": { "version": "7.16.0", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.16.0.tgz", diff --git a/package.json b/package.json index ec0289a..c0d1d38 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,10 @@ "build:browser": "rollup -c && cp types/index.d.ts dist/index.d.ts", "build:all": "npm run build && npm run build:browser", "prepublishOnly": "npm run build:all", - "test": "jest" + "test": "jest", + "test:coverage": "jest --coverage", + "test:types": "tsc --noEmit --strict --target es2020 --moduleResolution node --esModuleInterop --skipLibCheck types/test-types.ts", + "test:build": "node scripts/smoke-build.cjs lib/index.js dist/expression-language.min.js" }, "jest": { "testPathIgnorePatterns": [ @@ -47,7 +50,8 @@ "@rollup/plugin-node-resolve": "^16.0.0", "@rollup/plugin-terser": "^1.0.0", "jest": "^30.0.5", - "rollup": "^4.0.0" + "rollup": "^4.0.0", + "typescript": "~5.6.3" }, "overrides": { "js-yaml": "^4.1.0" diff --git a/scripts/smoke-build.cjs b/scripts/smoke-build.cjs new file mode 100644 index 0000000..07ba71d --- /dev/null +++ b/scripts/smoke-build.cjs @@ -0,0 +1,115 @@ +#!/usr/bin/env node +/* + * Smoke test of a BUILT bundle: `lib/index.js` (Babel, CommonJS) or `dist/expression-language.min.js` (Rollup + Terser). + * + * The unit tests run on the sources. This script runs the code that is actually published, which matters because a few features + * serialise functions with Function#toString() into the code compile() produces (a `matches` pattern known at runtime, the + * resolver of constant() / enum()). That only works while the build leaves those functions self-contained, which a transpiler + * or a minifier can silently stop doing. + * + * Usage: + * + * node scripts/smoke-build.cjs lib/index.js dist/expression-language.min.js + * + * Exits with 1 when a check of any of the given bundles fails. + */ + +const path = require('path'); + +const targets = process.argv.slice(2); +if (targets.length === 0) { + console.error('Usage: node scripts/smoke-build.cjs [...]'); + process.exit(2); +} + +function checksFor(lib) { + globalThis.SmokeApp = {Roles: {ADMIN: 'ROLE_ADMIN'}}; + const el = new lib.ExpressionLanguage(null, [new lib.ConstantFunctionProvider(['SmokeApp.Roles.*', 'Math.PI'])]); + // compiled code of the default semantics calls the runtime + const run = (language, expression, names = [], values = []) => + new Function('__runtime', ...names, 'return ' + language.compile(expression, names) + ';')(lib.CompileRuntime, ...values); + const throwsMatching = (callback, pattern) => { + try { + callback(); + } catch (e) { + return pattern.test(e.message); + } + + return false; + }; + + return { + 'a matches pattern known at runtime survives the build (toRegExp is serialised)': () => run(el, 's matches p', ['s', 'p'], ['ABC', '/^a/i']) === true, + 'an invalid pattern known at runtime is reported': () => throwsMatching(() => run(el, 's matches p', ['s', 'p'], ['a', 'nope']), /is not valid/), + 'constant() resolver survives the build (it is serialised)': () => run(el, 'constant("SmokeApp.Roles.ADMIN")') === 'ROLE_ADMIN', + 'compiled constant() refuses what is not allowed': () => throwsMatching(() => run(el, 'constant("process")'), /not allowed/), + 'evaluate() constant()': () => el.evaluate('constant("Math.PI")') === Math.PI, + 'constant() does not exist without a provider': () => throwsMatching(() => new lib.ExpressionLanguage().evaluate('constant("Math.PI")'), /does not exist/), + 'caseInsensitiveStringOperators option': () => new lib.ExpressionLanguage(null, [], {caseInsensitiveStringOperators: true}).evaluate('"ABC" contains "b"') === true, + 'null-safe array access': () => el.evaluate('a?.[0]', {a: null}) === null && el.evaluate('a?.[0]', {a: [4]}) === 4, + 'min / max / count': () => el.evaluate('min([3,1,2])') === 1 && el.evaluate('max(1,5)') === 5 && el.evaluate('count([1,2])') === 2, + 'semantics: Symfony\'s by default, JavaScript\'s and portable on request': () => { + const portable = new lib.ExpressionLanguage(null, [], {semantics: 'portable'}); + const refused = throwsMatching(() => portable.evaluate('"5" + 1'), /not portable/); + let isPortabilityError = false; + try { + portable.evaluate('"5" + 1'); + } catch (e) { + isPortabilityError = e instanceof lib.PortabilityError; + } + + return new lib.ExpressionLanguage().evaluate('"5" + 1') === 6 + && new lib.ExpressionLanguage(null, [], {semantics: 'js'}).evaluate('"5" + 1') === '51' + && refused && isPortabilityError; + }, + 'compiled code of every semantics runs': () => ['symfony', 'js', 'portable'].every((semantics) => + run(new lib.ExpressionLanguage(null, [], {semantics}), 'a * b > 10 and a != b', ['a', 'b'], [3, 4]) === true), + 'CompiledExpressionLanguage: JavaScript dump, loaded and evaluated': () => { + const providers = () => [new lib.StringProvider()]; + const source = new lib.CompiledExpressionLanguage(new lib.ExpressionLanguage(null, providers())) + .dumpCompiled(['strtoupper(n) ~ "!"', 'a ?: b', 'x?.[0]'], {format: 'expression'}); + const compiled = new lib.CompiledExpressionLanguage(new lib.ExpressionLanguage(null, providers()), source); + + return compiled.evaluate('strtoupper(n) ~ "!"', {n: 'ab'}) === 'AB!' + && compiled.evaluate('a ?: b', {a: 0, b: 4}) === 4 + && compiled.evaluate('x?.[0]', {x: null}) === null; + }, + 'CompiledExpressionLanguage: PHP dump': () => /return \(\$a \+ 1\.0\);/.test(new lib.CompiledExpressionLanguage(new lib.ExpressionLanguage()).dumpCompiled(['a + 1.0'], {target: 'php'})), + 'public exports': () => [ + 'ExpressionLanguage', 'ConstantFunctionProvider', 'CompiledExpressionLanguage', 'PhpCompiler', 'CompileRuntime', + 'PortabilityError', 'DivisionByZeroError', 'LogicException', 'SyntaxError', + 'CASE_INSENSITIVE_STRING_OPERATORS', 'SEMANTICS_JS', 'SEMANTICS_PORTABLE', + ].every((name) => name in lib), + }; +} + +let failed = 0; +for (const target of targets) { + const file = path.resolve(target); + console.log(`\n${target}`); + + let lib; + try { + lib = require(file); + } catch (e) { + console.log(` FAIL could not be loaded: ${e.message}`); + failed++; + continue; + } + + for (const [name, check] of Object.entries(checksFor(lib))) { + let ok = false; + try { + ok = check(); + } catch (e) { + console.log(` threw: ${e.message}`); + } + if (!ok) { + failed++; + } + console.log(` ${ok ? 'PASS' : 'FAIL'} ${name}`); + } +} + +console.log(failed ? `\n${failed} check(s) failed.` : '\nAll checks passed.'); +process.exit(failed ? 1 : 0);