Remove thecodingmachine/safe dependency

[SjorsO]

This PR drops the dependency on thecodingmachine/safe by absorbing the 7 functions that PHP-CSS-Parser actually uses (out of the 1,110+ functions that the dependency defines).

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

Metric	Before	After	Reduction
Uncompressed size (composer --no-dev)	13 MB	5.7 MB	7.3 MB (56% less)
Compressed size	5.6 MB	4.2 MB	1.4 MB (25% less)
Lines of text	274,141	44,676	84% less
Number of files	801	333	58% less

PHP-CSS-Parser is installed 3.8 million times per month, assuming one compressed download per install, then this PR would save ~5 TB of bandwidth monthly. Removing this many lines of text may also speed up IDE indexing, although that's hard to measure.

This PR is a breaking change, code like this will no longer work:

try {
    $parser->parse($css);
} catch (Safe\Exceptions\PcreException $e) {
    // ...
}

Code like this must be updated to catch Sabberworm\CSS\Safe\Exceptions\PcreException $e instead (although it seems unlikely to me that anyone is catching these exceptions specifically instead of Exception or Throwable).

I haven't removed the thecodingmachine/phpstan-safe-rule PHPStan rule, it still works as before.

I've excluded safe_functions.php from PHPStan both because the phpdocs don't match the inspections, and because it would warn about the unsafe functions in this file.

[coveralls]

coverage: 68.879% (-2.4%) from 71.315%
when pulling 8ea6bbf on SjorsO:drop-dependency
into 416f6a7 on MyIntervals:main.

[JakeQZ]

Thanks @SjorsO. I did not realize 'Safe' was such a large library. I think that may because it is made by autogenerating the code from the PHP documenation (on , and puts the entire lot in the DocBlocks. For example the DocBlock for preg_match_all() is 343 lines long. Also, it generates a separate set of functions for each supported PHP version (e.g. 8.1, 8.2, 8.3, 8.4, 8.5). Clearly the library could be improved to significantly reduce its size.

I've allowed the CI checks to run, which has picked up a few minor issues. Would you be able to resolve those?

This PR is a breaking change, code like this will no longer work:

try {
    $parser->parse($css);
} catch (Safe\Exceptions\PcreException $e) {
    // ...
}

We have only just started using 'Safe', and didn't consider that it may throw additional exceptions to be a breaking change.

Code like this must be updated to catch Sabberworm\CSS\Safe\Exceptions\PcreException $e instead (although it seems unlikely to me that anyone is catching these exceptions specifically instead of Exception or Throwable).

I agree; I think it's highly unlikely, particularly as we have only just started using 'Safe'.

[oliverklee]

If we copy those functions to our package, the added code becomes our responsibility and needs to follow the standards we have for new code: It needs proper test coverage, and we need to maintain it for the foreseeable future, including keeping it up to date with future PHP releases.

Someone needs to do this work. I’m inclined to not take this responsibility myself.

@SjorsO @JakeQZ Would any of you be willing to write the tests and shoulder maintaining the new functions?

[JakeQZ]

If we copy those functions to our package ...

It doesn't stop there. We are also now using the 'Safe' library in Emogrifier of which this package is a dependency. Emogrifier is used by the WooCommerce WordPress plugin which is installed on over 4 million websites.

I don't think this is a tenable solution, but I don't know what is.

Options might include:

• Apply fixes to the 'Safe' library so that
  • DocBlocks only include the necessary type annotations;
  • The set of generated functions is not duplicated for each PHP version.
• Revert all changes to use the 'Safe' library.

Obviously it is disappointing that the 'Safe' library turns out to be bloatware.

[JakeQZ]

Options might include:

Or create a separate package akin to 'Safe' that only includes the most commonly-used functions, and does not auto-generate the source.

[JakeQZ]

FWIW, I checked the size of the package I have installed with PHP 8.3. It's 6.31 MB, with 573 files. That does not seem like a lot to me, given that a typical photo from a phone is around 3 MB, and also considering that some popular WordPress plugins are much heftier:

• UpdraftPlus (1.22.11):
  • 25.7 MB
  • 1,638 files
• JetPack (10.3):
  • 25.6 MB
  • 1,822 files

Though I agree it's much more than it should be. I think the way forward is to get the issues fixed in the 'Safe' libraray:

• Do not include redundant DocBlock content; only include the type annotations;
• Avoid having a separate directory of functions for each PHP version;
• Have the fixes backported to the 1.x and 2.x branches.

[JakeQZ]

Though I agree it's much more than it should be. I think the way forward is to get the issues fixed in the 'Safe' libraray:

• Do not include redundant DocBlock content; only include the type annotations;
• Avoid having a separate directory of functions for each PHP version;
• Have the fixes backported to the 1.x and 2.x branches.

thecodingmachine/safe#706

[JakeQZ]

• Revert all changes to use the 'Safe' library.

That might be the best option for the time being, although a PITA.

[SjorsO]

• Revert all changes to use the 'Safe' library.

That might be the best option for the time being, although a PITA.

Personally I'd go with explicitly checking for false in places where it actually matters. I would avoid adding any dependency to this library, even a smaller or custom version of "thecodingmachine/safe".

I'm honestly not exactly sure why these "safe" functions are necessary. All other libraries I use don't seem to use them (including all of Laravel).

[oliverklee]

I'm honestly not exactly sure why these "safe" functions are necessary.

It helps us keep our code simple as we don't need to check for false when calling these functions, but can use the exceptions instead.

[SjorsO]

For reference, I've opened up a new PR with a different approach: #1484