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

phpstan level 7 for feedController.php #5373

Merged

Conversation

ColonelMoutarde
Copy link
Contributor

@ColonelMoutarde ColonelMoutarde commented May 5, 2023

Changes proposed in this pull request:

  • phpstan level 7 for feedController.php

Pull request checklist:

  • clear commit messages
  • code manually tested

@ColonelMoutarde ColonelMoutarde force-pushed the feature/phpstan7-for-feedController branch from 2996705 to 4471d92 Compare May 5, 2023 09:13
@Alkarex Alkarex added this to the 1.22.0 milestone May 5, 2023
@Alkarex Alkarex added the System care Everything related to the system care label May 5, 2023
@@ -564,21 +575,21 @@ public static function actualizeFeed(int $feed_id, string $feed_url, bool $force
}

if ($simplePie != null) {
if ($feed->name(true) == '') {
if ($feed->name(true) === '') {
Copy link
Member

Choose a reason for hiding this comment

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

It does not matter much here, but this is another example where === is less safe.
Imagine we change ->name() to return null if undefined or false in case of error (which is quite common), and the new code will lead to a bug, while the previous code was safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't matter much here, but this is another example where === is less safe.
Let's say we change ->name() so that it returns null if undefined or false on error (which is quite common), and the new code will lead to a bug, whereas the previous code was safe.

Here is the function that is called when we do ->name();

public function name(bool $raw = false): string {
		return $raw || $this->name != '' ? $this->name : preg_replace('%^https?://(www[.])?%i', '', $this->url);
	}

In any case we have a string in output if the function, what you do not want is that your string is of type: ""

Copy link
Member

Choose a reason for hiding this comment

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

I know what we have now. My point was that it could change, and null / false are quite common

@Alkarex
Copy link
Member

Alkarex commented May 5, 2023

A few fixes. Double-check welcome fef7939

@ColonelMoutarde
Copy link
Contributor Author

allright for me

@Alkarex Alkarex merged commit f90cd80 into FreshRSS:edge May 7, 2023
1 check passed
math-GH pushed a commit to math-GH/FreshRSS that referenced this pull request Jul 4, 2023
* phpstan level 7 for feedController.php

* phpstan level 7 for feedController.php

* phpstan level 7 for feedController.php

* phpstan level 7 for feedController.php

* A few fixes

---------

Co-authored-by: Luc <sanchezluc+freshrss@gmail.com>
Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
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