diff --git a/.gitignore b/.gitignore index 8eeb092..42c2425 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ /coverage.xml /tests/tmp/* /.phpunit.cache/ +/src-sa/.phpunit.cache/ /.claude/ /.qodo/ /web-query/ diff --git a/composer-require-checker.json b/composer-require-checker.json index f27f31d..8739b33 100644 --- a/composer-require-checker.json +++ b/composer-require-checker.json @@ -5,7 +5,22 @@ "array", "string", "int", "float", "bool", "iterable", "callable", "void", "object", "mixed", "never", "Roave\\BetterReflection\\Reflector\\DefaultReflector", "Roave\\BetterReflection\\Reflector\\ClassReflector", - "json_decode", "json_encode", "JSON_THROW_ON_ERROR" + "json_decode", "json_encode", "JSON_THROW_ON_ERROR", + "ctype_alnum", + "ctype_alpha", + "PhpParser\\Node", + "PhpParser\\Node\\Attribute", + "PhpParser\\Node\\Expr", + "PhpParser\\Node\\Expr\\ClassConstFetch", + "PhpParser\\Node\\Identifier", + "PhpParser\\Node\\Name", + "PhpParser\\Node\\Scalar\\String_", + "PhpParser\\Node\\Stmt\\ClassMethod", + "PHPStan\\Analyser\\Scope", + "PHPStan\\Reflection\\ReflectionProvider", + "PHPStan\\Rules\\Rule", + "PHPStan\\Rules\\RuleErrorBuilder", + "PHPStan\\Type\\ObjectType" ], "php-core-extensions" : [ "Core", diff --git a/composer.json b/composer.json index d1863b5..3a678df 100644 --- a/composer.json +++ b/composer.json @@ -26,10 +26,12 @@ }, "require-dev": { "bamarni/composer-bin-plugin": "^1.8", + "phpstan/phpstan": "^2.1", "phpunit/phpunit": "^11.5" }, "autoload": { "psr-4": { + "Ray\\MediaQuery\\PHPStan\\": "src-sa/src/", "Ray\\MediaQuery\\": [ "src/", "src-deprecated/" @@ -38,6 +40,7 @@ }, "autoload-dev": { "psr-4": { + "Ray\\MediaQuery\\PHPStan\\Tests\\": "src-sa/tests/", "Ray\\MediaQuery\\": [ "tests/", "tests/Fake" @@ -48,21 +51,23 @@ "scripts": { "coverage": "php -dzend_extension=xdebug.so -dxdebug.mode=coverage ./vendor/bin/phpunit --coverage-text --coverage-html=build/coverage", "pcov": "php -dextension=pcov.so -d pcov.enabled=1 ./vendor/bin/phpunit --coverage-text --coverage-html=build/coverage --coverage-clover=coverage.xml", - "cs": "./vendor/bin/phpcs", - "cs-fix": "./vendor/bin/phpcbf src tests docs/tutorial/src/Blog", - "metrics": "./vendor/bin/phpmetrics --report-html=build/metrics --exclude=Exception src", + "cs": "vendor-bin/tools/vendor/bin/phpcs", + "cs-fix": "vendor-bin/tools/vendor/bin/phpcbf src tests docs/tutorial/src/Blog src-sa/src src-sa/tests", + "metrics": "vendor-bin/tools/vendor/bin/phpmetrics --report-html=build/metrics --exclude=Exception src", "clean": [ - "./vendor/bin/phpstan clear-result-cache", - "./vendor/bin/psalm --clear-cache" + "vendor/bin/phpstan clear-result-cache", + "php -d error_reporting=8191 vendor-bin/tools/vendor/bin/psalm --clear-cache" ], "sa": [ - "./vendor/bin/psalm --monochrome --show-info=true", - "./vendor/bin/phpstan analyse -c phpstan.neon" + "php -d error_reporting=8191 vendor-bin/tools/vendor/bin/psalm --monochrome --show-info=true --no-cache", + "vendor/bin/phpstan analyse -c phpstan.neon --memory-limit=1G" ], + "test:phpstan": "vendor/bin/phpunit -c src-sa/phpunit.xml.dist", "test": "./vendor/bin/phpunit", "tutorial": "php -d zend.assertions=1 -d assert.exception=1 docs/tutorial/src/run.php", "tests": [ "@cs", + "@test:phpstan", "@sa", "@test", "@tutorial" @@ -78,6 +83,7 @@ "scripts-descriptions": { "test": "Run unit tests", "tutorial": "Run the tutorial answer code end-to-end as an integration check", + "test:phpstan": "Run PHPStan extension rule tests", "coverage": "Generate test coverage report", "pcov": "Generate test coverage report (pcov)", "cs": "Check the coding style", @@ -103,6 +109,7 @@ }, "suggest": { "koriym/csv-entities": "Provides one-to-many entity relation", + "phpstan/phpstan": "Required to use the optional Ray.MediaQuery PHPStan extension.", "ray/web-query": "For Web API query support" }, "extra": { diff --git a/phpcs.xml b/phpcs.xml index fba661c..930aee2 100755 --- a/phpcs.xml +++ b/phpcs.xml @@ -17,6 +17,8 @@ src tests + src-sa/src + src-sa/tests @@ -83,4 +85,5 @@ */Fake/* */tmp/* + */src-sa/tests/Rules/Data/* diff --git a/phpstan.neon b/phpstan.neon index 59a88a0..80bd01d 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,10 +1,22 @@ +includes: + - src-sa/extension.neon + parameters: level: max + parallel: + maximumNumberOfProcesses: 1 paths: - src + - src-sa/src - tests - docs/tutorial/src excludePaths: - tests/Fake/* - tests/tmp/* - src/MediaQueryModule.php + rayMediaQuery: + sqlDirectories: + - docs/tutorial/src/sql + factoryMethod: factory + strict: false + strictPlaceholderCheck: false diff --git a/src-sa/README.md b/src-sa/README.md new file mode 100644 index 0000000..bf3460f --- /dev/null +++ b/src-sa/README.md @@ -0,0 +1,39 @@ +# Ray.MediaQuery PHPStan Extension + +Static analysis rules for Ray.MediaQuery query contracts. + +The extension is bundled with `ray/media-query` as an optional development +feature. It is not a runtime dependency. Projects that want to enable it should +install PHPStan separately: + +```bash +composer require --dev phpstan/phpstan +``` + +## Configuration + +Include the extension from `phpstan.neon` and configure SQL directories: + +```neon +includes: + - vendor/ray/media-query/src-sa/extension.neon + +parameters: + rayMediaQuery: + sqlDirectories: + - docs/tutorial/src/sql +``` + +When developing this repository itself, include `src-sa/extension.neon` instead. + +The MVP rules verify that `#[DbQuery]` SQL files exist and are non-empty, +that `#[Pager]` methods return `PagesInterface`, that `PagesInterface` return +methods declare `#[Pager]`, and that `factory:` classes define the configured +factory method. + +## PHPStan result cache + +The rules read `.sql` files from disk. PHPStan's result cache does not always +track non-PHP file changes, so local runs may keep stale diagnostics after only +SQL files change. Clear the cache with `composer clean` or +`vendor/bin/phpstan clear-result-cache` when validating SQL-only edits. diff --git a/src-sa/extension.neon b/src-sa/extension.neon new file mode 100644 index 0000000..dd9e98d --- /dev/null +++ b/src-sa/extension.neon @@ -0,0 +1,23 @@ +parameters: + rayMediaQuery: + sqlDirectories: [] + factoryMethod: factory + strict: false + strictPlaceholderCheck: false + +parametersSchema: + rayMediaQuery: structure([ + sqlDirectories: listOf(string()) + factoryMethod: string() + strict: bool() + strictPlaceholderCheck: bool() + ]) + +services: + - + class: Ray\MediaQuery\PHPStan\Rules\DbQueryContractRule + arguments: + sqlDirectories: %rayMediaQuery.sqlDirectories% + factoryMethod: %rayMediaQuery.factoryMethod% + tags: + - phpstan.rules.rule diff --git a/src-sa/phpunit.xml.dist b/src-sa/phpunit.xml.dist new file mode 100644 index 0000000..ac12058 --- /dev/null +++ b/src-sa/phpunit.xml.dist @@ -0,0 +1,14 @@ + + + + + tests + + + + + + diff --git a/src-sa/src/Rules/DbQueryContractRule.php b/src-sa/src/Rules/DbQueryContractRule.php new file mode 100644 index 0000000..4cbc28c --- /dev/null +++ b/src-sa/src/Rules/DbQueryContractRule.php @@ -0,0 +1,213 @@ + */ +final class DbQueryContractRule implements Rule +{ + private SqlFileResolver $sqlFileResolver; + + /** @param list $sqlDirectories */ + public function __construct( + private readonly ReflectionProvider $reflectionProvider, + array $sqlDirectories, + private readonly string $factoryMethod = 'factory', + ) { + $this->sqlFileResolver = new SqlFileResolver($sqlDirectories); + } + + public function getNodeType(): string + { + return ClassMethod::class; + } + + /** + * @param ClassMethod $node + * + * @return list + */ + public function processNode(Node $node, Scope $scope): array + { + $dbQuery = $this->dbQueryMetadata($node, $scope); + if (! $dbQuery instanceof DbQueryMetadata) { + return []; + } + + $errors = []; + $sqlFile = $this->sqlFileResolver->resolve($dbQuery->id); + if ($sqlFile === null) { + $errors[] = RuleErrorBuilder::message(sprintf( + 'Ray.MediaQuery SQL file for query "%s" was not found in configured sqlDirectories.', + $dbQuery->id, + ))->identifier('rayMediaQuery.sqlFileNotFound')->build(); + } else { + $contents = file_get_contents($sqlFile); + if (is_string($contents) && trim($contents, " \t\n\r\0\x0B;") === '') { + $errors[] = RuleErrorBuilder::message(sprintf( + 'Ray.MediaQuery SQL file for query "%s" is empty.', + $dbQuery->id, + ))->identifier('rayMediaQuery.emptySqlFile')->build(); + } + } + + $classReflection = $scope->getClassReflection(); + if ($classReflection === null) { + return $errors; + } + + $methodReflection = $classReflection->getNativeMethod($node->name->toString()); + $returnType = $methodReflection->getVariants()[0]->getReturnType(); + $returnsPages = (new ObjectType(PagesInterface::class))->isSuperTypeOf($returnType)->yes(); + $returnsPostQuery = (new ObjectType(PostQueryInterface::class))->isSuperTypeOf($returnType)->yes(); + $hasPager = $this->hasAttribute($node, $scope, Pager::class); + + if ($hasPager && ! $returnsPages) { + $errors[] = RuleErrorBuilder::message( + 'Ray.MediaQuery method with #[Pager] must return PagesInterface or an implementation.', + )->identifier('rayMediaQuery.pagerReturnMismatch')->build(); + } + + if (! $hasPager && $returnsPages && ! $returnsPostQuery) { + $errors[] = RuleErrorBuilder::message( + 'Ray.MediaQuery method returning PagesInterface or an implementation must declare #[Pager].', + )->identifier('rayMediaQuery.missingPagerAttribute')->build(); + } + + if ($dbQuery->factory !== '') { + $this->validateFactory($dbQuery, $errors); + } + + return $errors; + } + + private function dbQueryMetadata(ClassMethod $node, Scope $scope): DbQueryMetadata|null + { + foreach ($node->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attribute) { + if ($this->attributeName($attribute, $scope) !== DbQuery::class) { + continue; + } + + $id = $this->attributeStringArgument($attribute, 0, 'id', $scope) ?? ''; + if ($id === '') { + return null; + } + + $factory = $this->attributeStringArgument($attribute, 2, 'factory', $scope) ?? ''; + + return new DbQueryMetadata($id, $factory); + } + } + + return null; + } + + private function hasAttribute(ClassMethod $node, Scope $scope, string $attributeClass): bool + { + foreach ($node->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attribute) { + if ($this->attributeName($attribute, $scope) === $attributeClass) { + return true; + } + } + } + + return false; + } + + private function attributeName(Attribute $attribute, Scope $scope): string + { + return $scope->resolveName($attribute->name); + } + + private function attributeStringArgument(Attribute $attribute, int $position, string $name, Scope $scope): string|null + { + foreach ($attribute->args as $index => $arg) { + if ($arg->name instanceof Identifier && $arg->name->toString() !== $name) { + continue; + } + + if ($arg->name === null && $index !== $position) { + continue; + } + + return $this->stringValue($arg->value, $scope); + } + + return null; + } + + private function stringValue(Expr $expr, Scope $scope): string|null + { + if ($expr instanceof String_) { + return $expr->value; + } + + if ($expr instanceof ClassConstFetch && $expr->name instanceof Identifier && $expr->name->toString() === 'class') { + if ($expr->class instanceof Name) { + return $this->resolveName($expr->class, $scope); + } + } + + return null; + } + + private function resolveName(Name $name, Scope $scope): string + { + return $scope->resolveName($name); + } + + /** @param list $errors */ + private function validateFactory(DbQueryMetadata $dbQuery, array &$errors): void + { + if (! $this->reflectionProvider->hasClass($dbQuery->factory)) { + $errors[] = RuleErrorBuilder::message(sprintf( + 'Ray.MediaQuery factory class "%s" for query "%s" was not found.', + $dbQuery->factory, + $dbQuery->id, + ))->identifier('rayMediaQuery.invalidFactory')->build(); + + return; + } + + $classReflection = $this->reflectionProvider->getClass($dbQuery->factory); + if ($classReflection->hasMethod($this->factoryMethod)) { + return; + } + + $errors[] = RuleErrorBuilder::message(sprintf( + 'Ray.MediaQuery factory class "%s" for query "%s" must define method "%s()".', + $dbQuery->factory, + $dbQuery->id, + $this->factoryMethod, + ))->identifier('rayMediaQuery.invalidFactory')->build(); + } +} diff --git a/src-sa/src/Support/DbQueryMetadata.php b/src-sa/src/Support/DbQueryMetadata.php new file mode 100644 index 0000000..ec0c928 --- /dev/null +++ b/src-sa/src/Support/DbQueryMetadata.php @@ -0,0 +1,14 @@ + */ + public function extract(string $sql): array + { + $state = self::DEFAULT; + $parameters = []; + $seen = []; + $length = strlen($sql); + + for ($i = 0; $i < $length; $i++) { + $char = $sql[$i]; + $next = $sql[$i + 1] ?? ''; + + if ($state === self::LINE_COMMENT) { + if ($char === "\n" || $char === "\r") { + $state = self::DEFAULT; + } + + continue; + } + + if ($state === self::BLOCK_COMMENT) { + if ($char === '*' && $next === '/') { + $i++; + $state = self::DEFAULT; + } + + continue; + } + + if ($state === self::SINGLE_QUOTE) { + if ($char === "'" && $next === "'") { + $i++; + continue; + } + + if ($char === "'") { + $state = self::DEFAULT; + } + + continue; + } + + if ($state === self::DOUBLE_QUOTE) { + if ($char === '"' && $next === '"') { + $i++; + continue; + } + + if ($char === '"') { + $state = self::DEFAULT; + } + + continue; + } + + if ($state === self::BACKTICK) { + if ($char === '`') { + $state = self::DEFAULT; + } + + continue; + } + + if ($state === self::BRACKET) { + if ($char === ']') { + $state = self::DEFAULT; + } + + continue; + } + + if ($char === '-' && $next === '-') { + $i++; + $state = self::LINE_COMMENT; + continue; + } + + if ($char === '/' && $next === '*') { + $i++; + $state = self::BLOCK_COMMENT; + continue; + } + + if ($char === "'") { + $state = self::SINGLE_QUOTE; + continue; + } + + if ($char === '"') { + $state = self::DOUBLE_QUOTE; + continue; + } + + if ($char === '`') { + $state = self::BACKTICK; + continue; + } + + if ($char === '[') { + $state = self::BRACKET; + continue; + } + + if ($char !== ':') { + continue; + } + + $previous = $sql[$i - 1] ?? ''; + if ($previous === ':' || $next === ':' || $next === '=') { + continue; + } + + if (! $this->isNameStart($next)) { + continue; + } + + $name = $next; + $i++; + while ($i + 1 < $length && $this->isNameChar($sql[$i + 1])) { + $i++; + $name .= $sql[$i]; + } + + if (isset($seen[$name])) { + continue; + } + + $seen[$name] = true; + $parameters[] = $name; + } + + return $parameters; + } + + private function isNameStart(string $char): bool + { + return $char === '_' || ($char !== '' && ctype_alpha($char)); + } + + private function isNameChar(string $char): bool + { + return $char === '_' || ($char !== '' && ctype_alnum($char)); + } +} diff --git a/src-sa/src/Support/SqlFileResolver.php b/src-sa/src/Support/SqlFileResolver.php new file mode 100644 index 0000000..d16da4a --- /dev/null +++ b/src-sa/src/Support/SqlFileResolver.php @@ -0,0 +1,47 @@ + $sqlDirectories */ + public function __construct( + private readonly array $sqlDirectories, + ) { + } + + public function resolve(string $sqlId): string|null + { + foreach ($this->sqlDirectories as $sqlDirectory) { + $directory = $this->absolutePath($sqlDirectory); + $sqlFile = sprintf('%s/%s.sql', rtrim($directory, '/'), $sqlId); + if (is_file($sqlFile)) { + return $sqlFile; + } + } + + return null; + } + + private function absolutePath(string $path): string + { + if ($path === '') { + return (string) getcwd(); + } + + if (preg_match('#^([A-Za-z]:)?/#', $path) === 1) { + return $path; + } + + return (string) getcwd() . '/' . ltrim($path, '/'); + } +} diff --git a/src-sa/tests/Rules/Data/FactoryWithoutFactoryMethod.php b/src-sa/tests/Rules/Data/FactoryWithoutFactoryMethod.php new file mode 100644 index 0000000..7c098a2 --- /dev/null +++ b/src-sa/tests/Rules/Data/FactoryWithoutFactoryMethod.php @@ -0,0 +1,15 @@ + */ + #[DbQuery('valid')] + #[Pager(perPage: 10)] + public function pages(): Pages; + + #[DbQuery('valid')] + public function postQuery(): ValidPostQueryResult; +} diff --git a/src-sa/tests/Rules/DbQueryContractRuleTest.php b/src-sa/tests/Rules/DbQueryContractRuleTest.php new file mode 100644 index 0000000..a8aaf95 --- /dev/null +++ b/src-sa/tests/Rules/DbQueryContractRuleTest.php @@ -0,0 +1,65 @@ + */ +final class DbQueryContractRuleTest extends RuleTestCase +{ + /** @return list */ + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/../phpstan-rule-tests.neon']; + } + + #[Override] + protected function getRule(): Rule + { + return new DbQueryContractRule( + self::getContainer()->getByType(ReflectionProvider::class), + [__DIR__ . '/Data/sql'], + ); + } + + public function testValidQueries(): void + { + $this->analyse([__DIR__ . '/Data/valid.php'], []); + } + + public function testInvalidQueries(): void + { + $this->analyse([__DIR__ . '/Data/invalid.php'], [ + [ + 'Ray.MediaQuery SQL file for query "missing" was not found in configured sqlDirectories.', + 13, + ], + [ + 'Ray.MediaQuery SQL file for query "empty" is empty.', + 19, + ], + [ + 'Ray.MediaQuery method with #[Pager] must return PagesInterface or an implementation.', + 25, + ], + [ + 'Ray.MediaQuery method returning PagesInterface or an implementation must declare #[Pager].', + 32, + ], + [ + 'Ray.MediaQuery factory class "Ray\\MediaQuery\\PHPStan\\Tests\\Rules\\Data\\MissingFactory" for query "valid" was not found.', + 38, + ], + [ + 'Ray.MediaQuery factory class "Ray\\MediaQuery\\PHPStan\\Tests\\Rules\\Data\\FactoryWithoutFactoryMethod" for query "valid" must define method "factory()".', + 44, + ], + ]); + } +} diff --git a/src-sa/tests/Support/NamedParameterExtractorTest.php b/src-sa/tests/Support/NamedParameterExtractorTest.php new file mode 100644 index 0000000..2d4f0fc --- /dev/null +++ b/src-sa/tests/Support/NamedParameterExtractorTest.php @@ -0,0 +1,34 @@ + $expected */ + #[DataProvider('sqlProvider')] + public function testExtract(string $sql, array $expected): void + { + self::assertSame($expected, (new NamedParameterExtractor())->extract($sql)); + } + + /** @return iterable}> */ + public static function sqlProvider(): iterable + { + yield 'simple' => ['SELECT * FROM todo WHERE id = :id', ['id']]; + yield 'string literal' => ['SELECT \':ignored\', id FROM todo WHERE id = :id', ['id']]; + yield 'line comment' => ["-- :ignored\nSELECT * FROM todo WHERE id = :id", ['id']]; + yield 'block comment' => ['SELECT /* :ignored */ * FROM todo WHERE id = :id', ['id']]; + yield 'postgres cast' => ['SELECT created_at::date FROM todo WHERE id = :id', ['id']]; + yield 'mysql assignment' => ['SELECT @x := 1, id FROM todo WHERE id = :id', ['id']]; + yield 'duplicates' => ['UPDATE todo SET parent_id = :id WHERE id = :id', ['id']]; + yield 'multiple statements' => ["SELECT 'a;b' FROM todo WHERE id = :id; UPDATE todo SET title = :title", ['id', 'title']]; + yield 'quoted identifiers' => ['SELECT `:ignored`, ":alsoIgnored", [stillIgnored] FROM todo WHERE id = :id', ['id']]; + yield 'camel and snake' => ['INSERT INTO memo VALUES (:user_id, :todoId)', ['user_id', 'todoId']]; + } +} diff --git a/src-sa/tests/bootstrap.php b/src-sa/tests/bootstrap.php new file mode 100644 index 0000000..188afe6 --- /dev/null +++ b/src-sa/tests/bootstrap.php @@ -0,0 +1,38 @@ + $root . '/src-sa/tests/', + 'Ray\\MediaQuery\\PHPStan\\' => $root . '/src-sa/src/', + 'Ray\\MediaQuery\\' => $root . '/src/', + 'Tutorial\\Blog\\' => $root . '/docs/tutorial/src/Blog/', + ]; + + foreach ($prefixes as $prefix => $baseDir) { + if (! str_starts_with($class, $prefix)) { + continue; + } + + $relative = substr($class, strlen($prefix)); + $file = $baseDir . str_replace('\\', '/', $relative) . '.php'; + if (is_file($file)) { + require_once $file; + } + + return; + } +}); diff --git a/src-sa/tests/phpstan-rule-tests.neon b/src-sa/tests/phpstan-rule-tests.neon new file mode 100644 index 0000000..3f77b95 --- /dev/null +++ b/src-sa/tests/phpstan-rule-tests.neon @@ -0,0 +1,4 @@ +parameters: + scanFiles: + - %currentWorkingDirectory%/src-sa/tests/Rules/Data/valid.php + - %currentWorkingDirectory%/src-sa/tests/Rules/Data/invalid.php