Skip to content

Commit

Permalink
Merge pull request #6 from worksome/prevent_resource
Browse files Browse the repository at this point in the history
Adds a new rule for disabling route resource partials
  • Loading branch information
lukeraymonddowning authored Jan 6, 2022
2 parents 7cfcdcb + 529c566 commit 6a37845
Show file tree
Hide file tree
Showing 13 changed files with 324 additions and 9 deletions.
24 changes: 16 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,8 @@ This section will list all the custom phpstan rules this package adds.
### Generic
Rules that are applied to all projects.

#### NamespaceBasedSuffix (configurable)

### Laravel
Rules that are only applied in a Laravel context.

#### DisallowAppHelperUsage
This rule disallows the usage of laravel's `app` helper function in favour of using dependency injection instead.
#### DeclareStrictTypes
This rule is used to ensure that all PHP files include with the `declare(strict_types=1)` statement.

#### NamespaceBasedSuffix
Sets up configuration for suffixing the following namespaces
Expand All @@ -59,4 +54,17 @@ Sets up configuration for suffixing the following namespaces
- `App\Policies`: `Policy`
- `App\Jobs`: `Job`

This makes sures events, listeners, policies and jobs has the same suffix.
This makes sures events, listeners, policies and jobs has the same suffix.

### Laravel
Rules that are only applied in a Laravel context.

#### DisallowAppHelperUsage
This rule disallows the usage of laravel's `app` helper function in favour of using dependency injection instead.

#### DisallowPartialRouteFacadeResource
This rule disallows the usage of the `Route::resource` method when combined with `only` or `except`. Instead,
partial route resources should be split into multiple routes.

