From 67d82849d304bf2c6ab3e70dbda4c3303464a675 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Tue, 14 Feb 2023 17:08:38 +0100 Subject: [PATCH 01/19] Initial scaffolding --- analyzers/rspec/cs/S3398_c#.html | 49 +++++++++++++++++++ analyzers/rspec/cs/S3398_c#.json | 17 +++++++ analyzers/rspec/cs/Sonar_way_profile.json | 1 + .../PrivateMethodUsedOnlyByInnerClass.cs | 43 ++++++++++++++++ .../PackagingTests/RuleTypeMappingCS.cs | 2 +- .../PrivateMethodUsedOnlyByInnerClassTest.cs | 33 +++++++++++++ .../PrivateMethodUsedOnlyByInnerClass.cs | 5 ++ 7 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 analyzers/rspec/cs/S3398_c#.html create mode 100644 analyzers/rspec/cs/S3398_c#.json create mode 100644 analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateMethodUsedOnlyByInnerClass.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/Rules/PrivateMethodUsedOnlyByInnerClassTest.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateMethodUsedOnlyByInnerClass.cs diff --git a/analyzers/rspec/cs/S3398_c#.html b/analyzers/rspec/cs/S3398_c#.html new file mode 100644 index 00000000000..0023954c5af --- /dev/null +++ b/analyzers/rspec/cs/S3398_c#.html @@ -0,0 +1,49 @@ +

When a private method is only invoked by an inner class, there’s no reason not to move it into that class. It will still have the same +access to the outer class' members, but the outer class will be clearer and less cluttered.

+

Noncompliant Code Example

+
+public class Outer
+{
+    public void OuterMethod()
+    {
+        Console.WriteLine("Called from an outer method.");
+    }
+
+    private static void Print(int num) // Noncompliant - method is only used by inner class, should be moved there
+    {
+        Console.WriteLine(num);
+    }
+
+    public class Inner
+    {
+        public void SomeMethod()
+        {
+            Outer.Print(42);
+        }
+    }
+}
+
+

Compliant Solution

