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

PECL extension support and introduce newly refined main class "Aspect" #214

Merged
merged 28 commits into from
Jul 7, 2024

Conversation

koriym
Copy link
Member

@koriym koriym commented Jul 7, 2024

  • PECL extension support
  • New Aspect class
    • PECL-enabled weave($srcDir) method
    • New newInstance($class) method that works without PECL

Summary by CodeRabbit

  • New Features

    • Introduced AOP (Aspect-Oriented Programming) functionality, allowing users to bind interceptors to classes and methods.
    • Added method interception capabilities with new methods and classes like Aspect, Bind, and Weaver.
    • Users can now create new instances with or without AOP support.
  • Bug Fixes

    • Ensured proper handling of method interception by adding required methods and classes.
  • Documentation

    • Improved documentation for AOP functionality with clear method and class descriptions.

koriym added 24 commits July 6, 2024 19:42
This commit adds two new classes, Aspect and PeclDispatcher for handling aspect-oriented operations. These classes function as part of an aspect-oriented programming (AOP) framework that intercepts method calls. Corresponding tests are included to verify the successful weaving of aspects and method interception.
This commit introduces the MethodInterceptorInterface. The interface defines the contract for method interceptors in the Ray.Aop framework. It includes an 'intercept' method which is invoked when an intercepted method is called.
The new specification document details the functionality of the Aspect class used in Aspect Oriented Programming (AOP). It includes description of the class definition, constructors, public methods as well as use cases and crucial notes. This document will serve as a useful reference for developers working with AOP.
The Weaver class has been updated to check for existence of instance bindings and compile new classes. The Aspect class no longer checks for the 'ray_aop' extension and now creates bindings directly in the newInstance method. The test suite has been expanded to include tests for instance creation and binding behaviors in Weaver and Aspect classes.
The PeclDispatcher class's interface was updated from InterceptHandlerInterface to MethodInterceptorInterface. This update aligns the PeclDispatcher with the new structure and functionality proposed by the MethodInterceptorInterface.
Revised how the Aspect class functions by separating class instantiation into two different private methods, based on finality and extension availability. The main constructor has been updated. For instance, the classDir member has been replaced with tmpDir, which defaults to system temp dire if not explicitly set. Some throw instructions were also inserted regarding extension requirements. The unit tests have been adapted to reflect these changes.
The README file is updated to reflect changes in the usage of the `Aspect` class for configuration. The revised examples present how to use `Aspect` for different scenarios. Additionally, more detailed explanations are provided on how interceptors work, including the order of execution and how to access various elements from the `MethodInvocation` object. The changes also include explanations of how to deal with annotations and PHP8 attributes.
Refined the explanation about interceptors for better clarity in README.md. Specifically, updated the description of `invocation->proceed()` and its process. Also, added a hyperlink to the PECL extension for more comprehensive user understanding.
Significantly edited README.ja.md to make it more readable and user-friendly. This includes restructuring, removing duplicated or unnecessary content, adding helpful explanations, and improving the formatting for better readability. Made adjustments to the code snippets and annotations for the better understanding of the user.
This commit includes several updates aiming to improve the compatibility with psalm-static analysis. Major updates include suppressing some psalm assertions producing false-positives, adding additional type checks and assertions, and refining the PHPDoc annotations for better type inference. Tests have also been adjusted accordingly.
The type hinting in the intercept method of the PeclDispatcher already ensures that the arguments are the correct type. Removing the explicit checks for is_object, is_string, and is_array simplifies the code. Additionally, a variable type declaration has been added to AspectTest in the test module. A new directory tests/AspectFake is also excluded from phpstan checks.
The type hinting in the intercept method of the PeclDispatcher already ensures that the arguments are the correct type. Removing the explicit checks for is_object, is_string, and is_array simplifies the code. Additionally, a variable type declaration has been added to AspectTest in the test module. A new directory tests/AspectFake is also excluded from phpstan checks.
This commit introduces a new function, method_intercept, to the mock testing suite. The function takes a class, method, parameter array and object as parameters. It instantiates an instance of the passed class before calling its intercept method with the given parameters.
The MethodInterceptorInterface file has been relocated to the src-mock directory. Amendments were made in the composer.json to reflect this change and the references in the PeclDispatcher file were updated accordingly. This restructuring ensures better organization and clarity in the codebase.
The annotation for 'runInSeparateProcess' has been corrected in AspectTest.php. The isolated process description remains the same, explaining it is required to avoid side effects from the aspect weaved classes.
The logo has been included in the README.md file. This gives better visual representation and helps with branding for the project. It gets displayed at the top of the README file, improving the overall aesthetics.
The PeclDispatcher file was deleted from the src directory and a similar file was added to the src-php8 directory. The main difference in the added file is the use of Ray\Aop\Exception\LogicException, replacing the generic LogicException.
The AspectTest class has been refactored to change the running order of the tests and revises their contents. The requirement for PHP 8.1 has been removed to increase compatibility. The isolated process notice and @runInSeparateProcess annotation have also been removed as they are no longer necessary.
This commit introduces a new composer-require-checker.json file. This file defines a whitelist of symbols that will not be checked for dependencies in our codebase. This includes basic types like null, bool, string, int, float, and few more, as well as 'method_intercept' and 'Ray\\Aop\\MethodInterceptorInterface'.
This commit introduces a new LogicException class. It extends the base \LogicException class and resides within the Ray\Aop\Exception namespace.
This commit simplifies the method_intercept function in the src-mock. The function parameters have been modified for efficiency, removing the object instantiation in favor of directly passing the MethodInterceptorInterface object.
This commit introduces a new GitHub Actions continuous integration workflow specifically for the Ray.Aop PECL extension. The workflow builds the extension, runs PHPUnit tests, checks for memory leaks with Valgrind, and performs various other checks across multiple PHP versions. The failed steps details are uploaded as artifacts for easier troubleshooting.
The PECL extension code usage examples in both the English and Japanese README files have been updated to be more descriptive. Furthermore, detailed installation instructions for the PECL extension, including the requirement of PHP 8.1 or higher, have been added.
Copy link
Contributor

