From e3f95d8e74fdf82f1287f3d046118e5de4994950 Mon Sep 17 00:00:00 2001 From: Costin Zaharia Date: Mon, 22 Nov 2021 15:28:24 +0100 Subject: [PATCH] Rule S1144: remove duplicate issues in top level statements - the symbols were processed twice, once for the Program symbol and the second time for the types defined in the top level statement file. --- .../Rules/UnusedPrivateMember.cs | 39 ++++++++++++------- .../Rules/UnusedPrivateMemberTest.cs | 6 ++- .../UnusedPrivateMember.CSharp9.Second.cs | 7 ++++ ...rivateMember.CSharp9.TopLevelStatements.cs | 5 --- .../TestCases/UnusedPrivateMember.CSharp9.cs | 4 ++ 5 files changed, 42 insertions(+), 19 deletions(-) create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp9.Second.cs diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs index 8d2510a1801..470ee8f065a 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs @@ -72,16 +72,11 @@ 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. + || IsTopLevelProgram(namedType) + || !IsType(namedType) + || namedType.DerivesFromAny(IgnoredTypes)) { return; } @@ -137,8 +132,24 @@ protected override void Initialize(SonarAnalysisContext context) => }); }); - private static IEnumerable GetDiagnosticsForUnusedPrivateMembers(CSharpSymbolUsageCollector usageCollector, ISet removableSymbols, string accessibility, - BidirectionalDictionary fieldLikeSymbols) + private static bool IsTopLevelProgram(ISymbol symbol) => + // If the symbol is named `Program` and the containing namespace is the global one, we will assume that we are analyzing a top level file. + // We cannot tell for sure, but in this case we prefer to have FNs instead of FPs, considering that there is a low chance to have + // a program class in the global namespace. + symbol.Name == "Program" + && symbol.ContainingNamespace.IsGlobalNamespace; + + private static bool IsType(ITypeSymbol namedType) => + namedType.TypeKind == TypeKind.Struct + || namedType.TypeKind == TypeKind.Class + || namedType.TypeKind == TypeKind.Delegate + || namedType.TypeKind == TypeKind.Enum + || namedType.TypeKind == TypeKind.Interface; + + private static IEnumerable GetDiagnosticsForUnusedPrivateMembers(CSharpSymbolUsageCollector usageCollector, + ISet removableSymbols, + string accessibility, + BidirectionalDictionary fieldLikeSymbols) { var unusedSymbols = GetUnusedSymbols(usageCollector, removableSymbols); @@ -181,7 +192,9 @@ private static IEnumerable GetDiagnosticsForUnreadFields(IEnumerable private static string GetFieldAccessibilityForMessage(ISymbol symbol) => symbol.DeclaredAccessibility == Accessibility.Private ? "private" : "private class"; - private static IEnumerable GetDiagnosticsForMembers(ICollection unusedSymbols, string accessibility, BidirectionalDictionary fieldLikeSymbols) + private static IEnumerable GetDiagnosticsForMembers(ICollection unusedSymbols, + string accessibility, + BidirectionalDictionary fieldLikeSymbols) { var diagnostics = new List(); var alreadyReportedFieldLikeSymbols = new HashSet(); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/UnusedPrivateMemberTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/UnusedPrivateMemberTest.cs index 9087a22ef92..a863a7f7afc 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/UnusedPrivateMemberTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/UnusedPrivateMemberTest.cs @@ -203,7 +203,11 @@ 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] diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp9.Second.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp9.Second.cs new file mode 100644 index 00000000000..5fd8d0fab90 --- /dev/null +++ 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/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp9.TopLevelStatements.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp9.TopLevelStatements.cs index 11d0b0d394c..1c94da7bba1 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.CSharp9.TopLevelStatements.cs +++ b/analyzers/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 index f988aa6c89a..21037b3750a 100644 --- 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 + { + } }