Skip to content

Add config option to exclude mixin classes#198

Merged
ondrejmirtes merged 1 commit intophpstan:masterfrom
canvural:exclude-mixin-classes
May 7, 2020
Merged

Add config option to exclude mixin classes#198
ondrejmirtes merged 1 commit intophpstan:masterfrom
canvural:exclude-mixin-classes

Conversation

@canvural
Copy link
Copy Markdown
Contributor

@canvural canvural commented May 7, 2020

From the discussion at larastan/larastan#555

This PR adds a config option to blacklist classes for @mixin annotation.

I did not find an easy way to test this. Because the config file needs to be changed. So, if you can point me how to test this, I will do it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure if $type->describe(VerbosityLevel::typeOnly()) is the best way to get the class names here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's not, because it would skip generic types in @mixin like Foo<T>. The best way is to ask $type instanceof TypeWithClassName and then about $type->getClassName().

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought describe would return Foo<T> if it is called on GenericObjectType.

What about the case when it's not an instance of TypeWithClassName?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I thought describe would return Foo if it is called on GenericObjectType.

Yes, exactly, which is why your code wouldn't work if Foo should be excluded.

I realized you should probably use TypeUtils::getDirectClassNames() which is the easiest and also takes unions into account.

@canvural canvural force-pushed the exclude-mixin-classes branch from 365f49c to b7b4e12 Compare May 7, 2020 10:39
@canvural canvural marked this pull request as ready for review May 7, 2020 10:40
@ondrejmirtes
Copy link
Copy Markdown
Member

Nice, thank you! Please note that once I release this and Larastan gets the configuration in its extension.neon, you also have to bump the minimal required PHPStan version, otherwise PHPStan will crash with an error message that it doesn't know this parameter. (That's why it's added to parametersSchema).

@ondrejmirtes ondrejmirtes merged commit b5a6597 into phpstan:master May 7, 2020
@canvural
Copy link
Copy Markdown
Contributor Author

canvural commented May 7, 2020

Yeah, sure!

Thank you!

@canvural canvural deleted the exclude-mixin-classes branch May 7, 2020 11:13
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