diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/ControllersHaveTooManyResponsibilities.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/ControllersHaveTooManyResponsibilities.cs index dc6670dce56..4bb16997459 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/ControllersHaveTooManyResponsibilities.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/ControllersHaveTooManyResponsibilities.cs @@ -112,8 +112,10 @@ private static void CheckApiController(SonarSymbolStartAnalysisContext symbolSta private static string BlockName(SyntaxNode block) => block switch { - MethodDeclarationSyntax method => method.GetName(), AccessorDeclarationSyntax { Parent.Parent: PropertyDeclarationSyntax property } => property.GetName(), + ArrowExpressionClauseSyntax { Parent: PropertyDeclarationSyntax property } => property.GetName(), + MethodDeclarationSyntax method => method.GetName(), + PropertyDeclarationSyntax property => property.GetName(), _ => null }; @@ -150,7 +152,14 @@ private static ImmutableHashSet RelevantMemberNames(INamedTypeSymbol sym } private static bool IsService(ISymbol symbol) => - symbol.GetSymbolType() is { TypeKind: TypeKind.Interface, Name: var name } && !ExcludedWellKnownServices.Contains(name); + symbol.GetSymbolType() switch + { + { TypeKind: TypeKind.Interface, Name: var name } => + !ExcludedWellKnownServices.Contains(name), + INamedTypeSymbol { TypeKind: TypeKind.Delegate or TypeKind.Class } type => + (type.IsAny(KnownType.SystemFuncVariants) || type.Is(KnownType.System_Lazy)) && IsService(type.TypeArguments.Last()), + _ => false, + }; private static IEnumerable SecondaryLocations(INamedTypeSymbol controllerSymbol, List> sets) { diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/ControllersHaveTooManyResponsibilities.CSharp12.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/ControllersHaveTooManyResponsibilities.CSharp12.cs index 1afe0e83a07..14089c5b041 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/ControllersHaveTooManyResponsibilities.CSharp12.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/ControllersHaveTooManyResponsibilities.CSharp12.cs @@ -578,21 +578,6 @@ public class WithLambdaCapturingService : ApiController // Noncompliant: s1Provi public void A2() { s2Provider(42).Use(); } // Secondary {{Belongs to responsibility #2.}} } - public class WithServiceWrappersInjection : ApiController // Noncompliant FP: no way to know whether s2Invoker wraps a service - { - private readonly Func s1Provider; - private readonly Action s2Invoker; - - public WithServiceWrappersInjection(Func s1Provider, Action s2Invoker) - { - this.s1Provider = s1Provider; - this.s2Invoker = s2Invoker; - } - - public void A1() { s1Provider().Use(); s2Invoker(); } // Secondary {{Belongs to responsibility #1.}} - public void A2() { s2Invoker(); } // Secondary {{Belongs to responsibility #2.}} - } - public class WithNonPublicConstructor : ApiController // Noncompliant: ctor visibility is irrelevant { private readonly IS1 s1; @@ -781,3 +766,79 @@ public class WithMethodsDependingOnEachOther : ApiController // Noncompliant void A21() { A22(); } // Secondary {{Belongs to responsibility #6.}} void A22() { s6.Use(); s7.Use(); } // Secondary {{Belongs to responsibility #6.}} } + +public class WithServiceProvidersInjectionCoupled : ApiController // Compliant: s1Provider and s2Provider are known to provide services +{ + private readonly Func s1Provider; + private readonly Func s2Provider; + + public WithServiceProvidersInjectionCoupled(Func s1Provider, Func s2Provider) + { + this.s1Provider = s1Provider; + this.s2Provider = s2Provider; + } + + public void A1() { s1Provider().Use(); var s2 = s2Provider(42); s2.Use(); } + public void A2() { (s2Provider(42)).Use(); } +} + +public class WithServiceProvidersInjectionUsedInGroups : ApiController // Noncompliant +{ + private IS1 _s1; + private Func, IS5> _s5Provider; + + private Func S2Provider { get; } + private Func S3Provider => i => null; + private Func S4Provider { get; set; } = (s, c) => null; + private Func, IS5> S5Provider => _s5Provider; + + public void A1() { _s1.Use(); } // Secondary {{Belongs to responsibility #1.}} + public void A2() { S2Provider().Use(); S3Provider(42).Use(); } // Secondary {{Belongs to responsibility #3.}} + public void A3() { S3Provider(42).Use(); } // Secondary {{Belongs to responsibility #3.}} + public void A4() { S4Provider("42", '4').Use(); _s5Provider(false, 3u, () => 42.0).Use(); } // Secondary {{Belongs to responsibility #2.}} + public void A5() { _s5Provider(true, 3u, () => 42.0).Use(); } // Secondary {{Belongs to responsibility #2.}} + public void A6() { S5Provider(true, 3u, () => 42.0).Use(); } // Secondary {{Belongs to responsibility #2.}} +} + +public class WithServiceWrappersInjection : ApiController // Noncompliant: FP no way to know whether s2Invoker wraps a service +{ + private readonly Func s1Provider; + private readonly Action s2Invoker; + + public WithServiceWrappersInjection(Func s1Provider, Action s2Invoker) + { + this.s1Provider = s1Provider; + this.s2Invoker = s2Invoker; + } + + public void A1() { s1Provider().Use(); s2Invoker(); } // Secondary {{Belongs to responsibility #1.}} + public void A2() { s2Invoker(); } // Secondary {{Belongs to responsibility #2.}} +} + + +public class WithLazyServicesInjection : ApiController // Noncompliant +{ + private readonly Lazy s1Lazy; + private readonly Lazy s2Lazy; + private readonly Lazy s3Lazy; + private readonly IS4 s4; + + public void A1() { s1Lazy.Value.Use(); s2Lazy.Value.Use(); } // Secondary {{Belongs to responsibility #1.}} + public void A2() { s2Lazy.Value.Use(); s4.Use(); } // Secondary {{Belongs to responsibility #1.}} + public void A3() { s3Lazy.Value.Use(); } // Secondary {{Belongs to responsibility #2.}} +} + +public class WithMixOfLazyServicesAndServiceProviders : ApiController // Noncompliant +{ + private readonly Lazy s1Lazy; + private readonly Lazy s2Lazy; + private readonly IS3 s3; + private readonly IS4 s4; + private readonly Func s5Provider; + private readonly Func s6Provider; + + public void A1() { s1Lazy.Value.Use(); s3.Use(); } // Secondary {{Belongs to responsibility #1.}} + public void A2() { s1Lazy.Value.Use(); s2Lazy.Value.Use(); } // Secondary {{Belongs to responsibility #1.}} + public void A3() { s4.Use(); var s5 = s5Provider(); s5.Use(); } // Secondary {{Belongs to responsibility #2.}} + public void A4() { s5Provider().Use(); var s6ProviderAlias = s6Provider(); s6ProviderAlias.Use(); } // Secondary {{Belongs to responsibility #2.}} +}