Skip to content

Commit

Permalink
Update S1144 to handle unused internal types
Browse files Browse the repository at this point in the history
  • Loading branch information
Amaury Levé committed Apr 28, 2017
1 parent 1a04b3c commit 139c6b9
Show file tree
Hide file tree
Showing 6 changed files with 138 additions and 37 deletions.
13 changes: 13 additions & 0 deletions ...ed/akka.net/Akka.Cluster.Sharding.Tests-{B369F314-4D37-4AC1-B4FB-64FF671AF0BC}-S1144.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@
"issues": [
{
"id": "S1144",
"message": "Remove the unused internal type 'IllustrateGracefulShutdown'.",
"location": {
"uri": "akka.net\src\contrib\cluster\Akka.Cluster.Sharding.Tests\ClusterShardingGracefulShutdownSpec.cs",
"region": {
"startLine": 71,
"startColumn": 9,
"endLine": 95,
"endColumn": 10
}
}
},
{
"id": "S1144",
"message": "Remove the unused private method 'CreateCoordinator'.",
"location": {
"uri": "akka.net\src\contrib\cluster\Akka.Cluster.Sharding.Tests\ClusterShardingSpec.cs",
Expand Down
13 changes: 13 additions & 0 deletions ...xpected/akka.net/Akka.Persistence.Tests-{4492004A-A8D0-45B0-BACC-05BA2F38EC55}-S1144.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,19 @@
},
{
"id": "S1144",
"message": "Remove the unused internal type 'FailingMemoryStore'.",
"location": {
"uri": "akka.net\src\core\Akka.Persistence.Tests\PersistentActorFailureSpec.cs",
"region": {
"startLine": 33,
"startColumn": 9,
"endLine": 54,
"endColumn": 10
}
}
},
{
"id": "S1144",
"message": "Remove the unused private field '_counter'.",
"location": {
"uri": "akka.net\src\core\Akka.Persistence.Tests\PersistentActorSpec.Actors.cs",
Expand Down
13 changes: 13 additions & 0 deletions ...dotnet/its/expected/akka.net/Akka.Tests-{3F786C7D-70E3-4836-8B33-EC9A5AF72330}-S1144.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,19 @@
},
{
"id": "S1144",
"message": "Remove the unused internal type 'Registration'.",
"location": {
"uri": "akka.net\src\core\Akka.Tests\IO\TcpConnectionSpec.cs",
"region": {
"startLine": 75,
"startColumn": 9,
"endLine": 85,
"endColumn": 10
}
}
},
{
"id": "S1144",
"message": "Remove the unused private method 'ByteStrings'.",
"location": {
"uri": "akka.net\src\core\Akka.Tests\Util\ByteStringSpec.cs",
Expand Down
133 changes: 96 additions & 37 deletions sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
Expand All @@ -27,7 +29,6 @@
using Microsoft.CodeAnalysis.Diagnostics;
using SonarAnalyzer.Common;
using SonarAnalyzer.Helpers;
using System;

namespace SonarAnalyzer.Rules.CSharp
{
Expand All @@ -36,7 +37,7 @@ namespace SonarAnalyzer.Rules.CSharp
public class UnusedPrivateMember : SonarDiagnosticAnalyzer
{
internal const string DiagnosticId = "S1144";
private const string MessageFormat = "Remove the unused private {0} '{1}'.";
private const string MessageFormat = "Remove the unused {0} {1} '{2}'.";
private const IdeVisibility ideVisibility = IdeVisibility.Hidden;

private static readonly DiagnosticDescriptor rule =
Expand All @@ -47,46 +48,91 @@ public class UnusedPrivateMember : SonarDiagnosticAnalyzer

private static readonly Accessibility maxAccessibility = Accessibility.Private;

private ConcurrentBag<Diagnostic> possibleUnusedInternalMembers;

protected sealed override void Initialize(SonarAnalysisContext context)
{
context.RegisterSymbolAction(
context.RegisterCompilationStartAction(
c =>
{
var namedType = (INamedTypeSymbol)c.Symbol;
if (!namedType.IsClassOrStruct() ||
namedType.ContainingType != null)
{
return;
}
var shouldRaise = true;
possibleUnusedInternalMembers = new ConcurrentBag<Diagnostic>();
var declarationCollector = new RemovableDeclarationCollector(namedType, c.Compilation);
c.RegisterSemanticModelAction(
cc =>
{
var isInternalsVisibleToAttributeFound = cc.SemanticModel.SyntaxTree.GetRoot()
.DescendantNodes()
.OfType<AttributeListSyntax>()
.SelectMany(list => list.Attributes)
.Any(a => IsInternalVisibleToAttribute(a, cc.SemanticModel));
if (isInternalsVisibleToAttributeFound)
{
shouldRaise = false;
}
});
var declaredPrivateSymbols = new HashSet<ISymbol>();
var fieldLikeSymbols = new BidirectionalDictionary<ISymbol, SyntaxNode>();
c.RegisterSymbolAction(
cc =>
{
var namedType = (INamedTypeSymbol)cc.Symbol;
if (!namedType.IsClassOrStruct() ||
namedType.ContainingType != null)
{
return;
}
CollectRemovableNamedTypes(declarationCollector, declaredPrivateSymbols);
CollectRemovableFieldLikeDeclarations(declarationCollector, declaredPrivateSymbols, fieldLikeSymbols);
CollectRemovableEventsAndProperties(declarationCollector, declaredPrivateSymbols);
CollectRemovableMethods(declarationCollector, declaredPrivateSymbols);
var declarationCollector = new RemovableDeclarationCollector(namedType, cc.Compilation);
if (!declaredPrivateSymbols.Any())
{
return;
}
var declaredPrivateSymbols = new HashSet<ISymbol>();
var fieldLikeSymbols = new BidirectionalDictionary<ISymbol, SyntaxNode>();
var usedSymbols = new HashSet<ISymbol>();
var emptyConstructors = new HashSet<ISymbol>();
CollectRemovableNamedTypes(declarationCollector, declaredPrivateSymbols);
CollectRemovableFieldLikeDeclarations(declarationCollector, declaredPrivateSymbols, fieldLikeSymbols);
CollectRemovableEventsAndProperties(declarationCollector, declaredPrivateSymbols);
CollectRemovableMethods(declarationCollector, declaredPrivateSymbols);
var propertyAccessorAccess = new Dictionary<IPropertySymbol, AccessorAccess>();
if (!declaredPrivateSymbols.Any())
{
return;
}
var usedSymbols = new HashSet<ISymbol>();
var emptyConstructors = new HashSet<ISymbol>();
var propertyAccessorAccess = new Dictionary<IPropertySymbol, AccessorAccess>();
CollectUsedSymbols(declarationCollector, usedSymbols, declaredPrivateSymbols, propertyAccessorAccess);
CollectUsedSymbolsFromCtorInitializerAndCollectEmptyCtors(declarationCollector,
usedSymbols, emptyConstructors);
ReportIssues(cc, usedSymbols, declaredPrivateSymbols, emptyConstructors, fieldLikeSymbols);
ReportUnusedPropertyAccessors(cc, usedSymbols, declaredPrivateSymbols, propertyAccessorAccess);
},
SymbolKind.NamedType);
c.RegisterCompilationEndAction(
cc =>
{
if (!shouldRaise)
{
return;
}
foreach (var diagnostic in possibleUnusedInternalMembers)
{
cc.ReportDiagnosticIfNonGenerated(diagnostic, cc.Compilation);
}
});
});
}

CollectUsedSymbols(declarationCollector, usedSymbols, declaredPrivateSymbols, propertyAccessorAccess);
CollectUsedSymbolsFromCtorInitializerAndCollectEmptyCtors(declarationCollector,
usedSymbols, emptyConstructors);
private static bool IsInternalVisibleToAttribute(AttributeSyntax attribute, SemanticModel semanticModel)
{
var attributeConstructor = semanticModel.GetSymbolInfo(attribute).Symbol as IMethodSymbol;

ReportIssues(c, usedSymbols, declaredPrivateSymbols, emptyConstructors, fieldLikeSymbols);
ReportUnusedPropertyAccessors(c, usedSymbols, declaredPrivateSymbols, propertyAccessorAccess);
},
SymbolKind.NamedType);
return attributeConstructor != null &&
attributeConstructor.ContainingType.Is(KnownType.System_Runtime_CompilerServices_InternalsVisibleToAttribute);
}

private static void ReportUnusedPropertyAccessors(SymbolAnalysisContext context, HashSet<ISymbol> usedSymbols,
Expand All @@ -109,7 +155,8 @@ private static void ReportUnusedPropertyAccessors(SymbolAnalysisContext context,
var accessorSyntax = GetAccessorSyntax(property.SetMethod);
if (accessorSyntax != null)
{
context.ReportDiagnosticIfNonGenerated(Diagnostic.Create(rule, accessorSyntax.GetLocation(), "set accessor in property", property.Name), context.Compilation);
context.ReportDiagnosticIfNonGenerated(Diagnostic.Create(rule, accessorSyntax.GetLocation(),
"private", "set accessor in property", property.Name), context.Compilation);
}
continue;
}
Expand All @@ -119,7 +166,8 @@ private static void ReportUnusedPropertyAccessors(SymbolAnalysisContext context,
var accessorSyntax = GetAccessorSyntax(property.GetMethod);
if (accessorSyntax != null && accessorSyntax.Body != null)
{
context.ReportDiagnosticIfNonGenerated(Diagnostic.Create(rule, accessorSyntax.GetLocation(), "get accessor in property", property.Name), context.Compilation);
context.ReportDiagnosticIfNonGenerated(Diagnostic.Create(rule, accessorSyntax.GetLocation(),
"private", "get accessor in property", property.Name), context.Compilation);
}
}
}
Expand All @@ -130,7 +178,7 @@ private static AccessorDeclarationSyntax GetAccessorSyntax(IMethodSymbol methodS
return methodSymbol?.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax() as AccessorDeclarationSyntax;
}

private static void ReportIssues(SymbolAnalysisContext context, HashSet<ISymbol> usedSymbols,
private void ReportIssues(SymbolAnalysisContext context, HashSet<ISymbol> usedSymbols,
HashSet<ISymbol> declaredPrivateSymbols, HashSet<ISymbol> emptyConstructors,
BidirectionalDictionary<ISymbol, SyntaxNode> fieldLikeSymbols)
{
Expand Down Expand Up @@ -180,10 +228,21 @@ private static void ReportIssues(SymbolAnalysisContext context, HashSet<ISymbol>

var memberKind = GetMemberType(unused.Symbol);
var memberName = GetMemberName(unused.Symbol);
var effectiveAccessibility = unused.Symbol.GetEffectiveAccessibility();

if (effectiveAccessibility == Accessibility.Internal)
{
// We can't report internal members directly because they might be used through another assembly.
possibleUnusedInternalMembers.Add(
Diagnostic.Create(rule, location, "internal", memberKind, memberName));
continue;
}

context.ReportDiagnosticIfNonGenerated(
Diagnostic.Create(rule, location, memberKind, memberName),
context.Compilation);
if (effectiveAccessibility == Accessibility.Private)
{
context.ReportDiagnosticIfNonGenerated(
Diagnostic.Create(rule, location, "private", memberKind, memberName), context.Compilation);
}
}
}

Expand Down Expand Up @@ -245,7 +304,7 @@ private static void CollectRemovableNamedTypes(RemovableDeclarationCollector dec
SemanticModel = container.SemanticModel
}))
.Select(node => node.SemanticModel.GetDeclaredSymbol(node.SyntaxNode))
.Where(symbol => RemovableDeclarationCollector.IsRemovable(symbol, maxAccessibility));
.Where(symbol => RemovableDeclarationCollector.IsRemovable(symbol, Accessibility.Internal));

declaredPrivateSymbols.UnionWith(symbols);
}
Expand Down
2 changes: 2 additions & 0 deletions sonaranalyzer-dotnet/src/SonarAnalyzer.Common/Helpers/KnownType.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ internal sealed class KnownType
public static readonly KnownType System_Diagnostics_Trace = new KnownType("System.Diagnostics.Trace");
public static readonly KnownType System_Diagnostics_TraceSource = new KnownType("System.Diagnostics.TraceSource");

public static readonly KnownType System_Runtime_CompilerServices_InternalsVisibleToAttribute = new KnownType("System.Runtime.CompilerServices.InternalsVisibleToAttribute");

#endregion

public string TypeName { get; }
Expand Down
1 change: 1 addition & 0 deletions sonaranalyzer-dotnet/src/Tests/SonarAnalyzer.UnitTest/TestCases/UnusedPrivateMember.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ public int this[int index]
get { return 42; }
}
}
internal class Class4 : MyInterface { } // Noncompliant {{Remove the unused internal type 'Class4'.}}
}
public static class MyExtension
{
Expand Down

0 comments on commit 139c6b9

Please sign in to comment.