nunomaduro / phpinsights

🔰 Instant PHP quality checks from your console
https://phpinsights.com
MIT License
5.26k stars 279 forks source link

Show error when a insight fails without crashing #242

Open douglasjam opened 5 years ago

douglasjam commented 5 years ago
Q A
Bug report? yes
Feature request? no
Library version 1.7.0
~/folderA/folderB/projectA (master) ./vendor/bin/phpinsights

 135/253 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░░░░░]  53%PHP Fatal error:  Uncaught TypeError: Argument 2 passed to SlevomatCodingStandard\Helpers\FunctionHelper::getParametersTypeHints() must be of the type int, null given, called in /Users/user/folderA/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/DisallowArrayTypeHintSyntaxSniff.php on line 150 and defined in /Users/user/folderA/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/FunctionHelper.php:143
Stack trace:
#0 /Users/user/folderA/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/DisallowArrayTypeHintSyntaxSniff.php(150): SlevomatCodingStandard\Helpers\FunctionHelper::getParametersTypeHints(Object(NunoMaduro\PhpInsights\Domain\File), NULL)
#1 /Users/user/folderA/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/DisallowArrayTypeHintSyntaxSniff.php(97): SlevomatCodingStandard\Sniffs\TypeHints\DisallowArrayTypeHintSyntaxSni in /Users/user/folderA/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/FunctionHelper.php on line 143

Fatal error: Uncaught TypeError: Argument 2 passed to SlevomatCodingStandard\Helpers\FunctionHelper::getParametersTypeHints() must be of the type int, null given, called in /Users/user/folderA/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/DisallowArrayTypeHintSyntaxSniff.php on line 150 and defined in /Users/user/folderA/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/FunctionHelper.php:143
Stack trace:
#0 /Users/user/folderA/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/DisallowArrayTypeHintSyntaxSniff.php(150): SlevomatCodingStandard\Helpers\FunctionHelper::getParametersTypeHints(Object(NunoMaduro\PhpInsights\Domain\File), NULL)
#1 /Users/user/folderA/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/DisallowArrayTypeHintSyntaxSniff.php(97): SlevomatCodingStandard\Sniffs\TypeHints\DisallowArrayTypeHintSyntaxSni in /Users/user/folderA/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/FunctionHelper.php on line 143

With verbose mode:

...
PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\DeprecatedFunctionsSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\Functions\FunctionCallArgumentSpacingSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\SAPIUsageSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\NamingConventions\UpperCaseConstantNameSniff
PHP_CodeSniffer\Standards\Squiz\Sniffs\WhiteSpace\SuperfluousWhitespaceSniff
ObjectCalisthenics\Sniffs\Classes\ForbiddenPublicPropertySniff
PHP_CodeSniffer\Standards\PSR2\Sniffs\Classes\PropertyDeclarationSniff
SlevomatCodingStandard\Sniffs\Variables\UnusedVariableSniff
ObjectCalisthenics\Sniffs\NamingConventions\ElementNameMinimalLengthSniff
SlevomatCodingStandard\Sniffs\TypeHints\TypeHintDeclarationSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\Functions\CallTimePassByReferenceSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\Functions\FunctionCallArgumentSpacingSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ArbitraryParenthesesSpacingSniff
PHP_CodeSniffer\Standards\Squiz\Sniffs\WhiteSpace\SuperfluousWhitespaceSniff
SlevomatCodingStandard\Sniffs\Classes\ModernClassNameReferenceSniff
SlevomatCodingStandard\Sniffs\PHP\OptimizedFunctionsWithoutUnpackingSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\Functions\CallTimePassByReferenceSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\DeprecatedFunctionsSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\Functions\FunctionCallArgumentSpacingSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\SAPIUsageSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\NamingConventions\UpperCaseConstantNameSniff
PHP_CodeSniffer\Standards\Squiz\Sniffs\WhiteSpace\SuperfluousWhitespaceSniff
SlevomatCodingStandard\Sniffs\Commenting\EmptyCommentSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\Commenting\FixmeSniff
PHP_CodeSniffer\Standards\Generic\Sniffs\Commenting\TodoSniff
SlevomatCodingStandard\Sniffs\Commenting\ForbiddenCommentsSniff
SlevomatCodingStandard\Sniffs\Commenting\InlineDocCommentDeclarationSniff
SlevomatCodingStandard\Sniffs\TypeHints\DisallowArrayTypeHintSyntaxSniff
PHP Fatal error:  Uncaught TypeError: Argument 2 passed to SlevomatCodingStandard\Helpers\FunctionHelper::getParametersTypeHints() must be of the type int, null given, called in /Users/user/company/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/DisallowArrayTypeHintSyntaxSniff.php on line 150 and defined in /Users/user/company/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/FunctionHelper.php:143
Stack trace:
#0 /Users/user/company/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/DisallowArrayTypeHintSyntaxSniff.php(150): SlevomatCodingStandard\Helpers\FunctionHelper::getParametersTypeHints(Object(NunoMaduro\PhpInsights\Domain\File), NULL)
#1 /Users/user/company/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/DisallowArrayTypeHintSyntaxSniff.php(97): SlevomatCodingStandard\Sniffs\TypeHints\DisallowArrayTypeHintSyntaxSni in /Users/user/company/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/FunctionHelper.php on line 143

