Discussion: Should Autofac assume nullable service types imply they are optional?

[tillig]

Some discussion has started in the pull request for #1355 about whether a nullable service type would imply that the service is optional.

The context is that, with required properties, the compiler technically allows you to explicitly initialize them to null assuming the property is an explicitly allowed nullable reference type. The questions arising, then, are:

• Should Autofac allow you to explicitly initialize a required property to null if the type is nullable?
• Should Autofac constructor handling be changed to allow injection of null for nullable services?

First, let me outline what the compiler allows, what Autofac currently allows, and what Microsoft dependency injection currently allows. Being able to cross reference the three things will help determine the next course of action.

Test Classes

Here are the test classes I'll use in illustrating what things are allowed.

public class TestConsumer
{
    public TestConsumer(Component1 c1, Component2? c2, Component3? c3 = null)
    {
        this.CtorNotNullableProp = c1;
        this.CtorNullableProp = c2;
        this.CtorOptionalProp = c3;
    }

    public Component1 CtorNotNullableProp { get; private set; }

    public Component2? CtorNullableProp { get; private set; }

    public Component3? CtorOptionalProp { get; private set; }

    public required Component4? RequiredProp { get; set; }

    public Component5? PublicSettableProp { get; set; }
}

public class Component1
{
}

public class Component2
{
}

public class Component3
{
}

public class Component4
{
}

public class Component5
{
}

What the Compiler Allows

The compiler allows a lot of stuff, not all of which makes sense in a DI situation:

// Minimum initialization.
new TestConsumer(new Component1(), null) { RequiredProp = new Component4() };

// Full initialization.
new TestConsumer(new Component1(), new Component2(), new Component3()) { RequiredProp = new Component4() };

// Force null into a non-NRT situation in the constructor.
new TestConsumer(null!, null) { RequiredProp = new Component4() };

// Explicitly initialize the required property to null.
new TestConsumer(new Component1(), null) { RequiredProp = null };

Something interesting to note here is that there isn't anything actually stopping you from having the constructor check both constructor parameters for null and throwing if they are. The nullable reference type annotations are more for analysis than for enforcement.

public class TestConsumer
{
    public TestConsumer(Component1 c1, Component2? c2, Component3? c3 = null)
    {
        this.CtorNotNullableProp ?? throw new ArgumentNullException(nameof(c1));
        this.CtorNullableProp ?? throw new ArgumentNullException(nameof(c2));
        this.CtorOptionalProp = c3;
    }
}

What Autofac Allows

This is more about what Autofac currently allows than what it could or should allow.

Given a basic container resolution test, like this:

var builder = new ContainerBuilder();
// Register the consumer and components - this is the part that changes!
var container = builder.Build();
container.Resolve<TestConsumer>();

...Autofac will currently respond like this:

// Simple/complete reflection initialization - RequiredProp and PublicSettableProp will be null.
builder.RegisterType<TestConsumer>();
builder.RegisterType<Component1>();
builder.RegisterType<Component2>();
builder.RegisterType<Component3>();
builder.RegisterType<Component4>();
builder.RegisterType<Component5>();

// Simple/complete reflection initialization with props autowired -
// every property will be set to something not null.
builder.RegisterType<TestConsumer>().PropertiesAutowired();
builder.RegisterType<Component1>();
builder.RegisterType<Component2>();
builder.RegisterType<Component3>();
builder.RegisterType<Component4>();
builder.RegisterType<Component5>();

// Reflection initialization with props autowired - optional ctor parameter is seen as
// optional. Required property isn't actually required, will be null.
builder.RegisterType<TestConsumer>().PropertiesAutowired();
builder.RegisterType<Component1>();
builder.RegisterType<Component2>();
builder.RegisterType<Component5>();

// Absolute minimum reflection initialization. Anything less will result in DRE.
// Only the two required props in the constructor will be set. Note that the nullable
// annotation on the constructor does NOT currently imply this is optional - if `Component2`
// isn't registered, resolution fails.
builder.RegisterType<TestConsumer>();
builder.RegisterType<Component1>();
builder.RegisterType<Component2>();

