goaop / framework

:gem: Go! AOP PHP - modern aspect-oriented framework for the new level of software development
go.aopphp.com
MIT License
1.66k stars 163 forks source link

[Step 1] Bump requirement to PHP 8.2 and parser-reflection 4.0.0-RC1 #492

Closed samsonasik closed 5 months ago

samsonasik commented 7 months ago

@lisachenko here the step 1 PR to upgrade to php 8.2 and upgrade parser-reflection to 4.0.0-RC1

samsonasik commented 7 months ago

@lisachenko there are 2 errors and 5 failures test left, It seems somehow cache not put into file, could you take a look on it? Thank you.

There were 2 errors:

1) Go\Functional\ClassWeavingTest::testPropertyWeaving
TypeError: preg_replace(): Argument #3 ($subject) must be of type array|string, bool given

/home/runner/work/framework/framework/tests/Go/PhpUnit/ProxyClassReflectionHelper.php:50
/home/runner/work/framework/framework/tests/Go/PhpUnit/ClassMemberWovenConstraint.php:38
/home/runner/work/framework/framework/tests/Go/Functional/BaseFunctionalTest.php:322
/home/runner/work/framework/framework/tests/Go/Functional/ClassWeavingTest.php:25

2) Go\Functional\ClassWeavingTest::testClassInitializationWeaving
TypeError: preg_replace(): Argument #3 ($subject) must be of type array|string, bool given

/home/runner/work/framework/framework/tests/Go/PhpUnit/ProxyClassReflectionHelper.php:50
/home/runner/work/framework/framework/tests/Go/PhpUnit/ClassMemberWovenConstraint.php:38
/home/runner/work/framework/framework/tests/Go/Functional/BaseFunctionalTest.php:375
/home/runner/work/framework/framework/tests/Go/Functional/ClassWeavingTest.php:51

--

There were 5 failures:

1) Go\Console\Command\CacheWarmupCommandTest::testItWarmsUpCache
Failed asserting that 'Go\Tests\TestProject\Application\Main' is woven class..

/home/runner/work/framework/framework/tests/Go/Functional/BaseFunctionalTest.php:170
/home/runner/work/framework/framework/tests/Go/Console/Command/CacheWarmupCommandTest.php:30

2) Go\Console\Command\DebugWeavingCommandTest::testReportInconsistentWeaving
Failed asserting that ' Weaving debug information =========================  ' matches PCRE pattern "/.+InconsistentlyWeavedClass.php.+generated.+on.+second.+"warmup".+pass.+/".

/home/runner/work/framework/framework/tests/Go/Console/Command/DebugWeavingCommandTest.php:23

3) Go\Functional\ClassWeavingTest::testItDoesNotWeaveAbstractMethods
Failed asserting that 'Go\Tests\TestProject\Application\Main' is woven class..

/home/runner/work/framework/framework/tests/Go/Functional/BaseFunctionalTest.php:170
/home/runner/work/framework/framework/tests/Go/Functional/ClassWeavingTest.php:38

4) Go\Functional\ClassWeavingTest::testItWeavesFinalClasses
Failed asserting that 'Go\Tests\TestProject\Application\FinalClass' is woven class..

/home/runner/work/framework/framework/tests/Go/Functional/BaseFunctionalTest.php:170
/home/runner/work/framework/framework/tests/Go/Functional/ClassWeavingTest.php:58

5) Go\Functional\Issue293Test::testItDoesNotWeaveDynamicMethodsForComplexStaticPointcut
Failed asserting that 'Go\Tests\TestProject\Application\Issue293StaticMembers' is woven class..

/home/runner/work/framework/framework/tests/Go/Functional/BaseFunctionalTest.php:170
/home/runner/work/framework/framework/tests/Go/Functional/Issue293Test.php:25
samsonasik commented 7 months ago

@lisachenko I debugged cache warmer, and it seems got deprecated notices:

