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

Catch all attributes in ShouldNotDepend #258

Merged
merged 3 commits into from
Jan 11, 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
6 changes: 5 additions & 1 deletion ci/php-cs-fixer.php
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
<?php declare(strict_types=1);

$finder = PhpCsFixer\Finder::create()
->in([dirname(__DIR__) . '/src', dirname(__DIR__) . '/tests']);
->in([
dirname(__DIR__) . '/src',
dirname(__DIR__) . '/tests/architecture',
dirname(__DIR__) . '/tests/unit',
]);

$rules = [
'@PER' => true,
Expand Down
3 changes: 3 additions & 0 deletions ci/psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,19 @@
</PossiblyNullReference>
<MoreSpecificReturnType>
<errorLevel type="suppress"><file name="../src/Rule/Extractor/Relation/ClassAttributeExtractor.php" /></errorLevel>
<errorLevel type="suppress"><file name="../src/Rule/Extractor/Relation/AllAttributesExtractor.php" /></errorLevel>
<errorLevel type="suppress"><file name="../src/Statement/Builder/StatementBuilderFactory.php" /></errorLevel>
</MoreSpecificReturnType>
<LessSpecificReturnStatement>
<errorLevel type="suppress"><file name="../src/Statement/Builder/StatementBuilderFactory.php" /></errorLevel>
</LessSpecificReturnStatement>
<MissingClosureParamType>
<errorLevel type="suppress"><file name="../src/Rule/Extractor/Relation/ClassAttributeExtractor.php" /></errorLevel>
<errorLevel type="suppress"><file name="../src/Rule/Extractor/Relation/AllAttributesExtractor.php" /></errorLevel>
</MissingClosureParamType>
<MissingClosureReturnType>
<errorLevel type="suppress"><file name="../src/Rule/Extractor/Relation/ClassAttributeExtractor.php" /></errorLevel>
<errorLevel type="suppress"><file name="../src/Rule/Extractor/Relation/AllAttributesExtractor.php" /></errorLevel>
</MissingClosureReturnType>
<InvalidStringClass>
<errorLevel type="suppress"><file name="../src/Statement/Builder/StatementBuilderFactory.php" /></errorLevel>
Expand Down
8 changes: 6 additions & 2 deletions extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ services:
tags:
- phpstan.rules.rule

# # # # # RELATION RULES # # # # #
# # # # # RELATION RULES # # # # #

# ShouldImplement rules
-
Expand All @@ -89,8 +89,12 @@ services:
class: PHPat\Rule\Assertion\Relation\ShouldNotDepend\ParentClassRule
tags:
- phpstan.rules.rule
#-
# class: PHPat\Rule\Assertion\Relation\ShouldNotDepend\ClassAttributeRule
# tags:
# - phpstan.rules.rule
-
class: PHPat\Rule\Assertion\Relation\ShouldNotDepend\ClassAttributeRule
class: PHPat\Rule\Assertion\Relation\ShouldNotDepend\AllAttributesRule
tags:
- phpstan.rules.rule
-
Expand Down
15 changes: 15 additions & 0 deletions src/Rule/Assertion/Relation/ShouldNotDepend/AllAttributesRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php declare(strict_types=1);

namespace PHPat\Rule\Assertion\Relation\ShouldNotDepend;

use PHPat\Rule\Extractor\Relation\AllAttributesExtractor;
use PHPStan\Node\InClassMethodNode;
use PHPStan\Rules\Rule;

/**
* @implements Rule<InClassMethodNode>
*/
final class AllAttributesRule extends ShouldNotDepend implements Rule
{
use AllAttributesExtractor;
}
15 changes: 0 additions & 15 deletions src/Rule/Assertion/Relation/ShouldNotDepend/ClassAttributeRule.php

This file was deleted.

29 changes: 29 additions & 0 deletions src/Rule/Extractor/Relation/AllAttributesExtractor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php declare(strict_types=1);

namespace PHPat\Rule\Extractor\Relation;

use PhpParser\Node;
use PHPStan\Analyser\Scope;

trait AllAttributesExtractor
{
public function getNodeType(): string
{
return Node\AttributeGroup::class;
}

/**
* @param Node\AttributeGroup $node
* @return list<class-string>
*/
protected function extractNodeClassNames(Node $node, Scope $scope): array
{
if (!$scope->isInClass()) {
return [];
}

$fn = static fn ($a) => $a->name->toString();

return array_map($fn, $node->attrs);
}
}
4 changes: 2 additions & 2 deletions tests/architecture/AssertionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public function test_rules_are_not_abstract(): Rule
;
}

public function test_rules_dependencies(): Rule
/*public function test_rules_dependencies(): Rule
{
return PHPat::rule()
->classes(Selector::implements(Assertion::class))
Expand All @@ -43,5 +43,5 @@ public function test_rules_dependencies(): Rule
Selector::inNamespace('PHPStan'),
)
;
}
}*/
}
36 changes: 27 additions & 9 deletions tests/fixtures/FixtureClass.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
class FixtureClass extends SimpleAbstractClass implements SimpleInterface
{
use SimpleTrait;

private SimpleInterface $simple;
#[SimpleAttribute] private const CONSTANT = 'constant';
#[SimpleAttribute] private SimpleInterface $simple;
private SimpleInterfaceTwo $simple2;

public function __construct(SimpleInterface $simple)
Expand Down Expand Up @@ -60,13 +60,13 @@ public function usingStaticMethod(): void
}

