Skip to content

Commit 787d60a

Browse files
ycerutoyceruto
committed
Deprecate returning a non-integer value from a \Closure function set via Command::setCode() 
1 parent 82cf90a commit 787d60a

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+258
-80
lines changed

‎UPGRADE-7.3.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ AssetMapper
1616
Console
1717
-------
1818

19-
* Omitting parameter types in callables configured via `Command::setCode()` method is deprecated
19+
* Omitting parameter types or returning a non-integer value from a `\Closure` set via `Command::setCode()` method is deprecated
2020

2121
Before:
2222

@@ -32,8 +32,10 @@ Console
3232
use Symfony\Component\Console\Input\InputInterface;
3333
use Symfony\Component\Console\Output\OutputInterface;
3434

35-
$command->setCode(function (InputInterface $input, OutputInterface $output) {
35+
$command->setCode(function (InputInterface $input, OutputInterface $output): int {
3636
// ...
37+
38+
return 0;
3739
});
3840
```
3941

‎src/Symfony/Bundle/FrameworkBundle/Tests/Console/ApplicationTest.php‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,11 @@ public function testRunOnlyWarnsOnUnregistrableCommand()
135135
$kernel
136136
->method('getBundles')
137137
->willReturn([$this->createBundleMock(
138-
[(new Command('fine'))->setCode(function (InputInterface $input, OutputInterface $output) { $output->write('fine'); })]
138+
[(new Command('fine'))->setCode(function (InputInterface $input, OutputInterface $output): int {
139+
$output->write('fine');
140+
141+
return 0;
142+
})]
139143
)]);
140144
$kernel
141145
->method('getContainer')
@@ -163,7 +167,11 @@ public function testRegistrationErrorsAreDisplayedOnCommandNotFound()
163167
$kernel
164168
->method('getBundles')
165169
->willReturn([$this->createBundleMock(
166-
[(new Command(null))->setCode(function (InputInterface $input, OutputInterface $output) { $output->write('fine'); })]
170+
[(new Command(null))->setCode(function (InputInterface $input, OutputInterface $output): int {
171+
$output->write('fine');
172+
173+
return 0;
174+
})]
167175
)]);
168176
$kernel
169177
->method('getContainer')
@@ -193,7 +201,11 @@ public function testRunOnlyWarnsOnUnregistrableCommandAtTheEnd()
193201
$kernel
194202
->method('getBundles')
195203
->willReturn([$this->createBundleMock(
196-
[(new Command('fine'))->setCode(function (InputInterface $input, OutputInterface $output) { $output->write('fine'); })]
204+
[(new Command('fine'))->setCode(function (InputInterface $input, OutputInterface $output): int {
205+
$output->write('fine');
206+
207+
return 0;
208+
})]
197209
)]);
198210
$kernel
199211
->method('getContainer')

‎src/Symfony/Component/Console/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ CHANGELOG
1212
* Deprecate methods `Command::getDefaultName()` and `Command::getDefaultDescription()` in favor of the `#[AsCommand]` attribute
1313
* Add support for Markdown format in `Table`
1414
* Add support for `LockableTrait` in invokable commands
15+
* Deprecate returning a non-integer value from a `\Closure` function set via `Command::setCode()`
1516

1617
7.2
1718
---

‎src/Symfony/Component/Console/Command/Command.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ public function complete(CompletionInput $input, CompletionSuggestions $suggesti
347347
*/
348348
public function setCode(callable $code): static
349349
{
350-
$this->code = new InvokableCommand($this, $code, triggerDeprecations: true);
350+
$this->code = new InvokableCommand($this, $code);
351351

352352
return $this;
353353
}

‎src/Symfony/Component/Console/Command/InvokableCommand.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ class InvokableCommand
3232
{
3333
private readonly \Closure $code;
3434
private readonly \ReflectionFunction $reflection;
35+
private bool $triggerDeprecations = false;
3536

3637
public function __construct(
3738
private readonly Command $command,
3839
callable $code,
39-
private readonly bool $triggerDeprecations = false,
4040
) {
4141
$this->code = $this->getClosure($code);
4242
$this->reflection = new \ReflectionFunction($this->code);
@@ -49,17 +49,17 @@ public function __invoke(InputInterface $input, OutputInterface $output): int
4949
{
5050
$statusCode = ($this->code)(...$this->getParameters($input, $output));
5151

52-
if (null !== $statusCode && !\is_int($statusCode)) {
52+
if (!\is_int($statusCode)) {
5353
if ($this->triggerDeprecations) {
5454
trigger_deprecation('symfony/console', '7.3', \sprintf('Returning a non-integer value from the command "%s" is deprecated and will throw an exception in Symfony 8.0.', $this->command->getName()));
5555

5656
return 0;
5757
}
5858

59-
throw new LogicException(\sprintf('The command "%s" must return either void or an integer value in the "%s" method, but "%s" was returned.', $this->command->getName(), $this->reflection->getName(), get_debug_type($statusCode)));
59+
throw new \TypeError(\sprintf('The command "%s" must return an integer value in the "%s" method, but "%s" was returned.', $this->command->getName(), $this->reflection->getName(), get_debug_type($statusCode)));
6060
}
6161

62-
return $statusCode ?? 0;
62+
return $statusCode;
6363
}
6464

6565
/**
@@ -85,6 +85,8 @@ private function getClosure(callable $code): \Closure
8585
return $code(...);
8686
}
8787

88+
$this->triggerDeprecations = true;
89+
8890
if (null !== (new \ReflectionFunction($code))->getClosureThis()) {
8991
return $code;
9092
}

‎src/Symfony/Component/Console/Tests/ApplicationTest.php‎

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,10 @@ public function testRegister()
196196

197197
public function testRegisterAmbiguous()
198198
{
199-
$code = function (InputInterface $input, OutputInterface $output) {
199+
$code = function (InputInterface $input, OutputInterface $output): int {
200200
$output->writeln('It works!');
201+
202+
return 0;
201203
};
202204

203205
$application = new Application();
@@ -1275,7 +1277,9 @@ public function testAddingOptionWithDuplicateShortcut()
12751277
->register('foo')
12761278
->setAliases(['f'])
12771279
->setDefinition([new InputOption('survey', 'e', InputOption::VALUE_REQUIRED, 'My option with a shortcut.')])
1278-
->setCode(function (InputInterface $input, OutputInterface $output) {})
1280+
->setCode(function (InputInterface $input, OutputInterface $output): int {
1281+
return 0;
1282+
})
12791283
;
12801284

12811285
$input = new ArrayInput(['command' => 'foo']);
@@ -1298,7 +1302,9 @@ public function testAddingAlreadySetDefinitionElementData($def)
12981302
$application
12991303
->register('foo')
13001304
->setDefinition([$def])
1301-
->setCode(function (InputInterface $input, OutputInterface $output) {})
1305+
->setCode(function (InputInterface $input, OutputInterface $output): int {
1306+
return 0;
1307+
})
13021308
;
13031309

13041310
$input = new ArrayInput(['command' => 'foo']);
@@ -1435,8 +1441,10 @@ public function testRunWithDispatcher()
14351441
$application->setAutoExit(false);
14361442
$application->setDispatcher($this->getDispatcher());
14371443

1438-
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output) {
1444+
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output): int {
14391445
$output->write('foo.');
1446+
1447+
return 0;
14401448
});
14411449

14421450
$tester = new ApplicationTester($application);
@@ -1491,8 +1499,10 @@ public function testRunDispatchesAllEventsWithExceptionInListener()
14911499
$application->setDispatcher($dispatcher);
14921500
$application->setAutoExit(false);
14931501

1494-
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output) {
1502+
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output): int {
14951503
$output->write('foo.');
1504+
1505+
return 0;
14961506
});
14971507

14981508
$tester = new ApplicationTester($application);
@@ -1559,8 +1569,10 @@ public function testRunAllowsErrorListenersToSilenceTheException()
15591569
$application->setDispatcher($dispatcher);
15601570
$application->setAutoExit(false);
15611571

1562-
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output) {
1572+
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output): int {
15631573
$output->write('foo.');
1574+
1575+
return 0;
15641576
});
15651577

15661578
$tester = new ApplicationTester($application);
@@ -1671,8 +1683,10 @@ public function testRunWithDispatcherSkippingCommand()
16711683
$application->setDispatcher($this->getDispatcher(true));
16721684
$application->setAutoExit(false);
16731685

1674-
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output) {
1686+
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output): int {
16751687
$output->write('foo.');
1688+
1689+
return 0;
16761690
});
16771691

16781692
$tester = new ApplicationTester($application);
@@ -1698,8 +1712,10 @@ public function testRunWithDispatcherAccessingInputOptions()
16981712
$application->setDispatcher($dispatcher);
16991713
$application->setAutoExit(false);
17001714

1701-
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output) {
1715+
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output): int {
17021716
$output->write('foo.');
1717+
1718+
return 0;
17031719
});
17041720

17051721
$tester = new ApplicationTester($application);
@@ -1728,8 +1744,10 @@ public function testRunWithDispatcherAddingInputOptions()
17281744
$application->setDispatcher($dispatcher);
17291745
$application->setAutoExit(false);
17301746

1731-
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output) {
1747+
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output): int {
17321748
$output->write('foo.');
1749+
1750+
return 0;
17331751
});
17341752

17351753
$tester = new ApplicationTester($application);
@@ -1858,12 +1876,12 @@ public function testFindAlternativesDoesNotLoadSameNamespaceCommandsOnExactMatch
18581876
'foo:bar' => function () use (&$loaded) {
18591877
$loaded['foo:bar'] = true;
18601878

1861-
return (new Command('foo:bar'))->setCode(function () {});
1879+
return (new Command('foo:bar'))->setCode(function (): int { return 0; });
18621880
},
18631881
'foo' => function () use (&$loaded) {
18641882
$loaded['foo'] = true;
18651883

1866-
return (new Command('foo'))->setCode(function () {});
1884+
return (new Command('foo'))->setCode(function (): int { return 0; });
18671885
},
18681886
]));
18691887

@@ -1934,8 +1952,10 @@ public function testThrowingErrorListener()
19341952
$application->setAutoExit(false);
19351953
$application->setCatchExceptions(false);
19361954

1937-
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output) {
1955+
$application->register('foo')->setCode(function (InputInterface $input, OutputInterface $output): int {
19381956
$output->write('foo.');
1957+
1958+
return 0;
19391959
});
19401960

19411961
$tester = new ApplicationTester($application);

‎src/Symfony/Component/Console/Tests/Command/CommandTest.php‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\Console\Command\Command;
1919
use Symfony\Component\Console\Exception\InvalidOptionException;
2020
use Symfony\Component\Console\Helper\FormatterHelper;
21+
use Symfony\Component\Console\Input\ArrayInput;
2122
use Symfony\Component\Console\Input\InputArgument;
2223
use Symfony\Component\Console\Input\InputDefinition;
2324
use Symfony\Component\Console\Input\InputInterface;
@@ -350,8 +351,10 @@ public function testRunWithProcessTitle()
350351
public function testSetCode()
351352
{
352353
$command = new \TestCommand();
353-
$ret = $command->setCode(function (InputInterface $input, OutputInterface $output) {
354+
$ret = $command->setCode(function (InputInterface $input, OutputInterface $output): int {
354355
$output->writeln('from the code...');
356+
357+
return 0;
355358
});
356359
$this->assertEquals($command, $ret, '->setCode() implements a fluent interface');
357360
$tester = new CommandTester($command);
@@ -396,8 +399,10 @@ public function testSetCodeWithStaticClosure()
396399

397400
private static function createClosure()
398401
{
399-
return function (InputInterface $input, OutputInterface $output) {
402+
return function (InputInterface $input, OutputInterface $output): int {
400403
$output->writeln(isset($this) ? 'bound' : 'not bound');
404+
405+
return 0;
401406
};
402407
}
403408

@@ -411,16 +416,20 @@ public function testSetCodeWithNonClosureCallable()
411416
$this->assertEquals('interact called'.\PHP_EOL.'from the code...'.\PHP_EOL, $tester->getDisplay());
412417
}
413418

414-
public function callableMethodCommand(InputInterface $input, OutputInterface $output)
419+
public function callableMethodCommand(InputInterface $input, OutputInterface $output): int
415420
{
416421
$output->writeln('from the code...');
422+
423+
return 0;
417424
}
418425

419426
public function testSetCodeWithStaticAnonymousFunction()
420427
{
421428
$command = new \TestCommand();
422-
$command->setCode(static function (InputInterface $input, OutputInterface $output) {
429+
$command->setCode(static function (InputInterface $input, OutputInterface $output): int {
423430
$output->writeln(isset($this) ? 'bound' : 'not bound');
431+
432+
return 0;
424433
});
425434
$tester = new CommandTester($command);
426435
$tester->execute([]);
@@ -495,14 +504,28 @@ public function testDeprecatedMethods()
495504

496505
new FooCommand();
497506
}
507+
508+
/**
509+
* @group legacy
510+
*/
511+
public function testDeprecatedNonIntegerReturnTypeFromClosureCode()
512+
{
513+
$this->expectDeprecation('Since symfony/console 7.3: Returning a non-integer value from the command "foo" is deprecated and will throw an exception in Symfony 8.0.');
514+
515+
$command = new Command('foo');
516+
$command->setCode(function () {});
517+
$command->run(new ArrayInput([]), new NullOutput());
518+
}
498519
}
499520

500521
// In order to get an unbound closure, we should create it outside a class
501522
// scope.
502523
function createClosure()
503524
{
504-
return function (InputInterface $input, OutputInterface $output) {
525+
return function (InputInterface $input, OutputInterface $output): int {
505526
$output->writeln($this instanceof Command ? 'bound to the command' : 'not bound to the command');
527+
528+
return 0;
506529
};
507530
}
508531

0 commit comments

Comments
 (0)