Add a python script to test the code quality of ABACUS codes - #7843
Open
mohanchen wants to merge 6 commits into
Open
Add a python script to test the code quality of ABACUS codes#7843mohanchen wants to merge 6 commits into
mohanchen wants to merge 6 commits into
Conversation
added 3 commits
August 20, 2026 22:27
Extends tools/03_code_analysis/code_quality_score.py with two changes:
- New `high_cyclomatic_complexity` rule: counts if/for/while/switch/case/
&&/|| per function body (McCabe complexity). Threshold 10, -1 per extra
point, capped at 30 per file. Identifies functions that should be split.
- `file_too_long` weight raised from -1 to -2 per 50-line block beyond 500
lines, reflecting the higher maintenance cost of very large files.
Implementation:
- `find_function_bodies()` locates function definitions with `{...}` bodies,
reusing the prefix/reject logic from find_long_function_signatures so that
function calls, lambdas, macros, and function-pointer typedefs are
excluded.
- `find_high_complexity_functions()` walks each body and counts control-flow
keywords via CYCLO_KEYWORDS_RE.
- Cyclomatic complexity follows McCabe: `else if` counts as two `if`,
`switch` + each `case` count separately, `&&`/`||` each add 1.
Scan results on source/ (1652 files, excluding test/ dirs):
- Average score: 79.1 (was 82.1)
- Passing rate (>=60): 1355/1652 = 82.1%
- high_cyclomatic_complexity triggered: 594 functions
- file_too_long triggered: 167 files
Top offenders identified by the new rule:
- source_hamilt/module_xc/xc_grad.cpp:28 `gradcorr` (complexity 145)
- source_lcao/force_stress_lcao.cpp:69 `getForceStress` (103)
- source_lcao/module_deepks/lcao_deepks_iface.cpp:63 `out_deepks_labels` (94)
- source_io/module_ctrl/ctrl_scf_lcao.cpp:82 `ctrl_scf_lcao` (68)
- source_estate/module_charge/charge.cpp:245 `atomic_rho` (60)
… matches
This commit extends tools/03_code_analysis/code_quality_score.py with one
new scoring rule, expands the test-file exclusion list, and fixes a
critical class of false positives for keyword-based rules.
1. New rule: post_cpp11_feature (-80, one-shot per file)
The ABACUS project keeps a C++11 baseline (see AGENTS.md § Required
Baseline rule 7). Any newer syntax is a compilation risk on older
compilers, so a one-shot -80 deduction is applied when any of the
following high-confidence, low-false-positive patterns is seen:
C++14 std::make_unique<T>(...)
digit separator in numeric literals (1'000'000)
C++17 if constexpr (...)
structured binding auto [a, b] = ...;
fold expressions (args + ...), (... + args), etc.
std::optional<T>, std::variant<T,U>, std::any
[[nodiscard]], [[maybe_unused]] attributes
C++20 concept / requires / consteval / constinit
coroutine keywords: co_await, co_yield, co_return
std::span<T>, std::ranges::*, std::format(...)
C++23 std::expected<T,E>, std::print(...), std::println(...)
Detection uses a list of (label, compiled_regex) pairs defined in
POST_CPP11_PATTERNS. A single Finding is emitted per file listing all
distinct features and their line numbers so the report is actionable.
2. Test directory exclusion: add "test_serial" to SKIP_DIRS
The exclusion set previously contained {test, tests, test_parallel,
unit_test, unittest} but missed test_serial/ under source_io and
source_base; nine files leaked into score summaries. Now skipped.
3. False-positive fix: introduce strip_strings() helper
strip_comments() erases comments but preserves string literals on
purpose (brace-matching parsers later rely on the real quote
boundaries). That meant keyword-based rules (e.g. the C++20 requires
regex) matched ordinary words inside user-facing strings such as
WARNING_QUIT("... eigensolver requires replicated ...").
The new strip_strings() function walks through content character by
character, tracks "... " and '...' modes, and replaces every
character inside quotes with a space (newlines are preserved so line
numbers stay correct). find_post_cpp11_features() now runs on
strip_strings(strip_comments(content)) — the double pass eliminates
string-literal false matches while still catching real keywords.
4. Results on source/ (1643 files, excluding test dirs):
- Avg score: 79.1 (previous scan w/ buggy version: 78.6)
- Pass rate (>=60): 1348/1643 = 82.0%
- post_cpp11_feature triggered on exactly 1 file after the fix:
source/source_hsolver/diago_pexsi.cpp -> std::make_unique (C++14)
The previous 16 files flagged as "requires (C++20)" were all
string-literal false matches and are now correctly cleared.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a python script to test the code quality of ABACUS codes