New Feature: No new lines between properties

PHP-CS-Fixer · Mar 20, 2020 · 8cd3cf8 · 8cd3cf8
1 parent f71cee9
commit 8cd3cf8
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 26 deletions.
diff --git a/README.rst b/README.rst
@@ -364,14 +364,14 @@ Choose from the list of available rules:
 
 * **class_attributes_separation** [@Symfony, @PhpCsFixer]
 
-  Class, trait and interface elements must be separated with one blank
-  line.
+  Class, trait and interface elements must be separated with one or no
+  blank line.
 
   Configuration options:
 
-  - ``elements`` (a subset of ``['const', 'method', 'property']``): list of classy
-    elements; 'const', 'method', 'property'; defaults to ``['const',
-    'method', 'property']``
+  - ``elements`` (``array``): dictionary of ``const|method|property`` => ``one|none``
+    values; defaults to ``['const' => 'one', 'property' => 'one', 'method' =>
+    'one']``
 
 * **class_definition** [@PSR2, @Symfony, @PhpCsFixer]
 

diff --git a/src/Fixer/ClassNotation/ClassAttributesSeparationFixer.php b/src/Fixer/ClassNotation/ClassAttributesSeparationFixer.php
@@ -15,7 +15,6 @@
 use PhpCsFixer\AbstractFixer;
 use PhpCsFixer\Fixer\ConfigurationDefinitionFixerInterface;
 use PhpCsFixer\Fixer\WhitespacesAwareFixerInterface;
-use PhpCsFixer\FixerConfiguration\AllowedValueSubset;
 use PhpCsFixer\FixerConfiguration\FixerConfigurationResolver;
 use PhpCsFixer\FixerConfiguration\FixerOptionBuilder;
 use PhpCsFixer\FixerDefinition\CodeSample;
@@ -26,6 +25,7 @@
 use PhpCsFixer\Tokenizer\Tokens;
 use PhpCsFixer\Tokenizer\TokensAnalyzer;
 use SplFileInfo;
