Skip to content

GH-19153: Validate #[\Attribute] targets #19154

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DanielEScherzer
Copy link
Member

Do not allow #[\Attribute] on traits, interfaces, enums, or abstract classes.

Do not allow #[\Attribute] on traits, interfaces, enums, or abstract classes.
@DanielEScherzer
Copy link
Member Author

Targeting master for the same reason as #15733 did - this adds a new error where previously there wasn't one until you tried to make use of the attribute, at which point ReflectionAttribute::newInstance() would complain that you can't instantiate a trait/enum/interface/abstract class

zend_attribute *attr, uint32_t target, zend_class_entry *scope)
{
const char *msg = NULL;
if (scope->ce_flags & ZEND_ACC_TRAIT) {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: You can use zend_get_object_type_case() for most of these.

Copy link
Member Author

Choose a reason for hiding this comment

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

But that will return "Class" (or "class") if its not a trait/interface/enum, and so

  • we still need to check if it is a trait/interface/enum
  • we still need to check if it is an abstract class

it would look something like

const char *type = NULL;
if (scope->ce_flags & (ZEND_ACC_TRAIT|ZEND_ACC_INTERFACE|ZEND_ACC_ENUM)) {
	type = zend_get_object_type_case(scope, false);
} else if (scope->ce_flags & ZEND_ACC_EXPLICIT_ABSTRACT_CLASS) {
	type = "abstract class";
}
if (type != NULL) {
	zend_error_noreturn(E_ERROR, "Cannot apply #[Attribute] to %s %s", type, ZSTR_VAL(scope->name));
}

and I don't think that that is more readable

Copy link
Member

Choose a reason for hiding this comment

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

It's more about keeping the wording consistent, but I'm ok either way.

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.

#[\Attribute] validation should error on trait/interface/enum/abstract class
2 participants