Closed yespire closed 3 years ago
You need to execute
composer cs-fix
command is showing error:
> /usr/bin/env php -d swoole.enable_library=Off ./vendor/bin/php-cs-fixer fix
Could not open input file: ./vendor/bin/php-cs-fixer
Script /usr/bin/env php -d swoole.enable_library=Off ./vendor/bin/php-cs-fixer fix handling the cs-fix event returned with error code 1
maybe you can hint me styling in which line(s) are not conforming, I can do a manually update
You need to execute
composer cs-fix
Here is my guess:
public function setFetchMode(int $mode, $colno_class_object = null, array $ctorarfg = [])
probably because 2nd parameter can not be type hinted, if that's case, maybe we have to setup 3+ setup methods to cover the underlying PDOStatement->setFetchMode()
PDOStatementProxy
How about ...$args
? If it works, code can be simplify and we just lose the paramaters tip (we can also add comment on this function).
How about
...$args
? If it works, code can be simplify and we just lose the paramaters tip (we can also add comment on this function).
refactored
Code check is complaining about N-path / Cyclomatic complexity level
i see the if condition / logic branching to be necessary for the method.
However we can refactor the if-block into a switch or a function mapping, but it is a bit subjective if this is more readable than if-block.
func_num_args()
instead of count($args)
to detect number of args.
- Add ignore files to your own user ignore list is better, or do this in another PR.
I will move the ignore change to a separate PR
- I am so sorry that I recongnized that my idea is bad for PHP-8.x, it will break support for named paramaters feature... could we back to previous implementation and we can use
func_num_args()
instead ofcount($args)
to detect number of args.
sure
@deminy Nice!
The function signature in PHP8 is
/** @return bool */
public function setFetchMode(int $mode, mixed ...$args) {}
See: https://github.com/php/php-src/blob/master/ext/pdo/pdo_stmt.stub.php#L64
The function signature in PHP8 is
/** @return bool */ public function setFetchMode(int $mode, mixed ...$args) {}
See: https://github.com/php/php-src/blob/master/ext/pdo/pdo_stmt.stub.php#L64
Good catch however if we add mixed type hinting, it will break in 7.X, mixed is added in php8 - https://wiki.php.net/rfc/mixed_type_v2
is "mixed" typing hinting a hard requirement ?
public function setFetchMode(int $mode, ...$args) : bool {
$this->setFetchModeContext = [$mode, ...$args];
return $this->__object->setFetchMode(...$this->setFetchModeContext);
}
public function setFetchMode(int $mode, ...$args) : bool { $this->setFetchModeContext = [$mode, ...$args]; return $this->__object->setFetchMode(...$this->setFetchModeContext); }
It's ok to drop mixed
. In my understanding, mixed
means we did not forget to declare type of param, the param accept all kinds of values indeed.
I agree with @twose . This is better than what I suggested:
public function setFetchMode(int $mode, ...$args) : bool;
The PR has been merged. @yespire thanks for your contribution!
about this PR
PHP Fatal error: Uncaught ArgumentCountError: PDOStatement::setFetchMode() expects exactly 2 arguments for the fetch mode provided, 3 given in @swoole-src/library/core/Database/PDOStatementProxy.php:132
for anyone encounters the same error message, alternative: