fix: drop abandoned symplify doc packages

[tiagoabsantos]

Fixes pestphp/pest#1940

Summary

• composer audit fails because symplify/rule-doc-generator-contracts is abandoned and was a production dependency. Pest pulls this plugin in require-dev, so the abandoned package shows up in Pest's lock file too.
• Rector never reads DocumentedRuleInterface or RuleDefinition. Those types now live under Pest\Rector, so the abandoned package, the replace block, and third-party/ are gone.
• symplify/rule-doc-generator is abandoned as well. It was only used by the composer docs script, so that dependency and the generated docs/rules.md snapshot are gone. Descriptions stay on each rule through getRuleDefinition().

Test plan

• composer audit exits 0 in this package
• A project that only requires pestphp/pest-plugin-rector no longer installs either abandoned package, and composer audit exits 0
• vendor/bin/rector --dry-run reports no changes
• Pest: 290 passed
• After this is tagged, update the Pest lock file so composer audit is clean there too

[MrPunyapal]

Thanks for the write up, this one has been sitting around for a while. I checked the branch against a local install of both 5.x and this one.

The core fix is right. composer audit on 5.x exits 1 with both packages flagged abandoned, and with this branch it exits 0. The vendored files are byte identical to upstream 11.2.0, composer validate --strict passes, and the suite is 290/290 here.

One thing worth putting in the issue for the record: Pest never required the contracts package directly. It only came in through this plugin sitting in require-dev, so dropping it here is enough to clean up the Pest lock file.

On the failing test in your plan, matchers_survive_chaining.php.inc also fails on 5.x without this branch, but only when the run is not parallel. composer test:unit passes either way, so that is a pre-existing isolation issue from #9. I will open a separate issue for it, nothing for this PR to carry.

Before we merge

I left inline notes on composer.json and pint.json. The main one is that I do not think we should ship this dependency at all, and if we do, three things need adjusting.

The short version: DocumentedRuleInterface is not something Rector asks for. RectorInterface does not extend it, Rector\AbstractRector does not reference it, and rector/rector ships as a scoped build where the unprefixed Symplify\... references in vendor/rector/rector/rules/ are dev only sources behind scoper-autoload.php. Its composer.json requires php and phpstan, nothing else.

So the interface, and every RuleDefinition we construct in 61 rule classes, exists only to feed a doc generator that this PR removes anyway. Moving the three symbols to Pest\Rector\Contract\DocumentedRuleInterface, Pest\Rector\ValueObject\RuleDefinition and Pest\Rector\ValueObject\CodeSample\CodeSample is a mechanical rename, our own rector.php prepared set can do it, and then we drop the replace block, drop third-party/, drop the pint change and stop carrying the license question.

There is a readability argument too. A plugin that registers another vendor's abandoned namespace as its own and declares a replace for it is a lot to explain to anyone landing on this repo, and it quietly makes us responsible for a namespace we do not own.

If you have a reason for keeping the vendoring that I have not thought of, put it here and I will take it. The inline notes cover what to change if we go that way.

Also needed, outside this PR

static.yml needs a composer audit step. The entire point of this change is the audit result and nothing in CI checks it, so the regression comes straight back the next time someone touches composer.json.

While you are in those workflows, both of them declare a dependency-version: [prefer-lowest, prefer-stable] matrix and then hardcode composer update --prefer-stable in the install step, so the matrix does nothing and prefer-lowest never runs. That is the run I would most want green given a dependency resolution change like this one.

One smaller thing

If we keep third-party/, can you add a short README.md next to the LICENSE with the source package, the version, the upstream tag and why it is vendored? Without it there is nothing telling the next person these files are not ours and should not be edited. While diffing I noticed AbstractCodeSample.php carries an upstream typo in one exception message ("Bad sample good code cannot be empty"), and leaving it verbatim is the right call, I just want it noted so nobody tidies it up later and breaks the byte identical guarantee.

[MrPunyapal]

Do we need this mapping at all?

