Analyzers for Framework

[JasonBock]

I wanted to propose the idea of adding analyzers for Benchmark.NET. There's a couple of rules I was thinking of for this library:

• Make sure any classes that contain [Benchmark] methods are not sealed
• Make sure that any [Benchmark] methods return a value (see the "Avoid Dead Code Elimination" section here (http://benchmarkdotnet.org/RulesOfBenchmarking.htm) for the reason why you want to do this)
• `[Benchmark methods should not have any parameters

There may be others. My point is, I'd like to see these analyzers for this framework. I'd also be interested in writing them as well.

[AndreyAkinshin]

Hey @JasonBock, I like the idea in general, but I have some concerns about the suggested analyzers:

Make sure any classes that contain [Benchmark] methods are not sealed

It's true for the default toolchain, but now we also have InProcessToolchain which supports not-sealed classes. And it's impossible to say which toolchain will be used in advance.

Make sure that any [Benchmark] methods return a value

BenchmarkDotNet supports void methods, and it's not a mistake. Dead code elimination could be a problem only in some cases and I'm not sure that it's possible to write an analyzer which detects all such cases correctly (without huge amount of false-positive results).

[Benchmark] methods should not have any parameters

Yeah, we don't support such methods for now, but I'm planning to support them in 1.0 (see also https://github.com/dotnet/BenchmarkDotNet/issues/60).

[adamsitnik]

Hi @JasonBock

We have something that you could extend to get what you want: it's called ExecutionValidator. Validator validates the input (config & types with benchmarks) before running the benchmarks. It's not enabled by default, you have to configure it on your own. More info can be found here

Make sure any classes that contain [Benchmark] methods are not sealed

It would be nice to have it

`[Benchmark methods should not have any parameters

@AndreyAkinshin I believe that Jason could implement it now and once we support methods with parameters we can delete this rule.

[ig-sinicyn]

@AndreyAkinshin

Make sure any classes that contain [Benchmark] methods are not sealed

It's true for the default toolchain, but now we also have InProcessToolchain which supports not-sealed classes. And it's impossible to say which toolchain will be used in advance.

As for me, the non-sealed restriction should be kept for in-process toolchain too. This way it will be always possible to switch from one toolchain to another.

@adamsitnik

We have something that you could extend

Well, I'm almost sure @JasonBock is talking about roslyn analysers, not BDN ones. He wrote in gitter:

Jason Bock I wanted to propose the idea of adding analyzers for Benchmark.NET. I've written analyzers for CSLA (https://github.com/MarimerLLC/csla) and I'm starting that for NUnit as well (https://github.com/nunit/nunit.analyzers)

[JasonBock]

This is old, but....yes, I was referring to Roslyn analyzers, not the validators in Benchmark.NET. Here's another example:

[Params("a", false)]
public bool HasParams { get; set; }

This is a silly mistake, but this will compile, and it won't give an error until you try to run the tests. It would be nice if an analyzer would catch this and ensure the values match the property type. I'm guessing other parameterization scenarios could benefit from analyzer checks.

Another example is having multiple test methods set as the baseline - [Benchmark(Baseline = true)]. Knowing that I have this condition as soon as I type this would be a nice thing :).

[JasonBock]

Another one I found is with [ParamsAllValues]. If it's used with a type that isn't in the supported list, it won't work. So if I did this:

[ParamsAllValues]
public Guid Id { get; set; }

An analyzer would flag it.

[JasonBock]

OK, one more :) -

BenchmarkRunner.Run<AClassWithNoBenchmarkTests>();

This doesn't fail if you run it, but....I'm guessing a class provided in the generic type parameter should be one that has benchmark tests.

[timcassell]

@JasonBock If you're still interested, we'd love to have some Roslyn Analyzers for BDN.