Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for const types #41

Merged
merged 6 commits into from
Jun 14, 2024
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
12 changes: 9 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ Instead of fixing all PHPStan errors at once, we can start with minimal require

<br>

What is the type coverage you ask? We have 3 type possible declarations in total here:
What is the type coverage you ask? We have 4 type possible declarations in total here:

```php
final class ConferenceFactory
{
private $talkFactory;
public const SPEAKER_TAG = 'speaker';

public function createConference(array $data)
{
Expand All @@ -32,16 +33,20 @@ final class ConferenceFactory

The param type is defined, but property and return types are missing.

* 1 out of 3 = 33 % coverage
* 1 out of 4 = 25 % coverage

Our code has only one third quality it could have. Let's get to 100 %!
Our code quality is only at one-quarter of its potential. Let's get to 100 %!

```diff
final class ConferenceFactory
{
- private $talkFactory;
+ private TalkFactory $talkFactory;

- public const SPEAKER_TAG = 'speaker';
+ public const string SPEAKER_TAG = 'speaker';


- public function createConference(array $data)
+ public function createConference(array $data): Conference
{
Expand Down Expand Up @@ -79,6 +84,7 @@ parameters:
return: 50
param: 35.5
property: 70
constant: 85
```

<br>
Expand Down
9 changes: 9 additions & 0 deletions config/extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ parametersSchema:
return_type: anyOf(float(), int())
param_type: anyOf(float(), int())
property_type: anyOf(float(), int())
constant_type: anyOf(float(), int())
print_suggestions: bool()
# aliases to avoid typos
return: anyOf(schema(float(), nullable()), schema(int(), nullable()))
param: anyOf(schema(float(), nullable()), schema(int(), nullable()))
property: anyOf(schema(float(), nullable()), schema(int(), nullable()))
constant: anyOf(schema(float(), nullable()), schema(int(), nullable()))

# measure
measure: bool()
Expand All @@ -24,12 +26,14 @@ parameters:
return_type: 99
param_type: 99
property_type: 99
constant_type: 99
# default, yet deprecated
print_suggestions: true
# aliases
return: null
param: null
property: null
constant: null

measure: false

Expand Down Expand Up @@ -57,6 +61,10 @@ services:
class: TomasVotruba\TypeCoverage\Collectors\PropertyTypeDeclarationCollector
tags:
- phpstan.collector
-
class: TomasVotruba\TypeCoverage\Collectors\ConstantTypeDeclarationCollector
tags:
- phpstan.collector

-
class: TomasVotruba\TypeCoverage\Collectors\DeclareCollector
Expand All @@ -67,4 +75,5 @@ rules:
- TomasVotruba\TypeCoverage\Rules\ParamTypeCoverageRule
- TomasVotruba\TypeCoverage\Rules\ReturnTypeCoverageRule
- TomasVotruba\TypeCoverage\Rules\PropertyTypeCoverageRule
- TomasVotruba\TypeCoverage\Rules\ConstantTypeCoverageRule
- TomasVotruba\TypeCoverage\Rules\DeclareCoverageRule
5 changes: 5 additions & 0 deletions ecs.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,9 @@

return ECSConfig::configure()
->withPaths([__DIR__ . '/src', __DIR__ . '/tests'])
->withSkip([
// these fixtures use PHP 8.3 features, we cannot check them wit lower versions
__DIR__ . '/tests/Rules/ConstantTypeCoverageRule/Fixture/SkipKnownConstantType.php',
__DIR__ . '/tests/Rules/ConstantTypeCoverageRule/Fixture/UnknownConstantType.php',
])
->withPreparedSets(common: true, psr12: true, cleanCode: true, symplify: true);
1 change: 1 addition & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ parameters:
return_type: 99.9
param_type: 100
property_type: 100
constant_type: 0 # requires PHP 8.3

# only show final data, no error report
# measure: true
Expand Down
77 changes: 77 additions & 0 deletions src/Collectors/ConstantTypeDeclarationCollector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
<?php

declare(strict_types=1);

namespace TomasVotruba\TypeCoverage\Collectors;

use PhpParser\Node;
use PhpParser\Node\Stmt\ClassConst;
use PHPStan\Analyser\Scope;
use PHPStan\Collectors\Collector;
use PHPStan\Node\ClassConstantsNode;
use PHPStan\Reflection\ClassReflection;

/**
* @see \TomasVotruba\TypeCoverage\Rules\ConstantTypeCoverageRule
*/
final class ConstantTypeDeclarationCollector implements Collector
{
/**
* @return class-string<Node>
*/
public function getNodeType(): string
{
return ClassConstantsNode::class;
}

/**
* @param ClassConstantsNode $node
* @return mixed[]
*/
public function processNode(Node $node, Scope $scope): array
{
// return typed properties/all properties
if (! $node instanceof ClassConstantsNode) {
throw new \LogicException('Node is ' . $node::class . ' instead of ' . ClassConstantsNode::class);
}

$constantCount = count($node->getConstants());

$missingTypeLines = [];

foreach ($node->getConstants() as $constant) {
// blocked by parent type
if ($this->isGuardedByParentClassConstant($scope, $constant)) {
continue;
}

// already typed
if ($constant->type instanceof Node) {
continue;
}

// give useful context
$missingTypeLines[] = $constant->getLine();
}

return [$constantCount, $missingTypeLines];
}

private function isGuardedByParentClassConstant(Scope $scope, ClassConst $const): bool
{
$constName = $const->consts[0]->name->toString();

$classReflection = $scope->getClassReflection();
if (! $classReflection instanceof ClassReflection) {
return false;
}

foreach ($classReflection->getParents() as $parentClassReflection) {
if ($parentClassReflection->hasConstant($constName)) {
return true;
}
}

return false;
}
}
5 changes: 5 additions & 0 deletions src/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ public function getRequiredPropertyTypeLevel(): float|int
return $this->parameters['property'] ?? $this->parameters['property_type'];
}

public function getRequiredConstantTypeLevel(): float|int
{
return $this->parameters['constant'] ?? $this->parameters['constant_type'];
}

public function getRequiredParamTypeLevel(): float|int
{
return $this->parameters['param'] ?? $this->parameters['param_type'];
Expand Down
72 changes: 72 additions & 0 deletions src/Rules/ConstantTypeCoverageRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<?php

declare(strict_types=1);

namespace TomasVotruba\TypeCoverage\Rules;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\CollectedDataNode;
use PHPStan\Rules\Rule;
use TomasVotruba\TypeCoverage\CollectorDataNormalizer;
use TomasVotruba\TypeCoverage\Collectors\ConstantTypeDeclarationCollector;
use TomasVotruba\TypeCoverage\Configuration;
use TomasVotruba\TypeCoverage\Formatter\TypeCoverageFormatter;

/**
* @see \TomasVotruba\TypeCoverage\Tests\Rules\PropertyTypeCoverageRule\ConstantTypeCoverageRuleTest
*
* @implements Rule<CollectedDataNode>
*/
final readonly class ConstantTypeCoverageRule implements Rule
{
/**
* @var string
*/
public const ERROR_MESSAGE = 'Out of %d possible constant types, only %d - %.1f %% actually have it. Add more constant types to get over %s %%';

public function __construct(
private TypeCoverageFormatter $typeCoverageFormatter,
private Configuration $configuration,
private CollectorDataNormalizer $collectorDataNormalizer,
) {
}

/**
* @return class-string<Node>
*/
public function getNodeType(): string
{
return CollectedDataNode::class;
}

/**
* @param CollectedDataNode $node
* @return mixed[]
*/
public function processNode(Node $node, Scope $scope): array
{
$constantTypeDeclarationCollector = $node->get(ConstantTypeDeclarationCollector::class);
$typeCountAndMissingTypes = $this->collectorDataNormalizer->normalize($constantTypeDeclarationCollector);

if ($this->configuration->showOnlyMeasure()) {
return [
sprintf(
'Class constant type coverage is %.1f %% out of %d possible',
$typeCountAndMissingTypes->getCoveragePercentage(),
$typeCountAndMissingTypes->getTotalCount()
),
];
}

if ($this->configuration->getRequiredConstantTypeLevel() === 0) {
return [];
}

return $this->typeCoverageFormatter->formatErrors(
self::ERROR_MESSAGE,
$this->configuration->getRequiredConstantTypeLevel(),
$typeCountAndMissingTypes
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php

declare(strict_types=1);

namespace TomasVotruba\TypeCoverage\Tests\Rules\ConstantTypeCoverageRule;

use Iterator;
use PHPStan\Collectors\Collector;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use PHPUnit\Framework\Attributes\DataProvider;
use TomasVotruba\TypeCoverage\Collectors\ConstantTypeDeclarationCollector;
use TomasVotruba\TypeCoverage\Rules\ConstantTypeCoverageRule;

final class ConstantTypeCoverageRuleTest extends RuleTestCase
{
public static function setUpBeforeClass(): void
{
if (PHP_VERSION_ID < 80300) {
self::markTestSkipped('not working under 8.3');
}
}

/**
* @param string[] $filePaths
* @param mixed[] $expectedErrorsWithLines
*/
#[DataProvider('provideData')]
public function testRule(array $filePaths, array $expectedErrorsWithLines): void
{
$this->analyse($filePaths, $expectedErrorsWithLines);
}

public static function provideData(): Iterator
{
echo __METHOD__;
yield [[__DIR__ . '/Fixture/SkipKnownConstantType.php'], []];
yield [[__DIR__ . '/Fixture/SkipParentBlockingConstant.php'], []];

$errorMessage = sprintf(ConstantTypeCoverageRule::ERROR_MESSAGE, 2, 1, 50.0, 80);
yield [[__DIR__ . '/Fixture/UnknownConstantType.php'], [[$errorMessage, 9]]];
}

/**
* @return string[]
*/
public static function getAdditionalConfigFiles(): array
{
return [__DIR__ . '/config/configured_rule.neon'];
}

protected function getRule(): Rule
{
return self::getContainer()->getByType(ConstantTypeCoverageRule::class);
}

/**
* @return Collector[]
*/
protected function getCollectors(): array
{
return [self::getContainer()->getByType(ConstantTypeDeclarationCollector::class)];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

namespace TomasVotruba\TypeCoverage\Tests\Rules\ConstantTypeCoverageRule\Fixture;

final class SkipKnownConstantType
{
public const string NAME = 'name';
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

declare(strict_types=1);

namespace TomasVotruba\TypeCoverage\Tests\Rules\ConstantTypeCoverageRule\Fixture;

use TomasVotruba\TypeCoverage\Tests\Rules\ConstantTypeCoverageRule\Source\ParentBlockingClass;

final class SkipParentBlockingConstant extends ParentBlockingClass
{
/**
* @var string[]
*/
public const NO_TYPE = ['first_no_type', 'second_no_type'];
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

declare(strict_types=1);

namespace TomasVotruba\TypeCoverage\Tests\Rules\ConstantTypeCoverageRule\Fixture;

final class UnknownConstantType
{
public const TAG = 'foo';

public const int AMOUNT = 123;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

namespace TomasVotruba\TypeCoverage\Tests\Rules\ConstantTypeCoverageRule\Source;

class ParentBlockingClass
{
public const NO_TYPE = '';
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
includes:
- ../../../../config/extension.neon

parameters:
type_coverage:
constant_type: 80
Loading