Skip to content

Commit

Permalink
Fix FNs
Browse files Browse the repository at this point in the history
  • Loading branch information
sagi1623 authored and mary-georgiou-sonarsource committed Jun 3, 2024
1 parent 46006e7 commit a687f6d
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,33 @@ public partial class InfiniteRecursion
{
private sealed class RoslynChecker : IChecker
{
public void CheckForNoExitProperty(SonarSyntaxNodeReportingContext c, PropertyDeclarationSyntax property, IPropertySymbol propertySymbol)
public void CheckForNoExitProperty(SonarSyntaxNodeReportingContext c, PropertyDeclarationSyntax property, IPropertySymbol propertySymbol) =>
CheckForNoExit(c,
propertySymbol,
property.ExpressionBody,
property.AccessorList,
property.Identifier.GetLocation(),
"property's recursion",
"property accessor's recursion");

public void CheckForNoExitIndexer(SonarSyntaxNodeReportingContext c, IndexerDeclarationSyntax indexer, IPropertySymbol propertySymbol) =>
CheckForNoExit(c,
propertySymbol,
indexer.ExpressionBody,
indexer.AccessorList,
indexer.ThisKeyword.GetLocation(),
"indexer's recursion",
"indexer accessor's recursion");

public void CheckForNoExitEvent(SonarSyntaxNodeReportingContext c, EventDeclarationSyntax eventDeclaration, IEventSymbol eventSymbol)
{
if (property.ExpressionBody?.Expression != null)
{
var cfg = ControlFlowGraph.Create(property.ExpressionBody, c.SemanticModel, c.Cancel);
var walker = new RecursionSearcher(new RecursionContext<ControlFlowGraph>(c, cfg, propertySymbol, property.Identifier.GetLocation(), "property's recursion"));
walker.CheckPaths();
}
else if (property.AccessorList != null)
if (eventDeclaration.AccessorList != null)
{
foreach (var accessor in property.AccessorList.Accessors.Where(a => a.HasBodyOrExpressionBody()))
foreach (var accessor in eventDeclaration.AccessorList.Accessors.Where(a => a.HasBodyOrExpressionBody()))
{
var cfg = ControlFlowGraph.Create(accessor, c.SemanticModel, c.Cancel);
var context = new RecursionContext<ControlFlowGraph>(c, cfg, propertySymbol, accessor.Keyword.GetLocation(), "property accessor's recursion");
var walker = new RecursionSearcher(context, !accessor.Keyword.IsAnyKind(SyntaxKind.SetKeyword, SyntaxKindEx.InitKeyword));
var context = new RecursionContext<ControlFlowGraph>(c, cfg, eventSymbol, accessor.Keyword.GetLocation(), "event accessor's recursion");
var walker = new RecursionSearcher(context);
walker.CheckPaths();
}
}
Expand All @@ -57,6 +69,32 @@ public void CheckForNoExitMethod(SonarSyntaxNodeReportingContext c, SyntaxNode b
}
}

private static void CheckForNoExit(SonarSyntaxNodeReportingContext c,
IPropertySymbol propertySymbol,
ArrowExpressionClauseSyntax expressionBody,
AccessorListSyntax accessorList,
Location location,
string arrowExpressionMessageArg,
string accessorMessageArg)
{
if (expressionBody?.Expression != null)
{
var cfg = ControlFlowGraph.Create(expressionBody, c.SemanticModel, c.Cancel);
var walker = new RecursionSearcher(new RecursionContext<ControlFlowGraph>(c, cfg, propertySymbol, location, arrowExpressionMessageArg));
walker.CheckPaths();
}
else if (accessorList != null)
{
foreach (var accessor in accessorList.Accessors.Where(a => a.HasBodyOrExpressionBody()))
{
var cfg = ControlFlowGraph.Create(accessor, c.SemanticModel, c.Cancel);
var context = new RecursionContext<ControlFlowGraph>(c, cfg, propertySymbol, accessor.Keyword.GetLocation(), accessorMessageArg);
var walker = new RecursionSearcher(context, !accessor.Keyword.IsAnyKind(SyntaxKind.SetKeyword, SyntaxKindEx.InitKeyword));
walker.CheckPaths();
}
}
}

private sealed class RecursionSearcher : CfgAllPathValidator
{
private readonly RecursionContext<ControlFlowGraph> context;
Expand Down Expand Up @@ -97,18 +135,18 @@ when IPropertyReferenceOperationWrapper.FromOperation(operation) is var property
OperationKindEx.Invocation
when IInvocationOperationWrapper.FromOperation(operation) is var invocation && (!invocation.IsVirtual || InstanceReferencesThis(invocation.Instance)) =>
invocation.TargetMethod,
OperationKindEx.Binary
when IBinaryOperationWrapper.FromOperation(operation) is var binaryOperation =>
binaryOperation.OperatorMethod,
OperationKindEx.Decrement
when IIncrementOrDecrementOperationWrapper.FromOperation(operation) is var decrementOperation =>
decrementOperation.OperatorMethod,
OperationKindEx.Increment
when IIncrementOrDecrementOperationWrapper.FromOperation(operation) is var incrementOperation =>
incrementOperation.OperatorMethod,
OperationKindEx.Unary
when IUnaryOperationWrapper.FromOperation(operation) is var unaryOperation =>
unaryOperation.OperatorMethod,
OperationKindEx.Binary =>
IBinaryOperationWrapper.FromOperation(operation).OperatorMethod,
OperationKindEx.Decrement =>
IIncrementOrDecrementOperationWrapper.FromOperation(operation).OperatorMethod,
OperationKindEx.Increment =>
IIncrementOrDecrementOperationWrapper.FromOperation(operation).OperatorMethod,
OperationKindEx.Unary =>
IUnaryOperationWrapper.FromOperation(operation).OperatorMethod,
OperationKindEx.Conversion=>
IConversionOperationWrapper.FromOperation(operation).OperatorMethod,
OperationKindEx.EventReference =>
IEventReferenceOperationWrapper.FromOperation(operation).Member,
_ => null
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ public void CheckForNoExitProperty(SonarSyntaxNodeReportingContext c, PropertyDe
}
}

public void CheckForNoExitIndexer(SonarSyntaxNodeReportingContext c, IndexerDeclarationSyntax indexer, IPropertySymbol propertySymbol)
{
// SonarCFG is out of support
}

public void CheckForNoExitEvent(SonarSyntaxNodeReportingContext c, EventDeclarationSyntax eventDeclaration, IEventSymbol eventSymbol)
{
// SonarCFG is out of support
}

public void CheckForNoExitMethod(SonarSyntaxNodeReportingContext c, SyntaxNode body, SyntaxToken identifier, IMethodSymbol symbol)
{
if (CSharpControlFlowGraph.TryGet(body, c.SemanticModel, out var cfg))
Expand Down
32 changes: 32 additions & 0 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/InfiniteRecursion.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ protected override void Initialize(SonarAnalysisContext context)
},
SyntaxKind.OperatorDeclaration);

context.RegisterNodeAction(
c =>
{
var conversionOperator = (ConversionOperatorDeclarationSyntax)c.Node;
CheckForNoExitMethod(c, conversionOperator.OperatorKeyword);
},
SyntaxKind.ConversionOperatorDeclaration);

context.RegisterNodeAction(
c =>
{
Expand All @@ -73,6 +81,28 @@ protected override void Initialize(SonarAnalysisContext context)
}
},
SyntaxKind.PropertyDeclaration);

