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

[Planned] Cleanup: Round 2 - clean code (phpstan update) #784

Closed
6 tasks
sreichel opened this issue Jun 23, 2019 · 11 comments
Closed
6 tasks

[Planned] Cleanup: Round 2 - clean code (phpstan update) #784

sreichel opened this issue Jun 23, 2019 · 11 comments
Assignees
Labels

Comments

@sreichel
Copy link
Contributor

sreichel commented Jun 23, 2019

Next planned steps ...

DOC updates

  • add @property annotation for undefined properties
  • complete/improve/fix doc updates from round 1
    • correct object/mixed annotations
    • return Type[] instead of array (should make some inline @var annotation obsolete)
    • add more getCollection(), getItems() hints for better 3rd patry-code support
  • add some DOCS to tempates *

Code changes

  • add typecasting and type checks to enforce correct parameter type where it it neccessary
  • replace is_null($var) with $var === null ... NOTE: seems useless, but from code analyte view it has a big impact ... same for return false instead of bool, where it is false.
  • years later ... better replace $var === null with is_null($var)

This should be far far less changes the DOC update ...

Goal

... get rid of the supressed erros from below and make it possible to run LTS against phpstan checks.


Excluded files:

  • controllers .... fails due to autoloading
  • all other (Adminhtml isnt complete) fails because of dynamic calls like Mage::getModel($someVar) (hast to be fixed in phpstan extension)

Ignored errors:

  • most comparison checks are "addressed" with this isue ... but require some small codeshanges ...

Current setup fir doc changes ...

includes:
    - vendor/inviqa/phpstan-magento1/extension.neon
