-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
fix attribute resolver on promoted properties #337
fix attribute resolver on promoted properties #337
Conversation
try { | ||
$carry[] = $attribute->newInstance(); | ||
} catch (Error) { | ||
// Do nothing: it is an attribute targeting a parameter promoted to a property |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's assumption that this is the error :). We may be swallowing others errors too.
If we can't check by exception type, can we do an message type check e.g. string contains cannot target property
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
} catch (Error $e) { | ||
if (\preg_match("/Attribute \"(.*)\" cannot target property/", $e->getMessage())) { | ||
// Do nothing: it is an attribute targeting a parameter promoted to a property | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also apply this to TypeResolver#86
? It's other way around there and we ignore all exceptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha, you found it ! I knew it was done elsewhere.
Description
Allow attributes targeting parameters to be used on php>8 promoted properties
Fixes #336
Type of change