Feature: warn for ctor not injecting logging (#206)

src/Dhgms.GripeWithRoslyn.Analyzer/Analyzers/Logging/ConstructorShouldAcceptLoggingFrameworkArgumentAnalyzer.cs

@@ -0,0 +1,215 @@
+// Copyright (c) 2019 DHGMS Solutions and Contributors. All rights reserved.
+// This file is licensed to you under the MIT license.
+// See the LICENSE file in the project root for full license information.
+
+using System;
+using System.Collections.Immutable;
+using System.Linq;
+using Dhgms.GripeWithRoslyn.Analyzer.Analyzers.Language;
+using Dhgms.GripeWithRoslyn.Analyzer.CodeCracker.Extensions;
+using Dhgms.GripeWithRoslyn.Analyzer.Extensions;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace Dhgms.GripeWithRoslyn.Analyzer.Analyzers.Logging
+{
+    /// <summary>
+    /// Analyzer for checking a constructor has a logging framework instance passed into it.
+    /// </summary>
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public sealed class ConstructorShouldAcceptLoggingFrameworkArgumentAnalyzer : DiagnosticAnalyzer
+    {
+        internal const string Title = "Constructor should have a logging framework instance as the final parameter.";
+
+        private const string MessageFormat = Title;
+
+        private const string Category = SupportedCategories.Design;
+
+        private readonly DiagnosticDescriptor _rule;
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ConstructorShouldAcceptLoggingFrameworkArgumentAnalyzer"/> class.
+        /// </summary>
+        public ConstructorShouldAcceptLoggingFrameworkArgumentAnalyzer()
+        {
+            _rule = new DiagnosticDescriptor(
+                DiagnosticIdsHelper.ConstructorShouldAcceptLoggingFrameworkArgument,
+                Title,
+                MessageFormat,
+                Category,
+                DiagnosticSeverity.Warning,
+                isEnabledByDefault: true,
+                description: DiagnosticResultDescriptionFactory.ConstructorShouldAcceptLoggingFrameworkArgument());
+        }
+
+        /// <inheritdoc />
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(_rule);
+
+        /// <inheritdoc />
+        public override void Initialize(AnalysisContext context)
+        {
+            context.EnableConcurrentExecution();
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
+            context.RegisterSyntaxNodeAction(AnalyzeInvocationExpression, SyntaxKind.ConstructorDeclaration);
+        }
+
+        private static string GetFullName(
+            ConstructorDeclarationSyntax constructorDeclarationSyntax,
+            ClassDeclarationSyntax classDeclarationSyntax)
+        {
+            var namespaceDeclarationSyntax = constructorDeclarationSyntax.GetAncestor<NamespaceDeclarationSyntax>();
+
+            var namespaceName = namespaceDeclarationSyntax.Name.ToString();
+            return $"global::{namespaceName}.{classDeclarationSyntax.Identifier}";
+        }
+
+        private void AnalyzeInvocationExpression(SyntaxNodeAnalysisContext context)
+        {
+            var node = context.Node;
+
+            var constructorDeclarationSyntax = (ConstructorDeclarationSyntax)context.Node;
+            var classDeclarationSyntax = constructorDeclarationSyntax.GetAncestor<ClassDeclarationSyntax>();
+
+            // skip if the class implements whipstaff ILogMessageActions<T> or ILogMessageActionsWrapper<T>
+            // or Splat IEnableLogger<T>
+            var baseClasses = Array.Empty<string>();
+
+            var interfaces = new[]
+            {
+                "global::Whipstaff.Core.Logging.ILogMessageActions",
+                "global::Whipstaff.Core.Logging.ILogMessageActionsWrapper",
+                "global::Splat.IEnableLogger"
+            };
+
+            if (classDeclarationSyntax.HasImplementedAnyOfType(baseClasses, interfaces, context.SemanticModel))
+            {
+                return;
+            }
+
+            // we can also skip out if the class has no methods
+            // as there won't be any logging going on.
+            if (!classDeclarationSyntax.ChildNodes().OfType<MethodDeclarationSyntax>().Any())
+            {
+                return;
+            }
+
+            // check the parameters
+            var parametersList = constructorDeclarationSyntax.ParameterList.Parameters;
+
+            if (parametersList.Count == 0)
+            {
+                // no parameters, so no logging framework
+                LogWarning(context, node);
+                return;
+            }
+
+            var lastParameter = parametersList.Last();
+            var lastParameterType = lastParameter.Type;
+            if (lastParameterType == null)
+            {
+                // this is a problem, as we can't determine the type
+                LogWarning(context, node);
+                return;
+            }
+
+            var typeInfo = ModelExtensions.GetTypeInfo(context.SemanticModel, lastParameterType);
+            var argType = typeInfo.Type;
+
+            if (argType == null)
+            {
+                // this is a problem, as we can't determine the type
+                LogWarning(context, node);
+                return;
+            }
+
+            // TODO: refactor this to take an array of types to check for, with an optional generic types array
+            var myType = GetFullName(constructorDeclarationSyntax, classDeclarationSyntax);
+            var typeFullName = argType.GetFullName();
+            if (typeFullName.Equals(
+                    $"global::XUnit.Abstractions.ITestOutputHelper",
+                    StringComparison.Ordinal))
+            {
+                return;
+            }
+
+            if (typeFullName.Equals(
+                    $"global::Microsoft.Extensions.Logging.ILogger",
+                    StringComparison.Ordinal))
+            {
+                CheckGenericArgument(context, lastParameter, node, myType);
+
+                return;
+            }
+
+            // check if implementing Whipstaff.Core.Logging.ILogMessageActionsWrapper<T>
+            var lastParameterAllInterfaces = argType.AllInterfaces;
+            foreach (var namedTypeSymbol in lastParameterAllInterfaces)
+            {
+                var interfaceName = namedTypeSymbol.GetFullName();
+                if (interfaceName.Equals(
+                        $"global::Whipstaff.Core.Logging.ILogMessageActionsWrapper",
+                        StringComparison.Ordinal)
+                    && namedTypeSymbol.TypeArguments.Any(x => x.GetFullName().Equals(myType, StringComparison.Ordinal)))
+                {
+                    return;
+                }
+            }
+
+            LogWarning(context, node);
+        }
+
+        private void CheckGenericArgument(
+            SyntaxNodeAnalysisContext context,
+            ParameterSyntax lastParameter,
+            SyntaxNode node,
+            string myType)
+        {
+            // var genericArgs = argType.GetGenericArguments();
+            var childNodes = lastParameter.ChildNodes();
+
+            // QualifiedNameSyntax
+            var qualifiedNameSyntax = childNodes.OfType<QualifiedNameSyntax>().FirstOrDefault();
+
+            // GenericNameSyntax
+            var genericNameSyntax = qualifiedNameSyntax.ChildNodes().OfType<GenericNameSyntax>().ToArray();
+
+            // GenericTokenSyntax
+            var genericTokenSyntax = genericNameSyntax[0];
+
+            // type arg list
+            var typeArgumentList = genericTokenSyntax.TypeArgumentList;
+            var typeArgumentListArgs = typeArgumentList.Arguments;
+
+            // we should only have 1 arg for ILogger<T>.
+            if (typeArgumentListArgs.Count != 1)
+            {
+                LogWarning(context, node);
+                return;
+            }
+
+            var genericArgType = ModelExtensions.GetTypeInfo(context.SemanticModel, typeArgumentListArgs[0]);
+            var genericArgTypeType = genericArgType.Type;
+            if (genericArgTypeType == null)
+            {
+                // this is a problem, as we can't determine the type
+                LogWarning(context, node);
+                return;
+            }
+
+            var genericArgTypeTypeFullName = genericArgTypeType.GetFullName();
+            if (!genericArgTypeTypeFullName.Equals(
+                    myType,
+                    StringComparison.Ordinal))
+            {
+                LogWarning(context, node);
+            }
+        }
+
+        private void LogWarning(SyntaxNodeAnalysisContext context, SyntaxNode node)
+        {
+            context.ReportDiagnostic(Diagnostic.Create(_rule, node.GetLocation()));
+        }
+    }
+}

src/Dhgms.GripeWithRoslyn.Analyzer/Analyzers/Logging/MethodShouldInvokeLoggingActionAnalyzer.cs

@@ -0,0 +1,14 @@
+// Copyright (c) 2019 DHGMS Solutions and Contributors. All rights reserved.
+// This file is licensed to you under the MIT license.
+// See the LICENSE file in the project root for full license information.
+
+using System;
+using System.Collections.Generic;
+using System.Text;
+
+namespace Dhgms.GripeWithRoslyn.Analyzer.Analyzers.Logging
+{
+    internal class MethodShouldInvokeLoggingActionAnalyzer
+    {
+    }
+}

src/Dhgms.GripeWithRoslyn.Analyzer/Dhgms.GripeWithRoslyn.Analyzer.csproj

@@ -8,6 +8,7 @@
     <SuppressDependenciesWhenPacking>true</SuppressDependenciesWhenPacking>
     <TargetsForTfmSpecificContentInPackage>$(TargetsForTfmSpecificContentInPackage);_AddAnalyzersToOutput</TargetsForTfmSpecificContentInPackage>
     <IncludeBuildOutput>false</IncludeBuildOutput>
+    <LangVersion>latest</LangVersion>
   </PropertyGroup>
 
   <ItemGroup>

src/Dhgms.GripeWithRoslyn.Analyzer/DiagnosticIdsHelper.cs

@@ -57,5 +57,7 @@ internal static class DiagnosticIdsHelper
         internal static string DoNotUseEnumToString => "GR0025";
 
         internal static string DoNotUseXUnitInlineDataAttribute => "GR0026";
+
+        internal static string ConstructorShouldAcceptLoggingFrameworkArgument => "GR0027";
     }
 }

