-
Notifications
You must be signed in to change notification settings - Fork 0
Port to libcst #64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Port to libcst #64
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #64 +/- ##
==========================================
+ Coverage 95.15% 96.62% +1.46%
==========================================
Files 7 7
Lines 475 504 +29
==========================================
+ Hits 452 487 +35
+ Misses 23 17 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…e gone by the time we get there
… in a test_ast_ai_slop.py file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR replaces the standard AST (Abstract Syntax Tree) with libcst (Concrete Syntax Tree) to enable more sophisticated syntax tree traversal and better handling of chained method calls. The change allows for catching complex patterns like method chaining that were previously difficult or impossible to analyze.
Key changes:
- Migration from Python's built-in
astmodule tolibcstfor all syntax tree operations - Introduction of a
ChainSimplifiertransformer to handle method chaining scenarios - Enhanced argument extraction with better support for various data types and user namespace resolution
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/test_ast.py | Updated all test cases to use libcst, added comprehensive tests for chain simplification and chained function calls |
| src/access_py_telemetry/ast.py | Complete rewrite using libcst with new ChainSimplifier class and enhanced CallListener visitor |
| pyproject.toml | Added libcst dependency and updated mypy configuration |
| .pre-commit-config.yaml | Updated mypy arguments to accommodate new code patterns |
Comments suppressed due to low confidence (1)
tests/test_ast.py:399
- The expected value has changed from
["some_item"]to["'some_item'"](with quotes), but there's no explanation for this behavior change. This suggests the string handling in argument extraction may have changed semantics, which should be documented or tested explicitly.
("mock", "MyClass.__getitem__", ["'some_item'"], {}),
| @@ -1,10 +1,12 @@ | |||
| # mypy: disable-error-code=has-type | |||
Copilot
AI
Aug 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mypy disable comment uses 'has-type' but this should be 'type-arg' or another valid mypy error code. 'has-type' is not a recognized mypy error code.
| # mypy: disable-error-code=has-type |
Replace AST with libcst, should let us do some smarter syntax tree traversal in order to catch chained calls, etc properly