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 Onigoetz\Deployer\Configuration; |
||
4 | |||
5 | use Onigoetz\Deployer\Configuration\Containers\ExtendableConfigurationContainer; |
||
6 | use Onigoetz\Deployer\Configuration\Sources\Cloned; |
||
7 | use Onigoetz\Deployer\Configuration\Sources\Upload; |
||
8 | |||
9 | class Source extends ExtendableConfigurationContainer |
||
10 | { |
||
11 | /** |
||
12 | * Create a source with the right class |
||
13 | * |
||
14 | * @param string $name The source name |
||
15 | * @param array $data |
||
16 | * @param ConfigurationManager $manager |
||
17 | * @param Source $parent |
||
18 | * @throws \LogicException |
||
19 | * @return Cloned|Upload |
||
20 | */ |
||
21 | 135 | public static function make($name, array $data, ConfigurationManager $manager, Source $parent = null) |
|
22 | { |
||
23 | 135 | $strategy = static::findStrategy($name, $data, $manager, $parent); |
|
24 | |||
25 | switch ($strategy) { |
||
26 | 132 | case 'clone': |
|
27 | 96 | return new Cloned($name, $data, $manager, $parent); |
|
0 ignored issues
–
show
|
|||
28 | break; |
||
0 ignored issues
–
show
break is not strictly necessary here and could be removed.
The break statement is not necessary if it is preceded for example by a return statement: switch ($x) {
case 1:
return 'foo';
break; // This break is not necessary and can be left off.
}
If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.
Loading history...
|
|||
29 | 27 | case 'upload': |
|
30 | 36 | return new Upload($name, $data, $manager, $parent); |
|
0 ignored issues
–
show
It seems like
$parent defined by parameter $parent on line 21 can also be of type object<Onigoetz\Deployer\Configuration\Source> ; however, Onigoetz\Deployer\Config...ontainer::__construct() does only seem to accept null|object<self> , maybe add an additional type check?
This check looks at variables that have been passed in as parameters and are passed out again to other methods. If the outgoing method call has stricter type requirements than the method itself, an issue is raised. An additional type check may prevent trouble.
Loading history...
|
|||
31 | break; |
||
0 ignored issues
–
show
break is not strictly necessary here and could be removed.
The break statement is not necessary if it is preceded for example by a return statement: switch ($x) {
case 1:
return 'foo';
break; // This break is not necessary and can be left off.
}
If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.
Loading history...
|
|||
32 | 2 | default: |
|
33 | 2 | } |
|
34 | |||
35 | 3 | throw new \LogicException("Unrecognized strategy '{$strategy}'"); |
|
36 | } |
||
37 | |||
38 | 135 | protected static function findStrategy($name, $data, ConfigurationManager $manager, Source $parent = null) |
|
39 | { |
||
40 | 135 | if (isset($data['strategy'])) { |
|
41 | 132 | return $data['strategy']; |
|
42 | } |
||
43 | |||
44 | 15 | if ($parent) { |
|
45 | 6 | return $parent->getStrategy(); |
|
46 | } |
||
47 | |||
48 | 12 | if (isset($data['extends']) && $manager->has('source', $data['extends'])) { |
|
49 | 9 | return $manager->get('source', $data['extends'])->getStrategy(); |
|
0 ignored issues
–
show
It seems like you code against a specific sub-type and not the parent class
Onigoetz\Deployer\Config...\ConfigurationContainer as the method getStrategy() does only exist in the following sub-classes of Onigoetz\Deployer\Config...\ConfigurationContainer : Onigoetz\Deployer\Configuration\Source , Onigoetz\Deployer\Configuration\Sources\Cloned , Onigoetz\Deployer\Configuration\Sources\Upload . Maybe you want to instanceof check for one of these explicitly?
Let’s take a look at an example: abstract class User
{
/** @return string */
abstract public function getPassword();
}
class MyUser extends 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 sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types
inside the if block in such a case.
Loading history...
|
|||
50 | } |
||
51 | |||
52 | 3 | throw new \LogicException("Cannot find a valid strategy for the source '$name'"); |
|
53 | } |
||
54 | |||
55 | /** |
||
56 | * {@inheritdoc} |
||
57 | */ |
||
58 | 48 | public function getContainerType() |
|
59 | { |
||
60 | 48 | return 'source'; |
|
61 | } |
||
62 | |||
63 | 12 | public function getStrategy() |
|
64 | { |
||
65 | 12 | return $this->getValueOrFail('strategy', "no 'strategy' specified for source '{$this->name}''"); |
|
66 | } |
||
67 | |||
68 | /** |
||
69 | * @throws \LogicException |
||
70 | * @return mixed |
||
71 | */ |
||
72 | 60 | public function getPath() |
|
73 | { |
||
74 | 60 | return $this->getValueOrFail('path', "no 'path' specified for source '{$this->name}''"); |
|
75 | } |
||
76 | |||
77 | /** |
||
78 | * {@inheritdoc} |
||
79 | */ |
||
80 | 27 | public function checkValidity() |
|
81 | { |
||
82 | 27 | $this->getPath(); |
|
83 | |||
84 | 21 | return true; |
|
85 | } |
||
86 | } |
||
87 |
This check looks at variables that have been passed in as parameters and are passed out again to other methods.
If the outgoing method call has stricter type requirements than the method itself, an issue is raised.
An additional type check may prevent trouble.