8000 [Console] Calling Command#getHelper() causes an error if there is no helperSet defined · Issue #18619 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Console] Calling Command#getHelper() causes an error if there is no helperSet defined #18619

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.

Sign up for GitHub

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

Jump to bottom
Closed
chalasr opened this issue Apr 23, 2016 · 0 comments
Closed

[Console] Calling Command#getHelper() causes an error if there is no helperSet defined #18619

chalasr opened this issue Apr 23, 2016 · 0 comments
Labels
Console

Comments

@chalasr
Copy link
Member
chalasr commented Apr 23, 2016 •
edited
Loading

Problem

If a command contains a call to $this->getHelper('helper') and there is no helperSet defined, the expected InvalidArgumentException (that should be thrown if the helper is not defined) is not thrown because of a PHP fatal error.

The error can be reproduced when testing a command with the CommandTester.
If the command contains $this->getHelper('question') and is executed without calling Command#setHelperSet(new HelperSet()), it causes:

PHP Fatal error: Call to a member function get() on null in /project/vendor/symfony/console/Command/Command.php on line 609

It's a bit hard to debug because the error only say where the get() is called, so there is no real trace containing the original method call.

Possible solution

My first thought, throw an exception in Command#getHelper() if there is no helperSet defined.
I have a ready patch:

use Symfony\Component\Console\Exception\LogicException;
// ...

/**
 * Gets a helper instance by name.
 *
 * @param string $name The helper name
 *
 * @return mixed The helper value
 *
 * @throws LogicException                  If there is no helperSet defined
 * @throws InvalidArgumentException if the helper is not defined
 */
public function getHelper($name)
{
    if (null === $this->helperSet) {
        throw new LogicException(sprintf('Unable to retrieve the helper "%s" because there is no HelperSet defined. Did you forget to call %s#setHelperSet()?', $name, get_class($this)));
    }

    return $this->helperSet->get($name);
}

That gives:

Symfony\Component\Console\Exception\LogicException: Unable to retrieve the helper "question" because there is no HelperSet defined. Did you forget to call Command\MyCommand#setHelperSet()?

Otherwise, we could define an HelperSet by default and so the InvalidArgumentException would be thrown.

Am I wrong when I think that this error should be prevented?
Or is there a better idea?

