-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PopSQL: add psalm, gh actions, param/return types #16
Merged
Merged
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
3c33cda
composer update
davidrans ca53420
Add .gitignore
davidrans 5d277ba
Replace Travis config with GH Actions
davidrans ac2384c
Fix expected/actual backwards in tests
davidrans fa18eaa
Drop support for deprecated PHP versions
davidrans c54740c
Install psalm and set up initial config / baseline
davidrans 96bdb52
Add property types
davidrans fe9d102
Add psalm action
davidrans 8e4ec5b
Do not use matrix of one php version
davidrans 8e9b3ba
Use psalm error level 2 to require types
davidrans 74967c0
Remove return by reference from methods that return self
davidrans cfaff59
Add property/param types
davidrans c03f92f
Add param/return types to test
davidrans b85bf8b
Style autofix
davidrans baae44b
Use variable name instead of comment
davidrans 3957efd
simplify return types
davidrans File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
name: psalm | ||
|
||
on: | ||
push: | ||
branches: | ||
- '**' | ||
pull_request: | ||
branches: | ||
- '**' | ||
|
||
jobs: | ||
psalm: | ||
name: psalm | ||
runs-on: ubuntu-latest | ||
|
||
timeout-minutes: 15 | ||
steps: | ||
- name: Checkout code | ||
uses: actions/checkout@v4 | ||
|
||
- name: Setup PHP | ||
uses: shivammathur/setup-php@v2 | ||
with: | ||
php-version: '8.3' | ||
|
||
- name: Run composer install | ||
run: composer install | ||
|
||
- name: Run psalm | ||
run: ./vendor/bin/psalm --output-format=github --update-baseline |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
name: phpunit | ||
|
||
on: | ||
push: | ||
branches: | ||
- '**' | ||
pull_request: | ||
branches: | ||
- '**' | ||
|
||
jobs: | ||
test: | ||
runs-on: ubuntu-latest | ||
|
||
steps: | ||
- name: Checkout code | ||
uses: actions/checkout@v4 | ||
|
||
- name: Setup PHP | ||
uses: shivammathur/setup-php@v2 | ||
with: | ||
php-version: '8.3' | ||
|
||
- name: Install dependencies | ||
run: composer install | ||
|
||
- name: Run tests | ||
run: vendor/bin/phpunit QueryGeneratorTest.php |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
vendor/ |
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,8 +39,16 @@ class QueryGenerator { | |
* The keys of this array are the set of clauses that can compose different | ||
* statements. These correspond to methods that can be called on this class. | ||
* The values are the syntax rules for collapsing the corresponding clauses. | ||
* | ||
* @var non-empty-array<string, array{ | ||
* clause: string, | ||
* prefix: string, | ||
* glue: string|false, | ||
* suffix: string, | ||
* requiresArgument?: bool | ||
* }> | ||
*/ | ||
private static $methods = [ | ||
private static array $methods = [ | ||
'select' => [ | ||
'clause' => 'SELECT <<MODIFIERS>> ', | ||
'prefix' => '', | ||
|
@@ -183,8 +191,10 @@ class QueryGenerator { | |
* The keys of this array are the primary clauses that can be present in a | ||
* MySQL query. Each primary clause has a set of valid sub-clauses that can | ||
* be present in a completed query of that type. | ||
* | ||
* @var non-empty-array<string, list<string>> | ||
*/ | ||
private static $possibleClauses = [ | ||
private static array $possibleClauses = [ | ||
'select' => ['from', 'join', 'where', 'group', 'having', 'order', 'limit', 'offset', 'forupdate'], | ||
'insert' => ['set', 'columns', 'values', 'duplicate', 'as'], | ||
'replace' => ['set', 'columns', 'values'], | ||
|
@@ -198,8 +208,10 @@ class QueryGenerator { | |
* this array correspond to the minimum required set of sub-clauses needed | ||
* in each of these grammar sub-trees. A query will be considered complete | ||
* if it has all the sub-clauses listed in any of these sets. | ||
* | ||
* @var array<string, list<list<string>>> | ||
*/ | ||
private static $minimumClauses = [ | ||
private static array $minimumClauses = [ | ||
'select' => [['from']], | ||
'insert' => [['set'], ['columns', 'values']], | ||
'replace' => [['set'], ['columns', 'values']], | ||
|
@@ -210,66 +222,63 @@ class QueryGenerator { | |
/** | ||
* Each query type can specify a certain selection of modifiers. They each | ||
* change some aspect of how the query runs. | ||
* | ||
* @var array<string, list<string>> | ||
*/ | ||
private static $queryModifiers = [ | ||
private static array $queryModifiers = [ | ||
'select' => [ | ||
'ALL', 'DISTINCT', 'DISTINCTROW', | ||
'HIGH_PRIORITY', | ||
'STRAIGHT_JOIN', | ||
'SQL_SMALL_RESULT', 'SQL_BIG_RESULT', 'SQL_BUFFER_RESULT', | ||
'SQL_CACHE', 'SQL_NO_CACHE', | ||
'SQL_CALC_FOUND_ROWS' | ||
'SQL_CALC_FOUND_ROWS', | ||
], | ||
'insert' => ['LOW_PRIORITY', 'DELAYED', 'HIGH_PRIORITY', 'IGNORE'], | ||
'replace' => ['LOW_PRIORITY', 'DELAYED'], | ||
'update' => ['LOW_PRIORITY', 'IGNORE'], | ||
'delete' => ['LOW_PRIORITY', 'QUICK', 'IGNORE'], | ||
]; | ||
|
||
private $clauses; | ||
private $params; | ||
private $validateQuery; | ||
private $useOr; | ||
|
||
public function __construct() { | ||
$this->clauses = []; | ||
$this->params = []; | ||
|
||
public function __construct( | ||
private array $clauses = [], | ||
private array $params = [], | ||
private bool $validateQuery = true, | ||
private bool $useOr = false, | ||
) { | ||
foreach (array_keys(self::$methods) as $method) { | ||
$this->clauses[$method] = []; | ||
$this->params[$method] = []; | ||
} | ||
|
||
$this->validateQuery = true; | ||
$this->useOr = false; | ||
} | ||
|
||
/** | ||
* Append the given clause components and parameters to their existing | ||
* counterparts for the specified clause. | ||
*/ | ||
public function &__call($method, $args) { | ||
public function __call(string $method, array $args) { | ||
$method = strtolower($method); | ||
|
||
if (!isset(self::$methods[$method])) { | ||
throw new Exception("Method \"$method\" does not exist."); | ||
} | ||
|
||
$requiresArgument = (isset(self::$methods[$method]['requiresArgument']) ? | ||
self::$methods[$method]['requiresArgument'] : false); | ||
$requiresArgument = (isset(self::$methods[$method]['requiresArgument']) | ||
? self::$methods[$method]['requiresArgument'] | ||
: false); | ||
|
||
if ($requiresArgument && count($args) < 1) { | ||
throw new Exception("Missing argument 1 (\$clauses) for $method()"); | ||
} else if (count($args) < 2) { | ||
$clauses = reset($args); | ||
$params = []; | ||
} else { | ||
list($clauses, $params) = $args; | ||
[$clauses, $params] = $args; | ||
} | ||
|
||
if ($clauses instanceOf QueryGenerator) { | ||
if ($clauses instanceof self) { | ||
$clauses->skipValidation(); | ||
list($clauses, $params) = $clauses->build(/* $skipClauses = */ true); | ||
[$clauses, $params] = $clauses->build(skipClauses: true); | ||
} | ||
|
||
if (!is_array($clauses)) { | ||
|
@@ -298,13 +307,15 @@ public function &__call($method, $args) { | |
* (one of MissingPrimaryClauseException or MissingRequiredClauseException) | ||
* unless `skipValidation` has been called. | ||
* | ||
* @param $skipClauses : Exclude the 'clause' part (WHERE, SELECT, FROM, | ||
* @param bool $skipClauses : Exclude the 'clause' part (WHERE, SELECT, FROM, | ||
* ...) of each sub-expression. See constructClause | ||
* for more info. This is mostly for internal usage. | ||
* | ||
* Returns an array containing the query and paramter list, respectively. | ||
* | ||
* @return array{0: string, 1: array<string, mixed>} | ||
*/ | ||
public function build($skipClauses = false) { | ||
public function build(bool $skipClauses = false): array { | ||
if ($this->validateQuery) { | ||
$this->assertCompleteQuery(); | ||
} | ||
|
@@ -333,30 +344,34 @@ public function build($skipClauses = false) { | |
/** | ||
* Bypass query validation when building. | ||
*/ | ||
public function &skipValidation() { | ||
public function skipValidation(): self { | ||
$this->validateQuery = false; | ||
return $this; | ||
} | ||
|
||
/** | ||
* Use OR when joining where conditions | ||
*/ | ||
public function &useOr() { | ||
public function useOr(): self { | ||
$this->useOr = true; | ||
return $this; | ||
} | ||
|
||
/** | ||
* Assert the completeness of this QueryGenerator instance by verifying | ||
* that all required clauses have been set. | ||
* | ||
* @throws MissingPrimaryClauseException | ||
* @throws MissingRequiredClauseException | ||
*/ | ||
private function assertCompleteQuery() { | ||
private function assertCompleteQuery(): void { | ||
$primaryMethod = $this->getPrimaryMethod(); | ||
|
||
if (!$primaryMethod) { | ||
$primaryClauseStr = implode("', '", $this->getPrimaryClauses()); | ||
throw new MissingPrimaryClauseException( | ||
"Missing primary clause. One of '$primaryClauseStr' needed."); | ||
"Missing primary clause. One of '$primaryClauseStr' needed." | ||
); | ||
} | ||
|
||
$minimumClauses = self::$minimumClauses[$primaryMethod]; | ||
|
@@ -380,29 +395,37 @@ private function assertCompleteQuery() { | |
}, $minimumClauses); | ||
$requiredClauseStr = '{' . implode('}, {', $requiredClauseOptions) . '}'; | ||
throw new MissingRequiredClauseException( | ||
"Missing required clauses. One of $requiredClauseStr needed."); | ||
"Missing required clauses. One of $requiredClauseStr needed." | ||
); | ||
} | ||
|
||
/** | ||
* Return the list of primary query clauses. | ||
* | ||
* @return non-empty-list<string> | ||
*/ | ||
private static function getPrimaryClauses() { | ||
private static function getPrimaryClauses(): array { | ||
return array_keys(self::$possibleClauses); | ||
} | ||
|
||
/** | ||
* Return the primary clause in this QueryGenerator instance. | ||
* If multiple primary clauses have been set, all but the first set clause | ||
* will be ignored. | ||
* | ||
* @return string|false | ||
*/ | ||
private function getPrimaryMethod() { | ||
private function getPrimaryMethod(): string|bool { | ||
$primaryClauses = self::getPrimaryClauses(); | ||
$setMethods = $this->getSetMethods(); | ||
$setPrimaryClauses = array_intersect($primaryClauses, $setMethods); | ||
return reset($setPrimaryClauses); | ||
} | ||
|
||
private function getSetMethods() { | ||
/** | ||
* @return array<string, string> | ||
*/ | ||
private function getSetMethods(): array { | ||
$methods = array_keys(array_filter($this->clauses)); | ||
return array_combine($methods, $methods); | ||
} | ||
|
@@ -416,7 +439,7 @@ private function getSetMethods() { | |
* constructClause('where') => 'WHERE (foo = ?) AND (bar != ?)' | ||
* constructClause('where', false) => '(foo = ?) AND (bar != ?)' | ||
*/ | ||
private function constructClause($method, $skipClause = false) { | ||
private function constructClause(string $method, bool $skipClause = false): string { | ||
$clauseInfo = self::$methods[$method]; | ||
$prefix = $clauseInfo['prefix']; | ||
$clause = $clauseInfo['clause']; | ||
|
@@ -441,8 +464,10 @@ private function constructClause($method, $skipClause = false) { | |
/** | ||
* return the appropriate glue string for the given clause, taking into | ||
* account $this->useOr | ||
* | ||
* @return string|false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. |
||
*/ | ||
private function getGlue($method) { | ||
private function getGlue(string $method): string|bool { | ||
if ($method !== 'where' || !$this->useOr) { | ||
return self::$methods[$method]['glue']; | ||
} else { | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't necessary.
false
is a literal type now (as of 8.0):https://php.watch/versions/8.0/union-types#false