Skip to content

Commit

Permalink
Fix S1854 FP: Value used in catch or when should LiveIn for all t…
Browse files Browse the repository at this point in the history
…ry blocks (#9449)
  • Loading branch information
pavel-mikula-sonarsource authored Jun 20, 2024
1 parent d9774fd commit cc797dd
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 10 deletions.
18 changes: 18 additions & 0 deletions analyzers/its/expected/Ember-MM/S2259-EmberAPI.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,24 @@
"Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/Ember-MM/EmberAPI/clsAPIDatabase.vb#L280",
"Location": "Line 280 Position 61-73"
},
{
"Id": "S2259",
"Message": "\u0027inList\u0027 is Nothing on at least one execution path.",
"Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/Ember-MM/EmberAPI/clsAPIScanner.vb#L1022",
"Location": "Line 1022 Position 60-66"
},
{
"Id": "S2259",
"Message": "\u0027dList\u0027 is Nothing on at least one execution path.",
"Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/Ember-MM/EmberAPI/clsAPIScanner.vb#L926",
"Location": "Line 926 Position 52-57"
},
{
"Id": "S2259",
"Message": "\u0027inList\u0027 is Nothing on at least one execution path.",
"Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/Ember-MM/EmberAPI/clsAPIScanner.vb#L995",
"Location": "Line 995 Position 56-62"
},
{
"Id": "S2259",
"Message": "\u0027movieName\u0027 is Nothing on at least one execution path.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,18 +93,25 @@ private void BuildBranches(BasicBlock block)
var catchesAll = false;
foreach (var catchOrFilterRegion in block.Successors.SelectMany(CatchOrFilterRegions))
{
AddBranch(block, Cfg.Blocks[catchOrFilterRegion.FirstBlockOrdinal]);
var catchOrFilterBlock = Cfg.Blocks[catchOrFilterRegion.FirstBlockOrdinal];
AddBranch(block, catchOrFilterBlock);
AddPredecessorsOutsideRegion(catchOrFilterBlock);
catchesAll = catchesAll || (catchOrFilterRegion.Kind == ControlFlowRegionKind.Catch && IsCatchAllType(catchOrFilterRegion.ExceptionType));
}
if (!catchesAll && block.EnclosingRegion(ControlFlowRegionKind.TryAndFinally)?.NestedRegion(ControlFlowRegionKind.Finally) is { } finallyRegion)
{
var finallyBlock = Cfg.Blocks[finallyRegion.FirstBlockOrdinal];
AddBranch(block, finallyBlock);
// We assume that this block can throw in its first operation. Therefore predecessors outside this tryRegion need to be redirected to finally
foreach (var predecessor in block.Predecessors.Where(x => x.Source.Ordinal < tryRegion.FirstBlockOrdinal || x.Source.Ordinal > tryRegion.LastBlockOrdinal))
{
AddBranch(predecessor.Source, finallyBlock);
}
AddPredecessorsOutsideRegion(finallyBlock);
}
}

void AddPredecessorsOutsideRegion(BasicBlock destination)
{
// We assume that current block can throw in its first operation. Therefore predecessors outside this tryRegion need to be redirected to catch/filter/finally
foreach (var predecessor in block.Predecessors.Where(x => x.Source.Ordinal < tryRegion.FirstBlockOrdinal || x.Source.Ordinal > tryRegion.LastBlockOrdinal))
{
AddBranch(predecessor.Source, destination);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -538,12 +538,38 @@ public void Catch_AssignedInTry_LiveOut()
""";
var context = CreateContextCS(code);
context.ValidateEntry();
context.Validate("Method(0);"/*Should be:, LiveOut("variable")*/);
context.Validate("Method(0);", LiveOut("variable"));
context.Validate("Method(1);", LiveOut("variable"));
context.Validate("Method(variable);", LiveIn("variable"));
context.Validate("Method(2);");
}

[TestMethod]
public void Catch_When_AssignedInTry_LiveOut()
{
var code = """
int variable = 42;
Method(0);
try
{
Method(1); // Can throw
variable = 0;
}
catch when(variable == 0)
{
Method(2);
}
Method(3);
""";
var context = CreateContextCS(code);
context.ValidateEntry();
context.Validate("Method(0);", LiveOut("variable"));
context.Validate("Method(1);", LiveOut("variable"));
context.Validate("variable == 0", LiveIn("variable"));
context.Validate("Method(2);");
context.Validate("Method(3);");
}

[TestMethod]
public void Catch_Loop_Propagates_LiveIn()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1436,7 +1436,7 @@ public void UsedInFinally_NestedInLambda()
{
}
}

public void UsedInFinally_Throw()
{
var value = 42; // Compliant
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4166,9 +4166,9 @@ void Method()
exception = e;
}
}
if (exception != null) // FN!
if (exception != null) // Noncompliant
{
Console.WriteLine(exception);
Console.WriteLine(exception); // Secondary
}
Console.WriteLine(success);
}
Expand Down

0 comments on commit cc797dd

Please sign in to comment.