Skip to content

Commit c60371e

Browse files
loren-osbornaik099
loren-osborn
authored and
aik099
committed
Scrutinizer fixes (#76)
Fixing issues, reported by Scrutinizer CI 1. most of issues addressed 2. normalized typecast operator look 3. replaced magic numbers with PHP constants 4. added parameter type checks for `ReflectionFile` and `ReflectionFileNamespace` classes
1 parent 49e6cb8 commit c60371e

16 files changed

+339
-23
lines changed

‎src/ReflectionClass.php

+14
Original file line numberDiff line numberDiff line change
@@ -125,4 +125,18 @@ protected function __initialize()
125125
{
126126
parent::__construct($this->getName());
127127
}
128+
129+
/**
130+
* Create a ReflectionClass for a given class name.
131+
*
132+
* @param string $className
133+
* The name of the class to create a reflection for.
134+
*
135+
* @return ReflectionClass
136+
* The apropriate reflection object.
137+
*/
138+
protected function createReflectionForClass($className)
139+
{
140+
return class_exists($className, false) ? new parent($className) : new static($className);
141+
}
128142
}

‎src/ReflectionFile.php

+8
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,14 @@ class ReflectionFile
5050
*/
5151
public function __construct($fileName, $topLevelNodes = null)
5252
{
53+
if (!is_string($fileName)) {
54+
throw new \InvalidArgumentException(
55+
sprintf(
56+
'$fileName must be a string, but a %s was passed',
57+
gettype($fileName)
58+
)
59+
);
60+
}
5361
$fileName = PathResolver::realpath($fileName);
5462
$this->fileName = $fileName;
5563
$this->topLevelNodes = $topLevelNodes ?: ReflectionEngine::parseFile($fileName);

‎src/ReflectionFileNamespace.php

+9-1
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,14 @@ class ReflectionFileNamespace
8383
*/
8484
public function __construct($fileName, $namespaceName, Namespace_ $namespaceNode = null)
8585
{
86+
if (!is_string($fileName)) {
87+
throw new \InvalidArgumentException(
88+
sprintf(
89+
'$fileName must be a string, but a %s was passed',
90+
gettype($fileName)
91+
)
92+
);
93+
}
8694
$fileName = PathResolver::realpath($fileName);
8795
if (!$namespaceNode) {
8896
$namespaceNode = ReflectionEngine::parseFileNamespace($fileName, $namespaceName);
@@ -172,7 +180,7 @@ public function getDocComment()
172180
$comments = $this->namespaceNode->getAttribute('comments');
173181

174182
if ($comments) {
175-
$docComment = (string) $comments[0];
183+
$docComment = (string)$comments[0];
176184
}
177185

178186
return $docComment;

‎src/ReflectionMethod.php

+10-3
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,9 @@ public function ___debugInfo()
8080
*/
8181
public function __toString()
8282
{
83-
$hasReturnType = $this->hasReturnType();
83+
// Internally $this->getReturnType() !== null is the same as $this->hasReturnType()
84+
$returnType = $this->getReturnType();
85+
$hasReturnType = $returnType !== null;
8486
$paramsNeeded = $hasReturnType || $this->getNumberOfParameters() > 0;
8587
$paramFormat = $paramsNeeded ? "\n\n - Parameters [%d] {%s\n }" : '';
8688
$returnFormat = $hasReturnType ? "\n - Return [ %s ]" : '';
@@ -107,14 +109,19 @@ public function __toString()
107109
$this->isFinal() ? ' final' : '',
108110
$this->isStatic() ? ' static' : '',
109111
$this->isAbstract() ? ' abstract' : '',
110-
join(' ', \Reflection::getModifierNames($this->getModifiers() & 1792)),
112+
join(
113+
' ',
114+
\Reflection::getModifierNames(
115+
$this->getModifiers() & (self::IS_PUBLIC | self::IS_PROTECTED | self::IS_PRIVATE)
116+
)
117+
),
111118
$this->getName(),
112119
$this->getFileName(),
113120
$this->getStartLine(),
114121
$this->getEndLine(),
115122
count($methodParameters),
116123
$paramString,
117-
$hasReturnType ? ReflectionType::convertToDisplayType($this->getReturnType()) : ''
124+
$returnType ? ReflectionType::convertToDisplayType($returnType) : ''
118125
);
119126
}
120127

‎src/ReflectionParameter.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public function __toString()
132132
}
133133
/* @see https://3v4l.org/DJOEb for behaviour changes */
134134
if (is_double($defaultValue) && fmod($defaultValue, 1.0) === 0.0) {
135-
$defaultValue = (int) $defaultValue;
135+
$defaultValue = (int)$defaultValue;
136136
}
137137

138138
$defaultValue = str_replace('\\\\', '\\', var_export($defaultValue, true));

‎src/ReflectionType.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public static function convertToDisplayType(\ReflectionType $type)
8585
'int' => 'integer',
8686
'bool' => 'boolean'
8787
];
88-
$displayType = $type->type;
88+
$displayType = (string)$type;
8989
if (isset($typeMap[$displayType])) {
9090
$displayType = $typeMap[$displayType];
9191
}

