Frontend::collect() - Code Metrics - Inspection of "Improve opcode emitting logic and add deferred inv..." - railt/sdl - Measure and Improve Code Quality continuously with Scrutinizer

Test Failed

Push — master ( e081e8...a6938b )

by Kirill

created 2018-08-28 00:15 UTC

Frontend::collect() A

↳ Parent: Frontend

Complexity

Conditions	5
Paths	9

Size

Total Lines

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
cc	5
nc	9
nop	2
dl	0
loc	34
rs	9.0648
c	0
b	0
f	0

<?php
/**
 * This file is part of Railt package.
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */
declare(strict_types=1);

namespace Railt\SDL\Frontend;

use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;
use Psr\Log\LoggerInterface;
use Railt\Io\Readable;
use Railt\Parser\Ast\NodeInterface;
use Railt\Parser\Ast\RuleInterface;
use Railt\Parser\Exception\UnexpectedTokenException;
use Railt\Parser\Exception\UnrecognizedTokenException;
use Railt\SDL\Exception\SyntaxException;
use Railt\SDL\Frontend\AST\ProvidesOpcode;
use Railt\SDL\Frontend\IR\Collection;
use Railt\SDL\Frontend\IR\Deferred;
use Railt\SDL\Frontend\IR\Opcode\OpenOpcode;
use Railt\SDL\Frontend\IR\OpcodeInterface;
use Railt\SDL\Frontend\IR\Prototype;
use Railt\SDL\Frontend\IR\UnmountedOpcodeInterface;

/**
 * Class Frontend
 */
class Frontend implements LoggerAwareInterface
{
    use LoggerAwareTrait;

    /**
     * @var Parser
     */
    private $parser;

    /**
     * Frontend constructor.
     */
    public function __construct()
    {
        $this->parser = new Parser();
    }

    /**
     * @param Readable $file
     * @return iterable|OpcodeInterface[]
     * @throws SyntaxException
     */
    public function load(Readable $file): iterable
    {
        $context = new Context();

        return $this->collect($file, $context);
    }

    /**
     * @param Readable $file
     * @param Context $context
     * @return Collection|OpcodeInterface[]
     * @throws SyntaxException
     */
    private function collect(Readable $file, Context $context): Collection
    {
        // Create a container in which all the opcodes are stored.
        $collection = new Collection($context);

        $collection->add(new OpenOpcode($file), $file);

        // We start bypassing and add each element to the collection.
        $iterator = $this->bypass($this->parse($file), $context);

        while ($iterator->valid()) {
            [$ast, $result] = [$iterator->key(), $iterator->current()];
function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

            // If an unmounted opcode (prototype) is returned,
            // then attach it and return it back.
            if ($result instanceof OpcodeInterface) {
                $result = $collection->add($result, $file, $ast->getOffset());
            }

            // If the result is callable/invocable, then we create
            // a pending (deferred) execution element.
            if (\is_callable($result)) {
                $result = new Deferred($ast, $result);
            }

            $iterator->send($result);

            if ($this->logger) {
                $this->log($result);
            }
        }

        return $collection;
    }

    /**
     * @param mixed $value
     * @return void
     */
    private function log($value): void
    {
        if ($value instanceof OpcodeInterface) {
            $this->logger->debug((string)$value);
        } else {
            $this->logger->info(\gettype($value));
        }
    }

    /**
     * A method for recursively traversing all rules of an Abstract
     * Syntax Tree to obtain all the opcodes that the tree provides.
     *
     * @param NodeInterface|RuleInterface $node
     * @param Context $context
     * @return iterable|OpcodeInterface[]|\Generator
     */
    private function bypass(NodeInterface $node, Context $context): \Generator
    {
        foreach ($node->getChildren() as $child) {
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements 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.
    }
}
            /** @var Deferred[] $deferred */
            $deferred = [];

            $current = $context->current();

            // Is AST Rule provides opcodes list?
            if ($child instanceof ProvidesOpcode) {
                $iterator = $this->extract($child, $child->getOpcodes($context));
function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);

