diff --git a/phpstan.php b/phpstan.php index 9678455..b635224 100644 --- a/phpstan.php +++ b/phpstan.php @@ -61,6 +61,14 @@ __DIR__ . '/tests/*/Generated/*', ], ], + [ + 'identifiers' => [ + 'shipmonk.deadMethod', + ], + 'paths' => [ + __DIR__ . '/tests/StaleImportRemoval/ControllerWithStaleImport.php', + ], + ], [ 'identifiers' => [ 'shipmonk.deadConstant', diff --git a/src/Executor/PlanExecutor.php b/src/Executor/PlanExecutor.php index 786d0dd..f99cd79 100644 --- a/src/Executor/PlanExecutor.php +++ b/src/Executor/PlanExecutor.php @@ -25,6 +25,7 @@ use Ruudk\GraphQLCodeGenerator\Generator\OperationClassGenerator; use Ruudk\GraphQLCodeGenerator\GraphQL\AST\Printer; use Ruudk\GraphQLCodeGenerator\PHP\Visitor\OperationInjector; +use Ruudk\GraphQLCodeGenerator\PHP\Visitor\StaleImportRemover; use Ruudk\GraphQLCodeGenerator\PHP\Visitor\UseStatementInserter; use Ruudk\GraphQLCodeGenerator\Planner\OperationPlan; use Ruudk\GraphQLCodeGenerator\Planner\Plan\DataClassPlan; @@ -138,6 +139,7 @@ public function execute(PlannerResult $plan) : array $newStmts = new NodeTraverser( new NodeConnectingVisitor(), + new StaleImportRemover($this->config->namespace, $fqcns), new UseStatementInserter($fqcns), )->traverse($newStmts); diff --git a/src/PHP/Visitor/StaleImportRemover.php b/src/PHP/Visitor/StaleImportRemover.php new file mode 100644 index 0000000..317fefc --- /dev/null +++ b/src/PHP/Visitor/StaleImportRemover.php @@ -0,0 +1,64 @@ + + */ + private array $validFqcns; + private string $namespacePattern; + + /** + * @param string $generatedNamespace The namespace for generated classes (e.g., "App\Generated") + * @param list $validFqcns The list of valid/current FQCNs to keep + */ + public function __construct( + string $generatedNamespace, + array $validFqcns, + ) { + $this->validFqcns = array_fill_keys($validFqcns, true); + + // Build a regex pattern to match generated operation imports + // Pattern: {namespace}\{Query|Mutation}\{OperationNameWithHash}\{ClassName} + // The hash is 6 hex characters + $escapedNamespace = preg_quote($generatedNamespace, '/'); + $this->namespacePattern = '/^' . $escapedNamespace . '\\\\(?:Query|Mutation)\\\\[A-Za-z]+[a-f0-9]{6}\\\\/'; + } + + #[Override] + public function enterNode(Node $node) : ?int + { + if ( ! $node instanceof Use_) { + return null; + } + + // Check each use clause + foreach ($node->uses as $use) { + $fqcn = $use->name->toString(); + + // If this import matches the generated pattern but is not in our valid list, remove it + if (preg_match($this->namespacePattern, $fqcn) === 1 && ! isset($this->validFqcns[$fqcn])) { + return NodeVisitor::REMOVE_NODE; + } + } + + return null; + } +} diff --git a/tests/StaleImportRemoval/ControllerWithStaleImport.php b/tests/StaleImportRemoval/ControllerWithStaleImport.php new file mode 100644 index 0000000..2a82fdf --- /dev/null +++ b/tests/StaleImportRemoval/ControllerWithStaleImport.php @@ -0,0 +1,24 @@ + $this->viewer ??= new Viewer($this->data['viewer']); + } + + /** + * @var list + */ + public readonly array $errors; + + /** + * @param array{ + * 'viewer': array{ + * 'login': string, + * }, + * } $data + * @param list $errors + */ + public function __construct( + private readonly array $data, + array $errors, + ) { + $this->errors = array_map(fn(array $error) => new Error($error), $errors); + } +} diff --git a/tests/StaleImportRemoval/Generated/Query/GetViewer6b9a6d/Data/Viewer.php b/tests/StaleImportRemoval/Generated/Query/GetViewer6b9a6d/Data/Viewer.php new file mode 100644 index 0000000..6fa9210 --- /dev/null +++ b/tests/StaleImportRemoval/Generated/Query/GetViewer6b9a6d/Data/Viewer.php @@ -0,0 +1,23 @@ + $this->login ??= $this->data['login']; + } + + /** + * @param array{ + * 'login': string, + * } $data + */ + public function __construct( + private readonly array $data, + ) {} +} diff --git a/tests/StaleImportRemoval/Generated/Query/GetViewer6b9a6d/Error.php b/tests/StaleImportRemoval/Generated/Query/GetViewer6b9a6d/Error.php new file mode 100644 index 0000000..31fe268 --- /dev/null +++ b/tests/StaleImportRemoval/Generated/Query/GetViewer6b9a6d/Error.php @@ -0,0 +1,23 @@ +message = $error['debugMessage'] ?? $error['message']; + } +} diff --git a/tests/StaleImportRemoval/Generated/Query/GetViewer6b9a6d/GetViewerQuery.php b/tests/StaleImportRemoval/Generated/Query/GetViewer6b9a6d/GetViewerQuery.php new file mode 100644 index 0000000..4d730f0 --- /dev/null +++ b/tests/StaleImportRemoval/Generated/Query/GetViewer6b9a6d/GetViewerQuery.php @@ -0,0 +1,40 @@ +client->graphql( + self::OPERATION_DEFINITION, + [ + ], + self::OPERATION_NAME, + ); + + return new Data( + $data['data'] ?? [], // @phpstan-ignore argument.type + $data['errors'] ?? [] // @phpstan-ignore argument.type + ); + } +} diff --git a/tests/StaleImportRemoval/Schema.graphql b/tests/StaleImportRemoval/Schema.graphql new file mode 100644 index 0000000..f197da5 --- /dev/null +++ b/tests/StaleImportRemoval/Schema.graphql @@ -0,0 +1,7 @@ +type Query { + viewer: Viewer! +} + +type Viewer { + login: String! +} diff --git a/tests/StaleImportRemoval/StaleImportRemovalTest.php b/tests/StaleImportRemoval/StaleImportRemovalTest.php new file mode 100644 index 0000000..1b0ded4 --- /dev/null +++ b/tests/StaleImportRemoval/StaleImportRemovalTest.php @@ -0,0 +1,99 @@ +withInlineProcessingDirectory(__DIR__); + } + + public function testStaleImportsAreRemoved() : void + { + // First, create a "stale" version of the controller file with old imports + $staleContent = <<<'PHP' + getConfig(); + $plan = new Planner($config)->plan(); + $files = new PlanExecutor($config)->execute($plan); + + // Check the output + self::assertArrayHasKey($controllerPath, $files, 'Controller file should be in output'); + $output = $files[$controllerPath]; + + // The stale imports should be removed + self::assertStringNotContainsString( + 'GetViewerabc123', + $output, + 'Stale import with old hash should be removed', + ); + self::assertStringNotContainsString( + 'GetViewerdef456', + $output, + 'Another stale import should also be removed', + ); + + // The correct import should be present + $matches = []; + preg_match_all( + '/use Ruudk\\\\GraphQLCodeGenerator\\\\StaleImportRemoval\\\\Generated\\\\Query\\\\GetViewer[a-f0-9]+\\\\GetViewerQuery/', + $output, + $matches, + ); + + self::assertCount( + 1, + $matches[0], + 'There should be exactly one import for GetViewerQuery with the correct hash', + ); + } finally { + // Restore the original content + file_put_contents($controllerPath, $originalContent); + } + } +} diff --git a/tests/StaleImportRemoval/StaleImportRemoverTest.php b/tests/StaleImportRemoval/StaleImportRemoverTest.php new file mode 100644 index 0000000..e9f7e5e --- /dev/null +++ b/tests/StaleImportRemoval/StaleImportRemoverTest.php @@ -0,0 +1,123 @@ +createForNewestSupportedVersion(); + $stmts = $parser->parse($code); + + self::assertNotNull($stmts); + + $remover = new StaleImportRemover( + 'App\Generated', + ['App\Generated\Query\GetViewer6b9a6d\GetViewerQuery'], // Only this one is valid + ); + + $traverser = new NodeTraverser(); + $traverser->addVisitor($remover); + $newStmts = $traverser->traverse($stmts); + + $printer = new Standard(); + $output = $printer->prettyPrintFile($newStmts); + + // The valid import should be... wait, it's not in the original code + // Let me check what happens to stale imports + self::assertStringNotContainsString('GetViewerabc123', $output, 'Stale abc123 should be removed'); + self::assertStringNotContainsString('GetViewerdef456', $output, 'Stale def456 should be removed'); + self::assertStringNotContainsString('CreateUser123abc', $output, 'Stale mutation should be removed'); + self::assertStringContainsString('SomeOther\Class\NotGenerated', $output, 'Non-generated import should remain'); + } + + public function testRegexPatternDirectly() : void + { + $namespace = 'App\Generated'; + $escapedNamespace = preg_quote($namespace, '/'); + $pattern = '/^' . $escapedNamespace . '\\\\(?:Query|Mutation)\\\\[A-Za-z]+[a-f0-9]{6}\\\\/'; + + // Should match stale imports + self::assertSame(1, preg_match($pattern, 'App\Generated\Query\GetViewerabc123\GetViewerQuery')); + self::assertSame(1, preg_match($pattern, 'App\Generated\Query\GetViewer6b9a6d\GetViewerQuery')); + self::assertSame(1, preg_match($pattern, 'App\Generated\Mutation\CreateUser123abc\CreateUserMutation')); + + // Should NOT match non-generated imports + self::assertSame(0, preg_match($pattern, 'SomeOther\Class\NotGenerated')); + self::assertSame(0, preg_match($pattern, 'App\Generated\Enum\SomeEnum')); + } + + public function testExactIntegrationScenario() : void + { + // This matches the exact scenario from the integration test + $namespace = 'Ruudk\GraphQLCodeGenerator\StaleImportRemoval\Generated'; + $escapedNamespace = preg_quote($namespace, '/'); + $pattern = '/^' . $escapedNamespace . '\\\\(?:Query|Mutation)\\\\[A-Za-z]+[a-f0-9]{6}\\\\/'; + + // The stale import + $stale = 'Ruudk\GraphQLCodeGenerator\StaleImportRemoval\Generated\Query\GetViewerabc123\GetViewerQuery'; + + // The correct import + $correct = 'Ruudk\GraphQLCodeGenerator\StaleImportRemoval\Generated\Query\GetViewer6b9a6d\GetViewerQuery'; + + self::assertSame(1, preg_match($pattern, $stale), 'Stale import should match pattern'); + self::assertSame(1, preg_match($pattern, $correct), 'Correct import should match pattern'); + } + + public function testRemovalWithMultipleVisitors() : void + { + // Simulate what happens in PlanExecutor + $code = <<<'PHP' + createForNewestSupportedVersion(); + $stmts = $parser->parse($code); + + self::assertNotNull($stmts); + + $fqcns = ['Ruudk\GraphQLCodeGenerator\StaleImportRemoval\Generated\Query\GetViewer6b9a6d\GetViewerQuery']; + + $traverser = new NodeTraverser(); + $traverser->addVisitor(new \PhpParser\NodeVisitor\NodeConnectingVisitor()); + $traverser->addVisitor(new StaleImportRemover('Ruudk\GraphQLCodeGenerator\StaleImportRemoval\Generated', $fqcns)); + $traverser->addVisitor(new \Ruudk\GraphQLCodeGenerator\PHP\Visitor\UseStatementInserter($fqcns)); + $newStmts = $traverser->traverse($stmts); + + $printer = new Standard(); + $output = $printer->prettyPrintFile($newStmts); + + self::assertStringNotContainsString('GetViewerabc123', $output, 'Stale abc123 should be removed'); + self::assertStringNotContainsString('GetViewerdef456', $output, 'Stale def456 should be removed'); + self::assertStringContainsString('GetViewer6b9a6d', $output, 'Correct import should be present'); + } +}