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

fix many "Only booleans are allowed in an if condition" #5501

Merged

Conversation

ColonelMoutarde
Copy link
Contributor

Changes proposed in this pull request:

  • By resetting the "booleansInConditions" directive to true, 370 errors are detected. I've corrected 20 of the errors that phpstan is reporting. There are still many...

Pull request checklist:

  • clear commit messages
  • code manually tested

@Alkarex Alkarex added this to the 1.22.0 milestone Jun 29, 2023
cli/create-user.php Outdated Show resolved Hide resolved
p/api/greader.php Outdated Show resolved Hide resolved
@Alkarex Alkarex added the System care Everything related to the system care label Jun 29, 2023
ColonelMoutarde and others added 3 commits June 29, 2023 17:16
Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
$ex->getMessage(),
$db['user'], Minz_Exception::ERROR
);
}
Copy link
Member

Choose a reason for hiding this comment

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

What happened here? This looks quite wrong

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if I don't do this phpstan returns an error because $ex is unknown at this point

Copy link
Member

Choose a reason for hiding this comment

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

I have reverted without problem, as far as I can see. But the point was that the logic was broken with your changes.

lib/Minz/Pdo.php Outdated Show resolved Hide resolved
lib/Minz/Request.php Outdated Show resolved Hide resolved
lib/lib_rss.php Outdated Show resolved Hide resolved
lib/favicons.php Outdated Show resolved Hide resolved
@Alkarex
Copy link
Member

Alkarex commented Jul 3, 2023

I have fixed several regressions and other smaller details

@Alkarex
Copy link
Member

Alkarex commented Jul 5, 2023

Let's not hurry making more changes of this type, especially because I believe that it leads to less readable and more error-prone code in many situations, for instance the calls to preg_grep(), preg_match(). Indeed, this original PR had several regressions, which I am guessing were at least partially due to the more complicated / less readable syntax.
I think we should relax the booleansInConditions rule to accept { 0, 1 } (and maybe null) in addition of { true, false }.
phpstan/phpstan-strict-rules#221

lib/http-conditional.php Outdated Show resolved Hide resolved
@ColonelMoutarde
Copy link
Contributor Author

Let's not hurry making more changes of this type, especially because I believe that it leads to less readable and more error-prone code in many situations, for instance the calls to preg_grep(), preg_match(). Indeed, this original PR had several regressions, which I am guessing were at least partially due to the more complicated / less readable syntax. I think we should relax the booleansInConditions rule to accept { 0, 1 } (and maybe null) in addition of { true, false }. phpstan/phpstan-strict-rules#221

hello, the problem is polymorphism. it must be reduced as much as possible. a function must return only one type.

See :
https://www.phptutorial.net/php-oop/php-polymorphism/

if a native php function is polymorphic (for historical reasons), the best thing to do is to create a method that encapsulates and returns only what is necessary.

@Alkarex Alkarex merged commit 7f9594b into FreshRSS:edge Jul 7, 2023
1 check passed
marienfressinaud added a commit to flusio/FreshRSS that referenced this pull request Sep 22, 2023
`preg_grep` returns an empty array if the username matches no elements
from the usernames array.

Regression introduced in 7f9594b

Reference: FreshRSS#5501
Alkarex pushed a commit that referenced this pull request Sep 22, 2023
`preg_grep` returns an empty array if the username matches no elements
from the usernames array.

Regression introduced in 7f9594b

Reference: #5501
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
System care Everything related to the system care
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants