Add test_annotationlib to v3.14.2 and fix related bugs#7001
Add test_annotationlib to v3.14.2 and fix related bugs#7001youknowone merged 2 commits intoRustPython:mainfrom
Conversation
|
Code has been automatically formatted The code in this PR has been formatted using:
git pull origin annotationlib |
📝 WalkthroughWalkthroughAdjusts interpolation source extraction to skip the opening brace using a TextSize-based offset (with fallback for dummy ranges), and preserves a leading whitespace after "{" when unparsing formatted expressions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 |
youknowone
force-pushed
the
annotationlib
branch
from
154fe45 to
f55ab22
Compare
youknowone
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/codegen/src/compile.rs`:
- Around line 8369-8379: The current logic uses after_brace =
interp.range.start() + TextSize::new(1) without verifying interp.range is valid,
which causes slices to drop the first character when interp.range is a dummy
(0..0); fix by first checking that interp.range actually encloses the expression
(e.g., interp.range.start() < interp.expression.range().start() and
interp.range.end() >= interp.expression.range().end()) before computing
after_brace and using self.source_file.slice(TextRange::new(after_brace,
expr_end)); if that containment check fails, fall back to slicing
interp.expression.range() as you already do in the else branch, ensuring
expr_source is only sliced with after_brace when the interpolation range is real
and encloses the expression.
🧹 Nitpick comments (1)
crates/codegen/src/unparse.rs (1)
564-576: Safe bounds checking, but consider defensive handling for edge cases.The bounds checking with
start > 0and.get(start - 1)is correct and prevents out-of-bounds access. The logic correctly identifies leading whitespace between{and the expression.However, this assumes
val.range()always represents a valid position in the source text. If the AST was constructed with synthetic/dummy ranges (as mentioned in compile.rs context), the check atstart - 1could inspect unrelated source text.Consider adding a sanity check that verifies the position is within the f-string context, or at minimum that the byte at
start - 1is either{or whitespace:🔧 Suggested defensive check
// Preserve leading whitespace between '{' and the expression let source_text = self.source.source_text(); let start = val.range().start().to_usize(); if start > 0 - && source_text + && start <= source_text.len() + && source_text .as_bytes() .get(start - 1) .is_some_and(|b| b.is_ascii_whitespace())
youknowone
force-pushed
the
annotationlib
branch
from
1933534 to
535638e
Compare
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] lib: cpython/Lib/annotationlib.py dependencies:
dependent tests: (43 tests)
[ ] test: cpython/Lib/test/test_list.py (TODO: 5) dependencies: dependent tests: (no tests depend on list) [x] lib: cpython/Lib/warnings.py dependencies:
dependent tests: (48 tests)
Legend:
|
1 participant
Summary by CodeRabbit