The generic type arg is always Object because the signature of the factory is Func<IServiceProvider,object>.

public interface IFoo { }
public class Foo : IFoo { }

var descriptor = ServiceDescriptor.Transient(typeof(IFoo), prov => new Foo());
var type = descriptor.GetImplementationType(); // return Object

Having a factory we cannot determine the implementation type because there can be multiple and some of them might be disposable and some are not:

public class Foo2 : IFoo, IDisposable
{
	public void Dispose()	{ 	}
}

var descriptor1 = ServiceDescriptor.Transient(typeof(IFoo), prov =>
{
	var foo = new Random().Next() == 42
				       ? (IFoo) new Foo()
  				       : new Foo2();
		
	return foo;
});

The generic type arg is always Object because the signature of the factory is Func<IServiceProvider,object>.

Actually, that;s not the case, as GetType() gets the runtime type. However it still doesn't work for other reasons 🙂 In the example you've provided, the factory is of type Func<IServiceProvider, IFoo>, so descriptor.GetImplementationType() returns IFoo - not object as you suggest, but also not Foo as intended!

Having a factory we cannot determine the implementation type because there can be multiple and some of them might be disposable and some are not.

Hmmm, yeah that's a very good point, I think the specific approach in this PR falls down fundamentally because of that.

Replacing a descriptor with the implementation type may break existing registrations

// Before registration of the decorator
ServiceCollection services = ...;

services.AddTransient<Foo, Foo2>();
services.AddTransient<IFoo, Foo>();
	
Resolve<IFoo>  -> Foo
Resolve<Foo>  -> Foo2

Now we will decorate IFoo with FooDecorator

// the decoration will replace
services.AddTransient<IFoo, Foo>();
// with
services.AddTransient<Foo, Foo>();
// this breaks the following registration
services.AddTransient<Foo, Foo2>();

Resolve<Foo>  => Foo instead of Foo2

Yeah, I was aware that's a potential breaking change, though I'd argue two things:

1. The example that you describe seems like it would be an edge case, that's some strange registrations going on there right?
2. It wouldn't actually break. Last registration wins, so Resolve<Foo> => Foo2 still. The difference is Resolve<IEnumerable<Foo>> returns both Foo and Foo2, instead of just Foo I think.

For me, I think that change is worth it. #91 is preventing me use the decoration entirely, the registration caveats are a necessary evil I think. I take it you don't agree? 🙂

1. DI lib must be reliable. There will be different kinds of registrations in the wild, especially if we trying to decorate a complex 3rd party lib (e.g. EF Core / ASP.NET Core).
2. It does :P
   try the following

var services = new ServiceCollection();
	
services.AddTransient<Foo, Foo2>();
services.AddTransient<IFoo, Foo>();
	
//services.Decorate<IFoo,FooDecorator>();
	
var provider = services.BuildServiceProvider();

without Decorate

provider.GetRequiredService<IFoo>().Dump();  // Foo
provider.GetRequiredService<Foo>().Dump();   // Foo2

with Decorate

provider.GetRequiredService<IFoo>().Dump();  // FooDecorator+Foo
provider.GetRequiredService<Foo>().Dump();   // Foo instead of Foo2

1. Absolutely agreed, my argument is that the current behaviour isn't "reliable" - disposable services aren't disposed by the container when you use the current implementation.
2. True, but you cheated in that example and moved the Foo2 registration up before the decorate call, it was after it previously 😛

I still agree that there's too many sharp edges for this to be acceptable. I think it's worth pursuing though. 🙂