Skip to content

Conversation

@koriym
Copy link
Member

@koriym koriym commented Oct 15, 2025

Summary

  • Add PHP 8.5 to CI testing configuration
  • Move PHP 8.4 from current_stable to old_stable versions
  • Set PHP 8.5 as current_stable for testing

Details

This PR updates the continuous integration workflow to include PHP 8.5 support. The existing composer.json requirement of "php": "^7.2 || ^8.0" already supports PHP 8.5, so no changes to package requirements were necessary. This update ensures that the package is tested against PHP 8.5 to maintain compatibility.

Test plan

  • Updated CI configuration to test with PHP 8.5
  • CI tests will run automatically on this PR
  • Verify all tests pass with PHP 8.5

🤖 Generated with Claude Code

Summary by Sourcery

Update CI workflow to add PHP 8.5 support by including it in the PHP versions matrix, demoting 8.4 to old_stable and marking 8.5 as current_stable.

CI:

  • Add PHP 8.5 to the CI testing configuration and set it as the current stable version
  • Move PHP 8.4 into the old_stable versions list

Summary by CodeRabbit

  • Chores

    • CI updated to the newest stable PHP (moved to 8.5) and extended prior stable listings.
    • CI workflow comments clarified around version tagging, repository trust, and NOSONAR suppression.
    • SonarCloud config updated to exclude workflow files from hotspot analysis.
    • Minor annotation casing correction in internal comments.
  • Style

    • Code style rules refined for language-construct spacing.
  • Tests

    • Added unit tests covering setter handling when a dependency is unbound and the alternate optional case.

Update continuous integration workflow to test against PHP 8.5 by:
- Adding PHP 8.4 to the old_stable versions list
- Setting current_stable to 8.5

This ensures the package is tested and compatible with PHP 8.5.
Note that composer.json already supports PHP 8.5 via the ^8.0 constraint.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@sourcery-ai
Copy link

sourcery-ai bot commented Oct 15, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

The PR enhances the GitHub Actions CI pipeline to test against PHP 8.5 by moving 8.4 into the legacy versions list and promoting 8.5 to the current stable target, with no changes required to package constraints.

Flow diagram for CI PHP version update process

flowchart TD
    Start([Start CI Workflow]) --> CheckOldStable["Check old_stable versions"]
    CheckOldStable --> UpdateOldStable["Add 8.4 to old_stable"]
    UpdateOldStable --> SetCurrentStable["Set current_stable to 8.5"]
    SetCurrentStable --> RunTests["Run tests with updated PHP versions"]
    RunTests --> End([End])
Loading

File-Level Changes

Change Details Files
Add PHP 8.5 support in CI
  • Include 8.4 in the old_stable versions array
  • Set current_stable version to 8.5
.github/workflows/continuous-integration.yml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link

coderabbitai bot commented Oct 15, 2025

Walkthrough

Updated CI matrix (added "8.4" to old_stable, bumped current_stable to "8.5"), added two unit tests in tests/di/SetterMethodTest.php covering Unbound behavior (including optional setter), corrected a code-coverage annotation casing in src/di/Injector.php, adjusted PHP-CS rules, and added SonarCloud exclusions.

Changes

