From 4ce821efd67ccc17f156e8829884a7c4af3319fb Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Tue, 31 Dec 2019 02:01:04 +0100
Subject: [PATCH] Squiz/DisallowMultipleAssignments: fix ignoring of property
 declarations

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
---
 .../PHP/DisallowMultipleAssignmentsSniff.php   | 18 ++++++++++--------
 .../DisallowMultipleAssignmentsUnitTest.inc    | 13 +++++++++++++
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/Standards/Squiz/Sniffs/PHP/DisallowMultipleAssignmentsSniff.php b/src/Standards/Squiz/Sniffs/PHP/DisallowMultipleAssignmentsSniff.php
index 058db55656..84032fec01 100644
--- a/src/Standards/Squiz/Sniffs/PHP/DisallowMultipleAssignmentsSniff.php
+++ b/src/Standards/Squiz/Sniffs/PHP/DisallowMultipleAssignmentsSniff.php
@@ -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
@@ -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.
diff --git a/src/Standards/Squiz/Tests/PHP/DisallowMultipleAssignmentsUnitTest.inc b/src/Standards/Squiz/Tests/PHP/DisallowMultipleAssignmentsUnitTest.inc
index f284b44878..98c1bb5661 100644
--- a/src/Standards/Squiz/Tests/PHP/DisallowMultipleAssignmentsUnitTest.inc
+++ b/src/Standards/Squiz/Tests/PHP/DisallowMultipleAssignmentsUnitTest.inc
@@ -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;
+}