Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent CS0121 (ambiguous method calls) resulting from config binding source gen #86363

Closed
layomia opened this issue May 17, 2023 · 8 comments
Closed
Assignees
Labels
area-Extensions-Configuration blocking-release bug source-generator Indicates an issue with a source generator feature
Milestone

Comments

@layomia
Copy link
Contributor

layomia commented May 17, 2023

From @andrewlock in #44493 (comment):


One potential issue I can foresee with the design of the generated code (because I've run into a similar issue with my own generators) is around internal classes and InternalsVisibleTo.

The fact that the generated code is always a fixed type (GeneratedConfigurationBinder) in the global namespace, means that something like this will no longer compile:

Project 1:

[assembly:InternalsVisible2(Project2)]
var config = new ConfigurationBuilder().Build();
var settings = new SomeOptions();
section.Bind(settings);

Project 2: (has a <ProjectReference> to Project1)

var config = new ConfigurationBuilder().Build();
var settings = new MoreOptions();
section.Bind(settings); // 👈 error

Gives

[CS0121] The call is ambiguous between the following methods or properties: 'GeneratedConfigurationBinder.Configure<T>(Microsoft.Extensions.DependencyInjection.IServiceCollection, Microsoft.Extensions.Configuration.IConfiguration)' and 'GeneratedConfigurationBinder.Configure<T>(Microsoft.Extensions.DependencyInjection.IServiceCollection, Microsoft.Extensions.Configuration.IConfiguration)'

You might consider it an edge case, but it was one of the first issues I ran into when building my first source generator.

Note: I've just seen that preview 4 bits are going onto NuGet, so apologies if this is no longer relevant! :D (Edit: looks like the preview 3 package is broken for me)

@layomia layomia added bug area-Extensions-Configuration source-generator Indicates an issue with a source generator feature labels May 17, 2023
@layomia layomia added this to the 8.0.0 milestone May 17, 2023
@layomia layomia self-assigned this May 17, 2023
@ghost
Copy link

ghost commented May 17, 2023

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

From @andrewlock in #44493 (comment):


One potential issue I can foresee with the design of the generated code (because I've run into a similar issue with my own generators) is around internal classes and InternalsVisibleTo.

The fact that the generated code is always a fixed type (GeneratedConfigurationBinder) in the global namespace, means that something like this will no longer compile:

Project 1:

[assembly:InternalsVisible2(Project2)]
var config = new ConfigurationBuilder().Build();
var settings = new SomeOptions();
section.Bind(settings);

Project 2: (has a <ProjectReference> to Project1)

var config = new ConfigurationBuilder().Build();
var settings = new MoreOptions();
section.Bind(settings); // 👈 error

Gives

[CS0121] The call is ambiguous between the following methods or properties: 'GeneratedConfigurationBinder.Configure<T>(Microsoft.Extensions.DependencyInjection.IServiceCollection, Microsoft.Extensions.Configuration.IConfiguration)' and 'GeneratedConfigurationBinder.Configure<T>(Microsoft.Extensions.DependencyInjection.IServiceCollection, Microsoft.Extensions.Configuration.IConfiguration)'

You might consider it an edge case, but it was one of the first issues I ran into when building my first source generator.

Note: I've just seen that preview 4 bits are going onto NuGet, so apologies if this is no longer relevant! :D (Edit: looks like the preview 3 package is broken for me)

Author: layomia
Assignees: layomia
Labels:

bug, area-Extensions-Configuration, source-generator

Milestone: 8.0.0

@layomia
Copy link
Contributor Author

layomia commented May 17, 2023

@davidfowl
Copy link
Member

@andrewlock do you also have Configure<T> calls in that assembly?

@layomia
Copy link
Contributor Author

layomia commented Jul 21, 2023

@tarekgh @eerhardt @ericstj, does it seem reasonable to generate random names to avoid the clashes? Thinking we can make this change when we adopt the Roslyn interceptors feature.

Right now this name clash issue doesn't seem crucial for 8.0, pending feedback from @andrewlock.

@davidfowl
Copy link
Member

@tarekgh @eerhardt @ericstj, does it seem reasonable to generate random names to avoid the clashes? Thinking we can make this change when we adopt the Roslyn interceptors feature.

This will fix it.

@tarekgh
Copy link
Member

tarekgh commented Jul 23, 2023

I have done similar things in the options source gen if you want to take a look at #89148

@ericstj ericstj added in-pr There is an active PR which will close this issue when it is merged and removed in-pr There is an active PR which will close this issue when it is merged labels Jul 25, 2023
@layomia layomia changed the title Prevent CS0121 (ambiguous method calls) resulting from config binding source gen Prevent CS0121 (ambiguous method calls) resulting from config binding & options source gen Aug 18, 2023
@layomia
Copy link
Contributor Author

layomia commented Aug 18, 2023

I have done similar things in the options source gen if you want to take a look at #89148

There was discussion in #90340 (comment) about the right approach to avoid name clashes. This needs to be addressed for the options generator, and I'd have to validate interception behavior in the config generator.

@layomia
Copy link
Contributor Author

layomia commented Aug 24, 2023

We're sticking with the interceptors approach for RC-1, where this problem does not arise. Options generator changes are being tracked in #90990.

@layomia layomia closed this as completed Aug 24, 2023
@layomia layomia changed the title Prevent CS0121 (ambiguous method calls) resulting from config binding & options source gen Prevent CS0121 (ambiguous method calls) resulting from config binding source gen Aug 24, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-Configuration blocking-release bug source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

No branches or pull requests

4 participants