Bug: long heredoc causes missing classes in classmap generation

[jrfnl]

The following deprecation notice shows during the generation of an optimized classmap on PHP 8.1:

Deprecation Notice: preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in phar:///usr/local/bin/composer/src/Composer/Autoload/ClassMapGenerator.php:251

Steps to reproduce:

• Clone the phpDocumentor repository
• Run composer install --no-progress --prefer-dist --optimize-autoloader while on PHP 8.1(beta1)

The error can be seen in the "Install Composer dependencies & cache dependencies" step in this CI transcript: https://github.com/jrfnl/phpDocumentor/runs/3241452949?check_suite_focus=true

This is caused by a bug in the regex for which I have a fix lined up. I'm still trying to verify, however, whether there is an existing test which already covers this or not.

This bug has probably been around since PHP 7.3 when PHP switched to PCRE 2.0, which contains stricter regex validation (preg_replace() returns null when there is an error in the regex).

[Seldaek]

There definitely should be tests covering this in AutoloadGeneratorTest. I am quite surprised by this. As far as I can tell the regex above works fine https://regex101.com/r/TxfKNj/2 - do you have more details about the PCRE 2 error?

[jrfnl]

@Seldaek I suspect it's a syntax error in the regex itself - in the code sample you have in regex101, you have fixed it already (removed the double \\ for the backreferences).

Per the PHP manual:

If matches are found, the new subject will be returned, otherwise subject will be returned unchanged or null if an error occurred.

A CI test run for just the AutoloadGeneratorTest and the ClassMapGeneratorTest classes does not show the expected deprecation notice, so I'm going to have to dig into phpDocumentor to see if I can find the exact code needed to create a reproduction sample to add to the tests.

[stof]

@jrfnl the double \ is about the PHP string escaping. They don't end up in the regexp itself.

[jrfnl]

@stof Thanks, I know, but that depends on the amount of \ and whether it needs escaping...

[jrfnl]

Okay, so I've done some more test runs and it's a Backtrack limit exhausted error underlying the notice.

Will do some more digging.

[stof]

@jrfnl it would be great to identify the file for which this limit gets exhausted, to reproduce the issue.

Also, can you try replacing the (?:\s*) (just before the second backreference) with [ \t]* to see if it improves the situation ?

[jrfnl]

Bear with me.. trying some things out now and debugging... will get back with an update in a little while.

[jrfnl]

Problem file is phpDocumentor/vendor/fzaninotto/faker/src/Faker/Provider/nl_BE/Text.php: https://github.com/fzaninotto/Faker/blob/master/src/Faker/Provider/nl_BE/Text.php

[jrfnl]

Lovely story in the heredoc 😂

[stof]

yeah, having a huge heredoc was what I expected. And this is a heredoc with lots of empty lines, for which my suggested change from above might help (as it reduces the way they can be matched)

[jrfnl]

Testing with the first ~200 lines of the text from the problem file in regex101, I've so far managed to bring the number of steps needed to get to a match down from 37.761 to 45 .... getting somewhere... now off to run the tests with the adjusted regex to see if everything still passes....

[jrfnl]

Tested with phpDocumentor: ✅ deprecation notice is gone
Tested with the full test CI run: ✅

Adding a test for this now to the testsuite.

[jrfnl]

Well that was fun... while creating the tests for my changes, I discovered a second bug in the regex.... which my fix (not completely) accidentally fixes as well 😂

[jrfnl]

I've opened PR #10050 which adds tests for both issues separately + applies a fix.