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.

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

[static-analysis] Fix static analysis level 2 errors. #679

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

Conversation

bahiirwa
Copy link
Contributor

@bahiirwa bahiirwa commented Dec 28, 2023

Proposed changes

  • Fix variables defined out of block yet are shared.
  • Fix misnamed/missing declaration of variables in code.
  • Added ignores for false positives like in the templates where we pass $VARS from the part including the partial file.
  • Improved the logic for where we have already set the data types like arrays and then still checking it they exist. This returns truthy/falsy errors. I have replaced those with count() You can advise on that. The goal was to maintain the checks in place and not modify functionality.

@bahiirwa bahiirwa force-pushed the feature/static-analysis-level-1-fixes branch from 1b17dbe to 2419e91 Compare January 15, 2024 18:34
@bahiirwa bahiirwa changed the base branch from master to develop January 15, 2024 18:36
includes/class-freemius.php Show resolved Hide resolved
includes/class-freemius.php Outdated Show resolved Hide resolved
includes/class-freemius.php Show resolved Hide resolved
includes/class-freemius.php Show resolved Hide resolved
includes/class-fs-plugin-updater.php Outdated Show resolved Hide resolved
includes/fs-plugin-info-dialog.php Outdated Show resolved Hide resolved
templates/account.php Outdated Show resolved Hide resolved
templates/account.php Outdated Show resolved Hide resolved
templates/account.php Outdated Show resolved Hide resolved
templates/account.php Outdated Show resolved Hide resolved
@bahiirwa bahiirwa force-pushed the feature/static-analysis-level-1-fixes branch 2 times, most recently from bfd1c8d to 48aaba0 Compare April 16, 2024 16:13
@Freemius Freemius deleted a comment from fajardoleo Apr 16, 2024
@bahiirwa bahiirwa marked this pull request as ready for review April 16, 2024 16:35
Copy link
Contributor

@swashata swashata left a comment

Choose a reason for hiding this comment

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

@bahiirwa please see my comments, thanks.

includes/class-freemius.php Outdated Show resolved Hide resolved
includes/class-freemius.php Outdated Show resolved Hide resolved
@@ -14323,6 +14320,7 @@ private function get_parent_and_addons_installs_info() {
)
);

$installed_addons_ids_map = array_flip( $installed_addons_ids );
Copy link
Contributor

Choose a reason for hiding this comment

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

So this is a bug fix?

Copy link
Contributor Author

@bahiirwa bahiirwa Jun 16, 2024

Choose a reason for hiding this comment

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

@swashata Yes, it is. Caught it while doing the checks. Should we push this in another PR?

@@ -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 ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this something PHPStan is forcing us to do?

includes/class-fs-plugin-updater.php Outdated Show resolved Hide resolved
includes/managers/class-fs-cache-manager.php Outdated Show resolved Hide resolved
templates/account.php Outdated Show resolved Hide resolved
templates/add-ons.php Outdated Show resolved Hide resolved
templates/debug.php Show resolved Hide resolved
templates/plugin-info/features.php Outdated Show resolved Hide resolved
@bahiirwa bahiirwa force-pushed the feature/static-analysis-level-1-fixes branch 2 times, most recently from 6ee7406 to fe76177 Compare June 16, 2024 14:21
@bahiirwa bahiirwa changed the title [static-analysis] Fix static analysis level 1 errors. [static-analysis] Fix static analysis level 2 errors. Jun 17, 2024
@bahiirwa bahiirwa force-pushed the feature/static-analysis-level-1-fixes branch 3 times, most recently from e47c49a to ae44f1c Compare June 17, 2024 09:18
@bahiirwa bahiirwa self-assigned this Sep 4, 2024
@bahiirwa bahiirwa force-pushed the feature/static-analysis-level-1-fixes branch 2 times, most recently from 5999f7f to 2bef074 Compare September 10, 2024 13:31
@bahiirwa bahiirwa force-pushed the feature/static-analysis-level-1-fixes branch from 2bef074 to aff954f Compare September 22, 2024 11:42
@bahiirwa bahiirwa force-pushed the feature/static-analysis-level-1-fixes branch from aff954f to 522f0e1 Compare November 7, 2024 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants