Skip to content

Commit

Permalink
DX: test "isRisky" method in fixer tests, not as auto review
Browse files Browse the repository at this point in the history
  • Loading branch information
kubawerlos committed May 27, 2020
1 parent 1579d6e commit 66b37df
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 31 deletions.
2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1596,7 +1596,7 @@ Choose from the list of available rules:
EXPERIMENTAL: Takes ``@return`` annotation of non-mixed types and adjusts
accordingly the function signature. Requires PHP >= 7.0.

*Risky rule: [1] This rule is EXPERIMENTAL and is not covered with backward compatibility promise. [2] ``@return`` annotation is mandatory for the fixer to make changes, signatures of methods without it (no docblock, inheritdocs) will not be fixed. [3] Manual actions are required if inherited signatures are not properly documented. [4] ``@inheritdocs`` support is under construction.*
*Risky rule: this rule is EXPERIMENTAL and [1] is not covered with backward compatibility promise. [2] ``@return`` annotation is mandatory for the fixer to make changes, signatures of methods without it (no docblock, inheritdocs) will not be fixed. [3] Manual actions are required if inherited signatures are not properly documented. [4] ``@inheritdocs`` support is under construction.*

Configuration options:

Expand Down
2 changes: 1 addition & 1 deletion src/Fixer/FunctionNotation/PhpdocToReturnTypeFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ function my_foo()
),
],
null,
'[1] This rule is EXPERIMENTAL and is not covered with backward compatibility promise. [2] `@return` annotation is mandatory for the fixer to make changes, signatures of methods without it (no docblock, inheritdocs) will not be fixed. [3] Manual actions are required if inherited signatures are not properly documented. [4] `@inheritdocs` support is under construction.'
'This rule is EXPERIMENTAL and [1] is not covered with backward compatibility promise. [2] `@return` annotation is mandatory for the fixer to make changes, signatures of methods without it (no docblock, inheritdocs) will not be fixed. [3] Manual actions are required if inherited signatures are not properly documented. [4] `@inheritdocs` support is under construction.'
);
}

Expand Down
8 changes: 0 additions & 8 deletions src/Fixer/Phpdoc/PhpdocAddMissingParamAnnotationFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,6 @@ public function isCandidate(Tokens $tokens)
return $tokens->isTokenKindFound(T_DOC_COMMENT);
}

/**
* {@inheritdoc}
*/
public function isRisky()
{
return false;
}

/**
* {@inheritdoc}
*/
Expand Down
7 changes: 0 additions & 7 deletions tests/AutoReview/FixerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,6 @@ public function testFixerDefinitions(AbstractFixer $fixer)
static::assertRegExp('/^[a-z_]+[a-z]$/', $option->getName(), sprintf('[%s] Option %s is not snake_case.', $fixerName, $option->getName()));
}
}

if ($fixer->isRisky()) {
self::assertValidDescription($fixerName, 'risky description', $definition->getRiskyDescription());
} else {
static::assertNull($definition->getRiskyDescription(), sprintf('[%s] Fixer is not risky so no description of it expected.', $fixerName));
}
}

/**
Expand Down Expand Up @@ -294,7 +288,6 @@ public function testFixersReturnTypes(FixerInterface $fixer)
$emptyTokens = new Tokens();

static::assertInternalType('int', $fixer->getPriority(), sprintf('Return type for ::getPriority of "%s" is invalid.', $fixer->getName()));
static::assertInternalType('bool', $fixer->isRisky(), sprintf('Return type for ::isRisky of "%s" is invalid.', $fixer->getName()));
static::assertInternalType('bool', $fixer->supports(new \SplFileInfo(__FILE__)), sprintf('Return type for ::supports of "%s" is invalid.', $fixer->getName()));

static::assertInternalType('bool', $fixer->isCandidate($emptyTokens), sprintf('Return type for ::isCandidate with empty tokens of "%s" is invalid.', $fixer->getName()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,6 @@ public function testConfigureResetsExclude()
$this->doTest($after, $before);
}

public function testIsRisky()
{
$fixer = $this->createFixer();

static::assertTrue($fixer->isRisky());
}

/**
* @dataProvider provideFixWithDefaultConfigurationCases
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,6 @@ public function baz($foo)
$this->doTest($after, $before);
}

public function testIsRisky()
{
$fixer = $this->createFixer();

static::assertTrue($fixer->isRisky());
}

/**
* @dataProvider provideFixWithDefaultConfigurationCases
*
Expand Down
49 changes: 49 additions & 0 deletions tests/Test/AbstractFixerTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
namespace PhpCsFixer\Tests\Test;

use PhpCsFixer\AbstractFixer;
use PhpCsFixer\AbstractProxyFixer;
use PhpCsFixer\Linter\CachingLinter;
use PhpCsFixer\Linter\Linter;
use PhpCsFixer\Linter\LinterInterface;
Expand Down Expand Up @@ -66,6 +67,29 @@ protected function tearDown()
Tokens::setLegacyMode(false);
}

final public function testIsRisky()
{
static::assertInternalType('bool', $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()
);
}

/**
* @return AbstractFixer
*/
Expand Down Expand Up @@ -207,4 +231,29 @@ private function getLinter()

return $linter;
}

/**
* @param string $fixerName
* @param string $descriptionType
* @param mixed $description
*/
private static function assertValidDescription($fixerName, $descriptionType, $description)
{
static::assertInternalType('string', $description);
static::assertRegExp('/^[A-Z`][^"]+\.$/', $description, sprintf('[%s] The %s must start with capital letter or a ` and end with dot.', $fixerName, $descriptionType));
static::assertNotContains('phpdocs', $description, sprintf('[%s] `PHPDoc` must not be in the plural in %s.', $fixerName, $descriptionType), true);
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));
}

/**
* @param string $needle
* @param string $haystack
* @param string $message
*/
private static function assertCorrectCasing($needle, $haystack, $message)
{
static::assertSame(substr_count(strtolower($haystack), strtolower($needle)), substr_count($haystack, $needle), $message);
}
}

0 comments on commit 66b37df

Please sign in to comment.