Rule S1144: remove duplicate issues in top level statements (#5081)

SonarSource · Nov 24, 2021 · 2414400 · 2414400
1 parent 5b1e8df
commit 2414400
Show file tree

Hide file tree

Showing 8 changed files with 71 additions and 49 deletions.
diff --git a/analyzers/src/SonarAnalyzer.CFG/ShimLayer/INamedTypeSymbolExtensions.cs b/analyzers/src/SonarAnalyzer.CFG/ShimLayer/INamedTypeSymbolExtensions.cs
@@ -1,14 +1,19 @@
 // Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
 // Licensed under the MIT License. See LICENSE in the project root for license information.
 
+using System;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+
 namespace StyleCop.Analyzers.Lightup
 {
-    using System;
-    using System.Collections.Immutable;
-    using Microsoft.CodeAnalysis;
-
     public static class INamedTypeSymbolExtensions
     {
+        private const string MainMethodImplicitName = "<Main>$";
+
+        private static readonly HashSet<string> ProgramClassImplicitName = new HashSet<string> { "Program", "<Program>$" };
         private static readonly Func<INamedTypeSymbol, INamedTypeSymbol> TupleUnderlyingTypeAccessor;
         private static readonly Func<INamedTypeSymbol, ImmutableArray<IFieldSymbol>> TupleElementsAccessor;
         private static readonly Func<INamedTypeSymbol, bool> IsSerializableAccessor;
@@ -20,19 +25,15 @@ static INamedTypeSymbolExtensions()
             IsSerializableAccessor = LightupHelpers.CreateSyntaxPropertyAccessor<INamedTypeSymbol, bool>(typeof(INamedTypeSymbol), nameof(IsSerializable));
         }
 
-        public static INamedTypeSymbol TupleUnderlyingType(this INamedTypeSymbol symbol)
-        {
-            return TupleUnderlyingTypeAccessor(symbol);
-        }
+        public static INamedTypeSymbol TupleUnderlyingType(this INamedTypeSymbol symbol) => TupleUnderlyingTypeAccessor(symbol);
 
-        public static ImmutableArray<IFieldSymbol> TupleElements(this INamedTypeSymbol symbol)
-        {
-            return TupleElementsAccessor(symbol);
-        }
+        public static ImmutableArray<IFieldSymbol> TupleElements(this INamedTypeSymbol symbol) => TupleElementsAccessor(symbol);
 
-        public static bool IsSerializable(this INamedTypeSymbol symbol)
-        {
-            return IsSerializableAccessor(symbol);
-        }
+        public static bool IsSerializable(this INamedTypeSymbol symbol) => IsSerializableAccessor(symbol);
+
+        public static bool IsTopLevelProgram(this INamedTypeSymbol symbol) =>
+            ProgramClassImplicitName.Contains(symbol.Name)
+            && symbol.ContainingNamespace.IsGlobalNamespace
+            && symbol.GetMembers(MainMethodImplicitName).Any();
     }
 }
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs
@@ -72,16 +72,10 @@ protected override void Initialize(SonarAnalysisContext context) =>
                         {
                             var namedType = (INamedTypeSymbol)cc.Symbol;
 
-                            if (namedType.TypeKind != TypeKind.Struct
-                                && namedType.TypeKind != TypeKind.Class
-                                && namedType.TypeKind != TypeKind.Delegate
-                                && namedType.TypeKind != TypeKind.Enum
-                                && namedType.TypeKind != TypeKind.Interface)
-                            {
-                                return;
-                            }
-
-                            if (namedType.ContainingType != null || namedType.DerivesFromAny(IgnoredTypes))
+                            if (namedType.ContainingType != null
+                                // We skip top level statements since they cannot have fields. Other declared types are analyzed separately.
+                                || namedType.IsTopLevelProgram()
+                                || namedType.DerivesFromAny(IgnoredTypes))
                             {
                                 return;
                             }
@@ -137,8 +131,10 @@ protected override void Initialize(SonarAnalysisContext context) =>
                         });
                 });
 
