Try...Suppress

[MGatner]

Description
The @ suppression was preventing errors from being thrown & caught in the try...catch block, causing delete_files() to return true even when it failed. The "failure" test was still passing because RecursiveIteratorIterator still failed on the non-existent directory.

@samsonasik Is there a Rector rule for @ suppression in try...catch? Seems like it should be an easy one.

Checklist:

• Securely signed commits
• Component(s) with PHPdocs
• Unit testing, with >80% coverage
• n/a User guide updated
• Conforms to style guide

[samsonasik]

Do you mean a rector rule that can remove @ error suppress? It seems not yet as far as I know.

There is ErrorSuppress node for that that we can use on create custom rector rule for it, like this:

@MGatner do you want me to create PR for it to add custom rector rule to https://github.com/codeigniter4/CodeIgniter4/tree/develop/utils/Rector for it?

[MGatner]

In my opinion, there should never be an error suppression (e.g. @unlink($file)) inside a try {} block, because it prevents the error from being thrown and caught. I assume any instance of these would be a logic error.

If that's easy to write it would be a nice addition, but don't spend a lot of time on it. I thought there might be something already since this seems like perhaps not an uncommon mistake.

[paulbalandan]

I don't think it is correct to change to simple comment. IDEs will not be able to parse the docblock

[paulbalandan]

I agree. The @ operator should not be present inside a try catch as that would be contradicting.

As there is an ErrorSuppress node, maybe there's an equivalent Node object for try catch in the php-parser library. We should check the presence of both node types before applying the rector rule.

[samsonasik]

Ok, so it should be RemoveErrorSuppressInsideTryRector

[samsonasik]

@MGatner @paulbalandan here the PR #4724