// Try to force Component2 to be null using RegisterInstance - ArgumentNullException on
// RegisterInstance; we don't allow instances to be null.
builder.RegisterType<TestConsumer>();
builder.RegisterType<Component1>();
builder.RegisterInstance<Component2>(null!);

// Try to force Component2 to be null using delegate registration - DRE because
// delegates aren't allowed to return null.
builder.RegisterType<TestConsumer>();
builder.RegisterType<Component1>();
builder.Register<Component2>(ctx => null!);

// Force the Component1 constructor parameter to be null using parameters. This works.
builder.RegisterType<TestConsumer>().WithParameter("c1", null!);
builder.RegisterType<Component2>();

// Force the public settable property to be null with a property parameter. This works.
builder.RegisterType<TestConsumer>().WithProperty("PublicSettableProp", null!);
builder.RegisterType<Component1>();
builder.RegisterType<Component2>();

So, to sum up:

• You aren't currently allowed to register an instance or delegate that returns null. This is something that would potentially have to change if we allowed explicit initialization to null based on NRT markup.
• You are currently allowed to provide explicit null values in a Parameter (which is used both for constructor or property parameters).
• By default, using reflection, Autofac does not consider a nullable reference type annotation to imply a constructor parameter is optional or that it may be initialized explicitly to null. It only considers actual optional properties as optional.

What Microsoft DI Allows

While Autofac doesn't map 1:1 with MS DI features, keeping relatively compatible makes our lives easier with respect to implementation of the conforming container IServiceProvider. Given that, it's interesting to note what's supported (or not) there to make compatible/informed decisions.

Important differences in features to note:

• MS DI does not support property injection. There is also no current discussion underway to add support for properties/required properties.
• MS DI does not have parameters on registrations. You can use a delegate, but there's no equivalent for WithParameter or WithProperty.

Given a basic container resolution test, like this:

var services = new ServiceCollection();
// Register the consumer and components - this is the part that changes!
var provider = services.BuildServiceProvider();
provider.GetService<TestConsumer>();

...MS DI will currently respond like this:

// Simple/complete reflection initialization - RequiredProp and PublicSettableProp will be null.
services.AddTransient<TestConsumer>();
services.AddTransient<Component1>();
services.AddTransient<Component2>();
services.AddTransient<Component3>();
services.AddTransient<Component4>();
services.AddTransient<Component5>();

// Complete initialization with properties must be done with a delegate.
// Every property will be set to something not null.
services.AddTransient<TestConsumer>(sp => new TestConsumer(sp.GetRequiredService<Component1>(), sp.GetService<Component2>(), sp.GetService<Component3>())
{
    RequiredProp = sp.GetService<Component4>(),
    PublicSettableProp = sp.GetService<Component5>(),
});
services.AddTransient<Component1>();
services.AddTransient<Component2>();
services.AddTransient<Component3>();
services.AddTransient<Component4>();
services.AddTransient<Component5>();

// Initialization with props included. GetService will return null if the service isn't
// registered. Required property isn't actually required, will be null.
services.AddTransient<TestConsumer>(sp => new TestConsumer(sp.GetRequiredService<Component1>(), sp.GetService<Component2>(), sp.GetService<Component3>())
{
    RequiredProp = sp.GetService<Component4>(),
    PublicSettableProp = sp.GetService<Component5>(),
});
services.AddTransient<Component1>();

// Absolute minimum reflection initialization. Anything less will result in InvalidOperationException.
// Only the two required props in the constructor will be set. Note that the nullable
// annotation on the constructor does NOT currently imply this is optional - if `Component2`
// isn't registered, resolution fails.
services.AddTransient<TestConsumer>();
services.AddTransient<Component1>();
services.AddTransient<Component2>();

// Try to force Component2 to be null using AddSingleton - ArgumentNullException on
// AddSingleton; they don't allow instances to be null.
services.AddTransient<TestConsumer>();
services.AddTransient<Component1>();
services.AddSingleton<Component2>((Component2)null!);

// Interestingly, if you try to resolve Component2 using GetRequiredService, you'll get
// an exception saying no service _is registered_, not that the registered service
// returned null;
provider.GetRequiredService<Component2>();

// Try to force Component2 to be null using delegate registration - This works.
// Delegates are allowed to return null.
services.AddTransient<TestConsumer>();
services.AddTransient<Component1>();
services.AddTransient<Component2>(sp => null!);

