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

Do not blame internal classes as uncovered cases #310

Closed
wants to merge 4 commits into from
Closed

Do not blame internal classes as uncovered cases #310

wants to merge 4 commits into from

Conversation

hugochinchilla
Copy link
Contributor

No description provided.


private function isInternalClass(string $class): bool
{
if (is_a($class, \Throwable::class, true)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this doesn't work with custom exceptions which extends \Exception or implements \Throwable. Would be interface_exists an alternative?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, it's fixed now.

@smoench
Copy link
Contributor

smoench commented Jun 9, 2020

@hugochinchilla Currently I try to integrate roave/better-reflection #312. It has the concept about SourceStubbers. One of them is the https://github.com/JetBrains/phpstorm-stubs, which represents all internal classes. Probably it won't make it in the 0.8 release

@smoench
Copy link
Contributor

smoench commented Jun 9, 2020

@hugochinchilla I provided another approach with #314. WDYT?

@hugochinchilla
Copy link
Contributor Author

@smoench it looks good, but I don't understand the need for an external dependency to get the internal classes. Is it because reflection may be slower?

I like how you thought of making the change of behavior configurable, I missed that.

@smoench
Copy link
Contributor

smoench commented Jun 10, 2020

ReflectionClass, interface_exists and class_exists depends on enabled extensions. So if the pdo extension is not enabled the PDO class would not be recognised as internal.

@hugochinchilla
Copy link
Contributor Author

good catch, I didn't think of that.

@smoench
Copy link
Contributor

smoench commented Jun 10, 2020

Thank you @hugochinchilla

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.

None yet

2 participants