Rule S3909: Collections should implement the generic interface (#389)

Rule S3909: Collections should implement the generic interface

sonaranalyzer-dotnet/rspec/cs/S3909_c#.html

@@ -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>
+

sonaranalyzer-dotnet/rspec/cs/S3909_c#.json

@@ -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"
+}

sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Helpers/CollectionUtils.cs

@@ -0,0 +1,53 @@
+/*
+ * 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;
+
+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);
+        }
+
+        public static TValue GetValueOrDefault<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key)
+        {
+            return dictionary.GetValueOrDefault(key, default(TValue));
+        }
+
+        public static TValue GetValueOrDefault<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, TValue defaultValue)
+        {
+            TValue result;
+            if (dictionary.TryGetValue(key, out result))
+            {
+                return result;
+            }
+
+            return defaultValue;
+        }
+    }
+}

sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/RspecStrings.resx

@@ -5937,6 +5937,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>

sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/CollectionsShouldImplementGenericInterface.cs

@@ -0,0 +1,99 @@
+/*
+ * 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 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<string, KnownType> nongenericToGenericMapping =
+            new Dictionary<string, KnownType>
+            {
+                [KnownType.System_Collections_ICollection.TypeName] = KnownType.System_Collections_Generic_ICollection_T,
+                [KnownType.System_Collections_IList.TypeName] = KnownType.System_Collections_Generic_IList_T,
+                [KnownType.System_Collections_IEnumerable.TypeName] = KnownType.System_Collections_Generic_IEnumerable_T,
+                [KnownType.System_Collections_CollectionBase.TypeName] = KnownType.System_Collections_ObjectModel_Collection_T,
+            };
+
+        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 implementedTypes = classDeclaration?.BaseList?.Types;
+                    if (implementedTypes == null)
+                    {
+                        return;
+                    }
+
+                    var issues = new List<Diagnostic>();
+                    foreach (var typeSyntax in implementedTypes)
+                    {
+                        var typeSymbol = c.SemanticModel.GetSymbolInfo(typeSyntax.Type).Symbol?.GetSymbolType();
+                        if (typeSymbol == null)
+                        {
+                            continue;
+                        }
+
+                        if (typeSymbol.OriginalDefinition.IsAny(genericTypes))
+                        {
+                            return;
+                        }
+
+                        var suggestedGenericType = SuggestGenericCollectionType(typeSymbol);
+                        if (suggestedGenericType != null)
+                        {
+                            issues.Add(Diagnostic.Create(rule,
+                                        classDeclaration.Identifier.GetLocation(),
+                                        suggestedGenericType));
+                        }
+                    }
+
+                    issues.ForEach(c.ReportDiagnostic);
+                },
+                SyntaxKind.ClassDeclaration);
+        }
+
+        private static string SuggestGenericCollectionType(ITypeSymbol typeSymbol)
+        {
+            return nongenericToGenericMapping.GetValueOrDefault(typeSymbol.ToDisplayString())?.TypeName;
+        }
+    }
+}

sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/SonarAnalyzer.CSharp.csproj

@@ -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" />

sonaranalyzer-dotnet/src/SonarAnalyzer.Common/Helpers/KnownType.cs

@@ -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");
@@ -181,6 +182,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>");
 

sonaranalyzer-dotnet/src/SonarAnalyzer.Utilities/Rules.Description/S3909.html

@@ -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>
+

sonaranalyzer-dotnet/src/Tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeTests.cs

@@ -254,6 +254,7 @@ public class RuleTypeTests
             ["3902"] = "CODE_SMELL",
             ["3903"] = "BUG",
             ["3904"] = "BUG",
+            ["3909"] = "CODE_SMELL",
             ["3925"] = "BUG",
             ["3926"] = "BUG",
             ["3928"] = "CODE_SMELL",

sonaranalyzer-dotnet/src/Tests/SonarAnalyzer.UnitTest/Rules/CollectionsShouldImplementGenericInterfaceTest.cs

@@ -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());
+        }
+    }
+}

sonaranalyzer-dotnet/src/Tests/SonarAnalyzer.UnitTest/TestCases/CollectionsShouldImplementGenericInterface.cs

@@ -0,0 +1,36 @@
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+
+namespace Tests.Diagnostics
+{
+    class TestClass_01 : CollectionBase { } // Noncompliant {{Refactor this collection to implement 'System.Collections.ObjectModel.Collection<T>'.}}
+//        ^^^^^^^^^^^^
+    class TestClass_02 : IList { } // Noncompliant {{Refactor this collection to implement 'System.Collections.Generic.IList<T>'.}}
+
+    class TestClass_03 : IEnumerable { } // Noncompliant {{Refactor this collection to implement 'System.Collections.Generic.IEnumerable<T>'.}}
+
+    class TestClass_04 : ICollection { } // Noncompliant {{Refactor this collection to implement 'System.Collections.Generic.ICollection<T>'.}}
+
+    class TestClass_05 : IEnumerable, ICollection { } // Noncompliant {{Refactor this collection to implement 'System.Collections.Generic.IEnumerable<T>'.}}
+    // Noncompliant@-1 {{Refactor this collection to implement 'System.Collections.Generic.ICollection<T>'.}}
+
+    class TestClass_06 : IEnumerable, ICollection<string> { }
+    class TestClass_07 : IEnumerable, IList<string> { }
+    class TestClass_08 : IEnumerable, IEnumerable<string> { }
+    class TestClass_09 : IEnumerable, Collection<string> { }
+    class TestClass_10<T> : IEnumerable, IList<T> { }
+
+    class TestClass_11<T> : IEnumerable, ICollection, ICollection<T> { }
+
+    class TestClass_12 : IEnumerable, IList, IEnumerable<string> { }
+
+    class TestClass_13 { }
+
+    class TestClass_14 : Exception { }
+
+    class TestClass_15 : IEqualityComparer { }
+
+    class TestClass_16 : IList, InvalidType { } // Noncompliant
+}