‎src/Traits/ReflectionClassLikeTrait.php

+15-2
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,7 @@ public function getInterfaces()
322322

323323
/**
324324
* {@inheritdoc}
325+
* @param string $name
325326
*/
326327
public function getMethod($name)
327328
{
@@ -442,7 +443,7 @@ public function getParentClass()
442443
$extendsNode = $hasExtends ? $this->classLikeNode->$extendsField : null;
443444
if ($extendsNode instanceof FullyQualified) {
444445
$extendsName = $extendsNode->toString();
445-
$parentClass = class_exists($extendsName, false) ? new parent($extendsName) : new static($extendsName);
446+
$parentClass = $this->createReflectionForClass($extendsName);
446447
}
447448
$this->parentClass = $parentClass;
448449
}
@@ -592,6 +593,7 @@ public function hasConstant($name)
592593

593594
/**
594595
* {@inheritdoc}
596+
* @param string $name
595597
*/
596598
public function hasMethod($name)
597599
{
@@ -622,6 +624,7 @@ public function hasProperty($name)
622624

623625
/**
624626
* {@inheritDoc}
627+
* @param string $interfaceName
625628
*/
626629
public function implementsInterface($interfaceName)
627630
{
@@ -698,7 +701,6 @@ public function isInstance($object)
698701
}
699702

700703
$className = $this->getName();
701-
702704
return $className === get_class($object) || is_subclass_of($object, $className);
703705
}
704706

@@ -954,4 +956,15 @@ private function collectSelfConstants()
954956
}
955957
}
956958
}
959+
960+
/**
961+
* Create a ReflectionClass for a given class name.
962+
*
963+
* @param string $className
964+
* The name of the class to create a reflection for.
965+
*
966+
* @return ReflectionClass
967+
* The apropriate reflection object.
968+
*/
969+
abstract protected function createReflectionForClass($className);
957970
}

‎src/Traits/ReflectionFunctionLikeTrait.php

