Skip to content

Commit

Permalink
Merge pull request #3325 from rectorphp/solid-const
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasVotruba authored May 6, 2020
2 parents 420c8c5 + b0bd524 commit e0f2366
Show file tree
Hide file tree
Showing 65 changed files with 1,082 additions and 239 deletions.
1 change: 1 addition & 0 deletions config/set/solid/solid.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ services:
Rector\SOLID\Rector\ClassMethod\ChangeReadOnlyVariableWithDefaultValueToConstantRector: null
Rector\SOLID\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector: null
Rector\SOLID\Rector\Property\AddFalseDefaultToBoolPropertyRector: null
Rector\SOLID\Rector\Class_\RepeatedLiteralToClassConstantRector: null
29 changes: 28 additions & 1 deletion docs/rector_rules_overview.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# All 493 Rectors Overview
# All 494 Rectors Overview

- [Projects](#projects)
- [General](#general)
Expand Down Expand Up @@ -9126,6 +9126,33 @@ Split if statement, when if condition always break execution flow

<br>

### `RepeatedLiteralToClassConstantRector`

- class: [`Rector\SOLID\Rector\Class_\RepeatedLiteralToClassConstantRector`](/../master/rules/solid/src/Rector/Class_/RepeatedLiteralToClassConstantRector.php)
- [test fixtures](/../master/rules/solid/tests/Rector/Class_/RepeatedLiteralToClassConstantRector/Fixture)

Replace repeated strings with constant

```diff
class SomeClass
{
+ /**
+ * @var string
+ */
+ private const REQUIRES = 'requires';
public function run($key, $items)
{
- if ($key === 'requires') {
- return $items['requires'];
+ if ($key === self::REQUIRES) {
+ return $items[self::REQUIRES];
}
}
}
```

<br>

### `UseInterfaceOverImplementationInConstructorRector`

- class: [`Rector\SOLID\Rector\ClassMethod\UseInterfaceOverImplementationInConstructorRector`](/../master/rules/solid/src/Rector/ClassMethod/UseInterfaceOverImplementationInConstructorRector.php)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,19 @@ final class EntityTagValueNode extends AbstractDoctrineTagValueNode implements P
{
use PhpAttributePhpDocNodePrintTrait;

/**
* @var string
*/
private const REPOSITORY_CLASS = 'repositoryClass';

/**
* @var string
*/
private const READ_ONLY = 'readOnly';

public function removeRepositoryClass(): void
{
$this->items['repositoryClass'] = null;
$this->items[self::REPOSITORY_CLASS] = null;
}

public function getShortName(): string
Expand All @@ -32,12 +42,12 @@ private function createAttributeItems(): array
{
$items = $this->items;

if ($items['repositoryClass'] !== null) {
$items['repositoryClass'] .= '::class';
if ($items[self::REPOSITORY_CLASS] !== null) {
$items[self::REPOSITORY_CLASS] .= '::class';
}

if ($items['readOnly'] !== null) {
$items['readOnly'] = $items['readOnly'] ? 'ORM\Entity::READ_ONLY' : '';
if ($items[self::READ_ONLY] !== null) {
$items[self::READ_ONLY] = $items[self::READ_ONLY] ? 'ORM\Entity::READ_ONLY' : '';
}

return $items;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ final class ManyToManyTagValueNode extends AbstractDoctrineTagValueNode implemen
{
use PhpAttributePhpDocNodePrintTrait;

/**
* @var string
*/
private const TARGET_ENTITY = 'targetEntity';

/**
* @var string|null
*/
Expand All @@ -32,7 +37,7 @@ public function __construct(

public function getTargetEntity(): string
{
return $this->items['targetEntity'];
return $this->items[self::TARGET_ENTITY];
}

public function getFullyQualifiedTargetEntity(): ?string
Expand Down Expand Up @@ -62,7 +67,7 @@ public function removeInversedBy(): void

public function changeTargetEntity(string $targetEntity): void
{
$this->items['targetEntity'] = $targetEntity;
$this->items[self::TARGET_ENTITY] = $targetEntity;
}

public function getShortName(): string
Expand All @@ -78,7 +83,7 @@ public function toAttributeString(): string
private function createAttributeItems(): array
{
$items = $this->items;
$items['targetEntity'] .= '::class';
$items[self::TARGET_ENTITY] .= '::class';

return $items;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,32 @@

final class SerializerTypeTagValueNode extends AbstractTagValueNode implements TypeAwareTagValueNodeInterface, ShortNameAwareTagInterface, SilentKeyNodeInterface
{
/**
* @var string
*/
private const NAME = 'name';

public function getShortName(): string
{
return '@Serializer\Type';
}

public function changeName(string $newName): void
{
$this->items['name'] = $newName;
$this->items[self::NAME] = $newName;
}

public function getName(): string
{
return $this->items['name'];
return $this->items[self::NAME];
}

public function replaceName(string $oldName, string $newName): bool
{
$oldNamePattern = '#\b' . preg_quote($oldName, '#') . '\b#';

$newNameValue = Strings::replace($this->items['name'], $oldNamePattern, $newName);
if ($newNameValue !== $this->items['name']) {
$newNameValue = Strings::replace($this->items[self::NAME], $oldNamePattern, $newName);
if ($newNameValue !== $this->items[self::NAME]) {
$this->changeName($newNameValue);
return true;
}
Expand All @@ -42,6 +47,6 @@ public function replaceName(string $oldName, string $newName): bool

public function getSilentKey(): string
{
return 'name';
return self::NAME;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@

final class TypeNodeAnalyzerTest extends AbstractKernelTestCase
{
/**
* @var string
*/
private const INT = 'int';

/**
* @var TypeNodeAnalyzer
*/
Expand All @@ -38,9 +43,9 @@ public function testContainsArrayType(TypeNode $typeNode, bool $expectedContains

public function provideDataForArrayType(): Iterator
{
$arrayTypeNode = new ArrayTypeNode(new IdentifierTypeNode('int'));
$arrayTypeNode = new ArrayTypeNode(new IdentifierTypeNode(self::INT));

yield [new IdentifierTypeNode('int'), false];
yield [new IdentifierTypeNode(self::INT), false];
yield [$arrayTypeNode, true];
yield [new UnionTypeNode([$arrayTypeNode]), true];
}
Expand All @@ -55,7 +60,7 @@ public function testIsIntersectionAndNotNullable(TypeNode $typeNode, bool $expec

public function provideDataForIntersectionAndNotNullable(): Iterator
{
yield [new IntersectionTypeNode([new IdentifierTypeNode('int')]), true];
yield [new IntersectionTypeNode([new NullableTypeNode(new IdentifierTypeNode('int'))]), false];
yield [new IntersectionTypeNode([new IdentifierTypeNode(self::INT)]), true];
yield [new IntersectionTypeNode([new NullableTypeNode(new IdentifierTypeNode(self::INT))]), false];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Interface_;
use PhpParser\Node\Stmt\Trait_;
use Rector\Core\Exception\NotImplementedException;
use Rector\Core\Exception\ShouldNotHappenException;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeTypeResolver\Node\AttributeKey;
Expand Down Expand Up @@ -232,7 +231,7 @@ public function findClassConstantByClassConstFetch(ClassConstFetch $classConstFe

$class = $this->resolveClassConstant($classConstFetch, $className);
if ($class === null) {
throw new NotImplementedException();
return null;
}

/** @var string $constantName */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@

final class VoidTypeMapper implements TypeMapperInterface
{
/**
* @var string
*/
private const VOID = 'void';

/**
* @var PhpVersionProvider
*/
Expand All @@ -36,7 +41,7 @@ public function getNodeClass(): string
*/
public function mapToPHPStanPhpDocTypeNode(Type $type): TypeNode
{
return new IdentifierTypeNode('void');
return new IdentifierTypeNode(self::VOID);
}

/**
Expand All @@ -52,7 +57,7 @@ public function mapToPhpParserNode(Type $type, ?string $kind = null): ?Node
return null;
}

return new Identifier('void');
return new Identifier(self::VOID);
}

public function mapToDocString(Type $type, ?Type $parentType = null): string
Expand All @@ -63,6 +68,6 @@ public function mapToDocString(Type $type, ?Type $parentType = null): string
}

// fallback for PHP 7.0 and older, where void type was only in docs
return 'void';
return self::VOID;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@

final class ComposerPackageAutoloadUpdater
{
/**
* @var string
*/
private const PSR_4 = 'psr-4';

/**
* @var JsonFileSystem
*/
Expand Down Expand Up @@ -54,8 +59,8 @@ public function processComposerAutoload(Configuration $configuration): void
return;
}

$composerJson['autoload']['psr-4'][$package->getSrcNamespace()] = $package->getSrcDirectory();
$composerJson['autoload-dev']['psr-4'][$package->getTestsNamespace()] = $package->getTestsDirectory();
$composerJson['autoload'][self::PSR_4][$package->getSrcNamespace()] = $package->getSrcDirectory();
$composerJson['autoload-dev'][self::PSR_4][$package->getTestsNamespace()] = $package->getTestsDirectory();

$this->jsonFileSystem->saveJsonToFile($composerJsonFilePath, $composerJson);

Expand Down Expand Up @@ -83,7 +88,7 @@ private function resolvePackage(Configuration $configuration): Package

private function isPackageAlreadyLoaded(array $composerJson, Package $package): bool
{
return isset($composerJson['autoload']['psr-4'][$package->getSrcNamespace()]);
return isset($composerJson['autoload'][self::PSR_4][$package->getSrcNamespace()]);
}

private function rebuildAutoload(): void
Expand Down
10 changes: 10 additions & 0 deletions packages/static-type-mapper/src/Mapper/PhpParserNodeMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
namespace Rector\StaticTypeMapper\Mapper;

use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Scalar\String_;
use PHPStan\Type\Type;
use Rector\Core\Exception\NotImplementedException;
use Rector\StaticTypeMapper\Contract\PhpParser\PhpParserNodeMapperInterface;
Expand All @@ -31,6 +33,14 @@ public function mapToPHPStanType(Node $node): Type
continue;
}

// do not let Expr collect all the types
// note: can be solve later with priorities on mapper interface, making this last
if ($phpParserNodeMapper->getNodeType() === Expr::class) {
if (is_a($node, String_::class)) {
continue;
}
}

return $phpParserNodeMapper->mapToPHPStan($node);
}

Expand Down
27 changes: 27 additions & 0 deletions packages/static-type-mapper/src/PhpParser/StringNodeMapper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

declare(strict_types=1);

namespace Rector\StaticTypeMapper\PhpParser;

use PhpParser\Node;
use PhpParser\Node\Scalar\String_;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
use Rector\StaticTypeMapper\Contract\PhpParser\PhpParserNodeMapperInterface;

final class StringNodeMapper implements PhpParserNodeMapperInterface
{
public function getNodeType(): string
{
return String_::class;
}

/**
* @param String_ $node
*/
public function mapToPHPStan(Node $node): Type
{
return new StringType();
}
}
29 changes: 17 additions & 12 deletions rector-ci.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
services:
Rector\SOLID\Rector\Class_\RepeatedLiteralToClassConstantRector: null

parameters:
sets:
- 'coding-style'
- 'code-quality'
- 'dead-code'
- 'nette-utils-code-quality'
- 'solid'
- 'privatization'
- 'naming'
# sets:
# - 'coding-style'
# - 'code-quality'
# - 'dead-code'
# - 'nette-utils-code-quality'
# - 'solid'
# - 'privatization'
# - 'naming'

paths:
- 'src'
Expand All @@ -23,7 +26,9 @@ parameters:
- 'packages/doctrine-annotation-generated/src/ConstantPreservingDocParser.php'
- 'packages/doctrine-annotation-generated/src/ConstantPreservingAnnotationReader.php'

exclude_rectors:
# @todo needs to check for "autoload-dev" local dependency
- 'Rector\Php55\Rector\String_\StringClassNameToClassConstantRector'
- 'Rector\CodingStyle\Rector\String_\SplitStringClassConstantToClassConstFetchRector'
# exclude_rectors:
# # @todo needs to check for "autoload-dev" local dependency
# - 'Rector\Php55\Rector\String_\StringClassNameToClassConstantRector'
# - 'Rector\CodingStyle\Rector\String_\SplitStringClassConstantToClassConstFetchRector'
# # fix in separated PR
# - 'Rector\SOLID\Rector\Class_\RepeatedLiteralToClassConstantRector'
8 changes: 1 addition & 7 deletions rector.yaml
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
imports:
- { resource: "create-rector.yaml", ignore_errors: 'not_found' }

services:
Rector\Core\Rector\ClassMethod\ChangeContractMethodSingleToManyRector:
$oldToNewMethodByType:
Rector\BetterPhpDocParser\Contract\PhpDocNodeFactoryInterface:
getClass: 'getClasses'

parameters:
# bleeding edge feature
is_cache_enabled: true
# is_cache_enabled: true

auto_import_names: true

Expand Down
Loading

0 comments on commit e0f2366

Please sign in to comment.