API Allow to filter for classes which are annoted by an attribute - #11997
beerbohmdo wants to merge 1 commit into
Conversation
|
I forgot to add tests, I will provide them. /edit: Done |
f448972 to
e43af19
Compare
e873239 to
a187d79
Compare
|
The CI fails but I am not sure if that is my fault. |
a187d79 to
af94d16
Compare
ChloeSartorelli
left a comment
There was a problem hiding this comment.
I also added something which bothers me for sometime: You currently can't use implementorsOf for interfaces which extends another interface :/. You have to know the child interfaces and call implementsOf manually for all child interfaces ...
Please split this into at least a separate commit if not a separate PR. It's a completely different concern that should ideally be reviewed on its own merits.
| /** | ||
| * Get the list of classes which are annonated by a given attribute | ||
| * | ||
| * @param class-string $attribute Name of a attribute class or an interface |
There was a problem hiding this comment.
Why "or an interface"? Isn't this about attributes specifically?
There was a problem hiding this comment.
Attributes are classes and these can have interfaces.
When you define custom attributes you normally use a composision pattern instead of a subclass pattern.
For example if you want to create a Validator attribute, you would create a Validator interface and than Attributes for Max, Min, MaxLength, MinLength, etc.
interface Validator
{
public function validate(ValidationResult $result, string $name, mixed $value): void;
}
#[Attribute(Attribute::TARGET_PARAMETER|Attribute::TARGET_PROPERTY)]
final class Max implements Validator
{
public function __construct(
public readonly int $max,
public readonly ?string $message = null,
) {
}
public function validate(ValidationResult $result, string $name, mixed $value): void
{
if ($value > $this->max) {
$result->addFieldError(
$name,
$this->message ?? $name . ' must be less than or equal to ' . $this->max
);
}
}
}
#[Attribute(Attribute::TARGET_PARAMETER|Attribute::TARGET_PROPERTY)]
final class MaxLength implements Validator
{
public function __construct(
public readonly int $maxLength,
public readonly ?string $message = null
) {
}
public function validate(ValidationResult $result, string $name, mixed $value): void
{
if (strlen($value) > $this->maxLength) {
$result->addFieldError(
$name,
$this->message ?? $name . ' must not exceed ' . $this->maxLength . ' characters'
);
}
}
}| * Get the list of classes which are annonated by a given attribute | ||
| * | ||
| * @param class-string $attribute Name of a attribute class or an interface | ||
| * @param bool $instanceOf Should subclasses of the Attribute be included? |
There was a problem hiding this comment.
$allowSubclasses seems like a more directly applicable name for that.
There was a problem hiding this comment.
instanceOf is the name php uses in Reflection 🤷, so I kept it.
| * @param "classes"|"interfaces"|"traits"|"enums" $type Which type of structure should returned, defaults to "classes" | ||
| * @return array | ||
| */ | ||
| public static function annotatedBy(string $attribute, bool $instanceOf = true, string $type = 'classes'): array |
There was a problem hiding this comment.
| public static function annotatedBy(string $attribute, bool $instanceOf = true, string $type = 'classes'): array | |
| public static function classesWithAttribute(string $attribute, bool $instanceOf = true, string $type = 'classes'): array |
I think this method name is more immediately applicable - if I don't remember the method name but know I want to get classes with this attribute, I'll likely type ClassInfo::attribute and let my IDE find the method for me.
| * | ||
| * @param class-string $attribute Name of a attribute class or an interface | ||
| * @param bool $instanceOf Should subclasses of the Attribute be included? | ||
| * @param "classes"|"interfaces"|"traits"|"enums" $type Which type of structure should returned, defaults to "classes" |
There was a problem hiding this comment.
This isn't a concept we have elsewhere in ClassInfo - can you please explain why we're including it here? Seems a bit outside the existing patterns.
There was a problem hiding this comment.
Theoretically all these types can have Attributes ... but normally you would only need it for classes.
The alternative would be a method for every type which only differs in one keyword and that for a use case which may never be needed.
| * @param class-string $attribute Name of a attribute class or an interface | ||
| * @param bool $instanceOf Should subclasses of the Attribute be included? | ||
| * @param "classes"|"interfaces"|"traits"|"enums" $type Which type of structure should returned, defaults to "classes" | ||
| * @return array |
There was a problem hiding this comment.
| * @return array |
We don't need PHPDocs that just repeat exactly what the method signature says.
| * @param string $className | ||
| * @param string $attributeName | ||
| * @param bool $instanceOf | ||
| * @return array |
There was a problem hiding this comment.
| * @param string $className | |
| * @param string $attributeName | |
| * @param bool $instanceOf | |
| * @return array |
Please give this method a PHPDoc description so the API docs explain what this method does.
I use this internally in the getAnnotatedBy method. I can split that, but than this PR would rely on the other. |
4cb28ea to
f5d1099
Compare
f5d1099 to
fa873fa
Compare
Implements #11996
Description
Add support for attributes to the class manifest. So that you get all classes which are annotated by a given attribute.
I also added something which bothers me for sometime: You currently can't use implementorsOf for interfaces which extends another interface :/. You have to know the child interfaces and call implementsOf manually for all child interfaces ...
Manual testing steps
Issues
Pull request checklist