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

Add combination operators to selectors #187

Conversation

gplanchat
Copy link

Following #186

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

No Coverage information No Coverage information
29.3% 29.3% Duplication

@carlosas
Copy link
Owner

Hey! Thank you a lot for the PR!

I had this feature in mind for a long time. Actually the current refactor to phpstan extension that I'm working on it's partially prepared for it https://github.com/carlosas/phpat/tree/phpstan/src/Selector/Modifier

I will review it ASAP in the following days

@gplanchat
Copy link
Author

Hello @carlosas,

I checked the phpstan branch and tried to implement it on this branch. I could not make the tests work.

@carlosas
Copy link
Owner

carlosas commented Jul 2, 2022

I just saw that the last version of PHPStan has changed some signatures, and my fake implementations of some dependencies got broken. I have disabled the unit tests for now and will work on testing properly.

If you want, you could drop a draft PR against the branch v0.10x and we can check manually if the approach works. Then I can help you polishing the feature and tests :)

Take into account that in v0.10 the method ->classes() accepts multiple Selectors and behaves as an OR (see the Test definition section in the new README)
Also I would like to discuss the name of the methods.

I'll close this PR for now. Do you mind opening a new one with the draft phpstan approach?
Thanks a lot!

@carlosas carlosas closed this Jul 2, 2022
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