Completed
Pull Request — master (#235)
by Kévin
05:19 queued 02:01
created

ClassDefinition   C

Complexity

Total Complexity 62

Size/Duplication

Total Lines 416
Duplicated Lines 0 %

Coupling/Cohesion

Components 3
Dependencies 13

Test Coverage

Coverage 66.26%

Importance

Changes 2
Bugs 0 Features 0
Metric Value
dl 0
loc 416
ccs 108
cts 163
cp 0.6626
rs 5.9493
c 2
b 0
f 0
wmc 62
lcom 3
cbo 13

21 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 6 1
A addProperty() 0 8 2
A addConst() 0 4 1
B hasMethod() 0 13 7
A hasConst() 0 8 4
A getMethod() 0 12 4
A hasProperty() 0 8 4
A getProperty() 0 14 4
A isAbstract() 0 4 1
A getExtendsClassDefinition() 0 4 1
A addInterface() 0 4 1
A getPropertyStatement() 0 12 4
A getFilepath() 0 4 1
A setFilepath() 0 4 1
A isFinal() 0 4 1
A setExtendsClass() 0 4 1
A setExtendsClassDefinition() 0 4 1
A getExtendsClass() 0 4 1
D mergeTrait() 0 29 9
A addMethod() 0 15 3
C compile() 0 87 10

How to fix   Complexity   

Complex Class

Complex classes like ClassDefinition often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use ClassDefinition, and based on these observations, apply Extract Interface, too.

1
<?php
2
/**
3
 * @author Patsura Dmitry https://github.com/ovr <[email protected]>
4
 */
5
6
namespace PHPSA\Definition;
7
8
use PHPSA\CompiledExpression;
9
use PHPSA\Context;
10
use PhpParser\Node;
11
use PHPSA\Variable;
12
use PHPSA\Compiler\Event;
13
14
/**
15
 * Class ClassDefinition
16
 * @package PHPSA\Definition
17
 */
18
class ClassDefinition extends ParentDefinition
0 ignored issues
show
Complexity introduced by
This class has a complexity of 62 which exceeds the configured maximum of 50.

The class complexity is the sum of the complexity of all methods. A very high value is usually an indication that your class does not follow the single reponsibility principle and does more than one job.

Some resources for further reading:

You can also find more detailed suggestions for refactoring in the “Code” section of your repository.

Loading history...
19
{
20
    /**
21
     * @var int
22
     */
23
    protected $type;
24
25
    /**
26
     * Class methods
27
     *
28
     * @var ClassMethod[]
29
     */
30
    protected $methods = array();
31
32
    /**
33
     * Class properties
34
     *
35
     * @var Node\Stmt\PropertyProperty[]
36
     */
37
    protected $properties = array();
38
39
    /**
40
     * Property Statements
41
     *
42
     * @var Node\Stmt\Property[]
43
     */
44
    protected $propertyStatements = array();
45
46
    /**
47
     * Class constants
48
     *
49
     * @var Node\Stmt\Const_[]
50
     */
51
    protected $constants = array();
52
53
    /**
54
     * @todo Use Finder
55
     *
56
     * @var string
57
     */
58
    protected $filepath;
59
60
    /**
61
     * @var Node\Stmt\Class_|null
62
     */
63
    protected $statement;
64
65
    /**
66
     * @var string|null
67
     */
68
    protected $extendsClass;
69
70
    /**
71
     * @var ClassDefinition|null
72
     */
73
    protected $extendsClassDefinition;
0 ignored issues
show
Comprehensibility Naming introduced by
The variable name $extendsClassDefinition exceeds the maximum configured length of 20.

Very long variable names usually make code harder to read. It is therefore recommended not to make variable names too verbose.

Loading history...
74
75
    /**
76
     * @var array
77
     */
78
    protected $interfaces = array();
79
80
    /**
81
     * @param string $name
82
     * @param Node\Stmt\Class_ $statement
83
     * @param integer $type
84
     */
85 874
    public function __construct($name, Node\Stmt\Class_ $statement = null, $type = 0)
86
    {
87 874
        $this->name = $name;
88 874
        $this->statement = $statement;
89 874
        $this->type = $type;
90 874
    }
91
92
    /**
93
     * @param ClassMethod $classMethod
94
     * @param bool $overwrite Should we overwrite method if it already exists
95
     * @return bool Did we overwrite method?
96
     */
97 40
    public function addMethod(ClassMethod $classMethod, $overwrite = true)
98
    {
99 40
        if ($overwrite) {
100 40
            $this->methods[$classMethod->getName()] = $classMethod;
101 40
        } else {
102 1
            $name = $classMethod->getName();
103
            if (isset($this->methods[$name])) {
104
                return false;
105
            } else {
106
                $this->methods[$name] = $classMethod;
107
            }
108
        }
109
110 40
        return true;
111
    }
112
113
    /**
114
     * @param Node\Stmt\Property $property
115
     */
116 7
    public function addProperty(Node\Stmt\Property $property)
117
    {
118 7
        foreach ($property->props as $propertyDefinition) {
119 7
            $this->properties[$propertyDefinition->name] = $propertyDefinition;
120 7
        }
121
122 7
        $this->propertyStatements[$propertyDefinition->name] = $property;
0 ignored issues
show
Bug introduced by
The variable $propertyDefinition seems to be defined by a foreach iteration on line 118. Are you sure the iterator is never empty, otherwise this variable is not defined?

It seems like you are relying on a variable being defined by an iteration:

foreach ($a as $b) {
}

// $b is defined here only if $a has elements, for example if $a is array()
// then $b would not be defined here. To avoid that, we recommend to set a
// default value for $b.


// Better
$b = 0; // or whatever default makes sense in your context
foreach ($a as $b) {
}

// $b is now guaranteed to be defined here.
Loading history...
123 7
    }
124
125
    /**
126
     * @param Node\Stmt\ClassConst $const
127
     */
128 4
    public function addConst(Node\Stmt\ClassConst $const)
129
    {
130 4
        $this->constants[$const->consts[0]->name] = $const;
131 4
    }
132
133
    /**
134
     * @param Context $context
135
     * @return $this
136
     */
137 41
    public function compile(Context $context)
138
    {
139 41
        if ($this->compiled) {
140
            return $this;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $this; (PHPSA\Definition\ClassDefinition) is incompatible with the return type declared by the abstract method PHPSA\Definition\AbstractDefinition::compile of type boolean.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
141
        }
142
143 41
        $context->getEventManager()->fire(
144 41
            Event\StatementBeforeCompile::EVENT_NAME,
145 41
            new Event\StatementBeforeCompile(
146 41
                $this->statement,
0 ignored issues
show
Bug introduced by
It seems like $this->statement can be null; however, __construct() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
147
                $context
148 41
            )
149 41
        );
150
151 41
        $this->compiled = true;
152 41
        $context->setFilepath($this->filepath);
153 41
        $context->setScope($this);
154
155
        // Compile event for properties
156 41
        foreach ($this->properties as $property) {
157 6
            if (!$property->default) {
158 2
                continue;
159
            }
160
161
            // fire expression event for property default
162 5
            $context->getEventManager()->fire(
163 5
                Event\ExpressionBeforeCompile::EVENT_NAME,
164 5
                new Event\ExpressionBeforeCompile(
165 5
                    $property->default,
166
                    $context
167 5
                )
168 5
            );
169 41
        }
170
171
        // Compile event for constants
172 41
        foreach ($this->constants as $const) {
173 2
            $context->getEventManager()->fire(
174 2
                Event\StatementBeforeCompile::EVENT_NAME,
175 2
                new Event\StatementBeforeCompile(
176 2
                    $const,
177
                    $context
178 2
                )
179 2
            );
180 41
        }
181
182
        // Compiler event for property statements
183 41
        foreach ($this->propertyStatements as $prop) {
184 6
            $context->getEventManager()->fire(
185 6
                Event\StatementBeforeCompile::EVENT_NAME,
186 6
                new Event\StatementBeforeCompile(
187 6
                    $prop,
188
                    $context
189 6
                )
190 6
            );
191 41
        }
192
193
        // Compile each method
194 41
        foreach ($this->methods as $method) {
195 39
            $context->clearSymbols();
196
197 39
            if (!$method->isStatic()) {
198 38
                $thisPtr = new Variable('this', $this, CompiledExpression::OBJECT);
199 38
                $thisPtr->incGets();
200 38
                $context->addVariable($thisPtr);
201 38
            }
202
203 39
            $method->compile($context);
204
205 39
            foreach ($context->getSymbols() as $name => $variable) {
206 38
                if (!$variable->isUnused()) {
207 38
                    continue;
208
                }
209
210 9
                $context->notice(
211 9
                    'unused-' . $variable->getSymbolType(),
212 9
                    sprintf(
213 9
                        'Unused ' . $variable->getSymbolType() . ' $%s in method %s()',
214 9
                        $variable->getName(),
215 9
                        $method->getName()
216 9
                    ),
217 9
                    $variable->getDeclarationStatement()
0 ignored issues
show
Bug introduced by
It seems like $variable->getDeclarationStatement() can be null; however, notice() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
218 9
                );
219 39
            }
220 41
        }
221
222 41
        return $this;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $this; (PHPSA\Definition\ClassDefinition) is incompatible with the return type declared by the abstract method PHPSA\Definition\AbstractDefinition::compile of type boolean.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
223
    }
224
225
    /**
226
     * @param string $name
227
     * @param boolean|false $inherit
228
     * @return bool
229
     */
230 2
    public function hasMethod($name, $inherit = false)
231
    {
232 2
        if (isset($this->methods[$name])) {
233 2
            return true;
234
        }
235
236 1
        if ($inherit && $this->extendsClassDefinition && $this->extendsClassDefinition->hasMethod($name, $inherit)) {
237
            $method = $this->extendsClassDefinition->getMethod($name, $inherit);
238
            return $method && ($method->isPublic() || $method->isProtected());
239
        }
240
241 1
        return false;
242
    }
243
244
    /**
245
     * @param string $name
246
     * @param bool $inherit
247
     * @return bool
248
     */
249 2
    public function hasConst($name, $inherit = false)
250
    {
251 2
        if ($inherit && $this->extendsClassDefinition && $this->extendsClassDefinition->hasConst($name, $inherit)) {
252 1
            return true;
253
        }
254
255 2
        return isset($this->constants[$name]);
256
    }
257
258
    /**
259
     * @param $name
260
     * @param boolean|false $inherit
261
     * @return ClassMethod|null
262
     */
263 2
    public function getMethod($name, $inherit = false)
264
    {
265 2
        if (isset($this->methods[$name])) {
266 2
            return $this->methods[$name];
267
        }
268
269
        if ($inherit && $this->extendsClassDefinition) {
270
            return $this->extendsClassDefinition->getMethod($name, $inherit);
271
        }
272
273
        return null;
274
    }
275
276
    /**
277
     * @param $name
278
     * @param bool $inherit
279
     * @return bool
280
     */
281 3
    public function hasProperty($name, $inherit = false)
282
    {
283 3
        if (isset($this->properties[$name])) {
284 1
            return isset($this->properties[$name]);
285
        }
286
287 3
        return $inherit && $this->extendsClassDefinition && $this->extendsClassDefinition->hasProperty($name, true);
288
    }
289
290
    /**
291
     * @param string $name
292
     * @param bool $inherit
293
     * @return Node\Stmt\PropertyProperty
294
     */
295
    public function getProperty($name, $inherit = false)
296
    {
297
        assert($this->hasProperty($name, $inherit));
298
299
        if (isset($this->properties[$name])) {
300
            return $this->properties[$name];
301
        }
302
303
        if ($inherit && $this->extendsClassDefinition) {
304
            return $this->extendsClassDefinition->getProperty($name, true);
305
        }
306
307
        return null;
308
    }
309
310
    /**
311
     * @param string $name
312
     * @param bool $inherit
313
     * @return Node\Stmt\Property
314
     */
315
    public function getPropertyStatement($name, $inherit = false)
316
    {
317
        if (isset($this->propertyStatements[$name])) {
318
            return $this->propertyStatements[$name];
319
        }
320
321
        if ($inherit && $this->extendsClassDefinition) {
322
            return $this->extendsClassDefinition->getPropertyStatement($name, true);
323
        }
324
325
        return null;
326
    }
327
328
    /**
329
     * @return string
330
     */
331 41
    public function getFilepath()
332
    {
333 41
        return $this->filepath;
334
    }
335
336
    /**
337
     * @param string $filepath
338
     */
339 41
    public function setFilepath($filepath)
340
    {
341 41
        $this->filepath = $filepath;
342 41
    }
343
344
    /**
345
     * @return bool
346
     */
347
    public function isAbstract()
348
    {
349
        return (bool) ($this->type & Node\Stmt\Class_::MODIFIER_ABSTRACT);
350
    }
351
352
    /**
353
     * @return bool
354
     */
355 1
    public function isFinal()
356
    {
357 1
        return (bool) ($this->type & Node\Stmt\Class_::MODIFIER_FINAL);
358
    }
359
360
    /**
361
     * @param null|string $extendsClass
362
     */
363
    public function setExtendsClass($extendsClass)
364
    {
365
        $this->extendsClass = $extendsClass;
366
    }
367
368
    /**
369
     * @return null|ClassDefinition
370
     */
371
    public function getExtendsClassDefinition()
372
    {
373
        return $this->extendsClassDefinition;
374
    }
375
376
    /**
377
     * @param ClassDefinition $extendsClassDefinition
378
     */
379 1
    public function setExtendsClassDefinition(ClassDefinition $extendsClassDefinition)
0 ignored issues
show
Comprehensibility Naming introduced by
The variable name $extendsClassDefinition exceeds the maximum configured length of 20.

Very long variable names usually make code harder to read. It is therefore recommended not to make variable names too verbose.

Loading history...
380
    {
381 1
        $this->extendsClassDefinition = $extendsClassDefinition;
382 1
    }
383
384
    /**
385
     * @param string $interface
386
     */
387
    public function addInterface($interface)
388
    {
389
        $this->interfaces[] = $interface;
390
    }
391
392
    /**
393
     * @return null|string
394
     */
395 41
    public function getExtendsClass()
396
    {
397 41
        return $this->extendsClass;
398
    }
399
400
    /**
401
     * @param TraitDefinition $definition
402
     * @param Node\Stmt\TraitUseAdaptation\Alias[] $adaptations
403
     */
404
    public function mergeTrait(TraitDefinition $definition, array $adaptations)
405
    {
406
        $methods = $definition->getMethods();
407
        if ($methods) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $methods of type PHPSA\Definition\ClassMethod[] is implicitly converted to a boolean; are you sure this is intended? If so, consider using ! empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
408
            foreach ($adaptations as $adaptation) {
409
                // We don't support Trait name for now
410
                if (!$adaptation->trait) {
411
                    $methodNameFromTrait = $adaptation->method;
412
                    if (isset($methods[$methodNameFromTrait])) {
413
                        /** @var ClassMethod $method Method from Trait */
414
                        $method = $methods[$methodNameFromTrait];
415
                        if ($adaptation->newName
0 ignored issues
show
Bug Best Practice introduced by
The expression $adaptation->newName of type null|string is loosely compared to true; this is ambiguous if the string can be empty. You might want to explicitly use !== null instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For string values, the empty string '' is a special case, in particular the following results might be unexpected:

''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
Loading history...
416
                            || ($adaptation->newModifier && $method->getModifier() != $adaptation->newModifier)) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $adaptation->newModifier of type null|integer is loosely compared to true; this is ambiguous if the integer can be zero. You might want to explicitly use !== null instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For integer values, zero is a special case, in particular the following results might be unexpected:

0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
Loading history...
417
                            // Don't modify original method from Trait
418
                            $method = clone $method;
419
                            $method->setName($adaptation->newName);
420
                            $method->setModifier($adaptation->newModifier);
421
422
                            $methods[$methodNameFromTrait] = $method;
423
                        }
424
                    }
425
                }
426
            }
427
428
            foreach ($methods as $method) {
429
                $this->addMethod($method, false);
430
            }
431
        }
432
    }
433
}
434