Thank's.
@chalasr chalasr changed the title Calling Command#getHelper() causes an error if there is no helperSet defined [Console] Calling Command#getHelper() causes an error if there is no helperSet defined Apr 23, 2016
chalasr added a commit to chalasr/symfony that referenced this issue Apr 25, 2016
@chalasr
[Console][symfony#18619] Prevent fatal error when calling Command#get…
fef1a63 
…Helper() without helperSet
chalasr added a commit to chalasr/symfony that referenced this issue Apr 25, 2016
@chalasr
[Console][symfony#18619] Prevent fatal error when calling Command#get…
6b99f20 
…Helper() without helperSet
@chalasr chalasr mentioned this issue Apr 25, 2016
Closed
chalasr added a commit to chalasr/symfony that referenced this issue Apr 25, 2016
@chalasr
[Console][symfony#18619] Prevent fatal error when calling Command#get…
50816e2 
…Helper() without helperSet

Use Command::setHelperSet rather than Command#setHelperSet in exception msg
chalasr added a commit to chalasr/symfony that referenced this issue May 2, 2016
@chalasr
[Console][symfony#18619] Prevent fatal error when calling Command#get…
c232ecb 
…Helper() without helperSet

Use Command::setHelperSet rather than Command#setHelperSet in exception msg

Simplify exception message
chalasr added a commit to chalasr/symfony that referenced this issue May 2, 2016
@chalasr
[Console][symfony#18619] Prevent fatal error when calling Command#get…
6045a23 
…Helper() without helperSet

Use Command::setHelperSet rather than Command#setHelperSet in exception msg

Simplify exception message

Add DidYouForget to exception msg
chalasr added a commit to chalasr/symfony that referenced this issue May 2, 2016
@chalasr
[Console][symfony#18619] Prevent fatal error when calling Command#get…
ede4d92 
…Helper() without helperSet

Use Command::setHelperSet rather than Command#setHelperSet in exception msg

Simplify exception message

Add DidYouForget to exception msg
fabpot pushed a commit that referenced this issue May 13, 2016
@chalasr @fabpot
[Console][#18619] Prevent fatal error when calling Command#getHelper(…
31285c2 
…) without helperSet

Use Command::setHelperSet rather than Command#setHelperSet in exception msg

Simplify exception message

Add DidYouForget to exception msg
fabpot added a commit that referenced this issue May 13, 2016
@fabpot
bug #18635 [Console] Prevent fatal error when calling Command::getHel…
f999e77 
…per without helperSet (chalasr)

This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #18635).

Discussion
----------

[Console] Prevent fatal error when calling Command::getHelper without helperSet

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #18619
| License       | MIT
| Doc PR        | n/a

Patch attached to #18619

Commits
-------

31285c2 [Console][#18619] Prevent fatal error when calling Command#getHelper() without helperSet
@fabpot fabpot closed this as completed May 13, 2016
fabpot added a commit that referenced this issue May 13, 2016
@fabpot
Merge branch '2.3' into 2.7
58f8f51 
* 2.3:
  Update HTTP statuses list
  [Console][#18619] Prevent fatal error when calling Command#getHelper() without helperSet
  Add SplFileInfo array doc on Finder iterator methods so that IDE will know what it returns
  [2.3] [Form] Modified iterator_to_array's 2nd parameter to false in ViolationMapper
  Updated the link to the list of currency codes
fabpot added a commit that referenced this issue May 13, 2016
@fabpot
Merge branch '2.7' into 2.8
1253b1d 
* 2.7:
  added missing constant in Response
  Update HTTP statuses list
  [Console][#18619] Prevent fatal error when calling Command#getHelper() without helperSet
  added StaticVerionStrategyTest
  Add SplFileInfo array doc on Finder iterator methods so that IDE will know what it returns
  [2.3] [Form] Modified iterator_to_array's 2nd parameter to false in ViolationMapper
  Updated the link to the list of currency codes
  [console][table] adjust width of colspanned cell.
fabpot added a commit that referenced this issue May 13, 2016
@fabpot
Merge branch '2.8' into 3.0
797e83f 
* 2.8:
  Fix computation of PR diffs for component matrix lines
  [console][table] adjust width of colspanned cell.
  [BUG] Delete class 'control-group' in bootstrap 3
  [2.8] [Form] Modified iterator_to_array's 2nd parameter to false in ViolationMapper
  added missing constant in Response
  Update HTTP statuses list
  [Console][#18619] Prevent fatal error when calling Command#getHelper() without helperSet
  added StaticVerionStrategyTest
  Add SplFileInfo array doc on Finder iterator methods so that IDE will know what it returns
  [2.3] [Form] Modified iterator_to_array's 2nd parameter to false in ViolationMapper
  Updated the link to the list of currency codes
  [console][table] adjust width of colspanned cell.
fabpot added a commit that referenced this issue May 13, 2016
@fabpot
Merge branch '3.0'
a1b2d8c 
* 3.0:
  Fix computation of PR diffs for component matrix lines
  [console][table] adjust width of colspanned cell.
  [BUG] Delete class 'control-group' in bootstrap 3
  [2.8] [Form] Modified iterator_to_array's 2nd parameter to false in ViolationMapper
  added missing constant in Response
  Update HTTP statuses list
  [Console][#18619] Prevent fatal error when calling Command#getHelper() without helperSet
  added StaticVerionStrategyTest
  Add SplFileInfo array doc on Finder iterator methods so that IDE will 
4E94
know what it returns
  [2.3] [Form] Modified iterator_to_array's 2nd parameter to false in ViolationMapper
  Updated the link to the list of currency codes
  Fixed DateTimeInterface comparaison
  [console][table] adjust width of colspanned cell.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Console
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@fabpot @javiereguiluz @chalasr
0