API analyzers should support undeclared 200 status codes
jawn opened this issue · 8 comments
Describe the bug
API analyzers should flag an undeclared 200 status code, and provide an option to fix it.
To Reproduce
Steps to reproduce the behavior:
- Using ASP.NET Core 3.0 with analyzers enabled
<PropertyGroup>
<IncludeOpenAPIAnalyzers>true</IncludeOpenAPIAnalyzers>
</PropertyGroup>
- Run this code (adapted from this sample with the ProducesResponseType attribute removed)
// GET api/contacts/{guid}
[HttpGet("{id}", Name = "GetById")]
public IActionResult Get(string id)
{
var contact = _contacts.Get(id);
if (contact == null)
{
return NotFound();
}
return Ok(contact);
}
- The API analyzer flags the NotFound using a warning (this is expected).
- The API analyzer does not flag the return Ok(...); (not expected)
Expected behavior
- The API analyzer does flag the return Ok(...) using a warning.
- An option to fix this warning is provided, choosing it adds the following attribute
[ProducesResponseType(typeof(Contact),StatusCodes.Status200OK)]
Why I think this is a bug
From the docs:
The analyzers package notifies you of any controller action that:
- Returns an undeclared status code.
- Returns an undeclared success result.
Additional context
Follow up to dotnet/AspNetCore.Docs#15269 (comment)
.NET Core SDK: 3.0.100
.NET Core runtimes installed:
Microsoft.NETCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
The 200 Status code is implicit for every action that does not specify any ProducesResponseType attributes. Admittedly it might a bit confusing for it to not flag the response type, but the analyzer inspect those.
Strictly speaking, this means no warning needs to be generated by the analyzers.
Would it still be possible to provide an optional quick fix to add the strongly typed ProducesResponseType for the Status Code 200?
Would it still be possible to provide an optional quick fix to add the strongly typed ProducesResponseType for the Status Code 200?
I think at some point our plan was to provide a code fix to use ActionResult<T> when the action returns an IActionResult and uses Ok, Created etc, but that was never developed. This does seem like an area the analyzer could improve upon.
Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.
We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.
Thanks for contacting us.
We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.
The 200 Status code is implicit for every action that does not specify any
ProducesResponseTypeattributes.
This produces weird behavior. Initially it warns you if you haven't documented another response code like 404. Upon fixing that warning, only then does it warn you that you haven't documented 200. And if you begin to rely on this warning, it will bite you when you have an action that only returns 200.
To maximize trust in and benefit from the analyzer, it should always warn if you don't document 200.
Thanks for contacting us.
We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.