Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serializable provider provider #291

Merged
merged 9 commits into from
Sep 10, 2024
Merged

Serializable provider provider #291

merged 9 commits into from
Sep 10, 2024

Conversation

koriym
Copy link
Member

@koriym koriym commented Sep 7, 2024

Enables serialization of providers to be injected with provider injection.

https://ray-di.github.io/manuals/1.0/en/injecting_providers.html

class LogFileTransactionLog implements TransactionLogInterface
{
    public function __construct(
        (#[Set(Connection::class)] private ProviderInterface $connectionProvider
    ) {}
    
    public function logChargeResult(ChargeResult $result) {
        /* only write failed charges to the database */
        if (! $result->wasSuccessful()) {
            $connection = $connectionProvider->get();
        }
    }
}

serialize(new LogFileTransactionLog); // Couldn't do it before.

@coderabbitai review

Summary by CodeRabbit

  • New Features
    • Updated the PHP version in workflows to 8.3, ensuring access to the latest features and improvements.
    • Introduced the ProviderProvider class to enhance dependency injection management.
    • Added unit tests for the ProviderProvider class to validate dependency injection functionality.
  • Bug Fixes
    • Expanded ignored paths in PHPStan configuration to improve static analysis accuracy.
  • Refactor
    • Replaced an anonymous class with the ProviderProvider class for better code organization in the ProviderSetProvider.
    • Modified the test environment setup to use a temporary directory for improved isolation in tests.

Replaced anonymous class with ProviderProvider in ProviderSetProvider. This makes it serialisable.This improves code reuse and readability by consolidating instance retrieval logic into a dedicated class.
Removed a PHPStan ignore comment in the LazyTo.php file to ensure proper type checking by the static analysis tool. This change improves code quality by allowing PHPStan to enforce type checks on the `getInstance` method call.
Added a new pattern to the PHPStan configuration to exclude all test files matching 'tests/di/Ray_Di_*'. This helps to prevent analysis of test artifacts that are irrelevant to the project's static code analysis results.
Upgraded the PHP version from 8.1 to 8.3 in both static analysis and coding standards GitHub Action workflows. This ensures compatibility with the latest language features and improvements.
The @phpstan-ignore-line comment was added to suppress a specific PHPStan warning when calling getInstance in the LazyTo class. This ensures that static analysis does not flag this line while maintaining the intended functionality.
Copy link

coderabbitai bot commented Sep 7, 2024

Walkthrough

The changes involve updates to several GitHub Actions workflow files and PHP configuration files. The PHP version has been upgraded from 8.1 to 8.3 in multiple workflows, ensuring compatibility with the latest PHP features. Additionally, a new class, ProviderProvider, has been introduced to enhance dependency injection, replacing an anonymous class in the ProviderSetProvider. The PHPStan configuration has also been updated to ignore additional paths.

Changes

File Path Change Summary
.github/workflows/*.yml Updated PHP version from 8.1 to 8.3 in coding standards, demo, and static analysis workflows.
phpstan.neon Added tests/di/Ray_Di_* to ignored paths for PHPStan.
src/di/ProviderProvider.php Introduced ProviderProvider class implementing ProviderInterface with updated constructor and method signatures.
src/di/ProviderSetProvider.php Replaced anonymous class with ProviderProvider instantiation in the get method.
tests/di/ProviderProviderTest.php Added unit test for ProviderProvider class to validate dependency injection functionality.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant ProviderSetProvider
    participant ProviderProvider
    participant Injector

    Client->>ProviderSetProvider: request instance
    ProviderSetProvider->>ProviderProvider: create instance
    ProviderProvider->>Injector: retrieve instance
    Injector-->>ProviderProvider: return instance
    ProviderProvider-->>ProviderSetProvider: return instance
    ProviderSetProvider-->>Client: return instance
Loading

🐰 "Oh, what a hop, what a leap,
PHP's now 8.3, oh so deep!
With ProviderProvider, clear and bright,
Dependencies dance, what a sight!
In workflows we trust, all is well,
Coding standards ring, like a bell!" 🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

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: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f6f39ef and 563bc0b.

Files selected for processing (6)
  • .github/workflows/coding-standards.yml (1 hunks)
  • .github/workflows/demo-php8.yml (1 hunks)
  • .github/workflows/static-analysis.yml (1 hunks)
  • phpstan.neon (1 hunks)
  • src/di/ProviderProvider.php (1 hunks)
  • src/di/ProviderSetProvider.php (1 hunks)
Files skipped from review due to trivial changes (1)
  • phpstan.neon
Additional comments not posted (4)
.github/workflows/static-analysis.yml (1)

12-12: PHP version updated to 8.3.

The update from PHP 8.1 to 8.3 is approved as it ensures compatibility with the latest PHP features and standards.

Consider verifying that all tools and scripts used in the static analysis job are compatible with PHP 8.3 to avoid potential issues.

.github/workflows/coding-standards.yml (1)

12-12: PHP version updated to 8.3.

The update from PHP 8.1 to 8.3 is approved as it ensures compatibility with the latest PHP features and standards.

Consider verifying that all tools and scripts used in the coding standards job are compatible with PHP 8.3 to avoid potential issues.

.github/workflows/demo-php8.yml (1)

19-19: PHP version updated to 8.3.

The update to PHP 8.3 in the GitHub Actions workflow is approved. Ensure to verify compatibility with existing code and dependencies.

Run the following script to verify the compatibility of PHP 8.3 with existing code and dependencies:

Verification successful

PHP 8.3 compatibility confirmed.

The PHP version update to 8.3 in the GitHub Actions workflow is compatible with the existing code and dependencies. The composer.json file allows for PHP 8.3, and no deprecated functions were found in the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify compatibility of PHP 8.3 with existing code and dependencies.

# Test: Check for PHP 8.3 compatibility issues in the codebase.
rg --type php -A 5 $'php 8.3'

Length of output: 30


Script:

#!/bin/bash
# Search for PHP version constraints in composer.json files
rg '"php":' --glob '**/composer.json' -A 2

# Check for deprecated features or functions in PHP 8.3
# Note: This requires knowledge of deprecated features in PHP 8.3, which we will simulate by searching for common deprecated functions.
rg --type php -e 'create_function' -e 'each' -e 'mysql_' -A 2

Length of output: 8517

src/di/ProviderSetProvider.php (1)

51-51: Refactoring to named class ProviderProvider.

The replacement of the anonymous class with ProviderProvider in ProviderSetProvider is approved. Ensure to verify that the behavior of the new class matches the previous implementation.

Run the following script to verify the behavior of the new class compared to the previous implementation:

Verification successful

Refactoring to named class ProviderProvider is consistent and appropriate.

The ProviderProvider class correctly implements the expected behavior of providing instances using the injector and set configuration. The refactoring from an anonymous class to ProviderProvider in ProviderSetProvider is consistent with dependency injection patterns and is approved.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the behavior of `ProviderProvider` compared to the previous anonymous class implementation.

# Test: Search for differences in behavior between `ProviderProvider` and the previous anonymous class.
rg --type php -A 5 $'ProviderProvider'

Length of output: 516


Script:

#!/bin/bash
# Extract methods and their implementations from ProviderProvider.php to understand its behavior.
ast-grep --lang php --pattern $'class ProviderProvider {
  $$$
}' --json

