diff --git a/.gitignore b/.gitignore index de4a392..1c9815d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ /vendor /composer.lock +.phpunit.result.cache diff --git a/.travis.yml b/.travis.yml index 78fed74..4e6068d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,8 @@ language: php php: + - 8.0 + - 7.4 - 7.1 - - 7.0 - - 5.6 install: composer install script: php vendor/bin/phpunit diff --git a/composer.json b/composer.json index eb7dbca..8662710 100644 --- a/composer.json +++ b/composer.json @@ -16,15 +16,15 @@ }, "autoload-dev": { "psr-4": { - "PhpCsFixer\\Tests\\": "vendor/friendsofphp/php-cs-fixer/tests/" + "Drew\\RemoveDebugStatements\\Tests\\": "tests/" } }, "require": { - "friendsofphp/php-cs-fixer": "^2.0" + "php": "^7.1 || ^8.0", + "friendsofphp/php-cs-fixer": "^3.0" }, "require-dev": { - "gecko-packages/gecko-php-unit": "^2.0 || ^3.0", - "phpunit/phpunit": "^4.5|^5|^6", - "satooshi/php-coveralls": "^1.0 || ^2.0" + "phpunit/phpunit": "^8.5", + "php-coveralls/php-coveralls": "^2.4" } } diff --git a/src/Dump.php b/src/Dump.php index c1099de..ee21f0e 100644 --- a/src/Dump.php +++ b/src/Dump.php @@ -5,6 +5,7 @@ use PhpCsFixer\AbstractFunctionReferenceFixer; use PhpCsFixer\FixerDefinition\CodeSample; use PhpCsFixer\FixerDefinition\FixerDefinition; +use PhpCsFixer\FixerDefinition\FixerDefinitionInterface; use PhpCsFixer\Tokenizer\Tokens; /** @@ -20,7 +21,7 @@ final class Dump extends AbstractFunctionReferenceFixer /** * {@inheritdoc} */ - public function isCandidate(Tokens $tokens) + public function isCandidate(Tokens $tokens): bool { return $tokens->isTokenKindFound(T_STRING); } @@ -28,7 +29,7 @@ public function isCandidate(Tokens $tokens) /** * {@inheritdoc} */ - public function getName() + public function getName(): string { return 'RemoveDebugStatements/dump'; } @@ -36,18 +37,20 @@ public function getName() /** * {@inheritdoc} */ - public function getDefinition() + public function getDefinition(): FixerDefinitionInterface { return new FixerDefinition( 'Removes dump/var_dump statements, which shouldn\'t be in production ever.', - array(new CodeSample("functions) . ' are redefined.' ); } /** * {@inheritdoc} */ - protected function applyFix(\SplFileInfo $file, Tokens $tokens) + protected function applyFix(\SplFileInfo $file, Tokens $tokens): void { foreach ($this->functions as $function) { $currIndex = 0; diff --git a/tests/AbstractFixerTestCase.php b/tests/AbstractFixerTestCase.php new file mode 100644 index 0000000..2e0c02a --- /dev/null +++ b/tests/AbstractFixerTestCase.php @@ -0,0 +1,466 @@ + + * + * @internal + */ +abstract class AbstractFixerTestCase extends TestCase +{ + use AssertTokensTrait; + + /** + * @var null|LinterInterface + */ + protected $linter; + + /** + * @var null|AbstractFixer + */ + protected $fixer; + + // do not modify this structure without prior discussion + private $allowedRequiredOptions = [ + 'header_comment' => ['header' => true], + ]; + + // do not modify this structure without prior discussion + private $allowedFixersWithoutDefaultCodeSample = [ + 'general_phpdoc_annotation_remove' => true, + 'general_phpdoc_tag_rename' => true, + ]; + + protected function setUp(): void + { + parent::setUp(); + + $this->linter = $this->getLinter(); + $this->fixer = $this->createFixer(); + } + + protected function tearDown(): void + { + parent::tearDown(); + + $this->linter = null; + $this->fixer = null; + } + + final public function testIsRisky(): void + { + static::assertIsBool($this->fixer->isRisky(), sprintf('Return type for ::isRisky of "%s" is invalid.', $this->fixer->getName())); + + if ($this->fixer->isRisky()) { + self::assertValidDescription($this->fixer->getName(), 'risky description', $this->fixer->getDefinition()->getRiskyDescription()); + } else { + static::assertNull($this->fixer->getDefinition()->getRiskyDescription(), sprintf('[%s] Fixer is not risky so no description of it expected.', $this->fixer->getName())); + } + + if ($this->fixer instanceof AbstractProxyFixer) { + return; + } + + $reflection = new \ReflectionMethod($this->fixer, 'isRisky'); + + // If fixer is not risky then the method `isRisky` from `AbstractFixer` must be used + static::assertSame( + !$this->fixer->isRisky(), + AbstractFixer::class === $reflection->getDeclaringClass()->getName() + ); + } + + final public function testFixerDefinitions(): void + { + $fixerName = $this->fixer->getName(); + $definition = $this->fixer->getDefinition(); + $fixerIsConfigurable = $this->fixer instanceof ConfigurableFixerInterface; + + self::assertValidDescription($fixerName, 'summary', $definition->getSummary()); + + $samples = $definition->getCodeSamples(); + static::assertNotEmpty($samples, sprintf('[%s] Code samples are required.', $fixerName)); + + $configSamplesProvided = []; + $dummyFileInfo = new StdinFileInfo(); + foreach ($samples as $sampleCounter => $sample) { + static::assertInstanceOf(CodeSampleInterface::class, $sample, sprintf('[%s] Sample #%d', $fixerName, $sampleCounter)); + static::assertIsInt($sampleCounter); + + $code = $sample->getCode(); + + static::assertIsString($code, sprintf('[%s] Sample #%d', $fixerName, $sampleCounter)); + static::assertNotEmpty($code, sprintf('[%s] Sample #%d', $fixerName, $sampleCounter)); + + if (!($this->fixer instanceof SingleBlankLineAtEofFixer)) { + static::assertSame("\n", substr($code, -1), sprintf('[%s] Sample #%d must end with linebreak', $fixerName, $sampleCounter)); + } + + $config = $sample->getConfiguration(); + if (null !== $config) { + static::assertTrue($fixerIsConfigurable, sprintf('[%s] Sample #%d has configuration, but the fixer is not configurable.', $fixerName, $sampleCounter)); + static::assertIsArray($config, sprintf('[%s] Sample #%d configuration must be an array or null.', $fixerName, $sampleCounter)); + + $configSamplesProvided[$sampleCounter] = $config; + } elseif ($fixerIsConfigurable) { + if (!$sample instanceof VersionSpecificCodeSampleInterface) { + static::assertArrayNotHasKey('default', $configSamplesProvided, sprintf('[%s] Multiple non-versioned samples with default configuration.', $fixerName)); + } + + $configSamplesProvided['default'] = true; + } + + if ($sample instanceof VersionSpecificCodeSampleInterface && !$sample->isSuitableFor(\PHP_VERSION_ID)) { + continue; + } + + if ($fixerIsConfigurable) { + // always re-configure as the fixer might have been configured with diff. configuration form previous sample + $this->fixer->configure($config ?? []); + } + + Tokens::clearCache(); + $tokens = Tokens::fromCode($code); + $this->fixer->fix( + $sample instanceof FileSpecificCodeSampleInterface ? $sample->getSplFileInfo() : $dummyFileInfo, + $tokens + ); + + static::assertTrue($tokens->isChanged(), sprintf('[%s] Sample #%d is not changed during fixing.', $fixerName, $sampleCounter)); + + $duplicatedCodeSample = array_search( + $sample, + \array_slice($samples, 0, $sampleCounter), + false + ); + + static::assertFalse( + $duplicatedCodeSample, + sprintf('[%s] Sample #%d duplicates #%d.', $fixerName, $sampleCounter, $duplicatedCodeSample) + ); + } + + if ($fixerIsConfigurable) { + if (isset($configSamplesProvided['default'])) { + reset($configSamplesProvided); + static::assertSame('default', key($configSamplesProvided), sprintf('[%s] First sample must be for the default configuration.', $fixerName)); + } elseif (!isset($this->allowedFixersWithoutDefaultCodeSample[$fixerName])) { + static::assertArrayHasKey($fixerName, $this->allowedRequiredOptions, sprintf('[%s] Has no sample for default configuration.', $fixerName)); + } + + if (\count($configSamplesProvided) < 2) { + static::fail(sprintf('[%s] Configurable fixer only provides a default configuration sample and none for its configuration options.', $fixerName)); + } + + $options = $this->fixer->getConfigurationDefinition()->getOptions(); + + foreach ($options as $option) { + static::assertRegExp('/^[a-z_]+[a-z]$/', $option->getName(), sprintf('[%s] Option %s is not snake_case.', $fixerName, $option->getName())); + } + } + } + + final public function testFixersAreFinal(): void + { + $reflection = new \ReflectionClass($this->fixer); + + static::assertTrue( + $reflection->isFinal(), + sprintf('Fixer "%s" must be declared "final".', $this->fixer->getName()) + ); + } + + final public function testDeprecatedFixersHaveCorrectSummary(): void + { + $reflection = new \ReflectionClass($this->fixer); + $comment = $reflection->getDocComment(); + + static::assertStringNotContainsString( + 'DEPRECATED', + $this->fixer->getDefinition()->getSummary(), + 'Fixer cannot contain word "DEPRECATED" in summary' + ); + + if ($this->fixer instanceof DeprecatedFixerInterface) { + static::assertStringContainsString('@deprecated', $comment); + } elseif (\is_string($comment)) { + static::assertStringNotContainsString('@deprecated', $comment); + } + } + + /** + * Blur filter that find candidate fixer for performance optimization to use only `insertSlices` instead of multiple `insertAt` if there is no other collection manipulation. + */ + public function testFixerUseInsertSlicesWhenOnlyInsertionsArePerformed(): void + { + $reflection = new \ReflectionClass($this->fixer); + $tokens = Tokens::fromCode(file_get_contents($reflection->getFileName())); + + $sequences = $this->findAllTokenSequences($tokens, [[T_VARIABLE, '$tokens'], [T_OBJECT_OPERATOR], [T_STRING]]); + + $usedMethods = array_unique(array_map(function (array $sequence) { + $last = end($sequence); + + return $last->getContent(); + }, $sequences)); + + // if there is no `insertAt`, it's not a candidate + if (!\in_array('insertAt', $usedMethods, true)) { + $this->addToAssertionCount(1); + + return; + } + + $usedMethods = array_filter($usedMethods, function (string $method) { + return 0 === Preg::match('/^(count|find|generate|get|is|rewind)/', $method); + }); + + $allowedMethods = ['insertAt']; + $nonAllowedMethods = array_diff($usedMethods, $allowedMethods); + + if ([] === $nonAllowedMethods) { + $fixerName = $this->fixer->getName(); + if (\in_array($fixerName, [ + // DO NOT add anything to this list at ease, align with core contributors whether it makes sense to insert tokens individually or by bulk for your case. + // The original list of the fixers being exceptions and insert tokens individually came from legacy reasons when it was the only available methods to insert tokens. + 'blank_line_after_namespace', + 'blank_line_after_opening_tag', + 'blank_line_before_statement', + 'class_attributes_separation', + 'date_time_immutable', + 'declare_strict_types', + 'doctrine_annotation_braces', + 'doctrine_annotation_spaces', + 'final_internal_class', + 'final_public_method_for_abstract_class', + 'function_typehint_space', + 'heredoc_indentation', + 'method_chaining_indentation', + 'native_constant_invocation', + 'new_with_braces', + 'no_short_echo_tag', + 'not_operator_with_space', + 'not_operator_with_successor_space', + 'php_unit_internal_class', + 'php_unit_no_expectation_annotation', + 'php_unit_set_up_tear_down_visibility', + 'php_unit_size_class', + 'php_unit_test_annotation', + 'php_unit_test_class_requires_covers', + 'phpdoc_to_param_type', + 'phpdoc_to_property_type', + 'phpdoc_to_return_type', + 'random_api_migration', + 'semicolon_after_instruction', + 'single_line_after_imports', + 'static_lambda', + 'strict_param', + 'void_return', + ], true)) { + static::markTestIncomplete(sprintf('Fixer "%s" may be optimized to use `Tokens::insertSlices` instead of `%s`, please help and optimize it.', $fixerName, implode(', ', $allowedMethods))); + } + static::fail(sprintf('Fixer "%s" shall be optimized to use `Tokens::insertSlices` instead of `%s`.', $fixerName, implode(', ', $allowedMethods))); + } + + $this->addToAssertionCount(1); + } + + final public function testFixerConfigurationDefinitions(): void + { + if (!$this->fixer instanceof ConfigurableFixerInterface) { + $this->addToAssertionCount(1); // not applied to the fixer without configuration + + return; + } + + $configurationDefinition = $this->fixer->getConfigurationDefinition(); + + static::assertInstanceOf(FixerConfigurationResolverInterface::class, $configurationDefinition); + + foreach ($configurationDefinition->getOptions() as $option) { + static::assertInstanceOf(FixerOptionInterface::class, $option); + static::assertNotEmpty($option->getDescription()); + + static::assertSame( + !isset($this->allowedRequiredOptions[$this->fixer->getName()][$option->getName()]), + $option->hasDefault(), + sprintf( + $option->hasDefault() + ? 'Option `%s` of fixer `%s` is wrongly listed in `$allowedRequiredOptions` structure, as it is not required. If you just changed that option to not be required anymore, please adjust mentioned structure.' + : 'Option `%s` of fixer `%s` shall not be required. If you want to introduce new required option please adjust `$allowedRequiredOptions` structure.', + $option->getName(), + $this->fixer->getName() + ) + ); + + static::assertStringNotContainsString( + 'DEPRECATED', + $option->getDescription(), + 'Option description cannot contain word "DEPRECATED"' + ); + } + } + + protected function createFixer(): AbstractFixer + { + $fixerClassName = preg_replace('/^(PhpCsFixer)\\\\Tests(\\\\.+)Test$/', '$1$2', static::class); + + return new $fixerClassName(); + } + + protected function getTestFile(string $filename = __FILE__): \SplFileInfo + { + static $files = []; + + if (!isset($files[$filename])) { + $files[$filename] = new \SplFileInfo($filename); + } + + return $files[$filename]; + } + + /** + * Tests if a fixer fixes a given string to match the expected result. + * + * It is used both if you want to test if something is fixed or if it is not touched by the fixer. + * It also makes sure that the expected output does not change when run through the fixer. That means that you + * do not need two test cases like [$expected] and [$expected, $input] (where $expected is the same in both cases) + * as the latter covers both of them. + * This method throws an exception if $expected and $input are equal to prevent test cases that accidentally do + * not test anything. + * + * @param string $expected The expected fixer output + * @param null|string $input The fixer input, or null if it should intentionally be equal to the output + * @param null|\SplFileInfo $file The file to fix, or null if unneeded + */ + protected function doTest(string $expected, ?string $input = null, ?\SplFileInfo $file = null): void + { + if ($expected === $input) { + throw new \InvalidArgumentException('Input parameter must not be equal to expected parameter.'); + } + + $file = $file ?: $this->getTestFile(); + $fileIsSupported = $this->fixer->supports($file); + + if (null !== $input) { + static::assertNull($this->lintSource($input)); + + Tokens::clearCache(); + $tokens = Tokens::fromCode($input); + + if ($fileIsSupported) { + static::assertTrue($this->fixer->isCandidate($tokens), 'Fixer must be a candidate for input code.'); + static::assertFalse($tokens->isChanged(), 'Fixer must not touch Tokens on candidate check.'); + $this->fixer->fix($file, $tokens); + } + + + static::assertSame($expected, $tokens->generateCode(), 'Code build on input code must match expected code.'); + static::assertTrue($tokens->isChanged(), 'Tokens collection built on input code must be marked as changed after fixing.'); + + $tokens->clearEmptyTokens(); + + static::assertSame( + \count($tokens), + \count(array_unique(array_map(static function (Token $token) { + return spl_object_hash($token); + }, $tokens->toArray()))), + 'Token items inside Tokens collection must be unique.' + ); + + Tokens::clearCache(); + $expectedTokens = Tokens::fromCode($expected); + static::assertTokens($expectedTokens, $tokens); + } + + static::assertNull($this->lintSource($expected)); + + Tokens::clearCache(); + $tokens = Tokens::fromCode($expected); + + if ($fileIsSupported) { + $this->fixer->fix($file, $tokens); + } + + static::assertSame($expected, $tokens->generateCode(), 'Code build on expected code must not change.'); + static::assertFalse($tokens->isChanged(), 'Tokens collection built on expected code must not be marked as changed after fixing.'); + } + + protected function lintSource(string $source): ?string + { + try { + $this->linter->lintSource($source)->check(); + } catch (\Exception $e) { + return $e->getMessage()."\n\nSource:\n{$source}"; + } + + return null; + } + + private function getLinter(): LinterInterface + { + static $linter = null; + + if (null === $linter) { + $linter = new CachingLinter( + getenv('FAST_LINT_TEST_CASES') ? new Linter() : new ProcessLinter() + ); + } + + return $linter; + } + + private static function assertValidDescription(string $fixerName, string $descriptionType, string $description): void + { + static::assertRegExp('/^[A-Z`][^"]+\.$/', $description, sprintf('[%s] The %s must start with capital letter or a ` and end with dot.', $fixerName, $descriptionType)); + static::assertStringNotContainsString('phpdocs', $description, sprintf('[%s] `PHPDoc` must not be in the plural in %s.', $fixerName, $descriptionType)); + static::assertCorrectCasing($description, 'PHPDoc', sprintf('[%s] `PHPDoc` must be in correct casing in %s.', $fixerName, $descriptionType)); + static::assertCorrectCasing($description, 'PHPUnit', sprintf('[%s] `PHPUnit` must be in correct casing in %s.', $fixerName, $descriptionType)); + static::assertFalse(strpos($descriptionType, '``'), sprintf('[%s] The %s must no contain sequential backticks.', $fixerName, $descriptionType)); + } + + private static function assertCorrectCasing(string $needle, string $haystack, string $message): void + { + static::assertSame(substr_count(strtolower($haystack), strtolower($needle)), substr_count($haystack, $needle), $message); + } + + private function findAllTokenSequences($tokens, $sequence): array + { + $lastIndex = 0; + $sequences = []; + while ($found = $tokens->findSequence($sequence, $lastIndex)) { + $keys = array_keys($found); + $sequences[] = $found; + $lastIndex = $keys[2]; + } + + return $sequences; + } +} diff --git a/tests/AssertTokensTrait.php b/tests/AssertTokensTrait.php new file mode 100644 index 0000000..691eef5 --- /dev/null +++ b/tests/AssertTokensTrait.php @@ -0,0 +1,44 @@ + + * + * @internal + */ +trait AssertTokensTrait +{ + private function assertTokens(Tokens $expectedTokens, Tokens $inputTokens) + { + $option = ['JSON_PRETTY_PRINT']; + + foreach ($expectedTokens as $index => $expectedToken) { + $inputToken = $inputTokens[$index]; + + $this->assertTrue( + $expectedToken->equals($inputToken), + sprintf("The token at index %d must be:\n%s,\ngot:\n%s.", $index, $expectedToken->toJson($option), $inputToken->toJson($option)) + ); + + $expectedTokenKind = $expectedToken->isArray() ? $expectedToken->getId() : $expectedToken->getContent(); + $this->assertTrue( + $inputTokens->isTokenKindFound($expectedTokenKind), + sprintf( + 'The token kind %s (%s) must be found in tokens collection.', + $expectedTokenKind, + \is_string($expectedTokenKind) ? $expectedTokenKind : Token::getNameForId($expectedTokenKind) + ) + ); + } + + $this->assertSame($expectedTokens->count(), $inputTokens->count(), 'Both collections must have the same length.'); + } +} diff --git a/tests/DumpTest.php b/tests/DumpTest.php index 59dc527..d37f492 100644 --- a/tests/DumpTest.php +++ b/tests/DumpTest.php @@ -3,7 +3,7 @@ namespace Drew\RemoveDebugStatements\Tests; use Drew\DebugStatementsFixers\Dump; -use PhpCsFixer\Test\AbstractFixerTestCase; +use PhpCsFixer\AbstractFixer; /** * @author Andrew Kovalyov @@ -22,20 +22,20 @@ public function testFix($expected, $input = null) $this->doTest($expected, $input); } - public function createFixer() + public function createFixer(): AbstractFixer { return new Dump(); } - public function provideFixCases() + public function provideFixCases(): array { - return array( - array('