Skip to content

Commit

Permalink
Squiz/DisallowMultipleAssignments: fix ignoring of property declarations
Browse files Browse the repository at this point in the history
The `Squiz.PHP.DisallowMultipleAssignments` intended to ignore property declarations, but did not allow for typed properties.

I've now moved the "is this a property ?" check up to bow out earlier for all properties.

Includes unit test.

I've also added tests for multi-property declarations. While it is debatable whether or not this sniff should report on these, the existing behaviour was to ignore them. This behaviour has been maintained and is now documented and safeguarded via the test.

Fixes 2787
  • Loading branch information
jrfnl committed Dec 31, 2019
1 parent 90b719d commit 4ce821e
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,16 @@ public function process(File $phpcsFile, $stackPtr)
}
}

// Ignore member var definitions.
if (empty($tokens[$stackPtr]['conditions']) === false) {
$conditions = $tokens[$stackPtr]['conditions'];
end($conditions);
$deepestScope = key($conditions);
if (isset(Tokens::$ooScopeTokens[$tokens[$deepestScope]['code']]) === true) {
return;
}
}

/*
The general rule is:
Find an equal sign and go backwards along the line. If you hit an
Expand Down Expand Up @@ -124,14 +134,6 @@ public function process(File $phpcsFile, $stackPtr)
$varToken = $start;
}

// Ignore member var definitions.
if (isset(Tokens::$scopeModifiers[$tokens[$varToken]['code']]) === true
|| $tokens[$varToken]['code'] === T_VAR
|| $tokens[$varToken]['code'] === T_STATIC
) {
return;
}

// Ignore the first part of FOR loops as we are allowed to
// assign variables there even though the variable is not the
// first thing on the line.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,16 @@ $closureWithDefaultParamter = function(array $testArray=array()) {};
while ( ( $csvdata = fgetcsv( $handle, 2000, $separator ) ) !== false ) {
// Do something.
}

// Issue #2787.
class Foo {
protected ?int $id = null;
}

class Bar
{
// Multi-property assignments.
private $a = false, $b = true; // Non-typed.
public bool $c = false, $d = true;
protected int $e = 123, $f = 987;
}

0 comments on commit 4ce821e

Please sign in to comment.