✗ php tests/Fixtures/project/bin/console --no-ansi cache:warmup:aop tests/Fixtures/project/web/../web/index.php
PHP Deprecated:  Creation of dynamic property Go\Core\CachedAspectLoader::$loader is deprecated in /Users/samsonasik/www/framework-3/src/Core/CachedAspectLoader.php on line 89
...
PHP Deprecated:  Return type of Dissect\Lexer\TokenStream\ArrayTokenStream::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /Users/samsonasik/www/framework-3/vendor/jakubledl/dissect/src/Dissect/Lexer/TokenStream/ArrayTokenStream.php on line 114
PHP Deprecated:  Return type of Dissect\Lexer\TokenStream\ArrayTokenStream::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /Users/samsonasik/www/framework-3/vendor/jakubledl/dissect/src/Dissect/Lexer/TokenStream/ArrayTokenStream.php on line 122
PHP Deprecated:  Go\Aop\Framework\BeforeInterceptor implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in /Users/samsonasik/www/framework-3/src/Aop/Framework/BeforeInterceptor.php on line 23
...
PHP Deprecated:  Go\Aop\Framework\AfterInterceptor implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in /Users/samsonasik/www/framework-3/src/Aop/Framework/AfterInterceptor.php on line 23
...
Total 18 files to process.

[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Annotation/Loggable.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/InconsistentlyWeavingAspect.php
[OK]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/WeavingAspect.php
[OK]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/LoggingAspect.php
[OK]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/DoSomethingAspect.php
[OK]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/PropertyInterceptAspect.php
[OK]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/InitializationAspect.php
[OK]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/Issue293Aspect.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/Issue293DynamicMembers.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/Main.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/FinalClass.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/AbstractBar.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/InconsistentlyWeavedClass.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/FooInterface.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/ParentWithFinalMethod.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/Issue293StaticMembers.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Kernel/DefaultAspectKernel.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Kernel/InconsistentlyWeavingAspectKernel.php

[DONE]: Total processed 18, 12 errors.
samsonasik commented 7 months ago

@lisachenko jakubledl/dissectseems too old, it was updated 12 years ago, is it possible to fork it in goaop organization?

lisachenko commented 6 months ago

@lisachenko jakubledl/dissectseems too old, it was updated 12 years ago, is it possible to fork it in goaop organization?

Yes, this library is old, but still works here ) We can keep it as-is, if there are not any failing cases or bugs related to outdated version. It is out of our scope, so we might ignore it now.

lisachenko commented 6 months ago

I'm checking now this branch to see what is failing and where

lisachenko commented 6 months ago

Aha, I see now that utf8_decode is inside ./framework/vendor/jakubledl/dissect/src/Dissect/Util/Util.php on line 46

lisachenko commented 6 months ago

Let's switch here to the https://github.com/WalterWoshid/php-dissect, @WalterWoshid has already created fresh fork

Nope, same error... Deprecated

lisachenko commented 6 months ago

It's pity, but it also contains same error...

samsonasik commented 6 months ago

Ok, updated to use walterwoshid/dissect https://github.com/goaop/framework/pull/492/commits/a1d357caffe8b9b8f19c71d3dadddd218ed9cbf6 , The error on cache warmer seems seame, running got:

php tests/Fixtures/project/bin/console --no-ansi cache:warmup:aop tests/Fixtures/project/web/../web/index.php

got error:

PHP Deprecated:  Function utf8_decode() is deprecated in /Users/samsonasik/www/framework-3/vendor/walterwoshid/dissect/src/Dissect/Util/Util.php on line 46
...
Total 18 files to process.

[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Annotation/Loggable.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/InconsistentlyWeavingAspect.php
[OK]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/WeavingAspect.php
[OK]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/LoggingAspect.php
[OK]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/DoSomethingAspect.php
[OK]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/PropertyInterceptAspect.php
[OK]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/InitializationAspect.php
[OK]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Aspect/Issue293Aspect.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/Issue293DynamicMembers.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/Main.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/FinalClass.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/AbstractBar.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/InconsistentlyWeavedClass.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/FooInterface.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/ParentWithFinalMethod.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Application/Issue293StaticMembers.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Kernel/DefaultAspectKernel.php
[ERR]: /Users/samsonasik/www/framework-3/tests/Fixtures/project/src/Kernel/InconsistentlyWeavingAspectKernel.php

[DONE]: Total processed 18, 12 errors.
➜  framework-3 git:(patch-1) 
lisachenko commented 6 months ago

@samsonasik Should we fork back again to the goaop org then to depend on own dependencies? At least it will be easier to maintain then

samsonasik commented 6 months ago

That will be better, I don't have access for this org so it seems you need to fork so we can patch there

lisachenko commented 6 months ago

Here we go https://github.com/goaop/dissect, let me adjust this fork for our needs...

samsonasik commented 6 months ago

@lisachenko thank you, please let me know if I can do something

lisachenko commented 6 months ago

Granted you admin rights on https://github.com/goaop/dissect, dependency can be temporary used as "goaop/dissect": "^3.0-dev"

lisachenko commented 6 months ago

I can't push directly to your branch to contribute on this PR, check my commits here https://github.com/goaop/framework/compare/samsonasik-patch-1%7E3...samsonasik-patch-1 or cherry-pick what is needed

lisachenko commented 6 months ago

It slowly starts to work again! 🚀 Kudos!

lisachenko commented 6 months ago

I can't push directly to your branch to contribute on this PR, check my commits here samsonasik-patch-1~3...samsonasik-patch-1 or cherry-pick what is needed

Definitely need to pick fixes for Serializable interface in the core and dynamic properties.

samsonasik commented 6 months ago

@lisachenko I cherry-picked your commit 👍 , actually, I checked "Allow edit by maintainers" checkbox so push to my fork should be allowed

Screenshot 2024-02-18 at 18 27 31

samsonasik commented 6 months ago

@lisachenko finally green 🎉 , the phpunit deprecation is expected due to $this usage, but can't be updated as it call non-static so it kept as is

1) Go\Aop\Framework\AbstractJoinpointTest::testSortingLogic
Data Provider method Go\Aop\Framework\AbstractJoinpointTest::sortingTestSource() is not static
samsonasik commented 6 months ago