We would be registering another vendor's abandoned namespace as ours, which makes us responsible for it going forward. Rector never reads these classes: RectorInterface does not extend DocumentedRuleInterface, Rector\AbstractRector does not reference it, and rector/rector ships as a scoped build whose unprefixed Symplify\... sources are dev only behind scoper-autoload.php.

The interface exists purely so the doc generator can read the rules, and we are dropping the generator in this same PR. Moving the three symbols under Pest\Rector\... is a mechanical rename and lets us delete this line, the replace block above, and the whole third-party/ directory.

If there is a reason to keep the vendoring that I have missed, say so here and I will live with it. I would just rather we did not ship it.

[MrPunyapal]

replace applies to the entire dependency graph, so this is a constraint we impose on every project that installs us, not just on us.

Pinning to 11.2.0 means a project requiring ^11.2.1, ~11.3 or ^12 now hits an unsolvable conflict where it installed fine before. That is a regression we would be handing our users to clean up our own audit output.

If we keep the vendoring, "*" is the safer value. It overclaims slightly, but an overclaim only makes us substitutable, whereas an underclaim breaks installs.

[MrPunyapal]

Could you confirm this one works on Linux? I could not get it to exclude anything on Windows. pint --test still flags all 15 vendored files, and it also flagged a deliberately bad file I dropped into tests/Fixture/ even though **/Fixture/** is already listed above, so notPath looks like a no op on Windows regardless of the pattern we pass.

Probably fine on CI since that runs Linux only, and not a blocker either way. Worth knowing though, because a maintainer on Windows running composer lint would silently rewrite the vendored files and break the byte identical guarantee we are relying on.

[tiagoabsantos]

Took the path you preferred. Rector never reads these types, so they now live under Pest\Rector and the vendoring is gone.

• Pest\Rector\Contract\DocumentedRuleInterface
• Pest\Rector\ValueObject\RuleDefinition
• Pest\Rector\ValueObject\CodeSample\CodeSample
• Pest\Rector\ValueObject\CodeSample\ConfiguredCodeSample, because ChainExpectCallsRector constructs one

Dropped the replace block, third-party/, and the Pint exclude. composer validate --strict passes, composer audit is clean, and the suite is 290/290.

The CI notes stay outside this PR, as you marked them. tests.yml already installs with ${{ matrix.dependency-version }}. static.yml still hardcodes --prefer-stable and has no composer audit step.

[MrPunyapal]

I wonder: if we drop it, how will we generate the docs/rules.md? We can also delete the rules.md...
btw the Laravel rector also uses the same; can you check if they're doing something different or audit fails there too?
https://github.com/driftingly/rector-laravel/blob/main/composer.json

[MrPunyapal]

update: see this comment driftingly/rector-laravel#574 (comment)

[tiagoabsantos]

The generator is already gone in this PR. composer docs was vendor/bin/rule-doc-generator generate src --output-file docs/rules.md, and that script left with the package. docs/rules.md is a checked-in snapshot now. Nothing in the README points at it.

I can delete docs/rules.md here. The descriptions stay on each rule through getRuleDefinition(), they just stop being rendered to that file.

rector-laravel has not done this differently. composer.json on main still requires symplify/rule-doc-generator-contracts at runtime and symplify/rule-doc-generator in require-dev, with the same composer docs script writing docs/rector_rules_overview.md. So composer audit fails there for the same reason.

The comment you linked is that diagnosis. Their option 2 is what this branch already does: keep the descriptions, move RuleDefinition and the code samples under our own namespace, and drop the dependency. They have not shipped a fix.

[MrPunyapal]

Okay, let's delete the rules.md and its references

[tiagoabsantos]

Deleted docs/rules.md in d3cccc0.

Nothing else in the tree pointed at it. No README link, no composer docs script, no workflow step. It was only the old generator snapshot (61 rules). Descriptions stay on each rule through getRuleDefinition().

I also updated the PR summary, which still said the file stays.