coderabbitai bot commented Jul 7, 2024

Walkthrough

The changes introduce method interception functionality to the Ray.Aop framework. This involves a new MethodInterceptorInterface that specifies how methods are intercepted, a method_intercept function to facilitate this, and several new classes and methods in the Aspect class to handle AOP tasks such as binding interceptors, weaving aspects, and creating instances.

Changes

File Change Summary
src-mock/MethodInterceptorInterface.php Added the MethodInterceptorInterface with an intercept method for method interception.
src-mock/method_intercept.php Introduced method_intercept function to intercept class methods using the specified interceptor.
src/Aspect.php Added multiple methods and supporting classes to the Aspect class for AOP functionality.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Aspect
    participant MethodInterceptorInterface
    participant TargetClass
    
    Client->>Aspect: bind(classMatcher, methodMatcher, interceptors)
    Aspect->>MethodInterceptorInterface: intercept(object, method, params)
    MethodInterceptorInterface->>TargetClass: method(params)
    TargetClass-->>MethodInterceptorInterface: return result
    MethodInterceptorInterface-->>Aspect: return intercepted result
    Aspect-->>Client: return final result
Loading

Poem

In code where methods weave their dance,
Interceptors take their stance.
With matches bound and aspects new,
A framework blossoms, sleek and true.
Through PHP's depth, the magic spins,
Ray.Aop with all it wins.
🎩✨


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 as 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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration 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
Contributor

@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: 2

Outside diff range and nitpick comments (8)
tests/Aspect/Fake/src/FakeMyClass.php (1)

1-13: Class and method look good but consider adding comments.

The class FakeMyClass and its method myMethod are correctly implemented. Adding comments to describe the purpose of the class and method can improve readability and maintainability.

+/**
+ * Class FakeMyClass
+ * A fake class for testing purposes.
+ */
class FakeMyClass
{
+    /**
+     * Returns a string 'original'.
+     *
+     * @return string
+     */
    public function myMethod(): string
    {
        return 'original';
    }
}
tests/Aspect/Fake/FakeMyInterceptor.php (1)

1-17: Class and method look good but consider adding comments.

The class FakeMyInterceptor and its method invoke are correctly implemented. Adding comments to describe the purpose of the class and method can improve readability and maintainability.