+
+public class Outer
+{
+    public void OuterMethod()
+    {
+        Console.WriteLine("Called from an outer method.");
+    }
+
+    public class Inner
+    {
+        public void SomeMethod()
+        {
+            Print(42);
+        }
+
+        private static void Print(int num)
+        {
+            Console.WriteLine(num);
+        }
+    }
+}
+
+ diff --git a/analyzers/rspec/cs/S3398_c#.json b/analyzers/rspec/cs/S3398_c#.json new file mode 100644 index 00000000000..ef177d698f6 --- /dev/null +++ b/analyzers/rspec/cs/S3398_c#.json @@ -0,0 +1,17 @@ +{ + "title": "\"private\" methods called only by inner classes should be moved to those classes", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "confusing" + ], + "defaultSeverity": "Minor", + "ruleSpecification": "RSPEC-3398", + "sqKey": "S3398", + "scope": "All", + "quickfix": "unknown" +} diff --git a/analyzers/rspec/cs/Sonar_way_profile.json b/analyzers/rspec/cs/Sonar_way_profile.json index 2b7c15c5eb5..51ab2633592 100644 --- a/analyzers/rspec/cs/Sonar_way_profile.json +++ b/analyzers/rspec/cs/Sonar_way_profile.json @@ -157,6 +157,7 @@ "S3358", "S3376", "S3397", + "S3398", "S3400", "S3415", "S3427", diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateMethodUsedOnlyByInnerClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateMethodUsedOnlyByInnerClass.cs new file mode 100644 index 00000000000..3e6c2ccf6d5 --- /dev/null +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateMethodUsedOnlyByInnerClass.cs @@ -0,0 +1,43 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2023 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.Rules.CSharp; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class PrivateMethodUsedOnlyByInnerClass : SonarDiagnosticAnalyzer +{ + private const string DiagnosticId = "S3398"; + private const string MessageFormat = "Move '{0}' inside '{1}'."; + + private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + protected override void Initialize(SonarAnalysisContext context) => + context.RegisterNodeAction(c => + { + var node = c.Node; + if (true) + { + c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation(), "MethodName", "InnerClassName")); + } + }, + SyntaxKind.MethodDeclaration); +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs index 4dc49032b0f..e6315ee10ac 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs @@ -3322,7 +3322,7 @@ internal static class RuleTypeMappingCS // ["S3395"], // ["S3396"], ["S3397"] = "BUG", - // ["S3398"], + ["S3398"] = "CODE_SMELL", // ["S3399"], ["S3400"] = "CODE_SMELL", // ["S3401"], diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/PrivateMethodUsedOnlyByInnerClassTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/PrivateMethodUsedOnlyByInnerClassTest.cs new file mode 100644 index 00000000000..e741d2c1f5c --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/PrivateMethodUsedOnlyByInnerClassTest.cs @@ -0,0 +1,33 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2023 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 SonarAnalyzer.Rules.CSharp; + +namespace SonarAnalyzer.UnitTest.Rules; + +[TestClass] +public class PrivateMethodUsedOnlyByInnerClassTest +{ + private readonly VerifierBuilder builder = new VerifierBuilder(); + + [TestMethod] + public void PrivateMethodUsedOnlyByInnerClass_CS() => + builder.AddPaths("PrivateMethodUsedOnlyByInnerClass.cs").Verify(); +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateMethodUsedOnlyByInnerClass.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateMethodUsedOnlyByInnerClass.cs new file mode 100644 index 00000000000..a790d6d1f92 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateMethodUsedOnlyByInnerClass.cs @@ -0,0 +1,5 @@ +using System; + +public class Program +{ +} From 2740e34de22a58aa47be03ace56f7c2e28b79de1 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Sat, 18 Feb 2023 12:22:37 +0100 Subject: [PATCH 02/19] Add common test cases --- .../PrivateMethodUsedOnlyByInnerClass.cs | 91 ++++++++++++++++++- 1 file changed, 89 insertions(+), 2 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateMethodUsedOnlyByInnerClass.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateMethodUsedOnlyByInnerClass.cs index a790d6d1f92..d15479e9179 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateMethodUsedOnlyByInnerClass.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateMethodUsedOnlyByInnerClass.cs @@ -1,5 +1,92 @@ -using System; +using System.Runtime.InteropServices; -public class Program +class OuterClass { + static void OnlyUsedOnceByNestedClass() { } // Noncompliant {{Move the method inside 'NestedClass'.}} + // ^^^^^^^^^^^^^^^^^^^^^^^^^ + static void OnlyUsedByNestedClassMultipleTimes() { } // Noncompliant + static void OnlyUsedByNestedClassWithClassName() { } // Noncompliant + static void UsedByMultipleSiblingNestedClasses() { } // Compliant - it needs to stay in the outer class + static void UsedByOuterAndNestedClasses() { } // Compliant - it's used by the outer class, so it needs to stay there + static void UsedBySiblingAndChildClasses() { } // Compliant - SiblingNestedClass and DeeperNestedClass both need access to the method, so it must stay in the outer class + + static void OnlyUsedByDeeperNestedClass() { } // Noncompliant {{Move the method inside 'DeeperNestedClass'.}} + static void UsedByNestedClassAndDeeperNestedClass() { } // Noncompliant {{Move the method inside 'NestedClass'.}} + + void NotStatic() { } // Compliant - not static + + public static void PublicMethod() { } // Compliant - not private + protected static void ProtectedMethod() { } // Compliant - not private + internal static void InternalMethod() { } // Compliant - not private + protected internal static void ProtectedInternalMethod() { } // Compliant - not private + private static void PrivateMethod() { } // Noncompliant + + static T GenericMethod(T arg) => arg; // Noncompliant + // ^^^^^^^^^^^^^ + + static int Recursive(int n) => Recursive(n - 1) // Noncompliant + static void MutuallyRecursive1() => MutuallyRecursive2(); // FN - both methods could be moved inisde the nested class + static void MutuallyRecursive2() => MutuallyRecursive1(); + + [DllImport("SomeLibrary.dll")] + private static extern void ExternalMethod(); // Noncompliant + + void Foo() + { + UsedByOuterAndNestedClasses(); + } + + class NestedClass + { + void Bar() + { + OnlyUsedOnceByNestedClass(); + OnlyUsedByNestedClassMultipleTimes(); + OuterClass.OnlyUsedByNestedClassWithClassName(); + UsedByMultipleSiblingNestedClasses(); + UsedByOuterAndNestedClasses(); + UsedByNestedClassAndDeeperNestedClass(); + new OuterClass().NotStatic(); + PrivateMethod(); + + GenericMethod(42); + + Recursive(42); + MutuallyRecursive1(); + + ExternalMethod(); + } + + void FooBaz() + { + OnlyUsedByNestedClassMultipleTimes(); + } + + class DeeperNestedClass + { + void FooBar() + { + OnlyUsedByDeeperNestedClass(); + UsedByNestedClassAndDeeperNestedClass(); + UsedBySiblingAndChildClasses(); + } + } + } + + class SiblingNestedClass + { + void Baz() + { + UsedByMultipleSiblingNestedClasses(); + UsedBySiblingAndChildClasses(); + } + } } + +// C# 11 - file modifier +// extern +// recursion +// partial +// records +// static classes +// generics From 2967a02741e88060a2244a78ecc606019231bee6 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Sun, 19 Feb 2023 00:31:01 +0100 Subject: [PATCH 03/19] Add more test cases --- ...ivateStaticMethodUsedOnlyByNestedClass.cs} | 6 +- .../PrivateMethodUsedOnlyByInnerClassTest.cs | 33 ---- ...teStaticMethodUsedOnlyByNestedClassTest.cs | 60 +++++++ .../PrivateMethodUsedOnlyByInnerClass.cs | 92 ----------- ...ticMethodUsedOnlyByNestedClass.CSharp10.cs | 43 +++++ ...aticMethodUsedOnlyByNestedClass.CSharp8.cs | 27 +++ ...aticMethodUsedOnlyByNestedClass.CSharp9.cs | 21 +++ ...rivateStaticMethodUsedOnlyByNestedClass.cs | 156 ++++++++++++++++++ 8 files changed, 310 insertions(+), 128 deletions(-) rename analyzers/src/SonarAnalyzer.CSharp/Rules/{PrivateMethodUsedOnlyByInnerClass.cs => PrivateStaticMethodUsedOnlyByNestedClass.cs} (88%) delete mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/Rules/PrivateMethodUsedOnlyByInnerClassTest.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/Rules/PrivateStaticMethodUsedOnlyByNestedClassTest.cs delete mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateMethodUsedOnlyByInnerClass.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp10.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp8.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp9.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateMethodUsedOnlyByInnerClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs similarity index 88% rename from analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateMethodUsedOnlyByInnerClass.cs rename to analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs index 3e6c2ccf6d5..958c3ad5b32 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateMethodUsedOnlyByInnerClass.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -21,10 +21,10 @@ namespace SonarAnalyzer.Rules.CSharp; [DiagnosticAnalyzer(LanguageNames.CSharp)] -public sealed class PrivateMethodUsedOnlyByInnerClass : SonarDiagnosticAnalyzer +public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAnalyzer { private const string DiagnosticId = "S3398"; - private const string MessageFormat = "Move '{0}' inside '{1}'."; + private const string MessageFormat = "Move the method inside '{1}'."; private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); @@ -36,7 +36,7 @@ public sealed class PrivateMethodUsedOnlyByInnerClass : SonarDiagnosticAnalyzer var node = c.Node; if (true) { - c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation(), "MethodName", "InnerClassName")); + c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation(), "InnerClassName")); } }, SyntaxKind.MethodDeclaration); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/PrivateMethodUsedOnlyByInnerClassTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/PrivateMethodUsedOnlyByInnerClassTest.cs deleted file mode 100644 index e741d2c1f5c..00000000000 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/PrivateMethodUsedOnlyByInnerClassTest.cs +++ /dev/null @@ -1,33 +0,0 @@ -/* - * SonarAnalyzer for .NET - * Copyright (C) 2015-2023 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 SonarAnalyzer.Rules.CSharp; - -namespace SonarAnalyzer.UnitTest.Rules; - -[TestClass] -public class PrivateMethodUsedOnlyByInnerClassTest -{ - private readonly VerifierBuilder builder = new VerifierBuilder(); - - [TestMethod] - public void PrivateMethodUsedOnlyByInnerClass_CS() => - builder.AddPaths("PrivateMethodUsedOnlyByInnerClass.cs").Verify(); -} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/PrivateStaticMethodUsedOnlyByNestedClassTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/PrivateStaticMethodUsedOnlyByNestedClassTest.cs new file mode 100644 index 00000000000..26f32cea041 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/PrivateStaticMethodUsedOnlyByNestedClassTest.cs @@ -0,0 +1,60 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2023 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 SonarAnalyzer.Rules.CSharp; + +namespace SonarAnalyzer.UnitTest.Rules; + +[TestClass] +public class PrivateStaticMethodUsedOnlyByNestedClassTest +{ + private readonly VerifierBuilder builder = new VerifierBuilder(); + + [TestMethod] + public void PrivateStaticMethodUsedOnlyByNestedClass_CS() => + builder + .AddPaths("PrivateStaticMethodUsedOnlyByNestedClass.cs") + .Verify(); + +#if NET + + [TestMethod] + public void PrivateStaticMethodUsedOnlyByNestedClass_CSharp8() => + builder + .AddPaths("PrivateStaticMethodUsedOnlyByNestedClass.CSharp8.cs") + .WithOptions(ParseOptionsHelper.FromCSharp8) + .Verify(); + +#endif + + [TestMethod] + public void PrivateStaticMethodUsedOnlyByNestedClass_CSharp9() => + builder + .AddPaths("PrivateStaticMethodUsedOnlyByNestedClass.CSharp9.cs") + .WithOptions(ParseOptionsHelper.FromCSharp9) + .Verify(); + + [TestMethod] + public void PrivateStaticMethodUsedOnlyByNestedClass_CSharp10() => + builder + .AddPaths("PrivateStaticMethodUsedOnlyByNestedClass.CSharp10.cs") + .WithOptions(ParseOptionsHelper.FromCSharp10) + .Verify(); +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateMethodUsedOnlyByInnerClass.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateMethodUsedOnlyByInnerClass.cs deleted file mode 100644 index d15479e9179..00000000000 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateMethodUsedOnlyByInnerClass.cs +++ /dev/null @@ -1,92 +0,0 @@ -using System.Runtime.InteropServices; - -class OuterClass -{ - static void OnlyUsedOnceByNestedClass() { } // Noncompliant {{Move the method inside 'NestedClass'.}} - // ^^^^^^^^^^^^^^^^^^^^^^^^^ - static void OnlyUsedByNestedClassMultipleTimes() { } // Noncompliant - static void OnlyUsedByNestedClassWithClassName() { } // Noncompliant - static void UsedByMultipleSiblingNestedClasses() { } // Compliant - it needs to stay in the outer class - static void UsedByOuterAndNestedClasses() { } // Compliant - it's used by the outer class, so it needs to stay there - static void UsedBySiblingAndChildClasses() { } // Compliant - SiblingNestedClass and DeeperNestedClass both need access to the method, so it must stay in the outer class - - static void OnlyUsedByDeeperNestedClass() { } // Noncompliant {{Move the method inside 'DeeperNestedClass'.}} - static void UsedByNestedClassAndDeeperNestedClass() { } // Noncompliant {{Move the method inside 'NestedClass'.}} - - void NotStatic() { } // Compliant - not static - - public static void PublicMethod() { } // Compliant - not private - protected static void ProtectedMethod() { } // Compliant - not private - internal static void InternalMethod() { } // Compliant - not private - protected internal static void ProtectedInternalMethod() { } // Compliant - not private - private static void PrivateMethod() { } // Noncompliant - - static T GenericMethod(T arg) => arg; // Noncompliant - // ^^^^^^^^^^^^^ - - static int Recursive(int n) => Recursive(n - 1) // Noncompliant - static void MutuallyRecursive1() => MutuallyRecursive2(); // FN - both methods could be moved inisde the nested class - static void MutuallyRecursive2() => MutuallyRecursive1(); - - [DllImport("SomeLibrary.dll")] - private static extern void ExternalMethod(); // Noncompliant - - void Foo() - { - UsedByOuterAndNestedClasses(); - } - - class NestedClass - { - void Bar() - { - OnlyUsedOnceByNestedClass(); - OnlyUsedByNestedClassMultipleTimes(); - OuterClass.OnlyUsedByNestedClassWithClassName(); - UsedByMultipleSiblingNestedClasses(); - UsedByOuterAndNestedClasses(); - UsedByNestedClassAndDeeperNestedClass(); - new OuterClass().NotStatic(); - PrivateMethod(); - - GenericMethod(42); - - Recursive(42); - MutuallyRecursive1(); - - ExternalMethod(); - } - - void FooBaz() - { - OnlyUsedByNestedClassMultipleTimes(); - } - - class DeeperNestedClass - { - void FooBar() - { - OnlyUsedByDeeperNestedClass(); - UsedByNestedClassAndDeeperNestedClass(); - UsedBySiblingAndChildClasses(); - } - } - } - - class SiblingNestedClass - { - void Baz() - { - UsedByMultipleSiblingNestedClasses(); - UsedBySiblingAndChildClasses(); - } - } -} - -// C# 11 - file modifier -// extern -// recursion -// partial -// records -// static classes -// generics diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp10.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp10.cs new file mode 100644 index 00000000000..ce3ddeb38f9 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp10.cs @@ -0,0 +1,43 @@ +record class OuterRecordClass +{ + static void UsedOnlyByNestedClass() { } // Noncompliant + static void UsedOnlyByNestedRecord() { } // Noncompliant + + class NestedClass + { + void Foo() + { + UsedOnlyByNestedClass(); + } + } + + record class NestedRecord + { + void Foo() + { + UsedOnlyByNestedRecord(); + } + } +} + +record struct OuterRecordStruct +{ + static void UsedOnlyByNestedClass() { } // Noncompliant + static void UsedOnlyByNestedRecord() { } // Noncompliant + + class NestedClass + { + void Foo() + { + UsedOnlyByNestedClass(); + } + } + + record struct NestedRecord + { + void Foo() + { + UsedOnlyByNestedRecord(); + } + } +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp8.cs new file mode 100644 index 00000000000..53c6b418f0c --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp8.cs @@ -0,0 +1,27 @@ +class OuterClass +{ + static void OnlyUsedByNestedInterface() { } // Noncompliant + private protected static void PrivateProtectedMethod() { } // Compliant - method is not private + + interface INestedInterface + { + void Foo() + { + OnlyUsedByNestedInterface(); + PrivateProtectedMethod(); + } + } +} + +interface IOuterInterface +{ + static void OnlyUsedByNestedClass() { } // Noncompliant + + class NestedClass + { + void Foo() + { + OnlyUsedByNestedClass(); + } + } +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp9.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp9.cs new file mode 100644 index 00000000000..177ecbf9f69 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp9.cs @@ -0,0 +1,21 @@ +record OuterRecord +{ + static void UsedOnlyByNestedClass() { } // Noncompliant + static void UsedOnlyByNestedRecord() { } // Noncompliant + + class NestedClass + { + void Foo() + { + UsedOnlyByNestedClass(); + } + } + + record NestedRecord + { + void Foo() + { + UsedOnlyByNestedRecord(); + } + } +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs new file mode 100644 index 00000000000..e3099c5cf93 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -0,0 +1,156 @@ +using System.Runtime.InteropServices; + +class OuterClass +{ + static void OnlyUsedOnceByNestedClass() { } // Noncompliant {{Move the method inside 'NestedClass'.}} + // ^^^^^^^^^^^^^^^^^^^^^^^^^ + static void OnlyUsedByNestedClassMultipleTimes() { } // Noncompliant + static void OnlyUsedByNestedClassWithClassName() { } // Noncompliant + static void UsedByMultipleSiblingNestedClasses() { } // Compliant - it needs to stay in the outer class + static void UsedByOuterAndNestedClasses() { } // Compliant - it's used by the outer class, so it needs to stay there + static void UsedBySiblingAndDeeperNestedClasses() { } // Compliant - SiblingNestedClass and DeeperNestedClass both need access to the method, so it must stay in the outer class + static void OnlyUsedByDeeperNestedClass() { } // Noncompliant {{Move the method inside 'DeeperNestedClass'.}} + static void UsedByNestedClassAndDeeperNestedClass() { } // Noncompliant {{Move the method inside 'NestedClass'.}} + static void UnusedMethod() { } // Compliant - no need to move unused method anywhere + + void NotStatic() { } // Compliant - method is not static + + public static void PublicMethod() { } // Compliant - method is not private + protected static void ProtectedMethod() { } // Compliant - method is not private + internal static void InternalMethod() { } // Compliant - method is not private + protected internal static void ProtectedInternalMethod() { } // Compliant - method is not private + private static void PrivateMethod() { } // Noncompliant + + static T GenericMethod(T arg) => arg; // Noncompliant + // ^^^^^^^^^^^^^ + + static int Recursive(int n) => Recursive(n - 1); // Noncompliant + static void MutuallyRecursive1() => MutuallyRecursive2(); // FN - both methods could be moved inisde the nested class + static void MutuallyRecursive2() => MutuallyRecursive1(); + + [DllImport("SomeLibrary.dll")] + private static extern void ExternalMethod(); // Noncompliant + + static int UsedOnlyByPropertyInNestedClass() => 42; // Noncompliant + static int UsedOnlyByFieldInitializerInNestedClass() => 42; // Noncompliant + static void UsedOnlyByConstructorInNestedClass() { } // Noncompliant + + void Foo() + { + UsedByOuterAndNestedClasses(); + } + + class NestedClass + { + int _field = UsedOnlyByFieldInitializerInNestedClass(); + + int Prop => UsedOnlyByPropertyInNestedClass(); + + public NestedClass() + { + UsedOnlyByConstructorInNestedClass(); + } + + static void OnlyUsedByDeeperNestedClass() { } // Noncompliant {{Move the method inside 'DeeperNestedClass'.}} + + void Bar() + { + OnlyUsedOnceByNestedClass(); + OnlyUsedByNestedClassMultipleTimes(); + OuterClass.OnlyUsedByNestedClassWithClassName(); + UsedByMultipleSiblingNestedClasses(); + UsedByOuterAndNestedClasses(); + UsedByNestedClassAndDeeperNestedClass(); + + new OuterClass().NotStatic(); + + PublicMethod(); + ProtectedMethod(); + InternalMethod(); + ProtectedInternalMethod(); + PrivateMethod(); + + GenericMethod(42); + + Recursive(42); + MutuallyRecursive1(); + + ExternalMethod(); + } + + void FooBaz() + { + OnlyUsedByNestedClassMultipleTimes(); + } + + class DeeperNestedClass + { + void FooBar() + { + OnlyUsedByDeeperNestedClass(); + UsedByNestedClassAndDeeperNestedClass(); + UsedBySiblingAndDeeperNestedClasses(); + OnlyUsedByDeeperNestedClass(); + } + } + } + + class SiblingNestedClass + { + void Baz() + { + UsedByMultipleSiblingNestedClasses(); + UsedBySiblingAndDeeperNestedClasses(); + } + } +} + +class ClassContainsStruct +{ + static void OnlyUsedByNestedStruct() { } // Noncompliant + + struct NestedStruct + { + void Foo() + { + OnlyUsedByNestedStruct(); + } + } +} + +struct StructContainsClass +{ + static void OnlyUsedByNestedClass() { } // Noncompliant + + class NestedClass + { + void Foo() + { + OnlyUsedByNestedClass(); + } + } +} + +partial class PartialOuterClass +{ + static void OnlyUsedByNestedClass() { } // Noncompliant + static partial void PartialOnlyUsedByNestedClass() { } // Noncompliant +} + +partial class PartialOuterClass +{ + static partial void PartialOnlyUsedByNestedClass(); + + class NestedClass + { + void Foo() + { + OnlyUsedByNestedClass(); + PartialOnlyUsedByNestedClass(); + } + } +} + +// static classes +// nameof +// top-level methods From af50177045177916394018df2e69be94dae09ec1 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Sun, 19 Feb 2023 11:23:13 +0100 Subject: [PATCH 04/19] More compliant test cases. --- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs index e3099c5cf93..019a1a0ad8d 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -14,6 +14,8 @@ class OuterClass static void UnusedMethod() { } // Compliant - no need to move unused method anywhere void NotStatic() { } // Compliant - method is not static + static int _outerField; // Compliant - not a method + static int OuterProp { get; set; } // Compliant - not a method public static void PublicMethod() { } // Compliant - method is not private protected static void ProtectedMethod() { } // Compliant - method is not private @@ -42,16 +44,15 @@ void Foo() class NestedClass { - int _field = UsedOnlyByFieldInitializerInNestedClass(); - - int Prop => UsedOnlyByPropertyInNestedClass(); + int _nestedField = UsedOnlyByFieldInitializerInNestedClass(); + int NestedProp => UsedOnlyByPropertyInNestedClass(); public NestedClass() { UsedOnlyByConstructorInNestedClass(); } - static void OnlyUsedByDeeperNestedClass() { } // Noncompliant {{Move the method inside 'DeeperNestedClass'.}} + static void OnlyUsedByDeeperNestedClass() { } // Noncompliant {{Move the method inside 'DeeperNestedClass'.}} void Bar() { @@ -63,6 +64,8 @@ void Bar() UsedByNestedClassAndDeeperNestedClass(); new OuterClass().NotStatic(); + _outerField = 42; + OuterProp = 42; PublicMethod(); ProtectedMethod(); @@ -107,7 +110,7 @@ void Baz() class ClassContainsStruct { - static void OnlyUsedByNestedStruct() { } // Noncompliant + static void OnlyUsedByNestedStruct() { } // Noncompliant struct NestedStruct { @@ -120,7 +123,7 @@ void Foo() struct StructContainsClass { - static void OnlyUsedByNestedClass() { } // Noncompliant + static void OnlyUsedByNestedClass() { } // Noncompliant class NestedClass { @@ -133,8 +136,8 @@ void Foo() partial class PartialOuterClass { - static void OnlyUsedByNestedClass() { } // Noncompliant - static partial void PartialOnlyUsedByNestedClass() { } // Noncompliant + static void OnlyUsedByNestedClass() { } // Noncompliant + static partial void PartialOnlyUsedByNestedClass() { } // Noncompliant } partial class PartialOuterClass @@ -150,7 +153,3 @@ void Foo() } } } - -// static classes -// nameof -// top-level methods From 673f13d996f0fdef07f20834b1372355dca7de40 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Tue, 21 Feb 2023 16:32:53 +0100 Subject: [PATCH 05/19] Add SyntaxWalker --- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 68 +++++++++++++++++-- ...ticMethodUsedOnlyByNestedClass.CSharp10.cs | 4 +- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 16 +++-- 3 files changed, 78 insertions(+), 10 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs index 958c3ad5b32..8bb11d7c45f 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -26,6 +26,15 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn private const string DiagnosticId = "S3398"; private const string MessageFormat = "Move the method inside '{1}'."; + private static readonly SyntaxKind[] AnalyzedSyntaxKinds = new[] + { + SyntaxKind.ClassDeclaration, + SyntaxKind.StructDeclaration, + SyntaxKind.InterfaceDeclaration, + SyntaxKindEx.RecordClassDeclaration, + SyntaxKindEx.RecordStructDeclaration + }; + private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); @@ -33,11 +42,62 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn protected override void Initialize(SonarAnalysisContext context) => context.RegisterNodeAction(c => { - var node = c.Node; - if (true) + var declaredType = (TypeDeclarationSyntax)c.Node; + + if (!IsPartial(declaredType) + && PrivateStaticMethodsOf(declaredType) is { Length: > 0 } candidates + && NestedTypeDeclarationsOf(declaredType) is { Length: > 0} nestedTypes) { - c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation(), "InnerClassName")); + c.ReportIssue(Diagnostic.Create(Rule, declaredType.GetLocation(), "InnerClassName")); } }, - SyntaxKind.MethodDeclaration); + AnalyzedSyntaxKinds); + + private static MethodDeclarationSyntax[] PrivateStaticMethodsOf(TypeDeclarationSyntax type) => + type.Members + .OfType() + .Where(x => IsPrivateAndStatic(x, type)) + .ToArray(); + + private static TypeDeclarationSyntax[] NestedTypeDeclarationsOf(TypeDeclarationSyntax type) => + type.Members + .OfType() + .Where(x => x is ClassDeclarationSyntax or StructDeclarationSyntax or InterfaceDeclarationSyntax + || RecordDeclarationSyntaxWrapper.IsInstance(x)) + .ToArray(); + + private static bool IsPrivateAndStatic(MethodDeclarationSyntax method, TypeDeclarationSyntax containingType) => + method.Modifiers.Any(x => x.IsKind(SyntaxKind.StaticKeyword)) + && ((HasAnyModifier(method, SyntaxKind.PrivateKeyword) && !HasAnyModifier(method, SyntaxKind.ProtectedKeyword)) + || (!HasAnyModifier(method, SyntaxKind.PublicKeyword, SyntaxKind.ProtectedKeyword, SyntaxKind.InternalKeyword) && IsClassOrRecordClassOrInterfaceDeclaration(containingType))); + + private static bool IsPartial(TypeDeclarationSyntax type) => + type.Modifiers.Any(x => x.IsKind(SyntaxKind.PartialKeyword)); + + private static bool IsClassOrRecordClassOrInterfaceDeclaration(TypeDeclarationSyntax type) => + type is ClassDeclarationSyntax or InterfaceDeclarationSyntax + || (RecordDeclarationSyntaxWrapper.IsInstance(type) && !((RecordDeclarationSyntaxWrapper)type).ClassOrStructKeyword.IsKind(SyntaxKind.StructKeyword)); + + private static bool HasAnyModifier(MethodDeclarationSyntax method, params SyntaxKind[] modifiers) => + method.Modifiers.Any(x => x.IsAnyKind(modifiers)); + + private class PotentialMethodReferenceCollector : CSharpSyntaxWalker + { + private readonly ISet methodsToFind; + internal Dictionary> PotentialMethodReferences { get; } = new(); + + public PotentialMethodReferenceCollector(IEnumerable methodsToFind) + { + this.methodsToFind = new HashSet(methodsToFind); + } + + public override void VisitIdentifierName(IdentifierNameSyntax identifier) + { + if (methodsToFind.FirstOrDefault(x => x.Identifier.ValueText == identifier.Identifier.ValueText) is { } method) + { + var referenceList = PotentialMethodReferences.GetOrAdd(method, _ => new()); + referenceList.Add(identifier); + } + } + } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp10.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp10.cs index ce3ddeb38f9..3c9aa3e079f 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp10.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.CSharp10.cs @@ -22,8 +22,8 @@ void Foo() record struct OuterRecordStruct { - static void UsedOnlyByNestedClass() { } // Noncompliant - static void UsedOnlyByNestedRecord() { } // Noncompliant + private static void UsedOnlyByNestedClass() { } // Noncompliant + private static void UsedOnlyByNestedRecord() { } // Noncompliant class NestedClass { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs index 019a1a0ad8d..d5cb9f9e42d 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -1,4 +1,5 @@ -using System.Runtime.InteropServices; +using System; +using System.Runtime.InteropServices; class OuterClass { @@ -36,6 +37,8 @@ class OuterClass static int UsedOnlyByPropertyInNestedClass() => 42; // Noncompliant static int UsedOnlyByFieldInitializerInNestedClass() => 42; // Noncompliant static void UsedOnlyByConstructorInNestedClass() { } // Noncompliant + static void AssignedToDelegateInNestedClass() { } // Noncompliant + static void UsedInNameOfExpressionInNestedClass() { } // Noncompliant void Foo() { @@ -79,6 +82,9 @@ void Bar() MutuallyRecursive1(); ExternalMethod(); + + Action methodDelegate = AssignedToDelegateInNestedClass; + string methodName = nameof(UsedInNameOfExpressionInNestedClass); } void FooBaz() @@ -123,7 +129,7 @@ void Foo() struct StructContainsClass { - static void OnlyUsedByNestedClass() { } // Noncompliant + private static void OnlyUsedByNestedClass() { } // Noncompliant class NestedClass { @@ -136,8 +142,8 @@ void Foo() partial class PartialOuterClass { - static void OnlyUsedByNestedClass() { } // Noncompliant - static partial void PartialOnlyUsedByNestedClass() { } // Noncompliant + static void OnlyUsedByNestedClass() { } // Compliant - partial classes are often a result of code generation, so their methods shouldn't be moved + static partial void PartialOnlyUsedByNestedClass() { } // Compliant } partial class PartialOuterClass @@ -153,3 +159,5 @@ void Foo() } } } + +// attributes - DebuggerDisplay From 041a92b83f4346286f6ce212a7d513d831e4dab4 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Wed, 22 Feb 2023 16:20:10 +0100 Subject: [PATCH 06/19] Draft --- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 31 ++++++++++++++++--- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 17 +++++++++- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs index 8bb11d7c45f..90566d256c9 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -45,9 +45,20 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn var declaredType = (TypeDeclarationSyntax)c.Node; if (!IsPartial(declaredType) - && PrivateStaticMethodsOf(declaredType) is { Length: > 0 } candidates - && NestedTypeDeclarationsOf(declaredType) is { Length: > 0} nestedTypes) + && NestedTypeDeclarationsOf(declaredType) is { Length: > 0 } nestedTypes + && PrivateStaticMethodsOf(declaredType) is { Length: > 0 } candidates) { + var references = PotentialReferencesOfMethodsInsideType(candidates, declaredType) + .Where(x => x.Value.Any(type => type.Item2 != declaredType)) + .Select(x => (x.Key, c.SemanticModel.GetSymbolInfo(x.Key), x.Value.Select(t => c.SemanticModel.GetSymbolInfo(t.Item1)))); + foreach (var reference in references) + { + var actualMethodReferences = reference.Item3.Where(x => x.Symbol == reference.Item2.Symbol).ToArray(); + // scenario 1: used in outer class + // scenario 2: used only in one of the nested classes + // scenario 3: used by multiple nested classes + } + c.ReportIssue(Diagnostic.Create(Rule, declaredType.GetLocation(), "InnerClassName")); } }, @@ -81,10 +92,17 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn private static bool HasAnyModifier(MethodDeclarationSyntax method, params SyntaxKind[] modifiers) => method.Modifiers.Any(x => x.IsAnyKind(modifiers)); + private static IDictionary> PotentialReferencesOfMethodsInsideType(IEnumerable methods, TypeDeclarationSyntax type) + { + var collector = new PotentialMethodReferenceCollector(methods); + collector.Visit(type); + return collector.PotentialMethodReferences; + } + private class PotentialMethodReferenceCollector : CSharpSyntaxWalker { private readonly ISet methodsToFind; - internal Dictionary> PotentialMethodReferences { get; } = new(); + internal Dictionary> PotentialMethodReferences { get; } = new(); public PotentialMethodReferenceCollector(IEnumerable methodsToFind) { @@ -96,7 +114,12 @@ public override void VisitIdentifierName(IdentifierNameSyntax identifier) if (methodsToFind.FirstOrDefault(x => x.Identifier.ValueText == identifier.Identifier.ValueText) is { } method) { var referenceList = PotentialMethodReferences.GetOrAdd(method, _ => new()); - referenceList.Add(identifier); + referenceList.Add((identifier, ContainingTypeDeclaration(identifier))); + } + + static TypeDeclarationSyntax ContainingTypeDeclaration(IdentifierNameSyntax identifier) + { + return identifier.Ancestors().OfType().First(); } } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs index d5cb9f9e42d..e5791b9e8bc 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.Runtime.InteropServices; class OuterClass @@ -23,6 +24,7 @@ class OuterClass internal static void InternalMethod() { } // Compliant - method is not private protected internal static void ProtectedInternalMethod() { } // Compliant - method is not private private static void PrivateMethod() { } // Noncompliant + private static void PrivateMethod(int arg) { } // Compliant - overloaded version of the previous method, not used anywhere static T GenericMethod(T arg) => arg; // Noncompliant // ^^^^^^^^^^^^^ @@ -160,4 +162,17 @@ void Foo() } } -// attributes - DebuggerDisplay +[DebuggerDisplay("{UsedByDebuggerDisplay()}")] +class DebugViewClass +{ + static string UsedByDebuggerDisplay() => ""; // FP - should not be moved to nested class, because it's also used by the attribute + + class NestedClass + { + void Foo() + { + UsedByDebuggerDisplay(); + } + } +} + From 737776ef52c6aaedaf8f265f41ed37e52d3bf4df Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Fri, 24 Feb 2023 16:44:28 +0100 Subject: [PATCH 07/19] Update test cases and basic implementation --- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 45 ++++++++++++------- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 6 +-- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs index 90566d256c9..9b94f1ff8b4 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -24,7 +24,7 @@ namespace SonarAnalyzer.Rules.CSharp; public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAnalyzer { private const string DiagnosticId = "S3398"; - private const string MessageFormat = "Move the method inside '{1}'."; + private const string MessageFormat = "Move the method inside '{0}'."; private static readonly SyntaxKind[] AnalyzedSyntaxKinds = new[] { @@ -49,18 +49,30 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn && PrivateStaticMethodsOf(declaredType) is { Length: > 0 } candidates) { var references = PotentialReferencesOfMethodsInsideType(candidates, declaredType) - .Where(x => x.Value.Any(type => type.Item2 != declaredType)) - .Select(x => (x.Key, c.SemanticModel.GetSymbolInfo(x.Key), x.Value.Select(t => c.SemanticModel.GetSymbolInfo(t.Item1)))); + .Where(x => x.Value.Any(id => ContainingTypeDeclaration(id) != declaredType)) + .Select(x => new { Refs = x, MethodSymbol = c.SemanticModel.GetDeclaredSymbol(x.Key) }) + .Select(x => new { MethodDeclaration = x.Refs.Key, References = x.Refs.Value.Where(t => c.SemanticModel.GetSymbolInfo(t).Symbol == x.MethodSymbol).Select(t => new { Identifier = t, Type = ContainingTypeDeclaration(t) }) }) + .Where(x => x.References.Any()) + .ToArray(); + foreach (var reference in references) { - var actualMethodReferences = reference.Item3.Where(x => x.Symbol == reference.Item2.Symbol).ToArray(); // scenario 1: used in outer class + if (reference.References.Any(x => x.Type == declaredType)) + { + continue; + } + // scenario 2: used only in one of the nested classes + if (reference.References.Select(x => x.Type).Distinct().Count() == 1) + { + string nestedClassName = reference.References.First().Type.Identifier.ValueText; + c.ReportIssue(Diagnostic.Create(Rule, reference.MethodDeclaration.Identifier.GetLocation(), nestedClassName)); + } + // scenario 3: used by multiple nested classes } - - c.ReportIssue(Diagnostic.Create(Rule, declaredType.GetLocation(), "InnerClassName")); - } + } }, AnalyzedSyntaxKinds); @@ -92,17 +104,25 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn private static bool HasAnyModifier(MethodDeclarationSyntax method, params SyntaxKind[] modifiers) => method.Modifiers.Any(x => x.IsAnyKind(modifiers)); - private static IDictionary> PotentialReferencesOfMethodsInsideType(IEnumerable methods, TypeDeclarationSyntax type) + private static IDictionary> PotentialReferencesOfMethodsInsideType(IEnumerable methods, TypeDeclarationSyntax type) { var collector = new PotentialMethodReferenceCollector(methods); collector.Visit(type); return collector.PotentialMethodReferences; } + private static TypeDeclarationSyntax ContainingTypeDeclaration(IdentifierNameSyntax identifier) => + identifier + .Ancestors() + .OfType() + .First(); + + private record MethodAndReferences(MethodDeclarationSyntax SyntaxNode, ISymbol MethodSymbol, ISymbol[] PotentialMethodReferences); + private class PotentialMethodReferenceCollector : CSharpSyntaxWalker { private readonly ISet methodsToFind; - internal Dictionary> PotentialMethodReferences { get; } = new(); + internal Dictionary> PotentialMethodReferences { get; } = new(); public PotentialMethodReferenceCollector(IEnumerable methodsToFind) { @@ -114,12 +134,7 @@ public override void VisitIdentifierName(IdentifierNameSyntax identifier) if (methodsToFind.FirstOrDefault(x => x.Identifier.ValueText == identifier.Identifier.ValueText) is { } method) { var referenceList = PotentialMethodReferences.GetOrAdd(method, _ => new()); - referenceList.Add((identifier, ContainingTypeDeclaration(identifier))); - } - - static TypeDeclarationSyntax ContainingTypeDeclaration(IdentifierNameSyntax identifier) - { - return identifier.Ancestors().OfType().First(); + referenceList.Add(identifier); } } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs index e5791b9e8bc..21755013b49 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -57,7 +57,7 @@ public NestedClass() UsedOnlyByConstructorInNestedClass(); } - static void OnlyUsedByDeeperNestedClass() { } // Noncompliant {{Move the method inside 'DeeperNestedClass'.}} + static void NestedClassMethodUsedByDeeperNestedClass() { } // Noncompliant {{Move the method inside 'DeeperNestedClass'.}} void Bar() { @@ -101,7 +101,7 @@ void FooBar() OnlyUsedByDeeperNestedClass(); UsedByNestedClassAndDeeperNestedClass(); UsedBySiblingAndDeeperNestedClasses(); - OnlyUsedByDeeperNestedClass(); + NestedClassMethodUsedByDeeperNestedClass(); } } } @@ -165,7 +165,7 @@ void Foo() [DebuggerDisplay("{UsedByDebuggerDisplay()}")] class DebugViewClass { - static string UsedByDebuggerDisplay() => ""; // FP - should not be moved to nested class, because it's also used by the attribute + static string UsedByDebuggerDisplay() => ""; // Noncompliant - FP: should not be moved to nested class, because it's also used by the attribute class NestedClass { From 56e27c13167fed8347b226c3b3dfb728aea263d0 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Fri, 24 Feb 2023 17:29:42 +0100 Subject: [PATCH 08/19] Fix recursive method call FN --- .../Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs index 9b94f1ff8b4..363e3f0a9dd 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -51,7 +51,7 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn var references = PotentialReferencesOfMethodsInsideType(candidates, declaredType) .Where(x => x.Value.Any(id => ContainingTypeDeclaration(id) != declaredType)) .Select(x => new { Refs = x, MethodSymbol = c.SemanticModel.GetDeclaredSymbol(x.Key) }) - .Select(x => new { MethodDeclaration = x.Refs.Key, References = x.Refs.Value.Where(t => c.SemanticModel.GetSymbolInfo(t).Symbol == x.MethodSymbol).Select(t => new { Identifier = t, Type = ContainingTypeDeclaration(t) }) }) + .Select(x => new { MethodDeclaration = x.Refs.Key, References = x.Refs.Value.Where(t => c.SemanticModel.GetSymbolInfo(t).Symbol is IMethodSymbol { } methodReference && (methodReference == x.MethodSymbol || methodReference.ConstructedFrom == x.MethodSymbol) && ContainingMethodDeclaration(t) != x.Refs.Key).Select(t => new { Identifier = t, Type = ContainingTypeDeclaration(t) }) }) .Where(x => x.References.Any()) .ToArray(); @@ -117,6 +117,12 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn .OfType() .First(); + private static MethodDeclarationSyntax ContainingMethodDeclaration(IdentifierNameSyntax identifier) => + identifier + .Ancestors() + .OfType() + .FirstOrDefault(); + private record MethodAndReferences(MethodDeclarationSyntax SyntaxNode, ISymbol MethodSymbol, ISymbol[] PotentialMethodReferences); private class PotentialMethodReferenceCollector : CSharpSyntaxWalker From 45795d50c029a46c1f2345b2209ad57f3c90279b Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Sun, 26 Feb 2023 14:33:54 +0100 Subject: [PATCH 09/19] Finish parser implementation --- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 39 ++++- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 161 ------------------ 2 files changed, 31 insertions(+), 169 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs index 363e3f0a9dd..2bd71d71814 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -51,7 +51,7 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn var references = PotentialReferencesOfMethodsInsideType(candidates, declaredType) .Where(x => x.Value.Any(id => ContainingTypeDeclaration(id) != declaredType)) .Select(x => new { Refs = x, MethodSymbol = c.SemanticModel.GetDeclaredSymbol(x.Key) }) - .Select(x => new { MethodDeclaration = x.Refs.Key, References = x.Refs.Value.Where(t => c.SemanticModel.GetSymbolInfo(t).Symbol is IMethodSymbol { } methodReference && (methodReference == x.MethodSymbol || methodReference.ConstructedFrom == x.MethodSymbol) && ContainingMethodDeclaration(t) != x.Refs.Key).Select(t => new { Identifier = t, Type = ContainingTypeDeclaration(t) }) }) + .Select(x => new { MethodDeclaration = x.Refs.Key, References = x.Refs.Value.Where(t => MethodReferenceFrom(t, c.SemanticModel) is { } methodReference && (methodReference == x.MethodSymbol || methodReference.ConstructedFrom == x.MethodSymbol) && ContainingMethodDeclaration(t) != x.Refs.Key).Select(t => new { Identifier = t, Type = ContainingTypeDeclaration(t) }) }) .Where(x => x.References.Any()) .ToArray(); @@ -63,16 +63,14 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn continue; } - // scenario 2: used only in one of the nested classes - if (reference.References.Select(x => x.Type).Distinct().Count() == 1) + var typeToMoveInto = LowestCommonAncestorSelf(reference.References.Select(x => x.Type)); + if (typeToMoveInto != declaredType) { - string nestedClassName = reference.References.First().Type.Identifier.ValueText; - c.ReportIssue(Diagnostic.Create(Rule, reference.MethodDeclaration.Identifier.GetLocation(), nestedClassName)); + string nestedTypeName = typeToMoveInto.Identifier.ValueText; + c.ReportIssue(Diagnostic.Create(Rule, reference.MethodDeclaration.Identifier.GetLocation(), nestedTypeName)); } - - // scenario 3: used by multiple nested classes - } } + } }, AnalyzedSyntaxKinds); @@ -123,8 +121,33 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn .OfType() .FirstOrDefault(); + private static TypeDeclarationSyntax LowestCommonAncestorSelf(IEnumerable declaredTypes) + { + var treePaths = declaredTypes.Distinct().Select(x => x.AncestorsAndSelf().OfType().Reverse().ToArray()).ToArray(); + int minPathLength = treePaths.Select(x => x.Length).Min(); + for (int i = 0; i < minPathLength; i++) + { + if (!treePaths.All(x => x[i] == treePaths.First()[i])) + { + return treePaths.First()[i - 1]; + } + } + + return treePaths.First()[minPathLength - 1]; + } + private record MethodAndReferences(MethodDeclarationSyntax SyntaxNode, ISymbol MethodSymbol, ISymbol[] PotentialMethodReferences); + private static IMethodSymbol MethodReferenceFrom(IdentifierNameSyntax identifier, SemanticModel model) + { + var symbolInfo = model.GetSymbolInfo(identifier); + if (symbolInfo.Symbol is IMethodSymbol { } methodSymbol) + { + return methodSymbol; + } + return symbolInfo.CandidateSymbols.FirstOrDefault() as IMethodSymbol; + } + private class PotentialMethodReferenceCollector : CSharpSyntaxWalker { private readonly ISet methodsToFind; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs index 21755013b49..0e578920a4c 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -4,175 +4,14 @@ class OuterClass { - static void OnlyUsedOnceByNestedClass() { } // Noncompliant {{Move the method inside 'NestedClass'.}} - // ^^^^^^^^^^^^^^^^^^^^^^^^^ - static void OnlyUsedByNestedClassMultipleTimes() { } // Noncompliant - static void OnlyUsedByNestedClassWithClassName() { } // Noncompliant - static void UsedByMultipleSiblingNestedClasses() { } // Compliant - it needs to stay in the outer class - static void UsedByOuterAndNestedClasses() { } // Compliant - it's used by the outer class, so it needs to stay there - static void UsedBySiblingAndDeeperNestedClasses() { } // Compliant - SiblingNestedClass and DeeperNestedClass both need access to the method, so it must stay in the outer class - static void OnlyUsedByDeeperNestedClass() { } // Noncompliant {{Move the method inside 'DeeperNestedClass'.}} - static void UsedByNestedClassAndDeeperNestedClass() { } // Noncompliant {{Move the method inside 'NestedClass'.}} - static void UnusedMethod() { } // Compliant - no need to move unused method anywhere - - void NotStatic() { } // Compliant - method is not static - static int _outerField; // Compliant - not a method - static int OuterProp { get; set; } // Compliant - not a method - - public static void PublicMethod() { } // Compliant - method is not private - protected static void ProtectedMethod() { } // Compliant - method is not private - internal static void InternalMethod() { } // Compliant - method is not private - protected internal static void ProtectedInternalMethod() { } // Compliant - method is not private - private static void PrivateMethod() { } // Noncompliant - private static void PrivateMethod(int arg) { } // Compliant - overloaded version of the previous method, not used anywhere - - static T GenericMethod(T arg) => arg; // Noncompliant - // ^^^^^^^^^^^^^ - - static int Recursive(int n) => Recursive(n - 1); // Noncompliant - static void MutuallyRecursive1() => MutuallyRecursive2(); // FN - both methods could be moved inisde the nested class - static void MutuallyRecursive2() => MutuallyRecursive1(); - - [DllImport("SomeLibrary.dll")] - private static extern void ExternalMethod(); // Noncompliant - - static int UsedOnlyByPropertyInNestedClass() => 42; // Noncompliant - static int UsedOnlyByFieldInitializerInNestedClass() => 42; // Noncompliant - static void UsedOnlyByConstructorInNestedClass() { } // Noncompliant - static void AssignedToDelegateInNestedClass() { } // Noncompliant static void UsedInNameOfExpressionInNestedClass() { } // Noncompliant - void Foo() - { - UsedByOuterAndNestedClasses(); - } - class NestedClass { - int _nestedField = UsedOnlyByFieldInitializerInNestedClass(); - int NestedProp => UsedOnlyByPropertyInNestedClass(); - - public NestedClass() - { - UsedOnlyByConstructorInNestedClass(); - } - - static void NestedClassMethodUsedByDeeperNestedClass() { } // Noncompliant {{Move the method inside 'DeeperNestedClass'.}} - void Bar() { - OnlyUsedOnceByNestedClass(); - OnlyUsedByNestedClassMultipleTimes(); - OuterClass.OnlyUsedByNestedClassWithClassName(); - UsedByMultipleSiblingNestedClasses(); - UsedByOuterAndNestedClasses(); - UsedByNestedClassAndDeeperNestedClass(); - - new OuterClass().NotStatic(); - _outerField = 42; - OuterProp = 42; - - PublicMethod(); - ProtectedMethod(); - InternalMethod(); - ProtectedInternalMethod(); - PrivateMethod(); - - GenericMethod(42); - - Recursive(42); - MutuallyRecursive1(); - - ExternalMethod(); - - Action methodDelegate = AssignedToDelegateInNestedClass; string methodName = nameof(UsedInNameOfExpressionInNestedClass); } - - void FooBaz() - { - OnlyUsedByNestedClassMultipleTimes(); - } - - class DeeperNestedClass - { - void FooBar() - { - OnlyUsedByDeeperNestedClass(); - UsedByNestedClassAndDeeperNestedClass(); - UsedBySiblingAndDeeperNestedClasses(); - NestedClassMethodUsedByDeeperNestedClass(); - } - } - } - - class SiblingNestedClass - { - void Baz() - { - UsedByMultipleSiblingNestedClasses(); - UsedBySiblingAndDeeperNestedClasses(); - } - } -} - -class ClassContainsStruct -{ - static void OnlyUsedByNestedStruct() { } // Noncompliant - - struct NestedStruct - { - void Foo() - { - OnlyUsedByNestedStruct(); - } - } -} - -struct StructContainsClass -{ - private static void OnlyUsedByNestedClass() { } // Noncompliant - - class NestedClass - { - void Foo() - { - OnlyUsedByNestedClass(); - } - } -} - -partial class PartialOuterClass -{ - static void OnlyUsedByNestedClass() { } // Compliant - partial classes are often a result of code generation, so their methods shouldn't be moved - static partial void PartialOnlyUsedByNestedClass() { } // Compliant -} - -partial class PartialOuterClass -{ - static partial void PartialOnlyUsedByNestedClass(); - - class NestedClass - { - void Foo() - { - OnlyUsedByNestedClass(); - PartialOnlyUsedByNestedClass(); - } - } -} - -[DebuggerDisplay("{UsedByDebuggerDisplay()}")] -class DebugViewClass -{ - static string UsedByDebuggerDisplay() => ""; // Noncompliant - FP: should not be moved to nested class, because it's also used by the attribute - - class NestedClass - { - void Foo() - { - UsedByDebuggerDisplay(); - } } } From 3c6e935337a77266639fcf127a5f97ad107eaefe Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Mon, 27 Feb 2023 12:41:22 +0100 Subject: [PATCH 10/19] Revert test cases --- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 48 +++--- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 160 ++++++++++++++++++ 2 files changed, 180 insertions(+), 28 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs index 2bd71d71814..288470c2b7b 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -45,29 +45,18 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn var declaredType = (TypeDeclarationSyntax)c.Node; if (!IsPartial(declaredType) - && NestedTypeDeclarationsOf(declaredType) is { Length: > 0 } nestedTypes + && HasNestedTypeDeclarations(declaredType) && PrivateStaticMethodsOf(declaredType) is { Length: > 0 } candidates) { - var references = PotentialReferencesOfMethodsInsideType(candidates, declaredType) - .Where(x => x.Value.Any(id => ContainingTypeDeclaration(id) != declaredType)) - .Select(x => new { Refs = x, MethodSymbol = c.SemanticModel.GetDeclaredSymbol(x.Key) }) - .Select(x => new { MethodDeclaration = x.Refs.Key, References = x.Refs.Value.Where(t => MethodReferenceFrom(t, c.SemanticModel) is { } methodReference && (methodReference == x.MethodSymbol || methodReference.ConstructedFrom == x.MethodSymbol) && ContainingMethodDeclaration(t) != x.Refs.Key).Select(t => new { Identifier = t, Type = ContainingTypeDeclaration(t) }) }) - .Where(x => x.References.Any()) - .ToArray(); - - foreach (var reference in references) - { - // scenario 1: used in outer class - if (reference.References.Any(x => x.Type == declaredType)) - { - continue; - } + var methodReferences = MethodReferencesInsideType(candidates, declaredType, c.SemanticModel); - var typeToMoveInto = LowestCommonAncestorSelf(reference.References.Select(x => x.Type)); + foreach (var reference in methodReferences) + { + var typeToMoveInto = LowestCommonAncestorOrSelf(reference.Value); if (typeToMoveInto != declaredType) { string nestedTypeName = typeToMoveInto.Identifier.ValueText; - c.ReportIssue(Diagnostic.Create(Rule, reference.MethodDeclaration.Identifier.GetLocation(), nestedTypeName)); + c.ReportIssue(Diagnostic.Create(Rule, reference.Key.Identifier.GetLocation(), nestedTypeName)); } } } @@ -80,12 +69,11 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn .Where(x => IsPrivateAndStatic(x, type)) .ToArray(); - private static TypeDeclarationSyntax[] NestedTypeDeclarationsOf(TypeDeclarationSyntax type) => + private static bool HasNestedTypeDeclarations(TypeDeclarationSyntax type) => type.Members - .OfType() - .Where(x => x is ClassDeclarationSyntax or StructDeclarationSyntax or InterfaceDeclarationSyntax - || RecordDeclarationSyntaxWrapper.IsInstance(x)) - .ToArray(); + .OfType() + .Any(x => x is ClassDeclarationSyntax or StructDeclarationSyntax or InterfaceDeclarationSyntax + || RecordDeclarationSyntaxWrapper.IsInstance(x)); private static bool IsPrivateAndStatic(MethodDeclarationSyntax method, TypeDeclarationSyntax containingType) => method.Modifiers.Any(x => x.IsKind(SyntaxKind.StaticKeyword)) @@ -102,11 +90,17 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn private static bool HasAnyModifier(MethodDeclarationSyntax method, params SyntaxKind[] modifiers) => method.Modifiers.Any(x => x.IsAnyKind(modifiers)); - private static IDictionary> PotentialReferencesOfMethodsInsideType(IEnumerable methods, TypeDeclarationSyntax type) + private static IDictionary MethodReferencesInsideType(IEnumerable methods, TypeDeclarationSyntax type, SemanticModel model) { var collector = new PotentialMethodReferenceCollector(methods); collector.Visit(type); - return collector.PotentialMethodReferences; + + return collector.PotentialMethodReferences + .Where(x => x.Value.Any(id => ContainingTypeDeclaration(id) != type)) + .Select(x => new { Refs = x, MethodSymbol = model.GetDeclaredSymbol(x.Key) }) + .Select(x => new { MethodDeclaration = x.Refs.Key, References = x.Refs.Value.Where(t => MethodReferenceFrom(t, model) is { } methodReference && (methodReference == x.MethodSymbol || methodReference.ConstructedFrom == x.MethodSymbol) && ContainingMethodDeclaration(t) != x.Refs.Key).Select(t => new { Identifier = t, Type = ContainingTypeDeclaration(t) }) }) + .Where(x => x.References.Any()) + .ToDictionary(x => x.MethodDeclaration, x => x.References.Select(t => t.Type).ToArray()); } private static TypeDeclarationSyntax ContainingTypeDeclaration(IdentifierNameSyntax identifier) => @@ -121,9 +115,9 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn .OfType() .FirstOrDefault(); - private static TypeDeclarationSyntax LowestCommonAncestorSelf(IEnumerable declaredTypes) + private static TypeDeclarationSyntax LowestCommonAncestorOrSelf(IEnumerable declaredTypes) { - var treePaths = declaredTypes.Distinct().Select(x => x.AncestorsAndSelf().OfType().Reverse().ToArray()).ToArray(); + var treePaths = declaredTypes.Select(x => x.AncestorsAndSelf().OfType().Reverse().ToArray()).ToArray(); int minPathLength = treePaths.Select(x => x.Length).Min(); for (int i = 0; i < minPathLength; i++) { @@ -136,8 +130,6 @@ private static TypeDeclarationSyntax LowestCommonAncestorSelf(IEnumerable(T arg) => arg; // Noncompliant + // ^^^^^^^^^^^^^ + + static int Recursive(int n) => Recursive(n - 1); // Noncompliant + static void MutuallyRecursive1() => MutuallyRecursive2(); // FN - both methods could be moved inisde the nested class + static void MutuallyRecursive2() => MutuallyRecursive1(); + + [DllImport("SomeLibrary.dll")] + private static extern void ExternalMethod(); // Noncompliant + + static int UsedOnlyByPropertyInNestedClass() => 42; // Noncompliant + static int UsedOnlyByFieldInitializerInNestedClass() => 42; // Noncompliant + static void UsedOnlyByConstructorInNestedClass() { } // Noncompliant + static void AssignedToDelegateInNestedClass() { } // Noncompliant static void UsedInNameOfExpressionInNestedClass() { } // Noncompliant + void Foo() + { + UsedByOuterAndNestedClasses(); + } + class NestedClass { + int _nestedField = UsedOnlyByFieldInitializerInNestedClass(); + int NestedProp => UsedOnlyByPropertyInNestedClass(); + + public NestedClass() + { + UsedOnlyByConstructorInNestedClass(); + } + + static void NestedClassMethodUsedByDeeperNestedClass() { } // Noncompliant {{Move the method inside 'DeeperNestedClass'.}} + void Bar() { + OnlyUsedOnceByNestedClass(); + OnlyUsedByNestedClassMultipleTimes(); + OuterClass.OnlyUsedByNestedClassWithClassName(); + UsedByMultipleSiblingNestedClasses(); + UsedByOuterAndNestedClasses(); + UsedByNestedClassAndDeeperNestedClass(); + + new OuterClass().NotStatic(); + _outerField = 42; + OuterProp = 42; + + PublicMethod(); + ProtectedMethod(); + InternalMethod(); + ProtectedInternalMethod(); + PrivateMethod(); + + GenericMethod(42); + + Recursive(42); + MutuallyRecursive1(); + + ExternalMethod(); + + Action methodDelegate = AssignedToDelegateInNestedClass; string methodName = nameof(UsedInNameOfExpressionInNestedClass); } + + void FooBaz() + { + OnlyUsedByNestedClassMultipleTimes(); + } + + class DeeperNestedClass + { + void FooBar() + { + OnlyUsedByDeeperNestedClass(); + UsedByNestedClassAndDeeperNestedClass(); + UsedBySiblingAndDeeperNestedClasses(); + NestedClassMethodUsedByDeeperNestedClass(); + } + } + } + + class SiblingNestedClass + { + void Baz() + { + UsedByMultipleSiblingNestedClasses(); + UsedBySiblingAndDeeperNestedClasses(); + } + } +} + +class ClassContainsStruct +{ + static void OnlyUsedByNestedStruct() { } // Noncompliant + + struct NestedStruct + { + void Foo() + { + OnlyUsedByNestedStruct(); + } + } +} + +struct StructContainsClass +{ + private static void OnlyUsedByNestedClass() { } // Noncompliant + + class NestedClass + { + void Foo() + { + OnlyUsedByNestedClass(); + } + } +} + +partial class PartialOuterClass +{ + static void OnlyUsedByNestedClass() { } // Compliant - partial classes are often a result of code generation, so their methods shouldn't be moved + static partial void PartialOnlyUsedByNestedClass() { } // Compliant +} + +partial class PartialOuterClass +{ + static partial void PartialOnlyUsedByNestedClass(); + + class NestedClass + { + void Foo() + { + OnlyUsedByNestedClass(); + PartialOnlyUsedByNestedClass(); + } } } +[DebuggerDisplay("{UsedByDebuggerDisplay()}")] +class DebugViewClass +{ + static string UsedByDebuggerDisplay() => ""; // Noncompliant - FP: should not be moved to nested class, because it's also used by the attribute + + class NestedClass + { + void Foo() + { + UsedByDebuggerDisplay(); + } + } +} From f4ec1e04914b13e4047e9e8e9d16b839f8922247 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Mon, 27 Feb 2023 14:18:46 +0100 Subject: [PATCH 11/19] Refactor LCA and other methods --- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 46 +++++++++++-------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs index 288470c2b7b..bd976c08ce6 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -72,8 +72,7 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn private static bool HasNestedTypeDeclarations(TypeDeclarationSyntax type) => type.Members .OfType() - .Any(x => x is ClassDeclarationSyntax or StructDeclarationSyntax or InterfaceDeclarationSyntax - || RecordDeclarationSyntaxWrapper.IsInstance(x)); + .Any(); private static bool IsPrivateAndStatic(MethodDeclarationSyntax method, TypeDeclarationSyntax containingType) => method.Modifiers.Any(x => x.IsKind(SyntaxKind.StaticKeyword)) @@ -90,19 +89,6 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn private static bool HasAnyModifier(MethodDeclarationSyntax method, params SyntaxKind[] modifiers) => method.Modifiers.Any(x => x.IsAnyKind(modifiers)); - private static IDictionary MethodReferencesInsideType(IEnumerable methods, TypeDeclarationSyntax type, SemanticModel model) - { - var collector = new PotentialMethodReferenceCollector(methods); - collector.Visit(type); - - return collector.PotentialMethodReferences - .Where(x => x.Value.Any(id => ContainingTypeDeclaration(id) != type)) - .Select(x => new { Refs = x, MethodSymbol = model.GetDeclaredSymbol(x.Key) }) - .Select(x => new { MethodDeclaration = x.Refs.Key, References = x.Refs.Value.Where(t => MethodReferenceFrom(t, model) is { } methodReference && (methodReference == x.MethodSymbol || methodReference.ConstructedFrom == x.MethodSymbol) && ContainingMethodDeclaration(t) != x.Refs.Key).Select(t => new { Identifier = t, Type = ContainingTypeDeclaration(t) }) }) - .Where(x => x.References.Any()) - .ToDictionary(x => x.MethodDeclaration, x => x.References.Select(t => t.Type).ToArray()); - } - private static TypeDeclarationSyntax ContainingTypeDeclaration(IdentifierNameSyntax identifier) => identifier .Ancestors() @@ -117,17 +103,39 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn private static TypeDeclarationSyntax LowestCommonAncestorOrSelf(IEnumerable declaredTypes) { - var treePaths = declaredTypes.Select(x => x.AncestorsAndSelf().OfType().Reverse().ToArray()).ToArray(); + var treePaths = declaredTypes.Select(PathFromTop); int minPathLength = treePaths.Select(x => x.Length).Min(); + var firstPath = treePaths.First(); + for (int i = 0; i < minPathLength; i++) { - if (!treePaths.All(x => x[i] == treePaths.First()[i])) + if (!treePaths.All(x => x[i] == firstPath[i])) { - return treePaths.First()[i - 1]; + return firstPath[i - 1]; } } - return treePaths.First()[minPathLength - 1]; + return firstPath[minPathLength - 1]; + + static TypeDeclarationSyntax[] PathFromTop(SyntaxNode node) => + node.AncestorsAndSelf() + .OfType() + .Distinct() + .Reverse() + .ToArray(); + } + + private static IDictionary MethodReferencesInsideType(IEnumerable methods, TypeDeclarationSyntax type, SemanticModel model) + { + var collector = new PotentialMethodReferenceCollector(methods); + collector.Visit(type); + + return collector.PotentialMethodReferences + .Where(x => x.Value.Any(id => ContainingTypeDeclaration(id) != type)) + .Select(x => new { Refs = x, MethodSymbol = model.GetDeclaredSymbol(x.Key) }) + .Select(x => new { MethodDeclaration = x.Refs.Key, References = x.Refs.Value.Where(t => MethodReferenceFrom(t, model) is { } methodReference && (methodReference == x.MethodSymbol || methodReference.ConstructedFrom == x.MethodSymbol) && ContainingMethodDeclaration(t) != x.Refs.Key).Select(t => new { Identifier = t, Type = ContainingTypeDeclaration(t) }) }) + .Where(x => x.References.Any()) + .ToDictionary(x => x.MethodDeclaration, x => x.References.Select(t => t.Type).ToArray()); } private static IMethodSymbol MethodReferenceFrom(IdentifierNameSyntax identifier, SemanticModel model) From 41764dc7ca8f250c313d94cdc214eb4821e2a354 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Mon, 27 Feb 2023 16:01:36 +0100 Subject: [PATCH 12/19] Simplify methods --- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 82 ++++++++++++------- 1 file changed, 51 insertions(+), 31 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs index bd976c08ce6..b24d3a132e2 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -18,6 +18,8 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using SonarAnalyzer.CFG.Helpers; + namespace SonarAnalyzer.Rules.CSharp; [DiagnosticAnalyzer(LanguageNames.CSharp)] @@ -48,15 +50,16 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn && HasNestedTypeDeclarations(declaredType) && PrivateStaticMethodsOf(declaredType) is { Length: > 0 } candidates) { + // TODO: naming (type hierarchy) var methodReferences = MethodReferencesInsideType(candidates, declaredType, c.SemanticModel); foreach (var reference in methodReferences) { - var typeToMoveInto = LowestCommonAncestorOrSelf(reference.Value); + var typeToMoveInto = LowestCommonAncestorOrSelf(reference.Types); if (typeToMoveInto != declaredType) { - string nestedTypeName = typeToMoveInto.Identifier.ValueText; - c.ReportIssue(Diagnostic.Create(Rule, reference.Key.Identifier.GetLocation(), nestedTypeName)); + var nestedTypeName = typeToMoveInto.Identifier.ValueText; + c.ReportIssue(Diagnostic.Create(Rule, reference.Method.Identifier.GetLocation(), nestedTypeName)); } } } @@ -89,18 +92,6 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn private static bool HasAnyModifier(MethodDeclarationSyntax method, params SyntaxKind[] modifiers) => method.Modifiers.Any(x => x.IsAnyKind(modifiers)); - private static TypeDeclarationSyntax ContainingTypeDeclaration(IdentifierNameSyntax identifier) => - identifier - .Ancestors() - .OfType() - .First(); - - private static MethodDeclarationSyntax ContainingMethodDeclaration(IdentifierNameSyntax identifier) => - identifier - .Ancestors() - .OfType() - .FirstOrDefault(); - private static TypeDeclarationSyntax LowestCommonAncestorOrSelf(IEnumerable declaredTypes) { var treePaths = declaredTypes.Select(PathFromTop); @@ -125,44 +116,73 @@ private static TypeDeclarationSyntax LowestCommonAncestorOrSelf(IEnumerable MethodReferencesInsideType(IEnumerable methods, TypeDeclarationSyntax type, SemanticModel model) + private static IEnumerable MethodReferencesInsideType( + IEnumerable methods, TypeDeclarationSyntax outerType, SemanticModel model) { var collector = new PotentialMethodReferenceCollector(methods); - collector.Visit(type); + collector.Visit(outerType); return collector.PotentialMethodReferences - .Where(x => x.Value.Any(id => ContainingTypeDeclaration(id) != type)) - .Select(x => new { Refs = x, MethodSymbol = model.GetDeclaredSymbol(x.Key) }) - .Select(x => new { MethodDeclaration = x.Refs.Key, References = x.Refs.Value.Where(t => MethodReferenceFrom(t, model) is { } methodReference && (methodReference == x.MethodSymbol || methodReference.ConstructedFrom == x.MethodSymbol) && ContainingMethodDeclaration(t) != x.Refs.Key).Select(t => new { Identifier = t, Type = ContainingTypeDeclaration(t) }) }) - .Where(x => x.References.Any()) - .ToDictionary(x => x.MethodDeclaration, x => x.References.Select(t => t.Type).ToArray()); - } + .Where(x => !OnlyUsedByOuterType(x)) + .Select(ResolveReferences) + .Where(x => x.Types.Any()) + .ToArray(); - private static IMethodSymbol MethodReferenceFrom(IdentifierNameSyntax identifier, SemanticModel model) - { - var symbolInfo = model.GetSymbolInfo(identifier); - if (symbolInfo.Symbol is IMethodSymbol { } methodSymbol) + MethodUsedByTypes ResolveReferences(MethodWithPotentialReferences m) { - return methodSymbol; + var methodSymbol = model.GetDeclaredSymbol(m.Method); + + var typesWhichUseTheMethod = m.PotentialReferences + .Where(x => + !IsRecursiveMethodCall(x, m.Method) + && model.GetSymbolOrCandidateSymbol(x) is IMethodSymbol { } methodReference + && (methodReference == methodSymbol || methodReference.ConstructedFrom == methodSymbol)) + .Select(ContainingTypeDeclaration) + .Distinct() + .ToArray(); + + return new MethodUsedByTypes(m.Method, typesWhichUseTheMethod); } - return symbolInfo.CandidateSymbols.FirstOrDefault() as IMethodSymbol; + + bool IsRecursiveMethodCall(IdentifierNameSyntax methodCall, MethodDeclarationSyntax methodDeclaration) => + methodCall.Ancestors().OfType().FirstOrDefault() == methodDeclaration; + + bool OnlyUsedByOuterType(MethodWithPotentialReferences m) => + m.PotentialReferences.All(x => ContainingTypeDeclaration(x) == outerType); } + private static TypeDeclarationSyntax ContainingTypeDeclaration(IdentifierNameSyntax identifier) => + identifier + .Ancestors() + .OfType() + .First(); + + private record MethodWithPotentialReferences(MethodDeclarationSyntax Method, IdentifierNameSyntax[] PotentialReferences); + private record MethodUsedByTypes(MethodDeclarationSyntax Method, TypeDeclarationSyntax[] Types); + + /// + /// Collects all the potential references to a set of methods inside the given syntax node. + /// The collector looks for identifiers which match any of the methods' names, but does not try to resolve them to symbols with the semantic model. + /// private class PotentialMethodReferenceCollector : CSharpSyntaxWalker { private readonly ISet methodsToFind; - internal Dictionary> PotentialMethodReferences { get; } = new(); + private readonly Dictionary> potentialMethodReferences; + + public IEnumerable PotentialMethodReferences => + potentialMethodReferences.Select(x => new MethodWithPotentialReferences(x.Key, x.Value.ToArray())); public PotentialMethodReferenceCollector(IEnumerable methodsToFind) { this.methodsToFind = new HashSet(methodsToFind); + potentialMethodReferences = new(); } public override void VisitIdentifierName(IdentifierNameSyntax identifier) { if (methodsToFind.FirstOrDefault(x => x.Identifier.ValueText == identifier.Identifier.ValueText) is { } method) { - var referenceList = PotentialMethodReferences.GetOrAdd(method, _ => new()); + var referenceList = potentialMethodReferences.GetOrAdd(method, _ => new()); referenceList.Add(identifier); } } From 279802cf67b2918008b3ab5ad44a533acd713da4 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Mon, 27 Feb 2023 16:07:47 +0100 Subject: [PATCH 13/19] Add test case --- .../PrivateStaticMethodUsedOnlyByNestedClass.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs index 2082cc1af8b..cf432f4a570 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -13,6 +13,7 @@ class OuterClass static void UsedBySiblingAndDeeperNestedClasses() { } // Compliant - SiblingNestedClass and DeeperNestedClass both need access to the method, so it must stay in the outer class static void OnlyUsedByDeeperNestedClass() { } // Noncompliant {{Move the method inside 'DeeperNestedClass'.}} static void UsedByNestedClassAndDeeperNestedClass() { } // Noncompliant {{Move the method inside 'NestedClass'.}} + static void UsedByDeeperNestedClassesOnTheSameLevel() { } // Noncompliant {{Move the method inside 'NestedClass'.}} static void UnusedMethod() { } // Compliant - no need to move unused method anywhere void NotStatic() { } // Compliant - method is not static @@ -100,10 +101,19 @@ void FooBar() { OnlyUsedByDeeperNestedClass(); UsedByNestedClassAndDeeperNestedClass(); + UsedByDeeperNestedClassesOnTheSameLevel(); UsedBySiblingAndDeeperNestedClasses(); NestedClassMethodUsedByDeeperNestedClass(); } } + + class AnotherDeeperNestedClass + { + void Foo() + { + UsedByDeeperNestedClassesOnTheSameLevel(); + } + } } class SiblingNestedClass From efb93d75f9a5cd23c0021a54b2a96174dcc7d26d Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Mon, 27 Feb 2023 19:16:57 +0100 Subject: [PATCH 14/19] Fix code smells --- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs index b24d3a132e2..cd6178c361f 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -50,8 +50,7 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn && HasNestedTypeDeclarations(declaredType) && PrivateStaticMethodsOf(declaredType) is { Length: > 0 } candidates) { - // TODO: naming (type hierarchy) - var methodReferences = MethodReferencesInsideType(candidates, declaredType, c.SemanticModel); + var methodReferences = TypesWhichUseTheMethods(candidates, declaredType, c.SemanticModel); foreach (var reference in methodReferences) { @@ -116,7 +115,7 @@ private static TypeDeclarationSyntax LowestCommonAncestorOrSelf(IEnumerable MethodReferencesInsideType( + private static IEnumerable TypesWhichUseTheMethods( IEnumerable methods, TypeDeclarationSyntax outerType, SemanticModel model) { var collector = new PotentialMethodReferenceCollector(methods); @@ -136,7 +135,7 @@ MethodUsedByTypes ResolveReferences(MethodWithPotentialReferences m) .Where(x => !IsRecursiveMethodCall(x, m.Method) && model.GetSymbolOrCandidateSymbol(x) is IMethodSymbol { } methodReference - && (methodReference == methodSymbol || methodReference.ConstructedFrom == methodSymbol)) + && (methodReference.Equals(methodSymbol) || methodReference.ConstructedFrom.Equals(methodSymbol))) .Select(ContainingTypeDeclaration) .Distinct() .ToArray(); @@ -157,14 +156,15 @@ MethodUsedByTypes ResolveReferences(MethodWithPotentialReferences m) .OfType() .First(); - private record MethodWithPotentialReferences(MethodDeclarationSyntax Method, IdentifierNameSyntax[] PotentialReferences); - private record MethodUsedByTypes(MethodDeclarationSyntax Method, TypeDeclarationSyntax[] Types); + private sealed record MethodWithPotentialReferences(MethodDeclarationSyntax Method, IdentifierNameSyntax[] PotentialReferences); + private sealed record MethodUsedByTypes(MethodDeclarationSyntax Method, TypeDeclarationSyntax[] Types); /// /// Collects all the potential references to a set of methods inside the given syntax node. /// The collector looks for identifiers which match any of the methods' names, but does not try to resolve them to symbols with the semantic model. + /// Performance gains: by only using the syntax tree to find matches we can eliminate certain methods (which are only used by the type which has declared it) without using the more costly symbolic lookup. /// - private class PotentialMethodReferenceCollector : CSharpSyntaxWalker + private sealed class PotentialMethodReferenceCollector : CSharpSyntaxWalker { private readonly ISet methodsToFind; private readonly Dictionary> potentialMethodReferences; @@ -178,12 +178,12 @@ public PotentialMethodReferenceCollector(IEnumerable me potentialMethodReferences = new(); } - public override void VisitIdentifierName(IdentifierNameSyntax identifier) + public override void VisitIdentifierName(IdentifierNameSyntax node) { - if (methodsToFind.FirstOrDefault(x => x.Identifier.ValueText == identifier.Identifier.ValueText) is { } method) + if (methodsToFind.FirstOrDefault(x => x.Identifier.ValueText == node.Identifier.ValueText) is { } method) { var referenceList = potentialMethodReferences.GetOrAdd(method, _ => new()); - referenceList.Add(identifier); + referenceList.Add(node); } } } From 451feb3d01d5cb8d6cec65379db39e9f988362ed Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Tue, 28 Feb 2023 11:19:42 +0100 Subject: [PATCH 15/19] Update RSPEC and minor refactoring --- analyzers/rspec/cs/S3398_c#.html | 28 ++++++++----------- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 10 +++---- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 10 +++---- 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/analyzers/rspec/cs/S3398_c#.html b/analyzers/rspec/cs/S3398_c#.html index 0023954c5af..11768b0b7ea 100644 --- a/analyzers/rspec/cs/S3398_c#.html +++ b/analyzers/rspec/cs/S3398_c#.html @@ -1,24 +1,21 @@ -

