[Process] Allow to pass null arguments to Process #28863
Conversation
|
Should target 3.4 is suppose. Could you please add a test case also? |
|
@nicolas-grekas , typehints were added in 4.0 with php 7.0. In 3.4 everything should work fine (see https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/Process/Process.php#L1714). |
|
OK, so maybe only the test case should target 3.4 then? |
|
Rebased onto 4.1, since 4.0 is for security fixes only. Ok, I will add another PR with a test case for 3.4 soon. |
|
@nicolas-grekas , a test case is ready in #28864 |
…nts (vudaltsov) This PR was merged into the 3.4 branch. Discussion ---------- [Process] A test case for stringifying of Process arguments | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | for #28863 | License | MIT | Doc PR | Checks that non-string arguments passed to Process constructor are typecasted to string Commits ------- 0d54342 Add a test case for stringifying of Process arguments
|
Ready! |
|
@nicolas-grekas , addressed your comment |
efd1235 to
acf8b83
Compare
|
Thank you @vudaltsov. |
…udaltsov) This PR was merged into the 4.1 branch. Discussion ---------- [Process] Allow to pass non-string arguments to Process | Q | A | ------------- | --- | Branch? | 4.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28609 | License | MIT | Doc PR | Sometimes you might pass integers, floats, nulls as command arguments to the Process constructor. Before 4.0 and #24722 that worked fine. Now it throws because of an invalid type. I think we can drop the type hint here safely and stringify the value implicitly in the method. That will be a little more convenient. Commits ------- acf8b83 Remove Process::escapeArgument argument type hint
|
Thank you @vudaltsov! :-) |
|
@nicolas-grekas , the point of my PR was to allow to pass ints and floats as well... Why did you remove that? |
|
I have two arguments:
|
|
I didn't remove that: this is PHP language, where types are cast automatically for you when needed. |
|
Am I right that this is because we don't have |
|
That's correct. |
Sometimes you might pass integers, floats, nulls as command arguments to the Process constructor.
Before 4.0 and #24722 that worked fine. Now it throws because of an invalid type.
I think we can drop the type hint here safely and stringify the value implicitly in the method. That will be a little more convenient.