Skip to content
This repository has been archived by the owner on Dec 3, 2023. It is now read-only.

[CodingStandard] ClassStringToClassCosntantFixer init #262

Merged
merged 13 commits into from
Jul 15, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions packages/CodingStandard/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,25 @@ class SomeClass
```


### `SomeClass::class` references should be used over string


- [Php/ClassStringToClassConstantFixer](/src/Fixer/Php/ClassStringToClassConstantFixer.php)
- This checker uses *[PHP-CS-Fixer](https://github.com/friendsofphp/php-cs-fixer)*

:x:

```php
$className = 'DateTime';
```

:+1:

```php
$className = DateTime::class;
```



### Array property should have default value, to prevent undefined array issues

Expand Down Expand Up @@ -78,6 +97,7 @@ class SomeClass

:+1:


``` php
class SomeClass
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
<?php declare(strict_types=1);

namespace Symplify\CodingStandard\Fixer\Php;

use Nette\Utils\Strings;
use PhpCsFixer\Fixer\DefinedFixerInterface;
use PhpCsFixer\FixerDefinition\CodeSample;
use PhpCsFixer\FixerDefinition\FixerDefinition;
use PhpCsFixer\FixerDefinition\FixerDefinitionInterface;
use PhpCsFixer\Tokenizer\CT;
use PhpCsFixer\Tokenizer\Token;
use PhpCsFixer\Tokenizer\Tokens;
use SplFileInfo;

final class ClassStringToClassConstantFixer implements DefinedFixerInterface
{
public function getDefinition(): FixerDefinitionInterface
{
return new FixerDefinition(
'"SomeClass::class" references should be used over string.',
[
new CodeSample(
'<?php

$className = "DateTime";
'),
new CodeSample(
'<?php

$interfaceName = "DateTimeInterface";
'),
]
);
}

public function isCandidate(Tokens $tokens): bool
{
return true;
}

public function fix(SplFileInfo $file, Tokens $tokens): void
{
foreach (array_reverse($tokens->toArray(), true) as $index => $token) {
/** @var Token $token */
if (! $this->isStringToken($token)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is possible directly look for code T_CONSTANT_ENCAPSED_STRING ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, that's the token I was looking for! Thank you

continue;
}

$potentialClassOrInterfaceName = trim($token->getContent(), "'");
if (class_exists($potentialClassOrInterfaceName) || interface_exists($potentialClassOrInterfaceName)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Class may not exist, because ECS is used for projects that uses own classes, so better is regexp checking. I am using this:

			if (preg_match('#^[A-Z]\w*[a-z]\w*(\\\\[A-Z]\w*[a-z]\w*)*\z#', $potentialClassOrInterfaceName)) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I will try it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is, it replaces non existing classes as well: 9a14c3b#diff-1ca8f9425dbe99f88cb78b89459d3a6e

So I'd probably stay with *_exists() function or do you know how to solve it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sry, the regexp should be '#^[A-Z]\w*[a-z]\w*(\\\\[A-Z]\w*[a-z]\w*)+\z#', ie. + instead of *.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, it can change false-classes, but in practice it rarely happens. Regexp will not detect non-namespace classes, which are usually native PHP classes, so perhaps ideal solution is class_exists || interface_exists || regexp.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixed regex, that makes sense.

Work much better now 🎂

Last thing I have to resolve is importing use statements.

$token->clear(); // overrideAt() fails on "Illegal offset type"
$tokens->insertAt($index, [
new Token([T_NS_SEPARATOR, '\\']),
new Token([T_STRING, $potentialClassOrInterfaceName]),
new Token([T_DOUBLE_COLON, '::']),
new Token([CT::T_CLASS_CONSTANT, 'class']),
]);
}
}
}

public function isRisky(): bool
{
return false;
}

public function getName(): string
{
return self::class;
}

public function getPriority(): int
{
// @todo combine with namespace import fixer/sniff
return 0;
}

public function supports(SplFileInfo $file): bool
{
return true;
}

private function isStringToken(Token $token): bool
{
return Strings::startsWith($token->getContent(), "'")
&& Strings::endsWith($token->getContent(), "'");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php declare(strict_types=1);

namespace Symplify\CodingStandard\Tests\Fixer\Php;

use PhpCsFixer\Fixer\FixerInterface;
use PhpCsFixer\Test\AbstractFixerTestCase;
use Symplify\CodingStandard\Fixer\Php\ClassStringToClassConstantFixer;

final class ClassStringToClassConstantFixerTest extends AbstractFixerTestCase
{
/**
* @dataProvider provideFixCases()
*/
public function testFix(string $expected, string $input): void
{
$this->doTest($expected, $input);
}

/**
* @return string[][]
*/
public function provideFixCases(): array
{
return [
[
file_get_contents(__DIR__ . '/fixed/fixed.php.inc'),
file_get_contents(__DIR__ . '/wrong/wrong.php.inc'),
],
[
file_get_contents(__DIR__ . '/fixed/fixed2.php.inc'),
file_get_contents(__DIR__ . '/wrong/wrong2.php.inc'),
],
];
}

protected function createFixer(): FixerInterface
{
return new ClassStringToClassConstantFixer;
}
}
3 changes: 3 additions & 0 deletions packages/CodingStandard/tests/Fixer/Php/fixed/fixed.php.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

$classType = \DateTime::class;
3 changes: 3 additions & 0 deletions packages/CodingStandard/tests/Fixer/Php/fixed/fixed2.php.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

$classType = \DateTimeInterface::class;
3 changes: 3 additions & 0 deletions packages/CodingStandard/tests/Fixer/Php/wrong/wrong.php.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

$classType = 'DateTime';
3 changes: 3 additions & 0 deletions packages/CodingStandard/tests/Fixer/Php/wrong/wrong2.php.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

$classType = 'DateTimeInterface';