Cohort / File(s) Summary
CI workflow
.github/workflows/continuous-integration.yml
Added "8.4" to old_stable and updated current_stable from 8.4 to 8.5; added clarifying comments about tag usage, repository trust, and NOSONAR. No behavioral changes.
Unit tests (DI)
tests/di/SetterMethodTest.php
Added testAcceptWithUnboundException and testAcceptWithUnboundExceptionOptional that use a Visitor throwing Unbound; verifies propagation vs. returning null when setter marked optional. Minor import adjustment.
PHP-CS configuration
phpcs.xml
Excluded Squiz.WhiteSpace.LanguageConstructSpacing in the Doctrine CS block and added Generic.WhiteSpace.LanguageConstructSpacing to additional rules.
Injector comment fix
src/di/Injector.php
Fixed casing of a code-coverage annotation from @CodeCoverageIgnore to @codeCoverageIgnore in an exception-path comment; no logic change.
SonarCloud config
.sonarcloud.properties
Added sonar.exclusions=.github/workflows/** to exclude workflow files from SonarCloud analysis.

Sequence Diagram(s)

sequenceDiagram
    participant T as Test
    participant S as SetterMethod
    participant V as VisitorImplementation
    rect rgb(240,248,255)
    Note over T,S: Setup SetterMethod and Visitor (visitor may throw Unbound)
    end

    T->>S: accept(visitor)
    S->>V: visitSetterMethod(setter)
    alt Visitor throws Unbound
        V-->>S: throws Unbound
        alt Setter is optional
            S-->>T: return null
        else
            S-->>T: propagate Unbound (exception)
        end
    else Visitor returns value
        V-->>S: value
        S-->>T: apply value / return result
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I hop through YAML and tests with glee,
A thrown Unbound surprised even me—
Optional petals catch the fall,
An annotation tweak, versions tall.
— a dev rabbit 🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Add PHP 8.5 support" directly aligns with the primary objective of this PR, which is to update the CI workflow to include PHP 8.5 in the testing matrix and set it as the current stable version. The title is clear, specific, and accurately reflects the main change evidenced by the .github/workflows/continuous-integration.yml updates. While the changeset includes additional modifications (new tests, code quality configurations), the PR objectives confirm that PHP 8.5 support is the primary focus, and per the evaluation criteria, the title is not expected to cover every detail of the changeset.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 797739d and be7c607.

📒 Files selected for processing (1)
  • .sonarcloud.properties (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .sonarcloud.properties
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Sourcery review

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 and usage tips.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@codecov
Copy link

codecov bot commented Oct 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (d016ec3) to head (be7c607).
⚠️ Report is 7 commits behind head on 2.x.

Additional details and impacted files
@@              Coverage Diff              @@
##                2.x      #300      +/-   ##
=============================================
+ Coverage     99.49%   100.00%   +0.50%     
  Complexity      365       365              
=============================================
  Files            50        50              
  Lines           996       995       -1     
=============================================
+ Hits            991       995       +4     
+ Misses            5         0       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

koriym and others added 2 commits October 20, 2025 07:58
This commit adds two new test cases to improve code coverage for the
SetterMethod::accept() method's exception handling logic:

1. testAcceptWithUnboundException: Tests that Unbound exceptions are
   properly re-thrown when isOptional is false
2. testAcceptWithUnboundExceptionOptional: Tests that Unbound exceptions
   are silently handled (not re-thrown) when isOptional is true

These tests cover the previously untested catch block in SetterMethod::accept(),
improving the method's line coverage from ~75% to 93.75%.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit updates the PHPCS configuration to use the recommended
Generic.WhiteSpace.LanguageConstructSpacing sniff instead of the
deprecated Squiz.WhiteSpace.LanguageConstructSpacing.

Changes:
- Exclude the deprecated Squiz.WhiteSpace.LanguageConstructSpacing from Doctrine rules
- Add Generic.WhiteSpace.LanguageConstructSpacing to Additional Rules

This eliminates the deprecation warning:
"The bearcs standard uses 1 deprecated sniff - Squiz.WhiteSpace.LanguageConstructSpacing
This sniff has been deprecated since v3.3.0 and will be removed in v4.0.0"

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
tests/di/SetterMethodTest.php (1)

101-155: LGTM with suggestion: Consider extracting the duplicated visitor stub.

The test correctly validates that SetterMethod::accept() returns null when the setter is optional and an Unbound exception is thrown. However, the anonymous visitor implementation (lines 107-151) is nearly identical to the one in the previous test (lines 52-96).

Consider extracting the visitor stub to reduce duplication:

private function createUnboundThrowingVisitor(): VisitorInterface
{
    return new class implements VisitorInterface
    {
        public function visitSetterMethod(string $method, Arguments $arguments): void
        {
            throw new Unbound(FakeTyreInterface::class);
        }

        // ... other stub methods
    };
}

Then both tests can reuse: $visitor = $this->createUnboundThrowingVisitor();

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0aa8dce and 718c274.

📒 Files selected for processing (1)
  • tests/di/SetterMethodTest.php (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/di/SetterMethodTest.php (5)
src/di/Exception/Unbound.php (1)
  • Unbound (16-62)
tests/di/Fake/FakeCar.php (2)
  • FakeCar (11-97)
  • postConstruct (89-96)
src/di/SetterMethod.php (2)
  • SetterMethod (15-82)
  • setOptional (62-65)
src/di/Arguments.php (1)
  • Arguments (14-78)
src/di/Argument.php (1)
  • isDefaultAvailable (81-84)
🪛 PHPMD (2.15.0)
tests/di/SetterMethodTest.php

59-59: Avoid unused parameters such as '$newInstance'. (undefined)

(UnusedFormalParameter)


59-59: Avoid unused parameters such as '$postConstruct'. (undefined)

(UnusedFormalParameter)


63-63: Avoid unused parameters such as '$isSingleton'. (undefined)

(UnusedFormalParameter)


63-63: Avoid unused parameters such as '$dependency'. (undefined)

(UnusedFormalParameter)


63-63: Avoid unused parameters such as '$context'. (undefined)

(UnusedFormalParameter)


69-69: Avoid unused parameters such as '$value'. (undefined)

(UnusedFormalParameter)


74-74: Avoid unused parameters such as '$aopBind'. (undefined)

(UnusedFormalParameter)


78-78: Avoid unused parameters such as '$class'. (undefined)

(UnusedFormalParameter)


78-78: Avoid unused parameters such as '$bind'. (undefined)

(UnusedFormalParameter)


83-83: Avoid unused parameters such as '$setterMethods'. (undefined)

(UnusedFormalParameter)


88-88: Avoid unused parameters such as '$arguments'. (undefined)

(UnusedFormalParameter)


93-93: Avoid unused parameters such as '$index'. (undefined)

(UnusedFormalParameter)


93-93: Avoid unused parameters such as '$isDefaultAvailable'. (undefined)

(UnusedFormalParameter)


93-93: Avoid unused parameters such as '$defaultValue'. (undefined)

(UnusedFormalParameter)


93-93: Avoid unused parameters such as '$parameter'. (undefined)

(UnusedFormalParameter)


114-114: Avoid unused parameters such as '$newInstance'. (undefined)

(UnusedFormalParameter)


114-114: Avoid unused parameters such as '$postConstruct'. (undefined)

(UnusedFormalParameter)


118-118: Avoid unused parameters such as '$isSingleton'. (undefined)

(UnusedFormalParameter)


118-118: Avoid unused parameters such as '$dependency'. (undefined)

(UnusedFormalParameter)


118-118: Avoid unused parameters such as '$context'. (undefined)

(UnusedFormalParameter)


124-124: Avoid unused parameters such as '$value'. (undefined)

(UnusedFormalParameter)


129-129: Avoid unused parameters such as '$aopBind'. (undefined)

(UnusedFormalParameter)


133-133: Avoid unused parameters such as '$class'. (undefined)

(UnusedFormalParameter)


133-133: Avoid unused parameters such as '$bind'. (undefined)

(UnusedFormalParameter)


138-138: Avoid unused parameters such as '$setterMethods'. (undefined)

(UnusedFormalParameter)


143-143: Avoid unused parameters such as '$arguments'. (undefined)

(UnusedFormalParameter)


148-148: Avoid unused parameters such as '$index'. (undefined)

(UnusedFormalParameter)


148-148: Avoid unused parameters such as '$isDefaultAvailable'. (undefined)

(UnusedFormalParameter)


148-148: Avoid unused parameters such as '$defaultValue'. (undefined)

(UnusedFormalParameter)


148-148: Avoid unused parameters such as '$parameter'. (undefined)

(UnusedFormalParameter)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Sourcery review
🔇 Additional comments (2)
tests/di/SetterMethodTest.php (2)

10-10: LGTM: Import added for visitor stub type hints.

The ReflectionParameter import is correctly added to support the type hints in the anonymous visitor implementations below.


46-99: LGTM: Test correctly verifies non-optional setter behavior.

The test properly validates that SetterMethod::accept() re-throws an Unbound exception when the setter is not marked as optional, matching the implementation in src/di/SetterMethod.php lines 54-68.

koriym and others added 2 commits October 20, 2025 08:32
Changed @CodeCoverageIgnore to @codeCoverageIgnore in Injector.php
to properly exclude the DirectoryNotWritable exception line from
coverage reports.

PHPUnit requires the annotation to be lowercase 'code' not 'Code'.
This fix ensures the line is correctly ignored by code coverage tools.

Coverage improvement:
- Injector class: now 100% (21/21 lines)
- Overall: 99.24% (787/793 lines)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add .sonarcloud.properties to exclude .github/workflows/** from
SonarCloud security hotspot analysis.

Rationale:
- GitHub Actions workflow files using version tags (@v1, @v2, etc.)
  trigger S7637 security hotspot warnings
- Using version tags is GitHub Actions best practice and industry standard
- These are references to our own trusted repositories
- Excluding workflows from analysis prevents false positive warnings

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@sonarqubecloud
Copy link

@koriym koriym merged commit 016b4ed into ray-di:2.x Oct 20, 2025
32 checks passed
@koriym koriym deleted the php8.5-support branch October 20, 2025 02:38
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