@lisachenko ready to merge 👍

lisachenko commented 6 months ago

From the demos folder (can be opened in built-in server) almost everything works! 🚀 🕺

Still issues with some parts (sorry, code coverage is not full) - should we look in this PR or in separate?

http://localhost:8080/?showcase=dynamic-interceptor http://localhost:8080/?showcase=dynamic-traits http://localhost:8080/?showcase=declare-errors (it should generate warning, but doesn't generate it...)

samsonasik commented 6 months ago

Yes, I think separate PR is better 👍 , just let me know what step by step to run the demo

lisachenko commented 6 months ago

Yes, I think separate PR is better 👍

Ok, good, then let me work then in the evening today (need to go now), I would like to give some love/cleaning to the both goaop/parser-reflection and goaop/dissect first and try to use rector for that ) Then we will publish RC tags for dependencies and update this PR again to use stable dependecies.

lisachenko commented 6 months ago

just let me know what step by step to run the demo

It is very easy, just follow https://github.com/goaop/framework?tab=readme-ov-file#step-0-optional-try-demo-examples-in-the-framework (but you can also just simply run the built-in PHP web server on 8080 port and point it to the demos/ folder and then open localhost:8080 in your browser (or even put breakpoints in demos/ folder, etc)

lisachenko commented 6 months ago

goaop/dissect part is ready - package is published, CI tool configured, cleaned a little bit with rector. Marked as major release 3.0.0 because we are bumping PHP requirement >=8.2

lisachenko commented 6 months ago

With my current available time goaop/parser-reflection cleaning/polishing will take some time too, please be patient ) Our final victory is near, I feel this

samsonasik commented 6 months ago

@lisachenko it is mergeable?

lisachenko commented 6 months ago

@lisachenko it is mergeable?

Not yet, need to clean a little bit first the goaop/parser-reflection and release stable version to replace "goaop/parser-reflection": "4.x-dev" with something more stable, dev dependencies shouldn't be merged into the master branch

lisachenko commented 6 months ago

@samsonasik FYI: During clean-up yesterday, I've identified serious logic break inside the NodeExpressionResolver class in dependant parser-reflection component. When I'm trying to get real value for constant fetches, it started to return string names instead of values due to the changes inside resolveExprConstFetch (I guess, you were trying to make __toString works, but for this you should try to use getConstantName).

I'm trying to fix issues with value resolver now.

uzmasterdev commented 6 months ago

@samsonasik @lisachenko very happy for there is something happening, I really enjoy with aspect-mock and glad to use it with newest php versions but see that is depends on this package. Thank you for your work, guys!

lisachenko commented 5 months ago

@samsonasik Here we go, this is starting point now, all main dependencies are looking good now to create something good on top of this.

Tests partially broken now, but at least all internal part now works as expected. I might join this PR later, but can't promise if it will be quick enough, because goaop/parser-reflection has also requested significant work from me to make it usable for modern PHP8.3 codebase with all Union/Intersection/DNF types.

scrutinizer-notifier commented 5 months ago

A new inspection was created.

lisachenko commented 5 months ago

Merged, with support for attributes instead of annotations

samsonasik commented 5 months ago

Thank you @lisachenko for taking care of it 🙏