Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Nov 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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();
}
}
24 changes: 11 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,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;
}
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
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
{
}
}
10 changes: 5 additions & 5 deletions analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/Verifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down