Skip to content

Add a python script to test the code quality of ABACUS codes - #7843

Open
mohanchen wants to merge 19 commits into
deepmodeling:developfrom
mohanchen:2026-08-20
Open

Add a python script to test the code quality of ABACUS codes#7843
mohanchen wants to merge 19 commits into
deepmodeling:developfrom
mohanchen:2026-08-20

Conversation

@mohanchen

Copy link
Copy Markdown
Collaborator

Add a python script to test the code quality of ABACUS codes

abacus_fixer 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)
@mohanchen
mohanchen requested a review from Critsium-xy August 20, 2026 14:53
@mohanchen mohanchen added the Compile & CICD & Docs & Dependencies Issues related to compiling ABACUS label Aug 20, 2026
abacus_fixer and others added 16 commits August 21, 2026 07:49
… 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.
@mohanchen
mohanchen requested a review from AsTonyshment August 24, 2026 01:22
@mohanchen mohanchen added the Tests/Examples Issues/PR related to unit tests and integrate tests label Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Compile & CICD & Docs & Dependencies Issues related to compiling ABACUS Tests/Examples Issues/PR related to unit tests and integrate tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant