-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Generate the new class synopsis markup #11809
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
Conversation
if ($this->type === "class") { | ||
foreach ($this->implements as $interfaceName) { | ||
$interface = $classMap[$interfaceName->toString()] ?? null; | ||
if ($interface === null) { | ||
throw new Exception("Missing implemented interface " . $interfaceName->toString()); | ||
} | ||
|
||
if ($interface->isException($classMap)) { | ||
return true; | ||
} | ||
} | ||
} |
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.
Is this just to check that Error
and Exception
have an implements Throwable
? that seems kinda overkill? logically every other exception must be extending one of these two classes so maybe it makes more sense to just special case those two classes and let the other loop do its job.
Either case this is also fine.
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.
I didn't want to rely on the fact that exceptions currently either extend Error
or Exception
so that's why I used the more general approach. I'm fine with special casing the logic if you want to.
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.
Eh it's fine with me :)
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.
LGTM
Following php/phd#77