framework icon indicating copy to clipboard operation
framework copied to clipboard

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

Open samsonasik opened this issue 1 year ago • 31 comments

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

samsonasik avatar Feb 04 '24 12:02 samsonasik

@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 avatar Feb 10 '24 07:02 samsonasik

@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 avatar Feb 10 '24 07:02 samsonasik

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

samsonasik avatar Feb 10 '24 09:02 samsonasik

@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 avatar Feb 18 '24 09:02 lisachenko

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

lisachenko avatar Feb 18 '24 09:02 lisachenko

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

lisachenko avatar Feb 18 '24 09:02 lisachenko

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

Nope, same error... Deprecated

lisachenko avatar Feb 18 '24 10:02 lisachenko

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

lisachenko avatar Feb 18 '24 10:02 lisachenko

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) 

samsonasik avatar Feb 18 '24 10:02 samsonasik

@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

lisachenko avatar Feb 18 '24 10:02 lisachenko

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

samsonasik avatar Feb 18 '24 10:02 samsonasik

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

lisachenko avatar Feb 18 '24 10:02 lisachenko

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

samsonasik avatar Feb 18 '24 10:02 samsonasik

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

lisachenko avatar Feb 18 '24 11:02 lisachenko

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 avatar Feb 18 '24 11:02 lisachenko

It slowly starts to work again! 🚀 Kudos!

lisachenko avatar Feb 18 '24 11:02 lisachenko

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.

lisachenko avatar Feb 18 '24 11:02 lisachenko

@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 avatar Feb 18 '24 11:02 samsonasik

@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 avatar Feb 18 '24 11:02 samsonasik

@lisachenko ready to merge 👍

samsonasik avatar Feb 18 '24 11:02 samsonasik

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

lisachenko avatar Feb 18 '24 11:02 lisachenko

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

samsonasik avatar Feb 18 '24 12:02 samsonasik

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 avatar Feb 18 '24 12:02 lisachenko

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 avatar Feb 18 '24 12:02 lisachenko

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 avatar Feb 19 '24 21:02 lisachenko

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

lisachenko avatar Feb 19 '24 21:02 lisachenko

@lisachenko it is mergeable?

samsonasik avatar Feb 19 '24 22:02 samsonasik

@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 avatar Feb 20 '24 08:02 lisachenko

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

lisachenko avatar Feb 25 '24 10:02 lisachenko

@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!

uzmasterdev avatar Mar 01 '24 09:03 uzmasterdev