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

update rules for phpstan 2 #68

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alekitto
Copy link

@alekitto alekitto commented Feb 19, 2025

I've updated the rules to be compatible with phpstan 2, dropping the support for phpstan 1.

Rules now returns an array of objects (instance of RuleError) instead of strings.
Hints are now added to RuleError object using RuleErrorBuilder::tip method.

Fix #66

Copy link

@mfn mfn left a comment

Choose a reason for hiding this comment

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

Very nice!

Since there's no github action implemented for this repo and I'm unable to see if the test suite was properly executed with this Scrutinizer, I checked out the code locally and let the test suite run -> all green \o/

~/src/phpstan-strict-rules ⎇ phpstan_v2
 $ vendor/bin/phpunit 
PHPUnit 9.6.22 by Sebastian Bergmann and contributors.

......                                                              6 / 6 (100%)

Time: 00:00.737, Memory: 68.00 MB

OK (6 tests, 6 assertions)

Generating code coverage report in Clover XML format ... done [00:00.001]

Generating code coverage report in HTML format ... done [00:00.011]


Code Coverage Report Summary:
  Classes: 100.00% (7/7)     
  Methods: 100.00% (14/14)   
  Lines:   100.00% (120/120) 

@mfn
Copy link

mfn commented Feb 20, 2025

@shish I've seen you active on other repos here, do you have any plans/interest/permission to also handle this repo? Or do you know who can?

@alekitto
Copy link
Author

Since there's no github action implemented for this repo ...

I've implemented a gha workflow on my fork to test if everything works. I can send that too (in a separate PR)

@shish
Copy link

shish commented Feb 20, 2025

do you have any plans/interest/permission to also handle this repo

I was only given access to safe and phpstan-safe-rule -- as far as I'm aware it's only the TCM team themselves who have access to this repo, and they seem very focussed on whatever it is they're doing 😓

@mfn
Copy link

mfn commented Mar 7, 2025

Honest / friendly question: should we fork it?

Those who are using it are locked from upgrading to Laravel 12.

@alekitto
Copy link
Author

Honest / friendly question: should we fork it?

I needed this for my projects, I've forked it (https://packagist.org/packages/kcs/phpstan-strict-rules).
If this will be merged soon, I'll delete the fork, otherwise I can maintain it under a new name.

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.

Support for phpstan 2?
3 participants