Skip to content

Conversation

@Chris-Wolfgang
Copy link
Contributor

Resolves #552 and #558

Introduced FromKeyedServicesAttribute for resolving services by key in method parameters. Updated ReflectionHelper to handle keyed service resolution using IKeyedServiceProvider. Added tests to validate this functionality.

Upgraded target frameworks to net8.0 across projects and updated test to target 8.0 and 9.0. Simplified reflection code by replacing Array.Empty<object>() with []. Updated dependencies to use Microsoft.Extensions.DependencyInjection.Abstractions and newer versions of related packages.

Enhanced test coverage with new tests for keyed DI and fixed typos in test method names. Adjusted project files to include additional package references and modernized exception messages for consistency.

Introduced `FromKeyedServicesAttribute` for resolving services by key in method parameters. Updated `ReflectionHelper` to handle keyed service resolution using `IKeyedServiceProvider`. Added tests to validate this functionality.

Upgraded target frameworks to `net8.0` and `net9.0` across projects. Simplified reflection code by replacing `Array.Empty<object>()` with `[]`. Updated dependencies to use `Microsoft.Extensions.DependencyInjection.Abstractions` and newer versions of related packages.

Enhanced test coverage with new tests for keyed DI and fixed typos in test method names. Adjusted project files to include additional package references and modernized exception messages for consistency.
natemcmaster and others added 3 commits December 27, 2025 10:37
Merged main branch into PR natemcmaster#560 to incorporate recent changes including:
- Updated package versions
- Target framework changes to net8.0

Resolved merge conflicts in:
- McMaster.Extensions.CommandLineUtils.csproj
- McMaster.Extensions.CommandLineUtils.Tests.csproj

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Three sample projects were still targeting net6.0 which is incompatible
with the main library now targeting net8.0. Updated:
- helloworld
- helloworld-async
- helloworld-attributes

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@codecov
Copy link

codecov bot commented Dec 27, 2025

Codecov Report

❌ Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.74%. Comparing base (5053c19) to head (1bc5c0b).

Files with missing lines Patch % Lines
src/CommandLineUtils/Internal/ReflectionHelper.cs 70.00% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #560      +/-   ##
==========================================
- Coverage   77.79%   77.74%   -0.05%     
==========================================
  Files         104      104              
  Lines        3333     3339       +6     
  Branches      728      731       +3     
==========================================
+ Hits         2593     2596       +3     
- Misses        580      581       +1     
- Partials      160      162       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@natemcmaster
Copy link
Owner

Thanks @Chris-Wolfgang . I pushed some updates to get this change to build with GitHub CI to validate it works.

The main library currently has no external package dependencies. Adding Microsoft.Extensions.DependencyInjection.Abstractions as a hard requirement would force all users to take this dependency, even if they don't use DI, and break the "minimal dependencies" philosophy of the core library.

What do you think about some of these alternatives?

Option 1: Reflection-based detection

Use reflection to detect if the keyed services types are available at runtime, without a compile-time dependency. The feature works automatically when users have the DI abstractions available; otherwise it's silently ignored (or throws a clear error only when someone actually uses [FromKeyedServices]).

Pros: No new dependencies, works if user brings their own DI
Cons: More complex code, potential runtime performance overhead (can be mitigated with caching)

Option 2: Separate package

Create a new package like McMaster.Extensions.CommandLineUtils.DependencyInjection that provides the keyed services integration, similar to how Hosting.CommandLine works.

Pros: Clean separation of concerns
Cons: Users need an additional package for keyed DI


I'm inclined toward Option 1 since it preserves the zero-dependency nature of the core library while still enabling the feature for users who have the DI abstractions. What do you think?

@Chris-Wolfgang
Copy link
Contributor Author

Chris-Wolfgang commented Dec 28, 2025

Thanks @Chris-Wolfgang . I pushed some updates to get this change to build with GitHub CI to validate it works.

The main library currently has no external package dependencies. Adding Microsoft.Extensions.DependencyInjection.Abstractions as a hard requirement would force all users to take this dependency, even if they don't use DI, and break the "minimal dependencies" philosophy of the core library.

What do you think about some of these alternatives?

Option 1: Reflection-based detection

Use reflection to detect if the keyed services types are available at runtime, without a compile-time dependency. The feature works automatically when users have the DI abstractions available; otherwise it's silently ignored (or throws a clear error only when someone actually uses [FromKeyedServices]).

Pros: No new dependencies, works if user brings their own DI Cons: More complex code, potential runtime performance overhead (can be mitigated with caching)

Option 2: Separate package

Create a new package like McMaster.Extensions.CommandLineUtils.DependencyInjection that provides the keyed services integration, similar to how Hosting.CommandLine works.

Pros: Clean separation of concerns Cons: Users need an additional package for keyed DI

I'm inclined toward Option 1 since it preserves the zero-dependency nature of the core library while still enabling the feature for users who have the DI abstractions. What do you think?

Maybe I'm not understanding option 1 but if I remove the reference to Microsoft.Extensions.DependencyInjection.Abstractions then we lose FromKeyedServicesAttribute and IKeyedServiceProvider so code like this would not work

