From 3cc105b36967da28d5369d9d0fd087ecc3520ef5 Mon Sep 17 00:00:00 2001 From: Can Vural Date: Sat, 5 Nov 2022 23:24:47 +0100 Subject: [PATCH] refactor the collection of views used in other views to new class --- extension.neon | 20 ++++- src/Collectors/UsedEmailViewCollector.php | 2 +- .../UsedViewFacadeMakeCollector.php | 59 +++++++++++++++ .../UsedViewInAnotherViewCollector.php | 39 +++++++--- src/Collectors/UsedViewMakeCollector.php | 53 +++++++++++++ ...iewWithMethodsClassReflectionExtension.php | 2 +- src/Rules/UnusedViewsRule.php | 33 ++++++--- src/Support/ViewFileHelper.php | 74 +++++++++++++++++++ .../resources/views/emails/markdown.blade.php | 1 + .../resources/views/emails/view.blade.php | 1 + .../resources/views/unused.blade.php | 1 + .../views/view-factory-make.blade.php | 1 + .../views/view-helper-make.blade.php | 1 + .../views/view-static-make.blade.php | 1 + ...nseMethodsClassReflectionExtensionTest.php | 2 +- tests/Rules/Data/FooController.php | 23 +++++- tests/Rules/UnusedViewsRuleTest.php | 27 +++++-- tests/Type/data/view.php | 6 +- 18 files changed, 309 insertions(+), 37 deletions(-) create mode 100644 src/Collectors/UsedViewFacadeMakeCollector.php create mode 100644 src/Collectors/UsedViewMakeCollector.php create mode 100644 src/Support/ViewFileHelper.php create mode 100644 tests/Application/resources/views/emails/markdown.blade.php create mode 100644 tests/Application/resources/views/emails/view.blade.php create mode 100644 tests/Application/resources/views/unused.blade.php create mode 100644 tests/Application/resources/views/view-factory-make.blade.php create mode 100644 tests/Application/resources/views/view-helper-make.blade.php create mode 100644 tests/Application/resources/views/view-static-make.blade.php diff --git a/extension.neon b/extension.neon index 37d8f0084..fab960024 100644 --- a/extension.neon +++ b/extension.neon @@ -5,6 +5,8 @@ parameters: earlyTerminatingFunctionCalls: - abort - dd + excludePaths: + - *.blade.php mixinExcludeClasses: - Eloquent bootstrapFiles: @@ -16,6 +18,7 @@ parameters: noUnnecessaryCollectionCallExcept: [] squashedMigrationsPath: [] databaseMigrationsPath: [] + viewDirectories: [] checkModelProperties: false checkPhpDocMissingReturn: false @@ -26,6 +29,7 @@ parametersSchema: noUnnecessaryCollectionCallOnly: listOf(string()) noUnnecessaryCollectionCallExcept: listOf(string()) databaseMigrationsPath: listOf(string()) + viewDirectories: listOf(string()) squashedMigrationsPath: listOf(string()) checkModelProperties: bool() @@ -450,10 +454,24 @@ services: class: NunoMaduro\Larastan\Collectors\UsedEmailViewCollector tags: - phpstan.collector + - - class: NunoMaduro\Larastan\Collectors\UsedViewInAnotherViewCollector + class: NunoMaduro\Larastan\Collectors\UsedViewMakeCollector + tags: + - phpstan.collector + + - + class: NunoMaduro\Larastan\Collectors\UsedViewFacadeMakeCollector tags: - phpstan.collector + - + class: NunoMaduro\Larastan\Collectors\UsedViewInAnotherViewCollector + arguments: + parser: @currentPhpVersionSimpleDirectParser + - + class: NunoMaduro\Larastan\Support\ViewFileHelper + arguments: + viewDirectories: %viewDirectories% rules: - NunoMaduro\Larastan\Rules\RelationExistenceRule - NunoMaduro\Larastan\Rules\UselessConstructs\NoUselessWithFunctionCallsRule diff --git a/src/Collectors/UsedEmailViewCollector.php b/src/Collectors/UsedEmailViewCollector.php index 7d3ae1417..a531a4495 100644 --- a/src/Collectors/UsedEmailViewCollector.php +++ b/src/Collectors/UsedEmailViewCollector.php @@ -29,7 +29,7 @@ public function processNode(Node $node, Scope $scope): ?string return null; } - if ($name->name !== 'markdown') { + if (! in_array($name->name, ['markdown', 'view'], true)) { return null; } diff --git a/src/Collectors/UsedViewFacadeMakeCollector.php b/src/Collectors/UsedViewFacadeMakeCollector.php new file mode 100644 index 000000000..2840540db --- /dev/null +++ b/src/Collectors/UsedViewFacadeMakeCollector.php @@ -0,0 +1,59 @@ + */ +final class UsedViewFacadeMakeCollector implements Collector +{ + public function getNodeType(): string + { + return Node\Expr\StaticCall::class; + } + + /** @param Node\Expr\StaticCall $node */ + public function processNode(Node $node, Scope $scope): ?string + { + $name = $node->name; + + if (! $name instanceof Node\Identifier) { + return null; + } + + if ($name->name !== 'make') { + return null; + } + + if (count($node->getArgs()) < 1) { + return null; + } + + $class = $node->class; + + if (! $class instanceof Node\Name) { + return null; + } + + $class = $scope->resolveName($class); + + if (! (new ObjectType(View::class))->isSuperTypeOf(new ObjectType($class))->yes()) { + return null; + } + + $template = $node->getArgs()[0]->value; + + if (! $template instanceof Node\Scalar\String_) { + return null; + } + + return ViewName::normalize($template->value); + } +} diff --git a/src/Collectors/UsedViewInAnotherViewCollector.php b/src/Collectors/UsedViewInAnotherViewCollector.php index bad2717ef..e74579b1e 100644 --- a/src/Collectors/UsedViewInAnotherViewCollector.php +++ b/src/Collectors/UsedViewInAnotherViewCollector.php @@ -2,30 +2,49 @@ namespace NunoMaduro\Larastan\Collectors; +use NunoMaduro\Larastan\Support\ViewFileHelper; use PhpParser\Node; -use PHPStan\Analyser\Scope; -use PHPStan\Collectors\Collector; -use PHPStan\Node\FileNode; +use PHPStan\Parser\Parser; +use PHPStan\Parser\ParserErrorsException; -/** @implements Collector */ -class UsedViewInAnotherViewCollector implements Collector +final class UsedViewInAnotherViewCollector { /** @see https://regex101.com/r/8gosof/1 */ private const VIEW_NAME_REGEX = '/@(extends|include(If|Unless|When|First)?)(\(.*?\'(.*?)\'(\)|,))/m'; - public function getNodeType(): string + public function __construct(private Parser $parser, private ViewFileHelper $viewFileHelper) { - return FileNode::class; } - public function processNode(Node $node, Scope $scope): ?array + /** @return list */ + public function getUsedViews(): array { - $nodes = array_filter($node->getNodes(), function (Node $node) { + $usedViews = []; + foreach ($this->viewFileHelper->getAllViewFilePaths() as $viewFile) { + try { + $parserNodes = $this->parser->parseFile($viewFile); + + $usedViews = array_merge($usedViews, $this->processNodes($parserNodes)); + } catch (ParserErrorsException $e) { + continue; + } + } + + return $usedViews; + } + + /** + * @param Node\Stmt[] $nodes + * @return list + */ + private function processNodes(array $nodes): array + { + $nodes = array_filter($nodes, function (Node $node) { return $node instanceof Node\Stmt\InlineHTML; }); if (count($nodes) === 0) { - return null; + return []; } $usedViews = []; diff --git a/src/Collectors/UsedViewMakeCollector.php b/src/Collectors/UsedViewMakeCollector.php new file mode 100644 index 000000000..48ed14469 --- /dev/null +++ b/src/Collectors/UsedViewMakeCollector.php @@ -0,0 +1,53 @@ + */ +final class UsedViewMakeCollector implements Collector +{ + public function getNodeType(): string + { + return Node\Expr\MethodCall::class; + } + + /** @param Node\Expr\MethodCall $node */ + public function processNode(Node $node, Scope $scope): ?string + { + $name = $node->name; + + if (! $name instanceof Node\Identifier) { + return null; + } + + if ($name->name !== 'make') { + return null; + } + + if (count($node->getArgs()) < 1) { + return null; + } + + $class = $node->var; + + if (! (new ObjectType(Factory::class))->isSuperTypeOf($scope->getType($class))->yes()) { + return null; + } + + $template = $node->getArgs()[0]->value; + + if (! $template instanceof Node\Scalar\String_) { + return null; + } + + return ViewName::normalize($template->value); + } +} diff --git a/src/Methods/ViewWithMethodsClassReflectionExtension.php b/src/Methods/ViewWithMethodsClassReflectionExtension.php index 95000790e..25fddf0ad 100644 --- a/src/Methods/ViewWithMethodsClassReflectionExtension.php +++ b/src/Methods/ViewWithMethodsClassReflectionExtension.php @@ -11,7 +11,7 @@ class ViewWithMethodsClassReflectionExtension implements MethodsClassReflectionE { public function hasMethod(ClassReflection $classReflection, string $methodName): bool { - if ($classReflection->getName() !== 'Illuminate\View\View') { + if (! in_array($classReflection->getName(), ['Illuminate\View\View', 'Illuminate\Contracts\View\View'], true)) { return false; } diff --git a/src/Rules/UnusedViewsRule.php b/src/Rules/UnusedViewsRule.php index 721098089..6896b146a 100644 --- a/src/Rules/UnusedViewsRule.php +++ b/src/Rules/UnusedViewsRule.php @@ -5,21 +5,29 @@ namespace NunoMaduro\Larastan\Rules; use function collect; -use Illuminate\Support\Facades\File; use Illuminate\View\Factory; use NunoMaduro\Larastan\Collectors\UsedEmailViewCollector; +use NunoMaduro\Larastan\Collectors\UsedViewFacadeMakeCollector; use NunoMaduro\Larastan\Collectors\UsedViewFunctionCollector; use NunoMaduro\Larastan\Collectors\UsedViewInAnotherViewCollector; +use NunoMaduro\Larastan\Collectors\UsedViewMakeCollector; +use NunoMaduro\Larastan\Support\ViewFileHelper; use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\Node\CollectedDataNode; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use Symfony\Component\Finder\SplFileInfo; /** @implements Rule */ final class UnusedViewsRule implements Rule { + /** @var list|null */ + private ?array $viewsUsedInOtherViews = null; + + public function __construct(private UsedViewInAnotherViewCollector $usedViewInAnotherViewCollector, private ViewFileHelper $viewFileHelper) + { + } + public function getNodeType(): string { return CollectedDataNode::class; @@ -27,17 +35,19 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { + if ($this->viewsUsedInOtherViews === null) { + $this->viewsUsedInOtherViews = $this->usedViewInAnotherViewCollector->getUsedViews(); + } + $usedViews = collect([ $node->get(UsedViewFunctionCollector::class), $node->get(UsedEmailViewCollector::class), - $node->get(UsedViewInAnotherViewCollector::class), + $node->get(UsedViewMakeCollector::class), + $node->get(UsedViewFacadeMakeCollector::class), + $this->viewsUsedInOtherViews, ])->flatten()->unique()->toArray(); - $allViews = array_map(function (SplFileInfo $file) { - return $file->getPathname(); - }, array_filter(File::allFiles(resource_path('views')), function (SplFileInfo $file) { - return ! str_contains($file->getPathname(), 'views/vendor') && str_ends_with($file->getFilename(), '.blade.php'); - })); + $allViews = iterator_to_array($this->viewFileHelper->getAllViewNames()); $existingViews = []; @@ -45,18 +55,17 @@ public function processNode(Node $node, Scope $scope): array $view = view(); foreach ($usedViews as $viewName) { - // Not existing views are reported with `view-string` type if ($view->exists($viewName)) { - $existingViews[] = $view->getFinder()->find($viewName); + $existingViews[] = $viewName; } } - $unusedViews = array_diff($allViews, $existingViews); + $unusedViews = array_diff($allViews, array_filter($existingViews)); $errors = []; foreach ($unusedViews as $file) { $errors[] = RuleErrorBuilder::message('This view is not used in the project.') - ->file($file) + ->file($file.'.blade.php') ->line(0) ->build(); } diff --git a/src/Support/ViewFileHelper.php b/src/Support/ViewFileHelper.php new file mode 100644 index 000000000..a6ace8480 --- /dev/null +++ b/src/Support/ViewFileHelper.php @@ -0,0 +1,74 @@ + $viewDirectories + */ + public function __construct(private array $viewDirectories, private FileHelper $fileHelper) + { + if (count($viewDirectories) === 0) { + $this->viewDirectories = [resource_path('views')]; // @phpstan-ignore-line + } + } + + public function getAllViewFilePaths(): Generator + { + foreach ($this->viewDirectories as $viewDirectory) { + $absolutePath = $this->fileHelper->absolutizePath($viewDirectory); + + if (! is_dir($absolutePath)) { + continue; + } + + $views = iterator_to_array( + new RegexIterator( + new RecursiveIteratorIterator(new RecursiveDirectoryIterator($absolutePath)), + '/\.blade\.php$/i' + ) + ); + + foreach ($views as $view) { + yield $view->getPathname(); + } + } + } + + public function getAllViewNames(): Generator + { + foreach ($this->viewDirectories as $viewDirectory) { + $absolutePath = $this->fileHelper->absolutizePath($viewDirectory); + + if (! is_dir($absolutePath)) { + continue; + } + + $views = iterator_to_array( + new RegexIterator( + new RecursiveIteratorIterator(new RecursiveDirectoryIterator($absolutePath)), + '/\.blade\.php$/i' + ) + ); + + foreach ($views as $view) { + if (str_contains($view->getPathname(), 'views/vendor')) { + continue; + } + + $viewName = explode($viewDirectory.'/', $view->getPathname()); + + yield str_replace(['/', '.blade.php'], ['.', ''], $viewName[1]); + } + } + } +} diff --git a/tests/Application/resources/views/emails/markdown.blade.php b/tests/Application/resources/views/emails/markdown.blade.php new file mode 100644 index 000000000..95f6db387 --- /dev/null +++ b/tests/Application/resources/views/emails/markdown.blade.php @@ -0,0 +1 @@ +// Used with $this->markdown('emails.markdown') diff --git a/tests/Application/resources/views/emails/view.blade.php b/tests/Application/resources/views/emails/view.blade.php new file mode 100644 index 000000000..43c035322 --- /dev/null +++ b/tests/Application/resources/views/emails/view.blade.php @@ -0,0 +1 @@ +// Used with $this->view('emails.view') diff --git a/tests/Application/resources/views/unused.blade.php b/tests/Application/resources/views/unused.blade.php new file mode 100644 index 000000000..1c8e4aca5 --- /dev/null +++ b/tests/Application/resources/views/unused.blade.php @@ -0,0 +1 @@ +// Not used in any other file diff --git a/tests/Application/resources/views/view-factory-make.blade.php b/tests/Application/resources/views/view-factory-make.blade.php new file mode 100644 index 000000000..7d3db43f7 --- /dev/null +++ b/tests/Application/resources/views/view-factory-make.blade.php @@ -0,0 +1 @@ +// Used by $viewFactory->make('view-factory-make') diff --git a/tests/Application/resources/views/view-helper-make.blade.php b/tests/Application/resources/views/view-helper-make.blade.php new file mode 100644 index 000000000..59e8c05f8 --- /dev/null +++ b/tests/Application/resources/views/view-helper-make.blade.php @@ -0,0 +1 @@ +// Used by view()->make('view-helper-make') diff --git a/tests/Application/resources/views/view-static-make.blade.php b/tests/Application/resources/views/view-static-make.blade.php new file mode 100644 index 000000000..12be948eb --- /dev/null +++ b/tests/Application/resources/views/view-static-make.blade.php @@ -0,0 +1 @@ +// Used by View::make('view-static-make') diff --git a/tests/Reflection/RedirectResponseMethodsClassReflectionExtensionTest.php b/tests/Reflection/RedirectResponseMethodsClassReflectionExtensionTest.php index 5e0f0b89c..0563104b3 100644 --- a/tests/Reflection/RedirectResponseMethodsClassReflectionExtensionTest.php +++ b/tests/Reflection/RedirectResponseMethodsClassReflectionExtensionTest.php @@ -91,6 +91,6 @@ public function redMethodProvider(): iterable public static function getAdditionalConfigFiles(): array { - return [__DIR__.'/../../extension.neon']; + return [__DIR__.'/../Rules/phpstan-rules.neon']; } } diff --git a/tests/Rules/Data/FooController.php b/tests/Rules/Data/FooController.php index dff1914c0..2d985f037 100644 --- a/tests/Rules/Data/FooController.php +++ b/tests/Rules/Data/FooController.php @@ -4,6 +4,7 @@ use Illuminate\Contracts\View\View; use Illuminate\Mail\Mailable; +use Illuminate\View\Factory; class FooController { @@ -32,11 +33,31 @@ class FooEmail extends Mailable { public function build(): self { - return $this->markdown('emails.orders.shipped'); + return $this->markdown('emails.markdown'); } public function foo(): self { return $this->markdown('home'); } + + public function bar(): self + { + return $this->view('emails.view'); + } +} + +function viewHelper(): View +{ + return view()->make('view-helper-make'); +} + +function viewFactory(Factory $factory): View +{ + return $factory->make('view-factory-make'); +} + +function viewStaticMake(): View +{ + return \Illuminate\Support\Facades\View::make('view-static-make'); } diff --git a/tests/Rules/UnusedViewsRuleTest.php b/tests/Rules/UnusedViewsRuleTest.php index 69d8532c4..6016a4556 100644 --- a/tests/Rules/UnusedViewsRuleTest.php +++ b/tests/Rules/UnusedViewsRuleTest.php @@ -3,9 +3,13 @@ namespace Rules; use NunoMaduro\Larastan\Collectors\UsedEmailViewCollector; +use NunoMaduro\Larastan\Collectors\UsedViewFacadeMakeCollector; use NunoMaduro\Larastan\Collectors\UsedViewFunctionCollector; use NunoMaduro\Larastan\Collectors\UsedViewInAnotherViewCollector; +use NunoMaduro\Larastan\Collectors\UsedViewMakeCollector; use NunoMaduro\Larastan\Rules\UnusedViewsRule; +use NunoMaduro\Larastan\Support\ViewFileHelper; +use PHPStan\File\FileHelper; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; @@ -14,7 +18,12 @@ class UnusedViewsRuleTest extends RuleTestCase { protected function getRule(): Rule { - return new UnusedViewsRule; + $viewFileHelper = new ViewFileHelper([__DIR__.'/../Application/resources/views'], $this->getContainer()->getByType(FileHelper::class)); + + return new UnusedViewsRule(new UsedViewInAnotherViewCollector( + $this->getContainer()->getService('currentPhpVersionSimpleDirectParser'), + $viewFileHelper, + ), $viewFileHelper); } protected function getCollectors(): array @@ -22,21 +31,25 @@ protected function getCollectors(): array return [ new UsedViewFunctionCollector, new UsedEmailViewCollector, - new UsedViewInAnotherViewCollector, + new UsedViewMakeCollector, + new UsedViewFacadeMakeCollector, ]; } public function testRule(): void { - $this->analyse([__DIR__.'/Data/FooController.php', __DIR__.'/../Application/resources/views/index.blade.php', __DIR__.'/../Application/resources/views/base.blade.php'], [ - [ - 'This view is not used in the project.', - 00, - ], + $this->analyse([__DIR__.'/Data/FooController.php'], [ [ 'This view is not used in the project.', 00, ], ]); } + + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__.'/../../extension.neon', + ]; + } } diff --git a/tests/Type/data/view.php b/tests/Type/data/view.php index 25a42b58c..9149137e2 100644 --- a/tests/Type/data/view.php +++ b/tests/Type/data/view.php @@ -5,6 +5,6 @@ use function PHPStan\Testing\assertType; assertType('Illuminate\Contracts\View\Factory', view()); -assertType('Illuminate\View\View', view('foo')); -assertType('Illuminate\View\View', view('foo')->with('bar', 'baz')); -assertType('Illuminate\View\View', view('foo')->withFoo('bar')); +assertType('Illuminate\Contracts\View\View', view('foo')); +assertType('Illuminate\Contracts\View\View', view('foo')->with('bar', 'baz')); +assertType('Illuminate\Contracts\View\View', view('foo')->withFoo('bar'));