-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Fix sanitization of type names in validations generator #61952
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
Conversation
@@ -92,6 +97,38 @@ private ValidatableTypeInfo CreateTestService() | |||
] | |||
); | |||
} | |||
private ValidatableTypeInfo CreateDictionary_2() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#61953 will change how this looks but we validate that compilable names are emitted here.
.Replace(",", "_") | ||
.Replace(" ", "_"); | ||
// Replace invalid characters with underscores | ||
return InvalidNameCharsRegex.Replace(typeName, "_"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can't start with a digit presumably?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's a restriction on metadata names starting with digits per the CLR spec. However, since we derive this MetadataName that we pass to this method from a TypeSymbol resolve via static analysis, it's probably come from C# code where an identifier starting with a number is invalid.
If we do hit that case, I think emitting a method name starting with _
is OK syntax so the generator shouldn't emit invalid syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 👍
Gonna merge this so it lands before the preview.5 cut-off. Happy to address feedback after merge in a follow-up PR. |
This pull request introduces several updates to the
ValidationsGenerator
in theMicrosoft.AspNetCore.Http
project, focusing on improving type name sanitization.SanitizeTypeName
with a compiledRegex
(InvalidNameCharsRegex
) for improved clarity and maintainability. (src/Http/Http.Extensions/gen/Microsoft.AspNetCore.Http.ValidationsGenerator/Emitters/ValidationsGenerator.Emitter.cs
, [1] [2]Fixes #61388.
Note: I realize that as a follow-up to this change we need to make updates to the implementation to support validating dictionary values. Since that's a bit more involved I'll pull it out into a separate PR.