// Check for FromKeyedServicesAttribute
var keyedAttr = methodParam.GetCustomAttribute<FromKeyedServicesAttribute>();
if (keyedAttr != null)
{
    var keyedServiceProvider = (IKeyedServiceProvider)command.AdditionalServices!;
    arguments[i] = keyedServiceProvider.GetKeyedService(methodParam.ParameterType, keyedAttr.Key)
                                       ?? throw new InvalidOperationException($"No keyed service found for type {methodParam.ParameterType} and key '{keyedAttr.Key}'.");
                    }

@natemcmaster
Copy link
Owner

To explain more about what I mean by reflection, I had Claude generate the reflection equivalent. Quoting below…

—-

Here's how the BindParameters method could be rewritten to use reflection instead:

// Cache these at class level for performance
private static readonly Type? s_fromKeyedServicesAttributeType;
private static readonly PropertyInfo? s_keyProperty;
private static readonly Type? s_keyedServiceProviderType;
private static readonly MethodInfo? s_getKeyedServiceMethod;

static ReflectionHelper()
{
    // Try to load keyed services types via reflection (available in .NET 8+)
    s_fromKeyedServicesAttributeType = Type.GetType(
        "Microsoft.Extensions.DependencyInjection.FromKeyedServicesAttribute, Microsoft.Extensions.DependencyInjection.Abstractions");

    if (s_fromKeyedServicesAttributeType != null)
    {
        s_keyProperty = s_fromKeyedServicesAttributeType.GetProperty("Key");
    }

    s_keyedServiceProviderType = Type.GetType(
        "Microsoft.Extensions.DependencyInjection.IKeyedServiceProvider, Microsoft.Extensions.DependencyInjection.Abstractions");

    if (s_keyedServiceProviderType != null)
    {
        s_getKeyedServiceMethod = s_keyedServiceProviderType.GetMethod(
            "GetKeyedService",
            new[] { typeof(Type), typeof(object) });
    }
}

public static object?[] BindParameters(
    MethodInfo method,
    CommandLineApplication command,
    CancellationToken cancellationToken)
{
    var methodParams = method.GetParameters();
    var arguments = new object?[methodParams.Length];

    for (var i = 0; i < methodParams.Length; i++)
    {
        var methodParam = methodParams[i];

        // Check for keyed services attribute using reflection
        if (TryResolveKeyedService(methodParam, command, out var keyedService))
        {
            arguments[i] = keyedService;
            continue;
        }

        // ... existing parameter binding logic for CancellationToken,
        // CommandLineApplication, etc. ...

        // Fall back to standard service resolution
        var service = command.AdditionalServices?.GetService(methodParam.ParameterType);
        if (service != null)
        {
            arguments[i] = service;
        }
    }

    return arguments;
}

private static bool TryResolveKeyedService(
    ParameterInfo parameter,
    CommandLineApplication command,
    out object? service)
{
    service = null;

    // If keyed services types aren't available, skip
    if (s_fromKeyedServicesAttributeType == null ||
        s_keyProperty == null ||
        s_keyedServiceProviderType == null ||
        s_getKeyedServiceMethod == null)
    {
        return false;
    }

    // Check if parameter has [FromKeyedServices] attribute
    var keyedAttr = parameter.GetCustomAttribute(s_fromKeyedServicesAttributeType);
    if (keyedAttr == null)
    {
        return false;
    }

    // Get the key from the attribute
    var key = s_keyProperty.GetValue(keyedAttr);

    // Check if the service provider supports keyed services
    if (command.AdditionalServices == null ||
        !s_keyedServiceProviderType.IsInstanceOfType(command.AdditionalServices))
    {
        throw new InvalidOperationException(
            $"Parameter '{parameter.Name}' has [FromKeyedServices] attribute, " +
            "but AdditionalServices does not implement IKeyedServiceProvider. " +
            "Ensure you're using a DI container that supports keyed services (.NET 8+).");
    }

    // Invoke GetKeyedService via reflection
    service = s_getKeyedServiceMethod.Invoke(
        command.AdditionalServices,
        new[] { parameter.ParameterType, key });

    if (service == null)
    {
        throw new InvalidOperationException(
            $"No keyed service found for type '{parameter.ParameterType}' " +
            $"with key '{key}'.");
    }

    return true;
}

Benefits of this approach:

  1. No new package dependency - The core library continues to work without Microsoft.Extensions.DependencyInjection.Abstractions

  2. Graceful degradation - On older frameworks or DI containers, the keyed services feature simply isn't available, but nothing breaks

  3. Performance - Types and methods are cached in static fields, so reflection cost is only paid once at startup

  4. Consistent pattern - This follows how the library already handles optional DI integration

Alternative: Keep it in Hosting.CommandLine

Another option would be to add this feature only to the McMaster.Extensions.Hosting.CommandLine package, which already has DI dependencies. Users of the generic host integration would get keyed services support, while the core library stays dependency-free.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade to .NET 8 and do a new release

2 participants