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 all 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
17 changes: 14 additions & 3 deletions easy-coding-standard.neon
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
checkers:
- Symplify\CodingStandard\Fixer\Php\ClassStringToClassConstantFixer
- Symplify\CodingStandard\Fixer\Property\ArrayPropertyDefaultValueFixer

# Classes
Expand Down Expand Up @@ -80,6 +81,9 @@ checkers:
- Symplify\CodingStandard\Sniffs\Namespaces\ClassNamesWithoutPreSlashSniff
- PhpCsFixer\Fixer\NamespaceNotation\SingleBlankLineBeforeNamespaceFixer
- PhpCsFixer\Fixer\NamespaceNotation\BlankLineAfterNamespaceFixer
- PhpCsFixer\Fixer\Import\OrderedImportsFixer
- PhpCsFixer\Fixer\Import\SingleImportPerStatementFixer
- PhpCsFixer\Fixer\Import\SingleLineAfterImportsFixer

# Naming
- Symplify\CodingStandard\Sniffs\Naming\AbstractClassNameSniff
Expand Down Expand Up @@ -129,7 +133,6 @@ checkers:
- PhpCsFixer\Fixer\Operator\NotOperatorWithSuccessorSpaceFixer
- PhpCsFixer\Fixer\ControlStructure\NoUselessElseFixer
- PhpCsFixer\Fixer\ReturnNotation\NoUselessReturnFixer
- PhpCsFixer\Fixer\Import\OrderedImportsFixer
- PhpCsFixer\Fixer\LanguageConstruct\CombineConsecutiveUnsetsFixer
- PhpCsFixer\Fixer\Strict\StrictComparisonFixer
- PhpCsFixer\Fixer\Alias\EregToPregFixer
Expand Down Expand Up @@ -158,8 +161,6 @@ checkers:
- PhpCsFixer\Fixer\Whitespace\SingleBlankLineAtEofFixer
PhpCsFixer\Fixer\ClassNotation\SingleClassElementPerStatementFixer:
- property
- PhpCsFixer\Fixer\Import\SingleImportPerStatementFixer
- PhpCsFixer\Fixer\Import\SingleLineAfterImportsFixer
- PhpCsFixer\Fixer\ControlStructure\SwitchCaseSemicolonToColonFixer
- PhpCsFixer\Fixer\ControlStructure\SwitchCaseSpaceFixer

Expand Down Expand Up @@ -282,3 +283,13 @@ parameters:
PhpCsFixer\Fixer\Semicolon\SpaceAfterSemicolonFixer:
# bugged fixer
- packages/CodingStandard/src/Fixer/DependencyInjection/InjectToConstructorInjectionFixer.php

