[static-analysis] Fix static analysis level 1 errors. #679
Conversation
This file contains 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
swashata
reviewed
Dec 31, 2023
bahiirwa
force-pushed
the
feature/static-analysis-level-1-fixes
branch
from
1b17dbe
to
2419e91
Compare
fajardoleo
requested changes
Feb 12, 2024
templates/account.php
Outdated
| @@ -482,7 +483,7 @@ class="dashicons dashicons-image-rotate"></i> <?php fs_esc_html_echo_x_inline( ' | |||
| $profile[] = array( | |||
| 'id' => 'bundle_plan', | |||
| 'title' => $bundle_plan_text, | |||
| 'value' => $bundle_plan_title | |||
| 'value' => $bundle_title | |||
There was a problem hiding this comment.
@bahiirwa Please check the comment above.
templates/account.php
Outdated
| @@ -468,10 +468,11 @@ class="dashicons dashicons-image-rotate"></i> <?php fs_esc_html_echo_x_inline( ' | |||
| ); | |||
| } | |||
| } else { | |||
| $bundle_title = ( $has_bundle_license ? ucfirst( $fs->get_module_type() ) : '' ); | |||
There was a problem hiding this comment.
@bahiirwa Please check the comments below.
added 8 commits
April 16, 2024 18:34
…. 2 parameters given but 1 required.
…iables are misnamed in code that follows.
bahiirwa
force-pushed
the
feature/static-analysis-level-1-fixes
branch
from
83a307d
to
9d67cdd
Compare
bahiirwa
force-pushed
the
feature/static-analysis-level-1-fixes
branch
from
bfd1c8d
to
48aaba0
Compare
fajardoleo
approved these changes
Apr 16, 2024
swashata
requested changes
May 13, 2024
| @@ -8958,7 +8955,7 @@ private function update_plugin_version_event() { | |||
| * @author Vova Feldman (@svovaf) | |||
| * @since 2.0.0 | |||
| * | |||
| * @param array [string]array $plugins | |||
| * @param string[] $plugins | |||
There was a problem hiding this comment.
@bahiirwa The correct php-doc type would be array<string, array>. This is not an array of string :)
| @@ -14104,7 +14101,7 @@ private function activate_license( | |||
| $result = $fs->activate_license_on_many_installs( $user, $license_key, $blog_2_install_map ); | |||
| } | |||
|
|
|||
| if ( true === $result && ! empty( $site_ids ) ) { | |||
| if ( true === $result && count( $site_ids ) > 0 ) { | |||
There was a problem hiding this comment.
@bahiirwa why this change? What rule was it breaking?
| @@ -14323,6 +14320,7 @@ private function get_parent_and_addons_installs_info() { | |||
| ) | |||
| ); | |||
|
|
|||
| $installed_addons_ids_map = array_flip( $installed_addons_ids ); | |||
| @@ -20061,7 +20057,7 @@ private function _store_licenses( $store = true, $module_id = false, $licenses = | |||
| } | |||
| } | |||
|
|
|||
| if ( ! empty( $new_user_licenses_map ) ) { | |||
| if ( count( $new_user_licenses_map ) > 0 ) { | |||
There was a problem hiding this comment.
Is this something PHPStan is forcing us to do?
| @@ -1048,7 +1049,7 @@ function plugins_api_filter( $data, $action = '', $args = null ) { | |||
| false | |||
| ); | |||
|
|
|||
| if ( ! empty( $addon_plugin_data ) ) { | |||
| if ( is_array( $addon_plugin_data ) && isset( $addon_plugin_data['Version'] ) ) { | |||
There was a problem hiding this comment.
Weird, that return type of get_plugin_data is array, yet we have to check with is_array here again. Maybe if you feed PHPStan with some WP stub, it will not give these false positives?
| @@ -166,7 +166,8 @@ function get( $key, $default = null ) { | |||
| isset( $cache_entry->timestamp ) && | |||
| is_numeric( $cache_entry->timestamp ) | |||
| ) { | |||
| return $cache_entry->result; | |||
| return $cache_entry->result; // @phpstan-ignore-line | |||
|
|
|||
There was a problem hiding this comment.
Let's remove this empty line please.
| @@ -157,7 +157,7 @@ | |||
|
|
|||
| $site_view_params[] = $view_params; | |||
|
|
|||
| if ( empty( $install ) ) { | |||
| if ( is_object( $install ) ) { | |||
There was a problem hiding this comment.
Bug 😓 . if ( ! is_object( $install ) )
| $has_trial = $has_trial || ( is_numeric( $plan->trial_period ) && ( $plan->trial_period > 0 ) ); | ||
|
|
||
| if ( isset( $plan->trial_period ) ) { | ||
| $has_trial = ( is_numeric( $plan->trial_period ) && ( $plan->trial_period > 0 ) ); |
There was a problem hiding this comment.
@bahiirwa why not
$has_trial = isset( $plan->trial_period ) && is_numeric(...) && $plan->trial_period > 0;|
|
||
| $active_modules_by_id[ $data->id ] = true; | ||
| } | ||
| ?> | ||
| <tr<?php if ( $is_active ) { |
There was a problem hiding this comment.
Was this change needed for PHPStan?
| @@ -27,7 +27,9 @@ | |||
| $features_plan_map[ $feature->id ] = array( 'feature' => $feature, 'plans' => array() ); | |||
| } | |||
|
|
|||
| $features_plan_map[ $feature->id ]['plans'][ $plan->id ] = $feature; | |||
| if ( ! empty( $plan->id ) ) { | |||
There was a problem hiding this comment.
What if you add here
/**
* @var Plan[] $plans
*/does it still need this empty check?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Proposed changes
count()You can advise on that. The goal was to maintain the checks in place and not modify functionality.