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

Added disallowedEnums, they use DisallowedConstant internally #243

Merged
merged 1 commit into from
Jan 22, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@
},
"scripts": {
"lint": "vendor/bin/parallel-lint --colors src/ tests/",
"lint-7.x": "vendor/bin/parallel-lint --colors src/ tests/ --exclude tests/src/TypesEverywhere.php --exclude tests/src/AttributesEverywhere.php --exclude tests/src/disallowed/functionCallsNamedParams.php --exclude tests/src/disallowed-allow/functionCallsNamedParams.php --exclude tests/src/disallowed/attributeUsages.php --exclude tests/src/disallowed-allow/attributeUsages.php --exclude tests/src/disallowed/constantDynamicUsages.php --exclude tests/src/disallowed-allow/constantDynamicUsages.php",
"lint-8.0": "vendor/bin/parallel-lint --colors src/ tests/ --exclude tests/src/TypesEverywhere.php --exclude tests/src/AttributesEverywhere.php --exclude tests/src/disallowed/constantDynamicUsages.php --exclude tests/src/disallowed-allow/constantDynamicUsages.php",
"lint-7.x": "vendor/bin/parallel-lint --colors src/ tests/ --exclude tests/src/TypesEverywhere.php --exclude tests/src/AttributesEverywhere.php --exclude tests/src/disallowed/functionCallsNamedParams.php --exclude tests/src/disallowed-allow/functionCallsNamedParams.php --exclude tests/src/disallowed/attributeUsages.php --exclude tests/src/disallowed-allow/attributeUsages.php --exclude tests/src/disallowed/constantDynamicUsages.php --exclude tests/src/disallowed-allow/constantDynamicUsages.php --exclude tests/src/Enums.php",
"lint-8.0": "vendor/bin/parallel-lint --colors src/ tests/ --exclude tests/src/TypesEverywhere.php --exclude tests/src/AttributesEverywhere.php --exclude tests/src/disallowed/constantDynamicUsages.php --exclude tests/src/disallowed-allow/constantDynamicUsages.php --exclude tests/src/Enums.php",
"lint-8.1": "vendor/bin/parallel-lint --colors src/ tests/ --exclude tests/src/AttributesEverywhere.php --exclude tests/src/disallowed/constantDynamicUsages.php --exclude tests/src/disallowed-allow/constantDynamicUsages.php",
"lint-8.2": "vendor/bin/parallel-lint --colors src/ tests/ --exclude tests/src/disallowed/constantDynamicUsages.php --exclude tests/src/disallowed-allow/constantDynamicUsages.php",
"lint-neon": "vendor/bin/neon-lint .",
Expand Down
13 changes: 12 additions & 1 deletion docs/custom-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ There are several different types (and configuration keys) that can be disallowe
5. `disallowedNamespaces` or `disallowedClasses` - for usages of classes or classes from a namespace
6. `disallowedSuperglobals` - for usages of superglobal variables like `$GLOBALS` or `$_POST`
7. `disallowedAttributes` - for attributes like `#[Entity(class: Foo::class, something: true)]`
8. `disallowedEnums` - for enums, both pure & backed, like `Suit::Hearts` (like class constants, enums need to be split to `enum: Suit` & `case: Hearts` in the configuration, see notes below)

Use them to add rules to your `phpstan.neon` config file. I like to use a separate file (`disallowed-calls.neon`) for these which I'll include later on in the main `phpstan.neon` config file. Here's an example, update to your needs:

Expand Down Expand Up @@ -65,13 +66,19 @@ parameters:
-
attribute: Entity
message: 'use our own custom Entity instead'

disallowedEnums:
-
enum: 'Suit'
case: 'Hearts'
message: 'use Diamonds instead'
```

The `message` key is optional. Functions and methods can be specified with or without `()`. Omitting `()` is not recommended though to avoid confusing method calls with class constants.

### Disallowing multiple items

If you want to disallow multiple calls, constants, class constants (same-class only), classes, namespaces or variables that share the same `message` and other config keys, you can use a list or an array to specify them all:
If you want to disallow multiple calls, constants, class constants (same-class only), enum cases (same-enum only), classes, namespaces or variables that share the same `message` and other config keys, you can use a list or an array to specify them all:
```neon
parameters:
disallowedFunctionCalls:
Expand Down Expand Up @@ -161,3 +168,7 @@ To disallow naive object creation (`new ClassName()` or `new $classname`), disal
### Constants

When [disallowing constants](disallowing-constants.md) please be aware of limitations and special requirements, see [docs](disallowing-constants.md).

### Enums

Similar to disallowing constants, enums have some limitations, see [docs](disallowing-enums.md).
18 changes: 18 additions & 0 deletions docs/disallowing-enums.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
## Disallowing enums

Similar to disallowing constants, enums have some limitations, because internally, enums are disallowed as constants.

First, disallowing enums doesn't support wildcards. The only use case I could think of would be disallowing all cases, e.g. `Enum::*`, which can be achieved by adding the enum to `disallowedClasses`.

Second, enums have to be specified using two keys: `enum` and `case`:
```neon
parameters:
disallowedEnums:
-
enum: 'Suit'
case: 'Hearts'
message: 'use Diamonds instead'
```
This is to prevent the enum case being treated like a real enum by the config parser.

Both pure enums and backed enums are supported.
18 changes: 17 additions & 1 deletion extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ parameters:
disallowedStaticCalls: []
disallowedFunctionCalls: []
disallowedConstants: []
disallowedEnums: []
disallowedSuperglobals: []
disallowedAttributes: []

Expand Down Expand Up @@ -165,6 +166,17 @@ parametersSchema:
?errorTip: string(),
])
)
disallowedEnums: listOf(
structure([
enum: string(),
case: anyOf(string(), listOf(string())),
?allowIn: listOf(string()),
?allowExceptIn: listOf(string()),
?disallowIn: listOf(string()),
?errorIdentifier: string(),
?errorTip: string(),
])
)
disallowedSuperglobals: listOf(
structure([
?superglobal: anyOf(string(), listOf(string())),
Expand Down Expand Up @@ -284,10 +296,14 @@ services:
factory: Spaze\PHPStan\Rules\Disallowed\Usages\ConstantUsages(disallowedConstants: %disallowedConstants%)
tags:
- phpstan.rules.rule
-
classConstantUsages:
factory: Spaze\PHPStan\Rules\Disallowed\Usages\ClassConstantUsages(disallowedConstants: %disallowedConstants%)
tags:
- phpstan.rules.rule
-
factory: Spaze\PHPStan\Rules\Disallowed\Usages\ClassConstantUsages(disallowedConstants: %disallowedEnums%)
tags:
- phpstan.rules.rule
-
factory: Spaze\PHPStan\Rules\Disallowed\Usages\VariableUsages(disallowedVariables: @Spaze\PHPStan\Rules\Disallowed\DisallowedSuperglobalFactory::getDisallowedVariables(%disallowedSuperglobals%))
tags:
Expand Down
10 changes: 5 additions & 5 deletions src/DisallowedConstantFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@ public function __construct(Normalizer $normalizer)


/**
* @param array<array{class?:string, constant?:string|list<string>, message?:string, allowIn?:list<string>, allowExceptIn?:list<string>, disallowIn?:list<string>, errorIdentifier?:string, errorTip?:string}> $config
* @param array<array{class?:string, enum?:string, constant?:string|list<string>, case?:string|list<string>, message?:string, allowIn?:list<string>, allowExceptIn?:list<string>, disallowIn?:list<string>, errorIdentifier?:string, errorTip?:string}> $config
* @return list<DisallowedConstant>
* @throws ShouldNotHappenException
*/
public function createFromConfig(array $config): array
{
$disallowedConstants = [];
foreach ($config as $disallowed) {
$constants = $disallowed['constant'] ?? null;
unset($disallowed['constant']);
$constants = $disallowed['constant'] ?? $disallowed['case'] ?? null;
unset($disallowed['constant'], $disallowed['case']);
if (!$constants) {
throw new ShouldNotHappenException("'constant' must be set in configuration items");
throw new ShouldNotHappenException("'constant', or 'case' for enums, must be set in configuration items");
}
foreach ((array)$constants as $constant) {
$class = $disallowed['class'] ?? null;
$class = $disallowed['class'] ?? $disallowed['enum'] ?? null;
$disallowedConstant = new DisallowedConstant(
$this->normalizer->normalizeNamespace($class ? "{$class}::{$constant}" : $constant),
$disallowed['message'] ?? null,
Expand Down
2 changes: 1 addition & 1 deletion src/Usages/ClassConstantUsages.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class ClassConstantUsages implements Rule
* @param DisallowedConstantFactory $disallowedConstantFactory
* @param TypeResolver $typeResolver
* @param Formatter $formatter
* @param array<array{class?:string, constant?:string|list<string>, message?:string, allowIn?:list<string>}> $disallowedConstants
* @param array<array{class?:string, enum?:string, constant?:string|list<string>, case?:string|list<string>, message?:string, allowIn?:list<string>}> $disallowedConstants
* @throws ShouldNotHappenException
*/
public function __construct(
Expand Down
3 changes: 2 additions & 1 deletion src/Usages/NamespaceUsages.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ public function processNode(Node $node, Scope $scope): array
} elseif ($node instanceof StaticCall && $node->class instanceof Name) {
$namespaces = [$node->class->toString()];
} elseif ($node instanceof ClassConstFetch && $node->class instanceof Name) {
$description = 'Class';
$classReflection = $scope->resolveTypeByName($node->class)->getClassReflection();
$description = $classReflection && $classReflection->isEnum() ? 'Enum' : 'Class';
$namespaces = [$node->class->toString()];
} elseif ($node instanceof Class_ && ($node->extends !== null || count($node->implements) > 0)) {
$namespaces = [];
Expand Down
100 changes: 100 additions & 0 deletions tests/Usages/ClassConstantEnumUsagesTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
<?php
declare(strict_types = 1);

namespace Spaze\PHPStan\Rules\Disallowed\Usages;

use Enums\BackedEnum;
use Enums\Enum;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use Spaze\PHPStan\Rules\Disallowed\DisallowedConstantFactory;
use Spaze\PHPStan\Rules\Disallowed\Formatter\Formatter;
use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedConstantRuleErrors;
use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver;

/**
* @requires PHP >= 8.1
*/
class ClassConstantEnumUsagesTest extends RuleTestCase
{

protected function getRule(): Rule
{
$container = self::getContainer();
return new ClassConstantUsages(
$container->getByType(DisallowedConstantRuleErrors::class),
$container->getByType(DisallowedConstantFactory::class),
$container->getByType(TypeResolver::class),
$container->getByType(Formatter::class),
[
[
'enum' => Enum::class,
'constant' => 'ENUM_CONST',
],
[
'enum' => Enum::class,
'case' => 'Foo',
],
[
'class' => Enum::class,
'constant' => 'Bar',
],
[
'enum' => BackedEnum::class,
'constant' => 'ENUM_CONST',
],
[
'enum' => BackedEnum::class,
'case' => 'Waldo',
],
[
'class' => BackedEnum::class,
'constant' => 'Quux',
],
]
);
}


public function testRule(): void
{
// Based on the configuration above, no errors in this file:
$this->analyse([__DIR__ . '/../src/Enums.php'], [
[
// expect this error message:
'Using Enums\Enum::ENUM_CONST is forbidden.',
// on this line:
16,
],
[
'Using Enums\Enum::Foo is forbidden.',
17,
],
[
'Using Enums\Enum::Bar is forbidden.',
18,
],
[
'Using Enums\BackedEnum::ENUM_CONST is forbidden.',
31,
],
[
'Using Enums\BackedEnum::Waldo is forbidden.',
32,
],
[
'Using Enums\BackedEnum::Quux is forbidden.',
33,
],
]);
}


public static function getAdditionalConfigFiles(): array
{
return [
__DIR__ . '/../../extension.neon',
];
}

}
2 changes: 1 addition & 1 deletion tests/Usages/ClassConstantInvalidUsagesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class ClassConstantInvalidUsagesTest extends RuleTestCase
*/
protected function getRule(): Rule
{
return self::getContainer()->getByType(ClassConstantUsages::class);
return self::getContainer()->getService('classConstantUsages');
}


Expand Down
34 changes: 34 additions & 0 deletions tests/src/Enums.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php
declare(strict_types = 1);

namespace Enums;

enum Enum
{

public const ENUM_CONST = true;
case Foo;
case Bar;
case Baz;

}

Enum::ENUM_CONST;
Enum::Foo;
Enum::Bar;
Enum::Baz;

enum BackedEnum: int
{

public const ENUM_CONST = true;
case Waldo = 1;
case Quux = 2;
case Fred = 3;

}

BackedEnum::ENUM_CONST;
BackedEnum::Waldo;
BackedEnum::Quux;
BackedEnum::Fred;