CS/QA: various small tweaks (#256)

* CS/QA: use single quotes for text strings without embedded variables

* CS/QA: always have a comma after each item in multi-line arrays

* CS/QA: use `self` to refer to current class

* CS/QA: no need for parentheses with require/include

These are language constructs, not functions, so the parentheses are unnecessary.

* CS/QA: consistently use short arrays

This fixes up the one instance where there was still a long array used in the code base.

* CS/QA: use strict comparisons

The `Helper::findContainingOpeningBracket()` method can return either an integer or `null`, so this comparison should be a strict comparison.

* CS/QA: use pre-increment

... instead of post-increment as it is less prone to surprising results if code is moved around.

Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>

Author: jrfnl

Tests/VariableAnalysisSniff/VariableAnalysisTest.php

@@ -465,7 +465,7 @@ public function testClassReferenceWarnings() {
 			22,
 			23,
 			24,
-			25
+			25,
 		];
 		$this->assertSame($expectedWarnings, $lines);
 	}
@@ -837,7 +837,7 @@ public function testValidUndefinedVariableNamesIgnoresUndefinedProperties() {
 			22,
 			23,
 			24,
-			25
+			25,
 		];
 		$this->assertSame($expectedWarnings, $lines);
 	}
@@ -856,7 +856,7 @@ public function testValidUndefinedVariableRegexpIgnoresUndefinedProperties() {
 			12,
 			13,
 			24,
-			25
+			25,
 		];
 		$this->assertSame($expectedWarnings, $lines);
 	}

Tests/bootstrap.php

@@ -1,3 +1,3 @@
 <?php
-require_once(__DIR__ . '/../vendor/squizlabs/php_codesniffer/tests/bootstrap.php');
-require_once(__DIR__ . '/BaseTestCase.php');
+require_once __DIR__ . '/../vendor/squizlabs/php_codesniffer/tests/bootstrap.php';
+require_once __DIR__ . '/BaseTestCase.php';

VariableAnalysis/Lib/Helpers.php