Symplify\CodingStandard\Fixer\Php\ClassStringToClassConstantFixer:
# optional package, interface might not exist
- packages/CodingStandard/src/Sniffs/Classes/EqualInterfaceImplementationSniff.php
# "Exception" is string part of the name
- packages/CodingStandard/src/Sniffs/Naming/ExceptionNameSniff.php
# "Error" is a name of Presenter
- packages/SymbioticController/tests/Adapter/Nette/Application/PresenterFactoryTest.php
# class is only for tested cases, it doesn't exist
- packages/SymbioticController/tests/Adapter/Nette/Routing/PresenterMapperTest.php
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
```


### `::class` references should be used over string for classes and interfaces


- [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,116 @@
<?php declare(strict_types=1);

namespace Symplify\CodingStandard\Fixer\Php;

use PhpCsFixer\Fixer\DefinedFixerInterface;
use PhpCsFixer\FixerDefinition\CodeSample;
use PhpCsFixer\FixerDefinition\FixerDefinition;
use PhpCsFixer\FixerDefinition\FixerDefinitionInterface;
use PhpCsFixer\Tokenizer\Token;
use PhpCsFixer\Tokenizer\Tokens;
use SplFileInfo;

final class ClassStringToClassConstantFixer implements DefinedFixerInterface
{
/**
* @var string
*/
private const CLASS_OR_INTERFACE_PATTERN = '#^[A-Z]\w*[a-z]\w*(\\\\[A-Z]\w*[a-z]\w*)+\z#';
Copy link

Choose a reason for hiding this comment

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

a lot of falsy positives here

Copy link
Member Author

Choose a reason for hiding this comment

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

Suggestion?

Copy link

Choose a reason for hiding this comment

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

drop anything that may creates wrong changes, it's better to convert too few than too much

Copy link
Member Author

Choose a reason for hiding this comment

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

For example?


public function getDefinition(): FixerDefinitionInterface
{
return new FixerDefinition(
'`::class` references should be used over string for classes and interfaces.',
[
new CodeSample(
'<?php

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

$interfaceName = "DateTimeInterface";
'),
new CodeSample(
'<?php

$interfaceName = "Nette\Utils\DateTime";
'),
]
);
}

public function isCandidate(Tokens $tokens): bool
{
return $tokens->isTokenKindFound(T_CONSTANT_ENCAPSED_STRING);
}

public function fix(SplFileInfo $file, Tokens $tokens): void
{
/** @var Token[] $revertedTokens */
$revertedTokens = array_reverse($tokens->toArray(), true);

foreach ($revertedTokens as $index => $token) {
if (! $token->isGivenKind(T_CONSTANT_ENCAPSED_STRING)) {
continue;
}

// remove quotes "" around the string
$potentialClassOrInterface = substr($token->getContent(), 1, -1);
if (! $this->isClassOrInterface($potentialClassOrInterface)) {
continue;
}

$token->clear();
$tokens->insertAt($index, $this->convertClassOrInterfaceNameToTokens($potentialClassOrInterface));
}
}

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

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

public function getPriority(): int
{
// should be run before the OrderedImportsFixer, after the NoLeadingImportSlashFixer
return -25;
}

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

private function isClassOrInterface(string $potentialClassOrInterface): bool
{
// exception for often used "error" string; because class_exists() is case-insensitive
if ($potentialClassOrInterface === 'error') {
return false;
}

return class_exists($potentialClassOrInterface)
|| interface_exists($potentialClassOrInterface)
|| (bool) preg_match(self::CLASS_OR_INTERFACE_PATTERN, $potentialClassOrInterface);
}

private function convertClassOrInterfaceNameToTokens(string $potentialClassOrInterface): Tokens
{
$tokens = Tokens::fromCode(sprintf(
'<?php echo \\%s::class;',
$potentialClassOrInterface
));

$tokens->clearRange(0, 2); // clear start "<?php"
$tokens[$tokens->getSize() - 1]->clear(); // clear end ";"
$tokens->clearEmptyTokens();

return $tokens;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

namespace Symplify\CodingStandard\Sniffs\Namespaces;

use DateTime;
use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;
use SplFileInfo;
use stdClass;
use Throwable;

/**
* Rules:
Expand All @@ -15,7 +19,7 @@ final class ClassNamesWithoutPreSlashSniff implements Sniff
* @var string[]
*/
private $excludedClassNames = [
'DateTime', 'stdClass', 'splFileInfo', 'Exception',
DateTime::class, stdClass::class, SplFileInfo::class, Throwable::class,
];

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?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'),
],
[
file_get_contents(__DIR__ . '/fixed/fixed3.php.inc'),
file_get_contents(__DIR__ . '/wrong/wrong3.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;
9 changes: 9 additions & 0 deletions packages/CodingStandard/tests/Fixer/Php/fixed/fixed3.php.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

class SomeClass
{
public function getSomeData($className = \Nette\Utils\DateTime::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';
9 changes: 9 additions & 0 deletions packages/CodingStandard/tests/Fixer/Php/wrong/wrong3.php.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

class SomeClass
{
public function getSomeData($className = 'Nette\Utils\DateTime')
{

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ private function getUnsortedMessages(): array
{
return [
'filePath' => [
new Error(5, 'error', 'SomeClass', false),
new Error(5, 'error message', 'SomeClass', false),
],
'anotherFilePath' => [
new Error(15, 'error', 'SomeClass', false),
new Error(5, 'error', 'SomeClass', false),
new Error(15, 'error message', 'SomeClass', false),
new Error(5, 'error message', 'SomeClass', false),
],
];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Symplify\SymbioticController\Tests\Adapter\Nette\Application;

use Error;
use Nette\Application\IPresenter;
use Nette\Application\IPresenterFactory;
use Nette\Application\UI\Presenter;
Expand Down Expand Up @@ -65,12 +64,9 @@ public function testGetPresenterClassForString(): void

public function testCreateErrorPresenter(): void
{
$this->assertTrue(class_exists('Error'));

$this->setupPresenterFactoryMapping();

$errorPresenter = $this->presenterFactory->createPresenter('Error');
$this->assertNotInstanceOf(Error::class, $errorPresenter);
$this->assertInstanceOf(ErrorPresenter::class, $errorPresenter);

/* @var Presenter $somePresenter */
Expand Down