-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Fix PHPStan level 0 issues in themes #10951
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
Changes from all commits
c1b4653
417f257
2edcfeb
fe5cc0f
3d0b3e9
2848245
cfb02ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,7 +29,7 @@ | |
| * | ||
| * @param array $args Arguments for displaying the site logo either as an image or text. | ||
| * @param bool $display Display or return the HTML. | ||
| * @return string Compiled HTML based on our arguments. | ||
| * @return string|void Compiled HTML based on our arguments. | ||
| */ | ||
| function twentytwenty_site_logo( $args = array(), $display = true ) { | ||
| $logo = get_custom_logo(); | ||
|
|
@@ -107,7 +107,7 @@ function twentytwenty_site_logo( $args = array(), $display = true ) { | |
| * @since Twenty Twenty 1.0 | ||
| * | ||
| * @param bool $display Display or return the HTML. | ||
| * @return string The HTML to display. | ||
| * @return string|void The HTML to display. | ||
| */ | ||
| function twentytwenty_site_description( $display = true ) { | ||
| $description = get_bloginfo( 'description' ); | ||
|
|
@@ -249,7 +249,7 @@ function twentytwenty_edit_post_link( $link, $post_id, $text ) { | |
| * | ||
| * @param int $post_id The ID of the post. | ||
| * @param string $location The location where the meta is shown. | ||
| * @return string Post meta HTML. | ||
| * @return string|void Post meta HTML. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function can return void in two situations, but I think returning an empty string could be better when there is no HTML to return. Maybe it's unwise to change that now, though.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, for this it seems best to document the existing behavior. |
||
| */ | ||
| function twentytwenty_get_post_meta( $post_id = null, $location = 'single-top' ) { | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -181,18 +181,17 @@ public static function get_svg( $group, $icon, $size ) { | |||||
| * | ||||||
| * @since Twenty Twenty-One 1.0 | ||||||
| * | ||||||
| * @param array $arr Array of icons. | ||||||
| * @param array<string, string> $arr Array of icons. | ||||||
| */ | ||||||
| $arr = apply_filters( "twenty_twenty_one_svg_icons_{$group}", $arr ); | ||||||
|
|
||||||
| $svg = ''; | ||||||
| if ( array_key_exists( $icon, $arr ) ) { | ||||||
| if ( isset( $arr[ $icon ] ) && is_string( $arr[ $icon ] ) ) { | ||||||
| $repl = sprintf( '<svg class="svg-icon" width="%d" height="%d" aria-hidden="true" role="img" focusable="false" ', $size, $size ); | ||||||
|
|
||||||
| $svg = preg_replace( '/^<svg /', $repl, trim( $arr[ $icon ] ) ); // Add extra attributes to SVG code. | ||||||
| $svg = (string) preg_replace( '/^<svg /', $repl, trim( $arr[ $icon ] ) ); // Add extra attributes to SVG code. | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The pattern and
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue that PHPStan is warning about is that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I prefer checking
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
| } | ||||||
|
|
||||||
| // @phpstan-ignore-next-line. | ||||||
| return $svg; | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -866,6 +866,14 @@ public function cache_delete() { | |||
| * | ||||
| * @param string $header Theme header. Name, Description, Author, Version, ThemeURI, AuthorURI, Status, Tags. | ||||
| * @return string|array|false String or array (for Tags header) on success, false on failure. | ||||
| * | ||||
| * @phpstan-return ( | ||||
| * $header is 'Tags' | ||||
| * ? string[]|false | ||||
| * : ( $header is 'Name'|'ThemeURI'|'Description'|'Author'|'AuthorURI'|'Version'|'Template'|'Status'|'TextDomain'|'DomainPath'|'RequiresWP'|'RequiresPHP'|'UpdateURI' | ||||
| * ? string|false | ||||
| * : false ) | ||||
| * ) | ||||
|
Comment on lines
+869
to
+876
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, because it fixes issues with calls to
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For example:
PHPStan is flagging this:
With the inclusion of the |
||||
| */ | ||||
| public function get( $header ) { | ||||
| if ( ! isset( $this->headers[ $header ] ) ) { | ||||
|
|
||||
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 seems right. For history, this was already in r25808, when the
Featured_Contentclass was added.