Resolves issue #4 #5
Conversation
There was a problem hiding this comment.
@sebastianfeldmann, I add some comments to mark these changes are about PHP_CodeSniifer or PHPStan suggested.
Hopefully it can help you to review and change these code snippets conveniently :).
| @@ -0,0 +1,3 @@ | |||
| <?php | |||
|
|
|||
| define('SF_CLI_PATH_FILES', realpath(__DIR__ . '/files')); | |||
There was a problem hiding this comment.
Let these constants split from tests/bootstrap.php file and this can fix following warning message:
1 | WARNING | [ ] A file should declare new symbols (classes,
| | functions, constants, etc.) and cause no other
| | side effects, or it should execute logic with
| | side effects, but should not do both. The first
| | symbol is defined on line 10 and the first side
| | effect is on line 13.
|
|
||
| require __DIR__ . '/../vendor/autoload.php'; | ||
|
|
||
| require __DIR__ . '/constants.php'; |
There was a problem hiding this comment.
These changes are for fix following warning message via PHP_CodeSniffer tool:
----------------------------------------------------------------------
FOUND 3 ERRORS AND 1 WARNING AFFECTING 3 LINES
----------------------------------------------------------------------
1 | WARNING | [ ] A file should declare new symbols (classes,
| | functions, constants, etc.) and cause no other
| | side effects, or it should execute logic with
| | side effects, but should not do both. The first
| | symbol is defined on line 10 and the first side
| | effect is on line 13.
1 | ERROR | [x] Header blocks must be separated by a single blank
| | line
9 | ERROR | [x] Header blocks must be separated by a single blank
| | line
13 | ERROR | [x] Expected 1 blank line at end of file; 2 found
| @@ -144,10 +146,10 @@ public function addOptionIfNotEmpty(string $option, $check, bool $asValue = true | |||
| /** | |||
| * Add argument to list. | |||
| * | |||
| * @param mixed <string|array> $argument | |||
There was a problem hiding this comment.
This change is for following message via PHPStan tool:
------ -----------------------------------------------------------------------
Line Command/Executable.php
------ -----------------------------------------------------------------------
152 PHPDoc tag @param for parameter $argument contains unresolvable type.
164 PHPDoc tag @param for parameter $argument contains unresolvable type.
------ -----------------------------------------------------------------------
| @@ -156,10 +158,10 @@ public function addArgument($argument) : Executable | |||
| /** | |||
| * Escape a shell argument. | |||
| * | |||
| * @param mixed <string|array> $argument | |||
There was a problem hiding this comment.
This change is for following message via PHPStan tool:
------ -----------------------------------------------------------------------
Line Command/Executable.php
------ -----------------------------------------------------------------------
152 PHPDoc tag @param for parameter $argument contains unresolvable type.
164 PHPDoc tag @param for parameter $argument contains unresolvable type.
------ -----------------------------------------------------------------------
| */ | ||
| public function getCode() : string | ||
| public function getCode(): int |
There was a problem hiding this comment.
This change is for following error via PHPStan tool:
------ ----------------------------------------------------------------------
Line Command/Runner/Result.php
------ ----------------------------------------------------------------------
79 Method SebastianFeldmann\Cli\Command\Runner\Result::getCode() should
return string but returns int.
------ ----------------------------------------------------------------------
| { | ||
| return $this->isPiped() ? ' | ' . implode(' | ', $this->pipeline) : ''; | ||
| } | ||
|
|
||
| /** | ||
| * Adds a cli command to list of commands to execute | ||
| * | ||
| * @param \SebastianFeldmann\Cli\Command | ||
| * @param \SebastianFeldmann\Cli\Command $cmd |
There was a problem hiding this comment.
This change is for following error via PHPStan tool:
Line CommandLine.php
------ -----------------------------------------------------------------------
173 PHPDoc tag @param has invalid value (\SebastianFeldmann\Cli\Command):
Unexpected token "\n *", expected variable at offset 113
------ -----------------------------------------------------------------------
| { | ||
| $process = method_exists('Symfony\Component\Process\Process', 'fromShellCommandline') | ||
| ? Process::fromShellCommandline($cmd) | ||
| : new Process($cmd); | ||
| : new Process([$cmd]); |
There was a problem hiding this comment.
This change is for following error via PHPStan tool:
------ ------------------------------------------------------------------
Line Processor/Symfony.php
------ ------------------------------------------------------------------
40 Parameter #1 $command of class Symfony\Component\Process\Process
constructor expects array, string given.
------ ------------------------------------------------------------------
There was a problem hiding this comment.
@sebastianfeldmann, this change is effected by the PHPStan tool.
To revert this issue, maybe we can tell PHPStan tool to ignore this parameter type check?
There was a problem hiding this comment.
@markvds, thanks for your reply and report, this change is effected by PHPstan tool, you should track this comment :).
There was a problem hiding this comment.
I put the error on the PHPStan ignore list for now. Maybe we can come up with something more elegant.
PHPStan supports an annotation @ignoreNextLine maybe we can use that one instead. I will investigate and see what I can come up with ;)
There was a problem hiding this comment.
Personally, I would annotate the code and put a comment to explain why it's okay to call the constructor with a string as first argument. It prevents the same mistake in the future; IDEs can also complain about the type mismatch and someone might someday think it's a good idea to correct the type mismatch :)
Changed log
PHP_CodeSniffertool.PHPStantool.