                yield from \iterator_reverse_each($iterator, function($response) use (&$deferred) {
function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
                    // In the event that the parent sends a deferred callback
                    // back, we memorize the reference to him in order
                    // to fulfill in the future.
                    if ($response instanceof Deferred) {
                        $deferred[] = $response;
                    }
                });
            }

            // Is the AST provides other children?
            if ($child instanceof RuleInterface) {
                yield from $this->bypass($child, $context);
            }

            // Execute pending elements before closing the context.
            foreach ($deferred as $callable) {
                yield from $this->extract($child, $callable->resolve());
            }

            // Is the context was changed at runtime - close
            // it and restore the previous one.
            if ($current !== $context->current()) {
                $context->close();
            }
        }
    }

    /**
     * Method for unpacking the list of opcodes from the rule.
     *
     * @param RuleInterface $key
     * @param \Generator $iterator
     * @return \Generator|OpcodeInterface[]
     */
    private function extract(RuleInterface $key, \Generator $iterator): \Generator
    {
        while ($iterator->valid()) {
            // Take an AST node
            $node = $this->extractKey($iterator, $key);

            // We return the prototype and get the opcode:
            // ie with reference to the file, line, column, offset, etc.,
            // including the identifier of the opcode inside the collection.
            $opcode = yield $node => $iterator->current();

            // Transfer control back to the AST.
            $iterator->send($opcode);
        }
    }

    /**
     * Make sure that the key is a valid AST rule that contains
     * a link to the position inside the file.
     *
     * @param \Generator $current
     * @param RuleInterface $parent
     * @return RuleInterface
     */
    private function extractKey(\Generator $current, RuleInterface $parent): RuleInterface
    {
        $key = $current->key();

        return $key instanceof RuleInterface ? $key : $parent;
    }

    /**
     * Parse the file using top-down parser and
     * return the Abstract Syntax Tree.
     *
     * @param Readable $file
     * @return RuleInterface
     * @throws SyntaxException
     */
    private function parse(Readable $file): RuleInterface
    {
        try {
            return $this->parser->parse($file);
        } catch (UnexpectedTokenException | UnrecognizedTokenException $e) {
            $error = new SyntaxException($e->getMessage(), $e->getCode());
            $error->throwsIn($file, $e->getLine(), $e->getColumn());

            throw $error;
        }
    }

    /**
     * @param LoggerInterface $logger
     * @return Frontend
     */
    public function setLogger(LoggerInterface $logger): self
    {
        $this->logger = $logger;

        return $this;
    }
}


Push — master ( e081e8...a6938b )

Frontend::collect() A

Complexity

Size

Duplication

Importance

Available Fixes

Available Fixes

