Skip to content

refactor: add #[\Override] and drop the psalm baseline - #64

Merged
roxblnfk merged 2 commits into
2.xfrom
static-analysis
Oct 9, 2026
Merged

roxblnfk merged 2 commits into
2.xfrom
static-analysis

Conversation

@roxblnfk

@roxblnfk roxblnfk commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

What was changed

  • Methods that implement or override a parent method carry #[\Override], and Psalm's MissingOverrideAttribute suppression is gone.
  • psalm-baseline.xml is removed. Its 46 entries are fixed in code without signature changes: accurate docblocks (collections, GitHub API shapes, stability, config sections), strict comparisons instead of truthy checks, and typed reads of console options. Psalm is green at level 1 without a baseline.
  • The psalm workflow runs on pushes to 2.x with a read-only token.

Review notes

  • ClassMustBeFinal stays suppressed: making the public classes final would break BC for users who extend them.
  • The undefined $config in GetBinaryCommand::installConfig() has an inline suppression pointing to .phar assets cannot be opened, and a failed config generation still writes .rr.yaml #62, so it is not fixed here.
  • Collection suppresses UnsafeGenericInstantiation: its constructor and every subclass are final, so new static cannot break the item type.
  • The --preset option and an empty GITHUB_TOKEN/temp directory are now checked against null/'' instead of truthiness. The only value treated differently is the string "0".

Checklist

  • Tested
    • Testo suite run locally: 200 passed, 4 skipped
    • Psalm and php-cs-fixer run locally

Summary by CodeRabbit

  • Bug Fixes
    • Corrected temporary-directory handling so values other than null or an empty string are validated as directory paths.
    • Improved configuration generation when a preset is empty, ensuring plugin selection is handled correctly.
    • Fixed edge-case handling for GitHub authorization tokens and project configuration checks.

refactor: resolve the Psalm baseline in code
ci: drop the psalm baseline and the MissingOverrideAttribute suppression

The baseline entries are fixed with accurate docblocks, strict comparisons and typed option reads, without signature changes. Two suppressions stay: ClassMustBeFinal, since making public classes final would break BC, and the undefined $config in GetBinaryCommand::installConfig(), tracked in #62. Collection suppresses UnsafeGenericInstantiation because its constructor and all subclasses are final.

Assisted-By: Claude Opus 5.5
ci: run psalm on pushes to 2.x with a read-only token

The previous commit only removed psalm-baseline.xml; this one carries the code and config changes it describes.

Assisted-By: Claude Opus 5.5
@roxblnfk
roxblnfk requested a review from a team October 9, 2026 21:16
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ad780b8f-7247-407b-af45-75b423c2833a

📥 Commits

Reviewing files that changed from the base of the PR and between 3818233 and 81df378.