So, to sum up:

• You aren't currently allowed to register an instance that returns null.
• You are currently allowed to have a delegate that returns null.
• MS DI GetService<T> can return null; GetRequiredService<T> can't.

So What Should We Do?

It seems like we have a couple of options:

1. Required properties with nullable annotations may not be null. This would be consistent with the current handling of nullable reference type markup on constructor parameters, where we don't consider NRT annotations to indicate optional parameters. If a property isn't actually required to be set to something, don't mark it required.
2. Required properties with nullable annotations may be explicitly set to null. This is sort of like ResolveOptional for nullable required properties. This is inconsistent with constructor parameter handling. If we want to be consistent across required properties and constructors, it would be a breaking behavioral change.

My Thoughts

I think that required properties with nullable annotations may not be null when resolved purely by reflection. If you want a property that is required to be allowed to be null, either remove the required markup and use PropertiesAutowired; or use delegates/property parameters to override the default behavior. Why?

Autofac is not the compiler. It doesn't have to support every possible combination of NRT markup, required/optional parameters, and There's an expected, somewhat opinionated, behavior around how Autofac is going to provide parameters: If it can't fill in something that's required with something that isn't null, it's going to tell you about it.

The number of use cases where you have a property or constructor parameter that's marked as required but where you want explicit null is, I think, relatively small. On the other hand, we run into support and compatibility issues:

• If the property handling allows explicit null but constructor parameters don't, then it's a challenging support situation due to the inconsistency. Not only that, it implies we have to change the ability for delegates or singletons to allow null to be explicitly registered (so the required property can be resolved to null) or we do a sort of ResolveOptional on a required property, which... all of that just feels painfully inconsistent.
• If the property handling allows explicit null and we want to be consistent with constructor parameters, it means NRT is more than just informative analysis markup and has implication on whether something actually is required or optional. Changing that is going to potentially break current users in subtle and difficult-to-troubleshoot ways. In some cases, folks retrofitting existing code from no-NRT to NRT will get ArgumentNullException instead of DependencyResolutionException for things as they add markup but maybe forget to register services that they'd normally be reminded by DRE to register. Tests that used to fail will succeed and vice versa.

In the end, I do think it should be consistent, but I think that means making required properties work like constructor parameters and not allow them to be null; rather than making constructor parameters consistent and changing the entire notion of reflection-based resolution to consider NRT the same as something like DataAnnotations and imply optional vs. required.

[alistairjevans]

A couple of thoughts, which lean in various directions.

The first is the big thing that may knock this on the head entirely, which is a comment from your MSDI example:

// Note that the nullable
// annotation on the constructor does NOT currently imply this is optional - if Component2
// isn't registered, resolution fails.

So, MSDI does not consider the nullability of parameters when resolving them (unsurprising really). This means that if we were to do so, any time someone switched from MSDI to Autofac, they could get different "default" behaviour. That seems undesirable.

---

Second point, one thing not mentioned here is that Autofac does already have a way of indicating that some constructor parameters are optional; multiple constructors:

class Component 
{
    private IOptionalService? _service;

    public Component() 
    {
       _service = null;
    }

    public Component(IOptionalService service)
    {
       _service = service;
    }
}

That is basically the way that we prescribe indicating that some of the services aren't always available.

The issue with properties is that there is no such equivalent, so no way to express optional services in the same way.

---

Currently, PropertiesAutowired is always optional. I.e. if we can't supply a value, we move on, even if the property is a non-nullable type.

That would feel inconsistent if we made a general global change to allow nullable reference types, since required properties and constructors would have this notion of optional vs mandatory properties, but PropertiesAutowired wouldn't follow the same rules.

---

To be honest, I can absolutely see the utility of IServiceA? meaning an optional service, and IServiceA meaning required. The thing is, the .net ecosystem still hasn't really caught up to NRTs in it's entirety, so switching it on "by default" will likely be painful.

[TonyValenti]

Would you consider having options that control whether unsolvable required NRTs are considered optional?

If it were an option that was off by default, then you could use the same logic for constructor injectors as required properties.

Or, you might have two independent options: one that makes NRT optional for required and another that makes NRT optional for constructors.

[tillig]