context.RegisterNodeAction(
c =>
{
var indexer = (IndexerDeclarationSyntax)c.Node;
if (c.SemanticModel.GetDeclaredSymbol(indexer) is { } indexerSymbol)
{
checker.CheckForNoExitIndexer(c, indexer, indexerSymbol);
}
},
SyntaxKind.IndexerDeclaration);

context.RegisterNodeAction(
c =>
{
var eventDeclaration = (EventDeclarationSyntax)c.Node;
if (c.SemanticModel.GetDeclaredSymbol(eventDeclaration) is { } eventSymbol)
{
checker.CheckForNoExitEvent(c, eventDeclaration, eventSymbol);
}
},
SyntaxKind.EventDeclaration);
}

private void CheckForNoExitMethod(SonarSyntaxNodeReportingContext c, SyntaxToken identifier)
Expand Down Expand Up @@ -124,6 +154,8 @@ public void ReportIssue() =>
private interface IChecker
{
void CheckForNoExitProperty(SonarSyntaxNodeReportingContext c, PropertyDeclarationSyntax property, IPropertySymbol propertySymbol);
void CheckForNoExitIndexer(SonarSyntaxNodeReportingContext c, IndexerDeclarationSyntax indexer, IPropertySymbol propertySymbol);
void CheckForNoExitEvent(SonarSyntaxNodeReportingContext c, EventDeclarationSyntax eventDeclaration, IEventSymbol eventSymbol);
void CheckForNoExitMethod(SonarSyntaxNodeReportingContext c, SyntaxNode body, SyntaxToken identifier, IMethodSymbol symbol);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -604,17 +604,38 @@ public IEnumerable<T> Repeat<T>(T element) // Noncompliant FP, it's not a recur
// https://github.com/SonarSource/sonar-dotnet/issues/6642
public class Repro_6642
{
private List<byte> list;

public int this[int i]
{
get { return this[i]; } // FN
set { this[i] = value; } // FN
get { return this[i]; } // Noncompliant
set { this[i] = value; } // Noncompliant
}

public char this[char i] => this[i]; // Noncompliant

public byte this[byte i]
{
get { return list[i]; } // Compliant
set { list[i] = value; } // Compliant
}

public short this[short i] => list[1623]; // Compliant

public string this[string i]
{
get { return this; } // Compliant
}

public static implicit operator string(Repro_6642 d) => "";
}

// https://github.com/SonarSource/sonar-dotnet/issues/6643
public class Repro_6643
{
public static implicit operator byte(Repro_6643 d) => d; // FN
public static implicit operator byte(Repro_6643 d) => d; // Noncompliant

public static explicit operator string(Repro_6643 d) => (string)d; // Noncompliant
}

// https://github.com/SonarSource/sonar-dotnet/issues/6644
Expand All @@ -623,15 +644,29 @@ public class Repro_6643
public class Repro_6644
{
public event SomeDelegate SomeEvent
{
add { SomeEvent += value; } // Noncompliant

remove { SomeEvent -= value; } // Noncompliant
}

public event SomeDelegate SomeEvent2
{
add { SomeEvent2 -= value; } // Noncompliant FP

remove { SomeEvent2 += value; } // Noncompliant FP
}

public event SomeDelegate SomeEvent3
{
add
{
SomeEvent += value; // FN
SomeEvent += value; // Compliant
}

remove
{
SomeEvent -= value; // FN
SomeEvent -= value; // Compliant
}
}
}

0 comments on commit a687f6d

Please sign in to comment.