Completed
Pull Request — master (#656)
by Juliette
01:59
created

AssignmentOrderSniff::process()   D

Complexity

Conditions 21
Paths 225

Size

Total Lines 126
Code Lines 60

Duplication

Lines 7
Ratio 5.56 %

Importance

Changes 0
Metric Value
dl 7
loc 126
rs 4.0194
c 0
b 0
f 0
cc 21
eloc 60
nc 225
nop 2

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
 * \PHPCompatibility\Sniffs\Lists\AssignmentOrderSniff.
4
 *
5
 * PHP version 7.0
6
 *
7
 * @category PHP
8
 * @package  PHPCompatibility
9
 * @author   Juliette Reinders Folmer <[email protected]>
10
 */
11
12
namespace PHPCompatibility\Sniffs\Lists;
13
14
use PHPCompatibility\Sniff;
15
16
/**
17
 * \PHPCompatibility\Sniffs\Lists\AssignmentOrderSniff.
18
 *
19
 * The list() construct no longer assigns variables in reverse order.
20
 * This affects all list constructs where non-unique variables are used.
21
 *
22
 * PHP version 7.0
23
 *
24
 * @category PHP
25
 * @package  PHPCompatibility
26
 * @author   Juliette Reinders Folmer <[email protected]>
27
 */
28
class AssignmentOrderSniff extends Sniff
29
{
30
31
    /**
32
     * Returns an array of tokens this test wants to listen for.
33
     *
34
     * @return array
35
     */
36
    public function register()
37
    {
38
        return array(
39
            T_LIST,
40
            T_OPEN_SHORT_ARRAY,
41
        );
42
    }
43
44
45
    /**
46
     * Processes this test, when one of its tokens is encountered.
47
     *
48
     * @param \PHP_CodeSniffer_File $phpcsFile The file being scanned.
49
     * @param int                   $stackPtr  The position of the current token in the
50
     *                                         stack passed in $tokens.
51
     *
52
     * @return void|int Void if not a valid list. If a list construct has been
53
     *                  examined, the stack pointer to the list closer to skip
54
     *                  passed any nested lists which don't need to be examined again.
55
     */
56
    public function process(\PHP_CodeSniffer_File $phpcsFile, $stackPtr)
57
    {
58
        if ($this->supportsAbove('7.0') === false) {
59
            return;
60
        }
61
62
        $tokens = $phpcsFile->getTokens();
63
64
        if ($tokens[$stackPtr]['code'] === T_OPEN_SHORT_ARRAY
65
            && $this->isShortList($phpcsFile, $stackPtr) === false
66
        ) {
67
            // Short array, not short list.
68
            return;
69
        }
70
71
        if ($tokens[$stackPtr]['code'] === T_LIST) {
72
            $nextNonEmpty = $phpcsFile->findNext(\PHP_CodeSniffer_Tokens::$emptyTokens, ($stackPtr + 1), null, true);
73 View Code Duplication
            if ($nextNonEmpty === false
74
                || $tokens[$nextNonEmpty]['code'] !== T_OPEN_PARENTHESIS
75
                || isset($tokens[$nextNonEmpty]['parenthesis_closer']) === false
76
            ) {
77
                // Parse error or live coding.
78
                return;
79
            }
80
81
            $opener = $nextNonEmpty;
82
            $closer = $tokens[$nextNonEmpty]['parenthesis_closer'];
83
        } else {
84
            // Short list syntax.
85
            $opener = $stackPtr;
86
87
            if (isset($tokens[$stackPtr]['bracket_closer'])) {
88
                $closer = $tokens[$stackPtr]['bracket_closer'];
89
            }
90
        }
91
92
        if (isset($opener, $closer) === false) {
93
            return;
94
        }
95
96
        /*
97
         * OK, so we have the opener & closer, now we need to check all the variables in the
98
         * list() to see if there are duplicates as that's the problem.
99
         */
100
        $hasVars = $phpcsFile->findNext(array(T_VARIABLE, T_DOLLAR), ($opener + 1), $closer);
0 ignored issues
show
Bug introduced by
The variable $closer 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

  1. 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;
        }
    }
    
  2. 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;
    }
    
  3. 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...
101
        if ($hasVars === false) {
102
            // Empty list, not our concern.
103
            return ($closer + 1);
104
        }
105
106
        // Set the variable delimiters based on the list type being examined.
107
        $stopPoints = array(T_COMMA);
108
        if ($tokens[$stackPtr]['code'] === T_OPEN_SHORT_ARRAY) {
109
            $stopPoints[] = T_CLOSE_SHORT_ARRAY;
110
        } else {
111
            $stopPoints[] = T_CLOSE_PARENTHESIS;
112
        }
113
114
        $listVars      = array();
115
        $emptyTokens   = array_flip(\PHP_CodeSniffer_Tokens::$emptyTokens);
116
        $lastStopPoint = $opener;
117
118
        /*
119
         * Create a list of all variables used within the `list()` construct.
120
         * We're not concerned with whether these are nested or not, as any duplicate
121
         * variable name used will be problematic, independent of nesting.
122
         */
123
        do {
124
            $nextStopPoint = $phpcsFile->findNext($stopPoints, ($lastStopPoint + 1), $closer);
125
            if ($nextStopPoint === false) {
126
                $nextStopPoint = $closer;
127
            }
128
129
            // Also detect this in PHP 7.1 keyed lists.
130
            $hasDoubleArrow = $phpcsFile->findNext(T_DOUBLE_ARROW, ($lastStopPoint + 1), $nextStopPoint);
131
            if ($hasDoubleArrow !== false) {
132
                $lastStopPoint = $hasDoubleArrow;
133
            }
134
135
            // Find the start of the variable, allowing for variable variables.
136
            $nextStartPoint = $phpcsFile->findNext(array(T_VARIABLE, T_DOLLAR), ($lastStopPoint + 1), $nextStopPoint);
137
            if ($nextStartPoint === false) {
138
                // Skip past empty bits in the list, i.e. `list( $a, , ,)`.
139
                $lastStopPoint = $nextStopPoint;
140
                continue;
141
            }
142
143
            /*
144
             * Gather the content of all non-empty tokens to determine the "variable name".
145
             * Variable name in this context includes array or object property syntaxes, such
146
             * as `$a['name']` and `$b->property`.
147
             */
148
            $varContent = '';
149
150
            for ($i = $nextStartPoint; $i < $nextStopPoint; $i++) {
151
                if (isset($emptyTokens[$tokens[$i]['code']])) {
152
                    continue;
153
                }
154
155
                $varContent .= $tokens[$i]['content'];
156
            }
157
158
            if ($varContent !== '') {
159
                $listVars[] = $varContent;
160
            }
161
162
            $lastStopPoint = $nextStopPoint;
163
164
        } while ($lastStopPoint < $closer);
165
166
        if (empty($listVars)) {
167
            // Shouldn't be possible, but just in case.
168
            return ($closer + 1);
169
        }
170
171
        // Verify that all variables used in the list() construct are unique.
172
        if (count($listVars) !== count(array_unique($listVars))) {
173
            $phpcsFile->addError(
174
                'list() will assign variable from left-to-right since PHP 7.0. Ensure all variables in list() are unique to prevent unexpected results.',
175
                $stackPtr,
176
                'Affected'
177
            );
178
        }
179
180
        return ($closer + 1);
181
    }
182
}
183