Class ClangTidyCmd disregards its `args` parameter

[akfreed]

I noticed this issue with ClangTidyCmd but it it is an issue for all the other similar concrete classes (CppcheckCmd, CpplintCmd, IncludeWhatYouUseCmd, etc.).

I am working on a cross-platform C++ repo that has windows/ and linux/ subfolders for implementations on a specific OS. I'm setting up linting. Clang-tidy, as expected, can only process the code for the current system (Linux / Windows). e.g. if I am on Linux and run your hooks with the command pre-commit run --all-files clang-tidy, it will try to parse the Windows files and error out.

Because pre-commit-config.yaml does not take variables, I created a shim script that filters out any files from the wrong folder. For example, if I am on Linux and pre-commit passes some file named foo/windows/bar.cpp in argv, the shim will create a new list without that file and call your hook.

.pre-commit-config.yaml:

repos:
  - repo: https://github.com/pocc/pre-commit-hooks
    rev: v1.3.5
    hooks:
      - id: clang-tidy
        entry: scripts/pocc-shim.py

scripts/pocc-shim.py:

def main() -> int:
    """Filter out filenames from other system implementations and call pocc's clang-tidy hook with the modified args."""

    if sys.platform == "win32":
        exclude = "linux"
    elif sys.platform == "linux":
        exclude = "windows"
    else:
        raise NotImplementedError(f"Not implemented for system: {sys.platform}")

    filtered_args = shim_utils.filter_args(exclude, sys.argv)
    return hooks.clang_tidy.main(filtered_args)

The issue is that this doesn't work. Clang-tidy is still failing on files shouldn't have been passed to it.

I had a look at utils.py and saw that Command.get_added_files is reading directly from sys.argv instead of the args variable passed in.

My current workaround is to just set sys.argv. scripts/pocc-shim.py:

filtered_args = shim_utils.filter_args(exclude, sys.argv)
sys.argv = filtered_args  # set sys.argv as workaround
return hooks.clang_tidy.main(filtered_args)

After this, the hook passes on both systems, properly ignoring files from the excluded folders.

I find this workaround distasteful, but it works fine and is acceptable for my purposes.

However, the underlying issue should still be addressed.

Repro

You can see my code here https://github.com/akfreed/CppSocketsXPlatform/commit/df8ac584207130e305de562e01e154407857e2ea. At the moment it is easier to reproduce on Linux. You will need clang-tidy version 13 (I installed with brew).

OS: Ubuntu 20 Toolchain: (All Ubuntu 20 apt default versions except clang-tidy)

git clone https://github.com/akfreed/CppSocketsXPlatform.git 
cd CppSocketsXPlatform
git checkout df8ac584207130e305de562e01e154407857e2ea
git submodule update --init --recursive
mkdir build && cd build
cmake ..
cd ..
python3 -m venv .venv
. .venv/bin/activate
pip install -U pip
pip install pre-commit
pre-commit run --all-files clang-tidy

Expected result

(.venv) freed@ubuntu20:~/cpp/repro$ pre-commit run --all-files clang-tidy
[INFO] Installing environment for https://github.com/pocc/pre-commit-hooks.                                     [INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
clang-tidy...............................................................Passed

Actual result

it fails on a file under the windows folder

(.venv) freed@ubuntu20:~/cpp/repro$ pre-commit run --all-files clang-tidy
clang-tidy...............................................................Failed
- hook id: clang-tidy                                                                                           - exit code: 1

/home/freed/cpp/repro/StrapperNet/lib/windows/SocketHandle.cpp:25:1: error: constructor does not initialize thes
e fields: m_socketId [cppcoreguidelines-pro-type-member-init,hicpp-member-init,-warnings-as-errors]
SocketHandle::SocketHandle() = default;                                                                         ^
...
...lots and lots of errors...
...

[akfreed]

Submitted a PR https://github.com/pocc/pre-commit-hooks/pull/47

[pocc]

Thank you so much for this detailed issue and PR. I should have time later this month.