Throw points (work in progress)

[rainbow-alex]

As discussed in #472 I've begun implementing the 'throw points' logic in NodeScopeResolver. As you suspected @ondrejmirtes assertVariableCertainty does the trick.

So far I've only implemented throw points at a statement level, so all expressions are still behaving as if they are nothrow, only an explicit throw statement creates a point. I just started extending the implementation to the expression level and I am running into an issue. I am trying to get this test to pass:

/**
 * @throws Exception
 */
function throws()
{
}

function () {
	try {
		throws();
		$foo = 1;
	} finally {
		assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
	}
};

But I can't seem to get throws() to resolve... when processing the FuncCall node, $functionReflection is always null. I'm not sure how get it to work, I hope I won't have to go messing around in TestCase::createBroker...

[ondrejmirtes]

Hi, I like starting with a minimal approach so 👍 on that! I'll review it once I get to chance, if it's a cohesive set of changes please don't add anything to it meanwhile.

As for the test you can't figure out - for functions you need to require_once the file in gatherAssertTypes. That's why you've put the assert code in an anonymous function below so that's good.

Alternatively you can test classes + calling methods which thanks to autoloading doesn't require such workarounds. Don't forget to call composer dump-autoload once you add the class.

[rainbow-alex]

As for the test you can't figure out - for functions you need to require_once the file in gatherAssertTypes. That's why you've put the assert code in an anonymous function below so that's good.

That works, thank you!

Alternatively you can test classes + calling methods which thanks to autoloading doesn't require such workarounds. Don't forget to call composer dump-autoload once you add the class.

I'll need to do both since FuncCall and MethodCall are separate nodes...

[ondrejmirtes]

It would be great if all the tests passed before I review this. Thank you.

[ondrejmirtes]

But the general direction looks promising :)

[ondrejmirtes]

Please use rebase instead of merge commits, I want to keep the history linear.

[rainbow-alex]

That should be green + no merge commits @ondrejmirtes.

Still to do:

• implement throw points for a lot of expression nodes, though some of the hardest ones (assignment and calls) are done already
• take into account throw points for expression nodes of statements (e.g. expr in if (expr) stmts)

I had to make some adjustments to the resolution of TryCatch to prevent regressions, I tried to keep the changes as small as possible (for this PR).

[rainbow-alex]

Nice! Looks like we're getting there, do you want me to add the test coverage for the nodes you implemented as well?

[ondrejmirtes]

@rainbow-alex It's fine like this, I'm almost finished locally :)

[ondrejmirtes]

@rainbow-alex This commit (e0182ce) is the main thing that was missing - the actual connection between throw points and specific catch blocks :)

[ondrejmirtes]

Thank you!