parameters:
    reportUnmatchedIgnoredErrors: false
    autoload_files:
        - %currentWorkingDirectory%/app/Mage.php
    excludes_analyse:
        - %currentWorkingDirectory%/app/code/core/Mage/*/controllers/*
        - %currentWorkingDirectory%/app/code/core/Mage/Admin/Model/Acl/Assert/Ip.php
        - %currentWorkingDirectory%/app/code/core/Mage/Admin/Model/Acl/Assert/Time.php
        - %currentWorkingDirectory%/app/code/core/Mage/Admin/Model/Config.php
        - %currentWorkingDirectory%/app/code/core/Mage/Admin/Model/User.php
        - %currentWorkingDirectory%/app/code/core/Mage/Admin/Model/Roles.php
        - %currentWorkingDirectory%/app/code/core/Mage/Adminhtml/Block/Catalog/Category/Tabs.php
        - %currentWorkingDirectory%/app/code/core/Mage/Adminhtml/Block/Catalog/Category/Widget/Chooser.php
        - %currentWorkingDirectory%/app/code/core/Mage/Adminhtml/Block/Catalog/Product/Composite/Fieldset/Options.php
        - %currentWorkingDirectory%/app/code/core/Mage/Adminhtml/Block/Catalog/Product/Edit/Tab/Options/Option.php
        - %currentWorkingDirectory%/app/code/core/Mage/Adminhtml/Block/Catalog/Product/Edit/Tabs.php
        - %currentWorkingDirectory%/app/code/core/Mage/Adminhtml/Block/Catalog/Product/Helper/Form/Gallery/Content.php
        - %currentWorkingDirectory%/app/code/core/Mage/Adminhtml/Block/Customer/Edit/Tab/View/Grid/Renderer/Item.php
        - %currentWorkingDirectory%/app/code/core/Mage/Adminhtml/Block/Dashboard/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Adminhtml/Model/Config/Data.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api/Model/Acl/Assert/Ip.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api/Model/Acl/Assert/Time.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api/Model/Config.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api/Model/Roles.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api/Model/User.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api/Model/Server.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api/Model/Server/Handler/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api2/Model/Acl/Filter/Attribute/ResourcePermission.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api2/Model/Auth.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api2/Model/Auth/Adapter.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api2/Model/Auth/User.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api2/Model/Config.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api2/Model/Dispatcher.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api2/Model/Renderer.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api2/Model/Request/Interpreter.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api2/Model/Request/Model/Resource.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api2/Model/Resource.php
        - %currentWorkingDirectory%/app/code/core/Mage/Api2/Model/Resource/Validator/Eav.php
        - %currentWorkingDirectory%/app/code/core/Mage/Bundle/Block/Catalog/Product/View/Type/Bundle.php
        - %currentWorkingDirectory%/app/code/core/Mage/Bundle/Model/Product/Price.php
        - %currentWorkingDirectory%/app/code/core/Mage/Captcha/Block/Captcha.php
        - %currentWorkingDirectory%/app/code/core/Mage/Captcha/Helper/Data.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Block/Layer/Filter/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Block/Layer/View.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Block/Product/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Block/Product/List.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Block/Product/View/Media.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Block/Product/View/Options.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Block/Product/View/Tabs.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Helper/Data.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Helper/Product/Flat.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Model/Factory.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Model/Product.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Model/Api2/Product/Validator/Product.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Model/Product/Option.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Model/Product/Type.php
        - %currentWorkingDirectory%/app/code/core/Mage/Catalog/Model/Resource/Product/Indexer/Price.php
        - %currentWorkingDirectory%/app/code/core/Mage/CatalogIndex/Model/Indexer.php
        - %currentWorkingDirectory%/app/code/core/Mage/CatalogIndex/Model/Retreiver.php
        - %currentWorkingDirectory%/app/code/core/Mage/CatalogInventory/Model/Resource/Indexer/Stock.php
        - %currentWorkingDirectory%/app/code/core/Mage/CatalogRule/Model/Resource/Rule.php
        - %currentWorkingDirectory%/app/code/core/Mage/CatalogSearch/Helper/Data.php
        - %currentWorkingDirectory%/app/code/core/Mage/Checkout/Block/Cart/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Checkout/Block/Cart/Totals.php
        - %currentWorkingDirectory%/app/code/core/Mage/Checkout/Helper/Cart.php
        - %currentWorkingDirectory%/app/code/core/Mage/Cms/Helper/Data.php
        - %currentWorkingDirectory%/app/code/core/Mage/Cms/Helper/Page.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/functions.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Block/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Controller/Varien/Action.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Controller/Varien/Front.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Helper/Js.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Helper/Url.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Model/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Model/App.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Model/Config.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Model/Email/Template.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Model/Email/Template/Filter.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Model/Factory.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Model/Input/Filter.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Model/Layout.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Model/Resource/Db/Collection/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Model/Store.php
        - %currentWorkingDirectory%/app/code/core/Mage/Core/Model/Url.php
        - %currentWorkingDirectory%/app/code/core/Mage/Cron/Model/Observer.php
        - %currentWorkingDirectory%/app/code/core/Mage/Customer/Model/Customer.php
        - %currentWorkingDirectory%/app/code/core/Mage/Directory/Model/Observer.php
        - %currentWorkingDirectory%/app/code/core/Mage/Eav/Model/Attribute/Data.php
        - %currentWorkingDirectory%/app/code/core/Mage/Eav/Model/Config.php
        - %currentWorkingDirectory%/app/code/core/Mage/Eav/Model/Convert/Adapter/Entity.php
        - %currentWorkingDirectory%/app/code/core/Mage/Eav/Model/Convert/Adapter/Grid.php
        - %currentWorkingDirectory%/app/code/core/Mage/Eav/Model/Entity/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Eav/Model/Entity/Attribute/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Eav/Model/Entity/Collection/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Eav/Model/Entity/Type.php
        - %currentWorkingDirectory%/app/code/core/Mage/Eav/Model/Form.php
        - %currentWorkingDirectory%/app/code/core/Mage/GiftMessage/Model/Message.php
        - %currentWorkingDirectory%/app/code/core/Mage/ImportExport/Model/Export.php
        - %currentWorkingDirectory%/app/code/core/Mage/ImportExport/Model/Export/Entity/Product.php
        - %currentWorkingDirectory%/app/code/core/Mage/ImportExport/Model/Import.php
        - %currentWorkingDirectory%/app/code/core/Mage/ImportExport/Model/Import/Entity/Product.php
        - %currentWorkingDirectory%/app/code/core/Mage/Index/Model/Lock.php
        - %currentWorkingDirectory%/app/code/core/Mage/Index/Model/Process.php
        - %currentWorkingDirectory%/app/code/core/Mage/Newsletter/Helper/Data.php
        - %currentWorkingDirectory%/app/code/core/Mage/PageCache/Helper/Data.php
        - %currentWorkingDirectory%/app/code/core/Mage/Payment/Helper/Data.php
        - %currentWorkingDirectory%/app/code/core/Mage/Payment/Model/Config.php
        - %currentWorkingDirectory%/app/code/core/Mage/Persistent/Model/Persistent/Config.php
        - %currentWorkingDirectory%/app/code/core/Mage/Reports/Block/Product/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Reports/Model/Report.php
        - %currentWorkingDirectory%/app/code/core/Mage/Rss/Block/Catalog/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Rss/Controller/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Rule/Model/Action/Collection.php
        - %currentWorkingDirectory%/app/code/core/Mage/Rule/Model/Condition/Combine.php
        - %currentWorkingDirectory%/app/code/core/Mage/SalesRule/Model/Validator.php
        - %currentWorkingDirectory%/app/code/core/Mage/Weee/Helper/Data.php
        - %currentWorkingDirectory%/app/code/core/Mage/Weee/Model/Attribute/Backend/Weee/Tax.php
        - %currentWorkingDirectory%/app/code/core/Mage/Weee/Model/Config/Source/Fpt/Tax.php
        - %currentWorkingDirectory%/app/code/core/Mage/Widget/Block/Adminhtml/Widget/Instance/Edit/Chooser/Block.php
        - %currentWorkingDirectory%/app/code/core/Mage/Widget/Block/Adminhtml/Widget/Instance/Edit/Chooser/Layout.php
        - %currentWorkingDirectory%/app/code/core/Mage/Widget/Block/Adminhtml/Widget/Options.php
        - %currentWorkingDirectory%/app/code/core/Mage/Widget/Model/Template/Filter.php
        - %currentWorkingDirectory%/app/code/core/Mage/Widget/Model/Widget.php
        - %currentWorkingDirectory%/app/code/core/Mage/Widget/Model/Widget/Instance.php
        - %currentWorkingDirectory%/app/code/core/Mage/Wishlist/Block/Abstract.php
        - %currentWorkingDirectory%/app/code/core/Mage/Wishlist/Block/Customer/Wishlist.php
        - %currentWorkingDirectory%/app/code/core/Mage/Wishlist/Block/Customer/Wishlist/Item/Options.php
        - %currentWorkingDirectory%/app/code/core/Mage/Wishlist/Helper/Data.php
    ignoreErrors:
        - '#Class [a-zA-Z_]+Controller not found.#'
        - '#Call to method [a-zA-Z_]+\(\) on an unknown class [a-zA-Z_]+Controller.#'
        - '#Return typehint of method [a-zA-Z_]+::+[a-zA-Z_]+\(\) has invalid type [a-zA-Z_]+Controller.#'
        - '#[a-zA-Z_]+::__construct\(\) does not call parent constructor from Varien_Object.#'
        - '#Call to an undefined method Varien_Object::+[a-zA-Z]#'
        - '#Call to an undefined method Varien_Data_Collection::+[a-zA-Z]#'
        - '#Call to an undefined method Mage_Core_Model_Resource_Helper_Abstract::+[a-zA-Z]#'
        - '#Call to function is_null\(\) with +[a-zA-Z_]+ will always evaluate to false.#'
        - '#Parameter \#2 $arguments of static method Mage::getModel\(\) expects array|object, string given.#'
        - '#Else branch is unreachable because previous condition is always true.#'
        - '#Else branch is unreachable because ternary operator condition is always true.#'
        - '#Elseif branch is unreachable because previous condition is always true.#'
        - '#Negated boolean expression is always false.#'
        - '#Negated boolean expression is always true.#'
        - '#Strict comparison using === between string and false will always evaluate to false.#'
        - '#Left side of && is always true.#'
        - '#Right side of && is always true.#'
        - '#If condition is always true.#'
        - '#Ternary operator condition is always true.#'
        - '#Varien_Db_Adapter_Interface|Zend_Db_Adapter_Abstract::+[a-zA-Z_]+\(\).#'
        - "#Variable \\$+[a-zA-Z_]+ might not be defined.#"
        -
            message: '#Undefined variable: \$conn#'
            paths:
                - %currentWorkingDirectory%/app/code/*/*/*/sql/*
        -
            message: '#Undefined variable: \$this#'
            paths:
                - %currentWorkingDirectory%/app/code/*/*/*/data/*
                - %currentWorkingDirectory%/app/code/*/*/*/sql/*
        -
            message: '#Using \$this outside a class.#'
            paths:
                - %currentWorkingDirectory%/app/code/*/*/*/data/*
                - %currentWorkingDirectory%/app/code/*/*/*/sql/*

@Flyingmana before i start ... do you agreee with PRs to fix this?

@sreichel
Copy link
Contributor Author

sreichel commented May 9, 2020

Ref #708 check resetValue()

    /**
     * Resets custom Currency symbol for all store views, websites and default value
     */
    public function resetAction()
    {
        Mage::getModel('currencysymbol/system_currencysymbol')->resetValues();
        $this->_redirectReferer();
    }

