Updated Rector/ECS YAML to PHP configuration, as current standard.
I'm currently working as Rector consultant for Spaceflow, Prague-based rising startup with very nice codebase. One of the pre-requisites for Rector is to have code that static analyser can work with. PHPStan that Rector uses depends on @var
annotations and not every property had that. Well... over 2500 of them.
I'll show you how we completed them without any manual change of the code and how you can do the same... today.
This post has 2 parts:
@var
types of used properties - good for analytical thinking and pattern-algorithms
The latter part increased the @var
annotation count from 1790 to 4418.
Here you can see the whole process in real-time:
So what will you do when you see property like this?
<?php
class ProductController
{
private $productRepository;
}
This is the most common case in most projects:
<?php
class ProductController
{
private $productRepository;
public function __construct(ProductRepository $productRepository)
{
// ah, it's a "ProductRepository"
$this->productRepository = $productRepository;
}
}
setUp
Assign in TestsThe next biggest group of missing @var
annotations was in tests. Instead of __construct
we havesetUp
:
<?php
class BuilderTest extends TestCase
{
private $factory;
protected function setUp(): void
{
// ah, it's a "Builder"
$this->factory = new Builder;
}
}
Sometimes constructor is for setting default values:
<?php
class HomeController
{
private $maxNumberOfGirlfriends;
public function __construct()
{
// ah, it's an "int"
$this->maxNumberOfGirlfriends = 1;
}
}
So far quite simple, right? 3 files are ok, but imagine doing this for 2500+ properties 🧠🤯
Some object don't have constructor injection, or are missing the information there, e.g.:
<?php
class Product
{
private $name;
public function __construct($name)
{
$this->name = $name;
}
}
How can we know, what is $name
? Well, we don't. Statical analysis can't help here, we can only guess and maybe crash the production code. To solve cases like this we need dynamical analysis - data are much better and solid than "probability guessing" of static analysis. Don't worry, I'll write about it in the future with a case study.
But what about this code:
<?php
class Product
{
private $name;
public function __construct($name)
{
$this->name = $name;
}
public function getName(): string
{
// now we know it's a "string"
return $this->name;
}
}
Although it could be also non-string value:
<?php
$product = new Product(5); // this passes without error
But the working code getName(): string
says it must be string
, so we can rely on it.
Also, this is just @var
annotation not typed properties, so the change cannot break anything.
Let's take this one step further. Also, let's be more realistic - there are barely any type declarations (PHP 7.0+) out there, so we have to take annotations into account:
<?php
class Product
{
private $name;
/**
* @param string $name
*/
public function __construct($name = null)
{
$this->name = $name;
}
/**
* @param $name
*/
public function setName($name)
{
$this->name = $name;
}
/**
* @return string
*/
public function getName()
{
return $this->name;
}
}
What types can be in $name
?
There is a type string
everywhere, so probably string
. But what happens in this case?
<?php
$product = new Product();
The $name
will be actually null
. So this is final correct code change:
<?php
class Product
{
+ /**
+ * @var string|null
+ */
private $name;
}
Before we get into Doctrine entities, let's get relaxed with a simple case:
<?php
class Victim
{
private $name = 'Tomas';
public function getName()
{
return $this->name;
}
}
I don't have to tell you the $name
property is always string
.
What if we have no constructor, no getters, no setters, no default values... are we lost?
This actually happens more often than you think. Take this as just an example, in reality it could be much better, but is often much worse:
<?php
class ProductController
{
private $activeProduct;
/**
* @var ProductRepository
*/
private $productRepository;
public function default($id = null)
{
if ($id) {
$this->activeProduct = $this->productRepository->get($id);
} else {
$this->activeProduct = $this->productRepository->getMainProduct();
}
// ...
}
}
This is hell to do manually. Not in 5 lines-long method, but in 30, 60 or even 100 lines in one method (don't forget you have to deliver feature and don't have paid time to play with stupid useless @var
annotation).
The process has just 2 step:
$this->activeProduct = X
X
expressionAnd that's it:
<?php
class ProductController
{
+ /**
+ * @var Product
+ */
private $activeProduct;
// ...
}
We could argue if the $activeProduct
could be also null
. It can, but in our case base it never was. But if you find such a case that is invalid for your code, please report an issue to improve this Rector rule.
Missing Doctrine annotations are hard to spot, because there is always some annotation. Just not the one we need:
<?php
use Doctrine\ORM\Mapping as ORM;
/**
* @ORM\Entity
*/
class Product
{
/**
* @ORM\Column(type="text", nullable=true)
*/
private $content;
}
Pretty easy, right?
/**
* @ORM\Column(type="text", nullable=true)
+ * @var text|null
*/
private $content;
Ups, not so fast. Database types and PHP scalar types are not 1:1 compatible. To pick a few: longblob, decimal, set, time or year (mini-quiz: int
or DateTimeInterface
)?
string
is the correct choice here:
/**
* @ORM\Column(type="text", nullable=true)
+ * @var string|null
*/
private $content;
Let's finish with the most complex case, yet brutally common. What do we mean by Doctrine Relation annotations?
<?php
class Product
{
/**
* @ORM\ManyToMany(targetEntity="App\Entity\Storage")
*/
private $storage;
/**
* @ORM\ManyToOne(targetEntity="App\Entity\Category", inversedBy="products")
* @ORM\JoinColumn(name="category_id", referencedColumnName="id", nullable=true)
*/
private $category;
}
The docblock looks full and rich for useful content, so it's hard to spot the missing @var
in there.
You probably know, that OneToMany
and ManyToMany
are not just an array of objects, but also Doctrine\Common\Collections\Collection
.
So what @var
we'd add here?
<?php
class Product
{
/**
* @ORM\ManyToMany(targetEntity="App\Entity\Storage")
+ * @var \App\Entity\Storage[]|\Doctrine\Common\Collections\Collection
*/
private $storage;
/**
* @ORM\ManyToOne(targetEntity="App\Entity\Category", inversedBy="products")
* @ORM\JoinColumn(name="category_id", referencedColumnName="id", nullable=true)
+ * @var \App\Entity\Category|null
*/
private $category;
}
That's it! Did you find any more cases that we forgot? Share with us in the comments.
Now the 2nd practical part - your project ↓
All above is not limited just to our project. You can use it too!
I'll guide you from step zero to final merge of pull-request, so the gif will not be just a picture, but a real change in your project.
Install Rector + ECS
composer require rector/rector --dev
composer require symplify/easy-coding-standard --dev
composer require slevomat/coding-standard --dev
Run ECS to check missing @var
annotations at properties
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
use Symplify\EasyCodingStandard\ValueObject\Option;
return function (ContainerConfigurator $containerConfigurator): void {
$services = $containerConfigurator->services();
// every property should have @var annotation
$services->set(SlevomatCodingStandard\Sniffs\TypeHints\TypeHintDeclarationSniff::class);
$parameters = $containerConfigurator->parameters();
$parameters->set(Option::SKIP, [
// this part is needed, because `TypeHintDeclarationSniff` is actually mix of 7 rules we don't need
// (they also delete code, so be sure to have this section here)
'SlevomatCodingStandard\Sniffs\TypeHints\TypeHintDeclarationSniff.UselessDocComment' => null,
'SlevomatCodingStandard\Sniffs\TypeHints\TypeHintDeclarationSniff.MissingTraversablePropertyTypeHintSpecification' => null,
'SlevomatCodingStandard\Sniffs\TypeHints\TypeHintDeclarationSniff.MissingTraversableReturnTypeHintSpecification' => null,
'SlevomatCodingStandard\Sniffs\TypeHints\TypeHintDeclarationSniff.MissingTraversableParameterTypeHintSpecification' => null,
'SlevomatCodingStandard\Sniffs\TypeHints\TypeHintDeclarationSniff.MissingParameterTypeHint' => null,
'SlevomatCodingStandard\Sniffs\TypeHints\TypeHintDeclarationSniff.MissingReturnTypeHint' => null,
]);
};
Run coding standard on your code:
vendor/bin/ecs check src tests
↓
2 500 errors? Not a problem.
1. Configure Rector
use Rector\TypeDeclaration\Rector\Property\PropertyTypeDeclarationRector;
use Rector\Config\RectorConfig;
return function (RectorConfig $rectorConfig): void {
$rectorConfig->rule(PropertyTypeDeclarationRector::class);
};
2. Run Rector to fix your code
vendor/bin/rector process src tests
Then run coding standard again, to see how useful Rector was:
vendor/bin/ecs check src tests
0 errors? Congrats and enjoy your vacation :)
1+ errors? Create an issue with missed PHP code snippet. We'll look at it and add support for it to Rector if possible.
I hope you'll find this useful as we did!
Happy coding!
Do you learn from my contents or use open-source packages like Rector every day?
Consider supporting it on GitHub Sponsors.
I'd really appreciate it!