+/**
+ * Class FakeMyInterceptor
+ * A fake interceptor for testing purposes.
+ */
class FakeMyInterceptor implements MethodInterceptor
{
+    /**
+     * Invokes the method interception logic.
+     *
+     * @param MethodInvocation $invocation The method invocation instance.
+     * @return string The result of the method invocation with interception.
+     */
    public function invoke(MethodInvocation $invocation): string
    {
        // Pre-processing logic
        $result = $invocation->proceed();

        // Post-processing logic
        return 'intercepted ' . $result;
    }
}
src-mock/MethodInterceptorInterface.php (1)

1-31: Interface and method look good but consider adding comments.

The interface MethodInterceptorInterface and its method intercept are correctly implemented. Adding comments to describe the purpose of the interface and method can improve readability and maintainability.

+/**
+ * Method Interceptor Interface
+ * This interface defines the contract for method interceptors in the Ray.Aop framework.
+ */
interface MethodInterceptorInterface
{
    /**
     * Intercept method
     * This method is called when an intercepted method is invoked.
     *
     * @param object       $object The object on which the method was called
     * @param string       $method The name of the method being called
     * @param array<mixed> $params An array of parameters passed to the method
     *
     * @return mixed The result of the method invocation
     */
    public function intercept(object $object, string $method, array $params): mixed;
}
src-php8/PeclDispatcher.php (1)

24-36: Improve the exception message.

The exception message in the intercept method could be more informative by including the class and method names.

- throw new LogicException('Interceptors not found');
+ throw new LogicException("Interceptors not found for method $method in class $class");
README.md (4)

15-18: Fix grammatical issues.

There are minor grammatical issues in the explanation.

- It's suited for cross cutting concerns ("aspects"), such as transactions, security and logging.
+ It's suited for cross-cutting concerns ("aspects"), such as transactions, security, and logging.
Tools
LanguageTool

[misspelling] ~16-~16: This expression is normally spelled as one or with a hyphen.
Context: ...hing method is invoked. It's suited for cross cutting concerns ("aspects"), such as transacti...

(EN_COMPOUNDS_CROSS_CUTTING)


[uncategorized] ~16-~16: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ather than objects, their use is called Aspect Oriented Programming (AOP). A [Matcher](https:...

(EN_COMPOUND_ADJECTIVE_INTERNAL)


96-117: Fix grammatical issues.

There are minor grammatical issues in the PECL Extension section.

- Ray.Aop will automatically utilize it when available. PHP 8.1+ is required for the PECL extension.
+ Ray.Aop will automatically utilize it when available. PHP 8.1 or higher is required for the PECL extension.

Line range hint 219-240: Fix grammatical issues.

There are minor grammatical issues in the section on the MethodInvocation object.

- An extended `ClassRefletion` and `MethodReflection` holds methos to get PHP 8 attribute and doctrine annotation(s).
+ An extended `ClassReflection` and `MethodReflection` hold methods to get PHP 8 attribute and doctrine annotation(s).
Tools
Markdownlint

223-223: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


224-224: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


225-225: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


226-226: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


227-227: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


133-133: Specify language for fenced code blocks.

Fenced code blocks should have a language specified.

- ```
+ ```php
Tools
Markdownlint

133-133: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 941db40 and bbb90c7.

Files selected for processing (17)
  • .github/workflows/continuous-integration-pecl.yml (1 hunks)
  • .github/workflows/static-analysis.yml (1 hunks)
  • README.ja.md (6 hunks)
  • README.md (7 hunks)
  • composer-require-checker.json (1 hunks)
  • composer.json (1 hunks)
  • docs/spec.md (1 hunks)
  • phpstan.neon (1 hunks)
  • src-mock/MethodInterceptorInterface.php (1 hunks)
  • src-mock/method_intercept.php (1 hunks)
  • src-php8/PeclDispatcher.php (1 hunks)
  • src/Aspect.php (1 hunks)
  • src/Exception/LogicException.php (1 hunks)
  • src/Weaver.php (2 hunks)
  • tests/Aspect/AspectTest.php (1 hunks)
  • tests/Aspect/Fake/FakeMyInterceptor.php (1 hunks)
  • tests/Aspect/Fake/src/FakeMyClass.php (1 hunks)
Files skipped from review due to trivial changes (4)
  • .github/workflows/static-analysis.yml
  • composer-require-checker.json
  • phpstan.neon
  • src/Exception/LogicException.php
Additional context used
PHPStan
tests/Aspect/AspectTest.php

14-14: Class Ray\Aop\AspectTest extends unknown class PHPUnit\Framework\TestCase.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


32-32: Call to an undefined method Ray\Aop\AspectTest::assertNotSame().

(method.notFound)


35-35: Call to an undefined method Ray\Aop\AspectTest::assertEquals().

(method.notFound)


41-41: Call to an undefined method Ray\Aop\AspectTest::assertInstanceOf().

(method.notFound)


62-62: Call to an undefined method Ray\Aop\AspectTest::assertSame().

(method.notFound)


64-64: Call to an undefined method Ray\Aop\AspectTest::assertEquals().

(method.notFound)

src/Aspect.php

22-22: Used function method_intercept not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(function.notFound)


128-128: Instantiated class Ray\Aop\PeclDispatcher not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


131-131: Function method_intercept not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(function.notFound)

GitHub Check: sa / Psalm
src/Aspect.php

[failure] 131-131:
InvalidArgument: Argument 3 of method_intercept expects MethodInterceptorInterface, but Ray\Aop\PeclDispatcher provided

GitHub Check: sa / PHPStan
src/Aspect.php

[failure] 131-131:
Parameter #3 $interceptor of function method_intercept expects MethodInterceptorInterface, Ray\Aop\PeclDispatcher given.

LanguageTool
README.ja.md

[uncategorized] ~150-~150: 「ことができる」という表現は冗長な可能性があります。
Context: ...一のメソッドに対して複数のインターセプターを適用し、バインドされた順序で実行することができます。 インターセプターA、B、Cがメソッドにバインドされている場合の実行フロ...

(DOUSI_KOTOGADEKIRU)

README.md

[misspelling] ~16-~16: This expression is normally spelled as one or with a hyphen.
Context: ...hing method is invoked. It's suited for cross cutting concerns ("aspects"), such as transacti...

(EN_COMPOUNDS_CROSS_CUTTING)


[uncategorized] ~16-~16: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ather than objects, their use is called Aspect Oriented Programming (AOP). A [Matcher](https:...

(EN_COMPOUND_ADJECTIVE_INTERNAL)

Markdownlint
README.ja.md

163-163: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)


167-167: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)


