Scrutinizer GitHub App not installed

We could not synchronize checks via GitHub's checks API since Scrutinizer's GitHub App is not installed for this repository.

Install GitHub App

Completed
Pull Request — master (#23)
by Jérémiah
12:19
created

AbstractQuerySecurity::collectFieldASTsAndDefs()   C

Complexity

Conditions 18
Paths 72

Size

Total Lines 67
Code Lines 48

Duplication

Lines 0
Ratio 0 %

Importance

Changes 2
Bugs 0 Features 1
Metric Value
c 2
b 0
f 1
dl 0
loc 67
rs 5.7575
cc 18
eloc 48
nc 72
nop 5

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
/*
4
 * This file is part of the OverblogGraphQLBundle package.
5
 *
6
 * (c) Overblog <http://github.com/overblog/>
7
 *
8
 * For the full copyright and license information, please view the LICENSE
9
 * file that was distributed with this source code.
10
 */
11
12
namespace Overblog\GraphQLBundle\Request\Validator\Rule;
13
14
use GraphQL\Language\AST\Field;
15
use GraphQL\Language\AST\FragmentDefinition;
16
use GraphQL\Language\AST\FragmentSpread;
17
use GraphQL\Language\AST\InlineFragment;
18
use GraphQL\Language\AST\Node;
19
use GraphQL\Language\AST\SelectionSet;
20
use GraphQL\Type\Definition\Type;
21
use GraphQL\Type\Introspection;
22
use GraphQL\Utils\TypeInfo;
23
use GraphQL\Validator\ValidationContext;
24
25
abstract class AbstractQuerySecurity
26
{
27
    const DISABLED = 0;
28
29
    /** @var FragmentDefinition[] */
30
    private $fragments = [];
31
32
    /**
33
     * @return \GraphQL\Language\AST\FragmentDefinition[]
34
     */
35
    protected function getFragments()
36
    {
37
        return $this->fragments;
38
    }
39
40
    /**
41
     * check if equal to 0 no check is done. Must be greater or equal to 0.
42
     *
43
     * @param $value
44
     */
45
    protected static function checkIfGreaterOrEqualToZero($name, $value)
46
    {
47
        if ($value < 0) {
48
            throw new \InvalidArgumentException(sprintf('$%s argument must be greater or equal to 0.', $name));
49
        }
50
    }
51
52
    protected function gatherFragmentDefinition(ValidationContext $context)
53
    {
54
        // Gather all the fragment definition.
55
        // Importantly this does not include inline fragments.
56
        $definitions = $context->getDocument()->definitions;
57
        foreach ($definitions as $node) {
58
            if ($node instanceof FragmentDefinition) {
59
                $this->fragments[$node->name->value] = $node;
60
            }
61
        }
62
    }
63
64
    protected function getFragment(FragmentSpread $fragmentSpread)
65
    {
66
        $spreadName = $fragmentSpread->name->value;
67
        $fragments = $this->getFragments();
68
69
        return isset($fragments[$spreadName]) ? $fragments[$spreadName] : null;
70
    }
71
72
    protected function invokeIfNeeded(ValidationContext $context, array $validators)
73
    {
74
        $this->gatherFragmentDefinition($context);
75
76
        // is disabled?
77
        if (!$this->isEnabled()) {
78
            return [];
79
        }
80
81
        return $validators;
82
    }
83
84
    /**
85
     * Given a selectionSet, adds all of the fields in that selection to
86
     * the passed in map of fields, and returns it at the end.
87
     *
88
     * Note: This is not the same as execution's collectFields because at static
89
     * time we do not know what object type will be used, so we unconditionally
90
     * spread in all fragments.
91
     *
92
     * @see GraphQL\Validator\Rules\OverlappingFieldsCanBeMerged
93
     *
94
     * @param ValidationContext $context
95
     * @param Type|null         $parentType
96
     * @param SelectionSet      $selectionSet
97
     * @param \ArrayObject      $visitedFragmentNames
98
     * @param \ArrayObject      $astAndDefs
99
     *
100
     * @return \ArrayObject
101
     */
102
    protected function collectFieldASTsAndDefs(ValidationContext $context, $parentType, SelectionSet $selectionSet, \ArrayObject $visitedFragmentNames = null, \ArrayObject $astAndDefs = null)
103
    {
104
        $_visitedFragmentNames = $visitedFragmentNames ?: new \ArrayObject();
105
        $_astAndDefs = $astAndDefs ?: new \ArrayObject();
106
107
        foreach ($selectionSet->selections as $selection) {
108
            switch ($selection->kind) {
0 ignored issues
show
Bug introduced by
Accessing kind on the interface GraphQL\Language\AST\Selection suggest that you code against a concrete implementation. How about adding an instanceof check?

If you access a property on an interface, you most likely code against a concrete implementation of the interface.

Available Fixes

  1. Adding an additional type check:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeInterface $object) {
        if ($object instanceof SomeClass) {
            $a = $object->a;
        }
    }
    
  2. Changing the type hint:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeClass $object) {
        $a = $object->a;
    }
    
Loading history...
109
                case Node::FIELD:
110
                    $fieldName = $selection->name->value;
0 ignored issues
show
Bug introduced by
Accessing name on the interface GraphQL\Language\AST\Selection suggest that you code against a concrete implementation. How about adding an instanceof check?

If you access a property on an interface, you most likely code against a concrete implementation of the interface.

Available Fixes

  1. Adding an additional type check:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeInterface $object) {
        if ($object instanceof SomeClass) {
            $a = $object->a;
        }
    }
    
  2. Changing the type hint:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeClass $object) {
        $a = $object->a;
    }
    
Loading history...
111
                    $fieldDef = null;
112
                    if ($parentType && method_exists($parentType, 'getFields')) {
113
                        $tmp = $parentType->getFields();
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class GraphQL\Type\Definition\Type as the method getFields() does only exist in the following sub-classes of GraphQL\Type\Definition\Type: GraphQL\Type\Definition\InputObjectType, GraphQL\Type\Definition\InterfaceType, GraphQL\Type\Definition\ObjectType, Overblog\GraphQLBundle\Definition\InputObjectType, Overblog\GraphQLBundle\Definition\InterfaceType, Overblog\GraphQLBundle\Definition\ObjectType, Overblog\GraphQLBundle\R...nnection\ConnectionType, Overblog\GraphQLBundle\Relay\Connection\EdgeType, Overblog\GraphQLBundle\R...Connection\PageInfoType, Overblog\GraphQLBundle\Relay\Mutation\InputType, Overblog\GraphQLBundle\Relay\Mutation\PayloadType, Overblog\GraphQLBundle\R...\Node\NodeInterfaceType. 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...
114
                        $schemaMetaFieldDef = Introspection::schemaMetaFieldDef();
115
                        $typeMetaFieldDef = Introspection::typeMetaFieldDef();
116
                        $typeNameMetaFieldDef = Introspection::typeNameMetaFieldDef();
117
118
                        if ($fieldName === $schemaMetaFieldDef->name && $context->getSchema()->getQueryType() === $parentType) {
119
                            $fieldDef = $schemaMetaFieldDef;
120
                        } elseif ($fieldName === $typeMetaFieldDef->name && $context->getSchema()->getQueryType() === $parentType) {
121
                            $fieldDef = $typeMetaFieldDef;
122
                        } elseif ($fieldName === $typeNameMetaFieldDef->name) {
123
                            $fieldDef = $typeNameMetaFieldDef;
124
                        } elseif (isset($tmp[$fieldName])) {
125
                            $fieldDef = $tmp[$fieldName];
126
                        }
127
                    }
128
                    $responseName = $this->getFieldName($selection);
0 ignored issues
show
Documentation introduced by
$selection is of type object<GraphQL\Language\AST\Selection>, but the function expects a object<GraphQL\Language\AST\Field>.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
129
                    if (!isset($_astAndDefs[$responseName])) {
130
                        $_astAndDefs[$responseName] = new \ArrayObject();
131
                    }
132
                    // create field context
133
                    $_astAndDefs[$responseName][] = [$selection, $fieldDef];
134
                    break;
135
                case Node::INLINE_FRAGMENT:
136
                    /* @var InlineFragment $inlineFragment */
137
                    $_astAndDefs = $this->collectFieldASTsAndDefs(
138
                        $context,
139
                        TypeInfo::typeFromAST($context->getSchema(), $selection->typeCondition),
0 ignored issues
show
Bug introduced by
Accessing typeCondition on the interface GraphQL\Language\AST\Selection suggest that you code against a concrete implementation. How about adding an instanceof check?

If you access a property on an interface, you most likely code against a concrete implementation of the interface.

Available Fixes

  1. Adding an additional type check:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeInterface $object) {
        if ($object instanceof SomeClass) {
            $a = $object->a;
        }
    }
    
  2. Changing the type hint:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeClass $object) {
        $a = $object->a;
    }
    