📒 Files selected for processing (61)
  • .github/workflows/psalm.yml
  • psalm-baseline.xml
  • psalm.xml
  • src/Archive/Factory.php
  • src/Archive/PharArchive.php
  • src/Archive/PharAwareArchive.php
  • src/Archive/TarPharArchive.php
  • src/Archive/ZipPharArchive.php
  • src/Command.php
  • src/Command/ArchitectureOption.php
  • src/Command/InstallationLocationOption.php
  • src/Command/OperatingSystemOption.php
  • src/Command/Option.php
  • src/Command/StabilityOption.php
  • src/Command/VersionFilterOption.php
  • src/Configuration/Generator.php
  • src/Configuration/Plugins.php
  • src/Configuration/Section/AbstractSection.php
  • src/Configuration/Section/Amqp.php
  • src/Configuration/Section/Beanstalk.php
  • src/Configuration/Section/Boltdb.php
  • src/Configuration/Section/Broadcast.php
  • src/Configuration/Section/Endure.php
  • src/Configuration/Section/Fileserver.php
  • src/Configuration/Section/Grpc.php
  • src/Configuration/Section/Http.php
  • src/Configuration/Section/Jobs.php
  • src/Configuration/Section/Kv.php
  • src/Configuration/Section/Logs.php
  • src/Configuration/Section/Metrics.php
  • src/Configuration/Section/Nats.php
  • src/Configuration/Section/Otel.php
  • src/Configuration/Section/Redis.php
  • src/Configuration/Section/Reload.php
  • src/Configuration/Section/Rpc.php
  • src/Configuration/Section/SectionInterface.php
  • src/Configuration/Section/Server.php
  • src/Configuration/Section/Service.php
  • src/Configuration/Section/Sqs.php
  • src/Configuration/Section/Status.php
  • src/Configuration/Section/Tcp.php
  • src/Configuration/Section/Temporal.php
  • src/Configuration/Section/Version.php
  • src/Configuration/Section/Websockets.php
  • src/DownloadProtocBinaryCommand.php
  • src/Environment/OperatingSystem.php
  • src/Environment/Stability.php
  • src/GetBinaryCommand.php
  • src/MakeConfigCommand.php
  • src/Repository/Asset.php
  • src/Repository/Collection.php
  • src/Repository/GitHub/GitHubAsset.php
  • src/Repository/GitHub/GitHubRelease.php
  • src/Repository/GitHub/GitHubRepository.php
  • src/Repository/Release.php
  • src/Repository/ReleaseInterface.php
  • src/Repository/ReleasesCollection.php
  • src/Repository/RepositoriesCollection.php
  • src/Repository/RepositoryInterface.php
  • src/Repository/Version1/StaticRepository.php
  • src/VersionsCommand.php

💤 Files with no reviewable changes (3)
  • psalm-baseline.xml
  • src/Repository/ReleaseInterface.php
  • src/Repository/RepositoryInterface.php

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.



📝 Walkthrough

Walkthrough

This PR updates Psalm CI and configuration, removes the Psalm baseline, adds override attributes and type documentation across CLI, configuration, archive, and repository code, and adjusts checks for empty values, temporary directories, and extracted files.

Changes

Psalm and PHP typing

Layer / File(s) Summary
Psalm workflow and configuration
.github/workflows/psalm.yml, psalm-baseline.xml, psalm.xml
The Psalm workflow now targets 2.x and has read access to repository contents. Psalm no longer loads the deleted baseline, and its MissingOverrideAttribute suppression is removed.
Archive paths and command option values
src/Archive/*, src/Command.php, src/Command/*
Archive methods gain override attributes. Temporary-directory and GitHub-token checks now distinguish null or empty strings from other values. Command options add a string conversion helper and extract release versions from the collection array.
Configuration plugin and section contracts
src/Configuration/Generator.php, src/Configuration/Plugins.php, src/Configuration/Section/*
PHPDoc now describes plugin and section class-name lists. Configuration section methods gain override attributes, and SectionInterface documents the return type of getRequired().
Binary installation and configuration commands
src/DownloadProtocBinaryCommand.php, src/GetBinaryCommand.php, src/MakeConfigCommand.php, src/VersionsCommand.php, src/Environment/OperatingSystem.php
Binary and configuration commands add override attributes and checks for extracted files, working-directory paths, and empty presets. The operating-system factory call no longer receives the optional variables argument.
Repository and release types
src/Repository/*, src/Environment/Stability.php
Repository and release PHPDoc describes stability values and API response shapes. Repository classes gain override attributes, and two interface return-value docblocks are removed.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Suggested reviewers: msmakouz


Merge Risk: ⚪ Minimal · up to 81df3

No concrete regression is established in the reviewed changes, so the PR's merge risk is minimal, subject to normal CI checks.

Pre-merge checks | Passed 4 | Inconclusive 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage Inconclusive Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 126 functions across 50 files. (8 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely summarizes the main changes: adding #[\Override] attributes and removing the Psalm baseline.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 126 functions across 50 files. (8 skipped: 2 unsupported, 6 over the file limit.)



✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@roxblnfk
roxblnfk merged commit d8a5b3f into 2.x Oct 9, 2026
10 checks passed
@roxblnfk
roxblnfk deleted the static-analysis branch October 9, 2026 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant