Fixed curly sniff to also add violations when multi-line curly incorr…

…ect (#25)
hostnet · Nov 6, 2017 · 4233cd9 · 4233cd9
1 parent a6d712d
commit 4233cd9
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 16 deletions.
diff --git a/src/Sniff/CurlySniff.php b/src/Sniff/CurlySniff.php
@@ -106,6 +106,40 @@ public function process(File $file, int $stack_ptr): void
                 $token->offsets[0] + strlen($token->chars)
             );
             return;
+        } else {
+            try {
+                $whitespace = $file->get($stack_ptr + 1)->chars;
+            } catch (\OutOfRangeException $e) {
+                return;
+            }
+
+            // Make sure there is a return after the first curly.
+            if (false === strpos($whitespace, "\n")) {
+                $file->addViolation(
+                    self::class,
+                    'Statement found on same line as opening bracket when expected on new line.',
+                    $token->lines[0],
+                    $token->offsets[0],
+                    $token->offsets[0] + strlen($token->chars)
+                );
+                return;
+            }
+
+            // Make sure there is a return before the end of the last curly.
+            $whitespace = $file->get($closing_curly_index - 1)->chars;
+
+            if (false === strpos($whitespace, "\n")) {
+                $close_curly = $file->get($closing_curly_index);
+
+                $file->addViolation(
+                    self::class,
+                    'Statement found on same line as closing bracket when expected on new line.',
+                    $close_curly->lines[0],
+                    $close_curly->offsets[0],
+                    $close_curly->offsets[0] + strlen($close_curly->chars)
+                );
+                return;
+            }
         }
     }
 

diff --git a/test/Configuration/StandardConfigurationTest.php b/test/Configuration/StandardConfigurationTest.php
@@ -31,22 +31,24 @@ public function testGetFile()
         }, $this->null_configuration->getFiles());
 
         sort($files);
+
+        $ds = DIRECTORY_SEPARATOR;
 
         self::assertEquals([
-            dirname(__DIR__) . '/Configuration/test.less',
-            dirname(__DIR__) . '/Sniff/fixtures/args.less',
-            dirname(__DIR__) . '/Sniff/fixtures/bad_class.less',
-            dirname(__DIR__) . '/Sniff/fixtures/bad_colors.less',
-            dirname(__DIR__) . '/Sniff/fixtures/bad_variable.less',
-            dirname(__DIR__) . '/Sniff/fixtures/color_variants.less',
-            dirname(__DIR__) . '/Sniff/fixtures/comments.less',
-            dirname(__DIR__) . '/Sniff/fixtures/curly.less',
-            dirname(__DIR__) . '/Sniff/fixtures/empty.less',
-            dirname(__DIR__) . '/Sniff/fixtures/generated_class.less',
-            dirname(__DIR__) . '/Sniff/fixtures/indent.less',
-            dirname(__DIR__) . '/Sniff/fixtures/newlines.less',
-            dirname(__DIR__) . '/Sniff/fixtures/quotes.less',
-            dirname(__DIR__) . '/Sniff/fixtures/semicolon.less',
+            dirname(__DIR__) . $ds . 'Configuration'. $ds . 'test.less',
+            dirname(__DIR__) . $ds . 'Sniff'. $ds . 'fixtures' . $ds. 'args.less',
+            dirname(__DIR__) . $ds . 'Sniff'. $ds . 'fixtures' . $ds. 'bad_class.less',
+            dirname(__DIR__) . $ds . 'Sniff'. $ds . 'fixtures' . $ds. 'bad_colors.less',
+            dirname(__DIR__) . $ds . 'Sniff'. $ds . 'fixtures' . $ds. 'bad_variable.less',
+            dirname(__DIR__) . $ds . 'Sniff'. $ds . 'fixtures' . $ds. 'color_variants.less',
+            dirname(__DIR__) . $ds . 'Sniff'. $ds . 'fixtures' . $ds. 'comments.less',
+            dirname(__DIR__) . $ds . 'Sniff'. $ds . 'fixtures' . $ds. 'curly.less',
+            dirname(__DIR__) . $ds . 'Sniff'. $ds . 'fixtures' . $ds. 'empty.less',
+            dirname(__DIR__) . $ds . 'Sniff'. $ds . 'fixtures' . $ds. 'generated_class.less',
+            dirname(__DIR__) . $ds . 'Sniff'. $ds . 'fixtures' . $ds. 'indent.less',
+            dirname(__DIR__) . $ds . 'Sniff'. $ds . 'fixtures' . $ds. 'newlines.less',
+            dirname(__DIR__) . $ds . 'Sniff'. $ds . 'fixtures' . $ds. 'quotes.less',
+            dirname(__DIR__) . $ds . 'Sniff'. $ds . 'fixtures' . $ds. 'semicolon.less',
         ], $files);
     }
 

diff --git a/test/Sniff/CurlySniffTest.php b/test/Sniff/CurlySniffTest.php
@@ -40,8 +40,22 @@ public function testSniff()
             new Violation(CurlySniff::class, 'Closing curly bracket should be proceeded by only one space.', 8, 27, 28),
             new Violation(CurlySniff::class, 'Multiple statements found on one line.', 9, 13, 57),
             new Violation(CurlySniff::class, 'One statements found and should be on one line.', 10, 13, 14),
-            new Violation(CurlySniff::class, 'Media query should always be one multiple lines.', 30, 50, 51),
-            new Violation(CurlySniff::class, 'Media query should always be one multiple lines.', 31, 52, 53),
+            new Violation(
+                CurlySniff::class,
+                'Statement found on same line as opening bracket when expected on new line.',
+                13,
+                13,
+                14
+            ),
+            new Violation(
+                CurlySniff::class,
+                'Statement found on same line as closing bracket when expected on new line.',
+                18,
+                30,
+                31
+            ),
+            new Violation(CurlySniff::class, 'Media query should always be one multiple lines.', 36, 50, 51),
+            new Violation(CurlySniff::class, 'Media query should always be one multiple lines.', 37, 52, 53),
         ], $file->getViolations());
     }
 }
diff --git a/test/Sniff/fixtures/curly.less b/test/Sniff/fixtures/curly.less
@@ -10,6 +10,12 @@
 .bad-multi2 {
     color: #ffffff;
 }
+.bad-multi3 { color: #ffffff;
+    font-family: sans-serif;
+}
+.bad-multi4 {
+    color: #ffffff;
+    font-family: sans-serif; }
 .ignore {}
 .valid { background-@{property}: #999; }
 @media only screen and (min-device-width: 320px) and (max-device-width: 480px) {