autofac / Autofac.Analyzers

Roslyn code analyzers to help with Autofac usage.
MIT License
1 stars 0 forks source link

Analyzer Proposal - Delegate Registration should have an As<TService> #3

Open alistairjevans opened 4 years ago

alistairjevans commented 4 years ago

Based on the current Autofac guidance, here, delegate registrations should have an As<TService> call to inform the limit type.

Proposed Behaviour

Where a registration is declared from a delegate, using one of the two Register extension methods, if that same registration is determined to not have an As call, a warning will be raised.

For example:

class MyClass : IMyService { }

interface IMyService { }

static void Main(string[] args)
{
    var builder = new ContainerBuilder();

    // Warning Generated on this registration call.
    builder.Register(c => new MyClass());

    // No Warning Generated on this registration call.
    builder.Register(c => new MyClass()).As<IMyService>();
}

Possible Intelligent Detection

@nblumhardt, @tillig for comments.

tillig commented 4 years ago

OK, re-reading the documentation with an adequate increase in attention, I see that the intent is more about how we can't figure out if it's IComponent or Component that you're registering with a lambda like

// Component implements IComponent
builder.Register(c => new Component);

that is, it's implicitly AsSelf() and folks may not realize that. I'm guessing we probably had some frequent questions or confusion about that at some point, though it's been a while so I don't remember.

I think the idea about being more intelligent is reasonable - if it derives from an abstract class or interface, being explicit is reasonable (either include As<T>() or AsSelf()) where if it doesn't, the implicit AsSelf() stands.

alistairjevans commented 4 years ago

Ok, if it derives from an abstract class or an interface, we'll encourage people to use As<T> or As<TSelf>. That should be pretty doable.

nblumhardt commented 4 years ago

Sounds great 👍

nblumhardt commented 4 years ago

(Probably need to whitelist IDisposable)

alistairjevans commented 4 years ago

(Probably need to whitelist IDisposable)

Can you elaborate slightly? You mean that returning an IDisposable from the lambda should not require AsSelf/As? Since IDisposable doesn't derive from any interfaces, the standard rule should cover it?

nblumhardt commented 4 years ago

I think the intention of applying the check for components that derive from an ABC or interface is that the ABC or interface might represent a service; since IDisposable isn't used as a service, the As call shouldn't be needed if it's the only interface a component supports. HTH!

alistairjevans commented 4 years ago

Oh, yes, I understand now. Better add IAsyncDisposable to that as well.