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

Make ClassLike::from return type assert the subclass type #154

Merged
merged 3 commits into from
Mar 7, 2024

Conversation

GromNaN
Copy link
Contributor

@GromNaN GromNaN commented Feb 26, 2024

  • new feature
  • BC break? yes
  • doc PR: nette/docs#???

When reading a ClassType from a FQCN or a file, the result can be anything from the 4 subclasses. When we know the type of the object that is parsed, having a more strict return type has additional benefits:

  • Assert the object type, and throw a PHP error if the type is not the expected one.
  • Helps static analysis to identifies the methods that can be used on the object (IDE completion, error detection)

Usage:
If the type is unknown, use ClassLike::from($class).
Otherwise, when the expected type is known, use the more specific ClassType::from($fqcn), TraitType::from($fqcn), InterfaceType::from($fqcn), or EnumType::from($fqcn).

This is a breaking change for users that call ClassType::from($fqcn) on something that is not a class name.

@GromNaN GromNaN changed the title Make ClassLike::from return type assert the type Make ClassLike::from return type assert the subclass type Feb 26, 2024
->fromClassReflection(new \ReflectionClass($class), $withBodies);

if (!$class instanceof static) {
throw new Nette\InvalidArgumentException("Object '$class' is not an instance of " . static::class);
Copy link
Member

Choose a reason for hiding this comment

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

The error message should be different so that the user understands what he is doing wrong. And please create a test case for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'm thinking of a message that provides better guidance:

"Trait1" is not a "ClassType". Use TraitType::from() or ClassLike::from() instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test added and exception message improved. Is that all right?

@GromNaN GromNaN requested a review from dg March 7, 2024 11:25
@dg
Copy link
Member

dg commented Mar 7, 2024

Thanks!

@dg dg merged commit 42279b7 into nette:master Mar 7, 2024
8 of 10 checks passed
@dg
Copy link
Member

dg commented Mar 7, 2024

Since this is a BC break, for now I'll just put a warning instead of throwing exceptions, and I'll put this change in the bigger version.

@GromNaN GromNaN deleted the from-class-type branch March 8, 2024 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants