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

NonNullableReferenceTypesAsRequired should not consider MvcOptions #3203

Open
pinkfloydx33 opened this issue Dec 16, 2024 · 1 comment
Open
Labels
bug help-wanted A change up for grabs for contributions from the community

Comments

@pinkfloydx33
Copy link

Describe the bug

The NonNullableReferenceTypesAsRequired option only works if you've also set the MvcOptions.SuppressImplicitRequiredAttributeForNonNullableReferenceTypes to false

This is surprising. If I opt into a feature it shouldn't be predicated on an MVC-specific option also being enabled.

Expected behavior

NonNullableReferenceTypesAsRequired should work no matter how the MvcOption is configured.

Actual behavior

The feature does not work if MvcOptions.SuppressImplicitRequiredAttributeForNonNullableReferenceTypes is true

Steps to reproduce

No response

Exception(s) (if any)

No response

Swashbuckle.AspNetCore version

7.2.0

.NET Version

Net9

Anything else?

We are trying to upgrade to NET9. We've had a custom filter in place for years that does the exact same thing as the new NonNullableReferenceTypesAsRequired option and we were excited to finally be able to remove it.

Unfortunately we explicitly configure the MvcOptions.SuppressImplicitRequiredAttributeForNonNullableReferenceTypes to true. This is because the MVC option is a huge hammer. We'd like our API to be forgiving in certain things it accepts, but have the API specification indicate the desired state of affairs. Required-ness is also not limited to inputs, as read-only schemas can indicate properties are required, ie. that they will exist in the returned data. We also rely on the readonly properties when passing our api specification off to code generators, but again don't want to enforce it across the board at the MVC layer except where done explicitly.

In the PR that added this, there was a little discussion on including the MvcOptions, but it was mostly about breaking APIs.

It's also surprising that if I were building a netstandard library and opt into the feature that it would work since it ifdef's around, which feels very inconsistent. Also given that this is predicated on an MVC-specific option, does that mean the filter won't work properly with minimal apis?

Please consider either removing the dependency on MvcOptions or providing us a way to forcefully opt-in. We'll keep our custom filter in the meantime, bummer!

@martincostello
Copy link
Collaborator

Relevant line of code for this request:

var markNonNullableTypeAsRequired = _generatorOptions.NonNullableReferenceTypesAsRequired
#if !NETSTANDARD2_0
&& (!_mvcOptions?.Value.SuppressImplicitRequiredAttributeForNonNullableReferenceTypes ?? true)
#endif
&& (dataProperty.MemberInfo?.IsNonNullableReferenceType() ?? false);

@martincostello martincostello added the help-wanted A change up for grabs for contributions from the community label Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help-wanted A change up for grabs for contributions from the community
Projects
None yet
Development

No branches or pull requests

2 participants