addCompositeRule_recursion()   F
last analyzed

Complexity

Conditions 25
Paths 78

Size

Total Lines 128

Duplication

Lines 8
Ratio 6.25 %

Code Coverage

Tests 84
CRAP Score 25.0079

Importance

Changes 0
Metric Value
cc 25
nc 78
nop 3
dl 8
loc 128
ccs 84
cts 86
cp 0.9767
crap 25.0079
rs 3.3333
c 0
b 0
f 0

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
/**
3
 * LogicalFilter
4
 *
5
 * @package php-logical-filter
6
 * @author  Jean Claveau
7
 */
8
namespace JClaveau\LogicalFilter;
9
10
use JClaveau\LogicalFilter\Rule\AbstractRule;
11
use JClaveau\LogicalFilter\Rule\AbstractOperationRule;
12
use JClaveau\LogicalFilter\Rule\AndRule;
13
use JClaveau\LogicalFilter\Rule\OrRule;
14
use JClaveau\LogicalFilter\Rule\NotRule;
15
16
/**
17
 * This "parser" (lexer?) generates a tree of rule instances from a
18
 * description of rules which can be a composed of builtin types, based
19
 * on a tree of arrays mixed with segments of rule tree or LogicalFilter
20
 * instances.
21
 */
22
// abstract class RuleDescriptionParser
23
class RuleDescriptionParser
24
{
25
    /**
26
     * This method parses different ways to define the rules of a LogicalFilter.
27
     * + You can add N already instanciated Rules.
28
     * + You can provide 3 arguments: $field, $operator, $value
29
     * + You can provide a tree of rules:
30
     * ['or',
31
     *      ['and',
32
     *          ['field_5', 'above', 'a'],
33
     *          ['field_5', 'below', 'a'],
34
     *      ],
35
     *      ['field_6', 'equal', 'b'],
36
     *  ]
37
     *
38
     * @param  string            $operation         and | or
39
     * @param  array             $rules_description Rules description
40
     * @param  AbstractRule|null $filter_rules The existing ruletree of the filter
41
     * @param  array             $options
42
     *
43
     * @return AbstractRule  The updated rule tree
0 ignored issues
show
Documentation introduced by
Should the return type not be AbstractRule|null?

This check compares the return type specified in the @return annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch.

Loading history...
44
     */
45 276
    public static function updateRuleTreeFromDescription(
46
        $operation,
47
        array $rules_description,
48
        $filter_rules,
49
        $options
50
    ) {
51 276
        if ($rules_description == [null]) {
52
            // TODO this is due to the bad design of using "Null" instead of
53
            // TrueRule when a Filter "has no rule". So it's the equivalent of
54
            // "and true" or "or true".
55
            // Remove it while fixing https://github.com/jclaveau/php-logical-filter/issues/59
56 37
            if (AndRule::operator == $operation) {
57
                // A && True <=> A
58 19
                return $filter_rules;
59
            }
60 19
            elseif (OrRule::operator == $operation) {
61
                // A || True <=> True
62
                // TODO replace by true or TrueRule
63 19
                $filter_rules = null;
0 ignored issues
show
Coding Style introduced by
Consider using a different name than the parameter $filter_rules. This often makes code more readable.
Loading history...
64 19
                return $filter_rules;
65
            }
66
            else {
67
                throw new InvalidArgumentException(
68
                    "Unhandled operation '$operation'"
69
                );
70
            }
71
        }
72
73 276
        if (   3 == count($rules_description)
74 276
            && is_string($rules_description[0])
75 276
            && is_string($rules_description[1])
76 276
        ) {
77
            // Atomic rules
78 5
            $new_rule = AbstractRule::generateSimpleRule(
79 5
                $rules_description[0], // field
80 5
                $rules_description[1], // operator
81 5
                $rules_description[2], // value
82
                $options
83 5
            );
84
85 5
            $filter_rules = static::addRule($new_rule, $operation, $filter_rules);
0 ignored issues
show
Coding Style introduced by
Consider using a different name than the parameter $filter_rules. This often makes code more readable.
Loading history...
86 4
        }
87
        elseif (count($rules_description) == count(array_filter($rules_description, function($arg) {
88 272
            return $arg instanceof LogicalFilter;
89 272
        })) ) {
90
            // Already instanciated rules
91 2
            foreach ($rules_description as $i => $filter_inside_description) {
92 2
                $rules = $filter_inside_description->getRules();
93 2
                if (null !== $rules) {
94 1
                    $filter_rules = static::addRule($rules, $operation, $filter_rules);
0 ignored issues
show
Coding Style introduced by
Consider using a different name than the parameter $filter_rules. This often makes code more readable.
Loading history...
95 1
                }
96 2
            }
97 2
        }
98
        elseif (count($rules_description) == count(array_filter($rules_description, function($arg) {
99 271
            return $arg instanceof AbstractRule;
100 271
        })) ) {
101
            // Already instanciated rules
102 10
            foreach ($rules_description as $i => $new_rule) {
103 10
                $filter_rules = static::addRule($new_rule, $operation, $filter_rules);
0 ignored issues
show
Coding Style introduced by
Consider using a different name than the parameter $filter_rules. This often makes code more readable.
Loading history...
104 10
            }
105 10
        }
106 270
        elseif (1 == count($rules_description) && is_array($rules_description[0])) {
107
            if (count($rules_description[0]) == count(array_filter($rules_description[0], function($arg) {
108 270
                return $arg instanceof AbstractRule;
109 270
            })) ) {
110
                // Case of $filter->or_([AbstractRule, AbstractRule, AbstractRule, ...])
111 2
                foreach ($rules_description[0] as $i => $new_rule) {
112 2
                    $filter_rules = static::addRule($new_rule, $operation, $filter_rules);
0 ignored issues
show
Coding Style introduced by
Consider using a different name than the parameter $filter_rules. This often makes code more readable.
Loading history...
113 2
                }
114 2
            }
115
            else {
116 268
                $fake_root = new AndRule;
117
118 268
                static::addCompositeRule_recursion(
119 268
                    $rules_description[0],
120 268
                    $fake_root,
121
                    $options
122 268
                );
123
124 264
                $filter_rules = static::addRule($fake_root->getOperands()[0], $operation, $filter_rules);
0 ignored issues
show
Coding Style introduced by
Consider using a different name than the parameter $filter_rules. This often makes code more readable.
Loading history...
125
            }
126 266
        }
127
        else {
128 1
            throw new \InvalidArgumentException(
129
                "Bad set of arguments provided for rules addition: "
130 1
                .var_export($rules_description, true)
131 1
            );
132
        }
133
134 272
        return $filter_rules;
135
    }
136
137
    /**
138
     * Add one rule object to the filter
139
     *
140
     * @param AbstractRule $rule
141
     * @param string       $operation
142
     *
143
     * @return $filter_rules The updated rule tree of the filter
0 ignored issues
show
Documentation introduced by
The doc-type $filter_rules could not be parsed: Unknown type name "$filter_rules" at position 0. (view supported doc-types)

This check marks PHPDoc comments that could not be parsed by our parser. To see which comment annotations we can parse, please refer to our documentation on supported doc-types.

Loading history...
144
     */
145 272
    protected static function addRule( AbstractRule $rule, $operation=AndRule::operator, AbstractRule $filter_rules=null)
146
    {
147
        if ($filter_rules && in_array( get_class($filter_rules), [AndRule::class, OrRule::class])
148 272
            && ! $filter_rules->getOperands()) {
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class JClaveau\LogicalFilter\Rule\AbstractRule as the method getOperands() does only exist in the following sub-classes of JClaveau\LogicalFilter\Rule\AbstractRule: JClaveau\LogicalFilter\Rule\AboveOrEqualRule, JClaveau\LogicalFilter\Rule\AbstractOperationRule, JClaveau\LogicalFilter\Rule\AndRule, JClaveau\LogicalFilter\Rule\BelowOrEqualRule, JClaveau\LogicalFilter\Rule\BetweenOrEqualBothRule, JClaveau\LogicalFilter\R...BetweenOrEqualLowerRule, JClaveau\LogicalFilter\R...BetweenOrEqualUpperRule, JClaveau\LogicalFilter\Rule\BetweenRule, JClaveau\LogicalFilter\Rule\InRule, JClaveau\LogicalFilter\Rule\NotEqualRule, JClaveau\LogicalFilter\Rule\NotInRule, JClaveau\LogicalFilter\Rule\NotRule, JClaveau\LogicalFilter\Rule\OrRule. 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

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. 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.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
149 272
            throw new \LogicException(
150 1
                 "You are trying to add rules to a LogicalFilter which had "
151
                ."only contradictory rules that have already been simplified: "
152
                .$filter_rules
153 1
            );
154 1
        }
155
156
        if (null === $filter_rules) {
157 272
            return $rule;
158 271
        }
159
        elseif (($tmp_rules = $filter_rules) // $this->rules::operator not supported in PHP 5.6
160 16
            && ($tmp_rules::operator != $operation)
161 16
        ) {
162 16
            if (AndRule::operator == $operation) {
163 15
                return new AndRule([$filter_rules, $rule]);
164 10
            }
165
            elseif (OrRule::operator == $operation) {
166 6
                return new OrRule([$filter_rules, $rule]);
167 5
            }
168
            else {
169
                throw new \InvalidArgumentException(
170 1
                    "\$operation must be '".AndRule::operator."' or '".OrRule::operator
171 1
                    ."' instead of: ".var_export($operation, true)
172 1
                );
173 1
            }
174
        }
175
        else {
176
            $filter_rules->addOperand($rule);
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class JClaveau\LogicalFilter\Rule\AbstractRule as the method addOperand() does only exist in the following sub-classes of JClaveau\LogicalFilter\Rule\AbstractRule: JClaveau\LogicalFilter\Rule\AboveOrEqualRule, JClaveau\LogicalFilter\Rule\AbstractOperationRule, JClaveau\LogicalFilter\Rule\AndRule, JClaveau\LogicalFilter\Rule\BelowOrEqualRule, JClaveau\LogicalFilter\Rule\BetweenOrEqualBothRule, JClaveau\LogicalFilter\R...BetweenOrEqualLowerRule, JClaveau\LogicalFilter\R...BetweenOrEqualUpperRule, JClaveau\LogicalFilter\Rule\BetweenRule, JClaveau\LogicalFilter\Rule\InRule, JClaveau\LogicalFilter\Rule\NotEqualRule, JClaveau\LogicalFilter\Rule\NotInRule, JClaveau\LogicalFilter\Rule\NotRule, JClaveau\LogicalFilter\Rule\OrRule. 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

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. 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.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
177 6
        }
178
179
        return $filter_rules;
180 6
    }
181
182
    /**
183
     * Recursion auxiliary of addCompositeRule.
184
     *
185
     * @param array                 $rules_composition  The description of the
186
     *                                                  rules to add.
187
     * @param AbstractOperationRule $recursion_position The position in the
188
     *                                                  tree where rules must
189
     *                                                  be added.
190
     */
191
    protected static function addCompositeRule_recursion(
192 268
        array $rules_composition,
193
        AbstractOperationRule $recursion_position,
194
        $options
195
    ) {
196
        if (! array_filter($rules_composition, function ($rule_composition_part) {
197
            return is_string($rule_composition_part);
198 268
        })) {
199 268
            // at least one operator is required for operation rules
200
            throw new \InvalidArgumentException(
201 1
                "Please provide an operator for the operation: \n"
202
                .var_export($rules_composition, true)
203 1
            );
204 1
        }
205
        elseif ( 3 == count($rules_composition)
206 267
            && AbstractRule::isLeftOperand($rules_composition[0])
207 267
            && AbstractRule::isOperator($rules_composition[1])
208 267
        ) {
209 267
            // atomic or composit rules
210
            $operand_left  = $rules_composition[0];
211 264
            $operation     = $rules_composition[1];
212 264
            $operand_right = $rules_composition[2];
213 264
214
            $rule = AbstractRule::generateSimpleRule(
215 264
                $operand_left, $operation, $operand_right, $options
216 264
            );
217 264
            $recursion_position->addOperand($rule);
218 264
        }
219 264
        else {
220
            // operations
221
            if (   NotRule::operator == $rules_composition[0]
222 142
                || $rules_composition[0] == AbstractRule::findSymbolicOperator( NotRule::operator ) ) {
223 142
                $rule = new NotRule();
224 22
            }
225 22 View Code Duplication
            elseif (in_array( AndRule::operator, $rules_composition )
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

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

Loading history...
226 133
                || in_array( AbstractRule::findSymbolicOperator( AndRule::operator ), $rules_composition)) {
227 133
                $rule = new AndRule();
228 115
            }
229 115 View Code Duplication
            elseif (in_array( OrRule::operator, $rules_composition )
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

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

Loading history...
230 73
                || in_array( AbstractRule::findSymbolicOperator( OrRule::operator ), $rules_composition) ) {
231 73
                $rule = new OrRule();
232 72
            }
233 72
            else {
234
                throw new \InvalidArgumentException(
235 1
                    "A rule description seems to be an operation but do "
236
                    ."not contains a valid operator: ".var_export($rules_composition, true)
237 1
                );
238 1
            }
239
240
            $operator = $rule::operator;
241 141
242
            $operands_descriptions = array_filter(
243 141
                $rules_composition,
244 141
                function ($operand) use ($operator) {
245
                    return ! in_array($operand, [$operator, AbstractRule::findSymbolicOperator($operator)]);
246 141
                }
247
            );
248 141
249
            $non_true_rule_descriptions = array_filter(
250 141
                $operands_descriptions,
251 141
                function($operand) {
252
                    return null !== $operand  // no rule <=> true
253 140
                        || true !== $operand
254 140
                        ;
255 140
                }
256
            );
257 141
258
            foreach ($operands_descriptions as $i => $operands_description) {
259 141
                if (false === $operands_description) {
260 140
                    $operands_descriptions[ $i ] = ['and']; // FalseRule hack
261 1
                }
262 1
                elseif (null === $operands_description || true === $operands_description) {
263 140
                    $operands_description = ['and'];
264 1
                    if (empty($non_true_rule_descriptions)) {
265 1
                        throw new \LogicException(
266
                            "TrueRules are not implemented. Please add "
267
                            ."them to operations having other type of rules"
268
                        );
269
                    }
270
271
                    unset($operands_descriptions[ $i ]);
272 1
                }
273 1
            }
274 141
275
            $remaining_operations = array_filter(
276 141
                $operands_descriptions,
277 141
                function($operand) {
278 140
                    return ! is_array($operand)
279 140
                        && ! $operand instanceof AbstractRule
280 140
                        && ! $operand instanceof LogicalFilter
281 140
                        ;
282 140
                }
283
            );
284 141
285
            if (! empty($remaining_operations)) {
286 141
                throw new \InvalidArgumentException(
287 1
                    "Mixing different operations in the same rule level not implemented: \n["
288
                    . implode(', ', $remaining_operations)."]\n"
289 1
                    . 'in ' . var_export($rules_composition, true)
290 1
                );
291 1
            }
292
293
            if (NotRule::operator == $operator && 1 != count($operands_descriptions)) {
294 141
                throw new \InvalidArgumentException(
295 1
                    "Negations can have only one operand: \n"
296
                    .var_export($rules_composition, true)
297 1
                );
298 1
            }
299
300
            foreach ($operands_descriptions as $operands_description) {
301 140
                if ($operands_description instanceof AbstractRule) {
302 139
                    $rule->addOperand($operands_description);
303 1
                }
304 1
                elseif ($operands_description instanceof LogicalFilter) {
305 139
                    $rule->addOperand($operands_description->getRules());
306 2
                }
307 2
                else {
308
                    static::addCompositeRule_recursion(
309 138
                        $operands_description,
310 138
                        $rule,
311 138
                        $options
312
                    );
313 138
                }
314
            }
315 140
316
            $recursion_position->addOperand($rule);
317 139
        }
318
    }
319 265
320
    /**/
321
}
322