Create own codegen

[koriym]

• Remove nikic/php-parser
• Motivated by increasing speed and reducing dependence
• Not easy job

独自のcodegen実装が困難な理由

nikic/php-parserを使わないで独自のcodegenでオリジナルのコードを変更するのは困難です。以下がその理由。

• リフレクションだけでは必要な情報が集められない。例えばuseが取得できない。
• token_get_allで得られるtokenでは内容の改変が困難。
• 親クラスのクラスのメソッドも取得して１つののコードにする必要あるが、親クラスの必要な情報だけをtokenからだけ取得するのは非常に困難。

解決策

*　token_get_allで得られるtokenとリフレクションのハイブリッドで行う
*　classの宣言まではtokenとその改変で行う
*　メソッド情報はリフレクションから取得。これは引数の名前解決がtokenでは困難なため。

そもそもこの実装が最適化されたとしてもどの程度パフォーマンスアップ、低メモリになるかは未知数ですが実装してみないことにはわかりません。いうまでもありませんがプロダクションではコンパイルされたコードが使われるのでAOPによるパフォーマンスインパクトはほとんどありません。このPRは開発時やディプロイの時の最初のヘルスチェックでコンパイルを行う時のパフォーマンス向上です。

Not easy

It is difficult to modify the original code with our own codegen without using nikic/php-parser. Here is why.

• Reflection alone cannot gather necessary information. For example, use cannot be obtained.
• It is difficult to modify the contents with TOKEN obtained by token_get_all.
• It is very difficult to get only necessary information of the parent class from token, although it is necessary to get methods of the parent class to make a single code.

Solution

• Use a hybrid of token and reflection obtained by token_get_all.
• Use token and its modification to declare classes.
• Method information is obtained from reflection. This is because name resolution of arguments is difficult with token.

Even if this implementation is optimized, it is unknown how much performance and memory reduction will be achieved. Needless to say, the performance impact of AOP will be negligible in production, where compiled code is used. The PR is a performance improvement when compiling at the first health check during development and deployment.

Todo

• unionタイプのリターンテスト
• 導入する価値があるかベンチマーク x20+
• SA & coverage 100%
• アトリビュートの変換を名前付き引数に

Related: #206

Closes #206

---

パーサトークンの一覧

Summary by CodeRabbit

• New Features

  • Introduced new classes for aspect-oriented programming enhancements.
  • Added support for PHP 8.2.
  • Enhanced functionality in ReflectionClass and other components to improve code handling and type management.

• Bug Fixes

  • Adjusted logic and fixed issues in various classes to ensure smoother operation and interaction.

• Refactor

  • Refactored several components to optimize performance and maintainability.

• Tests

  • Added and updated tests to cover new functionality and ensure reliability.

• Documentation

  • Updated configuration files to align with new features and standards.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (0961e4a) to head (1f50707).
Report is 11 commits behind head on 2.x.

❗ Current head 1f50707 differs from pull request most recent head 56bf55f. Consider uploading reports for the commit 56bf55f to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##                 2.x      #205   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity       168       189   +21     
===========================================
  Files             30        26    -4     
  Lines            470       503   +33     
===========================================
+ Hits             470       503   +33     

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

[koriym]

@NaokiTsuchiya PHP8.2のDNF型、その他に対応しました。
https://github.com/ray-di/Ray.Aop/blob/8507909b4f92148026c795329f89c4c724a675a0/tests/Fake/FakePhp82Types.php

[koriym]

リファクタリングしてAopCodeGenを廃止し、AopCode(GeneratedCode)に集約しました。(OOP)

https://github.com/ray-di/Ray.Aop/pull/205/files#diff-27283633b526d5b90eca1c0d03ff97fd779708eb2ecae60ece12bae1775ae110R47-R49

[jingu]

手元のプロジェクトではメソッドの戻り値がstaticでエラーが出ましたのでsuggestしました。

[jingu]

