-
-
Notifications
You must be signed in to change notification settings - Fork 416
[depre] Deprecate strict-types rules as risky and not practical #7523
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
Conversation
7f08705 to
ac7227d
Compare
942cce2 to
636c14f
Compare
| $node->cond, | ||
| $this->treatAsNonEmpty | ||
| throw new ShouldNotHappenException( | ||
| 'This rule is deprecated and will be removed as risky and requires manual checking' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't know what "this rule" is, should mention self::class instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the output error? I think deprecated rules are reported on Rector run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this show:
"/home/runner/work/CodeIgniter4/CodeIgniter4/app/Config/Mimes.php"
file, due to:
"System error: "This rule is deprecated and will be removed as risky
and requires manual checking"
Run Rector with "--debug" option and post the report here: https://github.com/rectorphp/rector/issues/new". On line:
62
see https://github.com/codeigniter4/CodeIgniter4/actions/runs/18720569673/job/53391545753#step:12:48
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, that seems fine then 👍

Revisiting couple rules I've added in #955 that are risky as assuming inner empty type. These rules were made to resolve https://github.com/phpstan/phpstan-strict-rules. As Rector moves to more practical rather academical area, these ruels would be best fit in own niche community ruleset.
Now we have better, more stable strategy to improve code quality like:
Those should be used instead to have upgrade path under control.
The only practical rule remains -
DisallowedEmptyRuleFixerRector. It might need split to multiple tailored rules, but works well for now. This one is already part of code quality set 👍