I've been noodling on this for a few days and I think I'm still in the same place - NRT markup should not imply optional.

As noted, constructors already have an explicit way of indicating optional parameters:

// This won't compile because there are equivalent constructors listed.
public class Demo
{
  // No required parameters
  public Demo() {}

  // Still no required parameters - there's a default set.
  public Demo(Dependency? dep = null) { }

  // Is the first parameter required? Now it's confusing.
  public Demo(SomeService? svc, Dependency? dep = null) { }
}

It also occurs to me that nullable reference types and nullable value types are going to be problematic in combination if we start considering nullable things optional.

public class Demo
{
  public Demo(int? integer, Exception? ex) { }
}

Looking at something like that in ILDasm, the first one is valuetype[System.Runtime]System.Nullable`1<int32> integer and the second is class [System.Runtime]System.Exception exception. If you're really looking at it, it's easy to spot, but it implies you know about reference and value types, the Nullable<T> construct, etc. - which is not always the case. It'd also be pretty easy to skim past it when looking for reasons why something is or is not populated, like skimming past a missing semicolon or something.

I could see the potential value of having an option to enable the "nullable means optional" thing, but we'd need to consider Nullable<T> here, too, and what that would mean. And, honestly, I'm not a fan of options if we can choose a path and stick with it:

• Options are a support burden. We have to make sure all combinations of all options always work. This gets hard with features like decorators and interceptors, where we already have a few issues filed talking about how combining all these things is yielding unpredictable results. We have to answer questions about options and write documentation (Why didn't it fill in int? integer? What's Nullable<T>?). We have to benchmark the combinations of options and make sure they still perform. Options aren't free.
• Options are a design failure. There's a prevailing (though potentially not 100% correct) theory that options are a form of design failure. Having options means there's more burden on the users of your product to understand those options and set them up correctly. Not having options may feel inflexible or opinionated, but it also means "easier path to the Pit of Success," which is a goal of good design.
• Options are a performance penalty. Every option we add means we have to check the value of that option at runtime and change behavior based on the option. Adding another if/else block in every resolve pipeline adds a tiny perf impact, which doesn't seem like much, but folks benchmark these things, so it matters.

And, again, we aren't the compiler. Once you start loading classes into IoC containers, initialization is an opinionated thing. We don't have to support all the potential scenarios the compiler does.

It seems like not allowing required-yet-nullable properties to be optional is:

• Consistent
• Less confusing (NRT vs NVT)
• The majority use case

If folks don't want the property to be required to be initialized, don't mark it required. I'd say the same thing to someone asking for a way to make a nullable constructor parameter optional - don't want it to be required, either make a constructor overload without it or provide a default value so it's seen as optional.

[TonyValenti]

@tillig - thanks so much for thinking though all of this. Optional dependencies are certainly in the minority of my use cases so I'm completely fine with however you move this forward. I'm just really itching for required properties to land so that I can start trimming out thousands of lines of boilerplate constructors.

[TonyValenti]

I know everyone here is a volunteer, but I was wondering when the next update might become available? I'm itching to start refactoring all my code to take advantage of required properties.

[tillig]

While I recognize and appreciate the excitement, given this will be a major semantic version release we need to ensure we get any other breaking changes in here while we have the opportunity. We also need to make sure we get in any pending large features, like PR #1350 to allow better support for plugin frameworks. Given that, I'm not going to commit to a timeline. It'll get here when it gets here.

Unfortunately, since we are unpaid volunteers currently swamped in our day jobs, that means things move a little slower than on projects that have a steady income like ESLint. We're also not working entirely unemployed like on core-js since we have families to support.

If folks want to see things move faster, contributions are the way to go - not even really monetary contributions, but time contributions. Go follow the autofac tag on Stack Overflow and answer questions relatively real-time for folks (ie, don't let them sit for days unanswered, do it same/next day). Chip in on issues and PRs, both for core Autofac and for the other integration packages. Take ownership of one of the integration packages. Update the docs. Make sure the samples are up to date.

The more time we don't have to spend doing other things, the more we can focus what little time we have on getting core Autofac out the door.

[TonyValenti]

I think this can be closed.

An easy enough solution is the have an OptionalT class that has a constructor that accepts a T with a default value.