[TASK] Declare psr/log in the packages that use it - #1354
Conversation
|
We are currenlty requiring "symfony/http-client": "^6.4.9 || ^7.4 || ^8", which allow psr/log: ^1|^2|^3 Adding "psr/log": "^3.0" to our requirements could be a breaking change for some Projects and could therefore not be backported to Guides 1.0. |
632839a to
2e4c765
Compare
|
You are right, and The range is safe for the code as written, which I checked rather than assumed: the production code uses only Also dropped from this branch: the Branch rebased on current main. Full suite 828 tests, no failures, and green again with Assisted by claude-code:claude-fable-5 — Session |
|
Copilot has no review quota left this month, so no bot review was possible on this head. Reviewed it myself instead; here is what was checked rather than assumed. The mapping is exact and complete. Ten packages, eight of which use The constraint is not a narrowing. The range is safe for the code as written. Only Verified with the tool the repository already ships. Full suite 828 tests, no failures. PHPStan and PHPCS clean. One CI note that is not about this change: the Assisted by claude-code:claude-fable-5 — Session |
Seven packages type-hint Psr\Log\LoggerInterface or read LogLevel in their production code, but none of them requires psr/log. It resolves today because symfony/http-client and monolog/monolog pull it in, so nothing fails and nothing warns. Each package is sub-split into its own published package, where that indirection becomes a consumer's problem. The constraint matches what symfony/http-client already allows, so no consumer loses a resolution it has today. Nothing implements LoggerInterface, and only LoggerInterface and LogLevel are used, both unchanged across the three majors. phpdocumentor/dev-server already declares the same range. packages/guides carried psr/log in require-dev while LoggerInterface is a required constructor argument in ParseFileHandler and others; that moves to require. The lock refresh also picks up path-package metadata that was already stale: scrivo/highlight.php in guides-code and the doctrine/deprecations and guides-restructured-text entries in guides-theme-bootstrap. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de> Assisted-by: claude-code:claude-fable-5 Agent-Session: https://claude.ai/code/session_0114KJz3vqq2WWfx4FUdmcss Agent-Host: 0493f0
2e4c765 to
2e7916c
Compare
Problem
Seven packages use PSR-3 in their production code without requiring it.
Psr\Log\LoggerInterfaceis a required constructor argument in classes such asParseFileHandler, andPsr\Log\LogLevelis read in the DI extension — but no package'srequirementionspsr/log.packages/guidesdeclares it underrequire-dev, which does not cover production use.It resolves today because
symfony/http-client(arequireofphpdocumentor/guides) andmonolog/monolog(arequireofphpdocumentor/guides-cli) both pullpsr/login. That is a stable path, not a coincidence, but it is undeclared: if either dependency is dropped or makespsr/logoptional, everyLoggerInterfacetype hint breaks at autoload time and Composer will not have warned about it. Since each package is sub-split into its own published package, the indirection becomes a consumer's problem rather than this repository's.Changes
psr/log: ^1.0 || ^2.0 || ^3.0added torequireof guides, guides-cli, guides-code, guides-graphs, guides-markdown, guides-restructured-text and guides-theme-bootstrap. Forpackages/guidesthis is a move out ofrequire-dev.The range is deliberately the one
symfony/http-clientalready allows, so no consumer loses a resolution it has today and nothing here needs to be excluded from a 1.0 backport. It is also whatphpdocumentor/dev-serveralready declares in this repository.That range is safe for the code as written: only
LoggerInterfaceandLogLevelare used, both unchanged across the three majors, and no class in any package implementsLoggerInterface— which is where a v1/v3 signature difference would bite.Verification
composer-require-checkeris present in.phive/phars.xmlbut is not invoked by any workflow, composer script or Makefile, so nothing currently detects this class of defect. I ran it manually as a controlled before/after on a standalone install ofguides-code, the smallest affected package: without the declarationPsr\Log\LoggerInterfaceis listed as an unknown symbol, with it the symbol is gone.Full suite on the branch: 828 tests, no failures. Also run with
--prefer-lowest, which resolvespsr/logto 2.0.0 and passes.composer validate --strictpasses for the root and every package.Note on the lock
The lock refresh carries three corrections that were already stale before this branch:
scrivo/highlight.phpin guides-code, and thedoctrine/deprecationsandphpdocumentor/guides-restructured-textentries in guides-theme-bootstrap. They are a side effect ofcomposer update --lock, not edits to thosecomposer.jsonfiles.Out of scope
An earlier revision of this branch also added
fig/log-testto therequire-devof guides-code and guides-restructured-text, whose tests usePsr\Log\Test\TestLogger. That is dropped: every package marks/tests export-ignore, so the published packages contain no tests and the declaration would describe a dependency nothing in the package uses. The rootcomposer.json, where the tests actually run, already declares it.The same tool run against
guides-codereports further unknown symbols beyondPsr\Log, among themSymfony\Component\DependencyInjection\*,Symfony\Component\Config\*,Twig\*andphpDocumentor\Guides\*. That is the same class of defect at a larger scale and some of it may be deliberate for optional integrations, so it is left alone. Happy to open an issue, or to wirecomposer-require-checkerinto CI so it stops recurring.Assisted by claude-code:claude-fable-5 — Session