180-180: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)


181-181: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)


182-182: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)


183-183: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)


180-180: null
No empty links

(MD042, no-empty-links)


181-181: null
No empty links

(MD042, no-empty-links)


182-182: null
No empty links

(MD042, no-empty-links)


183-183: null
No empty links

(MD042, no-empty-links)

README.md

133-133: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


223-223: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


224-224: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


225-225: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


226-226: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


227-227: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


237-237: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


238-238: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


239-239: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


240-240: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


267-267: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


271-271: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)

Additional comments not posted (35)
docs/spec.md (4)

8-14: Class definition and constructor documentation looks good.

The class definition and constructor documentation are accurate and well-structured.


19-32: Public methods documentation looks good.

The bind and weave methods documentation are clear and match the method signatures.


34-42: Internal operations documentation looks good.

The internal operations documentation is clear and provides a good understanding of the internal processes.


43-53: Usage example looks good.

The usage example is clear and matches the documented methods.

tests/Aspect/AspectTest.php (4)

19-22: setUp method looks good.

The setUp method correctly initializes the Aspect instance.


51-65: Verify the availability of PHPUnit assertion methods.

The test method is well-structured, but ensure that the assertSame and assertEquals methods are available in the PHPUnit\Framework\TestCase class.

Tools
PHPStan

62-62: Call to an undefined method Ray\Aop\AspectTest::assertSame().

(method.notFound)


64-64: Call to an undefined method Ray\Aop\AspectTest::assertEquals().

(method.notFound)


38-42: Verify the availability of PHPUnit assertion methods.

The test method is well-structured, but ensure that the assertInstanceOf method is available in the PHPUnit\Framework\TestCase class.

Tools
PHPStan

41-41: Call to an undefined method Ray\Aop\AspectTest::assertInstanceOf().

(method.notFound)


24-36: Verify the availability of PHPUnit assertion methods.

The test method is well-structured, but ensure that the assertNotSame and assertEquals methods are available in the PHPUnit\Framework\TestCase class.

Tools
PHPStan

