[clang-tidy] RFC: use module name as a directory for organizing files

clang-tidy has lots of associated files that are at least one file per check (tests may include multiple files).

In the source code, files are organized by subdirectories according to their module, e.g. all the modernize checks are under clang-tools-extra/clang-tidy/modernize.

This is not the case for our documentation and test files, however.

  • clang-tools-extra/docs/clang-tidy/checks has 450 files
  • clang-tools-extra/test/clang-tidy/checkers has 573 files
  • clang-tools-extra/test/clang-tidy/checkers/Inputs has a directory per-check that uses extra inputs, but so far this is only 26 subdirectories, so it doesn’t feel cumbersome yet

Before preparing a commit that organizes these files into subdirectories by module, I want to take the pulse of other clang-tidy devs and see if they are OK with it.

2 Likes

Looks reasonable to me!

I think there’s two ways to read this, and I’d like to be sure we’re all on the same page. Are you thinking:

clang-tools-extra/docs/clang-tidy/checks/bugprone/
clang-tools-extra/docs/clang-tidy/checks/cert/
(etc for all the modules)
clang-tools-extra/test/clang-tidy/checkers/bugprone/
clang-tools-extra/test/clang-tidy/checkers/cert/
(etc for all the modules)

Or were you thinking more along the lines of?

clang-tools-extra/clang-tidy/cert/docs/
clang-tools-extra/clang-tidy/cert/tests/
clang-tools-extra/clang-tidy/bugprone/docs/
clang-tools-extra/clang-tidy/bugprone/tests/
(etc for all the modules)

CC @alexfh for code owner awareness.

The former, I am proposing new subdirs in existing dirs and existing files moving to the new subdirs.

OK, here’s an additional point of discussion…

While prototyping this for the documentation and modifying add_new_check.py to handle the subdirectory for each module, there arises the question of how best to organize the list of checks.

Currently we have one page with every single check listed on it, including all the aliases.

Do we want to preserve this as one giant list, or similarly break the list into a page per module and a list of checks in that module?

My preference is for the latter, but the script needs to be updated in either scenario, so feedback is welcome before going into a phabricator review.

Personally, I like the current approach of having one large page which lists all the checks. This allows me to use Ctrl+F in my browser to search for a keyword, across all the categories. E.g., when looking for checks which do something about unique_ptr, I can currently just search for unique in my browser…

Not sure if it’s the same for others, but I frequently struggle with predicting in which category to search for a certain clang-tidy rule. At least in my opinion, the categorization is sometimes a bit arbitrary or even counter-intuitive. E.g., as a person who started using unique_ptr only after 2014, I would have expected modernize-make-unique to be categorized as “bugprone” rather than “modernize”, because I originally didn’t even learn the old style of using the unique_ptr constructor in the first place. Instead I learned that the “use make_unique over the unique_ptr constructor” rule is there to avoid bugs, and would hence have expected it to be in the bug-prone category.

As such, making the categories a more prominent concept in the documentation, would make it harder (at least for me) to quickly find the checks I am looking for

Thanks for the clarification! I haven’t formed a strong opinion yet, but my initial thinking was that I actually like the latter from an organization standpoint (modules become more self-contained) but I think the former is likely to give the better developer experience (makes it easier to run all the tests, for example).

Perhaps we could split the middle: have one giant page, but reorganize the page to split by module? e.g., the top of the page is a TOC to the modules, each module gets its own heading + anchor (so you can jump from the TOC to a module) under which the table appears for that module’s checks. Then folks can still search one page to discover checks (like @avogelsgesang mentioned) but we get somewhat better organization rather than an overwhelming blob of checks right at the top of the page.

I’ll prototype with a single page, grouped by module. I think that’s a good idea.

Before putting this re-org up for review I’ve discovered some deficiencies in add_new_check.py along the way and I think fixing those first is a priority.

Here’s what I came up with: ⚙ D126495 [clang-tidy] Organize test docs into subdirectories by module (NFC)

This part organizes the test files per-module:
D128072 [clang-tidy] Organize test files into subdirectories by module (NFC)

For any interested readers, the changes have been pushed and tests and docs are now organized in per-module subdirectories.