Skip to content

refactor: fix phpstan only boolean allowed#9302

Merged
paulbalandan merged 5 commits intocodeigniter4:developfrom
neznaika0:fix-phpstan-only-boolean-allowed
Dec 28, 2024
Merged

refactor: fix phpstan only boolean allowed#9302
paulbalandan merged 5 commits intocodeigniter4:developfrom
neznaika0:fix-phpstan-only-boolean-allowed

Conversation

@neznaika0
Copy link
Contributor

@neznaika0 neznaika0 commented Dec 5, 2024

Description
Fixed all errors like

Only booleans are allowed in ...

I tried to apply the typing adequately during the checks. For a more accurate fix, you need to update the code instead of phpdoc.
It is possible to additionally check for null, false, <empty-string> somewhere
Fixed 92 errors out of 3925

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

paulbalandan
paulbalandan reviewed Dec 10, 2024
View reviewed changes
@github-actions github-actions bot added the stale Pull requests with conflicts label Dec 11, 2024
@github-actions
Copy link

github-actions bot commented Dec 11, 2024

👋 Hi, @neznaika0!

We detected conflicts in your PR against the base branch 🙊
You may want to sync 🔄 your branch with upstream!

Ref: Syncing Your Branch

@neznaika0 neznaika0 force-pushed the fix-phpstan-only-boolean-allowed branch from f6bb767 to 37a924d Compare December 11, 2024 17:54
@neznaika0 neznaika0 force-pushed the fix-phpstan-only-boolean-allowed branch 2 times, most recently from 0bf76f5 to f3a85de Compare December 11, 2024 19:32
@neznaika0 neznaika0 force-pushed the fix-phpstan-only-boolean-allowed branch from f3a85de to c31d553 Compare December 26, 2024 19:40
@neznaika0 neznaika0 mentioned this pull request Dec 27, 2024
Merged
5 tasks
@paulbalandan paulbalandan removed the stale Pull requests with conflicts label Dec 27, 2024
michalsn
michalsn requested changes Dec 27, 2024
View reviewed changes
Copy link
Member

@michalsn michalsn left a comment

Choose a reason for hiding this comment

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

Why is null typed into a string so many times? In many cases it is not necessary. I was just looking at the code, I don't know if phpstan forces such typing? In many places it is very strange.

@neznaika0
Copy link
Contributor Author

neznaika0 commented Dec 27, 2024

See comment @paulbalandan

I think this should also include check against ''.

Short code for check null or empty string. Empty string is incorrect to pass in the parameter.

@michalsn
Copy link
Member

michalsn commented Dec 27, 2024

@neznaika0 It would be easier if you could respond under the code you're referring to.

Okay, for CLI::print() and CLI::write() it indeed makes sense, but I kindly disagree in other cases.

@neznaika0
Copy link
Contributor Author

neznaika0 commented Dec 27, 2024

If the value can be null, then why can't it be checked for emptiness as ""? I've made changes where an empty line is also invalid.
So there are cases ?string $value
What other explanation is needed?

@michalsn
Copy link
Member

michalsn commented Dec 27, 2024

This is not a general conversation about null and empty strings. If you want to justify the disputed change, do it under the code I commented on. It will make our lives easier.

@neznaika0 neznaika0 force-pushed the fix-phpstan-only-boolean-allowed branch from c31d553 to 0b85e5c Compare December 27, 2024 19:05
@neznaika0 neznaika0 force-pushed the fix-phpstan-only-boolean-allowed branch from 0b85e5c to 26f3fb6 Compare December 28, 2024 08:57
@neznaika0
Copy link
Contributor Author

neznaika0 commented Dec 28, 2024

@michalsn revert comparison with null

michalsn
michalsn approved these changes Dec 28, 2024
View reviewed changes
Copy link
Member

@michalsn michalsn left a comment

Choose a reason for hiding this comment

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

Thank you!

@paulbalandan paulbalandan merged commit 8cff099 into codeigniter4:develop Dec 28, 2024
41 checks passed
@paulbalandan
Copy link
Member

paulbalandan commented Dec 28, 2024

Thank you, @neznaika0

@neznaika0 neznaika0 deleted the fix-phpstan-only-boolean-allowed branch January 8, 2025 19:03
@neznaika0 neznaika0 mentioned this pull request Sep 27, 2025
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michalsn michalsn michalsn approved these changes

@paulbalandan paulbalandan paulbalandan 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.

3 participants

@neznaika0 @michalsn @paulbalandan