aspnet / Mvc

[Archived] ASP.NET Core MVC is a model view controller framework for building dynamic web sites with clean separation of concerns, including the merged MVC, Web API, and Web Pages w/ Razor. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
5.62k stars 2.14k forks source link

Code description of HttpMethodAttribute may be misleading #8661

Closed Prologh closed 5 years ago

Prologh commented 5 years ago

Is this a Bug or Feature request?:

I think it is a slightly misleading code description for HttpMethodAttribute and ones deriving from it e.g. HttpGetAttribute.

Steps to reproduce (preferably a link to a GitHub repo with a repro project):

https://github.com/Prologh/http-method-attributes-sample

Description of the problem:

I think that code commentary on HttpMethodAttribute and on ones deriving from it e.g. HttpGetAttribute may be misleading. By saying code commentary I mean small description of an attribute placed at the top of the class in between the <summary> tags.

For HttpMethodAttribute it states:

Identifies an action that only supports a given set of HTTP methods.

...and for HttpGetAttribute, for example:

Identifies an action that only supports the HTTP GET method.

I am no natively speaking English man, but for me that "only" is suggesting that controller action annotated with such attribute is bound to support only specific type of HTTP method/s, which is untrue since HttpMethodAttribute allows using multiple attributes on the same member, as defined in AttributeUsage in source file.

In referenced by me sample repository, in HomeController.cs there are two attributes (HttpGetAttribute and HttpPostAttribute) applied on Index() method. Both are working. Index() method supports both HTTP GET and HTTP POST requests. If my way of understanding of code commentary is correct, then such behaviour is conflicting to what the code commentary is saying.

Take note that I am aware, that code written in such manner violates some good practises how to construct web applications or APIs but here it does not matter. Why? Because if no compile-time error is stopping me from writing such code, then code commentary should include such scenario.

Suggested solution to the problem

Remove the "only" word from in-code commentary description of HttpMethodAttribute class and derived ones.

Version of Microsoft.AspNetCore.All:

2.1.4

mkArtakMSFT commented 5 years ago

Thanks for contacting us, @Prologh. Would you be interested in submitting a PR for this? /cc @pranavkm

Prologh commented 5 years ago

Sure. Pull request submitted.