Completed
Pull Request — master (#138)
by Christophe
09:48
created

Configuration::getConfigTreeBuilder()   C

Complexity

Conditions 12
Paths 1

Size

Total Lines 111
Code Lines 91

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 92
CRAP Score 12

Importance

Changes 0
Metric Value
dl 0
loc 111
ccs 92
cts 92
cp 1
rs 5.034
c 0
b 0
f 0
cc 12
eloc 91
nc 1
nop 0
crap 12

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
3
namespace Http\HttplugBundle\DependencyInjection;
4
5
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
6
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
7
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
8
use Symfony\Component\Config\Definition\ConfigurationInterface;
9
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
10
11
/**
12
 * This class contains the configuration information for the bundle.
13
 *
14
 * This information is solely responsible for how the different configuration
15
 * sections are normalized, and merged.
16
 *
17
 * @author David Buchmann <[email protected]>
18
 * @author Tobias Nyholm <[email protected]>
19
 */
20
class Configuration implements ConfigurationInterface
21
{
22
    /**
23
     * Whether to use the debug mode.
24
     *
25
     * @see https://github.com/doctrine/DoctrineBundle/blob/v1.5.2/DependencyInjection/Configuration.php#L31-L41
26
     *
27
     * @var bool
28
     */
29
    private $debug;
30
31
    /**
32
     * @param bool $debug
33
     */
34 16
    public function __construct($debug)
35
    {
36 16
        $this->debug = (bool) $debug;
37 16
    }
38
39
    /**
40
     * {@inheritdoc}
41
     */
42 16
    public function getConfigTreeBuilder()
43
    {
44 16
        $treeBuilder = new TreeBuilder();
45 16
        $rootNode = $treeBuilder->root('httplug');
46
47 16
        $this->configureClients($rootNode);
48 16
        $this->configureSharedPlugins($rootNode);
49
50
        $rootNode
1 ignored issue
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Symfony\Component\Config...\Builder\NodeDefinition as the method fixXmlConfig() does only exist in the following sub-classes of Symfony\Component\Config...\Builder\NodeDefinition: Symfony\Component\Config...der\ArrayNodeDefinition. 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

  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 parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
51 16
            ->validate()
52
                ->ifTrue(function ($v) {
53 13
                    return !empty($v['classes']['client'])
54 13
                        || !empty($v['classes']['message_factory'])
55 10
                        || !empty($v['classes']['uri_factory'])
56 13
                        || !empty($v['classes']['stream_factory']);
57 16
                })
58
                ->then(function ($v) {
59 3
                    foreach ($v['classes'] as $key => $class) {
60 3
                        if (null !== $class && !class_exists($class)) {
61 1
                            throw new InvalidConfigurationException(sprintf(
62 1
                                'Class %s specified for httplug.classes.%s does not exist.',
63 1
                                $class,
64
                                $key
65 1
                            ));
66
                        }
67 2
                    }
68
69 2
                    return $v;
70 16
                })
71 16
            ->end()
72 16
            ->beforeNormalization()
73
                ->ifTrue(function ($v) {
74 16
                    return is_array($v) && array_key_exists('toolbar', $v) && is_array($v['toolbar']);
75 16
                })
76
                ->then(function ($v) {
77 4
                    if (array_key_exists('profiling', $v)) {
78 1
                        throw new InvalidConfigurationException('Can\'t configure both "toolbar" and "profiling" section. The "toolbar" config is deprecated as of version 1.3.0, please only use "profiling".');
79
                    }
80
81 3
                    @trigger_error('"httplug.toolbar" config is deprecated since version 1.3 and will be removed in 2.0. Use "httplug.profiling" instead.', E_USER_DEPRECATED);
0 ignored issues
show
Security Best Practice introduced by
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.

If you suppress an error, we recommend checking for the error condition explicitly:

// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
Loading history...
82
83 3
                    if (array_key_exists('enabled', $v['toolbar']) && 'auto' === $v['toolbar']['enabled']) {
84 1
                        @trigger_error('"auto" value in "httplug.toolbar" config is deprecated since version 1.3 and will be removed in 2.0. Use a boolean value instead.', E_USER_DEPRECATED);
1 ignored issue
show
Security Best Practice introduced by
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.

If you suppress an error, we recommend checking for the error condition explicitly:

// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
Loading history...
85 1
                        $v['toolbar']['enabled'] = $this->debug;
86 1
                    }
87
88 3
                    $v['profiling'] = $v['toolbar'];
89
90 3
                    unset($v['toolbar']);
91
92 3
                    return $v;
93 16
                })
94 16
            ->end()
95 16
            ->fixXmlConfig('client')
96 16
            ->children()
97 16
                ->arrayNode('main_alias')
98 16
                    ->addDefaultsIfNotSet()
99 16
                    ->info('Configure which service the main alias point to.')
100 16
                    ->children()
101 16
                        ->scalarNode('client')->defaultValue('httplug.client.default')->end()
102 16
                        ->scalarNode('message_factory')->defaultValue('httplug.message_factory.default')->end()
103 16
                        ->scalarNode('uri_factory')->defaultValue('httplug.uri_factory.default')->end()
104 16
                        ->scalarNode('stream_factory')->defaultValue('httplug.stream_factory.default')->end()
105 16
                    ->end()
106 16
                ->end()
107 16
                ->arrayNode('classes')
108 16
                    ->addDefaultsIfNotSet()
109 16
                    ->info('Overwrite a service class instead of using the discovery mechanism.')
110 16
                    ->children()
111 16
                        ->scalarNode('client')->defaultNull()->end()
112 16
                        ->scalarNode('message_factory')->defaultNull()->end()
113 16
                        ->scalarNode('uri_factory')->defaultNull()->end()
114 16
                        ->scalarNode('stream_factory')->defaultNull()->end()
115 16
                    ->end()
116 16
                ->end()
117 16
                ->arrayNode('profiling')
118 16
                    ->addDefaultsIfNotSet()
119 16
                    ->treatFalseLike(['enabled' => false])
120 16
                    ->treatTrueLike(['enabled' => true])
121 16
                    ->treatNullLike(['enabled' => $this->debug])
122 16
                    ->info('Extend the debug profiler with information about requests.')
123 16
                    ->children()
124 16
                        ->booleanNode('enabled')
125 16
                            ->info('Turn the toolbar on or off. Defaults to kernel debug mode.')
126 16
                            ->defaultValue($this->debug)
127 16
                        ->end()
128 16
                        ->scalarNode('formatter')->defaultNull()->end()
129 16
                        ->integerNode('captured_body_length')
130 16
                            ->defaultValue(0)
131 16
                            ->info('Limit long HTTP message bodies to x characters. If set to 0 we do not read the message body. Only available with the default formatter (FullHttpMessageFormatter).')
132 16
                        ->end()
133 16
                    ->end()
134 16
                ->end()
135 16
                ->arrayNode('discovery')
136 16
                    ->addDefaultsIfNotSet()
137 16
                    ->info('Control what clients should be found by the discovery.')
138 16
                    ->children()
139 16
                        ->scalarNode('client')
140 16
                            ->defaultValue('auto')
141 16
                            ->info('Set to "auto" to see auto discovered client in the web profiler. If provided a service id for a client then this client will be found by auto discovery.')
142 16
                        ->end()
143 16
                        ->scalarNode('async_client')
144 16
                            ->defaultNull()
145 16
                            ->info('Set to "auto" to see auto discovered client in the web profiler. If provided a service id for a client then this client will be found by auto discovery.')
146 16
                        ->end()
147 16
                    ->end()
148 16
                ->end()
149 16
            ->end();
150
151 16
        return $treeBuilder;
152
    }
153
154 16
    private function configureClients(ArrayNodeDefinition $root)
155
    {
156 16
        $root->children()
1 ignored issue
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Symfony\Component\Config...\Builder\NodeDefinition as the method fixXmlConfig() does only exist in the following sub-classes of Symfony\Component\Config...\Builder\NodeDefinition: Symfony\Component\Config...der\ArrayNodeDefinition. 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

  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 parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
157 16
            ->arrayNode('clients')
158 16
                ->useAttributeAsKey('name')
159 16
                ->prototype('array')
160 16
                ->fixXmlConfig('plugin')
161 16
                ->validate()
162
                    ->ifTrue(function ($config) {
163
                        // Make sure we only allow one of these to be true
164 7
                        return (bool) $config['flexible_client'] + (bool) $config['http_methods_client'] + (bool) $config['batch_client'] >= 2;
165 16
                    })
166 16
                    ->thenInvalid('A http client can\'t be decorated with both FlexibleHttpClient and HttpMethodsClient. Only one of the following options can be true. ("flexible_client", "http_methods_client", "batch_client")')
167 16
                ->end()
168 16
                ->children()
169 16
                    ->scalarNode('factory')
170 16
                        ->isRequired()
171 16
                        ->cannotBeEmpty()
172 16
                        ->info('The service id of a factory to use when creating the adapter.')
173 16
                    ->end()
174 16
                    ->booleanNode('flexible_client')
175 16
                        ->defaultFalse()
176 16
                        ->info('Set to true to get the client wrapped in a FlexibleHttpClient which emulates async or sync behavior.')
177 16
                    ->end()
178 16
                    ->booleanNode('http_methods_client')
179 16
                        ->defaultFalse()
180 16
                        ->info('Set to true to get the client wrapped in a HttpMethodsClient which emulates provides functions for HTTP verbs.')
181 16
                    ->end()
182 16
                    ->booleanNode('batch_client')
183 16
                        ->defaultFalse()
184 16
                        ->info('Set to true to get the client wrapped in a BatchClient which allows you to send multiple request at the same time.')
185 16
                    ->end()
186 16
                    ->variableNode('config')->defaultValue([])->end()
187 16
                    ->append($this->createClientPluginNode())
188 16
                ->end()
189 16
            ->end()
190 16
        ->end();
191 16
    }
192
193
    /**
194
     * @param ArrayNodeDefinition $root
195
     */
196 16
    private function configureSharedPlugins(ArrayNodeDefinition $root)
197
    {
198
        $pluginsNode = $root
199 16
            ->children()
200 16
                ->arrayNode('plugins')
201 16
                ->info('Global plugin configuration. Plugins need to be explicitly added to clients.')
202 16
                ->addDefaultsIfNotSet()
203
            // don't call end to get the plugins node
204 16
        ;
205 16
        $this->addSharedPluginNodes($pluginsNode);
206 16
    }
207
208
    /**
209
     * Createplugins node of a client.
210
     *
211
     * @return ArrayNodeDefinition The plugin node
212
     */
213 16
    private function createClientPluginNode()
214
    {
215 16
        $builder = new TreeBuilder();
216 16
        $node = $builder->root('plugins');
217
218
        /** @var ArrayNodeDefinition $pluginList */
219
        $pluginList = $node
220 16
            ->info('A list of plugin service ids and client specific plugin definitions. The order is important.')
221 16
            ->prototype('array')
222 16
        ;
223
        $pluginList
224
            // support having just a service id in the list
225 16
            ->beforeNormalization()
226
                ->always(function ($plugin) {
227 8
                    if (is_string($plugin)) {
228
                        return [
229
                            'reference' => [
230 7
                                'enabled' => true,
231 7
                                'id' => $plugin,
232 7
                            ],
233 7
                        ];
234
                    }
235
236 5
                    return $plugin;
237 16
                })
238 16
            ->end()
239
240 16
            ->validate()
241
                ->always(function ($plugins) {
242 7
                    foreach ($plugins as $name => $definition) {
243 7
                        if ('authentication' === $name) {
244 7
                            if (!count($definition)) {
245 7
                                unset($plugins['authentication']);
246 7
                            }
247 7
                        } elseif (!$definition['enabled']) {
248 7
                            unset($plugins[$name]);
249 7
                        }
250 7
                    }
251
252 7
                    return $plugins;
253 16
                })
254 16
            ->end()
255
        ;
256 16
        $this->addSharedPluginNodes($pluginList, true);
257
258
        $pluginList
259 16
            ->children()
260 16
                ->arrayNode('reference')
261 16
                    ->canBeEnabled()
262 16
                    ->info('Reference to a plugin service')
263 16
                    ->children()
264 16
                        ->scalarNode('id')
265 16
                            ->info('Service id of a plugin')
266 16
                            ->isRequired()
267 16
                            ->cannotBeEmpty()
268 16
                        ->end()
269 16
                    ->end()
270 16
                ->end()
271 16
                ->arrayNode('add_host')
272 16
                    ->canBeEnabled()
273 16
                    ->addDefaultsIfNotSet()
274 16
                    ->info('Set scheme, host and port in the request URI.')
275 16
                    ->children()
276 16
                        ->scalarNode('host')
277 16
                            ->info('Host name including protocol and optionally the port number, e.g. https://api.local:8000')
278 16
                            ->isRequired()
279 16
                            ->cannotBeEmpty()
280 16
                        ->end()
281 16
                        ->scalarNode('replace')
282 16
                            ->info('Whether to replace the host if request already specifies one')
283 16
                            ->defaultValue(false)
284 16
                        ->end()
285 16
                    ->end()
286 16
                ->end()
287 16
                ->arrayNode('header_append')
288 16
                    ->canBeEnabled()
289 16
                    ->info('Append headers to the request. If the header already exists the value will be appended to the current value.')
290 16
                    ->fixXmlConfig('header')
291 16
                    ->children()
292 16
                        ->arrayNode('headers')
293 16
                            ->info('Keys are the header names, values the header values')
294 16
                            ->normalizeKeys(false)
295 16
                            ->useAttributeAsKey('name')
296 16
                            ->prototype('scalar')->end()
297 16
                        ->end()
298 16
                    ->end()
299 16
                ->end()
300 16
                ->arrayNode('header_defaults')
301 16
                    ->canBeEnabled()
302 16
                    ->info('Set header to default value if it does not exist.')
303 16
                    ->fixXmlConfig('header')
304 16
                    ->children()
305 16
                        ->arrayNode('headers')
306 16
                            ->info('Keys are the header names, values the header values')
307 16
                            ->normalizeKeys(false)
308 16
                            ->useAttributeAsKey('name')
309 16
                            ->prototype('scalar')->end()
310 16
                        ->end()
311 16
                    ->end()
312 16
                ->end()
313 16
                ->arrayNode('header_set')
314 16
                    ->canBeEnabled()
315 16
                    ->info('Set headers to requests. If the header does not exist it wil be set, if the header already exists it will be replaced.')
316 16
                    ->fixXmlConfig('header')
317 16
                    ->children()
318 16
                        ->arrayNode('headers')
319 16
                            ->info('Keys are the header names, values the header values')
320 16
                            ->normalizeKeys(false)
321 16
                            ->useAttributeAsKey('name')
322 16
                            ->prototype('scalar')->end()
323 16
                        ->end()
324 16
                    ->end()
325 16
                ->end()
326 16
                ->arrayNode('header_remove')
327 16
                    ->canBeEnabled()
328 16
                    ->info('Remove headers from requests.')
329 16
                    ->fixXmlConfig('header')
330 16
                    ->children()
331 16
                        ->arrayNode('headers')
332 16
                            ->info('List of header names to remove')
333 16
                            ->prototype('scalar')->end()
334 16
                        ->end()
335 16
                    ->end()
336 16
                ->end()
337 16
            ->end()
338 16
        ->end();
339
340 16
        return $node;
341
    }
342
343
    /**
344
     * Add the definitions for shared plugin configurations.
345
     *
346
     * @param ArrayNodeDefinition $pluginNode The node to add to.
347
     * @param bool                $disableAll Some shared plugins are enabled by default. On the client, all are disabled by default.
348
     */
349 16
    private function addSharedPluginNodes(ArrayNodeDefinition $pluginNode, $disableAll = false)
350
    {
351 16
        $children = $pluginNode->children();
352
353 16
        $children->append($this->createAuthenticationPluginNode());
354
355 16
        $children->arrayNode('cache')
356 16
            ->canBeEnabled()
357 16
            ->addDefaultsIfNotSet()
358 16
                ->children()
359 16
                    ->scalarNode('cache_pool')
360 16
                        ->info('This must be a service id to a service implementing Psr\Cache\CacheItemPoolInterface')
361 16
                        ->isRequired()
362 16
                        ->cannotBeEmpty()
363 16
                    ->end()
364 16
                    ->scalarNode('stream_factory')
365 16
                        ->info('This must be a service id to a service implementing Http\Message\StreamFactory')
366 16
                        ->defaultValue('httplug.stream_factory')
367 16
                        ->cannotBeEmpty()
368 16
                    ->end()
369 16
                    ->arrayNode('config')
370 16
                        ->addDefaultsIfNotSet()
371 16
                        ->children()
372 16
                            ->scalarNode('default_ttl')->defaultNull()->end()
373 16
                            ->scalarNode('respect_cache_headers')->defaultTrue()->end()
374 16
                        ->end()
375 16
                    ->end()
376 16
                ->end()
377 16
            ->end();
378
        // End cache plugin
379
380 16
        $children->arrayNode('cookie')
381 16
            ->canBeEnabled()
382 16
                ->children()
383 16
                    ->scalarNode('cookie_jar')
384 16
                        ->info('This must be a service id to a service implementing Http\Message\CookieJar')
385 16
                        ->isRequired()
386 16
                        ->cannotBeEmpty()
387 16
                    ->end()
388 16
                ->end()
389 16
            ->end();
390
        // End cookie plugin
391
392 16
        $decoder = $children->arrayNode('decoder');
393 16
        $disableAll ? $decoder->canBeEnabled() : $decoder->canBeDisabled();
394 16
        $decoder->addDefaultsIfNotSet()
395 16
            ->children()
396 16
                ->scalarNode('use_content_encoding')->defaultTrue()->end()
397 16
            ->end()
398 16
        ->end();
399
        // End decoder plugin
400
401 16
        $children->arrayNode('history')
402 16
            ->canBeEnabled()
403 16
                ->children()
404 16
                    ->scalarNode('journal')
405 16
                        ->info('This must be a service id to a service implementing Http\Client\Plugin\Journal')
406 16
                        ->isRequired()
407 16
                        ->cannotBeEmpty()
408 16
                    ->end()
409 16
                ->end()
410 16
            ->end();
411
        // End history plugin
412
413 16
        $logger = $children->arrayNode('logger');
414 16
        $disableAll ? $logger->canBeEnabled() : $logger->canBeDisabled();
415 16
        $logger->addDefaultsIfNotSet()
416 16
            ->children()
417 16
                ->scalarNode('logger')
418 16
                    ->info('This must be a service id to a service implementing Psr\Log\LoggerInterface')
419 16
                    ->defaultValue('logger')
420 16
                    ->cannotBeEmpty()
421 16
                ->end()
422 16
                ->scalarNode('formatter')
423 16
                    ->info('This must be a service id to a service implementing Http\Message\Formatter')
424 16
                    ->defaultNull()
425 16
                ->end()
426 16
            ->end()
427 16
        ->end();
428
        // End logger plugin
429
430 16
        $redirect = $children->arrayNode('redirect');
431 16
        $disableAll ? $redirect->canBeEnabled() : $redirect->canBeDisabled();
432 16
        $redirect->addDefaultsIfNotSet()
433 16
            ->children()
434 16
                ->scalarNode('preserve_header')->defaultTrue()->end()
435 16
                ->scalarNode('use_default_for_multiple')->defaultTrue()->end()
436 16
            ->end()
437 16
        ->end();
438
        // End redirect plugin
439
440 16
        $retry = $children->arrayNode('retry');
441 16
        $disableAll ? $retry->canBeEnabled() : $retry->canBeDisabled();
442 16
        $retry->addDefaultsIfNotSet()
443 16
            ->children()
444 16
                ->scalarNode('retry')->defaultValue(1)->end() // TODO: should be called retries for consistency with the class
445 16
            ->end()
446 16
        ->end();
447
        // End retry plugin
448
449 16
        $stopwatch = $children->arrayNode('stopwatch');
450 16
        $disableAll ? $stopwatch->canBeEnabled() : $stopwatch->canBeDisabled();
451 16
        $stopwatch->addDefaultsIfNotSet()
452 16
            ->children()
453 16
                ->scalarNode('stopwatch')
454 16
                    ->info('This must be a service id to a service extending Symfony\Component\Stopwatch\Stopwatch')
455 16
                    ->defaultValue('debug.stopwatch')
456 16
                    ->cannotBeEmpty()
457 16
                ->end()
458 16
            ->end()
459 16
        ->end();
460
        // End stopwatch plugin
461 16
    }
462
463
    /**
464
     * Create configuration for authentication plugin.
465
     *
466
     * @return NodeDefinition Definition for the authentication node in the plugins list.
467
     */
468 16
    private function createAuthenticationPluginNode()
469
    {
470 16
        $builder = new TreeBuilder();
471 16
        $node = $builder->root('authentication');
472
        $node
1 ignored issue
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Symfony\Component\Config...\Builder\NodeDefinition as the method children() does only exist in the following sub-classes of Symfony\Component\Config...\Builder\NodeDefinition: Symfony\Component\Config...der\ArrayNodeDefinition. 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

  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 parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
473 16
            ->useAttributeAsKey('name')
474 16
            ->prototype('array')
475 16
                ->validate()
476 16
                    ->always()
477 16
                    ->then(function ($config) {
478 5
                        switch ($config['type']) {
479 5
                            case 'basic':
480 4
                                $this->validateAuthenticationType(['username', 'password'], $config, 'basic');
481 4
                                break;
482 2
                            case 'bearer':
483 1
                                $this->validateAuthenticationType(['token'], $config, 'bearer');
484 1
                                break;
485 2
                            case 'service':
486 2
                                $this->validateAuthenticationType(['service'], $config, 'service');
487 1
                                break;
488 1
                            case 'wsse':
489 1
                                $this->validateAuthenticationType(['username', 'password'], $config, 'wsse');
490 1
                                break;
491 4
                        }
492
493 4
                        return $config;
494 16
                    })
495 16
                ->end()
496 16
                ->children()
497 16
                    ->enumNode('type')
498 16
                        ->values(['basic', 'bearer', 'wsse', 'service'])
499 16
                        ->isRequired()
500 16
                        ->cannotBeEmpty()
501 16
                    ->end()
502 16
                    ->scalarNode('username')->end()
503 16
                    ->scalarNode('password')->end()
504 16
                    ->scalarNode('token')->end()
505 16
                    ->scalarNode('service')->end()
506 16
                    ->end()
507 16
                ->end()
508 16
            ->end(); // End authentication plugin
509
510 16
        return $node;
511
    }
512
513
    /**
514
     * Validate that the configuration fragment has the specified keys and none other.
515
     *
516
     * @param array  $expected Fields that must exist
517
     * @param array  $actual   Actual configuration hashmap
518
     * @param string $authName Name of authentication method for error messages
519
     *
520
     * @throws InvalidConfigurationException If $actual does not have exactly the keys specified in $expected (plus 'type')
521
     */
522 5
    private function validateAuthenticationType(array $expected, array $actual, $authName)
523
    {
524 5
        unset($actual['type']);
525 5
        $actual = array_keys($actual);
526 5
        sort($actual);
527 5
        sort($expected);
528
529 5
        if ($expected === $actual) {
530 4
            return;
531
        }
532
533 1
        throw new InvalidConfigurationException(sprintf(
534 1
            'Authentication "%s" requires %s but got %s',
535 1
            $authName,
536 1
            implode(', ', $expected),
537 1
            implode(', ', $actual)
538 1
        ));
539
    }
540
}
541