32-32: Call to an undefined method Ray\Aop\AspectTest::assertNotSame().

(method.notFound)


35-35: Call to an undefined method Ray\Aop\AspectTest::assertEquals().

(method.notFound)

src/Weaver.php (2)

Line range hint 35-52:
newInstance method looks good.

The newInstance method correctly creates a new instance of the weaved class.


Line range hint 54-79:
weave method looks good.

The weave method correctly performs the weaving of the class.

composer.json (3)

32-32: LGTM! Autoload path update.

The addition of "src-php8" to the autoload paths is appropriate given the PECL extension support.


39-39: LGTM! Autoload-dev path update.

The addition of new directories under "tests/Aspect" to the autoload-dev paths aligns with the new testing structure for the Aspect class.


41-46: LGTM! Autoload-dev files update.

The addition of "src-mock/method_intercept.php" and "src-mock/MethodInterceptorInterface.php" to the autoload-dev files is necessary for the new method interception functionalities.

.github/workflows/continuous-integration-pecl.yml (1)

1-114: LGTM! New CI workflow for PECL extension.

The new CI workflow is comprehensive and covers all necessary steps for testing the PECL extension across multiple PHP versions.

src/Aspect.php (10)

37-40: LGTM! Constructor update.

The constructor update to accept an optional temporary directory path is appropriate for the new functionalities introduced.


42-50: LGTM! New bind method.

The addition of the bind method to bind matchers and interceptors is essential for the aspect-oriented programming functionalities.


52-60: LGTM! New weave method.

The addition of the weave method to apply aspects to classes in a directory is essential for the PECL extension support.


62-81: LGTM! New scanAndCompile method.

The addition of the scanAndCompile method to scan and compile classes in a directory is essential for the weave functionality.


83-100: LGTM! New getClassNameFromFile method.

The addition of the getClassNameFromFile method to get the class name from a file is essential for the scanAndCompile functionality.


102-119: LGTM! New processClass method.

The addition of the processClass method to process a class and bind interceptors is essential for the scanAndCompile functionality.


122-134: LGTM! New applyInterceptors method.

The addition of the applyInterceptors method to apply interceptors to methods is essential for the weave functionality.

Tools
PHPStan

128-128: Instantiated class Ray\Aop\PeclDispatcher not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


131-131: Function method_intercept not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(function.notFound)

GitHub Check: sa / Psalm

[failure] 131-131:
InvalidArgument: Argument 3 of method_intercept expects MethodInterceptorInterface, but Ray\Aop\PeclDispatcher provided

GitHub Check: sa / PHPStan

[failure] 131-131:
Parameter #3 $interceptor of function method_intercept expects MethodInterceptorInterface, Ray\Aop\PeclDispatcher given.


144-153: LGTM! New newInstance method.

The addition of the newInstance method to create a new instance of a class is essential for the aspect-oriented programming functionalities.


163-171: LGTM! New newInstanceWithPecl method.

The addition of the newInstanceWithPecl method to create a new instance of a class using PECL is essential for the PECL extension support.


181-191: LGTM! New newInstanceWithPhp method.

The addition of the newInstanceWithPhp method to create a new instance of a class using PHP is essential for the aspect-oriented programming functionalities without PECL.

README.ja.md (5)

78-85: LGTM! Documentation update for Aspect class.

The examples of using the Aspect class are clear and provide necessary information for using the new functionalities.


99-131: LGTM! Documentation update for PECL extension support.

The examples of using the PECL extension are clear and provide necessary information for using the new functionalities.


133-133: LGTM! Documentation update for creating new instances with Aspect.

The examples of creating new instances with the Aspect class are clear and provide necessary information for using the new functionalities.


142-160: LGTM! Documentation update for interceptors.

The details about interceptors are clear and provide necessary information for using the new functionalities.

Tools
LanguageTool

[uncategorized] ~150-~150: 「ことができる」という表現は冗長な可能性があります。
Context: ...一のメソッドに対して複数のインターセプターを適用し、バインドされた順序で実行することができます。 インターセプターA、B、Cがメソッドにバインドされている場合の実行フロ...

(DOUSI_KOTOGADEKIRU)


187-205: LGTM! Documentation update for custom matchers.

The examples of creating custom matchers are clear and provide necessary information for using the new functionalities.

README.md (6)

