diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 589085b..8228838 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -9,7 +9,7 @@ jobs: runs-on: ubuntu-22.04 strategy: matrix: - php: ['7.1', '7.2', '7.3', '7.4', '8.0', '8.1'] + php: ['7.1', '7.2', '7.3', '7.4', '8.0', '8.1', '8.2'] name: PHP ${{ matrix.php }} steps: - uses: actions/checkout@v3 @@ -32,7 +32,7 @@ jobs: # see https://github.com/shivammathur/setup-php - uses: shivammathur/setup-php@v2 with: - php-version: 7.1 + php-version: '7.1' coverage: none - run: composer update --no-progress --prefer-lowest - run: composer phpstan diff --git a/.gitignore b/.gitignore index 26fe71e..d0a4c20 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ /vendor/ +/.idea/ /.*.cache /tests-report-html/ /composer.lock diff --git a/.php-cs-fixer.php b/.php-cs-fixer.php index ffd0d20..e33341e 100644 --- a/.php-cs-fixer.php +++ b/.php-cs-fixer.php @@ -10,8 +10,12 @@ $config = new Config(); return $config->setRules([ + '@PSR12' => true, + '@PSR12:risky' => true, '@PhpCsFixer' => true, '@PhpCsFixer:risky' => true, + '@PHP71Migration' => true, + '@PHP71Migration:risky' => true, 'array_syntax' => ['syntax' => 'short'], 'php_unit_test_class_requires_covers' => false, 'backtick_to_shell_exec' => true, diff --git a/composer.json b/composer.json index 53bf31e..49d9d08 100644 --- a/composer.json +++ b/composer.json @@ -38,18 +38,23 @@ }, "autoload": { "psr-4": { - "MC\\": "lib/MC" + "MC\\": "lib/MC/" } }, "autoload-dev": { "psr-4": { - "Tests\\": "tests" + "Tests\\": "tests/" } }, "scripts": { - "phpstan": "@php phpstan analyse -l max lib tests examples .php-cs-fixer.php", + "phpstan": "@php phpstan analyse -l max -c phpstan.neon lib tests examples .php-cs-fixer.php", "php-cs-fixer": "@php php-cs-fixer fix --allow-risky=yes", "php-cs-fixer-dry-run": "@php php-cs-fixer fix --dry-run --allow-risky=yes", - "phpunit": "@php phpunit" + "phpunit": "@php phpunit", + "test": [ + "@php-cs-fixer-dry-run", + "@phpstan", + "@phpunit" + ] } } diff --git a/examples/callback_fields.php b/examples/callback_fields.php index 05aea08..4d89894 100644 --- a/examples/callback_fields.php +++ b/examples/callback_fields.php @@ -35,7 +35,7 @@ function most_common(array $row): ?string 'pill' => ['field' => 'pill', 'type' => 'number'], 'iud' => ['field' => 'iud', 'type' => 'number'], 'condom' => ['field' => 'condom', 'type' => 'number'], - 'sterile_total' => ['field' => 'steril_total', 'type' => 'number'], + 'sterile_total' => ['field' => 'sterile_total', 'type' => 'number'], 'other_modern' => ['field' => 'other_modern', 'type' => 'number'], 'traditional' => ['field' => 'traditional', 'type' => 'number'], 'most_common' => [ @@ -54,7 +54,7 @@ function most_common(array $row): ?string Callback fields visualization example - + diff --git a/lib/MC/Google/Visualization.php b/lib/MC/Google/Visualization.php index 02a139e..fa269d7 100644 --- a/lib/MC/Google/Visualization.php +++ b/lib/MC/Google/Visualization.php @@ -22,7 +22,32 @@ * * @see \Tests\VisualizationTest * - * @phpstan-type FieldSpec array{callback?:callable,field?:string,extra?:array,fields?:string[],sort_field?:string,type?:string,join?:string} + * @phpstan-type FieldSpec array{ + * type: string, + * field?: string, + * fields?: string[], + * callback?: callable, + * extra?: mixed[], + * sort_field?: string, + * join?: string + * } + * @phpstan-type QueryParsed array{ + * select?: array, + * from?: string, + * where?: null|array, + * groupby?: string[], + * pivot?: string[], + * orderby?: string[], + * limit?: string, + * offset?: string, + * label?: string + * } + * @phpstan-type Entity array{ + * table: string, + * fields: array, + * joins: string[], + * where?: string + * } */ class Visualization { @@ -37,7 +62,7 @@ class Visualization /** * The entity schema that defines which tables are exposed to visualization clients, along with their fields, joins, and callbacks. * - * @var array + * @var array */ protected $entities = []; @@ -60,7 +85,7 @@ class Visualization /** * If a format string is not provided by the query, these will be used to format values by default. * - * @var array + * @var array */ protected $defaultFormat = [ 'date' => 'm/d/Y', @@ -156,8 +181,8 @@ public function setDefaultFormat(string $type, string $format): void * Handle the entire request, pulling the query from the $_GET variables and printing the results directly * if not specified otherwise. * - * @param bool $echo print response and set header - * @param null|array $queryParams query parameters + * @param bool $echo print response and set header + * @param null|array{tq:string, tqx:string, responseHandler?:string} $queryParams query parameters * * @return string the javascript response * @@ -199,14 +224,14 @@ public function handleRequest(bool $echo = true, ?array $queryParams = null): st /** * Handle a specific query. Use this if you want to gather the query parameters yourself instead of using handleRequest(). * - * @param string $query the visualization query to parse and execute - * @param array $params all extra params sent along with the query - must include at least "reqId" key + * @param string $query the visualization query to parse and execute + * @param array $params all extra params sent along with the query - must include at least "reqId" key * * @return string the javascript response */ public function handleQuery(string $query, array $params): string { - $reqId = null; + $reqId = -1; $response = ''; try { @@ -214,7 +239,7 @@ public function handleQuery(string $query, array $params): string throw new Visualization_Error('You must pass a PDO connection to the MC Google Visualization Server if you want to let the server handle the entire request'); } - $reqId = $params['reqId']; + $reqId = (int) $params['reqId']; $queryParsed = $this->parseQuery($query); $meta = $this->generateMetadata($queryParsed); $sql = $this->generateSQL($meta); @@ -236,13 +261,13 @@ public function handleQuery(string $query, array $params): string $response .= $this->getSuccessClose(); } catch (Visualization_Error $visualizationError) { - $response .= $this->handleError($reqId, $visualizationError->getMessage(), $params['responseHandler'], $visualizationError->type, $visualizationError->summary); + $response .= $this->handleError($reqId, $visualizationError->getMessage(), (string) $params['responseHandler'], $visualizationError->type, $visualizationError->summary); } catch (PDOException $pdoException) { - $response .= $this->handleError($reqId, $pdoException->getMessage(), $params['responseHandler'], 'invalid_query', 'Invalid Query - PDO exception'); + $response .= $this->handleError($reqId, $pdoException->getMessage(), (string) $params['responseHandler'], 'invalid_query', 'Invalid Query - PDO exception'); } catch (ParseError $parseError) { - $response .= $this->handleError($reqId, $parseError->getMessage(), $params['responseHandler'], 'invalid_query', 'Invalid Query - Parse Error'); + $response .= $this->handleError($reqId, $parseError->getMessage(), (string) $params['responseHandler'], 'invalid_query', 'Invalid Query - Parse Error'); } catch (Exception $exception) { - $response .= $this->handleError($reqId, $exception->getMessage(), $params['responseHandler']); + $response .= $this->handleError($reqId, $exception->getMessage(), (string) $params['responseHandler']); } return $response; @@ -251,9 +276,11 @@ public function handleQuery(string $query, array $params): string /** * Given the results of parseQuery(), introspect against the entity definitions provided and return the metadata array used to generate the SQL. * - * @param array $query the visualization query broken up into sections + * @param array $query the visualization query broken up into sections * - * @return array the metadata array from merging the query with the entity table definitions + * @phpstan-param QueryParsed $query + * + * @return array the metadata array from merging the query with the entity table definitions * * @throws Visualization_QueryError * @throws Visualization_Error @@ -704,7 +731,9 @@ public function getGrammar(): Def * * @param string $str the query string to parse * - * @return array the parsed query as an array, keyed by each part of the query (select, from, where, groupby, pivot, orderby, limit, offset, label, format, options + * @return array the parsed query as an array, keyed by each part of the query (select, from, where, groupby, pivot, orderby, limit, offset, label, format, options + * + * @phpstan-return QueryParsed * * @throws ParseError * @throws Visualization_QueryError @@ -727,8 +756,10 @@ public function parseQuery(string $str): array break; case 'from': - $vals = $token->getValues(); - $query['from'] = $vals[1]; + $from = $token->getValues(); + $from = $from[1]; + assert(null !== $from); + $query['from'] = $from; break; @@ -744,7 +775,7 @@ public function parseQuery(string $str): array $groupby = $token->getValues(); array_shift($groupby); array_shift($groupby); - $query['groupby'] = $groupby; + $query['groupby'] = array_filter($groupby); break; @@ -754,7 +785,7 @@ public function parseQuery(string $str): array } $pivot = $token->getValues(); array_shift($pivot); - $query['pivot'] = $pivot; + $query['pivot'] = array_filter($pivot); break; @@ -784,6 +815,7 @@ public function parseQuery(string $str): array case 'limit': $limit = $token->getValues(); $limit = $limit[1]; + assert(null !== $limit); $query['limit'] = $limit; break; @@ -791,6 +823,7 @@ public function parseQuery(string $str): array case 'offset': $offset = $token->getValues(); $offset = $offset[1]; + assert(null !== $offset); $query['offset'] = $offset; break; @@ -803,6 +836,7 @@ public function parseQuery(string $str): array $count = count($labels); for ($i = 0; $i < $count; $i += 2) { $field = $labels[$i]; + assert(null !== $labels[$i + 1]); $label = trim($labels[$i + 1], '\'"'); $queryLabels[$field] = $label; } @@ -818,6 +852,7 @@ public function parseQuery(string $str): array $count = count($formats); for ($i = 0; $i < $count; $i += 2) { $field = $formats[$i]; + assert(null !== $formats[$i + 1]); $queryFormats[$field] = trim($formats[$i + 1], '\'"'); } $query['formats'] = $queryFormats; @@ -1290,15 +1325,18 @@ protected function getFieldSQL(string $name, array $spec, bool $alias = false, s /** * Recursively process the dependant fields for callback entity fields. * - * @param array $field the spec array for the field to add (must have a "callback" key) - * @param array $entity the spec array for the entity to pull other fields from - * @param array $meta the query metadata array to append the results + * @param array $fieldSpec the spec array for the field to add (must have a "callback" key) + * @param array $entity the spec array for the entity to pull other fields from + * @param array $meta the query metadata array to append the results + * + * @phpstan-param FieldSpec $fieldSpec + * @phpstan-param Entity $entity * * @throws Visualization_Error */ - protected function addDependantCallbackFields(array $field, array $entity, array &$meta): void + protected function addDependantCallbackFields(array $fieldSpec, array $entity, array &$meta): void { - foreach ($field['fields'] as $dependant) { + foreach ($fieldSpec['fields'] ?? [] as $dependant) { if (!isset($entity['fields'][$dependant])) { throw new Visualization_Error('Unknown callback required field "'.$dependant.'"'); } @@ -1335,6 +1373,7 @@ protected function parseFieldTokens(Token $token, array &$fields = null): void if ($token->hasChildren()) { if ('function' === $token->name) { $field = $token->getValues(); + assert(null !== $field[0]); $field[0] = strtolower($field[0]); $fields[] = $field; } else { @@ -1350,8 +1389,8 @@ protected function parseFieldTokens(Token $token, array &$fields = null): void /** * Helper method for the query parser to recursively scan and flatten the where clause's conditions. * - * @param Token $token the token or token group to parse - * @param null|array $where the collector array of tokens that make up the where clause + * @param Token $token the token or token group to parse + * @param null|array $where the collector array of tokens that make up the where clause */ protected function parseWhereTokens(Token $token, array &$where = null): void { diff --git a/lib/MC/Parser.php b/lib/MC/Parser.php index 0282d93..291ad9b 100644 --- a/lib/MC/Parser.php +++ b/lib/MC/Parser.php @@ -32,17 +32,17 @@ class Parser /** * Return a Set with the function arguments as the subexpressions. */ - public function set(): Set + public function set(Def ...$args): Set { - return new Set(func_get_args()); + return new Set($args); } /** * Return a OneOf with the function arguments as the possible expressions. */ - public function oneOf(): OneOf + public function oneOf(Def ...$args): OneOf { - return new OneOf(func_get_args()); + return new OneOf($args); } /** diff --git a/lib/MC/Parser/Def.php b/lib/MC/Parser/Def.php index cd9f1b4..fe94492 100644 --- a/lib/MC/Parser/Def.php +++ b/lib/MC/Parser/Def.php @@ -28,7 +28,7 @@ public function parse(string $str): Token $str = ltrim($str); [$loc, $tok] = $this->parsePart($str, 0); - if ($loc !== strlen($str)) { + if ((null === $tok) || ($loc !== strlen($str))) { throw new ParseError('An error occurred: "'.substr($str, $loc).'"', $str, $loc); } @@ -38,7 +38,7 @@ public function parse(string $str): Token /** * Parse a string, cleaning up whitespace when we're done. * - * @return array A two-item array of the string location where parsing stopped, and the MC_Token instance that matches the grammar conditions + * @return array{int, null|Token} A two-item array of the string location where parsing stopped and the MC_Token instance that matches the grammar conditions * * @throws ParseError */ @@ -61,7 +61,7 @@ public function parsePart(string $str, int $loc): array * @param string $str the string to parse * @param int $loc the index to start parsing * - * @return array A two-item array of the string location where parsing stopped, and the MC_Token instance that matches the grammar conditions + * @return array{int, null|Token} A two-item array of the string location where parsing stopped, and the Token instance that matches the grammar conditions * * @throws ParseError */ @@ -103,11 +103,9 @@ public function suppress(): self /** * Return a token instance, copying over this Def's name and flagging suppression. * - * @param mixed $value - * * @return null|Token the new token, or null if the token should be suppressed */ - public function token($value): ?Token + public function token(?string $value): ?Token { if ($this->suppress) { return null; diff --git a/lib/MC/Parser/Def/OneOf.php b/lib/MC/Parser/Def/OneOf.php index 164bbc0..4e2c11b 100644 --- a/lib/MC/Parser/Def/OneOf.php +++ b/lib/MC/Parser/Def/OneOf.php @@ -15,6 +15,9 @@ class OneOf extends Def /** @var Def[] */ public $exprs = []; + /** + * @param Def[] $exprs + */ public function __construct(array $exprs = []) { $this->exprs = $exprs; diff --git a/lib/MC/Parser/Def/Regex.php b/lib/MC/Parser/Def/Regex.php index 8d3ec34..8a66e4e 100644 --- a/lib/MC/Parser/Def/Regex.php +++ b/lib/MC/Parser/Def/Regex.php @@ -8,7 +8,7 @@ use MC\Parser\ParseError; /** - * Generic grammar rule for matching a regular expresion. + * Generic grammar rule for matching a regular expression. */ class Regex extends Def { diff --git a/lib/MC/Parser/Def/Set.php b/lib/MC/Parser/Def/Set.php index 9d8c840..6f1320f 100644 --- a/lib/MC/Parser/Def/Set.php +++ b/lib/MC/Parser/Def/Set.php @@ -11,11 +11,11 @@ */ class Set extends Def { - /** @var array */ + /** @var Def[] */ public $exprs = []; /** - * Set constructor. + * @param Def[] $exprs */ public function __construct(array $exprs = []) { @@ -31,7 +31,7 @@ public function _parse(string $str, int $loc): array $res = $this->tokenGroup(); foreach ($this->exprs as $expr) { [$loc, $toks] = $expr->parsePart($str, $loc); - if ((null !== $toks) && (!is_array($toks) || (count($toks) > 0))) { + if (null !== $toks) { $res->append($toks); } } diff --git a/lib/MC/Parser/Def/Word.php b/lib/MC/Parser/Def/Word.php index d8b0086..e8c04ac 100644 --- a/lib/MC/Parser/Def/Word.php +++ b/lib/MC/Parser/Def/Word.php @@ -11,7 +11,7 @@ class Word extends Regex { /** * @param string $firstChars the characters allowed as the first character in the word - * @param null|string $restChars the characters allwed as the rest of the word - defaults to same as $first_chars + * @param null|string $restChars the characters allowed as the rest of the word - defaults to same as $first_chars */ public function __construct(string $firstChars, string $restChars = null) { diff --git a/lib/MC/Parser/Token.php b/lib/MC/Parser/Token.php index a199a96..d0ea72b 100644 --- a/lib/MC/Parser/Token.php +++ b/lib/MC/Parser/Token.php @@ -12,25 +12,29 @@ class Token /** @var null|string */ public $name; - /** @var mixed */ + /** @var null|string */ public $value; /** * Token constructor. - * - * @param mixed $value */ - public function __construct($value, ?string $name) + public function __construct(?string $value, ?string $name) { $this->value = $value; $this->name = $name; } + /** + * @return array + */ public function getValues(): array { return [$this->value]; } + /** + * @return array + */ public function getNameValues(): array { return [[$this->name, $this->value]]; diff --git a/lib/MC/Parser/Token/Group.php b/lib/MC/Parser/Token/Group.php index 2aa0fb3..341936d 100644 --- a/lib/MC/Parser/Token/Group.php +++ b/lib/MC/Parser/Token/Group.php @@ -14,10 +14,7 @@ class Group extends Token implements Countable /** @var Token[] */ public $subtoks = []; - /** - * @param null|string $name - */ - public function __construct($name) + public function __construct(?string $name) { parent::__construct(null, $name); } @@ -43,6 +40,9 @@ public function count(): int return count($this->subtoks); } + /** + * @return array + */ public function getValues(): array { $values = []; @@ -53,6 +53,9 @@ public function getValues(): array return $values; } + /** + * @return array + */ public function getNameValues(): array { $values = []; diff --git a/tests/ExampleTest.php b/tests/ExampleTest.php index 1756644..d787d34 100644 --- a/tests/ExampleTest.php +++ b/tests/ExampleTest.php @@ -86,6 +86,30 @@ public function testQuerySimple(): void self::assertStringEqualsFile(__DIR__.'/result2.js', $output); } + /** + * @throws Visualization_Error + */ + public function testQueryError(): void + { + $db = new PDO('sqlite:'.__DIR__.'/../examples/example.db'); + $vis = new Visualization($db); + $vis->addEntity('countries', [ + 'fields' => [ + 'id' => ['field' => 'id', 'type' => 'number'], + 'name' => ['field' => 'name', 'type' => 'text'], + ], + ]); + + $parameters = [ + 'tq' => 'select id, name from', + 'tqx' => 'reqId:2', + ]; + + $output = $vis->handleRequest(false, $parameters); + + self::assertSame('google.visualization.Query.setResponse({version:"0.5",reqId:"2",status:"error",errors:[{reason:"invalid_query",message:"Invalid Query - Parse Error",detailed_message:"An error occurred: \"from\""}]});', $output); + } + /** * @throws Visualization_Error */ diff --git a/tests/VisualizationTest.php b/tests/VisualizationTest.php index a168214..493ba68 100644 --- a/tests/VisualizationTest.php +++ b/tests/VisualizationTest.php @@ -359,6 +359,9 @@ public function testIsNull(): void self::assertSame('SELECT id AS id FROM orders WHERE (product IS NOT NULL)', $sql); } + /** + * @param array $row + */ public static function callbackTest(array $row): string { return 'callback-'.$row['field'];