#### DisallowPartialRouteFacadeResource
Similar to `DisallowPartialRouteFacadeResource`, but prevents partial resource usage when used in a route group.
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
},
"require-dev": {
"composer/composer": "^2.0",
"pestphp/pest": "^1.21"
"pestphp/pest": "^1.21",
"spatie/ray": "^1.32"
},
"autoload": {
"psr-4": {
Expand Down
2 changes: 2 additions & 0 deletions larastan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,6 @@ parameters:
App\Jobs: Job
rules:
- Worksome\CodingStyle\PHPStan\Laravel\DisallowAppHelperUsageRule
- Worksome\CodingStyle\PHPStan\Laravel\DisallowPartialRouteFacadeResourceRule
- Worksome\CodingStyle\PHPStan\Laravel\DisallowPartialRouteVariableResourceRule
services:
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

namespace Worksome\CodingStyle\PHPStan\Laravel\DisallowPartialRouteResource;

use Illuminate\Support\Facades\Route;
use PhpParser\Node;
use PhpParser\Node\Expr\StaticCall;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;

/**
* @implements Rule<Node\Expr\StaticCall>
*/
class DisallowPartialRouteFacadeResourceRule implements Rule
{
public function __construct(private PartialRouteResourceInspector $inspector)
{
}

public function getNodeType(): string
{
return Node\Expr\StaticCall::class;
}

/**
* @param Node\Expr\StaticCall $node
* @return RuleError[]
*/
public function processNode(Node $node, Scope $scope): array
{
if (! $this->inspector->isApplicable($node)) {
return [];
}

if (! $this->isCalledOnRouteFacade($node)) {
return [];
}

return $this->inspector->inspect($node);
}

private function isCalledOnRouteFacade(Node $node): bool
{
if (! $node instanceof StaticCall) {
return false;
}

return $node->class->toString() === Route::class;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php

namespace Worksome\CodingStyle\PHPStan\Laravel\DisallowPartialRouteResource;

use Illuminate\Support\Facades\Route;
use PhpParser\Node;
use PhpParser\Node\Expr\MethodCall;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;

/**
* @implements Rule<Node\Expr\MethodCall>
*/
class DisallowPartialRouteVariableResourceRule implements Rule
{
public function __construct(private PartialRouteResourceInspector $inspector)
{
}

public function getNodeType(): string
{
return Node\Expr\MethodCall::class;
}

/**
* @param Node\Expr\MethodCall $node
* @return RuleError[]
*/
public function processNode(Node $node, Scope $scope): array
{
if (! $this->inspector->isApplicable($node)) {
return [];
}

if (! $this->isCalledInRouteGroupClosure($node, $scope)) {
return [];
}

return $this->inspector->inspect($node);
}

private function isCalledInRouteGroupClosure(Node\Expr\CallLike $node, Scope $scope): bool
{
if (! $node instanceof MethodCall) {
return false;
}

if (! $scope->isInAnonymousFunction()) {
return false;
}

$parent = $node->getAttribute('parent');

while ($parent !== null) {
if ($parent instanceof Node\Expr\StaticCall) {
return $parent->class->toString() === Route::class;
}

$parent = $parent->getAttribute('parent');
}

return false;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

namespace Worksome\CodingStyle\PHPStan\Laravel\DisallowPartialRouteResource;

use PhpParser\Node;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;

final class PartialRouteResourceInspector
{
/**
* @var string[]
*/
private array $resourceMethods = [
'resource',
'apiResource',
];

/**
* @var string[]
*/
private array $partialMethods = [
'except',
'only',
];

public function isApplicable(Node $node): bool
{
if (! $node instanceof Node\Expr\CallLike) {
return false;
}

if (! in_array($node->name->name, $this->resourceMethods)) {
return false;
}

return true;
}

public function inspect(Node $node): array
{
$next = $node->getAttribute('next');

while ($next !== null) {
if (in_array($next->name, $this->partialMethods)) {
return [$this->errorFor($next->name)];
}

$next = $next->getAttribute('parent')->getAttribute('next');
}

return [];
}

private function errorFor(string $method): RuleError
{
return RuleErrorBuilder::message(
"Usage of [{$method}] method on route resource is disallowed. Please split the resource into multiple routes."
)->build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php

use Worksome\CodingStyle\PHPStan\Laravel\DisallowPartialRouteResource\DisallowPartialRouteFacadeResourceRule;
use Worksome\CodingStyle\PHPStan\Laravel\DisallowPartialRouteResource\DisallowPartialRouteVariableResourceRule;
use Worksome\CodingStyle\PHPStan\Laravel\DisallowPartialRouteResource\PartialRouteResourceInspector;

it('checks for partial route facade resources', function (string $path, array ...$errors) {
$this->rule = new DisallowPartialRouteFacadeResourceRule(new PartialRouteResourceInspector());

expect($path)->toHaveRuleErrors($errors);
})->with([
'calls route resource with except' => [
__DIR__ . '/Fixture/route_resource_with_except.php.inc',
[
'Usage of [except] method on route resource is disallowed. Please split the resource into multiple routes.',
5
]
],
'calls route resource with only' => [
__DIR__ . '/Fixture/route_resource_with_only.php.inc',
[
'Usage of [only] method on route resource is disallowed. Please split the resource into multiple routes.',
5
]
],
'calls API route resource with except' => [
__DIR__ . '/Fixture/api_route_resource_with_except.php.inc',
[
'Usage of [except] method on route resource is disallowed. Please split the resource into multiple routes.',
5
]
],
'calls except at end of method chain' => [
__DIR__ . '/Fixture/route_resource_with_except_after_after_chain.php.inc',
[
'Usage of [except] method on route resource is disallowed. Please split the resource into multiple routes.',
5
]
],
'calls complete route resource' => [
__DIR__ . '/Fixture/route_resource.php.inc'
],
]);

it('checks for partial route variable resources', function (string $path, array ...$errors) {
$this->rule = new DisallowPartialRouteVariableResourceRule(new PartialRouteResourceInspector());

expect($path)->toHaveRuleErrors($errors);
})->with([
'calls grouped resource' => [
__DIR__ . '/Fixture/grouped_route_resource_with_except.php.inc',
[
'Usage of [except] method on route resource is disallowed. Please split the resource into multiple routes.',
6
],
[
'Usage of [only] method on route resource is disallowed. Please split the resource into multiple routes.',
7
]
],
'calls complete route resource' => [
__DIR__ . '/Fixture/route_resource.php.inc',
],
]);
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?php

use Illuminate\Support\Facades\Route;

Route::apiResource('foo', 'FooController')->except(['show']);
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

use Illuminate\Support\Facades\Route;

Route::group([], function ($router) {
$router->resource('foo', 'FooController')->except(['show']);
$router->resource('bar', 'BarController')->only(['show']);
});

// Some other method call to make sure everything still works
$router->resource('baz', 'BazController');
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

use Illuminate\Support\Facades\Route;

/**
* All the following examples should be allowed
* as they are not partial route resources.
*/

Route::resource('foo', 'FooController');

Route::resource('bar', 'BarController')->shallow();

Route::apiResource('baz', 'BazController');

Route::apiResource('bam', 'BamController')->shallow();

Route::resource('qux', 'QuxController')->names([
'index' => 'qux.list'
]);

Route::resource('blogs', 'BlogController')->parameters([
'blogs' => 'post'
]);

Route::resource('photos.comments', 'PhotoCommentController')->scoped([
'comment' => 'slug',
]);

Route::group([], function ($router) {
$router->resource('photos', 'PhotoController')->scoped();
$router->apiResource('books', 'BookController')->scoped();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?php

use Illuminate\Support\Facades\Route;

Route::resource('foo', 'FooController')->except(['show']);
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?php

use Illuminate\Support\Facades\Route;

Route::resource('foo', 'FooController')->shallow()->except(['show']);
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?php

use Illuminate\Support\Facades\Route;

Route::resource('foo', 'FooController')->only(['show', 'update']);

0 comments on commit 6a37845

Please sign in to comment.