Completed
Pull Request — 5.6 (#2830)
by Jeroen
14:14
created

AdminBundle/Toolbar/ExceptionDataCollector.php (3 issues)

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

1
<?php
2
3
namespace Kunstmaan\AdminBundle\Toolbar;
4
5
use Doctrine\ORM\EntityManagerInterface;
6
use Kunstmaan\AdminBundle\Entity\Exception;
7
use Kunstmaan\AdminBundle\Helper\Toolbar\AbstractDataCollector;
8
use Symfony\Component\HttpFoundation\Request;
9
use Symfony\Component\HttpFoundation\Response;
10
11
class ExceptionDataCollector extends AbstractDataCollector
12
{
13
    /**
14
     * @var EntityManagerInterface
15
     */
16
    private $em;
17
18
    public function __construct(EntityManagerInterface $em)
19
    {
20
        $this->em = $em;
21
    }
22
23
    /**
24
     * @return array
0 ignored issues
show
Consider making the return type a bit more specific; maybe use string[].

This check looks for the generic type array as a return type and suggests a more specific type. This type is inferred from the actual code.

Loading history...
25
     */
26
    public function getAccessRoles()
27
    {
28
        return ['ROLE_ADMIN'];
29
    }
30
31
    /**
32
     * @return array
33
     */
34
    public function collectData()
35
    {
36
        $model = $this->em->getRepository(Exception::class)->findExceptionStatistics();
0 ignored issues
show
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method findExceptionStatistics() does only exist in the following implementations of said interface: Kunstmaan\AdminBundle\Re...ory\ExceptionRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
37
        if (isset($model['cp_all'], $model['cp_sum'])) {
38
            return [
39
                'data' => $model,
40
            ];
41
        } else {
42
            return [];
43
        }
44
    }
45
46
    public function collect(Request $request, Response $response, \Exception $exception = null)
47
    {
48
        if (false === $this->isEnabled()) {
49
            $this->data = false;
0 ignored issues
show
Documentation Bug introduced by
It seems like false of type false is incompatible with the declared type array of property $data.

Our type inference engine has found an assignment to a property that is incompatible with the declared type of that property.

Either this assignment is in error or the assigned type should be added to the documentation/type hint for that property..

Loading history...
50
        } else {
51
            $this->data = $this->collectData();
52
        }
53
    }
54
55
    /**
56
     * Gets the data for template
57
     *
58
     * @return array The request events
59
     */
60
    public function getTemplateData()
61
    {
62
        return $this->data;
63
    }
64
65
    /**
66
     * @return string
67
     */
68
    public function getName()
69
    {
70
        return 'kuma_exception';
71
    }
72
73
    /**
74
     * @return bool
75
     */
76
    public function isEnabled()
77
    {
78
        return true;
79
    }
80
81
    public function reset()
82
    {
83
        $this->data = [];
84
    }
85
}
86