Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Code quality improvements powered by PHPStan#6019

Merged
swissspidy merged 22 commits into
mainwp-cli/wp-cli:mainfrom
try/phpstanwp-cli/wp-cli:try/phpstanCopy head branch name to clipboard
Nov 26, 2024
Merged

Code quality improvements powered by PHPStan#6019
swissspidy merged 22 commits into
mainwp-cli/wp-cli:mainfrom
try/phpstanwp-cli/wp-cli:try/phpstanCopy head branch name to clipboard

Conversation

@swissspidy

@swissspidy swissspidy commented Nov 24, 2024

Copy link
Copy Markdown
Member

We can't use PHPStan out of the box right now because of the higher PHP version requirement (see wp-cli/wp-cli-tests#204), so I just ran it manually locally, level by level, fixing bugs along the way.

I went up to level ~5 here.

@swissspidy swissspidy added bug scope:documentation Related to documentation labels Nov 24, 2024
The array returned by `error_get_last` always has this key
The `WP_CLI::error` call above always terminates execution
`$explanation` is never empty at this point
The variable is always set
This method never returns false
The `RequestsLibrary::get_class_name` method never throws
There is an early return above if `self::$logger` isn't set
@swissspidy swissspidy marked this pull request as ready for review November 25, 2024 09:50
@swissspidy swissspidy requested a review from a team as a code owner November 25, 2024 09:50
* Composite commands can only be known by one name.
*
* @return false
* @return string|false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm this seems confusing since all this method does is return false 🧐 I wonder is this tool getting confused and considering the subcommand version of this method which extends CompositeCommand?

/**
* If an alias is set, grant access to it.
* Aliases permit subcommands to be instantiated
* with a secondary identity.
*
* @return string
*/
public function get_alias() {
return $this->alias;
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep exactly. I wouldn't call it confused though, because it's definitely an incorrect documentation.

If the parent method returns only false, a child class returning a string is a type mismatch.

@swissspidy swissspidy merged commit 0187f2b into main Nov 26, 2024
@swissspidy swissspidy deleted the try/phpstan branch November 26, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug scope:documentation Related to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.