src/Dhgms.GripeWithRoslyn.Analyzer/DiagnosticResultDescriptionFactory.cs

@@ -0,0 +1,13 @@
+// Copyright (c) 2019 DHGMS Solutions and Contributors. All rights reserved.
+// This file is licensed to you under the MIT license.
+// See the LICENSE file in the project root for full license information.
+
+using System;
+
+namespace Dhgms.GripeWithRoslyn.Analyzer
+{
+    internal static class DiagnosticResultDescriptionFactory
+    {
+        internal static string ConstructorShouldAcceptLoggingFrameworkArgument() => $"Constructors should have a final parameter of \nMicrosoft.Extensions.Logging.ILogging<T> or, \na sublass of Whipstaff.Core.ILogMessageActionsWrapper<T> or,\nXUnit.Abstractions.ITestOutputHelper.\n\nThis is to encourage a design that contains sufficient logging.";
+    }
+}

src/Dhgms.GripeWithRoslyn.Analyzer/DiagnosticResultTitleFactory.cs

@@ -0,0 +1,11 @@
+// Copyright (c) 2019 DHGMS Solutions and Contributors. All rights reserved.
+// This file is licensed to you under the MIT license.
+// See the LICENSE file in the project root for full license information.
+
+namespace Dhgms.GripeWithRoslyn.Analyzer
+{
+    internal static class DiagnosticResultTitleFactory
+    {
+        internal static string ConstructorShouldAcceptLoggingFrameworkArgument() => "Constructor should have a logging framework instance as the final parameter.";
+    }
+}

