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.

Sign up for GitHub

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

Jump to bottom

feat: add rule for missing 'provides' method in Deferrable ServiceProviders #1262

Merged
merged 20 commits into from
Aug 23, 2022
Merged

feat: add rule for missing 'provides' method in Deferrable ServiceProviders #1262

merged 20 commits into from
Aug 23, 2022

Conversation

PrinsFrank
Copy link
Contributor

  • Added or updated tests
  • Documented user facing changes
  • Updated CHANGELOG.md

Changes

This rule will check for a missing 'provides' method in deferrable ServiceProviders.

The 'provides' method is present in the DeferrableProvider interface, but also in the abstract ServiceProvider. If it was not present in the abstract class, this check would not be necessary, but that PR got rejected in Laravel.

Breaking changes

@szepeviktor
Copy link
Collaborator

szepeviktor commented Jun 3, 2022 •
edited
Loading

The problem appears in https://github.com/nunomaduro/larastan/blob/master/tests/Rules/Data/relation-existence-rule.php

@szepeviktor
Copy link
Collaborator

The rule name is also in extension.neon

@PrinsFrank
Copy link
Contributor Author

PrinsFrank commented Jun 3, 2022 •
edited
Loading

@szepeviktor I renamed the test case to confirm this is a test execution order issue. That seems to indeed be the case..

@szepeviktor
Copy link
Collaborator

It seems to be green :)
🍏

@szepeviktor
Copy link
Collaborator

What change in phpunit.xml.dist made tests pass?

@PrinsFrank
Copy link
Contributor Author

What change in phpunit.xml.dist made tests pass?

None, I added the beStrictAboutChangesToGlobalState setting to try to debug what was going on, but the issue ended up being in the way the container is booted and subsequently cached in the PHPStan RuleTestCase. All extensions need to be loaded or the next time the container is used it is missing dependencies. That's why I added the AbstractRuleTestCase.

@PrinsFrank
Copy link
Contributor Author

@szepeviktor @canvural @nunomaduro I'm cleaning up my forks, can I assume this is not going to get merged and close this PR?

@szepeviktor
Copy link
Collaborator

@PrinsFrank Please do not delete your fork.

We will take a look at it and merge it.

@PrinsFrank
Copy link
Contributor Author

@szepeviktor Ok, np!

@canvural canvural changed the title Add rule for missing 'provides' method in Deferrable ServiceProviders feat: add rule for missing 'provides' method in Deferrable ServiceProviders Aug 23, 2022
@canvural canvural merged commit 0a7164a into larastan:master Aug 23, 2022
@canvural
Copy link
Collaborator

Thank you! I made some changes to simplify it little bit.

@PrinsFrank PrinsFrank deleted the add-rule-for-deferrable-provider-validity branch August 23, 2022 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szepeviktor szepeviktor szepeviktor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@PrinsFrank @szepeviktor @canvural