FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Use `ast::visitor::Visitor` for collection annotations by ShaharNaveh · Pull Request #8139 · RustPython/RustPython · GitHub

  1. RustPython
  2. RustPython

Repository navigation

Use ast::visitor::Visitor for collection annotations - #8139

Merged
youknowone merged 2 commits into
RustPython:mainfrom
ShaharNaveh:collect-annotations
Jun 22, 2026
ConversationCommits (2)ChecksFiles changed
Merged

Use ast::visitor::Visitor for collection annotations#8139
youknowone merged 2 commits into
RustPython:mainfrom
ShaharNaveh:collect-annotations

Conversation

ShaharNaveh commented Jun 22, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

Summary by CodeRabbit

  • Refactor
    • Improved annotation handling logic through code simplification and refactoring, enhancing maintainability while preserving all existing functionality.

coderabbitai Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 7237b8ca-dd70-477f-986d-3626666026c6

📥 Commits

Reviewing files that changed from the base of the PR and between 55b96f9 and ee45752.

📒 Files selected for processing (1)
  • crates/codegen/src/compile.rs
📝 Walkthrough

Walkthrough

collect_annotations in compile.rs is refactored to use a local AnnotationsVisitor implementing ast::visitor::Visitor instead of a manual recursive walk. Additionally, the simple-annotation presence check in compile_annotation_for_symbol_cursor_only is simplified from a count comparison to a boolean .any(...).

Changes

collect_annotations Visitor Refactor

Layer / File(s) Summary
Visitor-based collect_annotations and boolean annotation check
crates/codegen/src/compile.rs		 collect_annotations now defines a local AnnotationsVisitor that implements Visitor, collecting AnnAssign nodes while explicitly skipping descent into ClassDef and FunctionDef bodies. In compile_annotation_for_symbol_cursor_only, the "has simple annotation" detection changes from a match count (simple_annotation_count == 0) to a direct boolean via .any(...).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐇 Hoppity-hop through the AST tree,
No more manual walking for me!
A Visitor arrives, neat and bright,
Collecting AnnAssign with delight.
Count becomes bool — simpler, hooray!
The rabbit refactors and hops away. 🌿

🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly corresponds to the main refactoring in the changeset: replacing manual annotation collection logic with the ast::visitor::Visitor pattern approach.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches 🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

ShaharNaveh force-pushed the collect-annotations branch from 0966a2c to f61aebc Compare June 22, 2026 08:35
ShaharNaveh force-pushed the collect-annotations branch from f61aebc to ee45752 Compare June 22, 2026 08:36
ShaharNaveh marked this pull request as ready for review June 22, 2026 11:32
youknowone enabled auto-merge (squash) June 22, 2026 12:56
youknowone disabled auto-merge June 22, 2026 12:56
youknowone merged commit 83beb3e into RustPython:main Jun 22, 2026
26 checks passed
youknowone pushed a commit that referenced this pull request Sep 16, 2026
Use ast::visitor::Visitor for collection annotations (#8139)
a70a2bb
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL