S6960: Support for Func<service> and Lazy<service>

analyzers/rspec/cs/S6960.json

@@ -0,0 +1,24 @@
+{
+  "title": "Controllers should not have too many responsibilities",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Linear",
+    "linearDesc": "responsibilities",
+    "linearFactor": "15min"
+  },
+  "tags": [
+    "asp.net"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-6960",
+  "sqKey": "S6960",
+  "scope": "Main",
+  "quickfix": "partial",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "HIGH"
+    },
+    "attribute": "MODULAR"
+  }
+}

analyzers/rspec/cs/Sonar_way_profile.json

@@ -317,6 +317,7 @@
     "S6930",
     "S6931",
     "S6934",
+    "S6960",
     "S6961"
   ]
 }

analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/ControllersHaveTooManyResponsibilities.cs

@@ -0,0 +1,180 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2024 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+using System.Collections.Concurrent;
+
+namespace SonarAnalyzer.Rules.CSharp;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class ControllersHaveTooManyResponsibilities : SonarDiagnosticAnalyzer
+{
+    private const string DiagnosticId = "S6960";
+    private const string MessageFormat = "This controller has multiple responsibilities and could be split into {0} smaller units.";
+
+    private static readonly HashSet<string> ExcludedWellKnownServices =
+    [
+        "ILogger",
+        "IMediator",
+        "IMapper",
+        "IConfiguration",
+        "IBus",
+        "IMessageBus"
+    ];
+
+    private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+    protected override void Initialize(SonarAnalysisContext context) =>
+        context.RegisterCompilationStartAction(compilationStartContext =>
+        {
+            if (compilationStartContext.Compilation.ReferencesControllers())
+            {
+                compilationStartContext.RegisterSymbolStartAction(symbolStartContext =>
+                {
+                    var symbol = (INamedTypeSymbol)symbolStartContext.Symbol;
+                    if (symbol.IsControllerType()
+                        && symbol.GetAttributesWithInherited().Any(x => x.AttributeClass.DerivesFrom(KnownType.Microsoft_AspNetCore_Mvc_ApiControllerAttribute)))
+                    {
+                        CheckApiController(symbolStartContext, symbol);
+                    }
+                }, SymbolKind.NamedType);
+            }
+        });
+
+    private static void CheckApiController(SonarSymbolStartAnalysisContext symbolStartContext, INamedTypeSymbol controllerSymbol)
+    {
+        var memberNames = RelevantMemberNames(controllerSymbol);
+
+        if (memberNames.Count < 2)
+        {
+            return;
+        }
+
+        var dependencies = new ConcurrentStack<Dependency>();
+
+        symbolStartContext.RegisterCodeBlockStartAction<SyntaxKind>(codeBlockStartContext =>
+        {
+            if (BlockName(codeBlockStartContext.CodeBlock) is { } blockName)
+            {
+                codeBlockStartContext.RegisterNodeAction(c =>
+                {
+                    if (c.Node.GetName() is { } dependencyName && memberNames.Contains(dependencyName))
+                    {
+                        dependencies.Push(new(blockName, dependencyName));
+                    }
+                }, SyntaxKind.IdentifierName);
+            }
+        });
+
+        symbolStartContext.RegisterSymbolEndAction(symbolEndContext =>
+        {
+            var dependencySets = new DisjointSets(memberNames);
+            foreach (var dependency in dependencies)
+            {
+                dependencySets.Union(dependency.From, dependency.To);
+            }
+
+            var responsibilityGroups = dependencySets.GetAllSets();
+            if (responsibilityGroups.Count > 1)
+            {
+                var secondaryLocations = SecondaryLocations(controllerSymbol, responsibilityGroups);
+                foreach (var primaryLocation in LocationIdentifiers<ClassDeclarationSyntax>(controllerSymbol))
+                {
+                    symbolEndContext.ReportIssue(Diagnostic.Create(
+                        Rule,
+                        primaryLocation,
+                        secondaryLocations.ToAdditionalLocations(),
+                        secondaryLocations.ToProperties(),
+                        responsibilityGroups.Count));
+                }
+            }
+        });
+    }
+
+    private static string BlockName(SyntaxNode block) =>
+        block switch
+        {
+            AccessorDeclarationSyntax { Parent.Parent: PropertyDeclarationSyntax property } => property.GetName(),
+            ArrowExpressionClauseSyntax { Parent: PropertyDeclarationSyntax property } => property.GetName(),
+            MethodDeclarationSyntax method => method.GetName(),
+            PropertyDeclarationSyntax property => property.GetName(),
+            _ => null
+        };
+
+    private static IEnumerable<Location> LocationIdentifiers<T>(ISymbol symbol) where T : SyntaxNode =>
+        symbol.DeclaringSyntaxReferences.Select(x => x.GetSyntax()).OfType<T>().Select(x => x.GetIdentifier()?.GetLocation());
+
+    private static ImmutableHashSet<string> RelevantMemberNames(INamedTypeSymbol symbol)
+    {
+        var builder = ImmutableHashSet.CreateBuilder<string>();
+        foreach (var member in symbol.GetMembers())
+        {
+            switch (member)
+            {
+                // Constructors are not considered because they have to be split anyway
+                // Accessors are not considered because they are part of properties, that are considered as a whole
+                case IMethodSymbol method when !method.IsConstructor() && !method.IsStaticConstructor() && method.AssociatedSymbol is not IPropertySymbol:
+                    builder.Add(method.Name);
+                    break;
+                // Primary constructor parameters may or may not generate fields, and must be considered
+                case IMethodSymbol method when method.IsPrimaryConstructor():
+                    builder.UnionWith(method.Parameters.Where(IsService).Select(x => x.Name));
+                    break;
+                // Backing fields are excluded for auto-properties, since they are considered part of the property
+                case IFieldSymbol field when IsService(field) && !field.IsImplicitlyDeclared:
+                    builder.Add(field.Name);
+                    break;
+                case IPropertySymbol property when IsService(property):
+                    builder.Add(property.Name);
+                    break;
+            }
+        }
+
+        return builder.ToImmutable();
+    }
+
+    private static bool IsService(ISymbol symbol) =>
+        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<SecondaryLocation> SecondaryLocations(INamedTypeSymbol controllerSymbol, List<List<string>> sets)
+    {
+        var methods = controllerSymbol.GetMembers().OfType<IMethodSymbol>().ToList();
+        for (var setIndex = 0; setIndex < sets.Count; setIndex++)
+        {
+            foreach (var memberLocation in sets[setIndex].SelectMany(MemberLocations))
+            {
+                yield return new SecondaryLocation(memberLocation, $"Belongs to responsibility #{setIndex + 1}.");
+            }
+        }
+
+        IEnumerable<Location> MemberLocations(string memberName) =>
+            methods.Where(x => x.Name == memberName).SelectMany(LocationIdentifiers<MethodDeclarationSyntax>);
+    }
+
+    private record struct Dependency(string From, string To);
+}

