Enable clang-tidy checks and fix warnings in code

elfenpiff commented 2 years ago

Brief feature description

At the moment a lot of clang-tidy checks in our root .clang-tidy file are disabled to ensure that our codebase is at least warning free with a minimal subset of clang-tidy checks.

We should enable the disabled checks one by one and solve the generated warnings in the same step. Some of the warnings require a larger refactoring. If this happens please create another issue and add a todo link in this issue.

ToDo

Remove the compiler warning suppression from all tests, see CMakeLists.txt and the entry set(TEST_CXX_FLAGS PRIVATE ${ICEORYX_WARNINGS} ${ICEORYX_SANITIZER_FLAGS} -Wno-pedantic -Wno-conversion) and fix all warnings.
Create a .clang-tidy file in every test disables warning which originate from the googletest fixtures itself. For instance TEST_F and TYPED_TEST have a rule of 5 warning.

Hints

The .clang-tidy file may have warnings which do not fit for us. If this is the case then please disable the warning and write a justification why this is rule is not appropriate. Example:

* rule: readability-identifier-length
  justification: In some functions single letter variables make sense, this is covered by the review process
  example:
     bad:
       int sum(int summand1, int summand2); // here we would like to have single letter variable arguments
     good:
       int sum(int a, int b); // failure since a and b have less than three letters

CI checks

iceoryx_hoofs diff checks
- [x] cxx @elfenpiff
- [x] concurrent @elfenpiff
- [x] design_pattern @elfenpiff
- [x] posix_wrapper @elfenpiff
- [x] units @elfenpiff
[x] iceoryx_hoofs full nightly scan based on .clang-tidy-diff-scans.txt @elfenpiff

Files

When a file is stated, it means the header, inline file, cpp file and also the tests are refined.

iceoryx_hoofs/cxx

[x] adaptive_wait @elfenpiff
[x] algorithm @elfenpiff
[x] attributes.hpp @elfenpiff (muahahha there were no warnings but it still counts!)
[x] convert @elfenpiff
[x] deadline_timer @elfenpiff
[x] expected @elfenpiff
[x] file_reader @FerdinandSpitzschnueffler
[x] filesystem @elfenpiff
[x] forward_list @FerdinandSpitzschnueffler
[x] function_ref @mossmaurice
[x] function @elfenpiff
[x] functional_interface @elfenpiff
[x] generic_raii @elfenpiff
[x] helplets @elfenpiff
[x] list @elfenpiff
[x] newtype @elBoberido
[x] optional @FerdinandSpitzschnueffler
[x] pair @elfenpiff
[x] poor_mans_heap @elBoberido
[x] reference_counter @elfenpiff
[x] requires @elfenpiff
[x] scoped_static @elfenpiff
[x] serialization @elBoberido
[x] set @elfenpiff
[x] static_storage @elfenpiff
[x] stack @elBoberido
[x] storable_function @elfenpiff
[x] string @FerdinandSpitzschnueffler
[x] types @elBoberido
[x] type_traits @elfenpiff
[x] unique_id @elfenpiff
[x] unique_ptr @mossmaurice
[x] variant @mossmaurice
[x] variant_queue @FerdinandSpitzschnueffler
[x] vector @mossmaurice

iceoryx_hoofs/units

[x] duration @elBoberido

iceoryx_hoofs/concurrent

[x] lockfree_queue complete folder + header @MatthiasKillat
[x] ~~active_object (ignore because it's obsolete?)~~
[x] fifo @FerdinandSpitzschnueffler
[x] loffli @FerdinandSpitzschnueffler
[x] periodic_task @FerdinandSpitzschnueffler
[x] smart_lock @FerdinandSpitzschnueffler
[x] sofi @FerdinandSpitzschnueffler
[x] taco @FerdinandSpitzschnueffler
[x] trigger_queue (no warnings to fix)

iceoryx_hoofs/rp

[x] PointerRepository @mossmaurice
[x] RelativePointer @mossmaurice
[x] BaseRelativePointer @mossmaurice
[x] RelativePointerData @mossmaurice
[x] relocatable_ptr @MatthiasKillat

posix_wrapper

[x] shared_memory_object - complete folder @MatthiasKillat
[x] access_control @elfenpiff
[x] file_lock @elfenpiff
[x] ipc_channel @elfenpiff
[x] message_queue @elfenpiff
[x] mutex @elfenpiff
[x] named_pipe @elfenpiff
[x] named_semaphore @elfenpiff
[x] posix_access_rights @elfenpiff
[x] posix_call @elfenpiff
[x] semaphore_interface @elfenpiff
[x] signal_handler @elfenpiff
[x] signal_watcher @elfenpiff
[x] system_configuration @FerdinandSpitzschnueffler
[x] thread @FerdinandSpitzschnueffler
[x] types @FerdinandSpitzschnueffler
[x] unix_domain_socket @FerdinandSpitzschnueffler
[x] unnamed_semaphore @elfenpiff

TODOs after merge of the integration branch

[x] Setup CI job to run a space check // NOLINTNEXTLINE (
[x] Tracked here: #1611 improve readability of brance initialization by adding spaces between variable name, braces and values;
```
// old
int foo{42};
// new
int foo { 42 };
```
[x] refactor operator<< in filesystem.cpp to reduce code complexity
[x] Tracked here: #1612 remove operator std::string() from cxx::string and implement toStdString method
[x] Tracked here: #1613 remove EXPECT_DEATH
[x] Tracked here: #1614 implement uninitialized array
[x] Tracked as todo in: #1032 replace requires.hpp with error handler
[x] Tracked as todo in: #260 refactor cxx::convert to handle all cxx::string capacities
[x] Tracked here: #1615 remove suppressions in code base for rules that are disabled in clang-tidy file
[x] Tracked here: #1616 strerror_s handling. Is only added to windows but must be added to platform libc layer
[x] Tracked here: #1617 Seconds_t and Nanoseconds_t in duration should use Newtype pattern to parameter mixup

elBoberido commented 2 years ago

@elfenpiff convert is not done yet. clang-tidy does not report warnings because the hpp is not included in the inl. I uncheck the checkbox

elBoberido commented 2 years ago

@elfenpiff I would exclude log and error handler from clang-tidy checks. These will be refactored anyway and there is no need to waste time on it right now.

eclipse-iceoryx / iceoryx