Fix a few issues with the C generator (part 6)

[eafer]

Before completing the pull request at #14379, I want to draw attention to a newer issue I've recently noticed. Building the petstore sample results in the following warnings:

/home/runner/work/openapi-generator/openapi-generator/samples/client/petstore/c/model/order.c: In function ‘order_convertToJSON’:
/home/runner/work/openapi-generator/openapi-generator/samples/client/petstore/c/model/order.c:97:48: warning: implicit declaration of function ‘statusorder_ToString’ [-Wimplicit-function-declaration]
   97 |     if(cJSON_AddStringToObject(item, "status", statusorder_ToString(order->status)) == NULL)
      |                                                ^~~~~~~~~~~~~~~~~~~~
/home/runner/work/openapi-generator/openapi-generator/samples/client/petstore/c/model/order.c:97:48: warning: passing argument 3 of ‘cJSON_AddStringToObject’ makes pointer from integer without a cast [-Wint-conversion]
   97 |     if(cJSON_AddStringToObject(item, "status", statusorder_ToString(order->status)) == NULL)
      |                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                |
      |                                                int
In file included from /home/runner/work/openapi-generator/openapi-generator/samples/client/petstore/c/model/order.h:11,
                 from /home/runner/work/openapi-generator/openapi-generator/samples/client/petstore/c/model/order.c:4:
/home/runner/work/openapi-generator/openapi-generator/samples/client/petstore/c/model/../external/cJSON.h:255:112: note: expected ‘const char * const’ but argument is of type ‘int’
  255 | CJSON_PUBLIC(cJSON*) cJSON_AddStringToObject(cJSON * const object, const char * const name, const char * const string);
      |                                                                                             ~~~~~~~~~~~~~~~~~~~^~~~~~
[ 60%] Building C object CMakeFiles/openapi_petstore.dir/model/pet.c.o
/home/runner/work/openapi-generator/openapi-generator/samples/client/petstore/c/model/pet.c: In function ‘pet_convertToJSON’:
/home/runner/work/openapi-generator/openapi-generator/samples/client/petstore/c/model/pet.c:151:[48](https://github.com/OpenAPITools/openapi-generator/actions/runs/12053425233/job/33609057818?pr=20200#step:4:49): warning: implicit declaration of function ‘statuspet_ToString’ [-Wimplicit-function-declaration]
  151 |     if(cJSON_AddStringToObject(item, "status", statuspet_ToString(pet->status)) == NULL)
      |                                                ^~~~~~~~~~~~~~~~~~
/home/runner/work/openapi-generator/openapi-generator/samples/client/petstore/c/model/pet.c:1[51](https://github.com/OpenAPITools/openapi-generator/actions/runs/12053425233/job/33609057818?pr=20200#step:4:52):48: warning: passing argument 3 of ‘cJSON_AddStringToObject’ makes pointer from integer without a cast [-Wint-conversion]
  151 |     if(cJSON_AddStringToObject(item, "status", statuspet_ToString(pet->status)) == NULL)
      |                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                |
      |                                                int
In file included from /home/runner/work/openapi-generator/openapi-generator/samples/client/petstore/c/model/pet.h:11,
                 from /home/runner/work/openapi-generator/openapi-generator/samples/client/petstore/c/model/pet.c:4:
/home/runner/work/openapi-generator/openapi-generator/samples/client/petstore/c/model/../external/cJSON.h:2[55](https://github.com/OpenAPITools/openapi-generator/actions/runs/12053425233/job/33609057818?pr=20200#step:4:56):112: note: expected ‘const char * const’ but argument is of type ‘int’
  255 | CJSON_PUBLIC(cJSON*) cJSON_AddStringToObject(cJSON * const object, const char * const name, const char * const string);
      |                                                                                             ~~~~~~~~~~~~~~~~~~~^~~~~~

These warnings are serious because they indicate that there is no definition anywhere for functions like statusorder_ToString(). It's a regression caused by commit 34c3f8c7aa84 ("[C][Client] Fix enum function names not matching headers in the model template (#17512)") (#17512, @bookerdj). The petstore schema covers this case, but the tests still pass because the warnings don't break the build.

So I propose here:

1. Change the compiler flags so that implicit function declarations result in build failure. This will make it harder for similar regressions to happen again in the future.
2. Revert the patch that caused the regression. I haven't looked into it in depth and I appreciate that it was probably a bugfix for another serious issue, but I think regressions are more important, especially for things that are covered by the existing test schemas.

Of course I'll appreciate any feedback on the matter.

@wing328 @ityuhui @zhemant @michelealbano

[wing328]

Change the compiler flags so that implicit function declarations result in build failure. This will make it harder for similar regressions to happen again in the future.

Agreed. We definitely want to catch similar issues moving forward.

[wing328]

Revert the patch that caused the regression. I haven't looked into it in depth and I appreciate that it was probably a bugfix for another serious issue, but I think regressions are more important, especially for things that are covered by the existing test schemas.

To better test the output from C client generator, we can start using the Echo API server with the C Echo API client for better test coverage:

https://github.com/OpenAPITools/openapi-generator/wiki/Integration-Tests#echo-server

We're gradually migrating other generators to have Echo API client tests as well.

[eafer]

I've been looking into it some more, and there was really nothing wrong with the patch by @bookerdj, he just forgot to update one line. So instead of reverting the whole thing, this pull request now fixes that one line.

[eafer]

To be able to test @bookerdj's patch (so that nobody tries to revert it again like I did), I propose that we also add -Werror=missing-declarations to the compiler flags. With this change all functions must either be static or have a declaration in the headers.

[wing328]

So instead of reverting the whole thing, this pull request now fixes that one line.

thanks for fixing it in this PR 👍

I propose that we also add -Werror=missing-declarations to the compiler flags

sounds good to me

[wing328]

we can improve the CI tests to catch the warnings in another PR

let's get this merged to keep the momentum going

[eafer]

Thanks! The compiler flag changes are already included in this pull request, I guess I wasn't clear enough about that. Next pull request here: #20366.

[afblom]

Just dropping a comment to say great catch! Was experiencing this regression when using V7-10. Decided to get stuck into the inner workings of openApi C generator for version 7.10 and completely onboard with the new compiler flags. Rather more strict than not 👍

[eafer]

Thanks! You'll find that there are still many issues with the C generator, though.