Fix problems reported by static analysis #19
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
szepeviktor
commented
Nov 29, 2023
| * @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 ) ) { |
Contributor
Author
There was a problem hiding this comment.
Here we need a WP instance not a Controller
szepeviktor
commented
Nov 29, 2023
| * Runs when a request is migratable. | ||
| * | ||
| * @param Controller $this Controller instance. | ||
| * @param Controller $instance Controller instance. |
Contributor
Author
There was a problem hiding this comment.
parameter naming
szepeviktor
commented
Nov 29, 2023
| 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 ); |
Contributor
Author
There was a problem hiding this comment.
only add_action has that last one
szepeviktor
commented
Nov 29, 2023
| */ | ||
| 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 ) ) { |
Contributor
Author
There was a problem hiding this comment.
$this->wp is always set in the constructor
szepeviktor
commented
Nov 29, 2023
| } | ||
|
|
||
| return true; | ||
| return wp_get_theme( $theme )->exists(); |
Contributor
Author
There was a problem hiding this comment.
wp_get_theme always returns a WP_Theme instance
szepeviktor
commented
Nov 29, 2023
Comment on lines
-55
to
+51
| return wp_get_theme( $theme )?->get( 'Name' ) ?? ''; | ||
| return wp_get_theme( $theme )->get( 'Name' ) ?: ''; |
Contributor
Author
There was a problem hiding this comment.
the get method may return false but never null
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
@emilyatmobtown Please see individual comments.
Please consider introducing PHPStan in Alley.https://github.com/alleyinteractive/.github/blob/main/.github/workflows/php-code-quality.yml#L7