Length of output: 76


Script:

#!/bin/bash
# Output the entire content of ProviderProvider.php to manually inspect its structure and behavior.
fd 'ProviderProvider.php' --exec cat

Length of output: 652

src/di/ProviderProvider.php Outdated Show resolved Hide resolved
This commit introduces a new test case in `ProviderProviderTest.php` to verify the `ProviderProvider` functionality. It ensures that the `ProviderProvider` returns an instance of `FakeEngine` when invoked with the correct binding in the injector.
Updated the container instantiation in VisitorTest to use a temporary directory. This change improves test isolation by ensuring temporary files do not interfere with other tests or code.
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: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 563bc0b and fe2f5f2.

Files selected for processing (2)
  • tests/di/ProviderProviderTest.php (1 hunks)
  • tests/di/VisitorTest.php (1 hunks)
Additional context used
PHPStan
tests/di/ProviderProviderTest.php

23-23: Parameter #2 $set of class Ray\Di\ProviderProvider constructor expects Ray\Di\Di\Set, Ray\Di\Di\Set<Ray\Di\FakeEngineInterface> given.
Template type T on class Ray\Di\Di\Set is not covariant. Learn more: https://phpstan.org/blog/whats-up-with-template-covariant

(argument.type)

GitHub Check: sa / PHPStan
tests/di/ProviderProviderTest.php

[failure] 23-23:
Parameter #2 $set of class Ray\Di\ProviderProvider constructor expects Ray\Di\Di\Set, Ray\Di\Di\Set<Ray\Di\FakeEngineInterface> given.

Additional comments not posted (1)
tests/di/VisitorTest.php (1)

33-33: Review of changes in setUp method in VisitorTest.

The modification to initialize the container with a temporary directory (__DIR__ . '/tmp') is a significant change. This adjustment likely aims to isolate the test environment by using a dedicated temporary directory for container configuration or resources.

This change is beneficial as it can help avoid side effects that might occur when tests interact with persistent data or configurations. It also makes the tests more robust by ensuring that each test run starts with a clean state.

Ensure that the temporary directory is properly managed, possibly cleaned up or recreated before each test run to maintain isolation and prevent issues from residual data.

tests/di/ProviderProviderTest.php Outdated Show resolved Hide resolved
Renamed the method `testInvoke` to `testGet` to better reflect its functionality. Added a type hint for the `$set` variable to indicate it contains objects, improving code clarity and type safety.
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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between fe2f5f2 and 1653c07.

Files selected for processing (1)
  • tests/di/ProviderProviderTest.php (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • tests/di/ProviderProviderTest.php

@koriym koriym changed the title Provider serializable Serializable provider provider Sep 7, 2024
Updated the type annotations to use a generic template for better type safety and removed an unused comment in tests. This improves code readability and maintains consistency in type declarations.
Copy link

sonarcloud bot commented Sep 7, 2024

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1653c07 and 2c209a6.

Files selected for processing (2)
  • src/di/ProviderProvider.php (1 hunks)
  • tests/di/ProviderProviderTest.php (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • src/di/ProviderProvider.php
  • tests/di/ProviderProviderTest.php

@koriym koriym requested a review from jingu September 10, 2024 05:54
@koriym
Copy link
Member Author

koriym commented Sep 10, 2024

@coderabbitai Approved?

Copy link

coderabbitai bot commented Sep 10, 2024

@koriym: Yes, the PR is approved. The changes look good, and the new functionality is well-tested.

Actions performed

Comments resolved.

@koriym koriym merged commit 3155fc6 into 2.x Sep 10, 2024
44 checks passed
@koriym koriym deleted the provider-serialisable branch September 10, 2024 16: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