👍

[coderabbitai]

Walkthrough

This update focuses on enhancing the Ray.Aop library by upgrading to PHP 8.2, refining the coding standards, and optimizing aspect-oriented programming capabilities. Notable changes include the addition of new classes for AOP, adjustments to existing functionalities, and a shift from using an external PHP-Parser to a custom code generation engine, aiming for significant performance improvements.

Changes

File Pattern	Change Summary
.github/workflows/...	Updated PHP version to 8.2
.gitignore	Modified exclusion pattern for PHPUnit cache directories
composer.json	Updated required extensions and dependencies
phpcs.xml, phpmd.xml, phpstan.neon, psalm.xml	Updated coding standards and static analysis configurations
src/**/*.php	Introduced new classes and methods, refactored existing ones, enhanced AOP functionalities
tests/**/*.php	Updated and added new test cases to reflect changes in the source code

Assessment against linked issues

Objective	Addressed	Explanation
10-year anniversary celebration of Ray.Aop (#206)	❌	The PR focuses on technical updates and does not address celebration activities or benchmarks.

Poem

🐇 Hop, hop, hop, through the code we go,
🌟 Shining bright, with AOP's new flow.
📅 Ten years pass, yet still we thrive,
🚀 Faster, smarter, our code comes alive.
🎉 To another decade, we now aspire,
With every commit, we aim higher and higher!

---

Recent Review Details

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 71d0698 and 56bf55f.

Files ignored due to path filters (1)

• vendor-bin/tools/composer.lock is excluded by !**/*.lock

Files selected for processing (42)

• .github/workflows/coding-standards.yml (1 hunks)
• .github/workflows/static-analysis.yml (1 hunks)
• .gitignore (1 hunks)
• composer.json (2 hunks)
• phpcs.xml (1 hunks)
• phpmd.xml (2 hunks)
• phpstan.neon (1 hunks)
• psalm.xml (1 hunks)
• sl-src/CacheReader.php (3 hunks)
• src/AopCode.php (1 hunks)
• src/AopPostfixClassName.php (1 hunks)
• src/Bind.php (1 hunks)
• src/Compiler.php (5 hunks)
• src/Exception/CompilationFailedException.php (1 hunks)
• src/InterceptTrait.php (2 hunks)
• src/Matcher/AnyMatcher.php (1 hunks)
• src/MethodMatch.php (2 hunks)
• src/MethodSignatureString.php (1 hunks)
• src/ReflectionClass.php (2 hunks)
• src/ReflectiveMethodInvocation.php (2 hunks)
• src/TokenIterator.php (1 hunks)
• src/TypeString.php (1 hunks)
• src/Weaver.php (3 hunks)
• tests-deprecated/CodeGenPhp71Test.php (1 hunks)
• tests-deprecated/CodeGenTest.php (1 hunks)
• tests-deprecated/ParserFactoryTest.php (1 hunks)
• tests/AopCodeTest.php (1 hunks)
• tests/BindTest.php (1 hunks)
• tests/CompilerTest.php (4 hunks)
• tests/Fake/Annotation/FakeMarker4.php (1 hunks)
• tests/Fake/Annotation/FakeMarkerName.php (1 hunks)
• tests/Fake/FakeConstructorClass.php (1 hunks)
• tests/Fake/FakeMockGrandChild.php (1 hunks)
• tests/Fake/FakeNullInterface.php (1 hunks)
• tests/Fake/FakeNullInterface1.php (1 hunks)
• tests/Fake/FakePhp82Types.php (1 hunks)
• tests/Fake/FakePhp8Types.php (1 hunks)
• tests/Fake/FakeTypedMock.php (1 hunks)
• tests/Fake/FakeTypedMockGrandChild.php (1 hunks)
• tests/Fake/FakeWeaverScript.php (1 hunks)
• tests/ReflectionClassTest.php (1 hunks)
• tests/tmp_unerase/Ray_Aop_FakeWeaverMock_2665014008.php (1 hunks)

Files skipped from review due to trivial changes (6)

• .gitignore
• phpstan.neon
• src/Exception/CompilationFailedException.php
• tests/Fake/FakeMockGrandChild.php
• tests/Fake/FakeNullInterface.php
• tests/Fake/FakeNullInterface1.php

Additional comments not posted (47)

.github/workflows/static-analysis.yml (1)

12-12: Update to PHP 8.2 approved.

Please ensure that all dependencies and actions used in this workflow are compatible with PHP 8.2.

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

12-12: Update to PHP 8.2 approved.

Please ensure that all dependencies and actions used in this workflow are compatible with PHP 8.2.

tests/Fake/FakeWeaverScript.php (1)

7-7: Implementation of FakeNullInterface and FakeNullInterface1 approved.

Please verify that all methods of FakeWeaverScript correctly implement the contracts defined in FakeNullInterface and FakeNullInterface1.

tests/Fake/FakeTypedMockGrandChild.php (1)

7-11: Extension of FakeTypedMockChild and addition of an empty constructor approved.

Consider whether the empty constructor in FakeTypedMockGrandChild is necessary, as it could be omitted if no additional initialization is required.

tests/Fake/FakeConstructorClass.php (2)

7-11: Addition of an empty constructor approved.

Consider whether the empty constructor in FakeConstructorClass is necessary, as it could be omitted if no additional initialization is required.

---

13-15: Implementation of getDouble method approved.

tests-deprecated/ParserFactoryTest.php (1)

7-7: Movement of use Ray\Aop\ParserFactory statement approved.

tests/Fake/FakeTypedMock.php (1)

15-15: Addition of #[FakeMarker2] attribute approved.

Please verify that the new #[FakeMarker2] attribute does not conflict with the existing @FakeMarker3 annotation in the passIterator method.

tests/Fake/Annotation/FakeMarker4.php (1)

13-24: Definition of FakeMarker4 class approved.

psalm.xml (2)

8-8: Update to PHP 8.2 is consistent with the project's upgrade to support new PHP features.

---

9-9: Correcting the format of findUnusedCode to use proper XML attribute syntax enhances readability and prevents potential configuration errors.

src/AopPostfixClassName.php (2)

21-26: Ensure that the file path obtained from Reflection is valid before using it with filemtime to prevent runtime errors.

Verification successful

---

The verification process has confirmed that the method getFileName() from the ReflectionClass is indeed used in the file src/AopPostfixClassName.php as well as in several test files (CompilerTest.php). The usage pattern aligns with the snippet provided in the review, where the file path obtained from ReflectionClass is used directly with filemtime() and file_get_contents().

Given this context, the original review comment suggesting to ensure that the file path obtained from ReflectionClass is valid before using it with filemtime to prevent runtime errors is appropriate. This is because if ReflectionClass fails to find the file or returns an invalid path, it could lead to runtime errors when passed to filemtime().

Therefore, the review comment is consistent with the findings and the usage pattern in the codebase. The developer should consider implementing a check to ensure the validity of the file path before using it in file operations.

Conclusion:

• The review comment is verified as relevant and appropriate based on the usage patterns found in the codebase.
• It is recommended to implement error handling around the usage of ReflectionClass->getFileName() to prevent potential runtime errors.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Check for potential issues with file paths returned by ReflectionClass in the context of `filemtime`.
ast-grep --lang php --pattern $'new ReflectionClass($_)->getFileName()'

Length of output: 179

---

---

Script:

#!/bin/bash
# Use ripgrep to search for the usage of ReflectionClass and getFileName in PHP files
rg 'new ReflectionClass\(.+\)->getFileName\(\)' -t php

Length of output: 485

---

23-25: The generation of postFix using crc32 and filemtime is a clever use of PHP functions to ensure a unique class name modifier.

tests/Fake/Annotation/FakeMarkerName.php (2)

15-15: Usage of PHP 8 attributes to define an annotation class aligns with modern PHP practices and enhances interoperability.

---

27-31: Constructor correctly initializes properties with provided arguments, ensuring the class's attributes are accurately represented.

tests/Fake/FakePhp82Types.php (1)

12-18: Methods correctly implement PHP 8.2 specific return types, which is essential for testing the framework's compatibility with the latest PHP version.

src/TokenIterator.php (2)

19-24: The getToken method correctly handles the possibility of a token being a string or an array, ensuring robustness in token processing.

---

27-31: The skipExtends method provides a clear and straightforward way to navigate past the tokens related to class inheritance, which is crucial for accurate token parsing.

src/InterceptTrait.php (2)

13-13: The updated type for $bindings reflects a more precise and restrictive type expectation, which can improve type safety and clarity in method interception.

---

26-26: Reordering parameters in the _intercept method to (string $func, array $args) improves readability and aligns with common coding practices where the function name typically precedes its arguments.

tests/tmp_unerase/Ray_Aop_FakeWeaverMock_2665014008.php (1)

13-34: The implementation of _intercept in method bodies ensures that method calls are intercepted as expected, which is crucial for the functionality of AOP (Aspect-Oriented Programming).

src/Matcher/AnyMatcher.php (1)

24-27: The modification to check if builtinMethods is not empty before setting methods is a good performance optimization, preventing unnecessary operations.

tests/ReflectionClassTest.php (1)

59-62: The new test method testGetParentClass() correctly tests the functionality of the getParentClass() method.

tests-deprecated/CodeGenTest.php (1)

8-16: The imports for the new code generation classes are correctly added and used in the setUp() method.

src/Weaver.php (1)

61-75: The modifications in the weave() method correctly implement the functionality using AopPostfixClassName for class weaving.

tests/Fake/FakePhp8Types.php (1)

10-65: The class FakePhp8Types correctly demonstrates various PHP 8 features and follows best practices.

src/ReflectionClass.php (1)

79-90: The new method getParentClass() correctly extends the functionality of the parent class's method and follows best practices.

phpmd.xml (1)

5-15: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [8-34]

The adjustments to the CyclomaticComplexity rule and the commenting out of the UnusedPrivateField rule are justified and correctly implemented.

composer.json (1)

14-22: The changes to the dependencies in composer.json are correctly implemented and align with the PR's objectives of enhancing the custom code generation system.

src/Bind.php (1)

42-45: The modification to skip the __construct method during binding in the bind() method is correctly implemented and follows best practices.

src/MethodMatch.php (2)

33-38: Handling of PriorityPointcut instances in __invoke method is correctly implemented.

---

83-88: Refactoring in onionOrderMatch method correctly handles pointcuts based on annotations.

src/ReflectiveMethodInvocation.php (1)

89-91: Simplification in getNamedArguments method improves readability and performance.

src/TypeString.php (3)

34-52: Handling of different reflection types in __invoke method is correctly implemented.

---

54-63: Conversion of intersection types to string in intersectionTypeToString method is correctly implemented.

---

65-82: Conversion of union types to string in getUnionType method, including handling of nested intersection types, is correctly implemented.

phpcs.xml (1)

Line range hint 1-1: Removal of the exclusion for SlevomatCodingStandard.ControlStructures.EarlyExit.EarlyExitNotUsed aligns with best practices for code readability.

src/MethodSignatureString.php (1)

34-89: Generation of complete method signatures in get method is correctly implemented, including handling of attributes, parameters, and return types.

src/Compiler.php (2)

68-83: Refactoring in compile method improves error handling and efficiency in class compilation.

---

110-121: Improvements in requireFile method ensure efficient handling of compiled class files.

tests-deprecated/CodeGenPhp71Test.php (1)

8-15: Addition of new imports in CodeGenPhp71Test aligns with the new code generation system.

tests/BindTest.php (1)

43-50: Addition of testBindWithConstructor method correctly tests binding functionality with constructor parameters.

sl-src/CacheReader.php (2)

165-165: Type check added before filemtime call enhances robustness.

---

189-189: Consistent type checking before filemtime call in trait modification time retrieval.

tests/AopCodeTest.php (1)

20-120: Enhanced test coverage for method signatures and type declarations.

src/AopCode.php (1)

26-193: Implementation of new code generation system in AopCode class.

tests/CompilerTest.php (1)

115-122: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-193]

Updated class instantiations and removed outdated test method in CompilerTest.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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.

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 a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @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.

[koriym]

@coderabbitai 日本語でもう一度お願いでします。

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@koriym, 以下はプルリクエスト #205の概要です：

プルリクエストの目的

このプルリクエストは、既存の nikic/php-parser をカスタムコード生成システムに置き換えることを目的としています。この変更により、パフォーマンスの向上と依存関係の削減が期待されます。このタスクの複雑さには、リフレクションだけではすべての必要な情報を収集できない（例えば、use ステートメントを取得できない）、token_get_all から取得したトークンで内容を変更することの難しさ、トークンだけを使用して単一のコードベースに親クラスのメソッドを統合する複雑さなどが含まれます。

これらの課題に対処するために、トークンとリフレクションを組み合わせたハイブリッドソリューションが提案されています。これには、クラス宣言にトークンを使用し、特にトークンベースの引数名解決が問題となる場合にメソッド情報を取得するためにリフレクションを使用することが含まれます。

プルリクエストでは、Ray.Aopの10周年を記念するリンクされた問題（#206）も参照されており、この新しいcodegenエンジンの採用に向けたプロジェクトの進化と将来の方向性について議論されています。初期のベンチマークでは、新しいエンジンが現在のPHP-Parserベースのシステムに比べて40倍以上のパフォーマンス向上を提供する可能性が示唆されています。

コメント要約

プルリクエストのコメントで、コントリビューターの koriym はいくつかの強化と修正について言及しています：

• 属性を名前付き引数に変換する問題が解決され、古いコンストラクタの使用（引数を配列形式で受け取る）との互換性が保たれています。
• PHP 8.2のDNF型など、最新のPHP機能に対応しました。
• AopCodeGenを廃止し、AopCode（GeneratedCode）に集約する大規模なリファクタリングが行われました。この変更は、コード生成に関与するクラスの数を減らすことでアーキテクチャを単純化し、オブジェクト指向プログラミングの原則に沿っています。

このプルリクエストは、プロジェクトをカスタムコード生成システムに更新するだけでなく、最新のPHP機能との互換性を向上させ、リファクタリングによってプロジェクト構造を改善し、パフォーマンスを大幅に向上させる可能性があります。

[koriym]

@coderabbitai Recent Review Detailsの部分も日本語でお願いします。

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@koriym, 以下は「Recent Review Details」の日本語訳です：

最近のレビュー詳細

プルリクエストのコメントで、コントリビューターの koriym はいくつかの強化と修正について言及しています：

• 属性を名前付き引数に変換する問題が解決され、古いコンストラクタの使用（引数を配列形式で受け取る）との互換性が保たれています。
• PHP 8.2のDNF型など、最新のPHP機能に対応しました。
• AopCodeGenを廃止し、AopCode（GeneratedCode）に集約する大規模なリファクタリングが行われました。この変更は、コード生成に関与するクラスの数を減らすことでアーキテクチャを単純化し、オブジェクト指向プログラミングの原則に沿っています。

このプルリクエストは、プロジェクトをカスタムコード生成システムに更新するだけでなく、最新のPHP機能との互換性を向上させ、リファクタリングによってプロジェクト構造を改善し、パフォーマンスを大幅に向上させる可能性があります。

[koriym]

@NaokiTsuchiya mergeしました！