From a687f6dd9a68d5c4965c784130c44740bc548e02 Mon Sep 17 00:00:00 2001 From: AraGimLeg Date: Thu, 14 Dec 2023 00:39:20 +0100 Subject: [PATCH] Fix FNs --- .../Rules/InfiniteRecursion.RoslynCfg.cs | 84 ++++++++++++++----- .../Rules/InfiniteRecursion.SonarCfg.cs | 10 +++ .../Rules/InfiniteRecursion.cs | 32 +++++++ .../TestCases/InfiniteRecursion.RoslynCfg.cs | 45 ++++++++-- 4 files changed, 143 insertions(+), 28 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/InfiniteRecursion.RoslynCfg.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/InfiniteRecursion.RoslynCfg.cs index 26476153140..bde85912804 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/InfiniteRecursion.RoslynCfg.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/InfiniteRecursion.RoslynCfg.cs @@ -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(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(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(c, cfg, eventSymbol, accessor.Keyword.GetLocation(), "event accessor's recursion"); + var walker = new RecursionSearcher(context); walker.CheckPaths(); } } @@ -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(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(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 context; @@ -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 }; diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/InfiniteRecursion.SonarCfg.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/InfiniteRecursion.SonarCfg.cs index b04a889163f..741dfcb0608 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/InfiniteRecursion.SonarCfg.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/InfiniteRecursion.SonarCfg.cs @@ -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)) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/InfiniteRecursion.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/InfiniteRecursion.cs index 6d68bc20cd7..d755e8e3b9c 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/InfiniteRecursion.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/InfiniteRecursion.cs @@ -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 => { @@ -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) @@ -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); } } diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/InfiniteRecursion.RoslynCfg.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/InfiniteRecursion.RoslynCfg.cs index df346f536a3..6a1e2bdb2b3 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/InfiniteRecursion.RoslynCfg.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/InfiniteRecursion.RoslynCfg.cs @@ -604,17 +604,38 @@ public IEnumerable Repeat(T element) // Noncompliant FP, it's not a recur // https://github.com/SonarSource/sonar-dotnet/issues/6642 public class Repro_6642 { + private List 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 @@ -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 } } }