Merge pull request #369 from tighten/drift/add-type-support-one-line

Account for different types in visibility changes
tighten · Dec 6, 2024 · e324d1f · e324d1f
2 parents bb64e99 + a2d34c9
commit e324d1f
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 4 deletions.
diff --git a/src/Formatters/OneLineBetweenClassVisibilityChanges.php b/src/Formatters/OneLineBetweenClassVisibilityChanges.php
@@ -72,8 +72,8 @@ public function enterNode(Node $node)
                     return null;
                 }
 
-                // Ignore nodes with exactly the same visibility
-                if ($this->previousNode->flags === $node->flags) {
+                // If the two nodes are the same type and have the same visibility skip
+                if ($node::class === $this->previousNode::class && $this->previousNode->flags === $node->flags) {
                     $this->previousNode = $node;
 
                     return null;
@@ -104,6 +104,8 @@ public function enterNode(Node $node)
                     }
                 }
 
+                $this->previousNode = $node;
+
                 $node->setAttribute('comments', [new Comment(''), ...$node->getComments()]);
 
                 return $node;

diff --git a/src/Linters/OneLineBetweenClassVisibilityChanges.php b/src/Linters/OneLineBetweenClassVisibilityChanges.php
@@ -31,8 +31,8 @@ public function lint(Parser $parser)
                         continue;
                     }
 
-                    // Ignore nodes with exactly the same visibility
-                    if ($previousNode->flags === $node->flags) {
+                    // If the two nodes are the same type and have the same visibility skip
+                    if ($previousNode::class === $node::class && $previousNode->flags === $node->flags) {
                         $previousNode = $node;
 
                         continue;

diff --git a/tests/Formatting/Formatters/OneLineBetweenClassVisibilityChangesTest.php b/tests/Formatting/Formatters/OneLineBetweenClassVisibilityChangesTest.php
@@ -19,7 +19,10 @@ public function catches_missing_line_between_visibility_changes()
             class Thing
             {
                 protected const OK = 1;
+                protected const DOKEY = 1;
+                protected $bar;
                 private $ok;
+                private $foo;
             }
             file;
 
@@ -31,8 +34,12 @@ class Thing
             class Thing
             {
                 protected const OK = 1;
+                protected const DOKEY = 1;
+
+                protected $bar;
 
                 private $ok;
+                private $foo;
             }
             file;
 
@@ -56,6 +63,7 @@ class Thing
                  * The description of something.
                  */
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -72,6 +80,7 @@ class Thing
                  * The description of something.
                  */
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -96,6 +105,7 @@ class Thing
                  * The description of something.
                  */
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -117,6 +127,7 @@ class Thing
                 protected const OK = 1;
                 // Note to self
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -131,6 +142,7 @@ class Thing
 
                 // Note to self
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -153,6 +165,7 @@ class Thing
                 // Note to self
                 // Another
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -168,6 +181,7 @@ class Thing
                 // Note to self
                 // Another
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -192,6 +206,7 @@ class Thing
                 // And another
                 // And another one
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -209,6 +224,7 @@ class Thing
                 // And another
                 // And another one
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -231,6 +247,7 @@ class Thing
 
                 // TODO
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -253,6 +270,7 @@ class Thing
                 // public const NOT_OK = 2;
 
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -281,6 +299,7 @@ class Thing
                 // Hi there
                 //
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -307,6 +326,7 @@ public function getThing(): NodeVisitorAbstract
                     {
                         protected const OK = 1;
                         private $ok;
+                        private $dokey;
                     };
                 }
             }
@@ -328,6 +348,7 @@ public function getThing(): NodeVisitorAbstract
                         protected const OK = 1;
 
                         private $ok;
+                        private $dokey;
                     };
                 }
             }
@@ -337,4 +358,42 @@ public function getThing(): NodeVisitorAbstract
 
         $this->assertSame($expected, $formatted);
     }
+
+    /** @test */
+    public function catches_missing_line_between_types()
+    {
+        $file = <<<'file'
+            <?php
+
+            namespace App;
+
+            class Thing
+            {
+                public const FOO = 'bar';
+                public $bar = 'baz';
+                protected $foo = 'bar';
+                protected $bar = 'baz';
+            }
+            file;
+
+        $expected = <<<'file'
+            <?php
+
+            namespace App;
+
+            class Thing
+            {
+                public const FOO = 'bar';
+
+                public $bar = 'baz';
+
+                protected $foo = 'bar';
+                protected $bar = 'baz';
+            }
+            file;
+
+        $formatted = (new TFormat)->format(new OneLineBetweenClassVisibilityChanges($file));
+
+        $this->assertSame($expected, $formatted);
+    }
 }
diff --git a/tests/Linting/Linters/OneLineBetweenClassVisibilityChangesTest.php b/tests/Linting/Linters/OneLineBetweenClassVisibilityChangesTest.php
@@ -20,6 +20,7 @@ class Thing
             {
                 protected const OK = 1;
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -45,6 +46,7 @@ class Thing
                  * The description of something.
                  */
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -71,6 +73,7 @@ class Thing
                  * The description of something.
                  */
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -94,6 +97,7 @@ class Thing
                 protected const OK = 1;
                 // Note to self
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -118,6 +122,7 @@ class Thing
                 // Note to self
                 // Another
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -144,6 +149,7 @@ class Thing
                 // And another
                 // And another one
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -168,6 +174,7 @@ class Thing
 
                 // TODO
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -192,6 +199,7 @@ class Thing
                 // public const NOT_OK = 2;
 
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -222,6 +230,7 @@ class Thing
                 // Hi there
                 //
                 private $ok;
+                private $dokey;
             }
             file;
 
@@ -231,4 +240,29 @@ class Thing
 
         $this->assertEmpty($lints);
     }
+
+    /** @test */
+    public function catches_missing_line_between_types()
+    {
+        $file = <<<'file'
+            <?php
+
+            namespace App;
+
+            class Thing
+            {
+                protected const OK = 1;
+                protected $bar;
+                private $ok;
+                private $dokey;
+            }
+            file;
+
+        $lints = (new TLint)->lint(
+            new OneLineBetweenClassVisibilityChanges($file)
+        );
+
+        $this->assertEquals(8, $lints[0]->getNode()->getLine());
+        $this->assertEquals(9, $lints[1]->getNode()->getLine());
+    }
 }