Skip to content

Conversation

@mitchdenny
Copy link
Member

Fixes #17761

Developers don't have a choice as to whether the NullableContextAttribute is emitted and decorates their method, it's a side effect of the arguments. This adds logic to ignore that attribute when considering whether a convention method is valid or not.

@mitchdenny mitchdenny requested a review from a team as a code owner July 10, 2023 02:47
@ghost ghost added old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Jul 10, 2023
@mitchdenny mitchdenny self-assigned this Jul 10, 2023
@mitchdenny mitchdenny requested a review from captainsafia July 10, 2023 02:51
@mitchdenny mitchdenny removed the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Jul 10, 2023
@mitchdenny
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

attribute is ProducesDefaultResponseTypeAttribute ||
attribute is ApiConventionNameMatchAttribute;
attribute is ApiConventionNameMatchAttribute ||
attribute.GetType().FullName == "System.Runtime.CompilerServices.NullableContextAttribute";
Copy link
Member

@captainsafia captainsafia Jul 10, 2023

Choose a reason for hiding this comment

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

Suggested change
attribute.GetType().FullName == "System.Runtime.CompilerServices.NullableContextAttribute";
attribute.GetType().FullName == "System.Runtime.CompilerServices.NullableContextAttribute";

Is there a reason we can't do attribute is NullableContextAttribute here?

Copy link
Member Author

Choose a reason for hiding this comment

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

NullableContextAttribute is a compiler code spit into the users code, example from SharpLab:

https://sharplab.io/#v2:EYLgtghglgdgNAExAagD4AEAsACAYge3wAoAhCAJwH5tgBKAWACgBvJgXyafQCZszymrRh0ZA===

@mitchdenny
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@mitchdenny mitchdenny merged commit 31545df into dotnet:main Jul 11, 2023
@ghost ghost added this to the 8.0-preview7 milestone Jul 11, 2023
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.

Custom web API convention + nullable = exception

2 participants