From d753850bcaa27f6363b307deaccbcb1ca1620a10 Mon Sep 17 00:00:00 2001 From: Jakob Gamper <97gamjak@gmail.com> Date: Sun, 27 Sep 2026 19:35:10 +0200 Subject: [PATCH 1/2] Rework documentation to have a better overview of the cpp ast checkers Fixes #129 --- CHANGELOG.md | 4 + docs/source/index.rst | 2 +- docs/source/sections/ast_checks/index.rst | 91 ++++++++++ .../sections/ast_checks/macro_replacement.rst | 60 +++++++ .../sections/ast_checks/no_global_using.rst | 58 +++++++ .../ast_checks/no_global_using_enum.rst | 54 ++++++ .../sections/ast_checks/no_throw_paren.rst | 29 ++++ .../ast_checks/param_name_for_type.rst | 66 ++++++++ docs/source/sections/configuration.rst | 159 +----------------- docs/source/sections/index.rst | 1 + docs/source/sections/overview.rst | 12 +- 11 files changed, 371 insertions(+), 165 deletions(-) create mode 100644 docs/source/sections/ast_checks/index.rst create mode 100644 docs/source/sections/ast_checks/macro_replacement.rst create mode 100644 docs/source/sections/ast_checks/no_global_using.rst create mode 100644 docs/source/sections/ast_checks/no_global_using_enum.rst create mode 100644 docs/source/sections/ast_checks/no_throw_paren.rst create mode 100644 docs/source/sections/ast_checks/param_name_for_type.rst diff --git a/CHANGELOG.md b/CHANGELOG.md index 9139ab0..7cc6dfb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ All notable changes to this project will be documented in this file. - Add `macroReplacement` AST check flagging invocations of a banned macro and suggesting its replacement, e.g. disallowing gtest's `EXPECT_THROW`/`ASSERT_THROW` in favor of custom `EXPECT_THROW_MSG`/`ASSERT_THROW_MSG` macros that also require a failure message. Ships with that mapping as a built-in default (no configuration required) and is fully configurable/extensible via `[cpp.ast_check_config.macroReplacement].macro_to_replacement`. Detection matches the macro name exactly and works whether the macro is defined in the same file or an included header +#### Documentation + +- Add a dedicated "AST-Based C++ Checks" section to the Sphinx docs with an overview table of every AST check and its own sidebar-linked page per check (`paramNameForType`, `macroReplacement`, `noGlobalUsing`, `noGlobalUsingEnum`, `noThrowParen`), each with flagged-code examples and its full configuration reference. The scattered/partial AST-check descriptions previously duplicated across the overview and configuration pages now point to these pages instead + ### Fixes #### CPP Rules diff --git a/docs/source/index.rst b/docs/source/index.rst index 35ba469..12e0ffb 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -6,7 +6,7 @@ A collection of DevOps related tools and scripts. Version |release| .. toctree:: - :maxdepth: 2 + :maxdepth: 3 :caption: User Guide sections/index diff --git a/docs/source/sections/ast_checks/index.rst b/docs/source/sections/ast_checks/index.rst new file mode 100644 index 0000000..4776a7e --- /dev/null +++ b/docs/source/sections/ast_checks/index.rst @@ -0,0 +1,91 @@ +AST-Based C++ Checks +===================== + +The text-based style checks (header guards, keyword order, license headers) +cannot express anything that depends on *meaning* — the type of a parameter, +whether a name resolves to ``std::``, what a macro expands to. For that, +``cpp_checks`` can optionally parse each file with libclang and run a set of +semantic checks over the resulting AST. + +Requirements & enabling +------------------------ + +AST checks need the optional ``ast`` extra (bundles ``libclang``): + +.. code-block:: console + + pip install devops[ast] + +They are on by default once installed. Toggle and tune them from the +``[cpp]`` section: + +.. list-table:: + :header-rows: 1 + :widths: 25 75 + + * - Key + - Purpose + * - ``ast_checks`` + - Master on/off switch (default ``true``). + * - ``ast_check_compile_commands_db`` + - Directory containing ``compile_commands.json``, for accurate per-file + compile flags. Strongly recommended for CMake projects. + * - ``ast_check_compile_args`` + - Fallback compiler flags (e.g. ``-std=c++23``) used when no compile + commands database is set, or a file isn't listed in it. + * - ``ast_check_enabled_ids`` / ``ast_check_disabled_ids`` + - Allowlist / denylist of check ids, by the ``Check id`` column below. + +See :doc:`../configuration` for the full key reference, defaults, and a +worked TOML example. + +How checks run +---------------- + +All enabled checks share a single preorder walk over each file's AST (see +``devops.cpp.ast.engine``) — adding another check never costs another parse. +Each one implements ``devops.cpp.ast.base.Check`` and reports zero or more +``Diagnostic``\ s, formatted like cppcheck output: + +.. code-block:: text + + src/box.cpp:42:18: style: parameter of type 'molsys::SimulationBox' named 'box' should be named 'simulationBox' [paramNameForType] + +The trailing ``[check-id]`` is what you put in ``ast_check_enabled_ids`` / +``ast_check_disabled_ids`` to select or silence that check. + +Available checks +------------------ + +.. list-table:: + :header-rows: 1 + :widths: 20 55 25 + + * - Check id + - Flags + - Configuration + * - :doc:`paramNameForType ` + - A parameter of a configured type not using its canonical name. + - Required (``type_to_name``) + * - :doc:`macroReplacement ` + - Use of a banned macro that has a required replacement. + - Optional (ships with a default mapping) + * - :doc:`noGlobalUsing ` + - ``using namespace X;`` / ``using X::Y;`` at global or namespace scope. + - Optional (name filters) + * - :doc:`noGlobalUsingEnum ` + - ``using enum X;`` at global or namespace scope. + - Optional (name filters) + * - :doc:`noThrowParen ` + - ``throw(...)`` wrapping the whole thrown expression. + - None + +.. toctree:: + :maxdepth: 1 + :hidden: + + param_name_for_type + macro_replacement + no_global_using + no_global_using_enum + no_throw_paren diff --git a/docs/source/sections/ast_checks/macro_replacement.rst b/docs/source/sections/ast_checks/macro_replacement.rst new file mode 100644 index 0000000..9f69bbf --- /dev/null +++ b/docs/source/sections/ast_checks/macro_replacement.rst @@ -0,0 +1,60 @@ +macroReplacement +================== + +Flags invocations of a banned macro and suggests the replacement macro that +should be used instead. Ships with a built-in default mapping and requires +no configuration to use: + +.. list-table:: + :header-rows: 1 + :widths: 30 30 + + * - Banned macro + - Use instead + * - ``EXPECT_THROW`` + - ``EXPECT_THROW_MSG`` + * - ``ASSERT_THROW`` + - ``ASSERT_THROW_MSG`` + +.. code-block:: cpp + + // Bad — flagged. + EXPECT_THROW(doSomething(), std::runtime_error); + + // Good. + EXPECT_THROW_MSG(doSomething(), std::runtime_error, "why it should throw"); + +Detection matches the macro name exactly (``EXPECT_THROW`` never matches +``EXPECT_THROW_MSG``) and works regardless of whether the macro is defined in +the same file or an included header (e.g. a gtest header). + +Configuration +-------------- + +.. list-table:: + :header-rows: 1 + :widths: 20 15 15 50 + + * - Key + - Type + - Default + - Description + * - ``macro_to_replacement`` + - table of string → string + - the built-in mapping above + - Replaces the built-in mapping entirely when set — extend it by + re-listing the defaults you want to keep alongside your own. + +.. code-block:: toml + + [cpp.ast_check_config.macroReplacement] + macro_to_replacement = { EXPECT_THROW = "EXPECT_THROW_MSG", + ASSERT_THROW = "ASSERT_THROW_MSG" } + +Disabling +---------- + +.. code-block:: toml + + [cpp] + ast_check_disabled_ids = ["macroReplacement"] diff --git a/docs/source/sections/ast_checks/no_global_using.rst b/docs/source/sections/ast_checks/no_global_using.rst new file mode 100644 index 0000000..d1cb0fa --- /dev/null +++ b/docs/source/sections/ast_checks/no_global_using.rst @@ -0,0 +1,58 @@ +noGlobalUsing +=============== + +Flags ``using namespace X;`` directives and ``using X::Y;`` declarations at +global or namespace scope. The same statements inside a function, lambda, or +class body are allowed, since their effect is confined to that scope. +``using enum`` is covered by the separate :doc:`no_global_using_enum` check. + +.. code-block:: cpp + + using namespace std; // Bad — flagged, at namespace scope. + + void run() { + using namespace std; // Good — confined to the function body. + } + +With no configuration, every such statement is reported; the keys below +narrow that down by name. Names are matched **exactly** against the +qualified name as written in the source (``std``, ``std::literals``, +``std::string``). A leading ``::`` on an entry is ignored. + +Configuration +-------------- + +.. list-table:: + :header-rows: 1 + :widths: 20 15 15 50 + + * - Key + - Type + - Default + - Description + * - ``enabled_names`` + - list of strings + - ``[]`` + - If non-empty, only using-statements whose name is in this list are + reported (allowlist). + * - ``disabled_names`` + - list of strings + - ``[]`` + - Using-statements whose name is in this list are never reported, + regardless of ``enabled_names``. + +.. code-block:: toml + + [cpp.ast_check_config.noGlobalUsing] + # Only complain about `using namespace std;` ... + enabled_names = ["std"] + # ... or, alternatively, complain about everything except these: + # disabled_names = ["std::literals", "std::chrono_literals"] + +Disabling +---------- + +.. code-block:: toml + + [cpp] + ast_check_disabled_ids = ["noGlobalUsing"] diff --git a/docs/source/sections/ast_checks/no_global_using_enum.rst b/docs/source/sections/ast_checks/no_global_using_enum.rst new file mode 100644 index 0000000..97927db --- /dev/null +++ b/docs/source/sections/ast_checks/no_global_using_enum.rst @@ -0,0 +1,54 @@ +noGlobalUsingEnum +=================== + +Flags ``using enum X;`` (C++20) at global or namespace scope, where it +injects every enumerator of ``X`` into the enclosing scope. Inside a function +or class body it is allowed. This is a separate check from +:doc:`no_global_using` so the two can be enabled independently. + +.. code-block:: cpp + + using enum Color; // Bad — flagged, at namespace scope. + + void run() { + using enum Color; // Good — confined to the function body. + } + +It accepts the same ``enabled_names`` / ``disabled_names`` keys as +``noGlobalUsing``, matched against the qualified enum name as written (e.g. +``"molsys::HybridZone"``); a leading ``::`` is ignored. + +Configuration +-------------- + +.. list-table:: + :header-rows: 1 + :widths: 20 15 15 50 + + * - Key + - Type + - Default + - Description + * - ``enabled_names`` + - list of strings + - ``[]`` + - If non-empty, only ``using enum`` declarations whose name is in this + list are reported (allowlist). + * - ``disabled_names`` + - list of strings + - ``[]`` + - ``using enum`` declarations whose name is in this list are never + reported, regardless of ``enabled_names``. + +.. code-block:: toml + + [cpp.ast_check_config.noGlobalUsingEnum] + disabled_names = ["molsys::LegacyZone"] + +Disabling +---------- + +.. code-block:: toml + + [cpp] + ast_check_disabled_ids = ["noGlobalUsingEnum"] diff --git a/docs/source/sections/ast_checks/no_throw_paren.rst b/docs/source/sections/ast_checks/no_throw_paren.rst new file mode 100644 index 0000000..fee7e07 --- /dev/null +++ b/docs/source/sections/ast_checks/no_throw_paren.rst @@ -0,0 +1,29 @@ +noThrowParen +============== + +Flags ``throw(...)`` where the parentheses wrap the *entire* thrown +expression, e.g. ``throw(x);`` or ``throw(SomeException(1));``. Write these +as ``throw x;`` / ``throw SomeException(1);`` instead. + +.. code-block:: cpp + + throw(SomeException(1)); // Bad — flagged, parens wrap the whole throw. + throw SomeException(1); // Good. + throw; // Good — a bare rethrow is always allowed. + +Parentheses that are only part of the thrown expression itself, such as a +constructor or function call (``throw SomeException(1);``), are not +affected — only parentheses spanning the whole expression are flagged. + +Configuration +-------------- + +This check takes no configuration. + +Disabling +---------- + +.. code-block:: toml + + [cpp] + ast_check_disabled_ids = ["noThrowParen"] diff --git a/docs/source/sections/ast_checks/param_name_for_type.rst b/docs/source/sections/ast_checks/param_name_for_type.rst new file mode 100644 index 0000000..83a712f --- /dev/null +++ b/docs/source/sections/ast_checks/param_name_for_type.rst @@ -0,0 +1,66 @@ +paramNameForType +================== + +Enforces that every parameter of a configured type uses one canonical name, +e.g. every ``molsys::SimulationBox`` parameter must be called ``simulationBox``. +Useful for keeping a consistent naming convention across a large codebase. + +.. code-block:: cpp + + // Bad — flagged: 'box' is not the configured name for SimulationBox. + void run(const molsys::SimulationBox &box); + + // Good. + void run(const molsys::SimulationBox &simulationBox); + +Type matching +-------------- + +Configured type names are matched after stripping cv-qualifiers, references, +and pointers, so ``"const SimulationBox &"``, ``"SimulationBox *"`` and +``"SimulationBox"`` all resolve to the same key and require the same +parameter name. A leading ``::`` on a key is also stripped automatically. + +Qualified keys (containing ``::``) use *suffix* matching, so +``"molsys::SimulationBox"`` also matches ``"std::molsys::SimulationBox"`` — a +known libclang/GCC quirk where some standard-library-adjacent contexts wrap +user namespaces. + +Configuration +-------------- + +.. list-table:: + :header-rows: 1 + :widths: 20 15 15 50 + + * - Key + - Type + - Default + - Description + * - ``type_to_name`` + - table (required) + - — + - Maps fully-qualified type names to the required parameter name, or a + list of accepted names. + * - ``unseen_type_is_error`` + - boolean + - ``false`` + - If ``true``, a configured type that is never encountered as a + parameter type across all checked files fails the run (instead of + just logging a warning). Useful for catching typos in the + ``type_to_name`` keys. + +.. code-block:: toml + + [cpp.ast_check_config.paramNameForType] + # Single accepted name: + type_to_name = { "molsys::SimulationBox" = "simulationBox" } + + # Multiple accepted names: + # type_to_name = { "molsys::SimulationBox" = ["simulationBox", "box"] } + + unseen_type_is_error = true + +Unlike the other AST checks, ``type_to_name`` is required — this check is a +no-op with an empty mapping, so it isn't meaningful to enable without +configuring it. diff --git a/docs/source/sections/configuration.rst b/docs/source/sections/configuration.rst index d6a1fde..a5601df 100644 --- a/docs/source/sections/configuration.rst +++ b/docs/source/sections/configuration.rst @@ -265,162 +265,9 @@ Controls the checks run by :ref:`cpp_checks `. Per-check configuration lives in sub-tables of ``[cpp.ast_check_config]``, one sub-table per check id. Each check defines its own keys; unknown keys are -silently ignored. - -.. rubric:: ``paramNameForType`` - -Enforces that parameters of specific types use a canonical name. Useful for -keeping a consistent naming convention across a large codebase (e.g. every -``SimulationBox`` parameter should be called ``simulationBox``). - -.. list-table:: - :header-rows: 1 - :widths: 20 15 15 50 - - * - Key - - Type - - Default - - Description - * - ``type_to_name`` - - table (required) - - — - - Maps fully-qualified type names to the required parameter name (or - a list of accepted names). Keys are matched after stripping - cv-qualifiers, references, and pointers, so ``"const Foo &"`` and - ``"Foo *"`` both resolve to ``"Foo"``. A leading ``::`` on a key - is also stripped automatically. Qualified keys (containing ``:``) - use suffix matching, so ``"molsys::Foo"`` also matches - ``"std::molsys::Foo"`` (a known libclang/GCC quirk). - * - ``unseen_type_is_error`` - - boolean - - ``false`` - - If ``true``, a configured type that was never encountered as a - parameter type across all checked files causes the run to fail - (instead of just logging a warning). Useful for catching typos in - the ``type_to_name`` keys. - -.. code-block:: toml - - [cpp.ast_check_config.paramNameForType] - # Single accepted name: - type_to_name = { "molsys::SimulationBox" = "simulationBox" } - - # Multiple accepted names: - # type_to_name = { "molsys::SimulationBox" = ["simulationBox", "box"] } - - unseen_type_is_error = true - -.. rubric:: ``noGlobalUsing`` - -Flags ``using namespace X;`` directives and ``using X::Y;`` declarations at -global or namespace scope. The same statements inside a function, lambda, or -class body are allowed. ``using enum`` is covered by the separate -``noGlobalUsingEnum`` check. With no configuration every such statement is -reported; the keys below narrow that down by name. - -Names are matched **exactly** against the qualified name as written in the -source (``std``, ``std::literals``, ``std::string``). A leading ``::`` on an -entry is ignored. - -.. list-table:: - :header-rows: 1 - :widths: 20 15 15 50 - - * - Key - - Type - - Default - - Description - * - ``enabled_names`` - - list of strings - - ``[]`` - - If non-empty, only using-statements whose name is in this list are - reported (allowlist). - * - ``disabled_names`` - - list of strings - - ``[]`` - - Using-statements whose name is in this list are never reported, - regardless of ``enabled_names``. - -.. code-block:: toml - - [cpp.ast_check_config.noGlobalUsing] - # Only complain about `using namespace std;` ... - enabled_names = ["std"] - # ... or, alternatively, complain about everything except these: - # disabled_names = ["std::literals", "std::chrono_literals"] - -.. rubric:: ``noGlobalUsingEnum`` - -Flags ``using enum X;`` (C++20) at global or namespace scope, where it injects -every enumerator of ``X`` into the enclosing scope. Inside a function or class -body it is allowed. It is a separate check from ``noGlobalUsing`` so the two -can be enabled independently, and it accepts the same ``enabled_names`` / -``disabled_names`` keys, matched against the qualified enum name as written -(e.g. ``"molsys::HybridZone"``). - -.. code-block:: toml - - [cpp.ast_check_config.noGlobalUsingEnum] - disabled_names = ["molsys::LegacyZone"] - -.. rubric:: ``noThrowParen`` - -Flags ``throw(...)`` where the parentheses wrap the *entire* thrown -expression, e.g. ``throw(x);`` or ``throw(SomeException(1));``. Write these as -``throw x;`` / ``throw SomeException(1);`` instead. Parentheses that are only -part of the thrown expression itself, such as a constructor or function call -(``throw SomeException(1);``), are not affected, and a bare rethrow -(``throw;``) is always allowed. This check takes no configuration; disable it -with:: - - [cpp] - ast_check_disabled_ids = ["noThrowParen"] - -.. rubric:: ``macroReplacement`` - -Flags invocations of a banned macro and suggests the replacement macro that -should be used instead. Ships with a built-in default mapping and requires no -configuration to use: - -.. list-table:: - :header-rows: 1 - :widths: 30 30 - - * - Banned macro - - Use instead - * - ``EXPECT_THROW`` - - ``EXPECT_THROW_MSG`` - * - ``ASSERT_THROW`` - - ``ASSERT_THROW_MSG`` - -Detection matches the macro name exactly (``EXPECT_THROW`` never matches -``EXPECT_THROW_MSG``) and works regardless of whether the macro is defined in -the same file or an included header (e.g. a gtest header). - -.. list-table:: - :header-rows: 1 - :widths: 20 15 15 50 - - * - Key - - Type - - Default - - Description - * - ``macro_to_replacement`` - - table of string → string - - the built-in mapping above - - Replaces the built-in mapping entirely when set — extend it by - re-listing the defaults you want to keep alongside your own. - -.. code-block:: toml - - [cpp.ast_check_config.macroReplacement] - macro_to_replacement = { EXPECT_THROW = "EXPECT_THROW_MSG", - ASSERT_THROW = "ASSERT_THROW_MSG" } - -To disable this check for a project set:: - - [cpp] - ast_check_disabled_ids = ["macroReplacement"] +silently ignored. See :doc:`ast_checks/index` for what each check flags and +its full configuration reference (``paramNameForType``, ``macroReplacement``, +``noGlobalUsing``, ``noGlobalUsingEnum``, ``noThrowParen``). ``[file]`` ^^^^^^^^^^ diff --git a/docs/source/sections/index.rst b/docs/source/sections/index.rst index 7b19b59..8e9cb6b 100644 --- a/docs/source/sections/index.rst +++ b/docs/source/sections/index.rst @@ -5,4 +5,5 @@ User Guide :maxdepth: 2 overview + ast_checks/index configuration diff --git a/docs/source/sections/overview.rst b/docs/source/sections/overview.rst index 73bc7e3..3450b22 100644 --- a/docs/source/sections/overview.rst +++ b/docs/source/sections/overview.rst @@ -27,14 +27,10 @@ and runs them against a project's C++ sources: contents of a configured license header file (see below). - **AST-based checks** (optional, requires ``pip install devops[ast]``) — uses libclang to parse each file and run semantic checks that text-based rules - cannot express: - - - ``paramNameForType`` — enforces that parameters of configured types use a - canonical name (e.g. every ``SimulationBox`` parameter must be called - ``simulationBox``). The type-to-name mapping is configured in - ``[cpp.ast_check_config.paramNameForType]``. Supports fully-qualified type - names, multiple accepted names per type, and per-file compile flags from a - ``compile_commands.json`` database. + cannot express, such as enforcing canonical parameter names for specific + types or banning particular macros. See + :doc:`AST-based C++ checks ` for the full list and how + each one is configured. Run it with :ref:`cpp_checks `. From 615e154a22bc11db304e0027e9315d0a745bf4cb Mon Sep 17 00:00:00 2001 From: Jakob Gamper <97gamjak@gmail.com> Date: Sun, 27 Sep 2026 19:49:28 +0200 Subject: [PATCH 2/2] Rework documentation to have a better overview of the cpp ast checkers Fixes #129 --- CHANGELOG.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c672622..cd615ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. ## Next Release +### Documentation + +- Add a dedicated "AST-Based C++ Checks" section to the Sphinx docs with an overview table of every AST check and its own sidebar-linked page per check (`paramNameForType`, `macroReplacement`, `noGlobalUsing`, `noGlobalUsingEnum`, `noThrowParen`), each with flagged-code examples and its full configuration reference. The scattered/partial AST-check descriptions previously duplicated across the overview and configuration pages now point to these pages instead + ## [0.5.0](https://github.com/repo/owner/releases/tag/0.5.0) - 2026-09-27 @@ -13,10 +17,6 @@ All notable changes to this project will be documented in this file. - Add `macroReplacement` AST check flagging invocations of a banned macro and suggesting its replacement, e.g. disallowing gtest's `EXPECT_THROW`/`ASSERT_THROW` in favor of custom `EXPECT_THROW_MSG`/`ASSERT_THROW_MSG` macros that also require a failure message. Ships with that mapping as a built-in default (no configuration required) and is fully configurable/extensible via `[cpp.ast_check_config.macroReplacement].macro_to_replacement`. Detection matches the macro name exactly and works whether the macro is defined in the same file or an included header -#### Documentation - -- Add a dedicated "AST-Based C++ Checks" section to the Sphinx docs with an overview table of every AST check and its own sidebar-linked page per check (`paramNameForType`, `macroReplacement`, `noGlobalUsing`, `noGlobalUsingEnum`, `noThrowParen`), each with flagged-code examples and its full configuration reference. The scattered/partial AST-check descriptions previously duplicated across the overview and configuration pages now point to these pages instead - ### Fixes #### CPP Rules