Skip to content

Fix sanitization of type names in validations generator - #61952

Merged
captainsafia merged 1 commit into
mainfrom
safia/vsg-generic-types
May 16, 2025
Merged

captainsafia merged 1 commit into
mainfrom
safia/vsg-generic-types

Conversation

@captainsafia

Copy link
Copy Markdown
Contributor

This pull request introduces several updates to the ValidationsGenerator in the Microsoft.AspNetCore.Http project, focusing on improving type name sanitization.

  • Replaced the custom logic for sanitizing type names in SanitizeTypeName with a compiled Regex (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.

@github-actions github-actions Bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label May 15, 2025
@captainsafia captainsafia added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-validation Issues related to model validation in minimal and controller-based APIs and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels May 15, 2025
]
);
}
private ValidatableTypeInfo CreateDictionary_2()

Copy link
Copy Markdown
Contributor Author

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, "_");

Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor Author

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.

@mikekistler mikekistler left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good! 👍

@captainsafia

Copy link
Copy Markdown
Contributor Author

Gonna merge this so it lands before the preview.5 cut-off. Happy to address feedback after merge in a follow-up PR.

@captainsafia
captainsafia merged commit 944f0f5 into main May 16, 2025
@captainsafia
captainsafia deleted the safia/vsg-generic-types branch May 16, 2025 21:02
@dotnet-policy-service dotnet-policy-service Bot added this to the 10.0-preview5 milestone May 16, 2025
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 feature-validation Issues related to model validation in minimal and controller-based APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Min API Validation fails for some generic types

3 participants