From f3377d90f9551225f23d937c193ed3e686860ab0 Mon Sep 17 00:00:00 2001 From: monayemislam Date: Thu, 15 May 2025 02:35:09 +0600 Subject: [PATCH 1/6] Enhance DocumentationMenuItem class with validation and new method. Added parameter descriptions for constructor, enforced href and label non-emptiness, and introduced hasDocumentation method to check file existence. --- src/Documentation/DocumentationMenuItem.php | 35 +++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/Documentation/DocumentationMenuItem.php b/src/Documentation/DocumentationMenuItem.php index 3e28aea48..c41756baa 100644 --- a/src/Documentation/DocumentationMenuItem.php +++ b/src/Documentation/DocumentationMenuItem.php @@ -10,7 +10,10 @@ final readonly class DocumentationMenuItem { /** - * @param non-empty-string|null $slug + * @param string $href The URL path for the documentation menu item. Must be a non-empty string starting with '/' + * @param string $label The display text for the menu item. Must be a non-empty string + * @param non-empty-string|null $slug The unique identifier for the documentation file. If null, indicates a menu item without content + * @param bool $isNew Whether this menu item represents new documentation */ public function __construct( private string $href, @@ -18,6 +21,9 @@ public function __construct( private ?string $slug, private bool $isNew = false, ) { + Assert::notEmpty($href, 'Documentation href cannot be empty'); + Assert::startsWith($href, '/', 'Documentation href must start with a forward slash'); + Assert::notEmpty($label, 'Documentation label cannot be empty'); } public function getHref(): string @@ -43,12 +49,37 @@ public function getSlug(): ?string return $this->slug; } + /** + * Checks if the documentation file exists for this menu item. + * + * @return bool True if the documentation file exists and is readable, false otherwise + */ + public function hasDocumentation(): bool + { + if ($this->slug === null) { + return false; + } + + $documentationFilePath = $this->getDocumentationFilePath(); + return file_exists($documentationFilePath) && is_readable($documentationFilePath); + } + public function getMarkdownContents(): string { Assert::notNull($this->slug); - $documentationFilePath = __DIR__ . '/../../resources/docs/' . $this->slug . '.md'; + $documentationFilePath = $this->getDocumentationFilePath(); Assert::fileExists($documentationFilePath); return FileSystem::read($documentationFilePath); } + + /** + * Gets the full path to the documentation file. + * + * @return string The absolute path to the documentation file + */ + private function getDocumentationFilePath(): string + { + return __DIR__ . '/../../resources/docs/' . $this->slug . '.md'; + } } From 6f4df6eed726014bf900297bf35b71b30ab7a6e9 Mon Sep 17 00:00:00 2001 From: monayemislam Date: Thu, 15 May 2025 02:44:47 +0600 Subject: [PATCH 2/6] refactor(AppliedRule): add class structure validation and constants --- src/ValueObject/AppliedRule.php | 46 ++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/src/ValueObject/AppliedRule.php b/src/ValueObject/AppliedRule.php index 5e341d098..c8a2dfeb6 100644 --- a/src/ValueObject/AppliedRule.php +++ b/src/ValueObject/AppliedRule.php @@ -6,9 +6,15 @@ use App\Exception\ShouldNotHappenException; use Nette\Utils\Strings; +use Webmozart\Assert\Assert; final readonly class AppliedRule { + private const EXPECTED_CLASS_PARTS_COUNT = 5; + private const CATEGORY_INDEX = 1; + private const NODE_CLASS_INDEX = 3; + private const SHORT_CLASS_INDEX = 4; + private string $shortRectorClass; public function __construct( @@ -40,16 +46,48 @@ public function getTestFixtureNamespace(): string public function getTestFixtureDirectoryPath(): string { $classParts = explode('\\', $this->rectorClass); + + // Validate class structure + Assert::count( + $classParts, + self::EXPECTED_CLASS_PARTS_COUNT, + sprintf( + 'Rector class "%s" must have exactly %d parts (e.g. "Rector\\Category\\Node\\SomeRector")', + $this->rectorClass, + self::EXPECTED_CLASS_PARTS_COUNT + ) + ); - $category = $classParts[1]; - $rulesDirectory = 'rules-tests/' . $category; + // Validate and get required parts + $category = $this->getClassPart($classParts, self::CATEGORY_INDEX, 'category'); + $nodeClass = $this->getClassPart($classParts, self::NODE_CLASS_INDEX, 'node class'); + $shortClass = $this->getClassPart($classParts, self::SHORT_CLASS_INDEX, 'short class'); - $nodeClass = $classParts[3]; - $shortClass = $classParts[4]; + $rulesDirectory = 'rules-tests/' . $category; return $rulesDirectory . '/Rector/' . $nodeClass . '/' . $shortClass . '/Fixture'; } + /** + * @param string[] $classParts + */ + private function getClassPart(array $classParts, int $index, string $partName): string + { + Assert::keyExists( + $classParts, + $index, + sprintf('Missing %s in rector class "%s"', $partName, $this->rectorClass) + ); + + $part = $classParts[$index]; + Assert::notEmpty( + $part, + sprintf('Empty %s in rector class "%s"', $partName, $this->rectorClass) + ); + + return $part; + } + /** * Mimics @see \App\RuleFilter\ValueObject\RuleMetadata::getSlug() */ From 8f3f937eeadb2955600e5c03d8ecf4b7e403bb41 Mon Sep 17 00:00:00 2001 From: monayemislam Date: Sun, 7 Dec 2025 10:50:56 +0600 Subject: [PATCH 3/6] fix(docs): throw exception in hasDocumentation() when file missing --- src/Documentation/DocumentationMenuItem.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Documentation/DocumentationMenuItem.php b/src/Documentation/DocumentationMenuItem.php index c41756baa..e096f17da 100644 --- a/src/Documentation/DocumentationMenuItem.php +++ b/src/Documentation/DocumentationMenuItem.php @@ -51,8 +51,10 @@ public function getSlug(): ?string /** * Checks if the documentation file exists for this menu item. + * Throws an exception if the file does not exist when a slug is provided. * - * @return bool True if the documentation file exists and is readable, false otherwise + * @return bool True if the documentation file exists, false if slug is null + * @throws \Webmozart\Assert\InvalidArgumentException If the file does not exist when slug is provided */ public function hasDocumentation(): bool { @@ -61,7 +63,9 @@ public function hasDocumentation(): bool } $documentationFilePath = $this->getDocumentationFilePath(); - return file_exists($documentationFilePath) && is_readable($documentationFilePath); + Assert::fileExists($documentationFilePath, sprintf('Documentation file must exist at "%s"', $documentationFilePath)); + + return true; } public function getMarkdownContents(): string From 68a6cba7dbd9552f1889b616329236a7e97b61e6 Mon Sep 17 00:00:00 2001 From: monayemislam Date: Sun, 7 Dec 2025 23:12:57 +0600 Subject: [PATCH 4/6] refactor: DocumentationMenuItem, AppliedRule --- src/Documentation/DocumentationMenuItem.php | 7 ++- src/ValueObject/AppliedRule.php | 51 +++++++++++++++------ 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src/Documentation/DocumentationMenuItem.php b/src/Documentation/DocumentationMenuItem.php index e096f17da..99575a0fc 100644 --- a/src/Documentation/DocumentationMenuItem.php +++ b/src/Documentation/DocumentationMenuItem.php @@ -10,7 +10,7 @@ final readonly class DocumentationMenuItem { /** - * @param string $href The URL path for the documentation menu item. Must be a non-empty string starting with '/' + * @param string $href The URL path for the documentation menu item. Must be a non-empty string starting with '/', 'http://', or 'https://' * @param string $label The display text for the menu item. Must be a non-empty string * @param non-empty-string|null $slug The unique identifier for the documentation file. If null, indicates a menu item without content * @param bool $isNew Whether this menu item represents new documentation @@ -22,7 +22,10 @@ public function __construct( private bool $isNew = false, ) { Assert::notEmpty($href, 'Documentation href cannot be empty'); - Assert::startsWith($href, '/', 'Documentation href must start with a forward slash'); + Assert::true( + str_starts_with($href, '/') || str_starts_with($href, 'http://') || str_starts_with($href, 'https://'), + 'Documentation href must start with a forward slash, http://, or https://' + ); Assert::notEmpty($label, 'Documentation label cannot be empty'); } diff --git a/src/ValueObject/AppliedRule.php b/src/ValueObject/AppliedRule.php index c8a2dfeb6..7481d60ef 100644 --- a/src/ValueObject/AppliedRule.php +++ b/src/ValueObject/AppliedRule.php @@ -10,10 +10,15 @@ final readonly class AppliedRule { - private const EXPECTED_CLASS_PARTS_COUNT = 5; + private const EXPECTED_CLASS_PARTS_COUNT_MIN = 5; + private const EXPECTED_CLASS_PARTS_COUNT_MAX = 6; private const CATEGORY_INDEX = 1; - private const NODE_CLASS_INDEX = 3; - private const SHORT_CLASS_INDEX = 4; + private const RECTOR_LITERAL_INDEX_5_PARTS = 2; + private const RECTOR_LITERAL_INDEX_6_PARTS = 3; + private const NODE_CLASS_INDEX_5_PARTS = 3; + private const NODE_CLASS_INDEX_6_PARTS = 4; + private const SHORT_CLASS_INDEX_5_PARTS = 4; + private const SHORT_CLASS_INDEX_6_PARTS = 5; private string $shortRectorClass; @@ -46,22 +51,42 @@ public function getTestFixtureNamespace(): string public function getTestFixtureDirectoryPath(): string { $classParts = explode('\\', $this->rectorClass); + $partsCount = count($classParts); - // Validate class structure - Assert::count( - $classParts, - self::EXPECTED_CLASS_PARTS_COUNT, + // Validate class structure - allow 5 or 6 parts (6 parts when version number is present) + Assert::true( + $partsCount >= self::EXPECTED_CLASS_PARTS_COUNT_MIN && $partsCount <= self::EXPECTED_CLASS_PARTS_COUNT_MAX, sprintf( - 'Rector class "%s" must have exactly %d parts (e.g. "Rector\\Category\\Node\\SomeRector")', + 'Rector class "%s" must have %d or %d parts (e.g. "Rector\\Category\\Rector\\Node\\SomeRector" or "Rector\\Category\\Version\\Rector\\Node\\SomeRector")', $this->rectorClass, - self::EXPECTED_CLASS_PARTS_COUNT + self::EXPECTED_CLASS_PARTS_COUNT_MIN, + self::EXPECTED_CLASS_PARTS_COUNT_MAX ) ); - // Validate and get required parts - $category = $this->getClassPart($classParts, self::CATEGORY_INDEX, 'category'); - $nodeClass = $this->getClassPart($classParts, self::NODE_CLASS_INDEX, 'node class'); - $shortClass = $this->getClassPart($classParts, self::SHORT_CLASS_INDEX, 'short class'); + // Validate first part is "Rector" + Assert::same($classParts[0], 'Rector', sprintf('Rector class "%s" must start with "Rector"', $this->rectorClass)); + + // Determine if we have a version part (6 parts) or not (5 parts) + $hasVersion = $partsCount === self::EXPECTED_CLASS_PARTS_COUNT_MAX; + + // Validate "Rector" literal is at the correct position + $rectorLiteralIndex = $hasVersion ? self::RECTOR_LITERAL_INDEX_6_PARTS : self::RECTOR_LITERAL_INDEX_5_PARTS; + Assert::same( + $classParts[$rectorLiteralIndex], + 'Rector', + sprintf('Rector class "%s" must have "Rector" at index %d', $this->rectorClass, $rectorLiteralIndex) + ); + + // Get required parts with correct indices + // When there's a version (6 parts), use the version as the category; otherwise use the base category + $categoryIndex = $hasVersion ? 2 : self::CATEGORY_INDEX; + $category = $this->getClassPart($classParts, $categoryIndex, 'category'); + $nodeClassIndex = $hasVersion ? self::NODE_CLASS_INDEX_6_PARTS : self::NODE_CLASS_INDEX_5_PARTS; + $shortClassIndex = $hasVersion ? self::SHORT_CLASS_INDEX_6_PARTS : self::SHORT_CLASS_INDEX_5_PARTS; + + $nodeClass = $this->getClassPart($classParts, $nodeClassIndex, 'node class'); + $shortClass = $this->getClassPart($classParts, $shortClassIndex, 'short class'); $rulesDirectory = 'rules-tests/' . $category; From 0cc7644c82a2875f7b846e41a2d0d455c364b308 Mon Sep 17 00:00:00 2001 From: monayemislam Date: Sun, 7 Dec 2025 23:39:10 +0600 Subject: [PATCH 5/6] refactor(FixtureLinkFactory): streamline package name handling and improve readability --- .../LinkFactory/FixtureLinkFactory.php | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/GitHubMagicLink/LinkFactory/FixtureLinkFactory.php b/src/GitHubMagicLink/LinkFactory/FixtureLinkFactory.php index 0ce9abce5..e5290cce8 100644 --- a/src/GitHubMagicLink/LinkFactory/FixtureLinkFactory.php +++ b/src/GitHubMagicLink/LinkFactory/FixtureLinkFactory.php @@ -53,29 +53,43 @@ public function create(RectorRun $rectorRun): string return $link; } - if (! in_array($match['Package'], self::RECTOR_PACKAGE_NAMES, true)) { - if (str_starts_with((string) $match['Package'], self::DOWNGRADE_PACKAGE_PREFIX)) { - return str_replace('rector-src', 'rector-downgrade-php', $link); + $packageName = (string) $match['Package']; + + // Check if it's a downgrade package + if (str_starts_with($packageName, self::DOWNGRADE_PACKAGE_PREFIX)) { + return str_replace('rector-src', 'rector-downgrade-php', $link); + } + + // Check if it's a known Rector package (e.g., Symfony, Symfony42, PHPUnit, Doctrine) + $isKnownPackage = false; + $basePackageName = null; + foreach (self::RECTOR_PACKAGE_NAMES as $knownPackage) { + if (str_starts_with($packageName, $knownPackage)) { + $isKnownPackage = true; + $basePackageName = $knownPackage; + break; } + } + if (! $isKnownPackage) { return $link; } $expectedNamespace = $rectorRun->getExpectedRectorTestNamespace(); $slashedexpectedNamespace = str_replace('\\', '/', $expectedNamespace); $slashedexpectedNamespace = str_replace( - 'Rector/Tests/' . $match['Package'] . '/', + 'Rector/Tests/' . $basePackageName . '/', '', $slashedexpectedNamespace ); $slashedexpectedNamespace = Strings::before($slashedexpectedNamespace, '/', 1); - $package = strtolower($match['Package']); + $package = strtolower($basePackageName); if (is_dir( __DIR__ . '/../../../vendor/rector/rector/vendor/rector/rector-' . $package . '/rules/' . $slashedexpectedNamespace )) { $link = str_replace( - 'rules-tests/' . $match['Package'] . '/Rector', + 'rules-tests/' . $packageName . '/Rector', 'rules-tests/' . $slashedexpectedNamespace, $link ); @@ -88,7 +102,7 @@ public function create(RectorRun $rectorRun): string return str_replace('rector-src', 'rector-' . $package, $link); } - $link = str_replace('rules-tests/' . $match['Package'] . '/Rector', 'tests/Rector', $link); + $link = str_replace('rules-tests/' . $packageName . '/Rector', 'tests/Rector', $link); return str_replace('rector-src', 'rector-' . $package, $link); } From fcf41858ec99ef25f26781965393cbe6e89ff187 Mon Sep 17 00:00:00 2001 From: monayemislam Date: Mon, 8 Dec 2025 00:04:58 +0600 Subject: [PATCH 6/6] Revert "refactor(FixtureLinkFactory): streamline package name handling and improve readability" This reverts commit 0cc7644c82a2875f7b846e41a2d0d455c364b308. --- .../LinkFactory/FixtureLinkFactory.php | 28 +++++-------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/src/GitHubMagicLink/LinkFactory/FixtureLinkFactory.php b/src/GitHubMagicLink/LinkFactory/FixtureLinkFactory.php index e5290cce8..0ce9abce5 100644 --- a/src/GitHubMagicLink/LinkFactory/FixtureLinkFactory.php +++ b/src/GitHubMagicLink/LinkFactory/FixtureLinkFactory.php @@ -53,43 +53,29 @@ public function create(RectorRun $rectorRun): string return $link; } - $packageName = (string) $match['Package']; - - // Check if it's a downgrade package - if (str_starts_with($packageName, self::DOWNGRADE_PACKAGE_PREFIX)) { - return str_replace('rector-src', 'rector-downgrade-php', $link); - } - - // Check if it's a known Rector package (e.g., Symfony, Symfony42, PHPUnit, Doctrine) - $isKnownPackage = false; - $basePackageName = null; - foreach (self::RECTOR_PACKAGE_NAMES as $knownPackage) { - if (str_starts_with($packageName, $knownPackage)) { - $isKnownPackage = true; - $basePackageName = $knownPackage; - break; + if (! in_array($match['Package'], self::RECTOR_PACKAGE_NAMES, true)) { + if (str_starts_with((string) $match['Package'], self::DOWNGRADE_PACKAGE_PREFIX)) { + return str_replace('rector-src', 'rector-downgrade-php', $link); } - } - if (! $isKnownPackage) { return $link; } $expectedNamespace = $rectorRun->getExpectedRectorTestNamespace(); $slashedexpectedNamespace = str_replace('\\', '/', $expectedNamespace); $slashedexpectedNamespace = str_replace( - 'Rector/Tests/' . $basePackageName . '/', + 'Rector/Tests/' . $match['Package'] . '/', '', $slashedexpectedNamespace ); $slashedexpectedNamespace = Strings::before($slashedexpectedNamespace, '/', 1); - $package = strtolower($basePackageName); + $package = strtolower($match['Package']); if (is_dir( __DIR__ . '/../../../vendor/rector/rector/vendor/rector/rector-' . $package . '/rules/' . $slashedexpectedNamespace )) { $link = str_replace( - 'rules-tests/' . $packageName . '/Rector', + 'rules-tests/' . $match['Package'] . '/Rector', 'rules-tests/' . $slashedexpectedNamespace, $link ); @@ -102,7 +88,7 @@ public function create(RectorRun $rectorRun): string return str_replace('rector-src', 'rector-' . $package, $link); } - $link = str_replace('rules-tests/' . $packageName . '/Rector', 'tests/Rector', $link); + $link = str_replace('rules-tests/' . $match['Package'] . '/Rector', 'tests/Rector', $link); return str_replace('rector-src', 'rector-' . $package, $link); }