Skip to content

Conversation

@kukulich
Copy link
Collaborator

@kukulich kukulich commented Sep 23, 2021

@kukulich
Copy link
Collaborator Author

kukulich commented Sep 23, 2021

ReflectionAttribute is final so I don't know how to implement the adapter: https://www.php.net/manual/en/class.reflectionattribute.php

@kukulich kukulich force-pushed the attributes branch 3 times, most recently from 9fe2c0a to 6659152 Compare September 24, 2021 08:19
@asgrim
Copy link
Member

asgrim commented Sep 24, 2021

ReflectionAttribute is final so I don't know how to implement the adapter: https://www.php.net/manual/en/class.reflectionattribute.php

I guess for now we just can't have an adapter for ReflectionAttribute, not a lot we can do about it I think :/

@kukulich
Copy link
Collaborator Author

@ondrejmirtes is trying to persuade PHP developers to remove the final.

@ondrejmirtes
Copy link
Contributor

I¨ve asked in Room 11: https://chat.stackoverflow.com/transcript/message/53093466#53093466

I'll open a bug if no one responds there.

@Ocramius
Copy link
Member

I think opening a bug would indeed be the best way forward: room 11 is full of helpful people, but if a change comes exclusively from there, it will be frown upon by internals

@ondrejmirtes
Copy link
Contributor

Alright, let's see: https://bugs.php.net/bug.php?id=81474

@kukulich kukulich force-pushed the attributes branch 3 times, most recently from d8d94f1 to 263a1df Compare September 25, 2021 17:15
@Ocramius
Copy link
Member

BTW, we could decide to merge without adapter support, and split that part of the problem out, for now

@kukulich
Copy link
Collaborator Author

Yes, I need to rebase it now and will send it to review.

@kukulich kukulich marked this pull request as ready for review September 25, 2021 20:33
@kukulich kukulich mentioned this pull request Sep 25, 2021
13 tasks
@kukulich
Copy link
Collaborator Author

@Ocramius This is ready to review. I would add getClass() later.

@donquixote
Copy link

A quick observation about this effort:

  • ReflectionAttribute in the PHPStan fork, master branch, does support PHP < 8, but it does not support named arguments for attributes.
  • This PR does appear to support named arguments, but is starting from a version of BetterReflection that requires PHP 8+.

I was hoping for a tool that would bring attributes to PHP < 8.

@andrew-demb
Copy link

@donquixote here it is https://github.com/spiral/attributes

@Ocramius
Copy link
Member

@kukulich meanwhile, php:8.0.12 is available since yesterday \o/

@Ocramius Ocramius added this to the 5.0.0 milestone Oct 22, 2021
@kukulich
Copy link
Collaborator Author

@Ocramius Yes, but even 24266a0 does not help to run the actions. We have to wait...

@kukulich kukulich force-pushed the attributes branch 2 times, most recently from 576f00e to b0c9f78 Compare October 22, 2021 15:17
@Ocramius
Copy link
Member

Looks like 8.0.12 finally made it into the PPA:

Therefore, https://github.com/shivammathur/setup-php is also picking it up now :)

@Ocramius
Copy link
Member

One failure left:



Time: 00:25.012, Memory: 1.94 GB

There was 1 failure:

1) Roave\BetterReflectionTest\Reflection\Adapter\ReflectionAttributeTest::testCoreReflectionMethods with data set "__toString" ('__toString')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'Roave\BetterReflection\Reflection\Adapter\ReflectionAttribute'
+'ReflectionAttribute'

/home/runner/work/BetterReflection/BetterReflection/test/unit/Reflection/Adapter/ReflectionAttributeTest.php:38

FAILURES!
Tests: 11466, Assertions: 69307, Failures: 1, Skipped: 6.
Error: Process completed with exit code 1.

@kukulich kukulich marked this pull request as ready for review October 24, 2021 13:57
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

@Ocramius Ocramius self-assigned this Oct 24, 2021
@Ocramius Ocramius merged commit cd5efcf into Roave:5.0.x Oct 24, 2021
@kukulich kukulich deleted the attributes branch October 24, 2021 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants