Clang-tidy breaks the code instead of fixing it

Advanced Solutions Static Code Analysis
1

I just started using platformio but I am facing many problems with clang-tidy and some with cppcheck.

When I run pio check I get the following errors:

Checking esp32-s3-devkitc-1 > clangtidy (board: esp32-s3-devkitc-1; platform: espressif32; framework: arduino)
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
src/App.h:6: [medium:warning] variable 'App' is non-const and globally accessible, consider making it const  [cppcoreguidelines-avoid-non-const-global-variables]
src/display/Display.cpp:4: [medium:warning] method 'create_display' can be made static  [readability-convert-member-functions-to-static]
src/display/Display.h:6: [medium:warning] variable 'Display' is non-const and globally accessible, consider making it const  [cppcoreguidelines-avoid-non-const-global-variables]
src/display/Display.h:6: [medium:warning] destructor of 'Display' is public and non-virtual  [cppcoreguidelines-virtual-class-destructor]
src/display/SerialDisplay.cpp:5: [medium:warning] method 'show_text' can be made static  [readability-convert-member-functions-to-static]
src/display/SerialDisplay.cpp:7: [medium:warning] variable 'buffer' is not initialized  [cppcoreguidelines-init-variables]
src/display/SerialDisplay.h:7: [medium:warning] variable 'SerialDisplay' is non-const and globally accessible, consider making it const  [cppcoreguidelines-avoid-non-const-global-variables]
src/display/SerialDisplay.h:7: [medium:warning] variable 'Display' is non-const and globally accessible, consider making it const  [cppcoreguidelines-avoid-non-const-global-variables]
src/main.cpp:7: [medium:warning] variable 'app' is non-const and globally accessible, consider making it const  [cppcoreguidelines-avoid-non-const-global-variables]
src/main.cpp:7: [medium:warning] variable 'app' provides global access to a non-const object; consider making the pointed-to data 'const'  [cppcoreguidelines-avoid-non-const-global-variables]
src/main.cpp:14: [medium:warning] assigning newly created 'gsl::owner<>' to non-owner 'App *'  [cppcoreguidelines-owning-memory]
================================================================================================================ [PASSED] Took 0.08 seconds ================================================================================================================
Checking esp32-s3-devkitc-1 > cppcheck (board: esp32-s3-devkitc-1; platform: espressif32; framework: arduino)
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
src/App.cpp:10: [low:style] The function 'update_text' is never used. [unusedFunction]
src/display/Display.cpp:4: [low:style] The function 'create_display' is never used. [unusedFunction]
src/display/SerialDisplay.cpp:5: [low:style] The function 'show_text' is never used. [unusedFunction]

In particular, these two get auto-fixed breaking the code:

src/display/Display.cpp:4: [medium:warning] method 'create_display' can be made static  [readability-convert-member-functions-to-static]
src/display/SerialDisplay.cpp:7: [medium:warning] variable 'buffer' is not initialized  [cppcoreguidelines-init-variables]

these two do not make sense to me, because those are classes definition not variables

src/display/SerialDisplay.h:7: [medium:warning] variable 'SerialDisplay' is non-const and globally accessible, consider making it const  [cppcoreguidelines-avoid-non-const-global-variables]
src/display/SerialDisplay.h:7: [medium:warning] variable 'Display' is non-const and globally accessible, consider making it const  [cppcoreguidelines-avoid-non-const-global-variables]

all the cppcheck unusedFunction also do not make sense, given that those methods are indeed used

Can someone help me understanding what is going on and how to fix these problems?

I had no problem with clang-tidy in non-embedded C++ projects in the past.

Here there is an example code to reproduce the issues:

2

Hi @simgunz, let me try to help here

In particular, these two get auto-fixed breaking the code:

These tools can help find issues in your code, but they should be complemented with manual reviews to ensure correct results and avoid false positives. This is the main reason why I’d not recommend automatic fixes when you’re checking code.

These two do not make sense to me, because those are classes definition not variables

That’s strange, I guess the problem here is that PlatformIO uses a bit outdated version of clang-tidy, probably they fixed this behavior in newer versions.

all the cppcheck unusedFunction also do not make sense, given that those methods are indeed used

That’s a long standing issue when analysis is done on the per-file basis, but the maintainer of the Cppcheck tools still hasn’t fixed it yet.