-        private static IEnumerable<Diagnostic> GetDiagnosticsForUnusedPrivateMembers(CSharpSymbolUsageCollector usageCollector, ISet<ISymbol> removableSymbols, string accessibility,
-            BidirectionalDictionary<ISymbol, SyntaxNode> fieldLikeSymbols)
+        private static IEnumerable<Diagnostic> GetDiagnosticsForUnusedPrivateMembers(CSharpSymbolUsageCollector usageCollector,
+                                                                                     ISet<ISymbol> removableSymbols,
+                                                                                     string accessibility,
+                                                                                     BidirectionalDictionary<ISymbol, SyntaxNode> fieldLikeSymbols)
         {
             var unusedSymbols = GetUnusedSymbols(usageCollector, removableSymbols);
 
@@ -181,7 +177,9 @@ private static IEnumerable<Diagnostic> GetDiagnosticsForUnreadFields(IEnumerable
         private static string GetFieldAccessibilityForMessage(ISymbol symbol) =>
             symbol.DeclaredAccessibility == Accessibility.Private ? "private" : "private class";
 
-        private static IEnumerable<Diagnostic> GetDiagnosticsForMembers(ICollection<ISymbol> unusedSymbols, string accessibility, BidirectionalDictionary<ISymbol, SyntaxNode> fieldLikeSymbols)
+        private static IEnumerable<Diagnostic> GetDiagnosticsForMembers(ICollection<ISymbol> unusedSymbols,
+                                                                        string accessibility,
+                                                                        BidirectionalDictionary<ISymbol, SyntaxNode> fieldLikeSymbols)
         {
             var diagnostics = new List<Diagnostic>();
             var alreadyReportedFieldLikeSymbols = new HashSet<ISymbol>();

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/UnusedPrivateMemberTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/UnusedPrivateMemberTest.cs
@@ -203,26 +203,24 @@ public void UnusedPrivateMember_FromCSharp8() =>
         [TestMethod]
         public void UnusedPrivateMember_FromCSharp9() =>
             Verifier.VerifyAnalyzerFromCSharp9Library(
-                @"TestCases\UnusedPrivateMember.CSharp9.cs",
+                new[]
+                {
+                    @"TestCases\UnusedPrivateMember.CSharp9.cs",
+                    @"TestCases\UnusedPrivateMember.CSharp9.Second.cs"
+                },
                 new UnusedPrivateMember());
 
         [TestMethod]
         public void UnusedPrivateMember_FromCSharp9_TopLevelStatements() =>
-            Verifier.VerifyAnalyzerFromCSharp9Console(
-                @"TestCases\UnusedPrivateMember.CSharp9.TopLevelStatements.cs",
-                new UnusedPrivateMember());
+            Verifier.VerifyAnalyzerFromCSharp9Console(@"TestCases\UnusedPrivateMember.CSharp9.TopLevelStatements.cs", new UnusedPrivateMember());
 
         [TestMethod]
         public void UnusedPrivateMember_FromCSharp10() =>
-            Verifier.VerifyAnalyzerFromCSharp10Library(
-                @"TestCases\UnusedPrivateMember.CSharp10.cs",
-                new UnusedPrivateMember());
+            Verifier.VerifyAnalyzerFromCSharp10Library(@"TestCases\UnusedPrivateMember.CSharp10.cs", new UnusedPrivateMember());
 
         [TestMethod]
         public void UnusedPrivateMember_FromCSharpPreview() =>
-            Verifier.VerifyAnalyzerCSharpPreviewLibrary(
-                @"TestCases\UnusedPrivateMember.CSharpPreview.cs",
-                new UnusedPrivateMember());
+            Verifier.VerifyAnalyzerCSharpPreviewLibrary(@"TestCases\UnusedPrivateMember.CSharpPreview.cs", new UnusedPrivateMember());
 #endif
 
         [TestMethod]

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp7.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp7.cs
@@ -159,4 +159,23 @@ public class EmptyCtor
         // That's invalid syntax, but it is still empty ctor and we should not raise for it, even if it is not used
         public EmptyCtor() => // Error [CS1525,CS1002]
     }
+
+    public class WithEnums
+    {
+        private enum X // Noncompliant
+        {
+            A
+        }
+
+        public void UseEnum()
+        {
+            var b = Y.B;
+        }
+
+        private enum Y
+        {
+            A,
+            B
+        }
+    }
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp9.Second.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp9.Second.cs
@@ -0,0 +1,7 @@
+namespace Tests.Diagnostics
+{
+    public partial class PartialMethods
+    {
+        partial void UnusedMethod(); // Noncompliant
+    }
+}
diff --git a/.../tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp9.TopLevelStatements.cs b/.../tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp9.TopLevelStatements.cs
@@ -13,19 +13,16 @@ public record Product(string Name, int CategoryId);
 public record Record
 {
     private int a; // Noncompliant {{Remove the unused private field 'a'.}}
-                   // Noncompliant@-1 - duplicate issue reported
 
     private int b;
     public int B() => b;
 
     private nint Value { get; init; }
     private nint UnusedValue { get; init; } // Noncompliant
-                                            // Noncompliant@-1 - duplicate issue reported
 
     public Record Create() => new() { Value = 1 };
 
     private interface IFoo // Noncompliant
-                           // Noncompliant@-1 - duplicate issue reported
     {
         public void Bar() { }
     }
@@ -45,7 +42,6 @@ public void UseNested2()
     }
 
     private record UnusedNested1(string Name, int CategoryId);   // Noncompliant
-                                                                 // Noncompliant@-1 - duplicate issue reported
     internal record UnusedNested2(string Name, int CategoryId);  // Noncompliant
     public record UnusedNested3(string Name, int CategoryId);
 
@@ -76,7 +72,6 @@ private TargetTypedNew(int arg)
     }
 
     private TargetTypedNew(string arg) // Noncompliant - FP
-                                       // Noncompliant@-1 - duplicate issue reported
     {
         var x = arg;
     }

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp9.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp9.cs
@@ -100,4 +100,8 @@ public static void Foo()
             PositionalRecord @record = new PositionalRecord("");
         }
     }
+
+    public partial class PartialMethods
+    {
+    }
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/Verifier.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/Verifier.cs
@@ -112,11 +112,11 @@ public static void VerifyAnalyzerFromCSharp9Console(string path, DiagnosticAnaly
 
         public static void VerifyAnalyzerFromCSharp10Console(string path, DiagnosticAnalyzer diagnosticAnalyzer, IEnumerable<MetadataReference> additionalReferences = null) =>
             VerifyNonConcurrentAnalyzer(new[] { path },
-                                new[] { diagnosticAnalyzer },
-                                ParseOptionsHelper.FromCSharp10,
-                                CompilationErrorBehavior.Default,
-                                OutputKind.ConsoleApplication,
-                                additionalReferences);
+                                        new[] { diagnosticAnalyzer },
+                                        ParseOptionsHelper.FromCSharp10,
+                                        CompilationErrorBehavior.Default,
+                                        OutputKind.ConsoleApplication,
+                                        additionalReferences);
 
         /// <summary>
         /// Verify analyzer from C# 9 with top level statements in non-concurrent execution mode.
-Original file line number
+Diff line change
@@ Expand Up / @@ -100,4 +100,8 @@ public static void Foo() @@
                 PositionalRecord @record = new PositionalRecord("");
             }
         }
+        public partial class PartialMethods
+        {
+        }
     }