analyzers/src/SonarAnalyzer.Common/Common/DisjointSets.cs

@@ -0,0 +1,47 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2024 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+namespace SonarAnalyzer.Common;
+
+/// <summary>
+/// Data structure for working with disjoint sets of strings, to perform union-find operations with equality semantics:
+/// i.e. reflexivity, symmetry and transitivity. See https://en.wikipedia.org/wiki/Disjoint-set_data_structure.
+///
+/// An example of use is to build undirected connected components of dependencies, where each node is the identifier.
+///
+/// Uses a dictionary of strings as a backing store. The dictionary represents a forest of trees, where each node is
+/// a string and each tree is a set of nodes.
+/// </summary>
+public class DisjointSets
+{
+    private readonly Dictionary<string, string> parents;
+
+    public DisjointSets(IEnumerable<string> elements) =>
+        parents = elements.ToDictionary(x => x, x => x);
+
+    public void Union(string from, string to) =>
+        parents[FindRoot(from)] = FindRoot(to);
+
+    public string FindRoot(string element) =>
+        parents[element] is var root && root != element ? FindRoot(root) : root;
+
+    public List<List<string>> GetAllSets() =>
+        [.. parents.GroupBy(x => FindRoot(x.Key), x => x.Key).Select(x => x.OrderBy(x => x).ToList()).OrderBy(x => x[0])];
+}

analyzers/src/SonarAnalyzer.Common/Helpers/SymbolHelper.cs

