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
13:51 queued 05:06
created

AbstractQuerySecurity::getFieldName()   A

Complexity

Conditions 2
Paths 2

Size

Total Lines 7
Code Lines 4

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 4
CRAP Score 2

Importance

Changes 2
Bugs 0 Features 1
Metric Value
c 2
b 0
f 1
dl 0
loc 7
ccs 4
cts 4
cp 1
rs 9.4285
cc 2
eloc 4
nc 2
nop 1
crap 2
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 13
    protected function getFragments()
36
    {
37 13
        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 84
    protected static function checkIfGreaterOrEqualToZero($name, $value)
46
    {
47 84
        if ($value < 0) {
48 2
            throw new \InvalidArgumentException(sprintf('$%s argument must be greater or equal to 0.', $name));
49
        }
50 82
    }
51
52 45
    protected function gatherFragmentDefinition(ValidationContext $context)
53
    {
54
        // Gather all the fragment definition.
55
        // Importantly this does not include inline fragments.
56 45
        $definitions = $context->getDocument()->definitions;
57 45
        foreach ($definitions as $node) {
58 45
            if ($node instanceof FragmentDefinition) {
59 13
                $this->fragments[$node->name->value] = $node;
60 13
            }
61 45
        }
62 45
    }
63
64 13
    protected function getFragment(FragmentSpread $fragmentSpread)
65
    {
66 13
        $spreadName = $fragmentSpread->name->value;
67 13
        $fragments = $this->getFragments();
68
69 13
        return isset($fragments[$spreadName]) ? $fragments[$spreadName] : null;
70
    }
71
72 82
    protected function invokeIfNeeded(ValidationContext $context, array $validators)
73
    {
74
        // is disabled?
75 82
        if (!$this->isEnabled()) {
76 46
            return [];
77
        }
78
79 45
        $this->gatherFragmentDefinition($context);
80
81 45
        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 12
    protected function collectFieldASTsAndDefs(ValidationContext $context, $parentType, SelectionSet $selectionSet, \ArrayObject $visitedFragmentNames = null, \ArrayObject $astAndDefs = null)
103
    {
104 12
        $_visitedFragmentNames = $visitedFragmentNames ?: new \ArrayObject();
105 12
        $_astAndDefs = $astAndDefs ?: new \ArrayObject();
106
107 12
        foreach ($selectionSet->selections as $selection) {
108 12
            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 12
                case Node::FIELD:
110
                    /* @var Field $selection */
111 12
                    $fieldName = $selection->name->value;
112 12
                    $fieldDef = null;
113 12
                    if ($parentType && method_exists($parentType, 'getFields')) {
114 12
                        $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...
115 12
                        $schemaMetaFieldDef = Introspection::schemaMetaFieldDef();
116 12
                        $typeMetaFieldDef = Introspection::typeMetaFieldDef();
117 12
                        $typeNameMetaFieldDef = Introspection::typeNameMetaFieldDef();
118
119 12
                        if ($fieldName === $schemaMetaFieldDef->name && $context->getSchema()->getQueryType() === $parentType) {
120 1
                            $fieldDef = $schemaMetaFieldDef;
121 12
                        } elseif ($fieldName === $typeMetaFieldDef->name && $context->getSchema()->getQueryType() === $parentType) {
122 1
                            $fieldDef = $typeMetaFieldDef;
123 12
                        } elseif ($fieldName === $typeNameMetaFieldDef->name) {
124 1
                            $fieldDef = $typeNameMetaFieldDef;
125 12
                        } elseif (isset($tmp[$fieldName])) {
126 12
                            $fieldDef = $tmp[$fieldName];
127 12
                        }
128 12
                    }
129 12
                    $responseName = $this->getFieldName($selection);
130 12
                    if (!isset($_astAndDefs[$responseName])) {
131 12
                        $_astAndDefs[$responseName] = new \ArrayObject();
132 12
                    }
133
                    // create field context
134 12
                    $_astAndDefs[$responseName][] = [$selection, $fieldDef];
135 12
                    break;
136 3
                case Node::INLINE_FRAGMENT:
137
                    /* @var InlineFragment $selection */
138 1
                    $_astAndDefs = $this->collectFieldASTsAndDefs(
139 1
                        $context,
140 1
                        TypeInfo::typeFromAST($context->getSchema(), $selection->typeCondition),
0 ignored issues
show
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...
141 1
                        $selection->selectionSet,
142 1
                        $_visitedFragmentNames,
143
                        $_astAndDefs
144 1
                    );
145 1
                    break;
146 2
                case Node::FRAGMENT_SPREAD:
147
                    /* @var FragmentSpread $selection */
148 2
                    $fragName = $selection->name->value;
149
150 2
                    if (empty($_visitedFragmentNames[$fragName])) {
151 2
                        $_visitedFragmentNames[$fragName] = true;
152 2
                        $fragment = $context->getFragment($fragName);
153
154 2
                        if ($fragment) {
155 2
                            $_astAndDefs = $this->collectFieldASTsAndDefs(
156 2
                                $context,
157 2
                                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...
158 2
                                $fragment->selectionSet,
159 2
                                $_visitedFragmentNames,
160
                                $_astAndDefs
161 2
                            );
162 2
                        }
163 2
                    }
164 2
                    break;
165 12
            }
166 12
        }
167
168 12
        return $_astAndDefs;
169
    }
170
171 12
    protected function getFieldName(Field $node)
172
    {
173 12
        $fieldName = $node->name->value;
174 12
        $responseName = $node->alias ? $node->alias->value : $fieldName;
175
176 12
        return $responseName;
177
    }
178
179
    abstract protected function isEnabled();
180
}
181