Skip to content

Commit

Permalink
Rule S3909: Collections should implement the generic interface
Browse files Browse the repository at this point in the history
  • Loading branch information
michalb-sonar committed May 31, 2017
1 parent c4a5826 commit c4f4a7c
Show file tree
Hide file tree
Showing 11 changed files with 342 additions and 3 deletions.
43 changes: 43 additions & 0 deletions sonaranalyzer-dotnet/rspec/cs/S3909_c#.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<p>The NET Framework 2.0 introduced the generic interface <code>System.Collections.Generic.IEnumerable&lt;T&gt;</code> and it should be preferred over
the older, non generic, interfaces.</p>
<p>This rule raises an issue when a public type implements <code>System.Collections.IEnumerable</code>.</p>
<h2>Noncompliant Code Example</h2>
<pre>
using System;
using System.Collections;

public class MyData
{
public MyData()
{
}
}

public class MyList : CollectionBase // Noncompliant
{
public void Add(MyData data)
{
InnerList.Add(data);
}

// ...
}
</pre>
<h2>Compliant Solution</h2>
<pre>
using System;
using System.Collections.ObjectModel;

public class MyData
{
public MyData()
{
}
}

public class MyList : Collection&lt;MyData&gt;
{
// Implementation...
}
</pre>

13 changes: 13 additions & 0 deletions sonaranalyzer-dotnet/rspec/cs/S3909_c#.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"title": "Collections should implement the generic interface",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "15min"
},
"tags": [

],
"defaultSeverity": "Major"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2017 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System.Collections.Generic;
using System.Linq;

namespace SonarAnalyzer.Helpers
{
public static class CollectionUtils
{
public static HashSet<T> ToHashSet<T>(this IEnumerable<T> enumerable)
{
if (enumerable == null)
{
return new HashSet<T>();
}

return new HashSet<T>(enumerable);
}
}
}
31 changes: 29 additions & 2 deletions sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/RspecStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -5856,6 +5856,33 @@
<data name="S3904_Type" xml:space="preserve">
<value>BUG</value>
</data>
<data name="S3909_Category" xml:space="preserve">
<value>Sonar Code Smell</value>
</data>
<data name="S3909_Description" xml:space="preserve">
<value>The NET Framework 2.0 introduced the generic interface System.Collections.Generic.IEnumerable&lt;T&gt; and it should be preferred over the older, non generic, interfaces.</value>
</data>
<data name="S3909_IsActivatedByDefault" xml:space="preserve">
<value>False</value>
</data>
<data name="S3909_Remediation" xml:space="preserve">
<value>Constant/Issue</value>
</data>
<data name="S3909_RemediationCost" xml:space="preserve">
<value>15min</value>
</data>
<data name="S3909_Severity" xml:space="preserve">
<value>Major</value>
</data>
<data name="S3909_Tags" xml:space="preserve">
<value />
</data>
<data name="S3909_Title" xml:space="preserve">
<value>Collections should implement the generic interface</value>
</data>
<data name="S3909_Type" xml:space="preserve">
<value>CODE_SMELL</value>
</data>
<data name="S3925_Category" xml:space="preserve">
<value>Sonar Bug</value>
</data>
Expand Down Expand Up @@ -6346,7 +6373,7 @@
<value>Sonar Code Smell</value>
</data>
<data name="S4002_Description" xml:space="preserve">
<value>This rule raises an issue when a disposable type contains fields of the following types and does not implement a finalizer:</value>
<value>This rule raises an issue when a disposable type contains fields of the following types and does not implement a proper finalizer:</value>
</data>
<data name="S4002_IsActivatedByDefault" xml:space="preserve">
<value>False</value>
Expand Down Expand Up @@ -6412,7 +6439,7 @@
<value />
</data>
<data name="S4005_Title" xml:space="preserve">
<value>"System.Uri" arguments should be used instead of strings</value>
<value>"System.Uri" argument should be passed instead of string</value>
</data>
<data name="S4005_Type" xml:space="preserve">
<value>CODE_SMELL</value>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2017 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using SonarAnalyzer.Common;
using SonarAnalyzer.Helpers;