Fatal error: Uncaught TypeError: Argument 2 passed to SlevomatCodingStandard\Helpers\FunctionHelper::getParametersTypeHints() must be of the type int, null given, called in /Users/user/company/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/DisallowArrayTypeHintSyntaxSniff.php on line 150 and defined in /Users/user/company/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/FunctionHelper.php:143
Stack trace:
#0 /Users/user/company/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/DisallowArrayTypeHintSyntaxSniff.php(150): SlevomatCodingStandard\Helpers\FunctionHelper::getParametersTypeHints(Object(NunoMaduro\PhpInsights\Domain\File), NULL)
#1 /Users/user/company/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Sniffs/TypeHints/DisallowArrayTypeHintSyntaxSniff.php(97): SlevomatCodingStandard\Sniffs\TypeHints\DisallowArrayTypeHintSyntaxSni in /Users/user/company/folderB/projectA/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/FunctionHelper.php on line 143

The file in which it threw the exception

<?php declare(strict_types=1);

/* imports supressed */

class CategoryPropagationService
{
    private const BATCH_SIZE = 100;

    /** @var CategoryRepository */
    private $categoryRepository;

    /** @var EventDispatcherInterface */
    private $dispatcher;

    private $batchSize = self::BATCH_SIZE;

    public function __construct(CategoryRepository $repository, EventDispatcherInterface $dispatcher)
    {
        $this->categoryRepository = $repository;
        $this->dispatcher = $dispatcher;
    }

    public function propagateNameAndUrl(string $categoryId, string $oldUrl): void
    {
        $newCategory = $this->categoryRepository->find($categoryId);
        $categories = [];
        $cursor = $this->categoryRepository->getDescendants($oldUrl);
        while ($category = $cursor->getNext()) {
            $category->setName($this->getNewName($category, $newCategory));
            $category->setUrl($this->getNewUrl($category, $newCategory));
            $count = array_push($categories, $category);

            if ($count === $this->batchSize) {
                $this->flush($categories);
                $categories = [];
            }
        }

        if (!empty($categories)) {
            $this->flush($categories);
        }
    }

    public function propagateInactivity(string $categoryId): void
    {
        $category = $this->categoryRepository->find($categoryId);

        $categories = [];
        $cursor = $this->categoryRepository->getDescendants($category->getUrl());
        while ($category = $cursor->getNext()) {
            $category->setStatus(Category::STATUS_INACTIVE);
            $count = array_push($categories, $category);

            if ($count === $this->batchSize) {
                $this->deactivate($categories);
                $categories = [];
            }
        }
        if (!empty($categories)) {
            $this->deactivate($categories);
        }
    }

    private function getNewName(Category $descendant, Category $newCategory): string
    {
        $nameTokens = $this->getTokens($descendant->getName());
        $newNameTokens = $this->getTokens($newCategory->getName());
        return $this->buildNewPath($newNameTokens, $nameTokens);
    }

    private function getNewUrl(Category $category, Category $newCategory): string
    {
        $nameTokens = $this->getTokens($category->getUrl());
        $newNameTokens = $this->getTokens($newCategory->getUrl());
        return $this->buildNewPath($newNameTokens, $nameTokens);
    }

    /** @return string[] */
    private function getTokens(string $field): array
    {
        return explode(Category::LEVEL_SEPARATOR, $field);
    }

    /**
     * @param string[] $newNameTokens
     * @param string[] $nameTokens
     * @return string[]
     */
    private function buildNewPath(array $newNameTokens, array $nameTokens): string
    {
        $count = \count($newNameTokens);
        for ($index = 0; $index < $count; $index++) {
            $nameTokens[$index] = $newNameTokens[$index];
        }

        return implode(Category::LEVEL_SEPARATOR, $nameTokens);
    }

    /** @param Category[] $categories */
    private function flush(array $categories): void
    {
        $this->categoryRepository->flushBatch($categories);
        $this->dispatcher->dispatch(CategoriesNameAndUrlUpdated::NAME, new CategoriesNameAndUrlUpdated($categories));
    }

    /** @param Category[] $categories */
    private function deactivate(array $categories): void
    {
        /** @param Category[] $categories */
        $this->categoryRepository->flushBatch($categories);
        $this->dispatcher->dispatch(CategoriesDeactivated::NAME, new CategoriesDeactivated($categories));
    }
}
olivernybroe commented 5 years ago

Not sure if this is actually a bug, the issue is because you have set the @param annotation inside a function.

The param annotation is only meant to be used as docblock for a function, not inside a function. In that case you should use @var instead.

The MVP for this error is the following

<?php
class MyClass
{
    private function method(array $categories): void
    {
        /** @param Category[] $categories */
    }
}

This issue is not from our package, but from the slevomat coding standard package. So I'll report the bug at their repo and see if they are up for fixing it or if they think it is just a coding mistake.

douglasjam commented 5 years ago

Indeed, it's a bug in the class, but shouldn't it ignore/log and do not crash the entire analyzer? I'm sure many legacies / badly written would like to use the analyzer, but fix everything before to get the analyze would be bad

https://en.wikipedia.org/wiki/Defensive_programming

olivernybroe commented 5 years ago

I posted it at Slevomat and they say it is a coding mistake and not something they will fix. https://github.com/slevomat/coding-standard/issues/732

Hmm, I see your point and I actually agree with you. I guess we should add support for throwing the error, but without crashing our analyzer. This way you know something went wrong, but you can still get all of the other insights.