Remove `thecodingmachine/safe` dependency by SjorsO · Pull Request #1484 · MyIntervals/PHP-CSS-Parser

SjorsO · 2026-01-27T18:56:46Z

This PR drops the dependency on thecodingmachine/safe.

Removing this dependency significantly reduces the size of PHP-CSS-Parser:

Metric	Before	After	Reduction
Uncompressed size (composer --no-dev)	2.6 MB	0.9 MB	1.7 MB (65% less)
Compressed size	0.58 MB	0.25 MB	0.33 MB (56% less)
Lines of text	91,111	29,309	68% less
Number of files	534	216	60% less

Removing this dependency also:

Reduces supply chain attack risk (worth taking seriously after the recent Axios incident)
Saves ~2TB of bandwidth per month (assuming 1 compressed download for 6.8 million installs this month)
Speeds up IDE indexing by removing ~61,000 LoC and 318 files

SjorsO · 2026-01-27T18:58:57Z

-        if (preg_match($expression, $input, $matches, PREG_OFFSET_CAPTURE) !== 1) {
+        /** @phpstan-ignore theCodingMachineSafe.function */
+        if (\preg_match($expression, $input, $matches, PREG_OFFSET_CAPTURE) !== 1) {
            throw new UnexpectedTokenException($expression, $this->peek(5), 'expression', $this->lineNumber);


No need to check for false here. If this preg_match returns false then we'll already throw an exception

SjorsO · 2026-01-27T18:59:36Z

        // Whitespace can't be adjusted within an attribute selector, as it would change its meaning
-        $this->selector = !$hasAttribute ? preg_replace('/\\s++/', ' ', $selector) : $selector;
+        /** @phpstan-ignore theCodingMachineSafe.function */
+        $this->selector = !$hasAttribute ? \preg_replace('/\\s++/', ' ', $selector) : $selector;


preg_replace cannot return false. The dependency has preg_replace as a special case.

I think it should be fine to use the normal version of preg_replace

According to https://www.php.net/manual/en/function.preg-replace.php#refsect1-function.preg-replace-returnvalues, it can return a string, an array or null.

You're right, I missed that. I've pushed a commit that checks for null

coveralls · 2026-01-27T19:01:07Z

coverage: 71.517% (-1.1%) from 72.604% — SjorsO:explicitly-check-for-false into MyIntervals:main

SjorsO · 2026-01-27T19:05:29Z

+                            /** @phpstan-ignore theCodingMachineSafe.function */
                            preg_match('/\\s/', $parserState->peek(1, -1)) !== 1
+                            /** @phpstan-ignore theCodingMachineSafe.function */
                            || preg_match('/\\s/', $parserState->peek(1, 1)) !== 1


if either of these returns false we'll already throw an exception

JakeQZ · 2026-01-28T00:44:33Z

Appreciate your efforts, @SjorsO (code changes look solid), but I honestly don't think this is the best way forward. I think we need to resolve the bloat in the in the 'Safe' package, for which I created thecodingmachine/safe#706, and have had a response.

SjorsO · 2026-01-28T03:57:17Z

... but I honestly don't think this is the best way forward. I think we need to resolve the bloat in the in the 'Safe' package

Given how widely used PHP-CSS-Parser and emogrifier are, I consider getting rid of any dependency a huge win.

Even if thecodingmachine/safe manages to slim down and remove all the docblocks and other PHP versions, it still autoloads 79 files that contain 1100+ functions (!!), I find this hard to justify, especially considering the alternative is simply checking for false.

I think this PR is a nice middle ground: no dependency that thousands of people have to install, but still a PHPStan rule that warns about unsafe usage during development.

JakeQZ · 2026-01-29T01:19:29Z

thecodingmachine/safe#660 is another reason to avoid using this dependency. In WordPress, each plugin (or theme) brings in its own versions of each dependency, which leads to conflicts, though, in fairness, that is an issue with WordPress.

oliverklee · 2026-02-09T21:36:07Z

Marking this as draft as we're currently working to drastically shrink that package in thecodingmachine/safe#706.

We should revisit this PR here once a new version of Safe has been released.

JakeQZ · 2026-02-15T17:11:13Z

Marking this as draft as we're currently working to drastically shrink that package in thecodingmachine/safe#706.

We should revisit this PR here once a new version of Safe has been released.

With version 3.4, the size is reduced from 6.31Mb to 1.58Mb.

To put it in context, this is comparable to the size of one or two photos in HD quality.

Another factor that impacts website managers is the number of individual files. Hosting providers using cPanel usually impose a limit on the number of file handles. The new version reduces the number of files from 573 to 318.

The new version is only availble for PHP versions >=8.1, and I don't expect will be backported - the older PHP versions are already EOL.

Where do we go from here?

I have no strong opinion, but don't see the download size and number of files as a significant impact, and would prefer to use the library to keep the code robust and clean, without having to implement additional checks and tests.

oliverklee · 2026-02-15T17:17:05Z

but don't see the download size and number of files as a significant impact, and would prefer to use the library to keep the code robust and clean, without having to implement additional checks and tests.

I fully agree.

SjorsO · 2026-02-16T02:34:30Z

For the record, I still strongly feel that adding 1.58mb and 318 files to thousands (millions?) of applications is way too much.

Removing this dependency is a good step to prevent everyone's vendor directory from turning into a second node_modules.

SjorsO · 2026-04-19T09:19:13Z

I still hope you'll reconsider and remove this dependency. thecodingmachine/safe still adds 318 extra files and 2.5MB to every project that uses DomPDF. Removing it would also reduce supply chain attack risk (something worth taking seriously after the recent Axios incident).

I'm happy to resolve any merge conflicts and re-review the changes if you decide to move forward with this PR.

oliverklee

I'm still hesitant about this.

For this PR to possibly get in, it still needs quite a lot of work:

We should also remove the thecodingmachine/phpstan-safe-rule dependency (as it's only relevant for projects using thecodingmachine/safe).
We need to drop the added @phpstan-ignore annotations.
So far, we've relied on Safe throwing exceptions when things go wrong. If we do that on our own, we'll need to fully cover the added exceptions with tests (or assert if we know some code will not fail).

@SjorsO Would you be willing to do the work? (Particularly, the test coverage will take some time.)

If so, we should split this up into smaller PRs, e.g., one PR per file.

@JakeQZ Would you be okay with this (assuming taht @SjorsO will do the heavy lifting)?

SjorsO · 2026-04-19T11:46:50Z

We should also remove the thecodingmachine/phpstan-safe-rule dependency (as it's only relevant for projects using thecodingmachine/safe).

We need to drop the added @PHPStan-Ignore annotations.

Keeping the thecodingmachine/phpstan-safe-rule gives us the best of both worlds: as a dev dependency, it doesn't affect end users, and it alerts us whenever we use an unsafe function. When the rule flags a new unsafe call, we handle the false return explicitly and add a @phpstan-ignore comment to signal "this unsafe call is being handled properly."

Keeping this PHPStan rule guarantees we won't miss any unsafe function calls in the future. Are you sure you want to remove it as well?

oliverklee · 2026-04-19T11:52:28Z

Keeping this PHPStan rule guarantees we won't miss any unsafe function calls in the future. Are you sure you want to remove it as well?

Ah, I hadn't though of that. I'll need to think about that.

JakeQZ · 2026-04-20T01:41:27Z

@JakeQZ Would you be okay with this (assuming taht @SjorsO will do the heavy lifting)?

The main areas of concern are preg_replace etc., where failure is unpredictable, and very much based on the $subject.

I added a class to Emogrifier to cater for this, which was removed in MyIntervals/emogrifier#1514 (I can't find when it was added because GitHub is a bit rubbish),

Perhaps that class can be reinstated for both projects, And somehow shared. Or an alternative solution be worked out.

I don't expect any of the other inbuilt functions to fail at all. Some assert()s will surely sort those out.

SjorsO · 2026-04-28T08:35:25Z

Keeping this PHPStan rule guarantees we won't miss any unsafe function calls in the future. Are you sure you want to remove it as well?

Ah, I hadn't though of that. I'll need to think about that.

Let me know once you've decided, then I'll get started

oliverklee · 2026-04-28T08:37:42Z

Let me know once you've decided, then I'll get started

Let's drop the PHPStan ruleset.

oliverklee · 2026-04-29T16:36:04Z

Let's drop the PHPStan ruleset.

But only after we've dropped the Safe package, i.e., after the last PR from a series of PRs (to make them faster to review) resulting from this PR here.

SjorsO · 2026-07-09T04:30:26Z

+            'missing space before -' => ['calc(100%- 20px)'],
+            'missing space before +' => ['calc(100%+ 20px)'],
+            'missing space after -' => ['calc(100% -20px)'],
+            'missing space after +' => ['calc(100% +20px)'],


These add test coverage for the two preg_match calls in CalcFunction (the second preg_match previously had no test coverage)

SjorsO · 2026-07-09T04:59:33Z

@oliverklee This is ready for review. I've updated the PR description with the impact removing this dependency will have.

I hope it's alright that I've left this as a single PR. The changes should be straightforward enough to review together.

23 call sites are changed:

17 are simple "grab result + false check"
3 already threw an exception if the result was false, so no changes were needed
3 involve slightly more work on a loop condition:
- ParserState's while loop
- CssString's while loop
- Value's do...while loop

I wasn't able to add tests for most of the changed preg_match calls because it only returns false if:

The pattern doesn't compile (impossible here, patterns are hardcoded)
The backtrack/recursion limit is hit (impossible here, none of the patterns do backtracking)
The /u flag is set and the subject is malformed UTF-8 (only two cases, I've added test coverage for those)

A follow-up PR should remove the thecodingmachine/phpstan-safe-rule dev dependency and the PHPStan annotations

Copilot

Pull request overview

This PR removes the thecodingmachine/safe dependency by switching to native PHP functions and adding explicit handling for failure return values, with corresponding test and static-analysis updates.

Changes:

Drop thecodingmachine/safe from composer.json and update codepaths previously relying on Safe wrappers.
Add/adjust runtime checks around functions like preg_*, iconv, class_alias, and file_get_contents.
Extend unit tests for calc operator whitespace handling and invalid UTF-8 behavior; update PHPStan ignore configuration accordingly.

Reviewed changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/Unit/Value/CalcFunctionTest.php	Adds more invalid `calc()` syntax cases around operator whitespace.
tests/Unit/Parsing/ParserStateTest.php	Adds coverage for invalid UTF-8 behavior in `ParserState`.
tests/RuleSet/LenientParsingTest.php	Replaces Safe `file_get_contents` usage with native `\file_get_contents`.
tests/ParserTest.php	Replaces Safe `opendir`/`file_get_contents` usage with native calls.
src/Value/Value.php	Adds explicit handling for `preg_match` failure while parsing unicode ranges.
src/Value/Size.php	Adds explicit handling for `preg_match`/`preg_replace` failure in rendering.
src/Value/CSSString.php	Adds explicit handling for `preg_match` failure when parsing unquoted strings.
src/Value/CalcFunction.php	Updates whitespace validation around `+`/`-` operators (still uses native `preg_match`).
src/RuleSet/RuleContainer.php	Replaces Safe `class_alias` with native call and explicit failure handling.
src/Rule/Rule.php	Replaces Safe `class_alias` with native call and explicit failure handling.
src/Property/Selector/SpecificityCalculator.php	Adds explicit handling for `preg_match_all` returning false.
src/Property/Selector/CompoundSelector.php	Adds explicit handling for `preg_match` returning false in selector validation.
src/Property/Selector.php	Adds explicit handling for `preg_match` returning false in selector validation.
src/Property/Declaration.php	Adds explicit handling for `preg_match` returning false when parsing font delimiter rules.
src/Parsing/ParserState.php	Replaces Safe `preg_*`/`iconv` usage with native calls and explicit failure handling (partial in `consumeExpression`).
src/CSSList/CSSList.php	Adds explicit handling for `preg_match` returning false in identifier checks.
composer.json	Removes `thecodingmachine/safe` requirement (keeps `ext-iconv`).
CHANGELOG.md	Documents removal of the Safe dependency.
Build/phpstan/phpstan.neon	Adds ignore rule for `theCodingMachineSafe.function` identifier in tests.
bin/quickdump.php	Switches to native `\file_get_contents` with explicit failure handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Remove thecodingmachine/safe dependency

2332f66

SjorsO commented Jan 27, 2026

View reviewed changes

SjorsO mentioned this pull request Jan 27, 2026

Remove thecodingmachine/safe dependency #1482

Closed

check preg_replace for null results

2320afc

oliverklee changed the title ~~Remove thecodingmachine/safe dependency (2)~~ Remove thecodingmachine/safe dependency Jan 28, 2026

oliverklee marked this pull request as draft February 9, 2026 21:34

oliverklee reviewed Apr 19, 2026

View reviewed changes

oliverklee assigned SjorsO Apr 19, 2026

SjorsO added 4 commits July 9, 2026 05:49

Merge upstream/main into explicitly-check-for-false

4c378c7

Update ParserState.php

32896dc

Update CalcFunction.php

c2b0583

Update CalcFunctionTest.php

49cf6e7

SjorsO commented Jul 9, 2026

View reviewed changes

SjorsO marked this pull request as ready for review July 9, 2026 04:30

Update ParserStateTest.php

67ad74e

oliverklee requested a review from Copilot July 9, 2026 07:28

Copilot started reviewing on behalf of oliverklee July 9, 2026 07:29 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread tests/Unit/Parsing/ParserStateTest.php

Comment thread src/Parsing/ParserState.php

Comment thread src/Value/CalcFunction.php

SjorsO added 2 commits July 9, 2026 10:01

fix tests on php 7.2

3a5543e

make phpcs happy by reducing line length

fe2e76b

Uh oh!

Conversation

SjorsO commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SjorsO Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

SjorsO Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oliverklee Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

SjorsO Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SjorsO Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

JakeQZ commented Jan 28, 2026

Uh oh!

SjorsO commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JakeQZ commented Jan 29, 2026

Uh oh!

oliverklee commented Feb 9, 2026

Uh oh!

JakeQZ commented Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oliverklee commented Feb 15, 2026

Uh oh!

SjorsO commented Feb 16, 2026

Uh oh!

SjorsO commented Apr 19, 2026

Uh oh!

oliverklee left a comment

Choose a reason for hiding this comment

Uh oh!

SjorsO commented Apr 19, 2026

Uh oh!

oliverklee commented Apr 19, 2026

Uh oh!

JakeQZ commented Apr 20, 2026

Uh oh!

SjorsO commented Apr 28, 2026

Uh oh!

oliverklee commented Apr 28, 2026

Uh oh!

oliverklee commented Apr 29, 2026

Uh oh!

SjorsO Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

SjorsO commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SjorsO commented Jan 27, 2026 •

edited

Loading

SjorsO Jan 27, 2026 •

edited

Loading

coveralls commented Jan 27, 2026 •

edited

Loading

SjorsO commented Jan 28, 2026 •

edited

Loading

JakeQZ commented Feb 15, 2026 •

edited

Loading

SjorsO commented Jul 9, 2026 •

edited

Loading