Skip to content

[SPEED-3094] Support not having a Return or a Throw if an underlying function throws and is not caught#164

Merged
Fladolcetta merged 19 commits into
masterfrom
flad/handleFunctionsThatThrow
Jun 25, 2026
Merged

[SPEED-3094] Support not having a Return or a Throw if an underlying function throws and is not caught#164
Fladolcetta merged 19 commits into
masterfrom
flad/handleFunctionsThatThrow

Conversation

@Fladolcetta

@Fladolcetta Fladolcetta commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

Problem:

The following code is legitimate; but guardrail doesn't think it is

function throwFunction () {
  throw Exception;
}

function anotherFunction(bool myBool): int {
  if (myBool) {
    return 1; 
  }
  throwFunction()
}

Solution:

Check if underlying functions throw.

  • Added new tests
  • Added a throwOnly option to several functions that previously checked for return or throw
  • Add new functions to handle when a function calls a function / method / staticmethod

@Fladolcetta
Fladolcetta requested review from a team and jongardiner as code owners March 6, 2026 20:22
@Fladolcetta Fladolcetta changed the title Flad/handle functions that throw Support not having a Return or a Throw if an underlying function throws and is not caught Mar 9, 2026
@Fladolcetta Fladolcetta changed the title Support not having a Return or a Throw if an underlying function throws and is not caught [SPEED-3094] Support not having a Return or a Throw if an underlying function throws and is not caught Mar 18, 2026

@jongardiner jongardiner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, but I don't think I can approve this. It won't cover a lot of real world scenerios where the body is in another file as is, and if we parse external files, we'll take a pretty significant performance hit.

We could maybe do something around methods that are documented to return "never" which is a new return type. (Although I'm not sure throwing counts as returning.)

*
* @return bool
*/
private function isMethodCallThatThrows(Node\Expr\MethodCall $methodCall): bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what we're doing. We're trying to grab the body of the function that is being called in order to look up whether is contains a throw statement. That's going to work if the function is in the same file, but if it is a function in a different file then it won't be able to get the method body the way that we do here. There is a way to grab the file contents and parse it, but it is a pretty big performance hit. I have a branch where I do some work around declared throws where we don't have to have the method body handy to know if a function might throw. We don't have anything for a method that ALWAYS throws, which I think is what we're looking for here.

@Fladolcetta
Fladolcetta merged commit 0774489 into master Jun 25, 2026
7 checks passed
@Fladolcetta
Fladolcetta deleted the flad/handleFunctionsThatThrow branch June 25, 2026 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants