Skip to content

Commit

Permalink
minor #5500 DX: Test that Transformers are adding only CustomTokens t…
Browse files Browse the repository at this point in the history
…hat they define and nothing else (keradus)

This PR was squashed before being merged into the 2.18 branch.

Discussion
----------

DX: Test that Transformers are adding only CustomTokens that they define and nothing else

The transformer classes are internal, so we can do whatever we want there without BC promise and upcoming BC breaks warnings.

Let's use the `getCustomTokens` method to ensure that Transformer is adding only allowed custom tokens, and no other transformer is adding them.

Unfortunately, to implement this check i had to bypass few things (eg access internals), but I couldn't find a good way to do it without refactoring main Tokenizer class into first-class service.

Commits
-------

9f54f1b DX: Test that Transformers are adding only CustomTokens that they define and nothing else
  • Loading branch information
keradus committed Mar 1, 2021
2 parents 711605a + 9f54f1b commit a8feb63
Show file tree
Hide file tree
Showing 27 changed files with 132 additions and 42 deletions.
1 change: 1 addition & 0 deletions .composer-require-checker.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"symbol-whitelist" : [
"LegacyPHPUnit\\TestCase",
"PhpCsFixer\\AccessibleObject\\AccessibleObject",
"PhpCsFixer\\PhpunitConstraintIsIdenticalString\\Constraint\\IsIdenticalString",
"PhpCsFixer\\Tests\\Test\\Constraint\\SameStringsConstraint",
"PhpCsFixer\\Tests\\Test\\IsIdenticalConstraint",
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/sca.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,5 +114,6 @@ jobs:
grep -v tests/Test/IntegrationCaseFactoryInterface.php |
grep -v tests/Test/InternalIntegrationCaseFactory.php |
grep -v tests/Test/IsIdenticalConstraint.php |
grep -v tests/Test/TokensWithObservedTransformers.php |
grep -v tests/TestCase.php \
&& (echo "UNKNOWN FILES DETECTED" && exit 1) || echo "NO UNKNOWN FILES"
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
"tests/Test/IntegrationCaseFactoryInterface.php",
"tests/Test/InternalIntegrationCaseFactory.php",
"tests/Test/IsIdenticalConstraint.php",
"tests/Test/TokensWithObservedTransformers.php",
"tests/TestCase.php"
]
},
Expand Down
12 changes: 1 addition & 11 deletions src/Tokenizer/AbstractTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,5 @@ public function getPriority()
/**
* {@inheritdoc}
*/
public function getCustomTokens()
{
@trigger_error(sprintf('%s is deprecated and will be removed in 3.0.', __METHOD__), E_USER_DEPRECATED);

return $this->getDeprecatedCustomTokens();
}

/**
* @return int[]
*/
abstract protected function getDeprecatedCustomTokens();
abstract public function getCustomTokens();
}
12 changes: 10 additions & 2 deletions src/Tokenizer/Tokens.php
Original file line number Diff line number Diff line change
Expand Up @@ -1103,8 +1103,7 @@ public function setCode($code)
$this[$index] = new Token($token);
}

$transformers = Transformers::create();
$transformers->transform($this);
$this->applyTransformers();

$this->foundTokenKinds = [];

Expand Down Expand Up @@ -1371,6 +1370,15 @@ public function valid()
return parent::valid();
}

/**
* @internal
*/
protected function applyTransformers()
{
$transformers = Transformers::create();
$transformers->transform($this);
}

