AG0009 (#120)

* Implement AG0009

* Refactoring in code review

src/Agoda.Analyzers.Test/Agoda.Analyzers.Test.csproj

@@ -34,6 +34,7 @@
     <PackageReference Include="Microsoft.AspNet.Mvc" Version="5.2.3" />
     <PackageReference Include="Microsoft.AspNet.Razor" Version="3.2.3" />
     <PackageReference Include="Microsoft.AspNet.WebPages" Version="3.2.3" />
+    <PackageReference Include="Microsoft.AspNetCore.Http" Version="2.1.1" />
     <PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="1.1.0" />
     <PackageReference Include="Microsoft.CodeAnalysis.Common" Version="1.3.2" />
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="1.3.2" />

src/Agoda.Analyzers.Test/AgodaCustom/AG0009UnitTests.cs

@@ -0,0 +1,68 @@
+using System.Threading.Tasks;
+using Agoda.Analyzers.AgodaCustom;
+using Agoda.Analyzers.Test.Helpers;
+using Microsoft.AspNetCore.Http;
+using Microsoft.CodeAnalysis.Diagnostics;
+using NUnit.Framework;
+
+namespace Agoda.Analyzers.Test.AgodaCustom
+{
+    class AG0009UnitTests : DiagnosticVerifier
+    {
+        protected override DiagnosticAnalyzer DiagnosticAnalyzer => new AG0009IHttpContextAccessorCannotBePassedAsMethodArgument();
+
+        protected override string DiagnosticId => AG0009IHttpContextAccessorCannotBePassedAsMethodArgument.DIAGNOSTIC_ID;
+
+        [Test]
+        public async Task TestIHttpContextAccessorAsArgument()
+        {
+            var code = new CodeDescriptor
+            {
+                References = new[] {typeof(IHttpContextAccessor).Assembly, typeof(HttpContextAccessor).Assembly},
+                Code = @"
+using Microsoft.AspNetCore.Http;
+
+interface ISomething
+{
+    void SomeMethod(IHttpContextAccessor c, string sampleString); // ugly interface method
+    void SomeMethod(HttpContextAccessor c, string sampleString); // ugly interface method
+}
+
+class TestClass : ISomething
+{
+    public void SomeMethod(IHttpContextAccessor context, string sampleString)
+    {
+        // this method is ugly
+    }
+
+    public void SomeMethod(HttpContextAccessor context, string sampleString)
+    {
+        //this method is ugly
+    }
+
+     public TestClass(Microsoft.AspNetCore.Http.IHttpContextAccessor context)
+    {
+        // this constructor is uglier
+    }
+
+    public TestClass(Microsoft.AspNetCore.Http.HttpContextAccessor context)
+    {
+        // this constructor is uglier
+    }
+}"
+            };
+
+            var expected = new[]
+            {
+                new DiagnosticLocation(6, 21),
+                new DiagnosticLocation(7, 21),
+                new DiagnosticLocation(12, 28),
+                new DiagnosticLocation(17, 28),
+                new DiagnosticLocation(22, 23),
+                new DiagnosticLocation(27, 22)
+            };
+            HttpContextAccessor a;
+            await VerifyDiagnosticsAsync(code, expected);
+        }
+    }
+}

src/Agoda.Analyzers/Agoda.Analyzers.csproj

@@ -27,6 +27,7 @@
     <WarningLevel>4</WarningLevel>
   </PropertyGroup>
   <ItemGroup>
+    <PackageReference Include="Microsoft.AspNetCore.Http" Version="2.1.1" />
     <PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="1.1.0" />
     <PackageReference Include="Microsoft.CodeAnalysis.Common" Version="1.3.2" />
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="1.3.2" />
@@ -87,10 +88,10 @@
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>
     <None Update="RuleContent\AG0018PermitOnlyCertainPubliclyExposedEnumerables.html">
-        <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>
     <None Update="RuleContent\AG0019PreventUseOfInternalsVisibleToAttribute.html">
-          <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>
     <None Update="RuleContent\AG0024PreventUseOfTaskFactoryStartNew.html">
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
@@ -104,14 +105,17 @@
     <None Update="RuleContent\AG0020AvoidReturningNullEnumerables.html">
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>
+    <None Update="RuleContent\AG0009IHttpContextAccessorCannotBePassedAsMethodArgument.html">
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+    </None>
     <None Update="RuleContent\AG0012TestMethodMustContainAtLeastOneAssertion.html">
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>
     <None Update="RuleContent\AG0013LimitNumberOfTestMethodParametersTo5.html">
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>
     <None Update="RuleContent\AG0022DoNotExposeBothSyncAndAsyncVersionsOfMethods.html">
-          <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>
     <None Update="RuleContent\AG0021PreferAsyncMethods.html">
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
@@ -132,4 +136,4 @@
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </None>
   </ItemGroup>
-</Project>
+</Project>

src/Agoda.Analyzers/AgodaCustom/AG0009IHttpContextAccessorCannotBePassedAsMethodArgument.cs

@@ -0,0 +1,60 @@
+using System;
+using System.Collections.Immutable;
+using Agoda.Analyzers.Helpers;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace Agoda.Analyzers.AgodaCustom
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public class AG0009IHttpContextAccessorCannotBePassedAsMethodArgument : DiagnosticAnalyzer
+    {
+        public const string DIAGNOSTIC_ID = "AG0009";
+
+        private static readonly LocalizableString Title = new LocalizableResourceString(
+            nameof(CustomRulesResources.AG0009Title),
+            CustomRulesResources.ResourceManager,
+            typeof(CustomRulesResources));
+
+        private static readonly LocalizableString MessageFormat = new LocalizableResourceString(
+            nameof(CustomRulesResources.AG0009MessageFormat),
+            CustomRulesResources.ResourceManager,
+            typeof(CustomRulesResources));
+
+        private static readonly LocalizableString Description =
+            DescriptionContentLoader.GetAnalyzerDescription(
+                nameof(AG0009IHttpContextAccessorCannotBePassedAsMethodArgument));
+
+        private static readonly DiagnosticDescriptor Descriptor =
+            new DiagnosticDescriptor(DIAGNOSTIC_ID, Title, MessageFormat, AnalyzerCategory.CustomQualityRules,
+                DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, null,
+                WellKnownDiagnosticTags.EditAndContinue);
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Descriptor);
+
+        private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
+        {
+            var methodParam = context.Node as ParameterSyntax;
+            var paramType = (methodParam.Type as QualifiedNameSyntax)?.Right ?? methodParam.Type as SimpleNameSyntax;
+            if (paramType == null)
+                return;
+
+            var paramTypeName = context.SemanticModel.GetTypeInfo(paramType).Type.ToDisplayString();
+            if ("Microsoft.AspNetCore.Http.IHttpContextAccessor" == paramTypeName
+                || "Microsoft.AspNetCore.Http.HttpContextAccessor" == paramTypeName)
+            {
+                context.ReportDiagnostic(Diagnostic.Create(Descriptor, context.Node.GetLocation()));
+            }
+        }
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+            context.EnableConcurrentExecution();
+
+            context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.Parameter);
+        }
+    }
+}