src/Dhgms.GripeWithRoslyn.Analyzer/Extensions/BaseTypeSyntaxExtensions.cs

@@ -0,0 +1,64 @@
+// Copyright (c) 2019 DHGMS Solutions and Contributors. All rights reserved.
+// This file is licensed to you under the MIT license.
+// See the LICENSE file in the project root for full license information.
+
+using System;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+
+namespace Dhgms.GripeWithRoslyn.Analyzer.Extensions
+{
+    /// <summary>
+    /// Extensions for <see cref="BaseTypeSyntax"/>.
+    /// </summary>
+    public static class BaseTypeSyntaxExtensions
+    {
+        /// <summary>
+        /// Checks if the <see cref="BaseTypeSyntax"/> has implemented any of the specified types.
+        /// </summary>
+        /// <param name="baseTypeSyntax">The base type syntax node to check.</param>
+        /// <param name="baseClasses">The base classes to check for.</param>
+        /// <param name="interfaces">The interfaces to check for.</param>
+        /// <param name="semanticModel">The semantic model for the code being checked.</param>
+        /// <returns>Whether the <see cref="BaseTypeSyntax"/> has implemented any of the specified types.</returns>
+        public static bool HasImplementedAnyOfType(
+            this BaseTypeSyntax baseTypeSyntax,
+            string[] baseClasses,
+            string[] interfaces,
+            SemanticModel semanticModel)
+        {
+            var typeSyntax = baseTypeSyntax.Type;
+            var baseTypeInfo = semanticModel.GetTypeInfo(typeSyntax);
+            var baseTypeSymbol = baseTypeInfo.Type;
+
+            if (baseTypeSymbol == null)
+            {
+                return false;
+            }
+
+            var baseTypeFullName = baseTypeSymbol.GetFullName();
+
+            if (baseClasses.Any(bc => bc.Equals(baseTypeFullName, StringComparison.Ordinal)))
+            {
+                return true;
+            }
+
+            if (interfaces.Any(i => i.Equals(baseTypeFullName, StringComparison.Ordinal)))
+            {
+                return true;
+            }
+
+            if (baseTypeSymbol.AllInterfaces.Any(symbol =>
+                {
+                    var fn = symbol.GetFullName();
+                    return interfaces.Any(i => i.Equals(fn, StringComparison.Ordinal));
+                }))
+            {
+                return true;
+            }
+
+            return false;
+        }
+    }
+}