@OpenMage OpenMage deleted a comment from tmotyl May 9, 2020
@sreichel
Copy link
Contributor Author

Ref #765 and ... all return Mage::getModel($model);

* @return false|Mage_Core_Model_Abstract|expected

@sreichel
Copy link
Contributor Author

Ref #765 check * @param null (default value)

@sreichel
Copy link
Contributor Author

Ref #749

I approve, but please create an issue for inconsisten and/or vs || &&

@sreichel
Copy link
Contributor Author

Ref #754

Check all formatDate() methods.

@sreichel
Copy link
Contributor Author

sreichel commented May 11, 2020

Ref #776
Ref #946

Check ... int vs bool

  • getIsFilterableInSearch()
  • getData('is_filterable_in_search')
  • setIs....
  • ...

@sreichel
Copy link
Contributor Author

Ref #745

Check where $store is passed as param ... could be int, numeric string, Mage_Core_Model_Store or null.

@sreichel
Copy link
Contributor Author

Ref #771

Check is_numeric() ...

@tmotyl
Copy link
Contributor

tmotyl commented Sep 27, 2021

here is a PR with working phpstan setup #1837
For the Varien_Object, I see that phpstan has some support for this kind of object, but I haven't tested them myself.
https://phpstan.org/config-reference#universal-object-crates

@sreichel
Copy link
Contributor Author

sreichel commented Dec 22, 2022

Current status - phpstan error count

ignoring nothing (just excluded paths)

Level Errors
0 36
1 450
2 950
3 1537
4 2849
5 3909

current config

Level Errors
0 40
1 117
2 617
3 1204
4 1722
5 2782*

* with open PRs its below 2500

L1: VAR might not be defined
L2: undefined method (500x)
L5: 1060x (step back to L4?)

I'll update depending on progress ;)

@sreichel
Copy link
Contributor Author

After yesterdays updates ...

current config

Level Errors
0 45
1 122
2 615
3 1155
4 1341
5 2400

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants