Skip to content

Conversation

captainsafia
Copy link
Member

Closes #61035.

@captainsafia captainsafia requested a review from a team as a code owner March 24, 2025 22:16
@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Mar 24, 2025
@captainsafia
Copy link
Member Author

@BrennanConroy @martincostello I talked with both of you online about the notion of switching to a string-based key and what the key would be. Turns out that MetadataName isn't full-proof since it's only defined/exposed for Types so we still have to figure out a mapping for MethodInfo-based values.

I dusted off an old idea around reusing the DocumentationIds that exist in the XML doc comment to support this. We know emit a helper method that emits a documentation ID from PropertyInfo, Type, or MethodInfo. I used Copilot to iterate on this API and it seems to provide coverage for the test cases we have here.

Worst case scenario: the failure mode here is a lot nicer since we won't emit any compiler errors for invalid keys. In the event that happens, the implementation will no-op and fail to apply a doc comment but it shouldn't impact users builds.

if (!omitGenericArity)
{
int arity = genericDef.GetGenericArguments().Length;
defName += "`" + arity;
Copy link
Member

Choose a reason for hiding this comment

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

I would err on the side of caution here and format the arity explicitly with CultureInfo.InvariantCulture so that users on systems configured for non-English won't potentially get incorrect number formatting (e.g. scenario of 1000 becoming 1,000 or 1.000). Might be some other occurrences of this elsewhere too.

Copy link
Member

@martincostello martincostello left a comment

Choose a reason for hiding this comment

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

:shipit:

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

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[OpenAPI] error CS0308: The non-generic type 'TypeName' cannot be used with type arguments

4 participants