style #165 Correct Style (and apply fixes from PhpStan) (sstok)

This PR was merged into the 2.0-dev branch.

Discussion
----------

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | 
| License       | MIT
| Doc PR        | 

- Set-up PHP-CS-Fixer using Travis
- Set-up PHPStan using Travis

And fix reported errors and style violations.

Commits
-------

ca08b5c Correct Style (and apply fixes from PhpStan)

.php_cs

@@ -39,6 +39,8 @@ return PhpCsFixer\Config::create()
         'single_import_per_statement' => false,
         'strict_comparison' => false,
         'strict_param' => true,
+        // Breaks with phpstan
+        'phpdoc_inline_tag' => false,
     ])
     ->setRiskyAllowed(true)
     ->setFinder(

.travis.yml

@@ -12,6 +12,8 @@ matrix:
           #env: coverage=1
         - php: '7.1'
           env: deps='low'
+        - php: '7.1'
+          env: lint=1
 
 cache:
     directories:
@@ -22,7 +24,9 @@ before_install:
     - if [[ $coverage = 1 ]]; then mkdir -p build/logs build/cov; fi
     - if [[ $coverage = 1 ]]; then wget https://phar.phpunit.de/phpcov.phar; fi
     - if [[ $coverage = 1 ]]; then wget https://github.com/satooshi/php-coveralls/releases/download/v1.0.1/coveralls.phar; fi
-    - export PATH="$PATH:$HOME/.composer/vendor/bin"
+    - if [[ $lint = 1 ]]; then wget https://github.com/FriendsOfPHP/PHP-CS-Fixer/releases/download/v2.3.2/php-cs-fixer.phar; fi
+    - if [[ $lint = 1 ]]; then composer global require phpstan/phpstan; fi
+    - export PATH="$PATH:$HOME/.composer/vendor/bin:./vendor/bin"
 
 install:
     - if [[ $coverage = 1 ]]; then composer require --dev --no-update 'phpunit/php-code-coverage:^4.0.1'; fi
@@ -31,8 +35,12 @@ install:
     - if [[ $deps = 'low' ]]; then composer update --prefer-dist --no-progress --no-suggest --prefer-stable --prefer-lowest --ansi; fi
 
 script:
-    - if [[ $coverage = 1 ]]; then phpdbg -qrr -dmemory_limit=-1 vendor/bin/phpunit  --verbose --coverage-php build/cov/coverage-phpunit.cov; else vendor/bin/phpunit --verbose; fi
+    - export SYMFONY_DEPRECATIONS_HELPER=strict
+    - if [[ $coverage = 1 ]]; then phpdbg -qrr -dmemory_limit=-1 vendor/bin/phpunit  --verbose --coverage-php build/cov/coverage-phpunit.cov; fi
     - if [[ $coverage = 1 ]]; then phpdbg -qrr phpcov.phar merge --clover build/logs/clover.xml build/cov; fi
+    - if [[ ! $lint ]]; then vendor/bin/phpunit --verbose; fi
+    - if [[ $lint = 1 ]]; then php php-cs-fixer.phar fix --dry-run --diff --no-ansi; fi
+    - if [[ $lint = 1 ]]; then phpstan analyse -c phpstan.neon -l5 --ansi src tests; fi
 
 after_success:
     - if [[ $coverage = 1 ]]; then travis_retry php coveralls.phar; fi

phpstan.neon

@@ -0,0 +1,32 @@
+parameters:
+    autoload_files:
+        - vendor/autoload.php
+    ignoreErrors:
+        #- '#__construct\(\) does not call parent constructor from .+#'
+
+        # ValueHolder guard there own correctness. A ValuesBag never returns a wrong object (collection).
+        - '#expects Rollerworks\\Component\\Search\\Value\\[a-zA-Z]+, Rollerworks\\Component\\Search\\Value\\ValueHolder given#'
+        - '#expects Rollerworks\\Component\\Search\\Value\\[a-zA-Z]+\[\], Rollerworks\\Component\\Search\\Value\\ValueHolder\[\] given#'
+        - '#Call to an undefined method Rollerworks\\Component\\Search\\Value\\ValueHolder\:\:#'
+
+        # False positive
+        - '#Call to an undefined method Rollerworks\\Component\\Search\\Field\\FieldConfig\:\:finalizeConfig\(\)#'
+        - '#Call to an undefined method DateTimeInterface\:\:setTimezone\(\)#'
+        - '#Call to an undefined method Money\\Exception\:\:getMessage\(\)#'
+        - '#Call to an undefined method Exception\:\:getErrors\(\)#'
+        - '#expects Rollerworks\\Component\\Search\\Field\\FieldTypeExtension\[\], mixed\[\]\[\] given#'
+        - '#\(mixed\[\]\[\]\) does not accept iterable\(iterable\(mixed\[\]\)\[\]\)#'
+        - '#Undefined variable: \$(c|flatChoices)#'
+
+        # Tests
+        - '#Call to an undefined method Prophecy\\Prophecy\\ObjectProphecy::[a-zA-Z0-9_]+\(\)#'
+        #- '#Access to an undefined property Prophecy\\Prophecy\\ObjectProphecy::\$[a-zA-Z0-9_]+#'
+        - '#Call to an undefined method PHPUnit_Framework_MockObject_MockObject::[a-zA-Z0-9_]+\(\)#'
+        - '#expects\s+[^\s]+, PHPUnit_Framework_MockObject_MockObject(\[\])? given#'
+        - '#does not accept PHPUnit_Framework_MockObject_MockObject#'
+        - '#but returns PHPUnit_Framework_MockObject_MockObject#'
+        - '#Call to an undefined static method Money\\Money\:\:#'
+
+        ## Transformations are tested to ensure there input type is valided.
+        - '#\:\:transform\(\) expects [^\s]+, [^\s]+ given#'
+        - '#\:\:reverseTransform\(\) expects [^\s]+, [^\s]+ given#'

src/ConditionErrorMessage.php

@@ -37,7 +37,7 @@ final class ConditionErrorMessage
     /**
      * The template for the error message.
      *
-     * @var string
+     * @var string|null
      */
     public $messageTemplate;
 
@@ -80,7 +80,7 @@ final class ConditionErrorMessage
      * @param int|null    $messagePluralization The value for error message pluralization
      * @param mixed       $cause                The cause of the error
      */
-    public function __construct(string $path, string $message, string $messageTemplate = null, array $messageParameters = [], int $messagePluralization = null, $cause = null)
+    public function __construct(string $path, string $message, ?string $messageTemplate = null, array $messageParameters = [], ?int $messagePluralization = null, $cause = null)
     {
         $this->path = $path;
         $this->message = $message;
@@ -94,7 +94,7 @@ public static function withMessageTemplate(string $path, string $messageTemplate
     {
         return new static(
             $path,
-            strtr((string) $messageTemplate, $messageParameters),
+            strtr($messageTemplate, $messageParameters),
             $messageTemplate,
             $messageParameters,
             $messagePluralization,
@@ -124,6 +124,6 @@ public function setTranslatedParameters(array $translatedParameters)
      */
     public function __toString()
     {
-        return (string) $this->message;
+        return $this->message;
     }
 }

src/Exporter/XmlExporter.php

@@ -31,7 +31,7 @@
 final class XmlExporter extends AbstractExporter
 {
     /**
-     * @var \DOMDocument
+     * @var \DOMDocument|null
      */
     private $document;
 

src/Extension/Core/ChoiceList/ChoiceLoaderTrait.php

@@ -22,7 +22,7 @@
 trait ChoiceLoaderTrait
 {
     /**
-     * @var ArrayChoiceList
+     * @var ArrayChoiceList|null
      */
     protected $choiceList;
 

src/Extension/Core/ChoiceList/Factory/PropertyAccessDecorator.php

@@ -43,23 +43,16 @@
  */
 final class PropertyAccessDecorator implements ChoiceListFactory
 {
-    /**
-     * @var ChoiceListFactory
-     */
     private $decoratedFactory;
-
-    /**
-     * @var PropertyAccessor
-     */
     private $propertyAccessor;
 
     /**
      * Decorates the given factory.
      *
      * @param ChoiceListFactory     $decoratedFactory The decorated factory
-     * @param null|PropertyAccessor $propertyAccessor The used property accessor
+     * @param PropertyAccessor|null $propertyAccessor The used property accessor
      */
-    public function __construct(ChoiceListFactory $decoratedFactory, PropertyAccessor $propertyAccessor = null)
+    public function __construct(ChoiceListFactory $decoratedFactory, ?PropertyAccessor $propertyAccessor = null)
     {
         $this->decoratedFactory = $decoratedFactory;
         $this->propertyAccessor = $propertyAccessor ?: PropertyAccess::createPropertyAccessor();
@@ -68,21 +61,21 @@ public function __construct(ChoiceListFactory $decoratedFactory, PropertyAccesso
     /**
      * Returns the decorated factory.
      *
-     * @return ChoiceListFactory The decorated factory
+     * @return ChoiceListFactory
      */
     public function getDecoratedFactory(): ChoiceListFactory
     {
         return $this->decoratedFactory;
     }
 
     /**
-     * {@inheritdoc}
+     * @inheritdoc
      *
      * @param array|\Traversable                $choices The choices
-     * @param null|callable|string|PropertyPath $value   The callable or path for
+     * @param callable|string|PropertyPath|null $value   The callable or path for
      *                                                   generating the choice values
      *
-     * @return ChoiceList The choice list
+     * @return ChoiceList
      */
     public function createListFromChoices($choices, $value = null): ChoiceList
     {
@@ -107,13 +100,13 @@ public function createListFromChoices($choices, $value = null): ChoiceList
     }
 
     /**
-     * {@inheritdoc}
+     * @inheritdoc
      *
      * @param ChoiceLoader                      $loader The choice loader
-     * @param null|callable|string|PropertyPath $value  The callable or path for
+     * @param callable|string|PropertyPath|null $value  The callable or path for
      *                                                  generating the choice values
      *
-     * @return ChoiceList The choice list
+     * @return ChoiceList
      */
     public function createListFromLoader(ChoiceLoader $loader, $value = null): ChoiceList
     {
@@ -138,7 +131,7 @@ public function createListFromLoader(ChoiceLoader $loader, $value = null): Choic
     }
 
     /**
-     * {@inheritdoc}
+     * @inheritdoc
      *
      * @param ChoiceList                              $list             The choice list
      * @param null|array|callable|string|PropertyPath $preferredChoices The preferred choices
@@ -147,7 +140,7 @@ public function createListFromLoader(ChoiceLoader $loader, $value = null): Choic
      * @param null|callable|string|PropertyPath       $groupBy          The callable or path generating the group names
      * @param null|array|callable|string|PropertyPath $attr             The callable or path generating the HTML attributes
      *
-     * @return ChoiceListView The choice list view
+     * @return ChoiceListView
      */
     public function createView(ChoiceList $list, $preferredChoices = null, $label = null, $index = null, $groupBy = null, $attr = null): ChoiceListView
     {

src/Extension/Core/DataTransformer/BaseDateTimeTransformer.php

@@ -44,12 +44,12 @@ abstract class BaseDateTimeTransformer implements DataTransformer
     /**
      * Constructor.
      *
-     * @param string $inputTimezone  The name of the input timezone
-     * @param string $outputTimezone The name of the output timezone
+     * @param string|null $inputTimezone  The name of the input timezone
+     * @param string|null $outputTimezone The name of the output timezone
      *
      * @throws UnexpectedTypeException if a timezone is not a string
      */
-    public function __construct(string $inputTimezone = null, string $outputTimezone = null)
+    public function __construct(?string $inputTimezone = null, ?string $outputTimezone = null)
     {
         $this->inputTimezone = $inputTimezone ?? date_default_timezone_get();
         $this->outputTimezone = $outputTimezone ?? date_default_timezone_get();

src/Extension/Core/DataTransformer/BirthdayTransformer.php

@@ -109,7 +109,7 @@ private function validateDate(\DateTimeInterface $value)
 
         if (!$currentDate) {
             $currentDate = new \DateTime('now', new \DateTimeZone('UTC'));
-            $currentDate->setTime(0, 0, 0);
+            $currentDate->setTime(0, 0);
         }
 
         if ($value > $currentDate) {

src/Extension/Core/DataTransformer/ChoiceToLabelTransformer.php

@@ -47,7 +47,8 @@ public function transform($choice)
             $this->choiceListView->initChoicesByLabel();
         }
 
-        $value = current($this->choiceList->getValuesForChoices([$choice]));
+        $value = $this->choiceList->getValuesForChoices([$choice]);
+        $value = current($value);
 
         if (!array_key_exists($value, $this->choiceListView->labelsByValue)) {
             throw new TransformationFailedException(sprintf('The choice "%s" does not exist or is not unique', $choice));

src/Extension/Core/DataTransformer/ChoiceToValueTransformer.php

@@ -31,7 +31,9 @@ public function __construct(ChoiceList $choiceList)
 
     public function transform($choice)
     {
-        return (string) current($this->choiceList->getValuesForChoices([$choice]));
+        $value = $this->choiceList->getValuesForChoices([$choice]);
+
+        return (string) current($value);
     }
 
     public function reverseTransform($value)

src/Extension/Core/DataTransformer/DateTimeToLocalizedStringTransformer.php

@@ -72,14 +72,14 @@ public function __construct(string $inputTimezone = null, string $outputTimezone
     /**
      * Transforms a normalized date into a localized date string/array.
      *
-     * @param \DateTimeInterface $dateTime A DateTimeInterface object
+     * @param \DateTimeInterface|null $dateTime A DateTimeInterface object
      *
      * @throws TransformationFailedException if the given value is not a \DateTimeInterface
      *                                       or if the date could not be transformed
      *
-     * @return string
+     * @return string|null
      */
-    public function transform($dateTime)
+    public function transform($dateTime): ?string
     {
         if (null === $dateTime) {
             return '';
@@ -101,7 +101,7 @@ public function transform($dateTime)
     /**
      * Transforms a localized date string/array into a normalized date.
      *
-     * @param string|array $value Localized date string/array
+     * @param string|null $value Localized date string
      *
      * @throws TransformationFailedException if the given value is not a string,
      *                                       if the date could not be parsed
@@ -110,11 +110,11 @@ public function transform($dateTime)
      */
     public function reverseTransform($value)
     {
-        if (!is_string($value)) {
-            throw new TransformationFailedException('Expected a string.');
+        if (null !== $value && !is_string($value)) {
+            throw new TransformationFailedException('Expected a string or null.');
         }
 
-        if ('' === $value) {
+        if (null === $value || '' === $value) {
             return null;
         }
 

src/Extension/Core/DataTransformer/DateTimeToRfc3339Transformer.php

@@ -23,7 +23,7 @@ final class DateTimeToRfc3339Transformer extends BaseDateTimeTransformer
     /**
      * {@inheritdoc}
      */
-    public function transform($dateTime)
+    public function transform($dateTime): ?string
     {
         if (null === $dateTime) {
             return '';
@@ -47,13 +47,13 @@ public function transform($dateTime)
     /**
      * {@inheritdoc}
      */
-    public function reverseTransform($rfc3339)
+    public function reverseTransform($rfc3339): ?\DateTime
     {
-        if (!is_string($rfc3339)) {
-            throw new TransformationFailedException('Expected a string.');
+        if (null !== $rfc3339 && !is_string($rfc3339)) {
+            throw new TransformationFailedException('Expected a string or null.');
         }
 
-        if ('' === $rfc3339) {
+        if (null === $rfc3339 || '' === $rfc3339) {
             return null;
         }
 

src/Extension/Core/DataTransformer/DateTimeToStringTransformer.php

@@ -82,9 +82,9 @@ public function __construct(string $inputTimezone = null, string $outputTimezone
      *                                       instance or if the output timezone
      *                                       is not supported
      *
-     * @return string A value as produced by PHP's date() function
+     * @return string|null A value as produced by PHP's date() function
      */
-    public function transform($value)
+    public function transform($value): ?string
     {
         if (null === $value) {
             return '';
@@ -118,14 +118,14 @@ public function transform($value)
      *
      * @return \DateTime|null An instance of \DateTime
      */
-    public function reverseTransform($value)
+    public function reverseTransform($value): ?\DateTime
     {
-        if ('' === $value || null === $value) {
-            return null;
+        if (null !== $value && !is_string($value)) {
+            throw new TransformationFailedException('Expected a string or null.');
         }
 
-        if (!is_string($value)) {
-            throw new TransformationFailedException('Expected a string.');
+        if (null === $value || '' === $value) {
+            return null;
         }
 
         try {

src/Extension/Core/DataTransformer/DateTimeToTimestampTransformer.php

@@ -34,7 +34,7 @@ final class DateTimeToTimestampTransformer extends BaseDateTimeTransformer
      *
      * @return int|null A timestamp
      */
-    public function transform($dateTime)
+    public function transform($dateTime): ?int
     {
         if (null === $dateTime) {
             return null;
@@ -57,14 +57,14 @@ public function transform($dateTime)
      *
      * @return \DateTime|null
      */
-    public function reverseTransform($value)
+    public function reverseTransform($value): ?\DateTime
     {
-        if (null === $value) {
-            return null;
+        if (null !== $value && !is_numeric($value)) {
+            throw new TransformationFailedException('Expected a numeric or null.');
         }
 
-        if (!is_numeric($value)) {
-            throw new TransformationFailedException('Expected a numeric.');
+        if (null === $value || '' === $value) {
+            return null;
         }
 
         try {