Skip to content

Add Exec and Query methods to FakePDO #6

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

Merged
merged 6 commits into from
Feb 3, 2021
Merged

Conversation

M1ke
Copy link
Contributor

@M1ke M1ke commented Feb 3, 2021

Adds other public methods (rather than just prepare() to FakePdo)

The current master branch state of Psalm surfaces the following fails:

ERROR: ArgumentTypeCoercion - src/Parser/SQLLexer.php:24:13 - Argument 4 of preg_split expects int(0)|int(1)|int(2)|int(3)|int(4)|int(5)|int(6)|int(7), parent type positive-int provided (see https://psalm.dev/193)
            \PREG_SPLIT_DELIM_CAPTURE | \PREG_SPLIT_NO_EMPTY | \PREG_SPLIT_OFFSET_CAPTURE


ERROR: InvalidArgument - src/Parser/SQLLexer.php:32:44 - Argument 1 of Vimeo\MysqlEngine\Parser\SQLLexer::groupComments expects list<array{string, int}>, list<array{string, int}|string> provided (see https://psalm.dev/004)
            $tokens = $this->groupComments($tokens);


ERROR: InvalidArgument - src/Parser/SQLLexer.php:35:51 - Argument 1 of Vimeo\MysqlEngine\Parser\SQLLexer::groupEscapeSequences expects list<array{string, int}>, list<array{string, int}|string> provided (see https://psalm.dev/004)
            $tokens = $this->groupEscapeSequences($tokens);


ERROR: InvalidArgument - src/Parser/SQLLexer.php:37:41 - Argument 1 of Vimeo\MysqlEngine\Parser\SQLLexer::groupQuotedTokens expects list<array{string, int}>, list<array{string, int}|string> provided (see https://psalm.dev/004)
        return $this->groupQuotedTokens($tokens);

This branch does not resolve these, but also does not add new fails.

src/FakePdo.php Outdated
}

$sth = $this->prepare($statement);
if ($sth->execute()){
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is the correct way to replicate the internal logic

src/FakePdo.php Outdated
{
$sth = $this->prepare($statement);
$sth->execute();
return $sth;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not certain if anything new is needed here to replicate internal logic but this does run properly

@M1ke
Copy link
Contributor Author

M1ke commented Feb 3, 2021

So the fail on the build is coming from the query declaration where I've declared:

query($statement, $mode = PDO::ATTR_DEFAULT_FETCH_MODE, $arg3 = null, array $ctorargs = [])

Whereas the tests run on PHP 8 which requires this to be typed as

query(string $statement, ?int $mode = null, mixed ...$fetchModeArgs)

But if we do that it will break on anything <PHP8. I wonder if this requires the tests to be run on the min supported version (PHP 7.1 according to composer.json)?

@muglug
Copy link
Collaborator

muglug commented Feb 3, 2021

Thanks!

Please merge/rebase master! I've introduced separate FakePdo classes for PHP 7 & 8, allowing you to use the different APIs without breaking everything.

@M1ke
Copy link
Contributor Author

M1ke commented Feb 3, 2021

So the last Psalm build on actions failed due to a Docker error... to make it build again I pushed some new .gitignore lines - they are relevant but obviously not directly to this PR so I can remove them if needed.

But on the plus side, all checks now pass with the split out PHP7/8 classes!

I'll update the Codeception branch at Codeception/module-db#20 to use a specific class too.

@muglug muglug merged commit 5d7c584 into vimeo:master Feb 3, 2021
@muglug
Copy link
Collaborator

muglug commented Feb 3, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants