From 8ec9ea41b770eb3430d432a03d8d711aa96016f9 Mon Sep 17 00:00:00 2001 From: nightlinus Date: Tue, 31 Mar 2020 23:04:33 +0300 Subject: [PATCH] AssignmentInConditionSniff: add option ignoreAssignmentsInsideFunctionCalls --- README.md | 9 ++++ .../AssignmentInConditionSniff.php | 37 ++++++++++++- .../AssignmentInConditionSniffTest.php | 26 ++++++++- .../data/allAssignmentsInConditions.php | 1 + ...nsIgnoreAssignmentsInsideFunctionCalls.php | 53 +++++++++++++++++++ 5 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 tests/Sniffs/ControlStructures/data/noAssignmentsInConditionsIgnoreAssignmentsInsideFunctionCalls.php diff --git a/README.md b/README.md index 9c8d38980..a247ec8eb 100644 --- a/README.md +++ b/README.md @@ -163,6 +163,15 @@ Assignment in `while` loop condition is specifically allowed because it's common This is a great addition to already existing `SlevomatCodingStandard.ControlStructures.DisallowYodaComparison` because it prevents the danger of assigning something by mistake instead of using comparison operator like `===`. +Sniff provides the following settings: +* `ignoreAssignmentsInsideFunctionCalls`: ignores assignment inside function calls, like this: + +```php +if (in_array(1, $haystack, $strict = true) { + +} +``` + #### SlevomatCodingStandard.ControlStructures.DisallowContinueWithoutIntegerOperandInSwitch 🔧 Disallows use of `continue` without integer operand in `switch` because it emits a warning in PHP 7.3 and higher. diff --git a/SlevomatCodingStandard/Sniffs/ControlStructures/AssignmentInConditionSniff.php b/SlevomatCodingStandard/Sniffs/ControlStructures/AssignmentInConditionSniff.php index 6f7ffd739..936fe0340 100644 --- a/SlevomatCodingStandard/Sniffs/ControlStructures/AssignmentInConditionSniff.php +++ b/SlevomatCodingStandard/Sniffs/ControlStructures/AssignmentInConditionSniff.php @@ -5,11 +5,16 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; use SlevomatCodingStandard\Helpers\TokenHelper; +use function array_keys; +use function count; +use function max; +use function reset; use function sprintf; use const T_DO; use const T_ELSEIF; use const T_EQUAL; use const T_IF; +use const T_STRING; use const T_WHILE; class AssignmentInConditionSniff implements Sniff @@ -17,6 +22,9 @@ class AssignmentInConditionSniff implements Sniff public const CODE_ASSIGNMENT_IN_CONDITION = 'AssignmentInCondition'; + /** @var bool */ + public $ignoreAssignmentsInsideFunctionCalls = false; + /** * @return array */ @@ -54,11 +62,36 @@ public function process(File $phpcsFile, $conditionStartPointer): void private function processCondition(File $phpcsFile, int $parenthesisOpener, int $parenthesisCloser, string $conditionType): void { - $equalsTokenPointer = TokenHelper::findNext($phpcsFile, T_EQUAL, $parenthesisOpener + 1, $parenthesisCloser); - if ($equalsTokenPointer === null) { + $equalsTokenPointers = TokenHelper::findNextAll($phpcsFile, T_EQUAL, $parenthesisOpener + 1, $parenthesisCloser); + $tokens = $phpcsFile->getTokens(); + + if (!$this->ignoreAssignmentsInsideFunctionCalls && count($equalsTokenPointers) !== 0) { + $this->error($phpcsFile, $conditionType, reset($equalsTokenPointers)); return; } + foreach ($equalsTokenPointers as $equalsTokenPointer) { + $parenthesisStarts = array_keys($tokens[$equalsTokenPointer]['nested_parenthesis'] ?? []); + $insideParenthesis = max($parenthesisStarts); + if ($insideParenthesis === $parenthesisOpener) { + $this->error($phpcsFile, $conditionType, $equalsTokenPointer); + //this is assignment inside if expression + continue; + } + + $functionCall = TokenHelper::findPrevious($phpcsFile, T_STRING, $insideParenthesis, $parenthesisOpener); + if ($functionCall !== null) { + //this is assignment inside function call + continue; + } + + //this is assignment inside nested parenthesis + $this->error($phpcsFile, $conditionType, $equalsTokenPointer); + } + } + + private function error(File $phpcsFile, string $conditionType, int $equalsTokenPointer): void + { $phpcsFile->addError( sprintf('Assignment in %s condition is not allowed.', $conditionType), $equalsTokenPointer, diff --git a/tests/Sniffs/ControlStructures/AssignmentInConditionSniffTest.php b/tests/Sniffs/ControlStructures/AssignmentInConditionSniffTest.php index 836753f6d..f30494b75 100644 --- a/tests/Sniffs/ControlStructures/AssignmentInConditionSniffTest.php +++ b/tests/Sniffs/ControlStructures/AssignmentInConditionSniffTest.php @@ -17,7 +17,31 @@ public function testCorrectFile(): void public function testIncorrectFile(): void { $resultFile = self::checkFile(__DIR__ . '/data/allAssignmentsInConditions.php'); - foreach (range(3, 6) as $lineNumber) { + foreach (range(3, 7) as $lineNumber) { + self::assertSniffError($resultFile, $lineNumber, AssignmentInConditionSniff::CODE_ASSIGNMENT_IN_CONDITION); + } + } + + public function testNoErrorsWithignoreAssignmentsInsideFunctionCalls(): void + { + $report = self::checkFile( + __DIR__ . '/data/noAssignmentsInConditionsIgnoreAssignmentsInsideFunctionCalls.php', + [ + 'ignoreAssignmentsInsideFunctionCalls' => true, + ] + ); + self::assertNoSniffErrorInFile($report); + } + + public function testErrorsWithignoreAssignmentsInsideFunctionCalls(): void + { + $resultFile = self::checkFile( + __DIR__ . '/data/allAssignmentsInConditions.php', + [ + 'ignoreAssignmentsInsideFunctionCalls' => true, + ] + ); + foreach (range(3, 7) as $lineNumber) { self::assertSniffError($resultFile, $lineNumber, AssignmentInConditionSniff::CODE_ASSIGNMENT_IN_CONDITION); } } diff --git a/tests/Sniffs/ControlStructures/data/allAssignmentsInConditions.php b/tests/Sniffs/ControlStructures/data/allAssignmentsInConditions.php index 09a73fcdb..f0eddd904 100644 --- a/tests/Sniffs/ControlStructures/data/allAssignmentsInConditions.php +++ b/tests/Sniffs/ControlStructures/data/allAssignmentsInConditions.php @@ -4,3 +4,4 @@ if ($foo) {} elseif ($foo = 5) {} do { } while ($foo = 5); if (($line = fgets($fp)) !== null) { } +if (($a = readfile($file = 'test'))) { } diff --git a/tests/Sniffs/ControlStructures/data/noAssignmentsInConditionsIgnoreAssignmentsInsideFunctionCalls.php b/tests/Sniffs/ControlStructures/data/noAssignmentsInConditionsIgnoreAssignmentsInsideFunctionCalls.php new file mode 100644 index 000000000..ef78b3c97 --- /dev/null +++ b/tests/Sniffs/ControlStructures/data/noAssignmentsInConditionsIgnoreAssignmentsInsideFunctionCalls.php @@ -0,0 +1,53 @@ +fetch()) !== null) { + +} + +do { + +} while ($foo === 5); + +if (in_array($a = 1, [], $strict = true)) { + +} + +if ($this->call($a = 1)) { + +}