Code Quality Fix: Invalid @var usage

[lbajsarowicz]

Description (*)

This Pull Request answers the need of consistent @var notation in all the Magento files.

The only valid format is: @var ["Type"] [element_name] [<description>] (https://docs.phpdoc.org/guide/references/phpdoc/tags/var.html) but Magento did not follow this format, which now needs to be fixed.

Related Pull Requests

Fixed Issues (if relevant)

1. Fixes magento/magento2#<issue_number>

Manual testing scenarios (*)

1. No change to the actual codebase, no testing is necessary.
2. ...

Questions or comments

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @lbajsarowicz. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

---

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

[ct-prd-pr-scan]

The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com.

[ct-prd-pr-scan]

The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com.

[lbajsarowicz]

@magento run all tests

(I submitted clear issue when the class was not defined, new version skips such)

[ct-prd-pr-scan]

The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com.

[lbajsarowicz]

@magento run all tests

I found a few more issues with the codebase, fixed.

[ihor-sviziev]

We discussed this with @lbajsarowicz in Slack. So far, merging 300+ files in one step is almost impossible, as it would require a huge amount of static test fixes and, certainly, cause conflicts with releases that would need to be fixed multiple times.
I don't think this realistically can be merged any time soon.
Instead, I suggest adding a static test that would fail if the @var is misused. Then, those failures could be fixed with smaller pieces

[hostep]

I suggest adding a static test that would fail if the @var is misused

Just FYI, running phpstan on level 2 can detect this, see the last 2 problems it finds:

$ ./vendor/bin/phpstan analyse --level=2 app/code/Magento/AdminNotification/Model/System/Message/Baseurl.php
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   Baseurl.php
 ------ -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  73     Call to an undefined method Magento\Framework\App\Config\ValueInterface::getCollection().
  73     PHPDoc tag @var has invalid value ($dataCollection \Magento\Config\Model\ResourceModel\Config\Data\Collection): Unexpected token "$dataCollection", expected type at offset 9
  77     PHPDoc tag @var has invalid value ($data \Magento\Framework\App\Config\ValueInterface): Unexpected token "$data", expected type at offset 9
 ------ -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

At the moment we currently only run phpstan on level 1 in static tests in Magento.

---

I don't fully agree with only fixing this when we touch a file, because custom code highly benefits from having correct type annotations. If all type annotations in core Magento would be correctly specified, checking custom code with static analysers would result in a significant better experience that what we have right now.
This will mostly be beneficial in phtml files, which we have to copy in a custom theme to change to our likings. So if the starting file would already have correct type annotations it will significantly help in analysing those phtml files in a custom theme.

So maybe we can try to introduce these fixes in a PR per module? As that will be easier to review in smaller chunks?