[shopsys] automated fixes and additions of annotations for extended classes

composer.json

@@ -122,6 +122,7 @@
         "prezent/doctrine-translatable-bundle": "^1.0.3",
         "psr/log": "^1.0",
         "ramsey/uuid": "^3.8",
+        "roave/better-reflection": "^3.5",
         "sensio/distribution-bundle": "^5.0.21",
         "sensio/framework-extra-bundle": "^3.0.29",
         "sensio/generator-bundle": "^3.1.7",

docs/introduction/console-commands-for-application-management-phing-targets.md

@@ -166,6 +166,17 @@ Exports all visible products to Elasticsearch.
 
 ### Coding standards
 
+#### annotations-check
+Checks whether annotations of extended classes in the project match the actual types according to [`ClassExtensionRegistry`](https://github.com/shopsys/shopsys/blob/master/packages/framework/src/Component/ClassExtension/ClassExtensionRegistry.php).
+Reported problems can be fixed using [`annotations-fix` phing target](#annotations-fix).
+
+#### annotations-fix
+Makes static analysis tools understand the extended code in your project by changing annotations and adding `@property` and `@method` annotations to relevant classes.
+
+You can read more in the ["Framework extensibility" article](./framework-extensibility.md#making-the-static-analysis-understand-the-extended-code).
+
+You can read more about the topic in the ["Framework extensibility" article](./framework-extensibility.md#making-the-static-analysis-understand-the-extended-code).
+
 #### standards / standards-diff
 Checks coding standards in source files. Checking all files may take a few minutes, `standards-diff` is much quicker as it checks only files changed against `origin/master`.
 

docs/introduction/faq-and-common-issues.md

@@ -27,6 +27,7 @@ For more detailed information about the Shopsys Framework, please see [Shopsys F
 - [Do you have any tips how to debug emails during development in Docker?](#do-you-have-any-tips-how-to-debug-emails-during-development-in-docker)
 - [Can I see what is really happening in the Codeception acceptance tests when using Docker?](#can-i-see-what-is-really-happening-in-the-codeception-acceptance-tests-when-using-docker)
 - [Why is there a faked PHP 7.2 platform in the Composer config?](#why-is-there-a-faked-php-72-platform-in-the-composer-config)
+- [How to make PHPStorm and PHPStan understand that I use extended classes?](#how-to-make-phpstorm-and-phpstan-understand-that-i-use-extended-classes)
 
 ## What are the phing targets?
 Every phing target is a task that can be executed simply by `php phing <target-name>` command.
@@ -162,3 +163,7 @@ See [Composer docs](https://getcomposer.org/doc/01-basic-usage.md#installing-wit
 Without this forced platform version, you could encounter issues when working on your project with developers that use a different version of PHP.
 For example, your `composer.lock` could contain dependencies that not all developers can install.
 If that's not your case, you can safely remove the `config.platform.php` option from your `composer.json` and run `composer update` to use higher versions of your dependencies.
+
+## How to make PHPStorm and PHPStan understand that I use extended classes?
+There is a phing target that automatically fixes all relevant `@var` and `@param` annotations, and adds proper `@method` and `@property` annotations to your classes so the static analysis understands the class extensions properly.
+You can read more in the ["Framework extensibility" article](./framework-extensibility.md#making-the-static-analysis-understand-the-extended-code).

docs/introduction/framework-extensibility.md

@@ -78,3 +78,216 @@ as well as a list of customizations that are not (and will not be) possible at a
     * separate users login credentials
     * share company attributes
     * change association from 1:1 to 1:N
+
+## Making the static analysis understand the extended code
+### Problem 1
+When extending framework classes, it may happen that tools for static analysis (e.g. PHPStan, PHPStorm) will not understand your code properly.
+Imagine this situation:
+
+- You have a controller that is dependent on a framework service:
+```php
+namespace Shopsys\ShopBundle\Controller\Front;
+
+use Shopsys\FrameworkBundle\Model\Product\ProductFacade;
+
+class ProductController
+{
+    /**
+     * @var \Shopsys\FrameworkBundle\Model\Product\ProductFacade
+     */
+    protected $productFacade;
+
+    /**
+     * @param \Shopsys\FrameworkBundle\Model\Product\ProductFacade $productFacade
+     */
+    public function __construct(ProductFacade $productFacade)
+    {
+        $this->productFacade = $productFacade;
+    }
+}
+```
+- In your project, you extend the framework's `ProductFacade` service:
+```php
+namespace Shopsys\ShopBundle\Model\Product;
+
+use Shopsys\FrameworkBundle\Model\Product\ProductFacade as BaseProductFacade;
+
+class ProductFacade extends BaseProductFacade
+{
+    public function myCustomAwesomeFunction()
+    {
+        return 42;
+    }
+}
+```
+- You register your extension in DI services configuration and thanks to that, your class is used in `ProductController` instead of the one from `FrameworkBundle`, so far so good:
+```yaml
+Shopsys\FrameworkBundle\Model\Product\ProductFacade: '@Shopsys\ShopBundle\Model\Product\ProductFacade'
+```
+**However, when you want to use your `myCustomAwesomeFunction()` in `ProductController`, the static analysis is not aware of that function.**
+#### Solution
+To fix this, you need to change the annotations properly:
+```diff
+class ProductController
+{
+      /**
+-      * @var \Shopsys\FrameworkBundle\Model\Product\ProductFacade
++      * @var \Shopsys\ShopBundle\Model\Product\ProductFacade
+       */
+      protected $productFacade;
+
+      /**
+-      * @param \Shopsys\FrameworkBundle\Model\Product\ProductFacade $productFacade
++      * @param \Shopsys\ShopBundle\Model\Product\ProductFacade $productFacade
+       */
+      public function __construct(ProductFacade $productFacade)
+      {
+          $this->productFacade = $productFacade;
+      }
+}
+```
+**Luckily, you do need to fix the annotations manually, there is the [Phing target `annotations-fix`](./console-commands-for-application-management-phing-targets.md#annotations-fix), that handles everything for you.**
+
+### Problem 2
+There might be yet another problem with static analysis when extending framework classes.
+Imagine the following situation:
+
+- In framework, there is `ProductFacade` that has `ProductRepository` property
+```php
+namespace Shopsys\FrameworkBundle\Model\Product;
+
+class ProductFacade
+{
+    /**
+     * @var \Shopsys\FrameworkBundle\Model\Product\ProductRepository
+     */
+    protected $productRepository;
+
+    /**
+     * @return \Shopsys\FrameworkBundle\Model\Product\ProductRepository
+     */
+    public function getProductRepository()
+    {
+        retrun $this->productRepository;
+    }
+}
+```
+- In your project, you extend `ProductRepository` and `ProductFacade` as well.
+- Then, in your extended facade, you want to access the repository (generally speaking, you want to access the parent's property that has a type that is extended in your project, or you want to access a method that returns a type that is already extended):
+```php
+namespace Shopsys\ShopBundle\Model\Product;
+
+use Shopsys\FrameworkBundle\Model\Product\ProductFacade as BaseProductFacade;
+
+class ProductFacade extends BaseProductFacade
+{
+    public function myCustomAwesomeFunction()
+    {
+        $this->productRepository; // static analysis thinks this is of type \Shopsys\FrameworkBundle\Model\Product\ProductRepository
+        $this->getProductRepository(); // static analysis thinks this is of type \Shopsys\FrameworkBundle\Model\Product\ProductRepository
+    }
+}
+```
+- **Once again, static analysis is not aware of the extension.**
+#### Solution
+To fix this, you don't need to override the method or property, you just need to add proper `@method` and `@property` annotations to your class:
+```diff
+namespace Shopsys\ShopBundle\Model\Product;
+
+use Shopsys\FrameworkBundle\Model\Product\ProductFacade as BaseProductFacade;
+
++ /**
++  * @method \Shopsys\ShopBundle\Model\Product\ProductRepository getProductRepository()
++  * @property \Shopsys\ShopBundle\Model\Product\ProductRepository $productRepository
++  */
+  class ProductFacade extends BaseProductFacade
+  {
+```
+**Even this scenario is covered by `annotations-fix` phing target.**
+
+### Problem 3
+There is one kind of problem that is not fixed automatically and needs to be addressed manually.
+Shopsys Framework uses a kind of magic for working with extended entities (see [`EntityNameResolver` class](https://github.com/shopsys/shopsys/blob/master/packages/framework/src/Component/EntityExtension/EntityNameResolver.php)),
+and static analysis tools are not aware of that fact.
+Imagine the following situation:
+- You have extended `Product` entity in your project
+- In the framework, there is `ProductFacade` class that is not extended in your project, and it has a method that returns instances of `Product` entity (in fact, it returns instances of your child `Product` entity thanks to the mentioned `EntityNameResolver` magic).
+```php
+namespace Shopsys\FrameworkBundle\Model\Product;
+
+// the class has no extension in your project
+class ProductFacade
+{
+
+    /**
+     * This class is not extended in the project either
+     * @var \Shopsys\FrameworkBundle\Model\Product\ProductRepository
+     */
+    protected $productRepository;
+
+    /**
+     * @return \Shopsys\FrameworkBundle\Model\Product\Product
+     */
+    public function getById($id)
+    {
+        // despite the annotation, extended Product entity from your project is returned
+        return $this->productRepository->getById($id);
+    }
+}
+```
+- You have a controller that is dependent on the framework service:
+```php
+namespace Shopsys\ShopBundle\Controller\Front;
+
+use Shopsys\FrameworkBundle\Model\Product\ProductFacade;
+
+class ProductController
+{
+    /**
+     * @var \Shopsys\FrameworkBundle\Model\Product\ProductFacade
+     */
+    protected $productFacade;
+
+    /**
+     * @return \Shopsys\FrameworkBundle\Model\Product\ProductFacade
+     */
+    public function __construct(ProductFacade $productFacade)
+    {
+        return $this->productFacade = $productFacade;
+    }
+
+    /**
+     * @param int $id
+     * Your Product instance is returned indeed, but static analysis is confused
+     * @return \Shopsys\ShopBundle\Model\Product\Product
+     */
+    private function myAwesomeMethod($id)
+    {
+        return $this->productFacade->getById($id);
+    }
+}
+```
+**In such a case, static analysis does not understand that the extended `Product` entity is returned.**
+#### Solution
+This needs to be fixed manually using a local variable with an inline annotation:
+```diff
+private function myAwesomeMethod($id)
+{
++    /** @var \Shopsys\ShopBundle\Model\Product\Product $product */
++    $product = $this->productFacade->getById($id);
+
+
+-    return $this->productFacade->getById($id);
++    return $product;
+}
+```
+
+As a workaround for this, you can create an empty class extending the one from the framework, register the extension in your `services.yml`, and then use `php phing annotations-fix` to fix appropriate annotations for you.
+
+Which way to go really depends on your situation. If you are likely to extend the given framework class sooner or later, or the same problem with the class is reported in many places, it would be better to create the empty extended class right away.
+Otherwise, it might be better just extracting and annotating the variable manually (like in [this commit in monorepo](https://github.com/shopsys/shopsys/commit/efd008b8d))
+as it is quicker and you can avoid having an unused empty class in your project.
+
+### Tip
+If you are a fan of an automation and PHPStorm user at the same time, you can simplify things even more and set your IDE to automatically run the phing target every time you e.g. change something in your project.
+This can be achieved by setting up a custom "[File watcher](https://www.jetbrains.com/help/phpstorm/using-file-watchers.html)".

docs/upgrade/UPGRADE-v8.1.0-dev.md

@@ -125,7 +125,24 @@ There you can find links to upgrade notes for other versions too.
         + docker-compose exec -T php-fpm composer install
         + docker-compose exec -T php-fpm ./phing db-create test-db-create build-demo-dev-quick error-pages-generate
         ```
+- run `php phing annotations-fix` to fix or add all the relevant annotations for your extended classes ([#1344](https://github.com/shopsys/shopsys/pull/1344))
+    - thanks to the fixes, your IDE (PHPStorm) will understand your code better
+    - you can read more about the whole topic in the ["Framework extensibility" article](../introduction/framework-extensibility.md#making-the-static-analysis-understand-the-extended-code)
+    - the checks and fixes of the proper annotations are now included in all `standards-*` phing targets, if you want to turn this feature off, add the following line into your `build.xml`:
+    ```diff
+      <property name="path.framework" value="${path.vendor}/shopsys/framework"/>
+    + <property name="check-and-fix-annotations" value="false"/>
 
+      <import file="${path.framework}/build.xml"/>
+    ```
+- increase your PHPStan level to 4 in your `build.xml` ([#1040](https://github.com/shopsys/shopsys/pull/1040))
+    ```diff
+  - <property name="phpstan.level" value="1"/>
+  + <property name="phpstan.level" value="4"/>
+    ```
+    - a lot of the possible issues should be already resolved if you followed the previous instruction and ran the `php phing annotations-fix` phing command
+    - some of the issues related to class extension need to be addressed manually nevertheless (see the ["Framework extensibility" article](../introduction/framework-extensibility.md#problem-3)) for more information
+    - you need to resolve all the other reported problems (any of them can be ignored in your `phpstan.neon`). You can find inspiration in [#1040](https://github.com/shopsys/shopsys/pull/1040)
 ### Database migrations
 - run database migrations so products will use a DateTime type for columns for "Selling start date" (selling_from) and "Selling end date" (selling_to) ([#1343](https://github.com/shopsys/shopsys/pull/1343))
     - please check [`Version20190823110846`](https://github.com/shopsys/shopsys/blob/master/packages/framework/src/Migrations/Version20190823110846.php)

packages/backend-api/install/build.xml.patch

@@ -5,7 +5,7 @@
 +    <property name="path.backend-api" value="${path.vendor}/shopsys/backend-api"/>
      <property name="path.framework" value="${path.vendor}/shopsys/framework"/>
 
-     <property name="phpstan.level" value="1"/>
+     <property name="phpstan.level" value="4"/>
 
      <import file="${path.framework}/build.xml"/>
 +    <import file="${path.backend-api}/build.xml"/>

packages/framework/build.xml

@@ -2,6 +2,7 @@
 <project name="shopsys_framework" default="list">
 
     <property name="is-multidomain" value="true" description="This property is @deprecated, see 'domains-info-load' target instead."/>
+    <property name="check-and-fix-annotations" value="true"/>
     <property name="path.app" value="${path.root}/app"/>
     <property name="path.bin" value="${path.vendor}/bin"/>
     <property name="path.bin-console" value="${path.root}/bin/console"/>
@@ -52,6 +53,43 @@
         </else>
     </if>
 
+    <target name="annotations-check" description="Checks whether annotations of extended classes in the project match the actual types according to ClassExtensionRegistry. Reported problems can be fixed using 'annotations-fix' phing target">
+        <if>
+            <istrue value="${check-and-fix-annotations}"/>
+            <then>
+                <exec executable="${path.php.executable}" passthru="true" checkreturn="true">
+                    <arg value="${path.bin-console}"/>
+                    <arg value="shopsys:extended-classes:annotations"/>
+                    <arg value="--dry-run"/>
+                </exec>
+            </then>
+            <else>
+                <echo>
+                    Annotations checks are turned off by configuration, see "check-and-fix-annotations" build property.
+                    You are still able to run "shopsys:extended-classes:annotations" Symfony command directly.
+                </echo>
+            </else>
+        </if>
+    </target>
+
+    <target name="annotations-fix" description="Fixes and adds annotations in project classes to improve static analysis and DX with extended classes">
+        <if>
+            <istrue value="${check-and-fix-annotations}"/>
+            <then>
+                <exec executable="${path.php.executable}" passthru="true" checkreturn="true">
+                    <arg value="${path.bin-console}"/>
+                    <arg value="shopsys:extended-classes:annotations"/>
+                </exec>
+            </then>
+            <else>
+                <echo>
+                    Annotations fixes are turned off by configuration, see "check-and-fix-annotations" build property.
+                    You are still able to run "shopsys:extended-classes:annotations" Symfony command directly.
+                </echo>
+            </else>
+        </if>
+    </target>
+
     <target name="assets" description="Installs web assets from external bundles into a public web directory.">
         <exec executable="${path.php.executable}" passthru="true" checkreturn="true" output="${dev.null}">
             <arg value="${path.bin-console}"/>
@@ -700,13 +738,13 @@
         </exec>
     </target>
 
-    <target name="standards" depends="phplint,ecs,phpstan,twig-lint,yaml-standards,eslint-check" description="Checks coding standards."/>
+    <target name="standards" depends="phplint,ecs,annotations-check,phpstan,twig-lint,yaml-standards,eslint-check" description="Checks coding standards."/>
 
-    <target name="standards-diff" depends="phplint-diff,ecs-diff,phpstan,twig-lint-diff,yaml-standards,eslint-check-diff" description="Checks coding standards in changed files."/>
+    <target name="standards-diff" depends="phplint-diff,ecs-diff,annotations-check,phpstan,twig-lint-diff,yaml-standards,eslint-check-diff" description="Checks coding standards in changed files."/>
 
-    <target name="standards-fix" depends="ecs-fix,yaml-standards-fix,eslint-fix" description="Automatically fixes *some* coding standards violations in all files. Always run 'standards' to be sure there are no further violations."/>
+    <target name="standards-fix" depends="ecs-fix,annotations-fix,yaml-standards-fix,eslint-fix" description="Automatically fixes *some* coding standards violations in all files. Always run 'standards' to be sure there are no further violations."/>
 
-    <target name="standards-fix-diff" depends="ecs-fix-diff,yaml-standards-fix,eslint-fix-diff" description="Automatically fixes *some* coding standards violations in changed files. Always run 'standards' to be sure there are no further violations."/>
+    <target name="standards-fix-diff" depends="ecs-fix-diff,annotations-fix,yaml-standards-fix,eslint-fix-diff" description="Automatically fixes *some* coding standards violations in changed files. Always run 'standards' to be sure there are no further violations."/>
 
     <target name="test-db-create" description="Creates test database for application with required configuration.">
         <exec executable="${path.php.executable}" passthru="true" checkreturn="true">

packages/framework/composer.json

@@ -74,6 +74,7 @@
         "prezent/doctrine-translatable-bundle": "^1.0.3",
         "psr/log": "^1.0",
         "ramsey/uuid": "^3.8",
+        "roave/better-reflection": "^3.5",
         "sensio/distribution-bundle": "^5.0.21",
         "sensio/framework-extra-bundle": "^3.0.29",
         "shopsys/doctrine-orm": "^2.6.2",

packages/framework/phpstan.neon

@@ -41,6 +41,9 @@ parameters:
         -
             message: '#Access to an undefined property PhpParser\\Node::\$name\.#'
             path: %currentWorkingDirectory%/src/Component/Translation/ConstraintViolationExtractor.php
+    excludes_analyses:
+        # Exclude "Source" folder dedicated for testing functionality connected to "shopsys:extended-classes:annotations" command
+        - %currentWorkingDirectory%/tests/Unit/Component/ClassExtension/Source/*
 includes:
     - vendor/phpstan/phpstan-doctrine/extension.neon
     - vendor/phpstan/phpstan-phpunit/extension.neon