Use clang-tidy in project

[teksturi]

It would be nice to get clang-tidy to use. It has lot of good rules which makes code base modern and more unified.

Releated items:

• 0 (or NULL) for null pointer shall be replaced with nullptr #4348
• Clang tidy showcase #4352
• enh(NumberFormatter): Introduce backward compatible options for formatHex functions #4333 (comment)

Task list

• Separate external source files to own folder

NOTE: Reorganisation moved to #4358.

It would be quite hard to exclude every external source without moving them to different folder. This same thing will happened also with other static analyzer tools, formatting tools, compiler options. So if we move them separated folders we can always exclude them more easily.

We can consider couple options here (example for zlib). Note that "external" can also be replaced with some other word if wanted.

1. Foundation/external/
2. Foundation/external/zlib/
3. Foundation/src/external/
4. Foundation/src/external/zlib/
5. Foundation/src/zlib
6. external/
7. external/zlib

My personal vote goes to 2, 4 or 7. Probably 7 because that way it so easy to seperete everything to one place. We can have example update script which checks all new version for those.

Still need to think what about external files which are also in include/ there we would probably need external folder. Example of this kind of file is "Foundation/include/Poco/ordered_map.h"

• Create .clang-tidy config file

This should probably have at least two different sections. One which have WarningsAsErrors true and one with false. This way we can separate what we always want to be clean. I belive there can be sections sepereted by "---"

• Add CMake targets tidy-fix and tidy-warn

  These will be excluded from target "all". Maybe also excluded if project is not root project. So external projects will not see this target.

• Add CI step

  CI step will run tidy-fix and tidy-warn separately. Even if other fails other step will be run so we catch all at once. Note that even tidy-fix will not fix those to PR. It will just show what could be automatically fixed.

• Take use chosen clang-tidy checks

See full list from https://clang.llvm.org/extra/clang-tidy/checks/list.html

I made public google sheet for different checks. It is not complete yet and it might never be but that is good start what we should start using. Note that it is currently my suggestion. Anyone has access right to it.

https://docs.google.com/spreadsheets/d/113ft-sPocWy9AK4uoPloZ9NXISxSWseizM65vj7dYLw/edit?usp=sharing

[teksturi]

Maybe something like this for external files.

I still need to prototype a little bit. We probably still do not want to build these as libs as before. Also we probably want to have nice include paths

#include "Poco/external/pdjson/pdjson.h"

We could also do some magic that we can always do that even when POCO_UNBUNDLED is used. But that is not very high priority but would be nice.

[matejk]

I propose to split the structure reorganisation of external files as an enhancement to a separate issue, because it can be done independently from using clang-tidy.

Regarding includes: AFAIK Poco uses the includes only internally and it does not export them for Poco users. My proposal for includes is that an include path is set for each bundled library in such a way that it can be used in the same way as for unbundled libraries from Poco sources.

[teksturi]

Structure reorganization is needed if we want to have clang-tidy automated in good way. Else we will clatter with exclude rules. That would be same work as moving files to different location. Moving them will also help in other stuff like taking clang-format in use.

Easiest reorganization would be this

Foundation/src/External/
Foundation/include/Poco/External/

This way almost nothing change but we can exclude those pretty easily.

[matejk]

I know that it is needed. However restructuring can be done independently of clang-tidy as a separate task. Then clang-tidy can be added on top.

[teksturi]

You are free to do seperete task for it and edit this one.