Issues in Configuration.php - All Issues - Inspection of "Add support for Symfony 3.0" - regularjack/frontend-bundle - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 9c1f83...0c1135 )

by Paulo Rodrigues

created 2016-02-11 01:43 UTC

DependencyInjection/Configuration.php (3 issues)

Labels

Bug 3

Severity

Minor 3

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

<?php

namespace Rj\FrontendBundle\DependencyInjection;

use Rj\FrontendBundle\Util\Util;
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
use Symfony\Component\Config\Definition\ConfigurationInterface;

class Configuration implements ConfigurationInterface
{
    const DEFAULT_PREFIX = 'assets';

    private $kernelRootDir;

    public function __construct($kernelRootDir)
    {
        $this->kernelRootDir = $kernelRootDir;
    }

    /**
     * {@inheritdoc}
     */
    public function getConfigTreeBuilder()
    {
        $self = $this;

        return $this->createRoot('rj_frontend', 'array')
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.
    }
}
            ->children()
                ->booleanNode('override_default_package')->defaultTrue()->end()
                ->arrayNode('fallback_patterns')
                    ->prototype('scalar')->end()
                    ->defaultValue(array('.*bundles\/.*'))
                ->end()
                ->append($this->addLivereloadSection())
                ->append($this->addPackagePrefixSection(self::DEFAULT_PREFIX))
                ->append($this->addPackageManifestSection())
                ->arrayNode('packages')
                    ->useAttributeAsKey('name')
                    ->prototype('array')
                        ->children()
                            ->append($this->addPackagePrefixSection())
                            ->append($this->addPackageManifestSection())
                        ->end()
                        ->beforeNormalization()
                            ->ifTrue(function ($config) use ($self) {
                                return $self->mustApplyManifestDefaultPath($config);
                            })
                            ->then(function ($config) use ($self) {
                                return $self->applyManifestDefaultPath($config);
                            })
                        ->end()
                    ->end()
                    ->validate()
                        ->ifTrue(function ($config) {
                            return in_array('default', array_keys($config));
                        })
                        ->thenInvalid("'default' is a reserved package name")
                    ->end()
                ->end()
            ->end()
            ->beforeNormalization()
                ->ifTrue(function ($config) use ($self) {
                    return $self->mustApplyManifestDefaultPath($config);
                })
                ->then(function ($config) use ($self) {
                    return $self->applyManifestDefaultPath($config);
                })
            ->end()
        ->end();
    }

    private function addLivereloadSection()
    {
        return $this->createRoot('livereload')
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.
    }
}
            ->canBeDisabled()
            ->children()
                ->scalarNode('url')
                    ->defaultValue('//localhost:35729/livereload.js')
                ->end()
            ->end()
        ;
    }

    private function addPackagePrefixSection($defaultValue = null)
    {
        $node = $this->createRoot('prefix')
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.
    }
}
            ->prototype('scalar')->end()
            ->defaultValue(array($defaultValue))
            ->cannotBeEmpty()
            ->beforeNormalization()
                ->ifString()
                ->then(function ($v) { return array($v); })
            ->end()
            ->validate()
                ->ifTrue(function ($prefixes) {
                    return Util::containsUrl($prefixes)
                        && Util::containsNotUrl($prefixes);
                })
                ->thenInvalid('Packages cannot have both URL and path prefixes')
            ->end()
            ->validate()
                ->ifTrue(function ($prefixes) {
                    return count($prefixes) > 1
                        && Util::containsNotUrl($prefixes);
                })
                ->thenInvalid('Packages can only have one path prefix')
            ->end()
        ;

        return $defaultValue === null
            ? $node->isRequired()
            : $node
        ;
    }

    private function addPackageManifestSection()
    {
        return $this->createRoot('manifest')
            ->canBeEnabled()
            ->children()
                ->scalarNode('format')
                    ->defaultValue('json')
                    ->validate()
                        ->ifNotInArray(array('json'))
                        ->thenInvalid('For the moment only JSON manifest files are supported')
                    ->end()
                ->end()
                ->scalarNode('path')->isRequired()->end()
                ->scalarNode('root_key')->defaultNull()->end()
            ->end()
            ->beforeNormalization()
                ->ifString()
                ->then(function ($v) { return array('enabled' => true, 'path' => $v); })
            ->end()
        ;
    }

    /**
     * Returns true if the manifest's path has not been defined AND:
     *  - a prefix has not been defined
     *  - OR if a prefix has been defined, it's not a URL
     *
     * Note that the manifest's configuration can be a string, in which case it
     * represents the path to the manifest file.
     *
     * This method is public because of the inability to use $this in closures
     * in PHP 5.3.
     *
     * @param  array   $config
     * @return boolean
     */
    public function mustApplyManifestDefaultPath($config)
    {
        return isset($config['manifest']) &&
            !is_string($config['manifest']) &&
            !isset($config['manifest']['path']) &&
            (!isset($config['prefix']) || !Util::containsUrl($config['prefix']))
        ;
    }

    /**
     * Apply a default manifest path computed from the defined prefix.
     *
     * After calling this method, the manifest's path will be
     * %kernel.root_dir%/../web/$prefix/manifest.json, where $prefix is the
     * configured prefix.
     *
     * Note that this method is used for both the default package's config and
     * for each custom package's config.
     *
     * This method is public because of the inability to use $this in closures
     * in PHP 5.3
     *
     * @param  array $config
     * @return array
     */
    public function applyManifestDefaultPath($config)
    {
        $prefix = isset($config['prefix']) ? $config['prefix'] : self::DEFAULT_PREFIX;

        if (is_array($prefix)) {
            $prefix = $prefix[0];
        }

        if (!is_array($config['manifest'])) {
            $config['manifest'] = array('enabled' => true);
        }

        $config['manifest']['path'] = implode('/', array(
            $this->kernelRootDir,
            '..',
            'web',
            $prefix,
            'manifest.json',
        ));

        return $config;
    }

    private function createRoot($root, $type = null)
    {
        $treeBuilder = new TreeBuilder();

        if ($type !== null) {
            return $treeBuilder->root($root, $type);
        }

        return $treeBuilder->root($root);
    }
}


