AbstractQuerySecurity::collectFieldASTsAndDefs() - Code Metrics - Inspection of "Query complexity security" - overblog/GraphQLBundle - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#23)

by Jérémiah

created 2016-04-02 20:29 UTC

AbstractQuerySecurity::collectFieldASTsAndDefs() C

↳ Parent: AbstractQuerySecurity

Complexity

Conditions	18
Paths	72

Size

Total Lines	67
Code Lines	48

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	55
CRAP Score	18

Importance

Changes	2
Bugs	0	Features	1

Metric	Value
c	2
b	0
f	1
dl	0
loc	67
ccs	55
cts	55
cp	1
rs	5.7575
cc	18
eloc	48
nc	72
nop	5
crap	18

How to fix Long Method Complexity

<?php

/*
 * This file is part of the OverblogGraphQLBundle package.
 *
 * (c) Overblog <http://github.com/overblog/>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

namespace Overblog\GraphQLBundle\Request\Validator\Rule;

use GraphQL\Language\AST\Field;
use GraphQL\Language\AST\FragmentDefinition;
use GraphQL\Language\AST\FragmentSpread;
use GraphQL\Language\AST\InlineFragment;
use GraphQL\Language\AST\Node;
use GraphQL\Language\AST\SelectionSet;
use GraphQL\Type\Definition\Type;
use GraphQL\Type\Introspection;
use GraphQL\Utils\TypeInfo;
use GraphQL\Validator\ValidationContext;

abstract class AbstractQuerySecurity
{
    const DISABLED = 0;

    /** @var FragmentDefinition[] */
    private $fragments = [];

    /**
     * @return \GraphQL\Language\AST\FragmentDefinition[]
     */
    protected function getFragments()
    {
        return $this->fragments;
    }

    /**
     * check if equal to 0 no check is done. Must be greater or equal to 0.
     *
     * @param $value
     */
    protected static function checkIfGreaterOrEqualToZero($name, $value)
    {
        if ($value < 0) {
            throw new \InvalidArgumentException(sprintf('$%s argument must be greater or equal to 0.', $name));
        }
    }

    protected function gatherFragmentDefinition(ValidationContext $context)
    {
        // Gather all the fragment definition.
        // Importantly this does not include inline fragments.
        $definitions = $context->getDocument()->definitions;
        foreach ($definitions as $node) {
            if ($node instanceof FragmentDefinition) {
                $this->fragments[$node->name->value] = $node;
            }
        }
    }

    protected function getFragment(FragmentSpread $fragmentSpread)
    {
        $spreadName = $fragmentSpread->name->value;
        $fragments = $this->getFragments();

        return isset($fragments[$spreadName]) ? $fragments[$spreadName] : null;
    }

    protected function invokeIfNeeded(ValidationContext $context, array $validators)
    {
        $this->gatherFragmentDefinition($context);

        // is disabled?
        if (!$this->isEnabled()) {
            return [];
        }

        return $validators;
    }

    /**
     * Given a selectionSet, adds all of the fields in that selection to
     * the passed in map of fields, and returns it at the end.
     *
     * Note: This is not the same as execution's collectFields because at static
     * time we do not know what object type will be used, so we unconditionally
     * spread in all fragments.
     *
     * @see GraphQL\Validator\Rules\OverlappingFieldsCanBeMerged
     *
     * @param ValidationContext $context
     * @param Type|null         $parentType
     * @param SelectionSet      $selectionSet
     * @param \ArrayObject      $visitedFragmentNames
     * @param \ArrayObject      $astAndDefs
     *
     * @return \ArrayObject
     */
    protected function collectFieldASTsAndDefs(ValidationContext $context, $parentType, SelectionSet $selectionSet, \ArrayObject $visitedFragmentNames = null, \ArrayObject $astAndDefs = null)
    {
        $_visitedFragmentNames = $visitedFragmentNames ?: new \ArrayObject();
        $_astAndDefs = $astAndDefs ?: new \ArrayObject();

        foreach ($selectionSet->selections as $selection) {
            switch ($selection->kind) {
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
                case Node::FIELD:
                    $fieldName = $selection->name->value;
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
                    $fieldDef = null;
                    if ($parentType && method_exists($parentType, 'getFields')) {
                        $tmp = $parentType->getFields();
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.
    }
}
                        $schemaMetaFieldDef = Introspection::schemaMetaFieldDef();
                        $typeMetaFieldDef = Introspection::typeMetaFieldDef();
                        $typeNameMetaFieldDef = Introspection::typeNameMetaFieldDef();

                        if ($fieldName === $schemaMetaFieldDef->name && $context->getSchema()->getQueryType() === $parentType) {
                            $fieldDef = $schemaMetaFieldDef;
                        } elseif ($fieldName === $typeMetaFieldDef->name && $context->getSchema()->getQueryType() === $parentType) {
                            $fieldDef = $typeMetaFieldDef;
                        } elseif ($fieldName === $typeNameMetaFieldDef->name) {
                            $fieldDef = $typeNameMetaFieldDef;
                        } elseif (isset($tmp[$fieldName])) {
                            $fieldDef = $tmp[$fieldName];
                        }
                    }
                    $responseName = $this->getFieldName($selection);
function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
                    if (!isset($_astAndDefs[$responseName])) {
                        $_astAndDefs[$responseName] = new \ArrayObject();
                    }
                    // create field context
                    $_astAndDefs[$responseName][] = [$selection, $fieldDef];
                    break;
                case Node::INLINE_FRAGMENT:
                    /* @var InlineFragment $inlineFragment */
                    $_astAndDefs = $this->collectFieldASTsAndDefs(
                        $context,
                        TypeInfo::typeFromAST($context->getSchema(), $selection->typeCondition),
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
                        $selection->selectionSet,
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
                        $_visitedFragmentNames,
                        $_astAndDefs
                    );
                    break;
                case Node::FRAGMENT_SPREAD:
                    /* @var FragmentSpread $selection */
                    $fragName = $selection->name->value;

                    if (empty($_visitedFragmentNames[$fragName])) {
                        $_visitedFragmentNames[$fragName] = true;
                        $fragment = $context->getFragment($fragName);

                        if ($fragment) {
                            $_astAndDefs = $this->collectFieldASTsAndDefs(
                                $context,
                                TypeInfo::typeFromAST($context->getSchema(), $fragment->typeCondition),

                                $fragment->selectionSet,
                                $_visitedFragmentNames,
                                $_astAndDefs
                            );
                        }
                    }
                    break;
            }
        }

        return $_astAndDefs;
    }

    protected function getFieldName(Field $node)
    {
        $fieldName = $node->name->value;
        $responseName = $node->alias ? $node->alias->value : $fieldName;

        return $responseName;
    }

    abstract protected function isEnabled();
}


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	82	protected function gatherFragmentDefinition(ValidationContext $context)
53		{
54		// Gather all the fragment definition.
55		// Importantly this does not include inline fragments.
56	82	$definitions = $context->getDocument()->definitions;
57	82	foreach ($definitions as $node) {
58	82	if ($node instanceof FragmentDefinition) {
59	15	$this->fragments[$node->name->value] = $node;
60	15	}
61	82	}
62	82	}
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	82	$this->gatherFragmentDefinition($context);
75
76		// is disabled?
77	82	if (!$this->isEnabled()) {
78	46	return [];
79		}
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 2016-04-02 20:15 UTC by Report Bug Copy Issue Report 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 Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } 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	12	$fieldName = $selection->name->value;
		0 ignored issues – show Bug introduced 2016-04-02 20:15 UTC by Report Bug Copy Issue Report 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 Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