namespace SonarAnalyzer.Rules.CSharp
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
[Rule(DiagnosticId)]
public sealed class CollectionsShouldImplementGenericInterface : SonarDiagnosticAnalyzer
{
internal const string DiagnosticId = "S3909";
private const string MessageFormat = "Refactor this collection to implement '{0}'.";

private static readonly DiagnosticDescriptor rule =
DiagnosticDescriptorBuilder.GetDescriptor(DiagnosticId, MessageFormat, RspecStrings.ResourceManager);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(rule);

private static readonly Dictionary<KnownType, KnownType> nongenericToGenericMapping =
new Dictionary<KnownType, KnownType>
{
[KnownType.System_Collections_ICollection] = KnownType.System_Collections_Generic_ICollection_T,
[KnownType.System_Collections_IList] = KnownType.System_Collections_Generic_IList_T,
[KnownType.System_Collections_IEnumerable] = KnownType.System_Collections_Generic_IEnumerable_T,
[KnownType.System_Collections_CollectionBase] = KnownType.System_Collections_ObjectModel_Collection_T,
};

private static readonly ISet<KnownType> nongenericTypes = nongenericToGenericMapping.Keys.ToHashSet();
private static readonly ISet<KnownType> genericTypes = nongenericToGenericMapping.Values.ToHashSet();

protected override void Initialize(SonarAnalysisContext context)
{
context.RegisterSyntaxNodeActionInNonGenerated(
c =>
{
var classDeclaration = c.Node as ClassDeclarationSyntax;
var issues = new List<Diagnostic>();
var implementedTypes = classDeclaration?.BaseList?.Types;
if (implementedTypes == null)
{
return;
}
foreach (var typeSyntax in implementedTypes)
{
var typeSymbol = c.SemanticModel.GetSymbolInfo(typeSyntax.Type).Symbol?.GetSymbolType();
var originalDefinition = typeSymbol?.OriginalDefinition;
if (originalDefinition.IsAny(genericTypes))
{
return;
}
var nongenericType = ToKnownType(typeSymbol, nongenericTypes);
if (nongenericType != null)
{
issues.Add(Diagnostic.Create(rule,
classDeclaration.Identifier.GetLocation(),
nongenericToGenericMapping[nongenericType].TypeName));
}
}
issues.ForEach(i => c.ReportDiagnostic(i));
},
SyntaxKind.ClassDeclaration);
}

private static KnownType ToKnownType(ITypeSymbol typeSymbol, IEnumerable<KnownType> knownTypes)
{
return knownTypes.FirstOrDefault(t => typeSymbol.Is(t));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
<Link>Properties\AssemblyInfo.Shared.cs</Link>
</Compile>
<Compile Include="Helpers\CognitiveComplexityWalker.cs" />
<Compile Include="Helpers\CollectionUtils.cs" />
<Compile Include="Helpers\ControlFlowGraph\Blocks\ForeachCollectionProducerBlock.cs" />
<Compile Include="Helpers\ControlFlowGraph\Blocks\ForInitializerBlock.cs" />
<Compile Include="Helpers\ControlFlowGraph\Blocks\LockBlock.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ internal sealed class KnownType
public static readonly KnownType System_Collections_Generic_IList_T = new KnownType(SpecialType.System_Collections_Generic_IList_T, "System.Collections.Generic.IList<T>");
public static readonly KnownType System_Collections_Generic_ICollection_T = new KnownType(SpecialType.System_Collections_Generic_ICollection_T, "System.Collections.Generic.ICollection<T>");
public static readonly KnownType System_Collections_ICollection = new KnownType("System.Collections.ICollection");
public static readonly KnownType System_Collections_CollectionBase = new KnownType("System.Collections.CollectionBase");

public static readonly KnownType System_Object = new KnownType(SpecialType.System_Object, "object");
public static readonly KnownType System_String = new KnownType(SpecialType.System_String, "string");
Expand Down Expand Up @@ -179,6 +180,7 @@ internal sealed class KnownType
public static readonly KnownType System_Collections_Immutable_ImmutableStack_T = new KnownType("System.Collections.Immutable.ImmutableStack<T>");
public static readonly KnownType System_Collections_Immutable_IImmutableStack_T = new KnownType("System.Collections.Immutable.IImmutableStack<T>");

public static readonly KnownType System_Collections_ObjectModel_Collection_T = new KnownType("System.Collections.ObjectModel.Collection<T>");
public static readonly KnownType System_Collections_ObjectModel_ReadOnlyCollection_T = new KnownType("System.Collections.ObjectModel.ReadOnlyCollection<T>");
public static readonly KnownType System_Collections_ObjectModel_ReadOnlyDictionary_TKey_TValue = new KnownType("System.Collections.ObjectModel.ReadOnlyDictionary<TKey, TValue>");

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<p>The NET Framework 2.0 introduced the generic interface <code>System.Collections.Generic.IEnumerable&lt;T&gt;</code> and it should be preferred over
the older, non generic, interfaces.</p>
<p>This rule raises an issue when a public type implements <code>System.Collections.IEnumerable</code>.</p>
<h2>Noncompliant Code Example</h2>
<pre>
using System;
using System.Collections;

public class MyData
{
public MyData()
{
}
}

public class MyList : CollectionBase // Noncompliant
{
public void Add(MyData data)
{
InnerList.Add(data);
}

// ...
}
</pre>
<h2>Compliant Solution</h2>
<pre>
using System;
using System.Collections.ObjectModel;

public class MyData
{
public MyData()
{
}
}

public class MyList : Collection&lt;MyData&gt;
{
// Implementation...
}
</pre>

Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ public class RuleTypeTests
["3902"] = "CODE_SMELL",
["3903"] = "BUG",
["3904"] = "BUG",
["3909"] = "CODE_SMELL",
["3925"] = "BUG",
["3926"] = "BUG",
["3928"] = "CODE_SMELL",
Expand Down Expand Up @@ -379,7 +380,7 @@ private static void DetectTypeChanges(ResourceManager resourceManager, IImmutabl
deletedRules.Should().BeEmpty($"YOU SHOULD NEVER DELETE RULES!");

// IMPORTANT: If this test fails, you should update the types of the changed rules
// in the dictionaries above. Also add a GitHub issue specifying the change of type
// in the dictionaries above. Also add a GitHub issue specifying the change of type
// and update peach and next.
var changedRules = items.Where(x => x.ActualType != null && x.ExpectedType != null);
changedRules.Should().BeEmpty($"you need to change the rules in {expectedTypesName}.");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2017 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using Microsoft.VisualStudio.TestTools.UnitTesting;
using SonarAnalyzer.Rules.CSharp;

namespace SonarAnalyzer.UnitTest.Rules
{
[TestClass]
public class CollectionsShouldImplementGenericInterfaceTest
{
[TestMethod]
[TestCategory("Rule")]
public void CollectionsShouldImplementGenericInterface()
{
Verifier.VerifyAnalyzer(@"TestCases\CollectionsShouldImplementGenericInterface.cs",
new CollectionsShouldImplementGenericInterface());
}
}
}
Loading

0 comments on commit c4f4a7c

Please sign in to comment.