src/Agoda.Analyzers/AgodaCustom/CustomRulesResources.resx

@@ -162,6 +162,17 @@
   <data name="AG0011Title" xml:space="preserve">
     <value>Controllers should not access HttpRequest.QueryString directly but use ASP.NET model binding instead</value>
   </data>
+  <data name="AG0009Title" xml:space="preserve">
+    <value>Do not pass IHttpContextAccessor as method argument</value>
+    <comment>AG0009MessageFormat</comment>
+  </data>
+  <data name="AG0009Description" xml:space="preserve">
+    <value>Pass only the fields that are actually needed, not the entire IHttpContextAccessor instance</value>
+  </data>
+  <data name="AG0009MessageFormat" xml:space="preserve">
+    <value>Pass only the fields that are actually needed, not the entire IHttpContextAccessor instance</value>
+    <comment>AG0009Description</comment>
+  </data>
   <data name="AG0012Title" xml:space="preserve">
     <value>Test method should contain at least one assertion</value>
   </data>

src/Agoda.Analyzers/RuleContent/AG0009IHttpContextAccessorCannotBePassedAsMethodArgument.html

@@ -0,0 +1,57 @@
+<p>
+    Passing the whole <code>Microsoft.AspNetCore.Http.IHttpContextAccessor</code> to your method as a parameter create hard dependency on the IHttpContextAccessor and makes testing really hard ( mocking IHttpContextAccessor itself). 
+    You should only pass parts of the <code>Microsoft.AspNetCore.Http.IHttpContextAccessor</code> that you actually using.
+</p>
+
+<h2>Noncompliant Code Example</h2>
+<pre>
+using Microsoft.AspNetCore.Http;
+
+interface ISomething
+{
+    void SomeMethod(IHttpContextAccessor c, string sampleString); // ugly interface method
+    void SomeMethod(HttpContextAccessor c, string sampleString); // ugly interface method
+}
+
+class TestClass : ISomething
+{
+    public void SomeMethod(IHttpContextAccessor context, string sampleString)
+    {
+        // this method is ugly
+    }
+
+    public void SomeMethod(HttpContextAccessor context, string sampleString)
+    {
+        // this method is ugly
+    }
+
+     public TestClass(Microsoft.AspNetCore.Http.IHttpContextAccessor context)
+    {
+        // this constructor is uglier
+    }
+
+    public TestClass(Microsoft.AspNetCore.Http.HttpContextAccessor context)
+    {
+        // this constructor is uglier
+    }
+}
+</pre>
+
+<h2>Compliant Code Example</h2>
+<pre>
+interface ISomething
+{
+    void SomeMethod(string userAgent, string sampleString);
+}
+
+class TestClass : ISomething
+{
+    public void SomeMethod(string userAgent, string sampleString)
+    {
+    }
+
+    public TestClass(string userAgent)
+    {
+    }
+}
+</pre>