Merge pull request #86 from Yoast/JRF/improve-function-spacing-rule

Add new `Yoast.WhiteSpace.FunctionSpacing` sniff

Yoast/Sniffs/WhiteSpace/FunctionSpacingSniff.php

@@ -0,0 +1,76 @@
+<?php
+/**
+ * YoastCS\Yoast\Sniffs\WhiteSpace\FunctionSpacingSniff.
+ *
+ * @package Yoast\YoastCS
+ * @author  Juliette Reinders Folmer
+ * @license https://opensource.org/licenses/MIT MIT
+ */
+
+namespace YoastCS\Yoast\Sniffs\WhiteSpace;
+
+use PHP_CodeSniffer\Standards\Squiz\Sniffs\WhiteSpace\FunctionSpacingSniff as Squiz_FunctionSpacingSniff;
+use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Util\Tokens;
+
+/**
+ * Verifies the space between methods.
+ *
+ * This differs from the upstream sniff in that this sniff will ignore functions
+ * in the global namespace as those are often wrapped in an if clause which causes
+ * a fixer conflict.
+ *
+ * @package Yoast\YoastCS
+ * @author  Juliette Reinders Folmer
+ *
+ * @since   1.0.0
+ */
+class FunctionSpacingSniff extends Squiz_FunctionSpacingSniff {
+
+	/**
+	 * The number of blank lines between functions.
+	 *
+	 * {@internal Upstream sniff defaults to 2.}}
+	 *
+	 * @var integer
+	 */
+	public $spacing = 1;
+
+	/**
+	 * The number of blank lines before the first function in a class.
+	 *
+	 * {@internal Upstream sniff defaults to 2.}}
+	 *
+	 * @var integer
+	 */
+	public $spacingBeforeFirst = 1;
+
+	/**
+	 * The number of blank lines after the last function in a class.
+	 *
+	 * {@internal Upstream sniff defaults to 2.}}
+	 *
+	 * @var integer
+	 */
+	public $spacingAfterLast = 0;
+
+	/**
+	 * Processes this test, when one of its tokens is encountered.
+	 *
+	 * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
+	 * @param int                         $stackPtr  The position of the current
+	 *                                               in the stack passed in $tokens.
+	 *
+	 * @return void
+	 */
+	public function process( File $phpcsFile, $stackPtr ) {
+		$tokens = $phpcsFile->getTokens();
+
+		// Check that the function is nested in an OO structure (class, trait, interface).
+		if ( $phpcsFile->hasCondition( $stackPtr, Tokens::$ooScopeTokens ) === false ) {
+			return;
+		}
+
+		return parent::process( $phpcsFile, $stackPtr );
+	}
+}

Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.inc

@@ -0,0 +1,90 @@
+<?php
+
+/**
+ * Comment
+ */
+function foo()
+{
+}
+
+
+
+
+function func1() {
+
+}//end func1()
+function func2() {
+
+}
+
+if ( ! function_exists( 'func3' ) ) {
+	/**
+	 * Docblock.
+	 */
+	function func3() {
+	}
+}
+
+
+class MyClass
+{
+	function func1() {
+
+	}//end func1()
+
+
+
+	function func2() {
+
+	}
+	function func3() {
+
+	}
+
+	function func4() {
+
+	}
+
+}
+
+
+interface MyInterface
+{
+	function func1();
+
+
+
+	function func2();
+	function func3();
+
+	function func4();
+
+}
+
+
+trait MyTrait
+{
+
+	function func1() {
+
+	}//end func1()
+
+	function func2() {
+
+	}
+
+
+	function func3() {
+
+	}
+
+	function func4() {
+
+	}
+}
+
+$util->setLogger(new class {
+	public function a(){}
+	private function b(){}
+	protected function c(){}
+});

Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.inc.fixed

@@ -0,0 +1,90 @@
+<?php
+
+/**
+ * Comment
+ */
+function foo()
+{
+}
+
+
+
+
+function func1() {
+
+}//end func1()
+function func2() {
+
+}
+
+if ( ! function_exists( 'func3' ) ) {
+	/**
+	 * Docblock.
+	 */
+	function func3() {
+	}
+}
+
+
+class MyClass
+{
+
+	function func1() {
+
+	}//end func1()
+
+	function func2() {
+
+	}
+
+	function func3() {
+
+	}
+
+	function func4() {
+
+	}
+}
+
+
+interface MyInterface
+{
+
+	function func1();
+
+	function func2();
+
+	function func3();
+
+	function func4();
+}
+
+
+trait MyTrait
+{
+
+	function func1() {
+
+	}//end func1()
+
+	function func2() {
+
+	}
+
+	function func3() {
+
+	}
+
+	function func4() {
+
+	}
+}
+
+$util->setLogger(new class {
+
+	public function a(){}
+
+	private function b(){}
+
+	protected function c(){}
+});

Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.php

@@ -0,0 +1,50 @@
+<?php
+/**
+ * Unit test class for the Yoast Coding Standard.
+ *
+ * @package Yoast\YoastCS
+ * @license https://opensource.org/licenses/MIT MIT
+ */
+
+namespace YoastCS\Yoast\Tests\WhiteSpace;
+
+use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
+
+/**
+ * Unit test class for the FunctionSpacing sniff.
+ *
+ * @package Yoast\YoastCS
+ *
+ * @since   1.0.0
+ */
+class FunctionSpacingUnitTest extends AbstractSniffUnitTest {
+
+	/**
+	 * Returns the lines where errors should occur.
+	 *
+	 * @return array <int line number> => <int number of errors>
+	 */
+	public function getErrorList() {
+		return array(
+			31 => 1,
+			33 => 1,
+			39 => 1,
+			46 => 1,
+			53 => 2,
+			57 => 1,
+			60 => 1,
+			74 => 1,
+			87 => 2,
+			88 => 1,
+		);
+	}
+
+	/**
+	 * Returns the lines where warnings should occur.
+	 *
+	 * @return array <int line number> => <int number of warnings>
+	 */
+	public function getWarningList() {
+		return array();
+	}
+}

Yoast/ruleset.xml

@@ -71,18 +71,6 @@
 	<rule ref="Generic.PHP.LowerCaseType"/>
 	<rule ref="PSR12.Keywords.ShortFormTypeKeywords"/>
 
-	<!-- CS: standardize the number of blank lines between functions. -->
-	<rule ref="Squiz.WhiteSpace.FunctionSpacing">
-		<properties>
-			<!-- In a class, there should be 1 blank line between functions ... -->
-			<property name="spacing" value="1"/>
-			<!-- and one blank line before the first function ... -->
-			<property name="spacingBeforeFirst" value="1"/>
-			<!-- and no blank line between the last function and the class closing brace. -->
-			<property name="spacingAfterLast" value="0"/>
-		</properties>
-	</rule>
-
 	<!-- CS: no blank line between the content of a function and a function close brace.-->
 	<rule ref="PSR2.Methods.FunctionClosingBrace"/>