@@ -181,6 +181,9 @@ public static Accessibility GetEffectiveAccessibility(this ISymbol symbol)
         public static bool IsConstructor(this ISymbol symbol) =>
             symbol.Kind == SymbolKind.Method && symbol.Name == ".ctor";
 
+        public static bool IsStaticConstructor(this ISymbol symbol) =>
+            symbol.Kind == SymbolKind.Method && ((IMethodSymbol)symbol).MethodKind == MethodKind.StaticConstructor;
+
         public static bool IsDestructor(this IMethodSymbol method) =>
             method.MethodKind == MethodKind.Destructor;
 

analyzers/tests/SonarAnalyzer.Test/Common/DisjointSetsTest.cs

@@ -0,0 +1,72 @@
+namespace SonarAnalyzer.Test.Common;
+
+[TestClass]
+public class DisjointSetsTest
+{
+    [TestMethod]
+    public void FindRootAndUnion_AreConsistent()
+    {
+        var elements = Enumerable.Range(1, 5).Select(x => x.ToString());
+        var sets = new DisjointSets(elements);
+        foreach (var element in elements)
+        {
+            sets.FindRoot(element).Should().Be(element);
+        }
+
+        sets.Union("1", "1");
+        sets.FindRoot("1").Should().Be("1");                // Reflexivity
+        sets.Union("1", "2");
+        sets.FindRoot("1").Should().Be(sets.FindRoot("2")); // Correctness
+        sets.Union("1", "2");
+        sets.FindRoot("1").Should().Be(sets.FindRoot("2")); // Idempotency
+        sets.Union("2", "1");
+        sets.FindRoot("1").Should().Be(sets.FindRoot("2")); // Symmetry
+
+        sets.FindRoot("3").Should().Be("3");
+        sets.Union("2", "3");
+        sets.FindRoot("2").Should().Be(sets.FindRoot("3"));
+        sets.FindRoot("1").Should().Be(sets.FindRoot("3")); // Transitivity
+        sets.Union("3", "4");
+        sets.FindRoot("1").Should().Be(sets.FindRoot("4")); // Double transitivity
+        sets.Union("4", "1");
+        sets.FindRoot("4").Should().Be(sets.FindRoot("1")); // Idempotency after transitivity
+    }
+
+    [TestMethod]
+    public void GetAllSetsAndUnion_AreConsistent()
+    {
+        var sets = new DisjointSets(Enumerable.Range(1, 5).Select(x => x.ToString()));
+        AssertSets([["1"], ["2"], ["3"], ["4"], ["5"]], sets); // Initial state
+        sets.Union("1", "2");
+        AssertSets([["1", "2"], ["3"], ["4"], ["5"]], sets);   // Correctness
+        sets.Union("1", "2");
+        AssertSets([["1", "2"], ["3"], ["4"], ["5"]], sets);   // Idempotency
+
+        sets.Union("2", "3");
+        AssertSets([["1", "2", "3"], ["4"], ["5"]], sets);     // Transitivity
+        sets.Union("1", "3");
+        AssertSets([["1", "2", "3"], ["4"], ["5"]], sets);     // Idempotency after transitivity
+
+        sets.Union("4", "5");
+        AssertSets([["1", "2", "3"], ["4", "5"]], sets);       // Separated trees
+        sets.Union("3", "4");
+        AssertSets([["1", "2", "3", "4", "5"]], sets);         // Merged trees
+    }
+
+    [TestMethod]
+    public void GetAllSetsAndUnion_OfNestedTrees()
+    {
+        var sets = new DisjointSets(Enumerable.Range(1, 6).Select(x => x.ToString()));
+        sets.Union("1", "2");
+        sets.Union("3", "4");
+        sets.Union("5", "6");
+        AssertSets([["1", "2"], ["3", "4"], ["5", "6"]], sets); // Merge of 1-height trees
+        sets.Union("2", "3");
+        AssertSets([["1", "2", "3", "4"], ["5", "6"]], sets);   // Merge of 2-height trees
+        sets.Union("4", "5");
+        AssertSets([["1", "2", "3", "4", "5", "6"]], sets);     // Merge of 1-height tree and 2-height tree
+    }
+
+    private static void AssertSets(List<List<string>> expected, DisjointSets sets) =>
+      sets.GetAllSets().Should().BeEquivalentTo(expected);
+}