src/Dhgms.GripeWithRoslyn.Analyzer/Extensions/ClassDeclarationSyntaxExtensions.cs

@@ -0,0 +1,55 @@
+// Copyright (c) 2019 DHGMS Solutions and Contributors. All rights reserved.
+// This file is licensed to you under the MIT license.
+// See the LICENSE file in the project root for full license information.
+
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+
+namespace Dhgms.GripeWithRoslyn.Analyzer.Extensions
+{
+    /// <summary>
+    /// Extensions for <see cref="ClassDeclarationSyntax"/>.
+    /// </summary>
+    public static class ClassDeclarationSyntaxExtensions
+    {
+        /// <summary>
+        /// Checks if the <see cref="ClassDeclarationSyntax"/> has implemented any of the specified types.
+        /// </summary>
+        /// <param name="classDeclarationSyntax">The class declaration node to check.</param>
+        /// <param name="baseClasses">The base classes to check for.</param>
+        /// <param name="interfaces">The interfaces to check for.</param>
+        /// <param name="semanticModel">The semantic model for the code being checked.</param>
+        /// <returns>Whether the <see cref="BaseTypeSyntax"/> has implemented any of the specified types.</returns>
+        public static bool HasImplementedAnyOfType(
+            this ClassDeclarationSyntax classDeclarationSyntax,
+            string[] baseClasses,
+            string[] interfaces,
+            SemanticModel semanticModel)
+        {
+            var baseList = classDeclarationSyntax.BaseList;
+            if (baseList == null)
+            {
+                return false;
+            }
+
+            var baseTypes = baseList.Types;
+            if (baseTypes.Count < 1)
+            {
+                return false;
+            }
+
+            foreach (var baseTypeSyntax in baseTypes)
+            {
+                if (baseTypeSyntax.HasImplementedAnyOfType(
+                        baseClasses,
+                        interfaces,
+                        semanticModel))
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+    }
+}