Skip to content

Comments

Allow to use consts in types as service KeySelector#20

Merged
Dreamescaper merged 2 commits intomainfrom
use-const-as-key
Mar 21, 2025
Merged

Allow to use consts in types as service KeySelector#20
Dreamescaper merged 2 commits intomainfrom
use-const-as-key

Conversation

@Dreamescaper
Copy link
Owner

@Dreamescaper Dreamescaper commented Mar 21, 2025

[GenerateServiceRegistrations(AssignableTo = typeof(IService), KeySelector = "Key")]
public partial static IServiceCollection AddKeyedServices(this IServiceCollection services);

public class MyService1 : IService 
{
    public const string Key = "MSR1";
}
public class MyService2 : IService 
{
    public const string Key = "MSR2";
}

@Dreamescaper Dreamescaper requested a review from Copilot March 21, 2025 14:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates the service registration code to support using constant fields or static properties as key selectors instead of only static methods. Key changes include:

  • Introducing a new KeySelectorType enum and updating code paths in AttributeModel, DependencyInjectionGenerator, and related files.
  • Modifying tests and documentation to reflect the changes.
  • Removing the diagnostic for a missing key selector method in favor of treating the key selector as a type member when a matching static method isn’t found.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
ServiceScan.SourceGenerator.Tests/AddServicesTests.cs Added a new test case to validate using constant fields as key selectors.
ServiceScan.SourceGenerator/DependencyInjectionGenerator.ParseMethodModel.cs Updated key selector lookup logic to support type members.
ServiceScan.SourceGenerator.Playground/Services.cs Revised service classes to include constant Key members and updated naming values.
ServiceScan.SourceGenerator/DependencyInjectionGenerator.cs Modified registration generation to rely on the new KeySelectorType handling.
ServiceScan.SourceGenerator/Model/AttributeModel.cs Replaced KeySelectorGeneric with KeySelectorType and updated key selector determination.
ServiceScan.SourceGenerator.Tests/DiagnosticTests.cs Removed obsolete diagnostic test for non-existing key selector methods.
ServiceScan.SourceGenerator/GenerateAttributeSource.cs Updated XML documentation to describe the dual key selector support.
README.md Revised documentation to include guidance on using const fields or static properties as key selectors.
ServiceScan.SourceGenerator.Playground/ServiceCollectionExtensions.cs Updated attribute usage to include the new KeySelector property.
ServiceScan.SourceGenerator/DiagnosticDescriptors.cs Removed descriptor for missing key selector method.
ServiceScan.SourceGenerator/DependencyInjectionGenerator.FindServicesToRegister.cs Adjusted parameter usage from KeySelectorGeneric to KeySelectorType.
ServiceScan.SourceGenerator/Model/ServiceRegistrationModel.cs Updated model to reflect the new key selector handling.
Comments suppressed due to low confidence (1)

ServiceScan.SourceGenerator/Model/AttributeModel.cs:49

  • [nitpick] Consider adding a comment to explain why the code falls back to 'TypeMember' for key selectors when no matching static method is found.
keySelectorType = Model.KeySelectorType.TypeMember;


public const string Key = "Key1";
}
public class MyServ111111111ice3 : IService
Copy link

Copilot AI Mar 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The class name 'MyServ111111111ice3' appears to contain a typo. Consider renaming it to 'MyService3' or another appropriate name.

Suggested change
public class MyServ111111111ice3 : IService
public class MyService3 : IService

Copilot uses AI. Check for mistakes.
@Dreamescaper Dreamescaper requested a review from Copilot March 21, 2025 14:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces support for using constant fields and static properties as key selectors for service registrations, updating both source generation and diagnostic behaviors accordingly.

  • Replace the previous KeySelector method diagnostic with a fallback to a TypeMember when the key selector method is not found.
  • Update various components (attribute model, registration model, generator, and documentation) to consistently use the new KeySelectorType enum.
  • Remove outdated tests and diagnostics related to the missing KeySelector method.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
ServiceScan.SourceGenerator.Tests/AddServicesTests.cs Adds a new test to verify registration when using constant fields as key selectors.
ServiceScan.SourceGenerator/DependencyInjectionGenerator.ParseMethodModel.cs Updates key selector handling logic to use KeySelectorType and removes the diagnostic for a missing static method.
ServiceScan.SourceGenerator/DependencyInjectionGenerator.cs Switches to use KeySelector and KeySelectorType in generating the service registrations.
ServiceScan.SourceGenerator/Model/AttributeModel.cs Replaces the old boolean KeySelectorGeneric with a robust KeySelectorType enum.
ServiceScan.SourceGenerator/GenerateAttributeSource.cs Updates the documentation to reflect the support for both method and constant/key property selectors.
ServiceScan.SourceGenerator.Tests/DiagnosticTests.cs Removes the obsolete test for missing KeySelector method, aligning with the new behavior.
ServiceScan.SourceGenerator/DiagnosticDescriptors.cs Removes the diagnostic descriptor for a missing KeySelector method.
ServiceScan.SourceGenerator/DependencyInjectionGenerator.FindServicesToRegister.cs and ServiceRegistrationModel.cs Adjusts references to use KeySelectorType in place of the old KeySelectorGeneric.
Comments suppressed due to low confidence (2)

ServiceScan.SourceGenerator.Tests/DiagnosticTests.cs:202

  • The removal of the 'KeySelectorMethodDoesNotExist' test reduces coverage for scenarios where a KeySelector method is missing. If the change in behavior (falling back to TypeMember) is intentional to support constants and static properties, consider adding a test to verify that an absent method correctly results in a fallback rather than a silent failure.
[Fact] public void KeySelectorMethodDoesNotExist() { ... }

ServiceScan.SourceGenerator/Model/AttributeModel.cs:40

  • [nitpick] Falling back to KeySelectorType.TypeMember when a static key selector method is not found could lead to ambiguity if a user mistakenly misspells the member name. Consider adding documentation or a warning to guide users in cases where the intended member is absent.
if (keySelectorMethod != null) { ... } else { keySelectorType = Model.KeySelectorType.TypeMember; }

@Dreamescaper Dreamescaper merged commit 424f1f9 into main Mar 21, 2025
1 check passed
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.

1 participant