Skip to content

Conversation

@chrisoverzero
Copy link
Contributor

Fully Qualify Type Names When Resolving in RDG

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

These two places correspond to resolution of a service from DI when attributed with FromServicesAttribute or with
FromKeyedServicesAttribute. When the type name of the service is not fully qualified, the generator can generate code which fails to compile.

Description

For example, given a service ExampleService in namespace Http, the generator can generate code to resolve an instance of such a service like this:

var e_local = httpContext.RequestServices.GetRequiredKeyedService<Http.ExampleService>("example");

…but Microsoft.AspNetCore.Http is a nearer match for the reference to "Http", so the compiler will look for a type
Microsoft.AspNetCore.Http.ExampleService (which doesn't exist – at least, not at time of writing), and compilation will fail.

Fixes #58633

These two places correspond to resolution of a service from DI when
attributed with `FromServicesAttribute` or with
`FromKeyedServicesAttribute`. When the type name of the service is not
fully qualified, the generator can generate code which fails to compile.

For example, given a service `ExampleService` in namespace `Http`, the
generator can generate code to resolve an instance of such a service
like this:

```c#
var e_local = httpContext.RequestServices.GetRequiredKeyedService<Http.ExampleService>("example");
```

…but `Microsoft.AspNetCore.Http` is a nearer match for the reference to
"Http", so the compiler will look for a type
`Microsoft.AspNetCore.Http.ExampleService` (which doesn't exist – at
least, not at time of writing), and compilation will fail.

Fixes: dotnet#58633
@ghost ghost added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Oct 25, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Oct 25, 2024
Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

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

Thanks for opening a PR for this! I left one recommendation inline.

Also -- any chance we can add test coverage for this?

Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

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

Looks good overall! Left one small comment about a possibly unneeded using in the base class for tests.

@captainsafia captainsafia changed the title Fully Qualify Type Names When Resolving in RDG Fully qualify type names for services when resolving in RequestDelegateGenerator Oct 28, 2024
@captainsafia captainsafia added feature-rdg area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions labels Oct 28, 2024
@captainsafia captainsafia enabled auto-merge (squash) October 29, 2024 00:34
@chrisoverzero
Copy link
Contributor Author

The failures here look unrelated. Is there a way to kick the build off again, or is this waiting for something?

@captainsafia
Copy link
Member

The failures here look unrelated. Is there a way to kick the build off again, or is this waiting for something?

Our builds can be flakey sometimes. I've just kicked it to hopefully get this through. 🤞🏽

@captainsafia captainsafia merged commit 29c9b13 into dotnet:main Nov 4, 2024
27 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-preview1 milestone Nov 4, 2024
@chrisoverzero chrisoverzero deleted the feature/fully-qualify-type-names-when-resolving branch November 4, 2024 22:00
captainsafia pushed a commit that referenced this pull request Feb 11, 2025
…teGenerator (#58640)

* Fully Qualify Type Names When Resolving in RDG

These two places correspond to resolution of a service from DI when
attributed with `FromServicesAttribute` or with
`FromKeyedServicesAttribute`. When the type name of the service is not
fully qualified, the generator can generate code which fails to compile.

For example, given a service `ExampleService` in namespace `Http`, the
generator can generate code to resolve an instance of such a service
like this:

```c#
var e_local = httpContext.RequestServices.GetRequiredKeyedService<Http.ExampleService>("example");
```

…but `Microsoft.AspNetCore.Http` is a nearer match for the reference to
"Http", so the compiler will look for a type
`Microsoft.AspNetCore.Http.ExampleService` (which doesn't exist – at
least, not at time of writing), and compilation will fail.

Fixes: #58633

* Correct Symbol Display Format and Add Tests

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

Labels

area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc community-contribution Indicates that the PR has been added by a community member feature-rdg

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FromKeyedServices Causes Generated Code to Fail to Compile

3 participants