Provide more extensive method documentation.

[DJ4ddi]

Something that I feel is sorely missing for this generator is clean XML docs. This PR adds all known properties covered by NSwag (which is reasonably complete in my opinion).

One extra that I added is response code documentation in the <throws> tag. Since this is not a common thing to do, I provided a setting to disable it.

I did not update any generated output (e.g. the files generated by tests or the docfx pages). There is also no command line option to change the setting. Test coverage of the new code is 96%.

[christianhelle]

@DJ4ddi First of all, thanks for taking the time to implement this!

Great idea and the implementation looks good to me

I'm a bit unsure if Refit at all throws an ApiException if the function returns Task<IApiResponse> or Task<IApiResponse<T>>.

We can merge this in, investigate the actual behavior, then eventually implement checking if the Refitter is configured with the --use-api-response and only add the <throws /> XML docs if exceptions are expected

What do you think @DJ4ddi ?

[christianhelle]

@DJ4ddi I pulled your branch and reviewed it locally to do a couple of experiments

I generated 2 Refit clients: using functions that returns Task<IApiResponse<T>> and another one that returns Task<T>

I ran the code in a small console app

using Refit;
using System;
using System.Threading.Tasks;

await TestPetstoreUsingApiResponse();
await TestPetstoreUsingDirectTypes();
await Task.Delay(0);

static async Task TestPetstoreUsingApiResponse()
{
    var responseClient = RestService.For<PetstoreUsingApiResponse.ISwaggerPetstore>("https://petstore3.swagger.io/api/v3");
    var response = await responseClient.GetPetById(1);

    Console.WriteLine("## Using IApiResponse<T> as return type ##");
    Console.WriteLine($"HTTP Status Code: {response.StatusCode}");
    Console.WriteLine($"Name: {response.Content?.Name}");
    Console.WriteLine($"Status: {response.Content?.Status}");

    var pets = await responseClient.FindPetsByStatus(PetstoreUsingApiResponse.Status.Available);
    Console.WriteLine("Found " + pets.Content?.Count + " available pet(s)");

    var taggedPets = await responseClient.FindPetsByTags(new[] { "tag1Updated", "new" });
    Console.WriteLine("Found " + taggedPets.Content?.Count + " tagged pet(s)");
    Console.WriteLine();
}

static async Task TestPetstoreUsingDirectTypes()
{
    var client = RestService.For<Petstore.ISwaggerPetstoreDirect>("https://petstore3.swagger.io/api/v3");
    var pet = await client.GetPetById(1);

    Console.WriteLine("## Using types directly ##");
    Console.WriteLine($"Name: {pet.Name}");
    Console.WriteLine($"Status: {pet.Status}");

    var pets = await client.FindPetsByStatus(Petstore.Status.Available);
    Console.WriteLine("Found " + pets.Count + " available pet(s)");

    var taggedPets = await client.FindPetsByTags(new[] { "tag1Updated", "new" });
    Console.WriteLine("Found " + taggedPets.Count + " tagged pet(s)");
    Console.WriteLine();
}

and this is the output I got

## Using IApiResponse<T> as return type ##
HTTP Status Code: NotFound
Name:
Status:
Found  available pet(s)
Found  tagged pet(s)

## Using types directly ##
Unhandled exception. Refit.ApiException: Response status code does not indicate success: 404 (Not Found).
   at Refit.RequestBuilderImplementation.<>c__DisplayClass14_0`2.<<BuildCancellableTaskFuncForMethod>b__0>d.MoveNext() in /_/Refit/RequestBuilderImplementation.cs:line 288
--- End of stack trace from previous location ---
   at Program.<<Main>$>g__TestPetstoreUsingDirectTypes|0_1() in C:\projects\christianhelle\refitter\test\ConsoleApp\Program.cs:line 33
   at Program.<Main>$(String[] args) in C:\projects\christianhelle\refitter\test\ConsoleApp\Program.cs:line 6
   at Program.<Main>(String[] args)

[christianhelle]

@all-contributors please add @DJ4ddi for ideas

[allcontributors]

@christianhelle

I've put up a pull request to add @DJ4ddi! 🎉

[christianhelle]

Changing

        if (method.HasResult && !string.IsNullOrWhiteSpace(method.ResultDescription))
        {
            this.AppendXmlCommentBlock("returns", method.ResultDescription, code);
        }

        this.AppendXmlCommentBlock("throws", this.BuildErrorDescription(method.Responses), code, new Dictionary<string, string>
            { ["cref"] = "ApiException" });

into

        if (_settings.ReturnIApiResponse)
        {
            this.AppendXmlCommentBlock("returns", "An instance of IApiResponse", code);
        }
        else
        {
            if (method.HasResult && !string.IsNullOrWhiteSpace(method.ResultDescription))
            {
                this.AppendXmlCommentBlock("returns", method.ResultDescription, code);
            }
        }

        if (!_settings.ReturnIApiResponse)
            this.AppendXmlCommentBlock("throws", this.BuildErrorDescription(method.Responses), code, new Dictionary<string, string>
                { ["cref"] = "ApiException" });

will generate the <throws /> XML docs when the success response type is directly returned

        /// <summary>Add a new pet to the store</summary>
        /// <remarks>Add a new pet to the store</remarks>
        /// <param name="body">Create a new pet in the store</param>
        /// <returns>Successful operation</returns>
        /// <throws cref="ApiException">
        /// Thrown when the request returns a non-success status code:
        /// 405: Invalid input
        /// </throws>
        [Headers("Accept: application/xml, application/json")]
        [Post("/pet")]
        Task<Pet> AddPet([Body] Pet body);

and when the return type is IApiResponse or IApiResponse<T> then the generated comments will look like this:

        /// <summary>Add a new pet to the store</summary>
        /// <remarks>Add a new pet to the store</remarks>
        /// <param name="body">Create a new pet in the store</param>
        /// <returns>An instance of IApiResponse</returns>
        [Headers("Accept: application/xml, application/json")]
        [Post("/pet")]
        Task<IApiResponse<Pet>> AddPet([Body] Pet body);

[DJ4ddi]

You are correct, I did not consider the IApiResponse case. I'll make changes as suggested with one addition: I'd like to follow the same status code pattern as in the exception (meaning that if a generic response is returned, specific codes and their description can be added to the documentation).

[christianhelle]

That would be great @DJ4ddi

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5584982) 97.46% compared to head (07dc374) 97.65%.
Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #281      +/-   ##
==========================================
+ Coverage   97.46%   97.65%   +0.18%     
==========================================
  Files          60       62       +2     
  Lines        2131     2300     +169     
==========================================
+ Hits         2077     2246     +169     
  Misses         37       37              
  Partials       17       17              

Flag	Coverage Δ
unittests	97.65% <100.00%> (+0.18%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sonarcloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[DJ4ddi]

• Added support for returning IApiResponse.
• Renamed the setting for generating status code details.
• Added documentation for the returned Task when there is no result or response description.

@christianhelle

[christianhelle]

Excellent work @DJ4ddi