Loading history...
Bug introduced by
It seems like \GraphQL\Utils\TypeInfo:...lection->typeCondition) targeting GraphQL\Utils\TypeInfo::typeFromAST() can also be of type object<GraphQL\Language\AST\Name>; however, Overblog\GraphQLBundle\R...llectFieldASTsAndDefs() does only seem to accept object<GraphQL\Type\Definition\Type>|null, maybe add an additional type check?

This check looks at variables that 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...
140
                        $selection->selectionSet,
0 ignored issues
show
Bug introduced by
Accessing selectionSet on the interface GraphQL\Language\AST\Selection suggest that you code against a concrete implementation. How about adding an instanceof check?

If you access a property on an interface, you most likely code against a concrete implementation of the interface.

Available Fixes

  1. Adding an additional type check:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeInterface $object) {
        if ($object instanceof SomeClass) {
            $a = $object->a;
        }
    }
    
  2. Changing the type hint:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeClass $object) {
        $a = $object->a;
    }
    
Loading history...
141
                        $_visitedFragmentNames,
142
                        $_astAndDefs
143
                    );
144
                    break;
145
                case Node::FRAGMENT_SPREAD:
146
                    /* @var FragmentSpread $selection */
147
                    $fragName = $selection->name->value;
148
149
                    if (empty($_visitedFragmentNames[$fragName])) {
150
                        $_visitedFragmentNames[$fragName] = true;
151
                        $fragment = $context->getFragment($fragName);
152
153
                        if ($fragment) {
154
                            $_astAndDefs = $this->collectFieldASTsAndDefs(
155
                                $context,
156
                                TypeInfo::typeFromAST($context->getSchema(), $fragment->typeCondition),
0 ignored issues
show
Bug introduced by
It seems like \GraphQL\Utils\TypeInfo:...ragment->typeCondition) targeting GraphQL\Utils\TypeInfo::typeFromAST() can also be of type object<GraphQL\Language\AST\Name>; however, Overblog\GraphQLBundle\R...llectFieldASTsAndDefs() does only seem to accept object<GraphQL\Type\Definition\Type>|null, maybe add an additional type check?

This check looks at variables that 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...
157
                                $fragment->selectionSet,
158
                                $_visitedFragmentNames,
159
                                $_astAndDefs
160
                            );
161
                        }
162
                    }
163
                    break;
164
            }
165
        }
166
167
        return $_astAndDefs;
168
    }
169
170
    protected function getFieldName(Field $node)
171
    {
172
        $fieldName = $node->name->value;
173
        $responseName = $node->alias ? $node->alias->value : $fieldName;
174
175
        return $responseName;
176
    }
177
178
    abstract protected function isEnabled();
179
}
180