+18
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,15 @@ public function getStartLine()
220220
*/
221221
public function getStaticVariables()
222222
{
223+
// In nikic/PHP-Parser < 2.0.0 the default behavior is cloning
224+
// nodes when traversing them. Passing FALSE to the constructor
225+
// prevents this.
226+
// In nikic/PHP-Parser >= 2.0.0 and < 3.0.0 the default behavior was
227+
// changed to not clone nodes, but the parameter was retained as
228+
// an option.
229+
// In nikic/PHP-Parser >= 3.0.0 the option to clone nodes was removed
230+
// as a constructor parameter, so Scrutinizer will pick this up as
231+
// an issue. It is retained for legacy compatibility.
223232
$nodeTraverser = new NodeTraverser(false);
224233
$variablesCollector = new StaticVariablesCollector($this);
225234
$nodeTraverser->addVisitor($variablesCollector);
@@ -274,6 +283,15 @@ public function isDeprecated()
274283
*/
275284
public function isGenerator()
276285
{
286+
// In nikic/PHP-Parser < 2.0.0 the default behavior is cloning
287+
// nodes when traversing them. Passing FALSE to the constructor
288+
// prevents this.
289+
// In nikic/PHP-Parser >= 2.0.0 and < 3.0.0 the default behavior was
290+
// changed to not clone nodes, but the parameter was retained as
291+
// an option.
292+
// In nikic/PHP-Parser >= 3.0.0 the option to clone nodes was removed
293+
// as a constructor parameter, so Scrutinizer will pick this up as
294+
// an issue. It is retained for legacy compatibility.
277295
$nodeTraverser = new NodeTraverser(false);
278296
$nodeDetector = new GeneratorDetector();
279297
$nodeTraverser->addVisitor($nodeDetector);

‎src/ValueResolver/NodeExpressionResolver.php

+28-10
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,7 @@ protected function resolve(Node $node)
112112
try {
113113
++$this->nodeLevel;
114114

115-
$nodeType = $node->getType();
116-
$methodName = 'resolve' . str_replace('_', '', $nodeType);
115+
$methodName = $this->getDispatchMethodFor($node);
117116
if (method_exists($this, $methodName)) {
118117
$value = $this->$methodName($node);
119118
}
@@ -142,7 +141,7 @@ protected function resolveScalarString(Scalar\String_ $node)
142141
protected function resolveScalarMagicConstMethod()
143142
{
144143
if ($this->context instanceof \ReflectionMethod) {
145-
$fullName = $this->context->getDeclaringClass()->getName() . '::' . $this->context->getShortName();
144+
$fullName = $this->context->getDeclaringClass()->name . '::' . $this->context->getShortName();
146145

147146
return $fullName;
148147
}
@@ -175,12 +174,12 @@ protected function resolveScalarMagicConstNamespace()
175174
protected function resolveScalarMagicConstClass()
176175
{
177176
if ($this->context instanceof \ReflectionClass) {
178-
return $this->context->getName();
177+
return $this->context->name;
179178
}
180179
if (method_exists($this->context, 'getDeclaringClass')) {
181180
$declaringClass = $this->context->getDeclaringClass();
182181
if ($declaringClass instanceof \ReflectionClass) {
183-
return $declaringClass->getName();
182+
return $declaringClass->name;
184183
}
185184
}
186185

@@ -213,7 +212,7 @@ protected function resolveScalarMagicConstLine(MagicConst\Line $node)
213212
protected function resolveScalarMagicConstTrait()
214213
{
215214
if ($this->context instanceof \ReflectionClass && $this->context->isTrait()) {
216-
return $this->context->getName();
215+
return $this->context->name;
217216
}
218217

219218
return '';
@@ -224,8 +223,6 @@ protected function resolveExprConstFetch(Expr\ConstFetch $node)
224223
$constantValue = null;
225224
$isResolved = false;
226225

227-
/** @var ReflectionFileNamespace|null $fileNamespace */
228-
$fileNamespace = null;
229226
$isFQNConstant = $node->name instanceof Node\Name\FullyQualified;
230227
$constantName = $node->name->toString();
231228

@@ -256,7 +253,22 @@ protected function resolveExprConstFetch(Expr\ConstFetch $node)
256253

257254
protected function resolveExprClassConstFetch(Expr\ClassConstFetch $node)
258255
{
259-
$refClass = $this->fetchReflectionClass($node->class);
256+
$classToReflect = $node->class;
257+
if (!($classToReflect instanceof Node\Name)) {
258+
$classToReflect = $this->resolve($classToReflect) ?: $classToReflect;
259+
if (!is_string($classToReflect)) {
260+
$reason = 'Unable';
261+
if ($classToReflect instanceof Expr) {
262+
$methodName = $this->getDispatchMethodFor($classToReflect);
263+
$reason = "Method " . __CLASS__ . "::{$methodName}() not found trying";
264+
}
265+
throw new ReflectionException("$reason to resolve class constant.");
266+
}
267+
// Strings evaluated as class names are always treated as fully
268+
// qualified.
269+
$classToReflect = new Node\Name\FullyQualified(ltrim($classToReflect, '\\'));
270+
}
271+
$refClass = $this->fetchReflectionClass($classToReflect);
260272
$constantName = $node->name;
261273

262274
// special handling of ::class constants
@@ -265,7 +277,7 @@ protected function resolveExprClassConstFetch(Expr\ClassConstFetch $node)
265277
}
266278

267279
$this->isConstant = true;
268-
$this->constantName = (string)$node->class . '::' . $constantName;
280+
$this->constantName = (string)$classToReflect . '::' . $constantName;
269281

270282
return $refClass->getConstant($constantName);
271283
}
@@ -430,6 +442,12 @@ protected function resolveExprBinaryOpLogicalXor(Expr\BinaryOp\LogicalXor $node)
430442
return $this->resolve($node->left) xor $this->resolve($node->right);
431443
}
432444

445+
private function getDispatchMethodFor(Node $node)
446+
{
447+
$nodeType = $node->getType();
448+
return 'resolve' . str_replace('_', '', $nodeType);
449+
}
450+
433451
/**
434452
* Utility method to fetch reflection class instance by name
435453
*

‎src/bootstrap.php

+4-5
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,9 @@
1212
use Go\ParserReflection\ReflectionEngine;
1313

1414
/**
15-
* This file is used for automatic configuration of Go\ParserReflection\ReflectionEngine class
15+
* This file is used for automatic configuration of
16+
* Go\ParserReflection\ReflectionEngine class
1617
*/
1718
ReflectionEngine::init(new ComposerLocator());
18-
// Polifyll for PHP<7.0
19-
if (!class_exists(ReflectionType::class, false)) {
20-
class ReflectionType {}
21-
}
19+
20+
require(__DIR__ . '/polyfill.php');

‎src/polyfill.php

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
/**
3+
* Parser Reflection API
4+
*
5+
* @copyright Copyright 2015, Lisachenko Alexander <[email protected]>
6+
*
7+
* This source file is subject to the license that is bundled
8+
* with this source code in the file LICENSE.
9+
*/
10+
11+
/**
12+
* This file is for ployfilling classes not defined in all supported
13+
* versions of PHP, (i.e. PHP < 7).
14+
*/
15+
if (!class_exists(ReflectionType::class, false)) {
16+
/* Dummy polyfill class */
17+
class ReflectionType
18+
{
19+
public function allowsNull()
20+
{
21+
return true;
22+
}
23+
24+
public function isBuiltin()
25+
{
26+
return false;
27+
}
28+
29+
public function __toString()
30+
{
31+
return '';
32+
}
33+
}
34+
}

‎tests/ReflectionFileNamespaceTest.php

+18
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,24 @@ protected function setUp()
2626
include_once $fileName;
2727
}
2828

29+
/**
30+
* @expectedException InvalidArgumentException
31+
* @expectedExceptionMessage $fileName must be a string, but a array was passed
32+
*/
33+
public function testBadFilenameTypeArray()
34+
{
35+
new ReflectionFileNamespace([1, 3, 5, 7], 'BogusNamespace');
36+
}
37+
38+
/**
39+
* @expectedException InvalidArgumentException
40+
* @expectedExceptionMessage $fileName must be a string, but a object was passed
41+
*/
42+
public function testBadFilenameTypeObject()
43+
{
44+
new ReflectionFileNamespace(new \DateTime(), 'BogusNamespace');
45+
}
46+
2947
public function testGetClass()
3048
{
3149
$refClass = $this->parsedRefFileNamespace->getClass('Unknown');

‎tests/ReflectionFileTest.php

+18
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,24 @@ protected function setUp()
2121
$this->parsedRefFile = $reflectionFile;
2222
}
2323

24+
/**
25+
* @expectedException InvalidArgumentException
26+
* @expectedExceptionMessage $fileName must be a string, but a array was passed
27+
*/
28+
public function testBadFilenameTypeArray()
29+
{
30+
new ReflectionFile([1, 3, 5, 7]);
31+
}
32+
33+
/**
34+
* @expectedException InvalidArgumentException
35+
* @expectedExceptionMessage $fileName must be a string, but a object was passed
36+
*/
37+
public function testBadFilenameTypeObject()
38+
{
39+
new ReflectionFile(new \DateTime());
40+
}
41+
2442
public function testGetName()
2543
{
2644
$fileName = $this->parsedRefFile->getName();

0 commit comments

Comments
 (0)