Update ruff to 0.15.19, use it from crates.io#8160
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates ruff workspace dependencies, adds ChangesRuff upgrade and ThinVec AST migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [ ] test: cpython/Lib/test/test_patma.py (TODO: 13) dependencies: dependent tests: (no tests depend on patma) [x] test: cpython/Lib/test/test_unpack.py dependencies: dependent tests: (no tests depend on unpack) [ ] test: cpython/Lib/test/test_syntax.py (TODO: 210) dependencies: dependent tests: (no tests depend on syntax) [ ] test: cpython/Lib/test/test_grammar.py (TODO: 11) dependencies: dependent tests: (no tests depend on grammar) [x] test: cpython/Lib/test/test_compile.py (TODO: 11) dependencies: dependent tests: (no tests depend on compile) Legend:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/codegen/src/compile.rs (1)
8271-8283: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract
keyonce and reuse it in this dict-comp path.
key.as_ref().expect(...)is repeated several times in the same flow. Bind it once and reuse it for compile/await/range operations.Proposed refactor
+ let key = key + .as_ref() + .expect("RustPython does not support PEP798 yet"); generators, &|compiler, collection_add_i| { // changed evaluation order for Py38 named expression PEP 572 - compiler.compile_expression( - &key.as_ref() - .expect("RustPython does not support PEP798 yet"), - )?; + compiler.compile_expression(key)?; compiler.compile_expression(value)?; compiler.set_source_range(TextRange::new( - key.as_ref() - .expect("RustPython does not support PEP798 yet") - .range() - .start(), + key.range().start(), value.range().end(), )); @@ - Self::contains_await( - &key.as_ref() - .expect("RustPython does not support PEP798 yet"), - ) || Self::contains_await(value) + Self::contains_await(key) || Self::contains_await(value) || Self::generators_contain_await(generators), *range, TextRange::new( - key.as_ref() - .expect("RustPython does not support PEP798 yet") - .range() - .start(), + key.range().start(), value.range().end(), ), - key.as_ref() - .expect("RustPython does not support PEP798 yet") - .range(), + key.range(), )?;As per coding guidelines, when logic is shared, extract the differing value first and call the common logic once to avoid duplication.
Also applies to: 8294-8309
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/codegen/src/compile.rs` around lines 8271 - 8283, The dict-comp handling in compile.rs repeats key.as_ref().expect(...) multiple times; extract that value once in the affected flow and reuse it for compile_expression, any await handling, and source-range calculation. Update the dict-comp path around the existing compiler.compile_expression and compiler.set_source_range calls, and apply the same refactor to the other referenced block so the shared logic uses a single bound key value instead of duplicated expectations.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/codegen/src/unparse.rs`:
- Around line 256-260: The call in unparse_expr for the key handling path has a
redundant leading borrow that triggers a needless_borrow warning. Update the
expression in unparse.rs around the self.unparse_expr call so it passes the
reference returned by key.as_ref().expect("RustPython does not support PEP798
yet") directly, without adding an extra &.
---
Nitpick comments:
In `@crates/codegen/src/compile.rs`:
- Around line 8271-8283: The dict-comp handling in compile.rs repeats
key.as_ref().expect(...) multiple times; extract that value once in the affected
flow and reuse it for compile_expression, any await handling, and source-range
calculation. Update the dict-comp path around the existing
compiler.compile_expression and compiler.set_source_range calls, and apply the
same refactor to the other referenced block so the shared logic uses a single
bound key value instead of duplicated expectations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: e4cf7347-2742-4018-9f90-8b6b99d09026
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockLib/test/test_grammar.pyis excluded by!Lib/**Lib/test/test_syntax.pyis excluded by!Lib/**
📒 Files selected for processing (16)
.github/dependabot.ymlCargo.tomlcrates/codegen/src/compile.rscrates/codegen/src/symboltable.rscrates/codegen/src/unparse.rscrates/vm/Cargo.tomlcrates/vm/src/stdlib/_ast.rscrates/vm/src/stdlib/_ast/argument.rscrates/vm/src/stdlib/_ast/constant.rscrates/vm/src/stdlib/_ast/elif_else_clause.rscrates/vm/src/stdlib/_ast/module.rscrates/vm/src/stdlib/_ast/node.rscrates/vm/src/stdlib/_ast/parameter.rscrates/vm/src/stdlib/_ast/pattern.rscrates/vm/src/stdlib/_ast/string.rscrates/vm/src/stdlib/_ast/validate.rs
c179e29 to
2117417
Compare
2117417 to
6b8f4d0
Compare
Summary
astral-sh/ruff#26271
Summary by CodeRabbit
ThinVec.ThinVecsupport to the AST conversion layer.thin-vecworkspace dependency.