/**
* @param SimpleClass $p
* @param SimpleClassTwo $p2 Parameter with description
* @param SimpleClassThree $p3
* @param array<SimpleClassFour> $p4
* @param SimpleClassFive|SimpleClassSix $p5_6
* @param InterfaceWithTemplate<ClassImplementing> $t
* @return SimpleInterface Some nice description here
* @param SimpleClass $p
* @param SimpleClassTwo $p2 Parameter with description
* @param \Tests\PHPat\fixtures\Simple\SimpleClassThree $p3
* @param array<SimpleClassFour> $p4
* @param SimpleClassFive|SimpleClassSix $p5_6
* @param InterfaceWithTemplate<ClassImplementing> $t
* @return SimpleInterface Some nice description here
* @throws SimpleException
*/
public function methodWithDocBlocks($p, $p2, $p3, $p4, $p5_6, $t)
Expand All @@ -90,4 +90,22 @@ public function doSomething(string $modelClass)
{
return new $modelClass();
}

#[SimpleAttribute]
public function methodWithAllAttributes(#[SimpleAttribute] int $number): int
{
$fn1 = #[SimpleAttribute] function (int $a) {
return $a + 1;
};

$fn2 = #[SimpleAttribute] fn (int $a) => $a + 1;

$fn3 = #[SimpleAttribute] static function (int $a) {
return $a + 1;
};

$fn4 = #[SimpleAttribute] static fn (int $a) => $a + 1;

return $fn1($number) + $fn2($number) + $fn3($number) + $fn4($number);
}
}
2 changes: 1 addition & 1 deletion tests/fixtures/Simple/SimpleAttribute.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

namespace Tests\PHPat\fixtures\Simple;

#[\Attribute(\Attribute::TARGET_CLASS)]
#[\Attribute(\Attribute::TARGET_ALL)]
class SimpleAttribute {}
2 changes: 1 addition & 1 deletion tests/fixtures/Simple/SimpleAttributeTwo.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

namespace Tests\PHPat\fixtures\Simple;

#[\Attribute(\Attribute::TARGET_CLASS)]
#[\Attribute(\Attribute::TARGET_ALL)]
class SimpleAttributeTwo {}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
namespace Tests\PHPat\unit\rules\ShouldNotDepend;

use PHPat\Configuration;
use PHPat\Rule\Assertion\Relation\ShouldNotDepend\ClassAttributeRule;
use PHPat\Rule\Assertion\Relation\ShouldNotDepend\AllAttributesRule;
use PHPat\Rule\Assertion\Relation\ShouldNotDepend\ShouldNotDepend;
use PHPat\Selector\Classname;
use PHPat\Statement\Builder\StatementBuilderFactory;
Expand All @@ -15,18 +15,22 @@
use Tests\PHPat\unit\FakeTestParser;

/**
* @extends RuleTestCase<ClassAttributeRule>
* @extends RuleTestCase<AllAttributesRule>
* @internal
* @coversNothing
*/
class ClassAttributeTest extends RuleTestCase
class AllAttributeTest extends RuleTestCase
{
public const RULE_NAME = 'test_FixtureClassShouldNotDependSimpleAndSpecial';

public function testRule(): void
{
$this->analyse(['tests/fixtures/FixtureClass.php'], [
[sprintf('%s should not depend on %s', FixtureClass::class, SimpleAttribute::class), 29],
[sprintf('%s should not depend on %s', FixtureClass::class, SimpleAttribute::class), 33],
[sprintf('%s should not depend on %s', FixtureClass::class, SimpleAttribute::class), 34],
[sprintf('%s should not depend on %s', FixtureClass::class, SimpleAttribute::class), 94],
[sprintf('%s should not depend on %s', FixtureClass::class, SimpleAttribute::class), 95],
]);
}

Expand All @@ -39,7 +43,7 @@ protected function getRule(): Rule
[new Classname(SimpleAttribute::class, false)]
);

return new ClassAttributeRule(
return new AllAttributesRule(
new StatementBuilderFactory($testParser),
new Configuration(false, true, false),
$this->createReflectionProvider(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
namespace Tests\PHPat\unit\rules\ShouldNotDepend;

use PHPat\Configuration;
use PHPat\Rule\Assertion\Relation\ShouldNotDepend\ClassAttributeRule;
use PHPat\Rule\Assertion\Relation\ShouldNotDepend\AllAttributesRule;
use PHPat\Rule\Assertion\Relation\ShouldNotDepend\ShouldNotDepend;
use PHPat\Selector\Classname;
use PHPat\Statement\Builder\StatementBuilderFactory;
Expand All @@ -15,7 +15,7 @@
use Tests\PHPat\unit\FakeTestParser;

/**
* @extends RuleTestCase<ClassAttributeRule>
* @extends RuleTestCase<AllAttributesRule>
* @internal
* @coversNothing
*/
Expand All @@ -27,6 +27,10 @@ public function testRule(): void
{
$this->analyse(['tests/fixtures/FixtureClass.php'], [
[sprintf('%s: %s should not depend on %s', self::RULE_NAME, FixtureClass::class, SimpleAttribute::class), 29],
[sprintf('%s: %s should not depend on %s', self::RULE_NAME, FixtureClass::class, SimpleAttribute::class), 33],
[sprintf('%s: %s should not depend on %s', self::RULE_NAME, FixtureClass::class, SimpleAttribute::class), 34],
[sprintf('%s: %s should not depend on %s', self::RULE_NAME, FixtureClass::class, SimpleAttribute::class), 94],
[sprintf('%s: %s should not depend on %s', self::RULE_NAME, FixtureClass::class, SimpleAttribute::class), 95],
]);
}

Expand All @@ -39,7 +43,7 @@ protected function getRule(): Rule
[new Classname(SimpleAttribute::class, false)]
);

return new ClassAttributeRule(
return new AllAttributesRule(
new StatementBuilderFactory($testParser),
new Configuration(false, true, true),
$this->createReflectionProvider(),
Expand Down
Loading