Push — master ( 9c1f83...0c1135 )

DependencyInjection/Configuration.php (3 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Available Fixes

Available Fixes

Available Fixes

1		<?php
2
3		namespace Rj\FrontendBundle\DependencyInjection;
4
5		use Rj\FrontendBundle\Util\Util;
6		use Symfony\Component\Config\Definition\Builder\TreeBuilder;
7		use Symfony\Component\Config\Definition\ConfigurationInterface;
8
9		class Configuration implements ConfigurationInterface
10		{
11		const DEFAULT_PREFIX = 'assets';
12
13		private $kernelRootDir;
14
15	71	public function __construct($kernelRootDir)
16		{
17	71	$this->kernelRootDir = $kernelRootDir;
18	71	}
19
20		/**
21		* {@inheritdoc}
22		*/
23	71	public function getConfigTreeBuilder()
24		{
25	71	$self = $this;
26
27	71	return $this->createRoot('rj_frontend', 'array')
		0 ignored issues – show Bug introduced 2016-01-01 19:29 UTC by Report Bug Copy Issue Report Show Similar Issues like this 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 Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
28	71	->children()
29	71	->booleanNode('override_default_package')->defaultTrue()->end()
30	71	->arrayNode('fallback_patterns')
31	71	->prototype('scalar')->end()
32	71	->defaultValue(array('.bundles\/.'))
33	71	->end()
34	71	->append($this->addLivereloadSection())
35	71	->append($this->addPackagePrefixSection(self::DEFAULT_PREFIX))
36	71	->append($this->addPackageManifestSection())
37	71	->arrayNode('packages')
38	71	->useAttributeAsKey('name')
39	71	->prototype('array')
40	71	->children()
41	71	->append($this->addPackagePrefixSection())
42	71	->append($this->addPackageManifestSection())
43	71	->end()
44	71	->beforeNormalization()
45		->ifTrue(function ($config) use ($self) {
46	38	return $self->mustApplyManifestDefaultPath($config);
47	71	})
48		->then(function ($config) use ($self) {
49	2	return $self->applyManifestDefaultPath($config);
50	71	})
51	71	->end()
52	71	->end()
53	71	->validate()
54		->ifTrue(function ($config) {
55	32	return in_array('default', array_keys($config));
56	71	})
57	71	->thenInvalid("'default' is a reserved package name")
58	71	->end()
59	71	->end()
60	71	->end()
61	71	->beforeNormalization()
62		->ifTrue(function ($config) use ($self) {
63	71	return $self->mustApplyManifestDefaultPath($config);
64	71	})
65		->then(function ($config) use ($self) {
66	6	return $self->applyManifestDefaultPath($config);
67	71	})
68	71	->end()
69	71	->end();
70		}
71
72	71	private function addLivereloadSection()
73		{
74	71	return $this->createRoot('livereload')
		0 ignored issues – show Bug introduced 2016-01-01 19:29 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a specific sub-type and not the parent class `Symfony\Component\Config...\Builder\NodeDefinition` as the method `canBeDisabled()` 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 Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
75	71	->canBeDisabled()
76	71	->children()
77	71	->scalarNode('url')
78	71	->defaultValue('//localhost:35729/livereload.js')
79	71	->end()
80	71	->end()
81		;
82		}
83
84	71	private function addPackagePrefixSection($defaultValue = null)
85		{
86	71	$node = $this->createRoot('prefix')
		0 ignored issues – show Bug introduced 2016-01-01 19:29 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a specific sub-type and not the parent class `Symfony\Component\Config...\Builder\NodeDefinition` as the method `prototype()` 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 Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
87	71	->prototype('scalar')->end()
88	71	->defaultValue(array($defaultValue))
89	71	->cannotBeEmpty()
90	71	->beforeNormalization()
91	71	->ifString()
92		->then(function ($v) { return array($v); })
93	71	->end()
94	71	->validate()
95		->ifTrue(function ($prefixes) {
96	46	return Util::containsUrl($prefixes)
97	46	&& Util::containsNotUrl($prefixes);
98	71	})
99	71	->thenInvalid('Packages cannot have both URL and path prefixes')
100	71	->end()
101	71	->validate()
102		->ifTrue(function ($prefixes) {
103	44	return count($prefixes) > 1
104	44	&& Util::containsNotUrl($prefixes);
105	71	})
106	71	->thenInvalid('Packages can only have one path prefix')
107	71	->end()
108		;
109
110	71	return $defaultValue === null
111	71	? $node->isRequired()
112	71	: $node
113		;
114		}
115
116	71	private function addPackageManifestSection()
117		{
118	71	return $this->createRoot('manifest')
119	71	->canBeEnabled()
120	71	->children()
121	71	->scalarNode('format')
122	71	->defaultValue('json')
123	71	->validate()
124	71	->ifNotInArray(array('json'))
125	71	->thenInvalid('For the moment only JSON manifest files are supported')
126	71	->end()
127	71	->end()
128	71	->scalarNode('path')->isRequired()->end()
129	71	->scalarNode('root_key')->defaultNull()->end()
130	71	->end()
131	71	->beforeNormalization()
132	71	->ifString()
133		->then(function ($v) { return array('enabled' => true, 'path' => $v); })
134	71	->end()
135		;
136		}
137
138		/**
139		* Returns true if the manifest's path has not been defined AND:
140		* - a prefix has not been defined
141		* - OR if a prefix has been defined, it's not a URL
142		*
143		* Note that the manifest's configuration can be a string, in which case it
144		* represents the path to the manifest file.
145		*
146		* This method is public because of the inability to use $this in closures
147		* in PHP 5.3.
148		*
149		* @param array $config
150		* @return boolean
151		*/
152	71	public function mustApplyManifestDefaultPath($config)
153		{
154	71	return isset($config['manifest']) &&
155	71	!is_string($config['manifest']) &&
156	71	!isset($config['manifest']['path']) &&
157	71	(!isset($config['prefix']) \|\| !Util::containsUrl($config['prefix']))
158		;
159		}
160
161		/**
162		* Apply a default manifest path computed from the defined prefix.
163		*
164		* After calling this method, the manifest's path will be
165		* %kernel.root_dir%/../web/$prefix/manifest.json, where $prefix is the
166		* configured prefix.
167		*
168		* Note that this method is used for both the default package's config and
169		* for each custom package's config.
170		*
171		* This method is public because of the inability to use $this in closures
172		* in PHP 5.3
173		*
174		* @param array $config
175		* @return array
176		*/
177	8	public function applyManifestDefaultPath($config)
178		{
179	8	$prefix = isset($config['prefix']) ? $config['prefix'] : self::DEFAULT_PREFIX;
180
181	8	if (is_array($prefix)) {
182		$prefix = $prefix[0];
183		}
184
185	8	if (!is_array($config['manifest'])) {
186	6	$config['manifest'] = array('enabled' => true);
187		}
188
189	8	$config['manifest']['path'] = implode('/', array(
190	8	$this->kernelRootDir,
191	8	'..',
192	8	'web',
193	8	$prefix,
194	8	'manifest.json',
195		));
196
197	8	return $config;
198		}
199
200	71	private function createRoot($root, $type = null)
201		{
202	71	$treeBuilder = new TreeBuilder();
203
204	71	if ($type !== null) {
205	71	return $treeBuilder->root($root, $type);
206		}
207
208	71	return $treeBuilder->root($root);
209		}
210		}
211

regularjack / frontend-bundle

Push — master ( 9c1f83...0c1135 )

DependencyInjection/Configuration.php (3 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Available Fixes

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like