Issues in RouteMatcher.php - All Issues - Inspection of "Adding additional testcases" - canax/router - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 799419...2ea7db )

by Mikael

created 2019-11-22 15:23 UTC

src/Route/RouteMatcher.php (9 issues)

Labels

Severity

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 Anax\Route;

use Anax\Commons\ContainerInjectableInterface;
use Anax\Route\Exception\ConfigurationException;

/**
 * Matching a incoming path to see it it matches a route.
 */
class RouteMatcher
{
    /**
     * @var null|array   $arguments     arguments for the callback, extracted
     *                                  from path
     * @var string       $methodMatched the matched method.
     * @var string       $pathMatched   the matched path.
     */
    public $arguments = [];
    public $methodMatched;
    public $pathMatched;



    /**
     * Check if part of route is a argument and optionally match type
     * as a requirement {argument:type}.
     *
     * @param string $rulePart   the rule part to check.
     * @param string $queryPart  the query part to check.
     * @param array  &$args      add argument to args array if matched
     *
     * @return boolean
     */
    private function checkPartAsArgument($rulePart, $queryPart, &$args)
    {
        if (substr($rulePart, -1) == "}"
            && !is_null($queryPart)
        ) {
            $part = substr($rulePart, 1, -1);
            $pos = strpos($part, ":");
            $type = null;
            if ($pos !== false) {
                $type = substr($part, $pos + 1);
                if (! $this->checkPartMatchingType($queryPart, $type)) {
function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
                    return false;
                }
            }
            $args[] = $this->typeConvertArgument($queryPart, $type);
function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
            return true;
        }
        return false;
    }



    /**
     * Check if value is matching a certain type of values.
     *
     * @param string $value   the value to check.
     * @param array  $type    the expected type to check against.
     *
     * @return boolean
     */
    private function checkPartMatchingType($value, $type)
    {
        switch ($type) {
            case "digit":
                return ctype_digit($value);
                break;
switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

            case "hex":
                return ctype_xdigit($value);
                break;
switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

            case "alpha":
                return ctype_alpha($value);
                break;
switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

            case "alphanum":
                return ctype_alnum($value);
                break;
switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

            default:
                return false;
        }
    }



    /**
     * Check if value is matching a certain type and do type
     * conversion accordingly.
     *
     * @param string $value   the value to check.
     * @param array  $type    the expected type to check against.
     *
     * @return boolean
     */
    private function typeConvertArgument($value, $type)
    {
        switch ($type) {
            case "digit":
                return (int) $value;
                break;
switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}

            default:
                return $value;
        }
    }



    /**
     * Match part of rule and query.
     *
     * @param string $rulePart   the rule part to check.
     * @param string $queryPart  the query part to check.
     * @param array  &$args      add argument to args array if matched
     *
     * @return boolean
     */
    private function matchPart($rulePart, $queryPart, &$args)
    {
        $match = false;
$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}
        $first = isset($rulePart[0]) ? $rulePart[0] : '';
        switch ($first) {
            case '*':
                $match = true;
                break;

            case '{':
                $match = $this->checkPartAsArgument($rulePart, $queryPart, $args);
                break;

            default:
                $match = ($rulePart == $queryPart);
                break;
        }
        return $match;
    }



    /**
     * Check if the request method matches.
     *
     * @param string $method    as request method.
     * @param string $supported as request methods that are valid.
     *
     * @return boolean true if request method matches
     */
    private function matchRequestMethod(
        string $method = null,
        array $supported = null
    ) {
        if ($supported && !in_array($method, $supported)) {
            return false;
        }

        return true;
    }



    /**
     * Check if the route matches a query and request method.
     *
     * @param string $mount           of the current route being matched.
     * @param string $relativePath    of the current route being matched.
     * @param string $absolutePath    of the current route being matched.
     * @param string $query           to match against
     * @param array  $methodSupported as supported request method
     * @param string $method          as request method
     *
     * @return boolean true if query matches the route
     */
    public function match(
        string $mount = null,
        string $relativePath = null,
        string $absolutePath = null,
        string $query,
        array $methodSupported = null,
        string $method = null
    ) {
        $this->arguments = [];
        $this->methodMatched = null;
        $this->pathMatched = null;

        if (!$this->matchRequestMethod($method, $methodSupported)) {
            return false;
        }

        // Is a null path  - mounted on empty, or mount path matches
        // initial query.
        $queryParts = explode('/', $query);

        echo "\nMATCHING: $query ($queryParts[0]) == $mount (" . strlen($mount) . ")";

        //strcmp($queryParts[0], $mount) == 0

        if (is_null($relativePath) && (empty($mount) 
            || strncmp($query, $mount, strlen($mount)) == 0)
        ) {
            echo "TRUE";
            $this->methodMatched = $method;
            $this->pathMatched = $query;
            return true;
        }
        echo "FALSE";

        // Check all parts to see if they matches
        $ruleParts  = explode('/', $absolutePath);
        //$queryParts = explode('/', $query);
        $ruleCount = max(count($ruleParts), count($queryParts));
        $args = [];

        for ($i = 0; $i < $ruleCount; $i++) {
            $rulePart  = isset($ruleParts[$i])  ? $ruleParts[$i]  : null;
            $queryPart = isset($queryParts[$i]) ? $queryParts[$i] : null;

            if ($rulePart === "**") {
                break;
            }

            if (!$this->matchPart($rulePart, $queryPart, $args)) {
                return false;
            }
        }

        $this->arguments = $args;
        $this->methodMatched = $method;
        $this->pathMatched = $query;
        return true;
    }
}


