From 67905c862dc573b31d235e832e5c5387b288f57a Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Tue, 28 Feb 2023 16:18:38 +0100 Subject: [PATCH 1/2] Fix code smells --- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs index 44998f457b3..a77744d5ddb 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -102,25 +102,18 @@ private static bool IsPrivateAndStatic(MethodDeclarationSyntax method, TypeDecla private static TypeDeclarationSyntax LowestCommonAncestorOrSelf(IEnumerable declaredTypes) { - var typeHierarchyFromTopToBottom = declaredTypes.Select(PathFromTop); + var typeHierarchyFromTopToBottom = declaredTypes.Select(PathFromTop).ToArray(); var minPathLength = typeHierarchyFromTopToBottom.Select(x => x.Length).Min(); var firstPath = typeHierarchyFromTopToBottom.First(); + var levels = Enumerable.Range(0, minPathLength); + var firstDifferingLevel = levels.FirstOrDefault(NotEveryNodeIsTheSameOnLevel); + int levelOfLowestCommonAncestor = firstDifferingLevel == default + ? minPathLength - 1 + : firstDifferingLevel - 1; + return firstPath[levelOfLowestCommonAncestor]; - var lastCommonPathIndex = 0; - for (int i = 0; i < minPathLength; i++) - { - var isPartOfCommonPath = typeHierarchyFromTopToBottom.All(x => x[i] == firstPath[i]); - if (isPartOfCommonPath) - { - lastCommonPathIndex = i; - } - else - { - break; - } - } - - return firstPath[lastCommonPathIndex]; + bool NotEveryNodeIsTheSameOnLevel(int level) => + typeHierarchyFromTopToBottom.Any(x => x[level] != firstPath[level]); static TypeDeclarationSyntax[] PathFromTop(SyntaxNode node) => node.AncestorsAndSelf() @@ -178,7 +171,8 @@ private sealed record MethodUsedByTypes(MethodDeclarationSyntax Method, TypeDecl /// /// 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. + /// 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 sealed class PotentialMethodReferenceCollector : CSharpSyntaxWalker { From d59de475b6aadcb197566529779bd633309b4f52 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Wed, 1 Mar 2023 10:56:49 +0100 Subject: [PATCH 2/2] Revert previous refactor --- ...rivateStaticMethodUsedOnlyByNestedClass.cs | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs index a77744d5ddb..bfb3748020a 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PrivateStaticMethodUsedOnlyByNestedClass.cs @@ -102,18 +102,25 @@ private static bool IsPrivateAndStatic(MethodDeclarationSyntax method, TypeDecla private static TypeDeclarationSyntax LowestCommonAncestorOrSelf(IEnumerable declaredTypes) { - var typeHierarchyFromTopToBottom = declaredTypes.Select(PathFromTop).ToArray(); + var typeHierarchyFromTopToBottom = declaredTypes.Select(PathFromTop); var minPathLength = typeHierarchyFromTopToBottom.Select(x => x.Length).Min(); var firstPath = typeHierarchyFromTopToBottom.First(); - var levels = Enumerable.Range(0, minPathLength); - var firstDifferingLevel = levels.FirstOrDefault(NotEveryNodeIsTheSameOnLevel); - int levelOfLowestCommonAncestor = firstDifferingLevel == default - ? minPathLength - 1 - : firstDifferingLevel - 1; - return firstPath[levelOfLowestCommonAncestor]; - - bool NotEveryNodeIsTheSameOnLevel(int level) => - typeHierarchyFromTopToBottom.Any(x => x[level] != firstPath[level]); + + var lastCommonPathIndex = 0; + for (int i = 0; i < minPathLength; i++) + { + var isPartOfCommonPath = typeHierarchyFromTopToBottom.All(x => x[i] == firstPath[i]); + if (isPartOfCommonPath) + { + lastCommonPathIndex = i; + } + else + { + break; + } + } + + return firstPath[lastCommonPathIndex]; static TypeDeclarationSyntax[] PathFromTop(SyntaxNode node) => node.AncestorsAndSelf()