111	12	$fieldDef = null;
112	12	if ($parentType && method_exists($parentType, 'getFields')) {
113	12	$tmp = $parentType->getFields();
		0 ignored issues – show Bug introduced 2016-04-02 20:15 UTC by Report Bug Copy Issue Report 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 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...
114	12	$schemaMetaFieldDef = Introspection::schemaMetaFieldDef();
115	12	$typeMetaFieldDef = Introspection::typeMetaFieldDef();
116	12	$typeNameMetaFieldDef = Introspection::typeNameMetaFieldDef();
117
118	12	if ($fieldName === $schemaMetaFieldDef->name && $context->getSchema()->getQueryType() === $parentType) {
119	1	$fieldDef = $schemaMetaFieldDef;
120	12	} elseif ($fieldName === $typeMetaFieldDef->name && $context->getSchema()->getQueryType() === $parentType) {
121	1	$fieldDef = $typeMetaFieldDef;
122	12	} elseif ($fieldName === $typeNameMetaFieldDef->name) {
123	1	$fieldDef = $typeNameMetaFieldDef;
124	12	} elseif (isset($tmp[$fieldName])) {
125	12	$fieldDef = $tmp[$fieldName];
126	12	}
127	12	}
128	12	$responseName = $this->getFieldName($selection);
		0 ignored issues – show Documentation introduced 2016-04-02 20:15 UTC by Report Bug Copy Issue Report `$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	12	if (!isset($_astAndDefs[$responseName])) {
130	12	$_astAndDefs[$responseName] = new \ArrayObject();
131	12	}
132		// create field context
133	12	$_astAndDefs[$responseName][] = [$selection, $fieldDef];
134	12	break;
135	3	case Node::INLINE_FRAGMENT:
136		/* @var InlineFragment $inlineFragment */
137	1	$_astAndDefs = $this->collectFieldASTsAndDefs(
138	1	$context,
139	1	TypeInfo::typeFromAST($context->getSchema(), $selection->typeCondition),
		0 ignored issues – show Bug introduced 2016-04-02 20:15 UTC by Report Bug Copy Issue Report 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 Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history... Bug introduced 2016-04-02 20:15 UTC by Report Bug Copy Issue Report 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	1	$selection->selectionSet,
		0 ignored issues – show Bug introduced 2016-04-02 20:15 UTC by Report Bug Copy Issue Report 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 Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
141	1	$_visitedFragmentNames,
142		$_astAndDefs
143	1	);
144	1	break;
145	2	case Node::FRAGMENT_SPREAD:
146		/* @var FragmentSpread $selection */
147	2	$fragName = $selection->name->value;
148
149	2	if (empty($_visitedFragmentNames[$fragName])) {
150	2	$_visitedFragmentNames[$fragName] = true;
151	2	$fragment = $context->getFragment($fragName);
152
153	2	if ($fragment) {
154	2	$_astAndDefs = $this->collectFieldASTsAndDefs(
155	2	$context,
156	2	TypeInfo::typeFromAST($context->getSchema(), $fragment->typeCondition),
		0 ignored issues – show Bug introduced 2016-04-02 20:15 UTC by Report Bug Copy Issue Report 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	2	$fragment->selectionSet,
158	2	$_visitedFragmentNames,
159		$_astAndDefs
160	2	);
161	2	}
162	2	}
163	2	break;
164	12	}
165	12	}
166
167	12	return $_astAndDefs;
168		}
169
170	12	protected function getFieldName(Field $node)
171		{
172	12	$fieldName = $node->name->value;
173	12	$responseName = $node->alias ? $node->alias->value : $fieldName;
174
175	12	return $responseName;
176		}
177
178		abstract protected function isEnabled();
179		}
180

overblog / GraphQLBundle

Scrutinizer GitHub App not installed

Pull Request — master (#23)

AbstractQuerySecurity::collectFieldASTsAndDefs() C

Complexity

Size

Duplication

Code Coverage

Importance

How to fix Long Method Complexity

Long Method

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like