Skip to content

Commit

Permalink
Rule S1144: remove duplicate issues in top level statements
Browse files Browse the repository at this point in the history
- the symbols were processed twice, once for the Program symbol and
the second time for the types defined in the top level statement file.
  • Loading branch information
costin-zaharia-sonarsource committed Nov 22, 2021
1 parent 8a4854b commit e3f95d8
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 19 deletions.
39 changes: 26 additions & 13 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -137,8 +132,24 @@ protected override void Initialize(SonarAnalysisContext context) =>
});
});

private static IEnumerable<Diagnostic> GetDiagnosticsForUnusedPrivateMembers(CSharpSymbolUsageCollector usageCollector, ISet<ISymbol> removableSymbols, string accessibility,
BidirectionalDictionary<ISymbol, SyntaxNode> 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<Diagnostic> GetDiagnosticsForUnusedPrivateMembers(CSharpSymbolUsageCollector usageCollector,
ISet<ISymbol> removableSymbols,
string accessibility,
BidirectionalDictionary<ISymbol, SyntaxNode> fieldLikeSymbols)
{
var unusedSymbols = GetUnusedSymbols(usageCollector, removableSymbols);

Expand Down Expand Up @@ -181,7 +192,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>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
namespace Tests.Diagnostics
{
public partial class PartialMethods
{
partial void UnusedMethod(); // Noncompliant
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() { }
}
Expand All @@ -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);

Expand Down Expand Up @@ -76,7 +72,6 @@ private TargetTypedNew(int arg)
}

private TargetTypedNew(string arg) // Noncompliant - FP
// Noncompliant@-1 - duplicate issue reported
{
var x = arg;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,8 @@ public static void Foo()
PositionalRecord @record = new PositionalRecord("");
}
}

public partial class PartialMethods
{
}
}

0 comments on commit e3f95d8

Please sign in to comment.