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

Store original attribute names in DETAIL_TARGET_ANNO #334

Merged
merged 4 commits into from
Jul 22, 2023

Conversation

thekid
Copy link
Member

@thekid thekid commented Jul 22, 2023

This fixes a problem when using old and new reflection libraries in conjunction. This is due to the fact that both use xp::$meta for caching reasons, but fill it differently.

<?php

use web\frontend\Handler;

#[Handler]
class Users {

}

Before

Using the lang.XPClass annotation API will infer with the results returned by lang.reflection.Type:

# Return value as expected:
$ xp -w 'return [...\lang\Reflection::type("Users")->annotations()]'
[web.frontend.Handler => lang.reflection.Annotation<web.frontend.Handler([])>]

# The annotation name is only returned in lowercase!
$ xp -w '\lang\XPClass::forName("Users")->getAnnotations();
return [...\lang\Reflection::type("Users")->annotations()]'
[handler => lang.reflection.Annotation<handler([])>]

After

The lang.XPClass annotation API now also records the original annotation names, and lang.reflection.Type now returns the expected annotation:

$ xp -w '\lang\XPClass::forName("Users")->getAnnotations();
return [...\lang\Reflection::type("Users")->annotations()]'
[web.frontend.Handler => lang.reflection.Annotation<web.frontend.Handler([])>]

@thekid thekid added the bug label Jul 22, 2023
@thekid thekid merged commit b114f3a into xp-framework:main Jul 22, 2023
18 checks passed
@thekid thekid deleted the fix/original-attribute-names branch July 22, 2023 09:52
@thekid
Copy link
Member Author

thekid commented Jul 22, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant