From 013413b9e3384d03877bab764eeecbc5621bded2 Mon Sep 17 00:00:00 2001 From: mamazu <14860264+mamazu@users.noreply.github.com> Date: Sun, 17 Mar 2024 17:47:54 +0100 Subject: [PATCH 1/3] Adding more types in the FileFinder --- .../Application/Finder/FileFinder.php | 28 +++++++++---- phpstan-baseline.neon | 40 ------------------- 2 files changed, 21 insertions(+), 47 deletions(-) diff --git a/lib/Extension/ClassMover/Application/Finder/FileFinder.php b/lib/Extension/ClassMover/Application/Finder/FileFinder.php index a62f289c7..1cb887cda 100644 --- a/lib/Extension/ClassMover/Application/Finder/FileFinder.php +++ b/lib/Extension/ClassMover/Application/Finder/FileFinder.php @@ -57,7 +57,7 @@ public function filesFor(Filesystem $filesystem, ReflectionClassLike $reflection return $this->pathsFromReflectionClass($reflection, $private); } - private function pathsFromReflectionClass(ReflectionClass $reflection, bool $private) + private function pathsFromReflectionClass(ReflectionClass $reflection, bool $private): FileList { $path = $reflection->sourceCode()->uri()?->path(); @@ -78,13 +78,17 @@ private function pathsFromReflectionClass(ReflectionClass $reflection, bool $pri return FileList::fromFilePaths($filePaths); } - private function allPhpFiles(Filesystem $filesystem) + private function allPhpFiles(Filesystem $filesystem): FileList { - $filePaths = $filesystem->fileList()->existing()->phpFiles(); - return $filePaths; + return $filesystem->fileList()->existing()->phpFiles(); } - private function parentFilePaths(ReflectionClass $reflection, $filePaths) + /** + * @param array $filePaths + * + * @return array + */ + private function parentFilePaths(ReflectionClass $reflection, array $filePaths): array { $context = $reflection->parent(); while ($context) { @@ -95,7 +99,12 @@ private function parentFilePaths(ReflectionClass $reflection, $filePaths) return $filePaths; } - private function traitFilePaths(ReflectionClass $reflection, $filePaths) + /** + * @param array $filePaths + * + * @return array + */ + private function traitFilePaths(ReflectionClass $reflection, array $filePaths): array { foreach ($reflection->traits() as $trait) { $filePaths[] = $trait->sourceCode()->uri()?->path(); @@ -103,7 +112,12 @@ private function traitFilePaths(ReflectionClass $reflection, $filePaths) return $filePaths; } - private function interfaceFilePaths(ReflectionClass $reflection, $filePaths) + /** + * @param array $filePaths + * + * @return array + */ + private function interfaceFilePaths(ReflectionClass $reflection, array $filePaths): array { foreach ($reflection->interfaces() as $interface) { $filePaths[] = $interface->sourceCode()->uri()?->path(); diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 41a723716..b025e7879 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1295,46 +1295,6 @@ parameters: count: 1 path: lib/Extension/ClassMover/Application/ClassReferences.php - - - message: "#^Method Phpactor\\\\Extension\\\\ClassMover\\\\Application\\\\Finder\\\\FileFinder\\:\\:allPhpFiles\\(\\) has no return type specified\\.$#" - count: 1 - path: lib/Extension/ClassMover/Application/Finder/FileFinder.php - - - - message: "#^Method Phpactor\\\\Extension\\\\ClassMover\\\\Application\\\\Finder\\\\FileFinder\\:\\:interfaceFilePaths\\(\\) has no return type specified\\.$#" - count: 1 - path: lib/Extension/ClassMover/Application/Finder/FileFinder.php - - - - message: "#^Method Phpactor\\\\Extension\\\\ClassMover\\\\Application\\\\Finder\\\\FileFinder\\:\\:interfaceFilePaths\\(\\) has parameter \\$filePaths with no type specified\\.$#" - count: 1 - path: lib/Extension/ClassMover/Application/Finder/FileFinder.php - - - - message: "#^Method Phpactor\\\\Extension\\\\ClassMover\\\\Application\\\\Finder\\\\FileFinder\\:\\:parentFilePaths\\(\\) has no return type specified\\.$#" - count: 1 - path: lib/Extension/ClassMover/Application/Finder/FileFinder.php - - - - message: "#^Method Phpactor\\\\Extension\\\\ClassMover\\\\Application\\\\Finder\\\\FileFinder\\:\\:parentFilePaths\\(\\) has parameter \\$filePaths with no type specified\\.$#" - count: 1 - path: lib/Extension/ClassMover/Application/Finder/FileFinder.php - - - - message: "#^Method Phpactor\\\\Extension\\\\ClassMover\\\\Application\\\\Finder\\\\FileFinder\\:\\:pathsFromReflectionClass\\(\\) has no return type specified\\.$#" - count: 1 - path: lib/Extension/ClassMover/Application/Finder/FileFinder.php - - - - message: "#^Method Phpactor\\\\Extension\\\\ClassMover\\\\Application\\\\Finder\\\\FileFinder\\:\\:traitFilePaths\\(\\) has no return type specified\\.$#" - count: 1 - path: lib/Extension/ClassMover/Application/Finder/FileFinder.php - - - - message: "#^Method Phpactor\\\\Extension\\\\ClassMover\\\\Application\\\\Finder\\\\FileFinder\\:\\:traitFilePaths\\(\\) has parameter \\$filePaths with no type specified\\.$#" - count: 1 - path: lib/Extension/ClassMover/Application/Finder/FileFinder.php - - message: "#^Parameter \\#2 \\$subject of function preg_match expects string, string\\|false given\\.$#" count: 1 From 36cb706dbb03b5d83bf87d879701a5317ce3d5ff Mon Sep 17 00:00:00 2001 From: mamazu <14860264+mamazu@users.noreply.github.com> Date: Sun, 17 Mar 2024 18:05:56 +0100 Subject: [PATCH 2/3] Adding type safety --- .../Application/Finder/FileFinder.php | 45 ++++++++++++++----- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/lib/Extension/ClassMover/Application/Finder/FileFinder.php b/lib/Extension/ClassMover/Application/Finder/FileFinder.php index 1cb887cda..ec30223db 100644 --- a/lib/Extension/ClassMover/Application/Finder/FileFinder.php +++ b/lib/Extension/ClassMover/Application/Finder/FileFinder.php @@ -41,8 +41,8 @@ public function filesFor(Filesystem $filesystem, ReflectionClassLike $reflection // we have public members or a non-class, we need to search the // whole tree, but we can discount any files which do not contain // the member name string. - return $this->allPhpFiles($filesystem)->filter(function (SplFileInfo $file) use ($memberName) { - return preg_match('{' . $memberName . '}', file_get_contents($file->getPathname())); + return $this->allPhpFiles($filesystem)->filter(function (SplFileInfo $file) use ($memberName): bool { + return preg_match('{' . $memberName . '}', file_get_contents($file->getPathname())) === 1; }); } @@ -62,7 +62,9 @@ private function pathsFromReflectionClass(ReflectionClass $reflection, bool $pri $path = $reflection->sourceCode()->uri()?->path(); if (!$path) { - throw new RuntimeException('Source has no path associated with it'); + throw new RuntimeException( + sprintf('Source class "%s" has no path associated with it', $reflection->name()), + ); } $filePaths = [ $path ]; @@ -84,15 +86,21 @@ private function allPhpFiles(Filesystem $filesystem): FileList } /** - * @param array $filePaths + * @param array $filePaths * - * @return array + * @return array */ private function parentFilePaths(ReflectionClass $reflection, array $filePaths): array { $context = $reflection->parent(); while ($context) { - $filePaths[] = $context->sourceCode()->uri()?->path(); + $path = $context->sourceCode()->uri()?->path(); + if (!$path) { + throw new RuntimeException( + sprintf('Source class "%s" has no path associated with it', $context->name()), + ); + } + $filePaths[] = $path; $context = $context->parent(); } @@ -100,27 +108,40 @@ private function parentFilePaths(ReflectionClass $reflection, array $filePaths): } /** - * @param array $filePaths + * @param array $filePaths * - * @return array + * @return array */ private function traitFilePaths(ReflectionClass $reflection, array $filePaths): array { foreach ($reflection->traits() as $trait) { - $filePaths[] = $trait->sourceCode()->uri()?->path(); + $path = $trait->sourceCode()->uri()?->path(); + if (!$path) { + throw new RuntimeException( + sprintf('Source trait "%s" has no path associated with it', $trait->name()), + ); + } + $filePaths[] = $path; } return $filePaths; } /** - * @param array $filePaths + * @param array $filePaths * - * @return array + * @return array */ private function interfaceFilePaths(ReflectionClass $reflection, array $filePaths): array { foreach ($reflection->interfaces() as $interface) { - $filePaths[] = $interface->sourceCode()->uri()?->path(); + $path = $interface->sourceCode()->uri()?->path(); + if (!$path) { + throw new RuntimeException( + sprintf('Source interface "%s" has no path associated with it', $interface->name()), + ); + } + + $filePaths[] = $path; } return $filePaths; From 41d974a7be34a8c557f9da1c5bb2f5829b998d7c Mon Sep 17 00:00:00 2001 From: mamazu <14860264+mamazu@users.noreply.github.com> Date: Mon, 18 Mar 2024 11:15:55 +0100 Subject: [PATCH 3/3] Using continue instead of exceptions --- .../Application/Finder/FileFinder.php | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/lib/Extension/ClassMover/Application/Finder/FileFinder.php b/lib/Extension/ClassMover/Application/Finder/FileFinder.php index ec30223db..4a8e62210 100644 --- a/lib/Extension/ClassMover/Application/Finder/FileFinder.php +++ b/lib/Extension/ClassMover/Application/Finder/FileFinder.php @@ -95,10 +95,8 @@ private function parentFilePaths(ReflectionClass $reflection, array $filePaths): $context = $reflection->parent(); while ($context) { $path = $context->sourceCode()->uri()?->path(); - if (!$path) { - throw new RuntimeException( - sprintf('Source class "%s" has no path associated with it', $context->name()), - ); + if ($path === null) { + continue; } $filePaths[] = $path; $context = $context->parent(); @@ -116,10 +114,8 @@ private function traitFilePaths(ReflectionClass $reflection, array $filePaths): { foreach ($reflection->traits() as $trait) { $path = $trait->sourceCode()->uri()?->path(); - if (!$path) { - throw new RuntimeException( - sprintf('Source trait "%s" has no path associated with it', $trait->name()), - ); + if ($path === null) { + continue; } $filePaths[] = $path; } @@ -135,12 +131,9 @@ private function interfaceFilePaths(ReflectionClass $reflection, array $filePath { foreach ($reflection->interfaces() as $interface) { $path = $interface->sourceCode()->uri()?->path(); - if (!$path) { - throw new RuntimeException( - sprintf('Source interface "%s" has no path associated with it', $interface->name()), - ); + if ($path === null) { + continue; } - $filePaths[] = $path; }