private function warnPhp8SplFixerArrayChange($method)
{
if (80000 <= \PHP_VERSION_ID) {
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/ArrayTypehintTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [CT::T_ARRAY_TYPEHINT];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/AttributeTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [
CT::T_ATTRIBUTE_CLOSE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [CT::T_BRACE_CLASS_INSTANTIATION_OPEN, CT::T_BRACE_CLASS_INSTANTIATION_CLOSE];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/ClassConstantTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [CT::T_CLASS_CONSTANT];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [
CT::T_CONSTRUCTOR_PROPERTY_PROMOTION_PUBLIC,
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/CurlyBraceTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [
CT::T_CURLY_CLOSE,
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/ImportTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [CT::T_CONST_IMPORT, CT::T_FUNCTION_IMPORT];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/NameQualifiedTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/NamedArgumentTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [
CT::T_NAMED_ARGUMENT_COLON,
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/NamespaceOperatorTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [CT::T_NAMESPACE_OPERATOR];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/NullableTypeTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [CT::T_NULLABLE_TYPE];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/ReturnRefTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [CT::T_RETURN_REF];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/SquareBraceTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [
CT::T_ARRAY_SQUARE_BRACE_OPEN,
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/TypeAlternationTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [CT::T_TYPE_ALTERNATION];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/TypeColonTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [CT::T_TYPE_COLON];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/UseTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [CT::T_USE_TRAIT, CT::T_USE_LAMBDA];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Tokenizer/Transformer/WhitespacyCommentTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [];
}
Expand Down
2 changes: 0 additions & 2 deletions src/Tokenizer/TransformerInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ interface TransformerInterface
* Get tokens created by Transformer.
*
* @return int[]
*
* @deprecated will be removed in 3.0
*/
public function getCustomTokens();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function process(Tokens $tokens, Token $token, $index)
/**
* {@inheritdoc}
*/
protected function getDeprecatedCustomTokens()
public function getCustomTokens()
{
return [];
}
Expand Down
37 changes: 32 additions & 5 deletions tests/Test/AbstractTransformerTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use PhpCsFixer\Tests\TestCase;
use PhpCsFixer\Tokenizer\CT;
use PhpCsFixer\Tokenizer\Token;
use PhpCsFixer\Tokenizer\Tokens;
use PhpCsFixer\Tokenizer\TransformerInterface;

Expand Down Expand Up @@ -64,10 +65,6 @@ public function testGetName()
static::assertMatchesRegularExpression('/^[a-z]+[a-z_]*[a-z]$/', $name);
}

/**
* @group legacy
* @expectedDeprecation PhpCsFixer\Tokenizer\AbstractTransformer::getCustomTokens is deprecated and will be removed in 3.0.
*/
public function testGetCustomTokens()
{
$name = $this->transformer->getName();
Expand Down Expand Up @@ -120,7 +117,8 @@ public function testTransformDoesNotChangeSimpleCode()
protected function doTest($source, array $expectedTokens = [], array $observedKindsOrPrototypes = [])
{
Tokens::clearCache();
$tokens = Tokens::fromCode($source);
$tokens = new TokensWithObservedTransformers();
$tokens->setCode($source);

static::assertSame(
\count($expectedTokens),
Expand All @@ -136,6 +134,35 @@ static function ($kindOrPrototype) {
'Number of expected tokens does not match actual token count.'
);

$customTokensOfTransformer = $this->transformer->getCustomTokens();
$transformerName = $this->transformer->getName();

foreach ($tokens->observedModificationsPerTransformer as $appliedTransformerName => $modificationsOfTransformer) {
foreach ($modificationsOfTransformer as $modification) {
if ($appliedTransformerName === $transformerName) {
static::assertContains(
$modification,
$customTokensOfTransformer,
sprintf(
'Transformation into "%s" must be allowed in self-documentation of the Transformer, currently allowed custom tokens are: %s',
Token::getNameForId($modification),
implode(', ', array_map(function ($ct) { return Token::getNameForId($ct); }, $customTokensOfTransformer))
)
);
} else {
static::assertNotContains(
$modification,
$customTokensOfTransformer,
sprintf(
'Transformation into "%s" must NOT be applied by other Transformer than "%s".',
Token::getNameForId($modification),
$transformerName
)
);
}
}
}

foreach ($expectedTokens as $index => $tokenIdOrContent) {
if (\is_string($tokenIdOrContent)) {
static::assertTrue($tokens[$index]->equals($tokenIdOrContent), sprintf('The token at index %d should be %s, got %s', $index, json_encode($tokenIdOrContent), $tokens[$index]->toJson()));
Expand Down
68 changes: 68 additions & 0 deletions tests/Test/TokensWithObservedTransformers.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?php

/*
* This file is part of PHP CS Fixer.
*
* (c) Fabien Potencier <[email protected]>
* Dariusz Rumiński <[email protected]>
*
* This source file is subject to the MIT license that is bundled
* with this source code in the file LICENSE.
*/

namespace PhpCsFixer\Tests\Test;

use PhpCsFixer\AccessibleObject\AccessibleObject;
use PhpCsFixer\Tokenizer\Token;
use PhpCsFixer\Tokenizer\Tokens;
use PhpCsFixer\Tokenizer\Transformers;

class TokensWithObservedTransformers extends Tokens
{
/**
* @var null|string
*/
public $currentTransformer;
public $observedModificationsPerTransformer = [];

public function offsetSet($index, $newval)
{
if (null !== $this->currentTransformer) {
$this->observedModificationsPerTransformer[$this->currentTransformer][] = $this->extractTokenKind($newval);
}
parent::offsetSet($index, $newval);
}

/**
* @internal
*/
protected function applyTransformers()
{
$this->observedModificationsPerTransformer = [];

$transformers = Transformers::create();
foreach (AccessibleObject::create($transformers)->items as $transformer) {
$this->currentTransformer = $transformer->getName();
$this->observedModificationsPerTransformer[$this->currentTransformer] = [];

foreach ($this as $index => $token) {
$transformer->process($this, $token, $index);
}
}

$this->currentTransformer = null;
}

/**
* @param array|string|Token $token token prototype
*
* @return int|string
*/
private function extractTokenKind($token)
{
return $token instanceof Token
? ($token->isArray() ? $token->getId() : $token->getContent())
: (\is_array($token) ? $token[0] : $token)
;
}
}
4 changes: 0 additions & 4 deletions tests/Tokenizer/AbstractTransformerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ public function testNameAndPriorityDefault()
static::assertSame('foo', $transformer->getName());
}

/**
* @group legacy
* @expectedDeprecation PhpCsFixer\Tokenizer\AbstractTransformer::getCustomTokens is deprecated and will be removed in 3.0.
*/
public function testCustomTokens()
{
$transformer = new FooTransformer();
Expand Down

0 comments on commit a8feb63

Please sign in to comment.