Re-implement 'mccabe' to reduce supply chain risks and optimize check#10551
Draft
Pierre-Sassoulas wants to merge 16 commits intomainfrom
Draft
Re-implement 'mccabe' to reduce supply chain risks and optimize check#10551Pierre-Sassoulas wants to merge 16 commits intomainfrom
Pierre-Sassoulas wants to merge 16 commits intomainfrom
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (98.26%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #10551 +/- ##
==========================================
- Coverage 96.16% 96.16% -0.01%
==========================================
Files 177 177
Lines 19617 19725 +108
==========================================
+ Hits 18865 18968 +103
- Misses 752 757 +5
🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
a799a4f to
1863240
Compare
This comment has been minimized.
This comment has been minimized.
Pierre-Sassoulas
added a commit
that referenced
this pull request
Sep 21, 2025
## Type of Changes <!-- Leave the corresponding lines for the applicable type of change: --> | | Type | | --- | ---------------------- | | ✓ | 📜 Docs | ## Description Refs #10551
1863240 to
035e9e5
Compare
This comment has been minimized.
This comment has been minimized.
035e9e5 to
76731a8
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1a9446e to
56318b7
Compare
This comment has been minimized.
This comment has been minimized.
When a message changes slightly (e.g. McCabe rating 18→17, or a function moves a few lines), the primer used to show it as "message no longer emitted" + "new message emitted", which is misleading. Now, after the exact-match pass, residual messages are fuzzy-matched by (symbol, path, obj) identity and difflib similarity. Matched pairs are shown in a "messages have been changed" section with a unified diff of the changed fields only. Also fix a bug where the "no longer emitted" closing </details> tag was always emitted even when there were no missing messages (the condition checked the dict itself, which is always truthy).
The counter-based approach (complexity_score += 1) couldn't replicate the original mccabe library's graph formula (E - N + 2). This caused 188 functions to have different scores on astroid alone, almost all under-counted because the counter misses how paths merge at try/except blocks and other constructs. Replace with a proper control flow graph that tracks edge and node counts, then computes E - N + 2. Only 2 expected differences remain vs the original mccabe library: match/case (unsupported in mccabe v0.7.0) and template strings (Python 3.14), both improvements. Key design: no PathNode objects needed — just integer counters for E and N. Simple statements are skipped since collapsing linear chains doesn't change E - N + 2. All performance optimizations retained (dict dispatch, skip expressions, walk astroid directly).
The dispatch dict mapping node types to visitor methods is now created once at module level using unbound methods, instead of being rebuilt for each PathGraphingAstVisitor instance. Callers pass self explicitly. Eliminates ~18K dict allocations (12 entries each) per ansible run.
Speeds up attribute access for _num_nodes, _num_edges, _tail, _active and graphs by using C-level struct offsets instead of __dict__ lookups. These attributes are read/written millions of times during traversal.
Eliminates ~187K method calls per ansible run by inlining the subgraph parse logic directly into the two callers. For if/for/while (no except handlers), the extra_blocks loop is dead code and removed entirely.
Astroid nodes are hashable by identity, so we can use them directly as
dict keys. This eliminates ~107K id() builtin calls per ansible run
and simplifies the graphs dict from {int: (result, node)} to
{node: result}.
merge _visit_classdef/_visit_with, use _walk_body from checker - Store complexity as plain int instead of _ComplexityResult object, eliminating ~107K object allocations and method calls. - Cache self._walk_body as local variable in hot methods to avoid repeated bound-method creation via descriptor protocol. - Merge _visit_classdef and _visit_with into _visit_body since both just recurse into node.body. - Use visitor._walk_body(module.body) from the checker instead of a dispatch loop, saving ~18K dispatch calls per run.
…elling - Add top-level `is_toplevel` handling to `_visit_match` (was an early return, inconsistent with `_visit_subgraph` and `_visit_try`) - Add functional test cases for module-level try/except and match - Fix spelling issues in comments and docstrings flagged by spell checker
d589d93 to
b693390
Compare
Contributor
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.
Type of Changes
Description
Draft following #9667. Seems like a lot of optimization can be done by having the code directly in pylint. typing, and removing inheritance, and edge's name handling in particular. Because we're using a tool that permits to draw graph and we don't want to draw anything, we just want to get the damn mccabe metric.