@@ -244,7 +244,7 @@ public static function getUseIndexForUseImport(File $phpcsFile, $stackPtr) {
 	public static function findFunctionCall(File $phpcsFile, $stackPtr) {
 		$tokens = $phpcsFile->getTokens();
 
-		$openPtr = Helpers::findContainingOpeningBracket($phpcsFile, $stackPtr);
+		$openPtr = self::findContainingOpeningBracket($phpcsFile, $stackPtr);
 		if (is_int($openPtr)) {
 			// First non-whitespace thing and see if it's a T_STRING function name
 			$functionPtr = $phpcsFile->findPrevious(Tokens::$emptyTokens, $openPtr - 1, null, true, null, true);
@@ -267,7 +267,7 @@ public static function findFunctionCallArguments(File $phpcsFile, $stackPtr) {
 		// Slight hack: also allow this to find args for array constructor.
 		if (($tokens[$stackPtr]['code'] !== T_STRING) && ($tokens[$stackPtr]['code'] !== T_ARRAY)) {
 			// Assume $stackPtr is something within the brackets, find our function call
-			$stackPtr = Helpers::findFunctionCall($phpcsFile, $stackPtr);
+			$stackPtr = self::findFunctionCall($phpcsFile, $stackPtr);
 			if ($stackPtr === null) {
 				return [];
 			}
@@ -289,7 +289,7 @@ public static function findFunctionCallArguments(File $phpcsFile, $stackPtr) {
 		$lastArgComma = $openPtr;
 		$nextPtr = $phpcsFile->findNext([T_COMMA], $lastPtr + 1, $closePtr);
 		while (is_int($nextPtr)) {
-			if (Helpers::findContainingOpeningBracket($phpcsFile, $nextPtr) == $openPtr) {
+			if (self::findContainingOpeningBracket($phpcsFile, $nextPtr) === $openPtr) {
 				// Comma is at our level of brackets, it's an argument delimiter.
 				$range = range($lastArgComma + 1, $nextPtr - 1);
 				$range = array_filter($range, function ($element) {
@@ -648,7 +648,7 @@ public static function getListAssignments(File $phpcsFile, $listOpenerIndex) {
 			$parents = isset($tokens[$listOpenerIndex]['nested_parenthesis']) ? $tokens[$listOpenerIndex]['nested_parenthesis'] : [];
 			// There's no record of nested brackets for short lists; we'll have to find the parent ourselves
 			if (empty($parents)) {
-				$parentSquareBracket = Helpers::findContainingOpeningSquareBracket($phpcsFile, $listOpenerIndex);
+				$parentSquareBracket = self::findContainingOpeningSquareBracket($phpcsFile, $listOpenerIndex);
 				if (is_int($parentSquareBracket)) {
 					// Collect the opening index, but we don't actually need the closing paren index so just make that 0
 					$parents[$parentSquareBracket] = 0;
@@ -677,7 +677,7 @@ public static function getListAssignments(File $phpcsFile, $listOpenerIndex) {
 			if (is_int($variablePtr)) {
 				$variablePtrs[] = $variablePtr;
 			}
-			$currentPtr++;
+			++$currentPtr;
 		}
 
 		return $variablePtrs;
@@ -715,7 +715,7 @@ public static function debug() {
 		if (PHP_CODESNIFFER_VERBOSITY <= 3) {
 			return;
 		}
-		$output = PHP_EOL . "VariableAnalysisSniff: DEBUG:";
+		$output = PHP_EOL . 'VariableAnalysisSniff: DEBUG:';
 		foreach ($messages as $message) {
 			if (is_string($message) || is_numeric($message)) {
 				$output .= ' "' . $message . '"';
@@ -866,7 +866,7 @@ public static function getScopeCloseForScopeOpen(File $phpcsFile, $scopeStartInd
 		}
 
 		if ($scopeStartIndex === 0) {
-			$scopeCloserIndex = Helpers::getLastNonEmptyTokenIndexInFile($phpcsFile);
+			$scopeCloserIndex = self::getLastNonEmptyTokenIndexInFile($phpcsFile);
 		}
 		return $scopeCloserIndex;
 	}

VariableAnalysis/Lib/VariableInfo.php

@@ -90,13 +90,13 @@ class VariableInfo {
 	/**
 	 * @var string[]
 	 */
-	public static $scopeTypeDescriptions = array(
+	public static $scopeTypeDescriptions = [
 		ScopeType::LOCAL  => 'variable',
 		ScopeType::PARAM  => 'function parameter',
 		ScopeType::STATICSCOPE => 'static variable',
 		ScopeType::GLOBALSCOPE => 'global variable',
 		ScopeType::BOUND  => 'bound variable',
-	);
+	];
 
 	/**
 	 * @param string $varName

VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

@@ -482,7 +482,7 @@ protected function markVariableDeclaration(
 				//    we catch declarations that come after implicit declarations like
 				//    use of a variable as a local.
 				$this->addWarning(
-					"Redeclaration of %s %s as %s.",
+					'Redeclaration of %s %s as %s.',
 					$stackPtr,
 					'VariableRedeclaration',
 					[
@@ -625,28 +625,28 @@ protected function markAllVariablesRead(File $phpcsFile, $stackPtr) {
 	 * @return void
 	 */
 	protected function processVariableAsFunctionDefinitionArgument(File $phpcsFile, $stackPtr, $varName, $outerScope) {
-		Helpers::debug("processVariableAsFunctionDefinitionArgument", $stackPtr, $varName);
+		Helpers::debug('processVariableAsFunctionDefinitionArgument', $stackPtr, $varName);
 		$tokens = $phpcsFile->getTokens();
 
 		$functionPtr = Helpers::getFunctionIndexForFunctionArgument($phpcsFile, $stackPtr);
 		if (! is_int($functionPtr)) {
 			throw new \Exception("Function index not found for function argument index {$stackPtr}");
 		}
 
-		Helpers::debug("processVariableAsFunctionDefinitionArgument found function definition", $tokens[$functionPtr]);
+		Helpers::debug('processVariableAsFunctionDefinitionArgument found function definition', $tokens[$functionPtr]);
 		$this->markVariableDeclaration($varName, ScopeType::PARAM, null, $stackPtr, $functionPtr);
 
 		// Are we pass-by-reference?
 		$referencePtr = $phpcsFile->findPrevious(Tokens::$emptyTokens, $stackPtr - 1, null, true, null, true);
 		if (($referencePtr !== false) && ($tokens[$referencePtr]['code'] === T_BITWISE_AND)) {
-			Helpers::debug("processVariableAsFunctionDefinitionArgument found pass-by-reference to scope", $outerScope);
+			Helpers::debug('processVariableAsFunctionDefinitionArgument found pass-by-reference to scope', $outerScope);
 			$varInfo = $this->getOrCreateVariableInfo($varName, $functionPtr);
 			$varInfo->referencedVariableScope = $outerScope;
 		}
 
 		//  Are we optional with a default?
 		if (Helpers::getNextAssignPointer($phpcsFile, $stackPtr) !== null) {
-			Helpers::debug("processVariableAsFunctionDefinitionArgument optional with default");
+			Helpers::debug('processVariableAsFunctionDefinitionArgument optional with default');
 			$this->markVariableAssignment($varName, $stackPtr, $functionPtr);
 		}
 	}
@@ -664,7 +664,7 @@ protected function processVariableAsFunctionDefinitionArgument(File $phpcsFile,
 	protected function processVariableAsUseImportDefinition(File $phpcsFile, $stackPtr, $varName, $outerScope) {
 		$tokens = $phpcsFile->getTokens();
 
-		Helpers::debug("processVariableAsUseImportDefinition", $stackPtr, $varName, $outerScope);
+		Helpers::debug('processVariableAsUseImportDefinition', $stackPtr, $varName, $outerScope);
 
 		$endOfArgsPtr = $phpcsFile->findPrevious([T_CLOSE_PARENTHESIS], $stackPtr - 1, null);
 		if (! is_int($endOfArgsPtr)) {
@@ -1194,7 +1194,7 @@ protected function processVariableAsForeachLoopVar(File $phpcsFile, $stackPtr, $
 		// Are we pass-by-reference?
 		$referencePtr = $phpcsFile->findPrevious(Tokens::$emptyTokens, $stackPtr - 1, null, true, null, true);
 		if (($referencePtr !== false) && ($tokens[$referencePtr]['code'] === T_BITWISE_AND)) {
-			Helpers::debug("processVariableAsForeachLoopVar: found foreach loop variable assigned by reference");
+			Helpers::debug('processVariableAsForeachLoopVar: found foreach loop variable assigned by reference');
 			$varInfo->isDynamicReference = true;
 		}
 
@@ -1548,7 +1548,7 @@ protected function processVariableInString(File $phpcsFile, $stackPtr) {
 		if (!preg_match_all(Constants::getDoubleQuotedVarRegexp(), $token['content'], $matches)) {
 			return;
 		}
-		Helpers::debug("examining token for variable in string", $token);
+		Helpers::debug('examining token for variable in string', $token);
 
 		foreach ($matches[1] as $varName) {
 			$varName = Helpers::normalizeVarName($varName);
@@ -1726,7 +1726,7 @@ protected function warnAboutUnusedVariable(File $phpcsFile, VariableInfo $varInf
 		foreach (array_unique($varInfo->allAssignments) as $indexForWarning) {
 			Helpers::debug("variable {$varInfo->name} at end of scope looks unused");
 			$phpcsFile->addWarning(
-				"Unused %s %s.",
+				'Unused %s %s.',
 				$indexForWarning,
 				'UnusedVariable',
 				[
@@ -1746,7 +1746,7 @@ protected function warnAboutUnusedVariable(File $phpcsFile, VariableInfo $varInf
 	 */
 	protected function warnAboutUndefinedVariable(File $phpcsFile, $varName, $stackPtr) {
 		$phpcsFile->addWarning(
-			"Variable %s is undefined.",
+			'Variable %s is undefined.',
 			$stackPtr,
 			'UndefinedVariable',
 			["\${$varName}"]
@@ -1762,7 +1762,7 @@ protected function warnAboutUndefinedVariable(File $phpcsFile, $varName, $stackP
 	 */
 	protected function warnAboutUndefinedArrayPushShortcut(File $phpcsFile, $varName, $stackPtr) {
 		$phpcsFile->addWarning(
-			"Array variable %s is undefined.",
+			'Array variable %s is undefined.',
 			$stackPtr,
 			'UndefinedVariable',
 			["\${$varName}"]
@@ -1778,7 +1778,7 @@ protected function warnAboutUndefinedArrayPushShortcut(File $phpcsFile, $varName
 	 */
 	protected function warnAboutUndefinedUnset(File $phpcsFile, $varName, $stackPtr) {
 		$phpcsFile->addWarning(
-			"Variable %s inside unset call is undefined.",
+			'Variable %s inside unset call is undefined.',
 			$stackPtr,
 			'UndefinedUnsetVariable',
 			["\${$varName}"]