diff --git a/src/Fixer.php b/src/Fixer.php index 093e4fce5d..3e1f482a72 100644 --- a/src/Fixer.php +++ b/src/Fixer.php @@ -201,7 +201,7 @@ public function fixFile() $this->enabled = false; - if ($this->numFixes > 0) { + if ($this->numFixes > 0 || $this->inConflict === true) { if (PHP_CODESNIFFER_VERBOSITY > 1) { if (ob_get_level() > 0) { ob_end_clean(); diff --git a/tests/Core/Fixer/FixFileReturnValueAllGoodTest.xml b/tests/Core/Fixer/FixFileReturnValueAllGoodTest.xml new file mode 100644 index 0000000000..6f4e4996eb --- /dev/null +++ b/tests/Core/Fixer/FixFileReturnValueAllGoodTest.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/tests/Core/Fixer/FixFileReturnValueConflictTest.xml b/tests/Core/Fixer/FixFileReturnValueConflictTest.xml new file mode 100644 index 0000000000..5e2ddb473d --- /dev/null +++ b/tests/Core/Fixer/FixFileReturnValueConflictTest.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/tests/Core/Fixer/FixFileReturnValueNotEnoughLoopsTest.xml b/tests/Core/Fixer/FixFileReturnValueNotEnoughLoopsTest.xml new file mode 100644 index 0000000000..25810c04e6 --- /dev/null +++ b/tests/Core/Fixer/FixFileReturnValueNotEnoughLoopsTest.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/tests/Core/Fixer/FixFileReturnValueTest.php b/tests/Core/Fixer/FixFileReturnValueTest.php new file mode 100644 index 0000000000..76e56364c4 --- /dev/null +++ b/tests/Core/Fixer/FixFileReturnValueTest.php @@ -0,0 +1,89 @@ +process(); + $fixed = $phpcsFile->fixer->fixFile(); + + $this->assertTrue($fixed); + + }//end testReturnValueIsTrueWhenFileWasFixed() + + + /** + * Test that the return value of the fixFile() method is false when the file failed to make all fixes. + * + * @param string $standard The ruleset file to use for the test. + * + * @dataProvider dataReturnValueIsFalse + * + * @return void + */ + public function testReturnValueIsFalse($standard) + { + $config = new ConfigDouble(["--standard=$standard"]); + $ruleset = new Ruleset($config); + + $testCaseFile = __DIR__.'/Fixtures/test.inc'; + $phpcsFile = new LocalFile($testCaseFile, $ruleset, $config); + $phpcsFile->process(); + $fixed = $phpcsFile->fixer->fixFile(); + + $this->assertFalse($fixed); + + }//end testReturnValueIsFalse() + + + /** + * Data provider. + * + * @return array> + */ + public static function dataReturnValueIsFalse() + { + return [ + 'when there is a fixer conflict' => [ + 'standard' => __DIR__.'/FixFileReturnValueConflictTest.xml', + ], + 'when the fixer ran out of loops before all fixes could be applied' => [ + 'standard' => __DIR__.'/FixFileReturnValueNotEnoughLoopsTest.xml', + ], + ]; + + }//end dataReturnValueIsFalse() + + +}//end class diff --git a/tests/Core/Fixer/Fixtures/TestStandard/Sniffs/FixFileReturnValue/AllGoodSniff.php b/tests/Core/Fixer/Fixtures/TestStandard/Sniffs/FixFileReturnValue/AllGoodSniff.php new file mode 100644 index 0000000000..20dbe2fe22 --- /dev/null +++ b/tests/Core/Fixer/Fixtures/TestStandard/Sniffs/FixFileReturnValue/AllGoodSniff.php @@ -0,0 +1,37 @@ +getTokens(); + + if ($tokens[($stackPtr + 1)]['code'] !== T_WHITESPACE + || $tokens[($stackPtr + 1)]['length'] > 51 + ) { + return; + } + + $error = 'There should be 52 spaces after an ECHO keyword'; + $fix = $phpcsFile->addFixableError($error, ($stackPtr + 1), 'ShortSpace'); + if ($fix === true) { + $phpcsFile->fixer->replaceToken(($stackPtr + 1), str_repeat(' ', 52)); + } + } +} diff --git a/tests/Core/Fixer/Fixtures/TestStandard/Sniffs/FixFileReturnValue/ConflictSniff.php b/tests/Core/Fixer/Fixtures/TestStandard/Sniffs/FixFileReturnValue/ConflictSniff.php new file mode 100644 index 0000000000..ca7afb9159 --- /dev/null +++ b/tests/Core/Fixer/Fixtures/TestStandard/Sniffs/FixFileReturnValue/ConflictSniff.php @@ -0,0 +1,101 @@ +getTokens(); + + // Demand a blank line after the PHP open tag. + // This error is here to ensure something will be fixed in the file. + $nextNonWhitespace = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), null, true); + + if (($tokens[$nextNonWhitespace]['line'] - $tokens[$stackPtr]['line']) !== 2) { + $error = 'There must be a single blank line after the PHP open tag'; + $fix = $phpcsFile->addFixableError($error, $stackPtr, 'BlankLineAfterOpen'); + if ($fix === true) { + $phpcsFile->fixer->addNewline($stackPtr); + + // This return is here deliberately to force a new loop. + // This should ensure that loop 50 does *NOT* apply any fixes. + return; + } + } + + // Skip to the end of the file. + $stackPtr = ($phpcsFile->numTokens - 1); + + $eolCharLen = strlen($phpcsFile->eolChar); + $lastChars = substr($tokens[$stackPtr]['content'], ($eolCharLen * -1)); + + // Demand a newline at the end of a file. + if ($lastChars !== $phpcsFile->eolChar) { + $error = 'File must end with a newline character'; + $fix = $phpcsFile->addFixableError($error, $stackPtr, 'NoNewlineEOF'); + if ($fix === true) { + $phpcsFile->fixer->addNewline($stackPtr); + } + } + + // Demand NO newline at the end of a file. + if ($lastChars === $phpcsFile->eolChar) { + $error = 'File must not end with a newline character'; + $fix = $phpcsFile->addFixableError($error, $stackPtr, 'NewlineEOF'); + if ($fix === true) { + $phpcsFile->fixer->beginChangeset(); + + for ($i = $stackPtr; $i > 0; $i--) { + $newContent = rtrim($tokens[$i]['content'], $phpcsFile->eolChar); + $phpcsFile->fixer->replaceToken($i, $newContent); + + if ($newContent !== '') { + break; + } + } + + $phpcsFile->fixer->endChangeset(); + } + } + + // Ignore the rest of the file. + return $phpcsFile->numTokens; + } +} diff --git a/tests/Core/Fixer/Fixtures/TestStandard/Sniffs/FixFileReturnValue/NotEnoughLoopsSniff.php b/tests/Core/Fixer/Fixtures/TestStandard/Sniffs/FixFileReturnValue/NotEnoughLoopsSniff.php new file mode 100644 index 0000000000..e42d3fd7ed --- /dev/null +++ b/tests/Core/Fixer/Fixtures/TestStandard/Sniffs/FixFileReturnValue/NotEnoughLoopsSniff.php @@ -0,0 +1,40 @@ +getTokens(); + + if ($tokens[($stackPtr + 1)]['code'] !== T_WHITESPACE + || $tokens[($stackPtr + 1)]['length'] > 60 + ) { + return; + } + + $error = 'There should be 60 spaces after an ECHO keyword'; + $fix = $phpcsFile->addFixableError($error, ($stackPtr + 1), 'ShortSpace'); + if ($fix === true) { + // The fixer deliberately only adds one space in each loop to ensure it runs out of loops before the file complies. + $phpcsFile->fixer->addContent($stackPtr, ' '); + } + } +} diff --git a/tests/Core/Fixer/Fixtures/TestStandard/ruleset.xml b/tests/Core/Fixer/Fixtures/TestStandard/ruleset.xml new file mode 100644 index 0000000000..345c4837eb --- /dev/null +++ b/tests/Core/Fixer/Fixtures/TestStandard/ruleset.xml @@ -0,0 +1,4 @@ + + + + diff --git a/tests/Core/Fixer/Fixtures/test.inc b/tests/Core/Fixer/Fixtures/test.inc new file mode 100644 index 0000000000..76195b04c4 --- /dev/null +++ b/tests/Core/Fixer/Fixtures/test.inc @@ -0,0 +1,2 @@ +