Repository: slevomat/coding-standard (php)

Issue:
Option to only count logical lines of code in SlevomatCodingStandard.Functions.FunctionLength ?
Was curious if you folks would be open to adding an option to only count logical lines of code in the SlevomatCodingStandard.Functions.FunctionLength` rule?

If so, I'd be up for taking a crack at a pull request. Thanks.

Pull request description:
FunctionLengthSniff / FunctionHelper:: getFunctionLengthInLines
Resolves #1285

Update Helpers/FunctionHelper::getFunctionLengthInLines() to not include comments and blank lines in line count by default

new `$flags` param allows including comments and/or blank lines

Patch:
diff --git a/README.md b/README.md
index ab57e5b7..226745dc 100644
--- a/README.md
+++ b/README.md
@@ -297,6 +297,8 @@ Reports closures not using `$this` that are not declared `static`.
 
 Disallows long functions. This sniff provides the following setting:
 
+* `includeComments`: should comments be included in the count (default value is false).
+* `includeWhitespace`: shoud empty lines be included in the count (default value is false).
 * `maxLinesLength`: specifies max allowed function lines length (default value is 20).
 
 #### SlevomatCodingStandard.PHP.DisallowDirectMagicInvokeCall 🔧
diff --git a/SlevomatCodingStandard/Helpers/FunctionHelper.php b/SlevomatCodingStandard/Helpers/FunctionHelper.php
index 2cf43427..1727c46f 100644
--- a/SlevomatCodingStandard/Helpers/FunctionHelper.php
+++ b/SlevomatCodingStandard/Helpers/FunctionHelper.php
@@ -4,6 +4,7 @@ namespace SlevomatCodingStandard\Helpers;
 
 use Generator;
 use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Util\Tokens;
 use SlevomatCodingStandard\Helpers\Annotation\ParameterAnnotation;
 use SlevomatCodingStandard\Helpers\Annotation\ReturnAnnotation;
 use function array_filter;
@@ -17,6 +18,7 @@ use function iterator_to_array;
 use function preg_match;
 use function preg_replace;
 use function sprintf;
+use function substr_count;
 use const T_ANON_CLASS;
 use const T_BITWISE_AND;
 use const T_CLASS;
@@ -32,6 +34,7 @@ use const T_STRING;
 use const T_TRAIT;
 use const T_USE;
 use const T_VARIABLE;
+use const T_WHITESPACE;
 use const T_YIELD;
 use const T_YIELD_FROM;
 
@@ -41,6 +44,9 @@ use const T_YIELD_FROM;
 class FunctionHelper
 {
 
+	public const LINE_INCLUDE_COMMENT = 1;
+	public const LINE_INCLUDE_WHITESPACE = 2;
+
 	public const SPECIAL_FUNCTIONS = [
 		'array_key_exists',
 		'array_slice',
@@ -428,19 +434,88 @@ class FunctionHelper
 		);
 	}
 
-	public static function getFunctionLengthInLines(File $file, int $position): int
+	/**
+	 * @param File $file
+	 * @param int $functionPosition
+	 * @param int $flags optional bitmask of self::LINE_INCLUDE_* constants
+	 */
+	public static function getFunctionLengthInLines(File $file, int $functionPosition, int $flags = 0): int
 	{
-		$tokens = $file->getTokens();
-		$token = $tokens[$position];
-
-		if (self::isAbstract($file, $position)) {
+		if (self::isAbstract($file, $functionPosition)) {
 			return 0;
 		}
 
-		$firstToken = $tokens[$token['scope_opener']];
-		$lastToken = $tokens[$token['scope_closer']];
+		$includeWhitespace = ($flags & self::LINE_INCLUDE_WHITESPACE) === self::LINE_INCLUDE_WHITESPACE;
+		$includeComments = ($flags & self::LINE_INCLUDE_COMMENT) === self::LINE_INCLUDE_COMMENT;
 
-		return $lastToken['line'] - $firstToken['line'] - 1;
+		$tokens = $file->getTokens();
+		$token = $tokens[$functionPosition];
+
+		$tokenOpenerPosition = $token['scope_opener'];
+		$tokenCloserPosition = $token['scope_closer'];
+		$tokenOpenerLine = $tokens[$tokenOpenerPosition]['line'];
+		$tokenCloserLine = $tokens[$tokenCloserPosition]['line'];
+
+		$lineCount = 0;
+		$lastCommentLine = null;
+		$previousIncludedPosition = null;
+
+		for ($position = $tokenOpenerPosition; $position <= $tokenCloserPosition - 1; $position++) {
+			$token = $tokens[$position];
+			if ($includeComments === false) {
+				if (in_array($token['code'], Tokens::$commentTokens, true)) {
+					if (
+						$previousIncludedPosition !== null &&
+						substr_count($token['content'], $file->eolChar) > 0 &&
+						$token['line'] === $tokens[$previousIncludedPosition]['line']
+					) {
+						// Comment with linebreak starting on same line as included Token
+						$lineCount++;
+					}
+					// Don't include comment
+					$lastCommentLine = $token['line'];
+					continue;
+				}
+				if (
+					$previousIncludedPosition !== null &&
+					$token['code'] === T_WHITESPACE &&
+					$token['line'] === $lastCommentLine &&
+					$token['line'] !== $tokens[$previousIncludedPosition]['line']
+				) {
+					// Whitespace after block comment... still on comment line...
+					// Ignore along with the comment
+					continue;
+				}
+			}
+			if ($token['code'] === T_WHITESPACE) {
+				$nextNonWhitespacePosition = $file->findNext(T_WHITESPACE, $position + 1, $tokenCloserPosition + 1, true);
+				if (
+					$includeWhitespace === false &&
+					$token['column'] === 1 &&
+					$nextNonWhitespacePosition !== false &&
+					$tokens[$nextNonWhitespacePosition]['line'] !== $token['line']
+				) {
+					// This line is nothing but whitepace
+					$position = $nextNonWhitespacePosition - 1;
+					continue;
+				}
+				if ($previousIncludedPosition === $tokenOpenerPosition && $token['line'] === $tokenOpenerLine) {
+					// Don't linclude line break after opening "{"
+					// Unless there was code or an (included) comment following the "{"
+					continue;
+				}
+			}
+			if ($token['code'] !== T_WHITESPACE) {
+				$previousIncludedPosition = $position;
+			}
+			$newLineFoundCount = substr_count($token['content'], $file->eolChar);
+			$lineCount += $newLineFoundCount;
+		}
+		if ($tokens[$previousIncludedPosition]['line'] === $tokenCloserLine) {
+			// There is code or comment on the closing "}" line...
+			$lineCount++;
+		}
+		return $lineCount;
 	}
 
 	/**
diff --git a/SlevomatCodingStandard/Sniffs/Functions/FunctionLengthSniff.php b/SlevomatCodingStandard/Sniffs/Functions/FunctionLengthSniff.php
index 51728646..4d0b66d3 100644
--- a/SlevomatCodingStandard/Sniffs/Functions/FunctionLengthSniff.php
+++ b/SlevomatCodingStandard/Sniffs/Functions/FunctionLengthSniff.php
@@ -6,6 +6,9 @@ use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
 use SlevomatCodingStandard\Helpers\FunctionHelper;
 use SlevomatCodingStandard\Helpers\SniffSettingsHelper;
+use function array_filter;
+use function array_keys;
+use function array_reduce;
 use function sprintf;
 use const T_FUNCTION;
 
@@ -17,6 +20,12 @@ class FunctionLengthSniff implements Sniff
 	/** @var int */
 	public $maxLinesLength = 20;
 
+	/** @var bool */
+	public $includeComments = false;
+
+	/** @var bool */
+	public $includeWhitespace = false;
+
 	/**
 	 * @return array<int, (int|string)>
 	 */
@@ -32,7 +41,15 @@ class FunctionLengthSniff implements Sniff
 	 */
 	public function process(File $file, $functionPointer): void
 	{
-		$length = FunctionHelper::getFunctionLengthInLines($file, $functionPointer);
+		$flags = array_keys(array_filter([
+			FunctionHelper::LINE_INCLUDE_COMMENT => $this->includeComments,
+			FunctionHelper::LINE_INCLUDE_WHITESPACE => $this->includeWhitespace,
+		]));
+		$flags = array_reduce($flags, static function ($carry, $flag): int {
+			return $carry | $flag;
+		}, 0);
+
+		$length = FunctionHelper::getFunctionLengthInLines($file, $functionPointer, $flags);
 
 		if ($length <= SniffSettingsHelper::normalizeInteger($this->maxLinesLength)) {
 			return;


Test patch:
diff --git a/tests/Helpers/FunctionHelperTest.php b/tests/Helpers/FunctionHelperTest.php
index f344e0e1..4f6eebf6 100644
--- a/tests/Helpers/FunctionHelperTest.php
+++ b/tests/Helpers/FunctionHelperTest.php
@@ -440,6 +440,29 @@ class FunctionHelperTest extends TestCase
 		self::assertSame(['foo', 'boo'], FunctionHelper::getAllFunctionNames($phpcsFile));
 	}
 
+	public function testGetFunctionLengthInLines(): void
+	{
+		$phpcsFile = $this->getCodeSnifferFile(__DIR__ . '/data/functionLength.php');
+		$functionPointer = $this->findFunctionPointerByName($phpcsFile, 'countMe');
+
+		self::assertSame(2, FunctionHelper::getFunctionLengthInLines($phpcsFile, $functionPointer));
+		self::assertSame(7, FunctionHelper::getFunctionLengthInLines(
+			$phpcsFile,
+			$functionPointer,
+			FunctionHelper::LINE_INCLUDE_COMMENT
+		));
+		self::assertSame(3, FunctionHelper::getFunctionLengthInLines(
+			$phpcsFile,
+			$functionPointer,
+			FunctionHelper::LINE_INCLUDE_WHITESPACE
+		));
+		self::assertSame(8, FunctionHelper::getFunctionLengthInLines(
+			$phpcsFile,
+			$functionPointer,
+			FunctionHelper::LINE_INCLUDE_COMMENT | FunctionHelper::LINE_INCLUDE_WHITESPACE
+		));
+	}
+
 	public function testFindClassPointer(): void
 	{
 		$phpcsFile = $this->getCodeSnifferFile(__DIR__ . '/data/functionNames.php');
diff --git a/tests/Helpers/data/functionLength.php b/tests/Helpers/data/functionLength.php
new file mode 100644
index 00000000..ac7fb534
--- /dev/null
+++ b/tests/Helpers/data/functionLength.php
@@ -0,0 +1,16 @@
+<?php
+
+class FooClass
+{
+
+	public function countMe(): string
+	{
+		/*
+		   block comment
+		*/
+		1 + 1; // slash comment
+		# hash comment
+
+		return 'This is the only line that matters (by default)'; /* inline comment */
+	/* inline comment */ }
+}
diff --git a/tests/Sniffs/Functions/FunctionLengthSniffTest.php b/tests/Sniffs/Functions/FunctionLengthSniffTest.php
index 3bb39cd9..b26c8342 100644
--- a/tests/Sniffs/Functions/FunctionLengthSniffTest.php
+++ b/tests/Sniffs/Functions/FunctionLengthSniffTest.php
@@ -19,7 +19,35 @@ class FunctionLengthSniffTest extends TestCase
 
 		self::assertSame(1, $report->getErrorCount());
 
-		self::assertSniffError($report, 3, FunctionLengthSniff::CODE_FUNCTION_LENGTH);
+		self::assertSniffError($report, 3, FunctionLengthSniff::CODE_FUNCTION_LENGTH, 'Currently using 21 lines');
+	}
+
+	public function testWithComments(): void
+	{
+		$report = self::checkFile(
+			__DIR__ . '/data/functionLengthErrors.php',
+			[
+				'includeComments' => true,
+			]
+		);
+
+		self::assertSame(1, $report->getErrorCount());
+
+		self::assertSniffError($report, 3, FunctionLengthSniff::CODE_FUNCTION_LENGTH, 'Currently using 24 lines');
+	}
+
+	public function testWithWhitespace(): void
+	{
+		$report = self::checkFile(
+			__DIR__ . '/data/functionLengthErrors.php',
+			[
+				'includeWhitespace' => true,
+			]
+		);
+
+		self::assertSame(1, $report->getErrorCount());
+
+		self::assertSniffError($report, 3, FunctionLengthSniff::CODE_FUNCTION_LENGTH, 'Currently using 22 lines');
 	}
 
 }
diff --git a/tests/Sniffs/Functions/data/functionLengthErrors.php b/tests/Sniffs/Functions/data/functionLengthErrors.php
index aea29a1c..2755090f 100644
--- a/tests/Sniffs/Functions/data/functionLengthErrors.php
+++ b/tests/Sniffs/Functions/data/functionLengthErrors.php
@@ -2,6 +2,10 @@
 
 function dummyFunctionWithTooManyLines()
 {
+	/*
+		Block comment
+	*/
+
 	echo 'line 1';
 	echo 'line 2';
 	echo 'line 3';