Add phpstan strict rules (sonata-project#618)

* 3.10.0 (sonata-project#615) * 3.10.0 * Update CHANGELOG.md Co-authored-by: Javier Spagnoletti <[email protected]> Co-authored-by: Javier Spagnoletti <[email protected]> * Fix * Add phpstan strict rules * Ignore error * DevKit updates * Add support for SimplePager and fix psalm * Fix phpstan Co-authored-by: Javier Spagnoletti <[email protected]> Co-authored-by: SonataCI <[email protected]>
WebDaMa · Aug 9, 2021 · 7be0f1f · 7be0f1f
1 parent b7d95a0
commit 7be0f1f
Show file tree

Hide file tree

Showing 41 changed files with 306 additions and 248 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -11,13 +11,13 @@ charset = utf-8
 insert_final_newline = true
 trim_trailing_whitespace = true
 
-[*.{yaml,yml,twig,php}]
+[*.{xml,yaml,yml,twig,php}]
 indent_size = 4
 
 [.yamllint]
 indent_size = 4
 
-[*.{js,json,scss,css}]
+[*.{xliff,js,json,scss,css}]
 indent_size = 2
 
 [composer.json]

diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
@@ -72,5 +72,8 @@ jobs:
             - name: Install required dependencies
               run: sudo apt-get update && sudo apt-get install libxml2-utils
 
-            - name: Lint files
+            - name: Lint xml files
               run: make lint-xml
+
+            - name: Lint xliff files
+              run: make lint-xliff
diff --git a/Makefile b/Makefile
@@ -6,7 +6,7 @@ all:
 	@echo "Please choose a task."
 .PHONY: all
 
-lint: lint-composer lint-yaml lint-xml lint-php
+lint: lint-composer lint-yaml lint-xml lint-xliff lint-php
 .PHONY: lint
 
 lint-composer:
@@ -19,7 +19,7 @@ lint-yaml:
 .PHONY: lint-yaml
 
 lint-xml:
-	find . \( -name '*.xml' -or -name '*.xliff' \) \
+	find . -name '*.xml' \
 		-not -path './vendor/*' \
 		-not -path './src/Resources/public/vendor/*' \
 		| while read xmlFile; \
@@ -30,6 +30,18 @@ lint-xml:
 
 .PHONY: lint-xml
 
+lint-xliff:
+	find . -name '*.xliff' \
+		-not -path './vendor/*' \
+		-not -path './src/Resources/public/vendor/*' \
+		| while read xmlFile; \
+	do \
+		XMLLINT_INDENT='  ' xmllint --encode UTF-8 --format "$$xmlFile"|diff - "$$xmlFile"; \
+		if [ $$? -ne 0 ] ;then exit 1; fi; \
+	done
+
+.PHONY: lint-xliff
+
 lint-php:
 	php-cs-fixer fix --ansi --verbose --diff --dry-run
 .PHONY: lint-php

diff --git a/composer.json b/composer.json
@@ -55,6 +55,7 @@
         "phpstan/extension-installer": "^1.1",
         "phpstan/phpstan": "^0.12.52",
         "phpstan/phpstan-phpunit": "^0.12.17",
+        "phpstan/phpstan-strict-rules": "^0.12.10",
         "phpstan/phpstan-symfony": "^0.12.20",
         "psalm/plugin-phpunit": "^0.15.1",
         "psalm/plugin-symfony": "^2.0",

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -1,7 +1,9 @@
 parameters:
-	ignoreErrors:
-		- # Fixed in https://github.com/doctrine/mongodb-odm/pull/2307
-			message: "#^Parameter \\#1 \\$expression of method Doctrine\\\\ODM\\\\MongoDB\\\\Query\\\\Builder\\:\\:not\\(\\) expects array\\|Doctrine\\\\ODM\\\\MongoDB\\\\Query\\\\Expr, MongoDB\\\\BSON\\\\Regex given\\.$#"
-			count: 1
-			path: src/Filter/StringFilter.php
+    ignoreErrors:
+        - # Disallow VariableMethodCallRule and VariablePropertyFetchRule
+            message: '#^Variable (method call|property access)#'
+            path: .
+        - # https://github.com/phpstan/phpstan/issues/5357
+            message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert\\:\\:assertInstanceOf\\(\\) with 'Sonata\\\\\\\\AdminBundle\\\\\\\\Datagrid\\\\\\\\Pager' and \\*NEVER\\* will always evaluate to true\\.$#"
+            path: tests/Builder/DatagridBuilderTest.php
 
diff --git a/src/Builder/DatagridBuilder.php b/src/Builder/DatagridBuilder.php
@@ -17,6 +17,8 @@
 use Sonata\AdminBundle\Builder\DatagridBuilderInterface;
 use Sonata\AdminBundle\Datagrid\Datagrid;
 use Sonata\AdminBundle\Datagrid\DatagridInterface;
+use Sonata\AdminBundle\Datagrid\PagerInterface;
+use Sonata\AdminBundle\Datagrid\SimplePager;
 use Sonata\AdminBundle\FieldDescription\FieldDescriptionInterface;
 use Sonata\AdminBundle\FieldDescription\TypeGuesserInterface;
 use Sonata\AdminBundle\Filter\FilterFactoryInterface;
@@ -124,7 +126,7 @@ public function addFilter(DatagridInterface $datagrid, ?string $type, FieldDescr
 
     public function getBaseDatagrid(AdminInterface $admin, array $values = []): DatagridInterface
     {
-        $pager = new Pager();
+        $pager = $this->getPager($admin->getPagerType());
 
         $defaultOptions = [];
         if ($this->csrfTokenEnabled) {
@@ -140,4 +142,28 @@ public function getBaseDatagrid(AdminInterface $admin, array $values = []): Data
 
         return new Datagrid($query, $admin->getList(), $pager, $formBuilder, $values);
     }
+
+    /**
+     * Get pager by pagerType.
+     *
+     * @throws \RuntimeException If invalid pager type is set
+     *
+     * @return PagerInterface<ProxyQueryInterface>
+     */
+    private function getPager(string $pagerType): PagerInterface
+    {
+        switch ($pagerType) {
+            case Pager::TYPE_DEFAULT:
+                return new Pager();
+
+            case Pager::TYPE_SIMPLE:
+                /** @var SimplePager<ProxyQueryInterface> $simplePager */
+                $simplePager = new SimplePager();
+
+                return $simplePager;
+
+            default:
+                throw new \RuntimeException(sprintf('Unknown pager type "%s".', $pagerType));
+        }
+    }
 }
diff --git a/src/Builder/ListBuilder.php b/src/Builder/ListBuilder.php
@@ -94,7 +94,7 @@ public function fixFieldDescription(FieldDescriptionInterface $fieldDescription)
 
         $fieldDescription->setOption('label', $fieldDescription->getOption('label', $fieldDescription->getName()));
 
-        if (!$fieldDescription->getTemplate()) {
+        if (null === $fieldDescription->getTemplate()) {
             $fieldDescription->setTemplate($this->getTemplate($type));
         }
 

diff --git a/src/Builder/ShowBuilder.php b/src/Builder/ShowBuilder.php
@@ -78,7 +78,7 @@ public function fixFieldDescription(FieldDescriptionInterface $fieldDescription)
 
         $fieldDescription->setOption('label', $fieldDescription->getOption('label', $fieldDescription->getName()));
 
-        if (!$fieldDescription->getTemplate()) {
+        if (null === $fieldDescription->getTemplate()) {
             $fieldDescription->setTemplate($this->getTemplate($type));
         }
 

diff --git a/src/Datagrid/ProxyQuery.php b/src/Datagrid/ProxyQuery.php
@@ -79,7 +79,7 @@ public function execute()
 
         // todo : check how doctrine behave, potential SQL injection here ...
         $sortBy = $this->getSortBy();
-        if ($sortBy) {
+        if (null !== $sortBy) {
             $queryBuilder->sort($sortBy, $this->getSortOrder() ?? 'asc');
         }
 

diff --git a/src/FieldDescription/FieldDescription.php b/src/FieldDescription/FieldDescription.php
@@ -20,11 +20,7 @@ final class FieldDescription extends BaseFieldDescription
 {
     public function getTargetModel(): ?string
     {
-        if ($this->associationMapping) {
-            return $this->associationMapping['targetDocument'];
-        }
-
-        return null;
+        return $this->associationMapping['targetDocument'] ?? null;
     }
 
     public function isIdentifier(): bool

diff --git a/src/Filter/Filter.php b/src/Filter/Filter.php
@@ -26,7 +26,7 @@ final public function apply(BaseProxyQueryInterface $query, FilterData $filterDa
             throw new \TypeError(sprintf('The query MUST implement "%s".', ProxyQueryInterface::class));
         }
 
-        $field = $this->getParentAssociationMappings() ? $this->getName() : $this->getFieldName();
+        $field = [] !== $this->getParentAssociationMappings() ? $this->getName() : $this->getFieldName();
 
         $this->filter($query, $field, $filterData);
     }

diff --git a/src/Filter/ModelFilter.php b/src/Filter/ModelFilter.php
@@ -90,10 +90,6 @@ protected function handleMultiple(ProxyQueryInterface $query, string $field, Fil
 
     protected function handleScalar(ProxyQueryInterface $query, string $field, FilterData $data): void
     {
-        if (empty($data->getValue())) {
-            return;
-        }
-
         $id = self::fixIdentifier($data->getValue()->getId());
 
         if ($data->isType(EqualOperatorType::TYPE_NOT_EQUAL)) {

diff --git a/tests/Builder/DatagridBuilderTest.php b/tests/Builder/DatagridBuilderTest.php
@@ -20,6 +20,7 @@
 use Sonata\AdminBundle\Datagrid\Datagrid;
 use Sonata\AdminBundle\Datagrid\DatagridInterface;
 use Sonata\AdminBundle\Datagrid\Pager;
+use Sonata\AdminBundle\Datagrid\SimplePager;
 use Sonata\AdminBundle\FieldDescription\FieldDescriptionCollection;
 use Sonata\AdminBundle\FieldDescription\TypeGuesserInterface;
 use Sonata\AdminBundle\Filter\FilterFactoryInterface;
@@ -79,22 +80,43 @@ protected function setUp(): void
         $this->admin = $this->createMock(AdminInterface::class);
     }
 
-    public function testGetBaseDatagrid(): void
+    /**
+     * @phpstan-param class-string $pager
+     *
+     * @dataProvider getBaseDatagridData
+     */
+    public function testGetBaseDatagrid(string $pagerType, string $pager): void
     {
         $proxyQuery = $this->createStub(ProxyQueryInterface::class);
         $fieldDescription = new FieldDescriptionCollection();
         $formBuilder = $this->createStub(FormBuilderInterface::class);
 
+        $this->admin->method('getPagerType')->willReturn($pagerType);
         $this->admin->method('createQuery')->willReturn($proxyQuery);
         $this->admin->method('getList')->willReturn($fieldDescription);
 
         $this->formFactory->method('createNamedBuilder')->willReturn($formBuilder);
 
-        $this->assertInstanceOf(
-            Datagrid::class,
-            $datagrid = $this->datagridBuilder->getBaseDatagrid($this->admin)
-        );
-        $this->assertInstanceOf(Pager::class, $datagrid->getPager());
+        $datagrid = $this->datagridBuilder->getBaseDatagrid($this->admin);
+        self::assertInstanceOf(Datagrid::class, $datagrid);
+        self::assertInstanceOf(Pager::class, $datagrid->getPager());
+    }
+
+    /**
+     * @phpstan-return iterable<array-key, array{string, class-string}>
+     */
+    public function getBaseDatagridData(): iterable
+    {
+        return [
+            'simple' => [
+                Pager::TYPE_SIMPLE,
+                SimplePager::class,
+            ],
+            'default' => [
+                Pager::TYPE_DEFAULT,
+                Pager::class,
+            ],
+        ];
     }
 
     public function testFixFieldDescription(): void
@@ -111,7 +133,7 @@ public function testFixFieldDescription(): void
 
         $this->datagridBuilder->fixFieldDescription($fieldDescription);
 
-        $this->assertSame($classMetadata->fieldMappings['name'], $fieldDescription->getOption('field_mapping'));
+        self::assertSame($classMetadata->fieldMappings['name'], $fieldDescription->getOption('field_mapping'));
     }
 
     public function testFixFieldDescriptionWithAssociationMapping(): void
@@ -128,18 +150,18 @@ public function testFixFieldDescriptionWithAssociationMapping(): void
         $fieldDescription->setAdmin($this->admin);
 
         $this->admin
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('attachAdminClass');
 
         $this->datagridBuilder->fixFieldDescription($fieldDescription);
 
-        $this->assertSame($classMetadata->associationMappings['embeddedDocument'], $fieldDescription->getOption('association_mapping'));
+        self::assertSame($classMetadata->associationMappings['embeddedDocument'], $fieldDescription->getOption('association_mapping'));
     }
 
     public function testAddFilterNoType(): void
     {
         $this->admin
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('addFilterFieldDescription');
 
         $datagrid = $this->createMock(DatagridInterface::class);
@@ -162,12 +184,12 @@ public function testAddFilterNoType(): void
         $this->admin->method('getLabelTranslatorStrategy')->willReturn(new FormLabelTranslatorStrategy());
 
         $datagrid
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('addFilter')
-            ->with($this->isInstanceOf(ModelFilter::class));
+            ->with(self::isInstanceOf(ModelFilter::class));
 
         $this->filterFactory
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('create')
             ->with('test', ModelFilter::class);
 
@@ -177,14 +199,14 @@ public function testAddFilterNoType(): void
             $fieldDescription
         );
 
-        $this->assertSame('guess_value', $fieldDescription->getOption('guess_option'));
-        $this->assertSame(['guess_array_value'], $fieldDescription->getOption('guess_array_option'));
+        self::assertSame('guess_value', $fieldDescription->getOption('guess_option'));
+        self::assertSame(['guess_array_value'], $fieldDescription->getOption('guess_array_option'));
     }
 
     public function testAddFilterWithType(): void
     {
         $this->admin
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('addFilterFieldDescription');
 
         $datagrid = $this->createMock(DatagridInterface::class);
@@ -197,16 +219,16 @@ public function testAddFilterWithType(): void
         $this->admin->method('getLabelTranslatorStrategy')->willReturn(new FormLabelTranslatorStrategy());
 
         $datagrid
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('addFilter')
-            ->with($this->isInstanceOf(ModelFilter::class));
+            ->with(self::isInstanceOf(ModelFilter::class));
 
         $this->datagridBuilder->addFilter(
             $datagrid,
             ModelFilter::class,
             $fieldDescription
         );
 
-        $this->assertSame(ModelFilter::class, $fieldDescription->getType());
+        self::assertSame(ModelFilter::class, $fieldDescription->getType());
     }
 }
diff --git a/tests/Builder/ListBuilderTest.php b/tests/Builder/ListBuilderTest.php
@@ -69,13 +69,13 @@ public function testAddListActionField(): void
         $list = $this->listBuilder->getBaseList();
 
         $this->admin
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('addListFieldDescription');
 
         $this->listBuilder
             ->addField($list, 'actions', $fieldDescription);
 
-        $this->assertSame(
+        self::assertSame(
             '@SonataAdmin/CRUD/list__action.html.twig',
             $list->get('foo')->getTemplate(),
             'Custom list action field has a default list action template assigned'
@@ -96,12 +96,12 @@ public function testCorrectFixedActionsFieldType(): void
         $list = $this->listBuilder->getBaseList();
 
         $this->admin
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('addListFieldDescription');
 
         $this->listBuilder->addField($list, null, $fieldDescription);
 
-        $this->assertSame(
+        self::assertSame(
             ListMapper::TYPE_ACTIONS,
             $list->get(ListMapper::NAME_ACTIONS)->getType(),
             'Standard list _action field has "actions" type'
@@ -127,8 +127,8 @@ public function testFixFieldDescriptionWithFieldMapping(): void
 
         $this->listBuilder->fixFieldDescription($fieldDescription);
 
-        $this->assertSame('@SonataAdmin/CRUD/list_string.html.twig', $fieldDescription->getTemplate());
-        $this->assertSame($classMetadata->getFieldMapping('name'), $fieldDescription->getFieldMapping());
+        self::assertSame('@SonataAdmin/CRUD/list_string.html.twig', $fieldDescription->getTemplate());
+        self::assertSame($classMetadata->getFieldMapping('name'), $fieldDescription->getFieldMapping());
     }
 
     public function testFixFieldDescriptionException(): void