1			<?php
2			/**
3			* This file is part of Railt package.
4			*
5			* For the full copyright and license information, please view the LICENSE
6			* file that was distributed with this source code.
7			*/
8			declare(strict_types=1);
9
10			namespace Railt\SDL\Frontend;
11
12			use Psr\Log\LoggerAwareInterface;
13			use Psr\Log\LoggerAwareTrait;
14			use Psr\Log\LoggerInterface;
15			use Railt\Io\Readable;
16			use Railt\Parser\Ast\NodeInterface;
17			use Railt\Parser\Ast\RuleInterface;
18			use Railt\Parser\Exception\UnexpectedTokenException;
19			use Railt\Parser\Exception\UnrecognizedTokenException;
20			use Railt\SDL\Exception\SyntaxException;
21			use Railt\SDL\Frontend\AST\ProvidesOpcode;
22			use Railt\SDL\Frontend\IR\Collection;
23			use Railt\SDL\Frontend\IR\Deferred;
24			use Railt\SDL\Frontend\IR\Opcode\OpenOpcode;
25			use Railt\SDL\Frontend\IR\OpcodeInterface;
26			use Railt\SDL\Frontend\IR\Prototype;
27			use Railt\SDL\Frontend\IR\UnmountedOpcodeInterface;
28
29			/**
30			* Class Frontend
31			*/
32			class Frontend implements LoggerAwareInterface
33			{
34			use LoggerAwareTrait;
35
36			/**
37			* @var Parser
38			*/
39			private $parser;
40
41			/**
42			* Frontend constructor.
43			*/
44			public function __construct()
45			{
46			$this->parser = new Parser();
47			}
48
49			/**
50			* @param Readable $file
51			* @return iterable\|OpcodeInterface[]
52			* @throws SyntaxException
53			*/
54			public function load(Readable $file): iterable
55			{
56			$context = new Context();
57
58			return $this->collect($file, $context);
59			}
60
61			/**
62			* @param Readable $file
63			* @param Context $context
64			* @return Collection\|OpcodeInterface[]
65			* @throws SyntaxException
66			*/
67			private function collect(Readable $file, Context $context): Collection
68			{
69			// Create a container in which all the opcodes are stored.
70			$collection = new Collection($context);
71
72			$collection->add(new OpenOpcode($file), $file);
73
74			// We start bypassing and add each element to the collection.
75			$iterator = $this->bypass($this->parse($file), $context);
76
77			while ($iterator->valid()) {
78			[$ast, $result] = [$iterator->key(), $iterator->current()];
			0 ignored issues – show Bug introduced 2018-08-24 00:23 UTC by Report Bug Copy Issue Report The variable `$ast` does not exist. Did you forget to declare it? This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug. Loading history... Bug introduced 2018-08-28 00:18 UTC by Report Bug Copy Issue Report The variable `$result` does not seem to be defined for all execution paths leading up to this point. If you define a variable conditionally, it can happen that it is not defined for all execution paths. Let’s take a look at an example: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } // $x is potentially undefined here. echo $x; } In the above example, the variable `$x` is defined if you pass “foo” or “bar” as argument for `$a`. However, since the `switch` statement has no default case statement, if you pass any other value, the variable `$x` would be undefined. Available Fixes Check for existence of the variable explicitly: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } if (isset($x)) { // Make sure it's always set. echo $x; } } Define a default value for the variable: function myFunction($a) { $x = ''; // Set a default which gets overridden for certain paths. switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } echo $x; } Add a value for the missing path: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; // We add support for the missing case. default: $x = ''; break; } echo $x; } Loading history...
79
80			// If an unmounted opcode (prototype) is returned,
81			// then attach it and return it back.
82			if ($result instanceof OpcodeInterface) {
83			$result = $collection->add($result, $file, $ast->getOffset());
84			}
85
86			// If the result is callable/invocable, then we create
87			// a pending (deferred) execution element.
88			if (\is_callable($result)) {
89			$result = new Deferred($ast, $result);
90			}
91
92			$iterator->send($result);
93
94			if ($this->logger) {
95			$this->log($result);
96			}
97			}
98
99			return $collection;
100			}
101
102			/**
103			* @param mixed $value
104			* @return void
105			*/
106			private function log($value): void
107			{
108			if ($value instanceof OpcodeInterface) {
109			$this->logger->debug((string)$value);
110			} else {
111			$this->logger->info(\gettype($value));
112			}
113			}
114
115			/**
116			* A method for recursively traversing all rules of an Abstract
117			* Syntax Tree to obtain all the opcodes that the tree provides.
118			*
119			* @param NodeInterface\|RuleInterface $node
120			* @param Context $context
121			* @return iterable\|OpcodeInterface[]\|\Generator
122			*/
123			private function bypass(NodeInterface $node, Context $context): \Generator
124			{
125			foreach ($node->getChildren() as $child) {
			0 ignored issues – show Bug introduced 2018-08-28 00:18 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Railt\Parser\Ast\NodeInterface` as the method `getChildren()` does only exist in the following implementations of said interface: `Railt\Compiler\Grammar\Delegate\IncludeDelegate`, `Railt\Compiler\Grammar\Delegate\RuleDelegate`, `Railt\Compiler\Grammar\Delegate\TokenDelegate`, `Railt\Parser\Ast\Rule`, `Railt\SDL\Frontend\AST\Common\TypeHintNode`, `Railt\SDL\Frontend\AST\Common\TypeNameNode`, `Railt\SDL\Frontend\AST\D...DirectiveDefinitionNode`, `Railt\SDL\Frontend\AST\D...tion\EnumDefinitionNode`, `Railt\SDL\Frontend\AST\D...ion\InputDefinitionNode`, `Railt\SDL\Frontend\AST\D...InterfaceDefinitionNode`, `Railt\SDL\Frontend\AST\D...on\ObjectDefinitionNode`, `Railt\SDL\Frontend\AST\D...on\ScalarDefinitionNode`, `Railt\SDL\Frontend\AST\D...on\SchemaDefinitionNode`, `Railt\SDL\Frontend\AST\D...tion\TypeDefinitionNode`, `Railt\SDL\Frontend\AST\D...ion\UnionDefinitionNode`, `Railt\SDL\Frontend\AST\D...\ArgumentDefinitionNode`, `Railt\SDL\Frontend\AST\D...ndentTypeDefinitionNode`, `Railt\SDL\Frontend\AST\D...EnumValueDefinitionNode`, `Railt\SDL\Frontend\AST\D...ent\FieldDefinitionNode`, `Railt\SDL\Frontend\AST\D...nputFieldDefinitionNode`, `Railt\SDL\Frontend\AST\I...\ArgumentInvocationNode`, `Railt\SDL\Frontend\AST\Invocation\BooleanValueNode`, `Railt\SDL\Frontend\AST\I...ation\ConstantValueNode`, `Railt\SDL\Frontend\AST\I...DirectiveInvocationNode`, `Railt\SDL\Frontend\AST\I...ion\InputInvocationNode`, `Railt\SDL\Frontend\AST\Invocation\ListValueNode`, `Railt\SDL\Frontend\AST\Invocation\NullValueNode`, `Railt\SDL\Frontend\AST\Invocation\NumberValueNode`, `Railt\SDL\Frontend\AST\Invocation\StringValueNode`, `Railt\SDL\Frontend\AST\Invocation\ValueNode`. Let’s take a look at an example: interface User { /** @return string / public function getPassword(); } class MyUser implements 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 implementation 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 interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
126			/** @var Deferred[] $deferred */
127			$deferred = [];
128
129			$current = $context->current();
130
131			// Is AST Rule provides opcodes list?
132			if ($child instanceof ProvidesOpcode) {
133			$iterator = $this->extract($child, $child->getOpcodes($context));
			0 ignored issues – show Documentation introduced 2018-08-28 00:18 UTC by Report Bug Copy Issue Report `$child` is of type `object<Railt\SDL\Frontend\AST\ProvidesOpcode>`, but the function expects a `object<Railt\Parser\Ast\RuleInterface>`. 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... Bug introduced 2018-08-28 00:18 UTC by Report Bug Copy Issue Report It seems like `$child->getOpcodes($context)` targeting `Railt\SDL\Frontend\AST\P...desOpcode::getOpcodes()` can also be of type `array<integer,object<Rai...nd\IR\OpcodeInterface>>`; however, `Railt\SDL\Frontend\Frontend::extract()` does only seem to accept `object<Generator>`, 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...
134
135			yield from \iterator_reverse_each($iterator, function($response) use (&$deferred) {
			0 ignored issues – show Documentation introduced 2018-08-28 00:18 UTC by Report Bug Copy Issue Report `$iterator` is of type `object<Generator>`, but the function expects a `object<iterable>`. 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...
136			// In the event that the parent sends a deferred callback
137			// back, we memorize the reference to him in order
138			// to fulfill in the future.
139			if ($response instanceof Deferred) {
140			$deferred[] = $response;
141			}
142			});
143			}
144
145			// Is the AST provides other children?
146			if ($child instanceof RuleInterface) {
147			yield from $this->bypass($child, $context);
148			}
149
150			// Execute pending elements before closing the context.
151			foreach ($deferred as $callable) {
152			yield from $this->extract($child, $callable->resolve());
153			}
154
155			// Is the context was changed at runtime - close
156			// it and restore the previous one.
157			if ($current !== $context->current()) {
158			$context->close();
159			}
160			}
161			}
162
163			/**
164			* Method for unpacking the list of opcodes from the rule.
165			*
166			* @param RuleInterface $key
167			* @param \Generator $iterator
168			* @return \Generator\|OpcodeInterface[]
169			*/
170			private function extract(RuleInterface $key, \Generator $iterator): \Generator
171			{
172			while ($iterator->valid()) {
173			// Take an AST node
174			$node = $this->extractKey($iterator, $key);
175
176			// We return the prototype and get the opcode:
177			// ie with reference to the file, line, column, offset, etc.,
178			// including the identifier of the opcode inside the collection.
179			$opcode = yield $node => $iterator->current();
180
181			// Transfer control back to the AST.
182			$iterator->send($opcode);
183			}
184			}
185
186			/**
187			* Make sure that the key is a valid AST rule that contains
188			* a link to the position inside the file.
189			*
190			* @param \Generator $current
191			* @param RuleInterface $parent
192			* @return RuleInterface
193			*/
194			private function extractKey(\Generator $current, RuleInterface $parent): RuleInterface
195			{
196			$key = $current->key();
197
198			return $key instanceof RuleInterface ? $key : $parent;
199			}
200
201			/**
202			* Parse the file using top-down parser and
203			* return the Abstract Syntax Tree.
204			*
205			* @param Readable $file
206			* @return RuleInterface
207			* @throws SyntaxException
208			*/
209			private function parse(Readable $file): RuleInterface
210			{
211			try {
212			return $this->parser->parse($file);
213			} catch (UnexpectedTokenException \| UnrecognizedTokenException $e) {
214			$error = new SyntaxException($e->getMessage(), $e->getCode());
215			$error->throwsIn($file, $e->getLine(), $e->getColumn());
216
217			throw $error;
218			}
219			}
220
221			/**
222			* @param LoggerInterface $logger
223			* @return Frontend
224			*/
225			public function setLogger(LoggerInterface $logger): self
226			{
227			$this->logger = $logger;
228
229			return $this;
230			}
231			}
232

railt / sdl

Push — master ( e081e8...a6938b )

Frontend::collect() A

Complexity

Size

Duplication

Importance

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like