sonar-dotnet icon indicating copy to clipboard operation
sonar-dotnet copied to clipboard

Break down VB S3385 rule for Exit based on block type

Open duncanp-lseg opened this issue 7 years ago • 2 comments

[Moved from https://github.com/SonarSource/sonarlint-visualstudio/issues/766, created by @burtonrodman]

In VB.Net, the SonarLint rule S3385 Remove this 'Exit' statement triggers on all types of Exit statements.

Please break this down into multiple rules so that severities can be assigned for each case:

There are some that have no replacement (like Exit Do, Exit While) -- these should be suggestions to restructure the code -- or ignored.

I think that Exit (i.e. kill process) should be banned entirely, so deserves a separate rule that can be marked as Error.

Exit Sub can blindly be replaced with Return, while Exit Function must be manually analyzed to ensure that it doesn't affect flow control/return value.

So as you can see each Exit variant deserves it's own rule so that severity can be managed appropriately for each.

Also, please improve the Description to provide more actionable guidance.

duncanp-lseg avatar Dec 05 '18 11:12 duncanp-lseg

I agree. This rule is a good start, but it needs improvement as described by @duncanp-sonar .

Corniel avatar Jan 06 '19 16:01 Corniel

I second this suggestion. Exit While, Exit For, and Exit Do should not even be suggestions. They're just the VB equivalent of a break statement in a loop.

dwilliss avatar Sep 12 '24 14:09 dwilliss

Moved to NET-2011

Where is the official way to get more info on the rationale around things like this. To me, a Return statement without a value, which is suggested as replacement for Exit Sub is unacceptable. So for Sub the only acceptable way for early exit is Exit Sub, and Return is the bad one.

caspChristian avatar Jun 16 '25 22:06 caspChristian

Here is a good place for discussion and suggestions focused strictly on what's in the issue itself.

https://community.sonarsource.com/ is a good place for everything else, like broader discussions, and new suggestions etc.

After looking into the rule, I've canceled NET-2011, as we've removed the loop control flows already from the rule in NET-987.

The reasoning behind a rule should always be in the description of the rule that you can find in the product you use (SQ Server, Cloud or IDE. NuGet package should provide a link to https://rules.sonarsource.com)

wow! this is a blast from the past! i filled this 7 years ago 😳

it’s been a minute since i wrote VB.net, even when I was, Return (from a Sub) seemed to be becoming the norm. Regardless, both are perfectly valid, and trivially replaceable — ie very much preference. UNLIKE Exit Function which may affect the return value, and therefore requires manual analysis.

burtonrodman avatar Jun 20 '25 22:06 burtonrodman