+use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException;
 
 /**
  * Make sure there is one blank line above and below class elements.
@@ -36,10 +36,15 @@
  */
 final class ClassAttributesSeparationFixer extends AbstractFixer implements ConfigurationDefinitionFixerInterface, WhitespacesAwareFixerInterface
 {
+    const SPACING_ONE = 'one';
+    const SPACING_NONE = 'none';
+
     /**
      * @var array<string, true>
      */
     private $classElementTypes = [];
+    private static $supportedSpacings = [self::SPACING_NONE, self::SPACING_ONE];
+    private static $supportedTypes = ['const', 'property', 'method'];
 
     /**
      * {@inheritdoc}
@@ -49,8 +54,8 @@ public function configure(array $configuration = null)
         parent::configure($configuration);
 
         $this->classElementTypes = []; // reset previous configuration
-        foreach ($this->configuration['elements'] as $element) {
-            $this->classElementTypes[$element] = true;
+        foreach ($this->configuration['elements'] as $elementType => $spacing) {
+            $this->classElementTypes[$elementType] = $spacing;
         }
     }
 
@@ -60,7 +65,7 @@ public function configure(array $configuration = null)
     public function getDefinition()
     {
         return new FixerDefinition(
-            'Class, trait and interface elements must be separated with one blank line.',
+            'Class, trait and interface elements must be separated with one or no blank line.',
             [
                 new CodeSample(
                     '<?php
@@ -85,7 +90,7 @@ class Sample
     private $b;
 }
 ',
-                    ['elements' => ['property']]
+                    ['elements' => ['property' => self::SPACING_ONE]]
                 ),
                 new CodeSample(
                     '<?php
@@ -96,7 +101,7 @@ class Sample
     const B = 3600;
 }
 ',
-                    ['elements' => ['const']]
+                    ['elements' => ['const' => self::SPACING_ONE]]
                 ),
             ]
         );
@@ -134,6 +139,8 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens)
                 continue; // not configured to be fixed
             }
 
+            $spacing = $this->classElementTypes[$element['type']];
+
             if ($element['classIndex'] !== $class) {
                 $class = $element['classIndex'];
                 $classStart = $tokens->getNextTokenOfKind($class, ['{']);
@@ -156,7 +163,7 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens)
             }
 
             // `const`, `property` or `method` of an `interface`
-            $this->fixSpaceBelowClassElement($tokens, $classEnd, $tokens->getNextTokenOfKind($index, [';']));
+            $this->fixSpaceBelowClassElement($tokens, $classEnd, $tokens->getNextTokenOfKind($index, [';']), $spacing);
             $this->fixSpaceAboveClassElement($tokens, $classStart, $index);
         }
     }
@@ -166,13 +173,40 @@ protected function applyFix(SplFileInfo $file, Tokens $tokens)
      */
     protected function createConfigurationDefinition()
     {
-        $types = ['const', 'method', 'property'];
-
         return new FixerConfigurationResolver([
-            (new FixerOptionBuilder('elements', sprintf('List of classy elements; \'%s\'.', implode("', '", $types))))
+            (new FixerOptionBuilder('elements', 'Dictionary of `const|method|property` => `one|none` values.'))
                 ->setAllowedTypes(['array'])
-                ->setAllowedValues([new AllowedValueSubset($types)])
-                ->setDefault(['const', 'method', 'property'])
+                ->setAllowedValues([static function ($option) {
+                    foreach ($option as $type => $spacing) {
+                        if (!\in_array($type, self::$supportedTypes, true)) {
+                            throw new InvalidOptionsException(
+                                sprintf(
+                                    'Unexpected element type, expected any of "%s", got "%s".',
+                                    implode('", "', self::$supportedTypes),
+                                    \is_object($type) ? \get_class($type) : \gettype($type).'#'.$type
+                                )
+                            );
+                        }
+
+                        if (!\in_array($spacing, self::$supportedSpacings, true)) {
+                            throw new InvalidOptionsException(
+                                sprintf(
+                                    'Unexpected spacing for element type "%s", expected any of "%s", got "%s".',
+                                    $spacing,
+                                    implode('", "', self::$supportedSpacings),
+                                    \is_object($spacing) ? \get_class($spacing) : (null === $spacing ? 'null' : \gettype($spacing).'#'.$spacing)
+                                )
+                            );
+                        }
+                    }
+
+                    return true;
+                }])
+                ->setDefault([
+                    'const' => self::SPACING_ONE,
+                    'property' => self::SPACING_ONE,
+                    'method' => self::SPACING_ONE,
+                ])
                 ->getOption(),
         ]);
     }
@@ -183,10 +217,11 @@ protected function createConfigurationDefinition()
      * Deals with comments, PHPDocs and spaces above the element with respect to the position of the
      * element within the class, interface or trait.
      *
-     * @param int $classEndIndex
-     * @param int $elementEndIndex
+     * @param int   $classEndIndex
+     * @param int   $elementEndIndex
+     * @param mixed $spacing
      */
-    private function fixSpaceBelowClassElement(Tokens $tokens, $classEndIndex, $elementEndIndex)
+    private function fixSpaceBelowClassElement(Tokens $tokens, $classEndIndex, $elementEndIndex, $spacing)
     {
         for ($nextNotWhite = $elementEndIndex + 1;; ++$nextNotWhite) {
             if (($tokens[$nextNotWhite]->isComment() || $tokens[$nextNotWhite]->isWhitespace()) && false === strpos($tokens[$nextNotWhite]->getContent(), "\n")) {
@@ -200,7 +235,13 @@ private function fixSpaceBelowClassElement(Tokens $tokens, $classEndIndex, $elem
             $nextNotWhite = $tokens->getNextNonWhitespace($nextNotWhite);
         }
 
-        $this->correctLineBreaks($tokens, $elementEndIndex, $nextNotWhite, $nextNotWhite === $classEndIndex ? 1 : 2);
+        if ($tokens[$nextNotWhite]->isGivenKind(T_FUNCTION)) {
+            $this->correctLineBreaks($tokens, $elementEndIndex, $nextNotWhite, 2);
+
+            return;
+        }
+
+        $this->correctLineBreaks($tokens, $elementEndIndex, $nextNotWhite, $nextNotWhite === $classEndIndex || self::SPACING_NONE === $spacing ? 1 : 2);
     }
 
     /**

diff --git a/src/Fixer/ClassNotation/MethodSeparationFixer.php b/src/Fixer/ClassNotation/MethodSeparationFixer.php
@@ -77,7 +77,7 @@ public function getSuccessorsNames()
     protected function createProxyFixers()
     {
         $fixer = new ClassAttributesSeparationFixer();
-        $fixer->configure(['elements' => ['method']]);
+        $fixer->configure(['elements' => ['method' => ClassAttributesSeparationFixer::SPACING_ONE]]);
 
         return [$fixer];
     }

diff --git a/src/RuleSet.php b/src/RuleSet.php
@@ -70,7 +70,7 @@ final class RuleSet implements RuleSetInterface
                 'allow_single_line_closure' => true,
             ],
             'cast_spaces' => true,
-            'class_attributes_separation' => ['elements' => ['method']],
+            'class_attributes_separation' => ['elements' => ['method' => 'one']],
             'class_definition' => ['single_line' => true],
             'concat_space' => true,
             'declare_equal_normalize' => true,

diff --git a/tests/Fixer/ClassNotation/ClassAttributesSeparationFixerTest.php b/tests/Fixer/ClassNotation/ClassAttributesSeparationFixerTest.php
@@ -12,6 +12,7 @@
 
 namespace PhpCsFixer\Tests\Fixer\ClassNotation;
 
+use PhpCsFixer\Fixer\ClassNotation\ClassAttributesSeparationFixer;
 use PhpCsFixer\Tests\Test\AbstractFixerTestCase;
 use PhpCsFixer\Tokenizer\Tokens;
 use PhpCsFixer\WhitespacesFixerConfig;
@@ -936,7 +937,29 @@ class A
 
                     function A(){}}
                 ',
-                ['elements' => ['property']],
+                ['elements' => ['property' => ClassAttributesSeparationFixer::SPACING_ONE]],
+            ],
+            [
+                '<?php
+                    class A
+                    {
+                        private $a = null;
+                        public $b = 1;
+
+                    function A(){}}
+                ',
+                '<?php
+                    class A
+                    {
+                        private $a = null;
+
+                        public $b = 1;
+
+
+
+                    function A(){}}
+                ',
+                ['elements' => ['property' => ClassAttributesSeparationFixer::SPACING_NONE]],
             ],
             [
                 '<?php
@@ -957,7 +980,7 @@ class A
                         const THREE = ONE + self::TWO; /* test */ # test
                         const B = 2;}
                 ',
-                ['elements' => ['const']],
+                ['elements' => ['const' => 'one']],
             ],
         ];
     }
@@ -1044,7 +1067,7 @@ public function pass($a, $b) {
     public function testFix71($expected, $input = null)
     {
         $this->fixer->configure([
-            'elements' => ['method', 'const'],
+            'elements' => ['method' => ClassAttributesSeparationFixer::SPACING_ONE, 'const' => ClassAttributesSeparationFixer::SPACING_ONE],
         ]);
         $this->doTest($expected, $input);
     }