@TomasVotruba it can only reduce the next/parent usage, but not actually remove it, as there is jump-next check for the following use case:

        if (rand(0, 1)) {
            try {
            } catch (SomeException $typoException) {
            }
        }

        if (isset($typoException)) {
            $this->verify($typoException);
            $this->verify2($typoException);
        }

which there is no way to check without that parent lookup and next

If you can't go without parent/next, the next best step would be having a separate visitor for this exact use-case which adds parent/next only on the nodes required for the rector in question.

I am thinking about "next_stmt" attribute which only for stmt instead of original "next" which can be both Expr and Stmt.

That still be general, not on per specific node, which it marked as next as far parent is not FunctionLike

To be honest, I don't think we should support rename of such variables as optional ones. We don't do that in any other variable rename rule and handling these is overly complex.

PHPStan should report those variables and we should focus only on current context.

I am thinking about "next_stmt" attribute which only for stmt instead of original "next" which can be both Expr and Stmt.

IMO this won't work.

we try to get rid of as much as possible attributes, because the references between the AST nodes prevent garbage collection (or at least make it pretty slow/inefficient). as soon as we decide on a attribute we put on every/most node(s), we have lost this battle.

Checking only on current scope make it still require "parent" attribute to skip if not directly inside `"FunctionLike", to be something like:

/** @var TryCatch $stmt */
$parentStmt = $stmt->getAttribute(AttributeKey::PARENT_NODE);
if (! $parentStmt instanceof FunctionLike) {
    continue;
}

that's the only way to skip possible variable re-use on parent-next, the drawbacks is that the current and then parent-next will no longer supported.

Checking only on current scope make it still require "parent" attribute to skip if not directly inside `"FunctionLike", to be something like:

By current scope I meant only try/catch content, not the lines after.

try { 
} catch (...) {
   // here
}

// not here

@TomasVotruba That will cause a bug as variable can be used later, variable after try catch need to replaced, just not if jump next, by looking parent.

I refactored to skip next by jump by verify if parent is not FunctionLike and not Namespace_ or FileWithoutNamespace so that means the try catch is on inline stmt, not in depth, which need to be skipped.

The following code is replaced correctly, and fine:

        try {
-        } catch (SomeException $typoException) {
+        } catch (SomeException $someException) {
        }


-        $this->verify($typoException);
+        $this->verify($someException);

but not this, as in depth:

        if (rand(0, 1)) {
            try {
            } catch (SomeException $typoException) {
            }
        }

        if (isset($typoException)) {
            $this->verify($typoException);
            $this->verify2($typoException);
        }

All checks have passed 🎉 @TomasVotruba I think it is ready.

This is why variable after try catch need to be changed, or it cause a bug https://3v4l.org/C0uci , there is fixture for it already

that's the different over the jump one

Thanks for the refactoring. I understand it could hypothetically create a bug situation, yet we want to avoid using next/prev nodes in all the rules, and think this code is extreemely rare, as it assumes a variable exists and also exception is thrown at the same time.

There are no solution, just trade offs. Let's take the leap for better performance :)