11-12: LGTM!

The addition of the image tag for the logo is correct.


Line range hint 67-96: LGTM!

The example of configuring everything using the Aspect class is clear and demonstrates the usage effectively.


118-121: LGTM!

The section provides clear instructions for installing the PECL extension.


122-133: LGTM!

The section on configuration options for the Aspect class is well-written and provides valuable information.

Tools
Markdownlint

133-133: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


169-218: LGTM!

The update to the Interceptor Details section provides a clear explanation of how interceptors work and the execution flow.


267-271: LGTM!

The note about the documentation source is clear and provides proper attribution.

Tools
Markdownlint

267-267: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)


271-271: Expected: dash; Actual: asterisk
Unordered list style

(MD004, ul-style)

src-mock/method_intercept.php Outdated Show resolved Hide resolved
src-php8/PeclDispatcher.php Show resolved Hide resolved
The code has been updated to import and assert the use of MethodInterceptorInterface on the dispatcher in Aspect.php. This ensures that the dispatcher instance conforms to the expected interface. It also introduces the use of array_keys to more effectively handle methods.
The namespace Ray\Aop was added to MethodInterceptorInterface.php for further class organization and better handling. This change will assist in specifying the full path of the interfaced class within directory structures.
The namespace Ray\Aop was added to the method_intercept.php file to align it with the rest of the project's structure. Additionally, an unused use statement referencing MethodInterceptorInterface was removed from Aspect.php to clean up the code.
The 'method_intercept' function which is a PECL-created function is updated to be ignored by PHPStan and Psalm. This change addresses static analysis warnings from these tools, ensuring that they do not flag this part of the code. The update will bring more clarity to issues identified during codebase analysis.
Copy link
Contributor

@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 bbb90c7 and d844ceb.

Files selected for processing (3)
  • src-mock/MethodInterceptorInterface.php (1 hunks)
  • src-mock/method_intercept.php (1 hunks)
  • src/Aspect.php (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • src-mock/MethodInterceptorInterface.php
  • src-mock/method_intercept.php
Additional context used
PHPStan
src/Aspect.php

129-129: Instantiated class Ray\Aop\PeclDispatcher not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


133-133: Class Ray\Aop\MethodInterceptorInterface not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)

Additional comments not posted (11)
src/Aspect.php (11)

29-41: Constructor and Properties Review

The constructor initializes the $tmpDir property, defaulting to the system temp directory if not provided. The properties $matchers and $bound are also declared.

  • The $tmpDir initialization is correctly handled with a null coalescing operator.
  • The PHPDoc comments for the properties are helpful in understanding their structure.

43-51: LGTM!

The bind method correctly stores the matchers and interceptors in the $matchers array. The PHPDoc comment is clear and helpful.


53-61: LGTM!

The weave method correctly checks for the rayaop extension and throws an exception if not loaded. It then calls scanAndCompile and applyInterceptors.


63-82: LGTM!

The scanAndCompile method correctly iterates over files in the given directory and processes PHP files by calling processClass.


84-101: LGTM!

The getClassNameFromFile method correctly retrieves the class name by comparing declared classes before and after including the file.


103-121: LGTM!

The processClass method correctly uses reflection to inspect the class and its methods, and binds interceptors based on matchers.


123-138: LGTM!

The applyInterceptors method correctly applies interceptors using the PeclDispatcher and calls method_intercept for each bound method.

Tools
PHPStan

129-129: Instantiated class Ray\Aop\PeclDispatcher not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


133-133: Class Ray\Aop\MethodInterceptorInterface not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(class.notFound)


140-157: LGTM!

The newInstance method correctly creates a new instance of a class, using either PECL or PHP-based instantiation based on the class properties and extension availability.


167-175: LGTM!

The newInstanceWithPecl method correctly creates a new instance of a class, processes it, and applies interceptors.


185-195: LGTM!

The newInstanceWithPhp method correctly handles the creation of a new instance using PHP-based AOP, ensuring the temporary directory is set and using a Weaver to create the instance.


197-218: LGTM!

The createBind method correctly creates a Bind object and binds interceptors to methods based on matchers.

@koriym koriym merged commit 28f4a78 into 2.x Jul 7, 2024
48 checks passed
@koriym koriym deleted the pecl branch July 7, 2024 05:08
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