Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate S2589: Fix rethrow FP #7706

Merged
merged 4 commits into from
Aug 7, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@ private ExplodedNode ProcessBranch(ExplodedNode node, ControlFlowBranch branch)
{
return FromFinally(node.FinallyPoint.CreateNext()); // Redirect from finally back to the original place (or outer finally on the same branch)
}
else if (branch.Source.BranchValue is { } branchValue)
{
// If a branch has no Destination but is part of conditional branching we need to call ConditionEvaluated. This happens when a Rethrow is following a condition.
var state = SetBranchingConstraints(branch, node.State, branchValue);
checks.ConditionEvaluated(new(branchValue.ToSonar(), state, false, node.VisitCount, lva.CapturedVariables));
return null;
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
return null; // We don't know where to continue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,30 @@ public void Branching_ConditionEvaluated()
SETestContext.CreateCS(code, check).Validator.ValidateTagOrder("Begin", "End");
}

[TestMethod]
public void Branching_Rethrow_CallsConditionEvaluated()
{
const string code = """
try
{
Console.WriteLine("may throw");
}
catch
{
if (boolParameter)
throw;
}
""";
var count = 0;
var check = new ConditionEvaluatedTestCheck(x =>
{
count++;
return x.State;
});
SETestContext.CreateCS(code, check).Validator.ValidateTagOrder();
count.Should().Be(2);
}

[TestMethod]
public void Branching_NullConstraints_VisitsIfBranch()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1576,6 +1576,25 @@ public void ObjectsShouldNotBeDisposedMoreThanOnce()
}
}

public void Throw(bool condition)
{
if (condition) // Compliant
throw new Exception();
}

public void Rethrow(bool condition)
{
try
{
Console.WriteLine("may throw");
}
catch
{
if (condition) // Compliant
throw;
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
}
}

public void FalseNegatives()
{
// We cannot detect the case in ObjectsShouldNotBeDisposedMoreThanOnce method above
Expand Down