3

I have a logger that records the method name (which I get through reflection) and parameters (which are manually passed to the logger). Here's an example of the proper way to do the logging:

public void Foo() {
   // This is correct - the method has no parameters, neither does the logging
   Logger.Log();
   // Method
}

public void Foo1(int a, int b) {
    // Log both of the parameters correctly
    Logger.Log(a, b);
    // Rest of method
}

However, people will periodically call this incorrectly. For example:

public void Foo1(int a, int b) {
   // Didn't record any of the parameters
   Logger.Log();
   // Rest of method
}

or

public void Foo1(int a, int b, int c) {
    // Someone changed the number of parameters but didn't update the logging call
    Logger.Log(a, b);
}

The signature of the Log method is:

public void Log(params object[] parameters)

I'd like to have some way of requiring that Logger.Log have the same number of parameters as the method calling it does.

I know how to do this at runtime (just use reflection to get the parameter list for the caller and compare it to what parameters I actually received), but that would be a really bad solution to this I think since the vast majority of the checks would be unnecessary. (It would also mean that you wouldn't know until runtime that you had written it incorrectly, and then only if you happened to execute that particular method).

Right now we're not using FxCop unfortunately (or I'd just write some kind of rule) and I suspect that I wouldn't succeed in changing that fact. Short of writing a compiler plugin of some kind, is there a way to force people to use this method correctly?

  • 1
    Long shot, and I haven't used it myself, but does this help https://blogs.msdn.microsoft.com/hkamel/2013/10/24/visual-studio-2013-static-code-analysis-in-depth-what-when-and-how/? You can create custom rules for the built-in static analysis tool. – itsme86 Dec 21 '16 at 20:39
  • 1
    Putting the logging code into each method doesn't seem to be good scalable & maintainable design. – Abhinav Galodha Dec 21 '16 at 20:39
  • 2
    I think you should be looking for some AOP solution for logging where you don't have to remember to add logging to every method or remember to update the parameter list when and if they change. – rory.ap Dec 21 '16 at 20:40

1 Answers1

3

You should be able to accomplish this using the new Roslyn API's. You'll want to install the SDK here:

https://marketplace.visualstudio.com/items?itemName=VisualStudioProductTeam.NETCompilerPlatformSDK

Once installed you should go to new project and navigate to Extensibility and you'll see the project type Analyzer with Code Fix (NuGet + VSIX) template. I created a sample project that I used to show the compiler errors:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace AnalyzerTest
{
    public static class Logger
    {
        public static void Log(params object[] parameters)
        {

        }
    }
}

namespace AnalyzerTest
{
    public class Foo
    {
        public void Foo1(int a, int b)
        {
            // Didn't record any of the parameters
            Logger.Log();
            // Rest of method
        }
    }
}

I created a separate project for the analyzer and here is the code for the analyzer class:

using System;
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Semantics;

namespace Analyzer1
{
    [DiagnosticAnalyzer(LanguageNames.CSharp)]
    public class LoggerAnalyzer : DiagnosticAnalyzer
    {
        public const string DiagnosticId = "Logging";
        internal const string Title = "Logging error";
        internal const string MessageFormat = "Logging error {0}";
        internal const string Description = "You should have the same amount of arguments in the logger as you do in the method.";
        internal const string Category = "Syntax";

        internal static DiagnosticDescriptor Rule =
          new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat,
          Category, DiagnosticSeverity.Error, isEnabledByDefault: true, description: Description);

        public override ImmutableArray<DiagnosticDescriptor>
          SupportedDiagnostics
        { get { return ImmutableArray.Create(Rule); } }

        public override void Initialize(AnalysisContext context)
        {
            context.RegisterSyntaxNodeAction(
              AnalyzeNode, SyntaxKind.InvocationExpression);
        }

        private void AnalyzeNode(SyntaxNodeAnalysisContext context)
        {
            var invocationExpr = (InvocationExpressionSyntax)context.Node;
            var memberAccessExpr = invocationExpr.Expression as MemberAccessExpressionSyntax;

            if (memberAccessExpr != null && memberAccessExpr.Name.ToString() != "Log")
            {
                return;
            }

            var memberSymbol =
                context.SemanticModel.GetSymbolInfo(memberAccessExpr).Symbol as IMethodSymbol;

            if (memberSymbol == null || !memberSymbol.ToString().StartsWith("AnalyzerTest.Logger.Log"))
            {
                return;
            }

            MethodDeclarationSyntax parent = GetParentMethod(context.Node);
            if(parent == null)
            {
                return;
            }

            var argumentList = invocationExpr.ArgumentList;

            Int32 parentArgCount = parent.ParameterList.Parameters.Count;
            Int32 argCount = argumentList != null ? argumentList.Arguments.Count : 0;

            if (parentArgCount != argCount)
            {
                var diagnostic = Diagnostic.Create(Rule, invocationExpr.GetLocation(), Description);
                context.ReportDiagnostic(diagnostic);
            }
        }

        private MethodDeclarationSyntax GetParentMethod(SyntaxNode node)
        {
            var parent = node.Parent as MethodDeclarationSyntax;
            if(parent == null)
            {
                return GetParentMethod(node.Parent);
            }

            return parent;
        }
    }
}

While in the Analyzer with Code Fix project you can hit F5 (as long as your .Vsix project is the startup project) and it will open up another VS instance and you can choose the project you would like to test the analyzer on.

Here is the result:

enter image description here

It also looks like you will have to install this as a NuGet package instead of a VS Extension, for whatever reason VS Extensions don't affect the build and you will only get the warning:

https://stackoverflow.com/a/39657967/1721372

For a more complete example see here:

https://msdn.microsoft.com/en-us/magazine/dn879356.aspx

Community
  • 1
  • 1
The Pax Bisonica
  • 2,154
  • 2
  • 26
  • 45