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.

Sign up for GitHub

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

Jump to bottom

Fix problems reported by static analysis #19

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Fix problems reported by static analysis #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3cb38c
Fix problems reported by static analysis
szepeviktor Nov 29, 2023
2a0573d
Revert phpstan
szepeviktor Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions src/class-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,10 @@ protected function set_migratability() {
/**
* Skip migration checks while doing cron.
*
* @param bool $skip_during_cron Whether to skip.
* @param WP_Theme_Migrator $this This WP_Theme_Migrator instance.
* @param bool $skip_during_cron Whether to skip.
* @param WP_Theme_Migrator_WP $instance This WP_Theme_Migrator instance.
*/
if ( apply_filters( 'wp_theme_migrator_skip_during_cron', true, $this ) ) {
if ( apply_filters( 'wp_theme_migrator_skip_during_cron', true, $this->wp ) ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we need a WP instance not a Controller

return;
}
}
Expand All @@ -117,10 +117,10 @@ protected function set_migratability() {
/**
* Skip migration checks while doing Ajax.
*
* @param bool $skip_during_ajax Whether to skip.
* @param WP_Theme_Migrator $this This WP_Theme_Migrator instance.
* @param bool $skip_during_ajax Whether to skip.
* @param WP_Theme_Migrator_WP $instance This WP_Theme_Migrator instance.
*/
if ( apply_filters( 'wp_theme_migrator_skip_during_ajax', true, $this ) ) {
if ( apply_filters( 'wp_theme_migrator_skip_during_ajax', true, $this->wp ) ) {
return;
}
}
Expand Down Expand Up @@ -230,7 +230,7 @@ protected function check_migratability(): bool {
/**
* Runs when a request is migratable.
*
* @param Controller $this Controller instance.
* @param Controller $instance Controller instance.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

parameter naming

*/
do_action( 'wp_theme_migrator_migrating', $this );
return true;
Expand All @@ -240,7 +240,7 @@ protected function check_migratability(): bool {
/**
* Runs when a request is not migratable.
*
* @param Controller $this Controller instance.
* @param Controller $instance Controller instance.
*/
do_action( 'wp_theme_migrator_not_migrating', $this );

Expand All @@ -252,7 +252,7 @@ protected function check_migratability(): bool {
*/
protected function stop_migrator() {
remove_action( 'setup_theme', [ $this, 'run' ], 8 );
remove_action( 'registered_post_type', [ $this, 'add_post_type_query_vars' ], 10, 2 );
remove_action( 'registered_post_type', [ $this, 'add_post_type_query_vars' ], 10 );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only add_action has that last one

}

/**
Expand All @@ -264,7 +264,7 @@ protected function stop_migrator() {
* @param \WP_Post_Type $post_type_object Arguments used to register the post type.
*/
public function add_post_type_query_vars( $post_type, $post_type_object ) {
if ( false !== $post_type_object->query_var && $this->wp && is_post_type_viewable( $post_type_object ) ) {
if ( false !== $post_type_object->query_var && is_post_type_viewable( $post_type_object ) ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

$this->wp is always set in the constructor

$this->wp->add_query_var( $post_type_object->query_var );
}
}
Expand Down
8 changes: 2 additions & 6 deletions src/trait-theme-helpers.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,7 @@ protected function is_valid_theme( string $theme ): bool {
}

// Check that the theme exists.
if ( false === ( wp_get_theme( $theme )?->exists() ?? false ) ) {
return false;
}

return true;
return wp_get_theme( $theme )->exists();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wp_get_theme always returns a WP_Theme instance

}

/**
Expand All @@ -52,7 +48,7 @@ protected function get_theme( string $theme, string $prop = '' ) {

switch ( $prop ) {
case 'name':
return wp_get_theme( $theme )?->get( 'Name' ) ?? '';
return wp_get_theme( $theme )->get( 'Name' ) ?: '';
Comment on lines -55 to +51
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the get method may return false but never null

default:
return wp_get_theme( $theme );
}
Expand Down