When a private method is only invoked by an inner class, there’s no reason not to move it into that class. It will still have the same -access to the outer class' members, but the outer class will be clearer and less cluttered.

+

When a private static method is only invoked by a nested class, there’s no reason not to move it into that class. It will still have +the same access to the outer class' static members, but the outer class will be clearer and less cluttered.

Noncompliant Code Example

 public class Outer
 {
-    public void OuterMethod()
-    {
-        Console.WriteLine("Called from an outer method.");
-    }
+    private const int base = 42;
 
-    private static void Print(int num) // Noncompliant - method is only used by inner class, should be moved there
+    private static void Print(int num)  // Noncompliant - static method is only used by the nested class, should be moved there
     {
-        Console.WriteLine(num);
+        Console.WriteLine(num + base);
     }
 
-    public class Inner
+    public class Nested
     {
         public void SomeMethod()
         {
-            Outer.Print(42);
+            Outer.Print(1);
         }
     }
 }
@@ -27,21 +24,18 @@ 
Compliant Solution

 
 public class Outer
 {
-    public void OuterMethod()
-    {
-        Console.WriteLine("Called from an outer method.");
-    }
+    private const int base = 42;
 
-    public class Inner
+    public class Nested
     {
         public void SomeMethod()
         {
-            Print(42);
+            Print(1);
         }
 
         private static void Print(int num)
         {
-            Console.WriteLine(num);
+            Console.WriteLine(num + base);
         }
     }
 }
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs
index cd6178c361f..459d3dd8bb4 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs
@@ -26,7 +26,7 @@ namespace SonarAnalyzer.Rules.CSharp;
 public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAnalyzer
 {
     private const string DiagnosticId = "S3398";
-    private const string MessageFormat = "Move the method inside '{0}'.";
+    private const string MessageFormat = "Move this method inside '{0}'.";
 
     private static readonly SyntaxKind[] AnalyzedSyntaxKinds = new[]
     {
@@ -93,13 +93,13 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn
 
     private static TypeDeclarationSyntax LowestCommonAncestorOrSelf(IEnumerable declaredTypes)
     {
-        var treePaths = declaredTypes.Select(PathFromTop);
-        int minPathLength = treePaths.Select(x => x.Length).Min();
-        var firstPath = treePaths.First();
+        var typeHierarchyToMethodsReference = declaredTypes.Select(PathFromTop);
+        int minPathLength = typeHierarchyToMethodsReference.Select(x => x.Length).Min();
+        var firstPath = typeHierarchyToMethodsReference.First();
 
         for (int i = 0; i < minPathLength; i++)
         {
-            if (!treePaths.All(x => x[i] == firstPath[i]))
+            if (!typeHierarchyToMethodsReference.All(x => x[i] == firstPath[i]))
             {
                 return firstPath[i - 1];
             }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs
index cf432f4a570..a777726e1a4 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs
@@ -4,16 +4,16 @@
 
 class OuterClass
 {
-    static void OnlyUsedOnceByNestedClass() { }                     // Noncompliant {{Move the method inside 'NestedClass'.}}
+    static void OnlyUsedOnceByNestedClass() { }                     // Noncompliant {{Move this method inside 'NestedClass'.}}
     //          ^^^^^^^^^^^^^^^^^^^^^^^^^
     static void OnlyUsedByNestedClassMultipleTimes() { }            // Noncompliant
     static void OnlyUsedByNestedClassWithClassName() { }            // Noncompliant
     static void UsedByMultipleSiblingNestedClasses() { }            // Compliant - it needs to stay in the outer class
     static void UsedByOuterAndNestedClasses() { }                   // Compliant - it's used by the outer class, so it needs to stay there
     static void UsedBySiblingAndDeeperNestedClasses() { }           // Compliant - SiblingNestedClass and DeeperNestedClass both need access to the method, so it must stay in the outer class
-    static void OnlyUsedByDeeperNestedClass() { }                   // Noncompliant {{Move the method inside 'DeeperNestedClass'.}}
-    static void UsedByNestedClassAndDeeperNestedClass() { }         // Noncompliant {{Move the method inside 'NestedClass'.}}
-    static void UsedByDeeperNestedClassesOnTheSameLevel() { }       // Noncompliant {{Move the method inside 'NestedClass'.}}
+    static void OnlyUsedByDeeperNestedClass() { }                   // Noncompliant {{Move this method inside 'DeeperNestedClass'.}}
+    static void UsedByNestedClassAndDeeperNestedClass() { }         // Noncompliant {{Move this method inside 'NestedClass'.}}
+    static void UsedByDeeperNestedClassesOnTheSameLevel() { }       // Noncompliant {{Move this method inside 'NestedClass'.}}
     static void UnusedMethod() { }                                  // Compliant - no need to move unused method anywhere
 
     void NotStatic() { }                                            // Compliant - method is not static
@@ -58,7 +58,7 @@ public NestedClass()
             UsedOnlyByConstructorInNestedClass();
         }
 
-        static void NestedClassMethodUsedByDeeperNestedClass() { }  // Noncompliant {{Move the method inside 'DeeperNestedClass'.}}
+        static void NestedClassMethodUsedByDeeperNestedClass() { }  // Noncompliant {{Move this method inside 'DeeperNestedClass'.}}
 
         void Bar()
         {

From dac2f90611e6612c820501328716477f3a11733e Mon Sep 17 00:00:00 2001
From: Andrei Epure 
Date: Tue, 28 Feb 2023 12:18:15 +0100
Subject: [PATCH 16/19] Refactor methods for better readability

---
 ...rivateStaticMethodUsedOnlyByNestedClass.cs | 45 +++++++++++++------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs
index 459d3dd8bb4..361c11312fd 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs
@@ -76,36 +76,53 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn
                 .OfType()
                 .Any();
 
-    private static bool IsPrivateAndStatic(MethodDeclarationSyntax method, TypeDeclarationSyntax containingType) =>
-        method.Modifiers.Any(x => x.IsKind(SyntaxKind.StaticKeyword))
-        && ((HasAnyModifier(method, SyntaxKind.PrivateKeyword) && !HasAnyModifier(method, SyntaxKind.ProtectedKeyword))
-           || (!HasAnyModifier(method, SyntaxKind.PublicKeyword, SyntaxKind.ProtectedKeyword, SyntaxKind.InternalKeyword) && IsClassOrRecordClassOrInterfaceDeclaration(containingType)));
-
     private static bool IsPartial(TypeDeclarationSyntax type) =>
         type.Modifiers.Any(x => x.IsKind(SyntaxKind.PartialKeyword));
 
-    private static bool IsClassOrRecordClassOrInterfaceDeclaration(TypeDeclarationSyntax type) =>
-        type is ClassDeclarationSyntax or InterfaceDeclarationSyntax
-        || (RecordDeclarationSyntaxWrapper.IsInstance(type) && !((RecordDeclarationSyntaxWrapper)type).ClassOrStructKeyword.IsKind(SyntaxKind.StructKeyword));
+
+    private static bool IsPrivateAndStatic(MethodDeclarationSyntax method, TypeDeclarationSyntax containingType)
+    {
+        return method.Modifiers.Any(x => x.IsKind(SyntaxKind.StaticKeyword))
+            && (IsExplicitlyPrivate() || IsImplicityPrivate());
+
+
+        bool IsExplicitlyPrivate() =>
+            HasAnyModifier(method, SyntaxKind.PrivateKeyword) && !HasAnyModifier(method, SyntaxKind.ProtectedKeyword);
+
+        // The default accessibility for record class members is private, but for record structs (like all structs) it's internal.
+        bool IsImplicityPrivate() =>
+            !HasAnyModifier(method, SyntaxKind.PublicKeyword, SyntaxKind.ProtectedKeyword, SyntaxKind.InternalKeyword)
+            && IsClassOrRecordClassOrInterfaceDeclaration(containingType);
+
+        static bool IsClassOrRecordClassOrInterfaceDeclaration(TypeDeclarationSyntax type) =>
+            type is ClassDeclarationSyntax or InterfaceDeclarationSyntax
+            || (RecordDeclarationSyntaxWrapper.IsInstance(type) && !((RecordDeclarationSyntaxWrapper)type).ClassOrStructKeyword.IsKind(SyntaxKind.StructKeyword));
+    }
 
     private static bool HasAnyModifier(MethodDeclarationSyntax method, params SyntaxKind[] modifiers) =>
         method.Modifiers.Any(x => x.IsAnyKind(modifiers));
 
     private static TypeDeclarationSyntax LowestCommonAncestorOrSelf(IEnumerable declaredTypes)
     {
-        var typeHierarchyToMethodsReference = declaredTypes.Select(PathFromTop);
-        int minPathLength = typeHierarchyToMethodsReference.Select(x => x.Length).Min();
-        var firstPath = typeHierarchyToMethodsReference.First();
+        var typeHierarchyFromTopToBottom = declaredTypes.Select(PathFromTop);
+        var minPathLength = typeHierarchyFromTopToBottom.Select(x => x.Length).Min();
+        var firstPath = typeHierarchyFromTopToBottom.First();
 
+        var lastCommonPathIndex = 0;
         for (int i = 0; i < minPathLength; i++)
         {
-            if (!typeHierarchyToMethodsReference.All(x => x[i] == firstPath[i]))
+            var isPartOfCommonPath = typeHierarchyFromTopToBottom.All(x => x[i] == firstPath[i]);
+            if (isPartOfCommonPath)
+            {
+                lastCommonPathIndex = i;
+            }
+            else
             {
-                return firstPath[i - 1];
+                break;
             }
         }
 
-        return firstPath[minPathLength - 1];
+        return firstPath[lastCommonPathIndex];
 
         static TypeDeclarationSyntax[] PathFromTop(SyntaxNode node) =>
             node.AncestorsAndSelf()

From d55f190981b5a06593fc219b2edd8e4aef21f8e5 Mon Sep 17 00:00:00 2001
From: Andrei Epure 
Date: Tue, 28 Feb 2023 12:36:43 +0100
Subject: [PATCH 17/19] Order method declarations based on execution order;
 group locals

---
 ...rivateStaticMethodUsedOnlyByNestedClass.cs | 61 +++++++++----------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs
index 361c11312fd..ff3912f3926 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs
@@ -65,27 +65,25 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn
             },
             AnalyzedSyntaxKinds);
 
-    private static MethodDeclarationSyntax[] PrivateStaticMethodsOf(TypeDeclarationSyntax type) =>
-        type.Members
-                .OfType()
-                .Where(x => IsPrivateAndStatic(x, type))
-                .ToArray();
+    private static bool IsPartial(TypeDeclarationSyntax type) =>
+        type.Modifiers.Any(x => x.IsKind(SyntaxKind.PartialKeyword));
 
     private static bool HasNestedTypeDeclarations(TypeDeclarationSyntax type) =>
         type.Members
-                .OfType()
-                .Any();
-
-    private static bool IsPartial(TypeDeclarationSyntax type) =>
-        type.Modifiers.Any(x => x.IsKind(SyntaxKind.PartialKeyword));
+            .OfType()
+            .Any();
 
+    private static MethodDeclarationSyntax[] PrivateStaticMethodsOf(TypeDeclarationSyntax type) =>
+        type.Members
+            .OfType()
+            .Where(x => IsPrivateAndStatic(x, type))
+            .ToArray();
 
     private static bool IsPrivateAndStatic(MethodDeclarationSyntax method, TypeDeclarationSyntax containingType)
     {
         return method.Modifiers.Any(x => x.IsKind(SyntaxKind.StaticKeyword))
             && (IsExplicitlyPrivate() || IsImplicityPrivate());
 
-
         bool IsExplicitlyPrivate() =>
             HasAnyModifier(method, SyntaxKind.PrivateKeyword) && !HasAnyModifier(method, SyntaxKind.ProtectedKeyword);
 
@@ -97,10 +95,10 @@ private static bool IsPrivateAndStatic(MethodDeclarationSyntax method, TypeDecla
         static bool IsClassOrRecordClassOrInterfaceDeclaration(TypeDeclarationSyntax type) =>
             type is ClassDeclarationSyntax or InterfaceDeclarationSyntax
             || (RecordDeclarationSyntaxWrapper.IsInstance(type) && !((RecordDeclarationSyntaxWrapper)type).ClassOrStructKeyword.IsKind(SyntaxKind.StructKeyword));
-    }
 
-    private static bool HasAnyModifier(MethodDeclarationSyntax method, params SyntaxKind[] modifiers) =>
-        method.Modifiers.Any(x => x.IsAnyKind(modifiers));
+        static bool HasAnyModifier(MethodDeclarationSyntax method, params SyntaxKind[] modifiers) =>
+            method.Modifiers.Any(x => x.IsAnyKind(modifiers));
+    }
 
     private static TypeDeclarationSyntax LowestCommonAncestorOrSelf(IEnumerable declaredTypes)
     {
@@ -139,23 +137,23 @@ private static TypeDeclarationSyntax LowestCommonAncestorOrSelf(IEnumerable !OnlyUsedByOuterType(x))
-                            .Select(ResolveReferences)
-                            .Where(x => x.Types.Any())
-                            .ToArray();
+                .Where(x => !OnlyUsedByOuterType(x))
+                .Select(ResolveReferences)
+                .Where(x => x.Types.Any())
+                .ToArray();
 
         MethodUsedByTypes ResolveReferences(MethodWithPotentialReferences m)
         {
             var methodSymbol = model.GetDeclaredSymbol(m.Method);
 
             var typesWhichUseTheMethod = m.PotentialReferences
-                .Where(x =>
-                    !IsRecursiveMethodCall(x, m.Method)
-                    && model.GetSymbolOrCandidateSymbol(x) is IMethodSymbol { } methodReference
-                    && (methodReference.Equals(methodSymbol) || methodReference.ConstructedFrom.Equals(methodSymbol)))
-                .Select(ContainingTypeDeclaration)
-                .Distinct()
-                .ToArray();
+                    .Where(x =>
+                        !IsRecursiveMethodCall(x, m.Method)
+                        && model.GetSymbolOrCandidateSymbol(x) is IMethodSymbol { } methodReference
+                        && (methodReference.Equals(methodSymbol) || methodReference.ConstructedFrom.Equals(methodSymbol)))
+                    .Select(ContainingTypeDeclaration)
+                    .Distinct()
+                    .ToArray();
 
             return new MethodUsedByTypes(m.Method, typesWhichUseTheMethod);
         }
@@ -165,15 +163,16 @@ MethodUsedByTypes ResolveReferences(MethodWithPotentialReferences m)
 
         bool OnlyUsedByOuterType(MethodWithPotentialReferences m) =>
             m.PotentialReferences.All(x => ContainingTypeDeclaration(x) == outerType);
-    }
 
-    private static TypeDeclarationSyntax ContainingTypeDeclaration(IdentifierNameSyntax identifier) =>
-        identifier
-            .Ancestors()
-            .OfType()
-            .First();
+        static TypeDeclarationSyntax ContainingTypeDeclaration(IdentifierNameSyntax identifier) =>
+            identifier
+                .Ancestors()
+                .OfType()
+                .First();
+    }
 
     private sealed record MethodWithPotentialReferences(MethodDeclarationSyntax Method, IdentifierNameSyntax[] PotentialReferences);
+
     private sealed record MethodUsedByTypes(MethodDeclarationSyntax Method, TypeDeclarationSyntax[] Types);
 
     /// 

From 43475a2d8541fd273135c2669920055714be6069 Mon Sep 17 00:00:00 2001
From: Zsolt Kolbay 
Date: Tue, 28 Feb 2023 13:08:38 +0100
Subject: [PATCH 18/19] Rename variables/methods

---
 .../PrivateStaticMethodUsedOnlyByNestedClass.cs  | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs
index ff3912f3926..44998f457b3 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs
@@ -44,18 +44,18 @@ public sealed class PrivateStaticMethodUsedOnlyByNestedClass : SonarDiagnosticAn
     protected override void Initialize(SonarAnalysisContext context) =>
         context.RegisterNodeAction(c =>
             {
-                var declaredType = (TypeDeclarationSyntax)c.Node;
+                var outerType = (TypeDeclarationSyntax)c.Node;
 
-                if (!IsPartial(declaredType)
-                    && HasNestedTypeDeclarations(declaredType)
-                    && PrivateStaticMethodsOf(declaredType) is { Length: > 0 } candidates)
+                if (!IsPartial(outerType)
+                    && HasNestedTypeDeclarations(outerType)
+                    && PrivateStaticMethodsOf(outerType) is { Length: > 0 } candidates)
                 {
-                    var methodReferences = TypesWhichUseTheMethods(candidates, declaredType, c.SemanticModel);
+                    var methodReferences = TypesWhichUseTheMethods(candidates, outerType, c.SemanticModel);
 
                     foreach (var reference in methodReferences)
                     {
                         var typeToMoveInto = LowestCommonAncestorOrSelf(reference.Types);
-                        if (typeToMoveInto != declaredType)
+                        if (typeToMoveInto != outerType)
                         {
                             var nestedTypeName = typeToMoveInto.Identifier.ValueText;
                             c.ReportIssue(Diagnostic.Create(Rule, reference.Method.Identifier.GetLocation(), nestedTypeName));
@@ -138,11 +138,11 @@ private static TypeDeclarationSyntax LowestCommonAncestorOrSelf(IEnumerable !OnlyUsedByOuterType(x))
-                .Select(ResolveReferences)
+                .Select(DeclaredTypesWhichActuallyUseTheMethod)
                 .Where(x => x.Types.Any())
                 .ToArray();
 
-        MethodUsedByTypes ResolveReferences(MethodWithPotentialReferences m)
+        MethodUsedByTypes DeclaredTypesWhichActuallyUseTheMethod(MethodWithPotentialReferences m)
         {
             var methodSymbol = model.GetDeclaredSymbol(m.Method);
 

From e5a2d45f885044076ffb5ded554e775a7c37b657 Mon Sep 17 00:00:00 2001
From: Andrei Epure 
Date: Tue, 28 Feb 2023 13:52:00 +0100
Subject: [PATCH 19/19] add edge case

---
 ...rivateStaticMethodUsedOnlyByNestedClass.cs | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs
index a777726e1a4..6210a94aa03 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PrivateStaticMethodUsedOnlyByNestedClass.cs
@@ -185,3 +185,37 @@ void Foo()
         }
     }
 }
+
+public class EdgeCaseWithLongCommonPaths
+{
+    private static void StaticMethod() { }                           // Noncompliant {{Move this method inside 'InsideMiddleOne'.}}
+
+    public class MiddleOne
+    {
+        public class InsideMiddleOne
+        {
+            public class Foo
+            {
+                public class FooLeaf
+                {
+                    public void Method() => StaticMethod();
+                }
+            }
+
+            public class Bar
+            {
+                public void Method() => StaticMethod();
+                public class BarLeaf
+                {
+                    public void Method() => StaticMethod();
+                }
+            }
+        }
+    }
+    public class MiddleTwo {
+        public void StaticMethod()
+        {
+        }
+        public void Method() => StaticMethod();
+    }
+}