1		<?php
2
3		namespace Anax\Route;
4
5		use Anax\Commons\ContainerInjectableInterface;
6		use Anax\Route\Exception\ConfigurationException;
7
8		/**
9		* Matching a incoming path to see it it matches a route.
10		*/
11		class RouteMatcher
12		{
13		/**
14		* @var null\|array $arguments arguments for the callback, extracted
15		* from path
16		* @var string $methodMatched the matched method.
17		* @var string $pathMatched the matched path.
18		*/
19		public $arguments = [];
20		public $methodMatched;
21		public $pathMatched;
22
23
24
25		/**
26		* Check if part of route is a argument and optionally match type
27		* as a requirement {argument:type}.
28		*
29		* @param string $rulePart the rule part to check.
30		* @param string $queryPart the query part to check.
31		* @param array &$args add argument to args array if matched
32		*
33		* @return boolean
34		*/
35	4	private function checkPartAsArgument($rulePart, $queryPart, &$args)
36		{
37	4	if (substr($rulePart, -1) == "}"
38	4	&& !is_null($queryPart)
39		) {
40	4	$part = substr($rulePart, 1, -1);
41	4	$pos = strpos($part, ":");
42	4	$type = null;
43	4	if ($pos !== false) {
44	1	$type = substr($part, $pos + 1);
45	1	if (! $this->checkPartMatchingType($queryPart, $type)) {
		0 ignored issues – show Documentation introduced 2017-06-27 09:15 UTC by Report Bug Copy Issue Report Show Similar Issues like this `$type` is of type `string`, but the function expects a `array`. 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 Best Practice introduced 2017-03-13 17:07 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$this->checkPartMatchingType($queryPart, $type)` of type `boolean\|null` is loosely compared to `false`; this is ambiguous if the boolean can be false. You might want to explicitly use `!== null` instead. If an expression can have both `false`, and `null` as possible values. It is generally a good practice to always use strict comparison to clearly distinguish between those two values. $a = canBeFalseAndNull(); // Instead of if ( ! $a) { } // Better use one of the explicit versions: if ($a !== null) { } if ($a !== false) { } if ($a !== null && $a !== false) { } Loading history...
46	1	return false;
47		}
48		}
49	4	$args[] = $this->typeConvertArgument($queryPart, $type);
		0 ignored issues – show Documentation introduced 2017-06-27 15:04 UTC by Report Bug Copy Issue Report Show Similar Issues like this `$type` is of type `string\|null`, but the function expects a `array`. 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...
50	4	return true;
51		}
52	3	return false;
53		}
54
55
56
57		/**
58		* Check if value is matching a certain type of values.
59		*
60		* @param string $value the value to check.
61		* @param array $type the expected type to check against.
62		*
63		* @return boolean
64		*/
65	1	private function checkPartMatchingType($value, $type)
66		{
67	1	switch ($type) {
68	1	case "digit":
69	1	return ctype_digit($value);
70		break;
		0 ignored issues – show Unused Code introduced 2017-03-13 17:07 UTC by Report Bug Copy Issue Report Show Similar Issues like this `break` is not strictly necessary here and could be removed. The `break` statement is not necessary if it is preceded for example by a `return` statement: switch ($x) { case 1: return 'foo'; break; // This break is not necessary and can be left off. } If you would like to keep this construct to be consistent with other `case` statements, you can safely mark this issue as a false-positive. Loading history...
71
72	1	case "hex":
73	1	return ctype_xdigit($value);
74		break;
		0 ignored issues – show Unused Code introduced 2017-04-24 07:31 UTC by Report Bug Copy Issue Report Show Similar Issues like this `break` is not strictly necessary here and could be removed. The `break` statement is not necessary if it is preceded for example by a `return` statement: switch ($x) { case 1: return 'foo'; break; // This break is not necessary and can be left off. } If you would like to keep this construct to be consistent with other `case` statements, you can safely mark this issue as a false-positive. Loading history...
75
76	1	case "alpha":
77	1	return ctype_alpha($value);
78		break;
		0 ignored issues – show Unused Code introduced 2017-04-24 07:28 UTC by Report Bug Copy Issue Report Show Similar Issues like this `break` is not strictly necessary here and could be removed. The `break` statement is not necessary if it is preceded for example by a `return` statement: switch ($x) { case 1: return 'foo'; break; // This break is not necessary and can be left off. } If you would like to keep this construct to be consistent with other `case` statements, you can safely mark this issue as a false-positive. Loading history...
79
80	1	case "alphanum":
81	1	return ctype_alnum($value);
82		break;
		0 ignored issues – show Unused Code introduced 2017-03-13 17:07 UTC by Report Bug Copy Issue Report Show Similar Issues like this `break` is not strictly necessary here and could be removed. The `break` statement is not necessary if it is preceded for example by a `return` statement: switch ($x) { case 1: return 'foo'; break; // This break is not necessary and can be left off. } If you would like to keep this construct to be consistent with other `case` statements, you can safely mark this issue as a false-positive. Loading history...
83
84		default:
85	1	return false;
86		}
87		}
88
89
90
91		/**
92		* Check if value is matching a certain type and do type
93		* conversion accordingly.
94		*
95		* @param string $value the value to check.
96		* @param array $type the expected type to check against.
97		*
98		* @return boolean
99		*/
100	4	private function typeConvertArgument($value, $type)
101		{
102	4	switch ($type) {
103	4	case "digit":
104	1	return (int) $value;
105		break;
		0 ignored issues – show Unused Code introduced 2017-06-27 09:15 UTC by Report Bug Copy Issue Report Show Similar Issues like this `break` is not strictly necessary here and could be removed. The `break` statement is not necessary if it is preceded for example by a `return` statement: switch ($x) { case 1: return 'foo'; break; // This break is not necessary and can be left off. } If you would like to keep this construct to be consistent with other `case` statements, you can safely mark this issue as a false-positive. Loading history...
106
107		default:
108	4	return $value;
109		}
110		}
111
112
113
114		/**
115		* Match part of rule and query.
116		*
117		* @param string $rulePart the rule part to check.
118		* @param string $queryPart the query part to check.
119		* @param array &$args add argument to args array if matched
120		*
121		* @return boolean
122		*/
123	80	private function matchPart($rulePart, $queryPart, &$args)
124		{
125	80	$match = false;
		0 ignored issues – show Unused Code introduced 2017-03-03 15:59 UTC by Report Bug Copy Issue Report Show Similar Issues like this `$match` is not used, you could remove the assignment. This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently. $myVar = 'Value'; $higher = false; if (rand(1, 6) > 3) { $higher = true; } else { $higher = false; } Both the `$myVar` assignment in line 1 and the `$higher` assignment in line 2 are dead. The first because `$myVar` is never used and the second because `$higher` is always overwritten for every possible time line. Loading history...
126	80	$first = isset($rulePart[0]) ? $rulePart[0] : '';
127	80	switch ($first) {
128	80	case '*':
129	3	$match = true;
130	3	break;
131
132	80	case '{':
133	4	$match = $this->checkPartAsArgument($rulePart, $queryPart, $args);
134	4	break;
135
136		default:
137	79	$match = ($rulePart == $queryPart);
138	79	break;
139		}
140	80	return $match;
141		}
142
143
144
145		/**
146		* Check if the request method matches.
147		*
148		* @param string $method as request method.
149		* @param string $supported as request methods that are valid.
150		*
151		* @return boolean true if request method matches
152		*/
153	152	private function matchRequestMethod(
154		string $method = null,
155		array $supported = null
156		) {
157	152	if ($supported && !in_array($method, $supported)) {
158	42	return false;
159		}
160
161	152	return true;
162		}
163
164
165
166		/**
167		* Check if the route matches a query and request method.
168		*
169		* @param string $mount of the current route being matched.
170		* @param string $relativePath of the current route being matched.
171		* @param string $absolutePath of the current route being matched.
172		* @param string $query to match against
173		* @param array $methodSupported as supported request method
174		* @param string $method as request method
175		*
176		* @return boolean true if query matches the route
177		*/
178	152	public function match(
179		string $mount = null,
180		string $relativePath = null,
181		string $absolutePath = null,
182		string $query,
183		array $methodSupported = null,
184		string $method = null
185		) {
186	152	$this->arguments = [];
187	152	$this->methodMatched = null;
188	152	$this->pathMatched = null;
189
190	152	if (!$this->matchRequestMethod($method, $methodSupported)) {
191	42	return false;
192		}
193
194		// Is a null path - mounted on empty, or mount path matches
195		// initial query.
196	152	$queryParts = explode('/', $query);
197
198	152	echo "\nMATCHING: $query ($queryParts[0]) == $mount (" . strlen($mount) . ")";
199
200		//strcmp($queryParts[0], $mount) == 0
201
202	152	if (is_null($relativePath) && (empty($mount)
203	152	\|\| strncmp($query, $mount, strlen($mount)) == 0)
204		) {
205	75	echo "TRUE";
206	75	$this->methodMatched = $method;
207	75	$this->pathMatched = $query;
208	75	return true;
209		}
210	80	echo "FALSE";
211
212		// Check all parts to see if they matches
213	80	$ruleParts = explode('/', $absolutePath);
214		//$queryParts = explode('/', $query);
215	80	$ruleCount = max(count($ruleParts), count($queryParts));
216	80	$args = [];
217
218	80	for ($i = 0; $i < $ruleCount; $i++) {
219	80	$rulePart = isset($ruleParts[$i]) ? $ruleParts[$i] : null;
220	80	$queryPart = isset($queryParts[$i]) ? $queryParts[$i] : null;
221
222	80	if ($rulePart === "**") {
223	2	break;
224		}
225
226	80	if (!$this->matchPart($rulePart, $queryPart, $args)) {
227	27	return false;
228		}
229		}
230
231	76	$this->arguments = $args;
232	76	$this->methodMatched = $method;
233	76	$this->pathMatched = $query;
234	76	return true;
235		}
236		}
237

canax / router

Push — master ( 799419...2ea7db )

src/Route/RouteMatcher.php (9 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like