Vscode-intelephense: Symbol $e marked as not used in try-catch

Created on 20 Nov 2019 · 11Comments · Source: bmewburn/vscode-intelephense

A try-catch block requires declaring a variable name for the catch definition, even if you don't intend to use it. But intelephense will mark it as unused.

I'm not 100% sure what's best. I lean towards not applying the check to catch blocks. But even a way to say "ignore this" would be okay, but I couldn't find a way to do that in the docs, though I certainly could have missed it. I could do something silly like adding $e; inside the block, but that doesn't seem the best.

Source

derekhamilton

All 11 comments

I'm highly against removing check for catch, in general you should handle exceptions in some way, and I want these errors showing in my projects (there are also opened issues for [optional] more strict checks). IMO $e; is ok-ish if you don't want to do that.

For now there isn't any way to suppress errors and I wouldn't expect that in this case.

Edit: I found most elegant suggestion to just unset variable.

KapitanOczywisty on 20 Nov 2019

It's moreso that handling the exception commonly doesn't involve doing anything with the exception object.

derekhamilton on 21 Nov 2019

I'd say the common action to resolve unused variables is to remove them which in this case would cause a parse error, so I'm leaning towards removing unused var checks in this case. Though I understand that some users may like the diagnostic here because it prompts them to do something useful with exception.

bmewburn on 21 Nov 2019

PhpStorm's behavior, for reference

An error for a typical unused variable

No indication for unused exception variable

Not that everything should copy PhpStorm, but it's a point of reference.

derekhamilton on 2 Dec 2019

Apologies for the dupe.

Possibly worth opening another issue for this, but if some kind of markup (e.g. @unused or @ignore) is ultimately used to suppress this problem, then it would work for this additional, semi-related issue:

If a class member variable is defined, but only referenced dynamically, then a similar warning is generated.

class Wibble
{
  private $_myVar = '123';

  public function foo($arg = 'myVar')
  {
    $prop = '_'.$arg;
    echo $this->$prop."\n";
  }
}

(new Wibble())->foo();

This example is contrived but I have some classes where arrays of valid values are defined as class variables and ultimately only referenced in this way.

As I say if some @unused or @ignore markup was added to these then the problem could go away which may also apply as a fix for the exception try-catch use case.

coling on 4 Dec 2019

For the avoidance of doubt, I'm suggesting something along the lines of:

...
catch (Exception /** @ignore */ $e)
...

type syntax here, although this might be very different to any other phpdoc parsing and against it's general principles, so not really a valid option.

coling on 4 Dec 2019

@coling if you want to write /** @ignore */ you can also write unset($e);

Variable variables are tricky. Extension can only go silent when there is any present.

These errors are hint level anyway, I'm not bother by that, but explicit suppress would be nice.

KapitanOczywisty on 4 Dec 2019

@coling if you want to write /** @ignore */ you can also write unset($e);

Yeah, but then you're relying on the optimiser to get rid of that operation rather than the pre-parser/op-code cacher. I'd guess a $e=$e would be more likely/obvious to optimise out the way but I really don't know the guts of Zend in that way.

Variable variables are tricky. Extension can only go silent when there is any present.

These errors are hint level anyway, I'm not bother by that, but explicit suppress would be nice.

Yeah it's not the end of the world, but would be nice to be able to suppress somehow.

coling on 4 Dec 2019