Overzealous PHP Sonarlint rule about exit();

Clean Code Report False-positive / False-negative...
1

I just opened a message on Stackoverflow about this, but thought (after finding this forum) to mention it here too, sorry if this violates community rules (I did not find any yet):

Sonarlint (a code quality tool) complains about exit(); calls in my code. The code is a target ( action="" ) in a form that creates a DB row, then redirects to an overview page. Simplified like this:

// do something
// on success:
header('Location: ' . $location);
exit;

Their explanation of this error.

Is there is a “proper” way to redirect in such cases without using the exit() call? The redirect can happen very early in the file and running through all the other code following this location seems redundant.

I don’t want to just make the warning disappear (I know it’s possible to do with // NOSONAR .) but rather understand how to proper program this piece of code or if this some kind of overzealous quality control thing.

I understand, that exit(); calls are not testable - the reason for this warning. On the other side, using, for instance, the Browser tester of Codeception I can test properly if a redirect is happening.

2

Unless you’re using PHP the “old-school” way (calling a script directly), there shouldn’t be many cases where you want to interrupt the process like that.

Ideally, you should have something like this:

if ($success) {
  header('Location: ' . $location);
} else {
  // ... handle the other cases.
  // ...
}
// EOF

Logic that doesn’t apply to the success case should not be at the same “level”, but contained within an else block, for example. In many frameworks, you would probably return something here, like a Response object containing the redirect HTTP header.

This way, the process stops nicely, and you don’t prevent some “post-processing” logic to be triggered down the road (of course, this usually applies to frameworks, not using scripts directly). And, of course, it’s easier to test :man_shrugging:.

Does that make sense?

1 Like
3

Sounds good, makes sense!