espressif / clang-tidy-runner

MIT License
9 stars 5 forks source link

Minimum IDF version? (RDT-208) #14

Closed lcrocker closed 2 years ago

lcrocker commented 2 years ago

I followed the instructions at

https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/tools/idf-clang-tidy.html

but I don't get the new idf.py commands at all. I don't have (or want) the clang toolchain, so I copied run-clang-tidy.py and clang-tidy-diff.py into /usr/local/bin so they're on the path. If I run "idf_clang_tidy ." manually, I just get thousands of errors in the libraries, e.g.:

/home/lee/.espressif/tools/xtensa-clang/12.0.1-d9341b81fc/xtensa-esp32-elf-clang/bin/clang-tidy -header-filter=.*\..* 
-checks=-*,clang-analyzer-core.NullDereference,clang-analyzer-unix.*,bugprone-*,-bugprone-macro-
parentheses,readability-*,performance-*,-readability-magic-numbers,-readability-avoid-const-params-in-decls
-p=/home/lee/work/fw_esp32_all/build /home/lee/esp/v4.4/components/esp_lcd/src/esp_lcd_panel_io.c
/home/lee/esp/v4.4/components/esp_common/include/esp_compiler.h:15:9: warning: declaration uses identifier
'__ESP_COMPILER_H', which is a reserved identifier [bugprone-reserved-identifier]
#define __ESP_COMPILER_H

I'm running Ubuntu 20.04 (soon to upgrade to 22.04), esp-idf release/v4.4.

Does this tool require 4.5? I haven't migrated by codebase yet, so if it does I won't be able to use it. Is there a command-line I can use with "idf_clang_tidy" that will work?

hfudev commented 2 years ago

Hi @lcrocker, the generated error here is the analyzed output from the clang-tidy.

idf_clang_tidy . is what we're using in our internal CI pipeline to run static code analysis on the whole codebase, which would generate thousands of errors.

From a user's perspective, could you try the command idf.py clang-check? This would analyze your current component only.

lcrocker commented 2 years ago

Thanks for the reply, but as I said in the original post, the "idf.py clang-check" command does not exist for me. It did not get added despite following the directions. If it had, I wouldn't be here.

hfudev commented 2 years ago

@lcrocker sorry for the ignorance. normally it doesn't require to reload the PATH via . export.sh again.

would you provide more info by providing the output of the following commands?

lcrocker commented 2 years ago
$ pip --version
pip 22.1 from /home/lee/.espressif/python_env/idf4.4_py3.8_env/lib/python3.8/site-packages/pip (python 3.8)    
$ which python
/home/lee/.espressif/python_env/idf4.4_py3.8_env/bin/python
$ python --version  
Python 3.8.10
$ pip list | grep clang 
pyclang               0.2.0
$ idf.py clang-check
Executing action: clang-check
ninja: error: unknown target 'clang-check'
command "clang-check" is not known to idf.py and is not a Ninja target
$ which run-clang-tidy.py
/usr/local/bin/run-clang-tidy.py
hfudev commented 2 years ago

@lcrocker Thank you for your report. Just check the code this feature is added on current master 5.0.

I'll make a backport commit back to the release v4.4 asap, it would take some time to release a new update.

If you want to use it now, here's a simple fix:

please add the following lines here: https://github.com/espressif/esp-idf/blob/release/v4.4/tools/idf.py#L684

    # Load component manager idf.py extensions if not explicitly disabled
    if os.getenv('IDF_COMPONENT_MANAGER') != '0':
        from idf_component_manager import idf_extensions
        extensions.append(('component_manager_ext', idf_extensions))

    # Optional load `pyclang` for additional clang-tidy related functionalities  # <-- new lines start from here
    try:
        from pyclang import idf_extension

        extensions.append(('idf_clang_tidy_ext', idf_extension))
    except ImportError:
        pass    # <-- end here

    for name, extension in extensions:
        try:
            all_actions = merge_action_lists(all_actions, extension.action_extensions(all_actions, project_dir))
        except AttributeError:
            print_warning('WARNING: Cannot load idf.py extension "%s"' % name)

I've tested locally, this should work.

lcrocker commented 2 years ago

Awesome, works well. Many thanks. I really wasn't ready to upgrade all my code yet (especially the bluetooth stuff--it's changed a lot).

hfudev commented 2 years ago

the backport fix is merged. https://github.com/espressif/esp-idf/blob/release/v4.4/tools/idf.py#L689. I'll close this issue.