Replace `compiler::source` module with `ruff_source_file` #6016

ShaharNaveh · 2025-07-21T21:45:32Z

I saw that compiler/source was mainly re-exporting items from ruff_source_file, so I replaced it with the upstream ruff_source_file::SourceFile which is a bit better implementation of what we have currently as it's uses Arc internally so it's practically free to clone & replaces the need for a dedicated SourceCodeOwned as SourceFile is already owned.

This PR contains a lot of noise as I renamed:

source_code -> source_file
SourceCode -> SourceFile
SourceCodeOwned -> SourceFile

Feel free to close this PR if this is an unwanted change.

Summary by CodeRabbit

Refactor
- Unified the handling of source code representation across the compiler and VM by replacing the previous type with a new, consistent type throughout all relevant modules and interfaces.
- Updated numerous method and function signatures to accept the new source type, improving maintainability and consistency.
- Deprecated and removed references to legacy source handling components.
- Adjusted dependencies to reflect the new approach and removed obsolete packages.
Chores
- Reformatted configuration files and cleaned up commented-out or unused dependencies.

coderabbitai · 2025-07-21T21:45:39Z

Walkthrough

This change removes the dependency on the deprecated rustpython-compiler-source crate throughout the codebase, replacing all usages of its SourceCode and related types with SourceFile from ruff_source_file. All function signatures, struct fields, and trait implementations that previously accepted or returned SourceCode are updated to use SourceFile. Corresponding dependency and feature declarations in Cargo.toml files are also updated, and the deprecated crate is marked as such.

Changes

File(s) / Path(s)	Change Summary
Cargo.toml, compiler/Cargo.toml, compiler/codegen/Cargo.toml,	Removed `rustpython-compiler-source` from workspace and dependencies; reformatted workspace members; updated lints.
compiler/source/Cargo.toml	Marked `rustpython-compiler-source` as deprecated and set explicit version.
vm/Cargo.toml	Removed `rustpython-compiler-source` dependency; made `ruff_source_file` mandatory; updated feature declarations.
compiler/codegen/src/compile.rs, compiler/codegen/src/symboltable.rs, compiler/codegen/src/unparse.rs	Replaced all uses of `SourceCode` with `SourceFile`; updated all related function/struct signatures and logic.
compiler/src/lib.rs	Replaced `SourceCode` with `SourceFile` in imports, function signatures, and internal logic.
vm/src/compiler.rs, vm/src/lib.rs	Removed re-exports of `rustpython_compiler_source` as `source`.
vm/src/builtins/code.rs, vm/src/builtins/traceback.rs, vm/src/frame.rs, vm/src/vm/vm_new.rs	Switched imports and usages from `LineNumber`/`SourceLocation` to `OneIndexed`/`SourceLocation` from `ruff_source_file`.
vm/src/stdlib/ast/*.rs (all submodules)	Systematically replaced `SourceCodeOwned` with `SourceFile` in all trait impls, function signatures, and calls.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant VM
    participant Compiler
    participant RuffSourceFile

    User->>VM: Provide source string and path
    VM->>RuffSourceFile: Build SourceFile from string/path
    VM->>Compiler: Call compile/parse with SourceFile
    Compiler->>RuffSourceFile: Query source locations, slices, etc.
    Compiler-->>VM: Return result (AST, code object, etc.)
    VM-->>User: Return result or error

Estimated code review effort

3 (~45 minutes)

Suggested reviewers

youknowone

Poem

A hop and a skip, we’ve shed the old code,
From SourceCode to SourceFile, down a new road.
No more the compiler-source, now deprecated and gone,
RuffSourceFile leads us, as we leap on.
With signatures tidy and dependencies light,
This bunny’s refactor feels just right! 🐇✨

📜 Recent review details

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af382dd and 776e57e.

📒 Files selected for processing (1)

compiler/source/Cargo.toml (2 hunks)

✅ Files skipped from review due to trivial changes (1)

compiler/source/Cargo.toml

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

GitHub Check: Ensure compilation on various targets
GitHub Check: Run rust tests (windows-latest)
GitHub Check: Run rust tests (ubuntu-latest)
GitHub Check: Run snippets and cpython tests (windows-latest)
GitHub Check: Run rust tests (macos-latest)
GitHub Check: Run snippets and cpython tests on wasm-wasi
GitHub Check: Check Rust code with rustfmt and clippy
GitHub Check: Check the WASM package and demo
GitHub Check: Run snippets and cpython tests (macos-latest)
GitHub Check: Run snippets and cpython tests (ubuntu-latest)

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

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 1

🔭 Outside diff range comments (1)

compiler/codegen/src/compile.rs (1)
340-361: Duplicate flags field – this will not compile

ir::CodeInfo already receives a flags value at line 341, but the same field is initialised again at line 360.
Rust does not allow duplicate struct fields; this is a hard‐error and will break the build.
-        stacksize: 0,
-        flags: bytecode::CodeFlags::empty(),
+        stacksize: 0,
Remove the second occurrence (or rename it if you intended a different field).

🧹 Nitpick comments (7)

vm/src/vm/vm_new.rs (1)
289-290: Stale “TODO” comment—should reference SourceFile, not SourceCode

Now that the file imports ruff_source_file::SourceLocation, the comment a few lines below (// TODO: replace to SourceCode) is misleading. Recommend updating or deleting it to avoid confusion.
-        // TODO: replace to SourceCode
+        // TODO: revisit: `SourceFile` and `SourceLocation` now come from `ruff_source_file`
Cargo.toml (1)

117-135: Consider keeping workspace member list alphabetised

The reordered member list still contains "." alongside crate paths, but the sequence is no longer alphabetical ("compiler" precedes "."). Sorting keeps large workspace manifests easier to scan and merge-conflict-free.
No functional impact—purely a readability / maintenance nit.
vm/src/builtins/traceback.rs (1)
55-58: Minor API ergonomics: consider exposing tb_lineno as usize and OneIndexed

tb_lineno() returns the raw usize, which mirrors CPython, but any Rust-side consumer that already has a OneIndexed will need to convert back.
Optionally expose a second getter:
+#[pygetset(name = "tb_lineno_oneindexed")]
+const fn tb_lineno_oneindexed(&self) -> OneIndexed {
+    self.lineno
+}
Purely ergonomic – feel free to skip.

Also applies to: 74-78
compiler/codegen/src/compile.rs (4)
240-248: Avoid repeated to_source_code() conversions

Calling self.source_file.to_source_code() twice per location calculation is wasteful and clutters the code.
Store the SourceCode once (or call the location helpers directly on SourceFile if available).
-    let start = zelf
-        .source_file
-        .to_source_code()
-        .source_location(zelf.current_source_range.start());
+    let sc = zelf.source_file.to_source_code();
+    let start = sc.source_location(zelf.current_source_range.start());
5318-5322: Minor: cache SourceCode for line look-ups

get_source_line_number re-creates a SourceCode wrapper on every call.
When walking large ASTs this shows up in profiles. Consider caching or adding a thin helper on SourceFile.

5224-5231: Hot path alloc – consider re-using SourceLocation

_emit allocates a SourceLocation for every bytecode instruction.
For large inputs this dominates compile-time. You can:

Compute the location once per line and reuse.

Delay the translation until after IR generation.

Not blocking, but worth a TODO.

5848-5855: SourceFileBuilder::new("source_path", …) leaks dummy path into error messages

Tests hard-code "source_path" which shows up in tracebacks and diagnostics.
Consider passing the real test-file name (or at least a sentinel like "<test>") to avoid confusion downstream.

📜 Review details

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 57bdf35 and af382dd.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (33)

Cargo.toml (1 hunks)
compiler/Cargo.toml (1 hunks)
compiler/codegen/Cargo.toml (0 hunks)
compiler/codegen/src/compile.rs (22 hunks)
compiler/codegen/src/symboltable.rs (8 hunks)
compiler/codegen/src/unparse.rs (4 hunks)
compiler/source/Cargo.toml (0 hunks)
compiler/source/src/lib.rs (0 hunks)
compiler/src/lib.rs (5 hunks)
vm/Cargo.toml (2 hunks)
vm/src/builtins/code.rs (2 hunks)
vm/src/builtins/traceback.rs (4 hunks)
vm/src/compiler.rs (1 hunks)
vm/src/frame.rs (1 hunks)
vm/src/lib.rs (0 hunks)
vm/src/stdlib/ast.rs (9 hunks)
vm/src/stdlib/ast/argument.rs (2 hunks)
vm/src/stdlib/ast/basic.rs (3 hunks)
vm/src/stdlib/ast/constant.rs (9 hunks)
vm/src/stdlib/ast/elif_else_clause.rs (1 hunks)
vm/src/stdlib/ast/exception.rs (3 hunks)
vm/src/stdlib/ast/expression.rs (23 hunks)
vm/src/stdlib/ast/module.rs (8 hunks)
vm/src/stdlib/ast/node.rs (4 hunks)
vm/src/stdlib/ast/operator.rs (8 hunks)
vm/src/stdlib/ast/other.rs (6 hunks)
vm/src/stdlib/ast/parameter.rs (9 hunks)
vm/src/stdlib/ast/pattern.rs (16 hunks)
vm/src/stdlib/ast/statement.rs (27 hunks)
vm/src/stdlib/ast/string.rs (5 hunks)
vm/src/stdlib/ast/type_ignore.rs (4 hunks)
vm/src/stdlib/ast/type_parameters.rs (6 hunks)
vm/src/vm/vm_new.rs (1 hunks)

💤 Files with no reviewable changes (4)

compiler/codegen/Cargo.toml
compiler/source/Cargo.toml
vm/src/lib.rs
compiler/source/src/lib.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style (cargo fmt to format)
Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

vm/src/vm/vm_new.rs
vm/src/frame.rs
vm/src/compiler.rs
vm/src/builtins/code.rs
vm/src/stdlib/ast/operator.rs
vm/src/builtins/traceback.rs
vm/src/stdlib/ast/node.rs
vm/src/stdlib/ast/module.rs
vm/src/stdlib/ast/argument.rs
vm/src/stdlib/ast/type_ignore.rs
compiler/src/lib.rs
vm/src/stdlib/ast/basic.rs
vm/src/stdlib/ast/string.rs
vm/src/stdlib/ast/parameter.rs
vm/src/stdlib/ast/elif_else_clause.rs
compiler/codegen/src/symboltable.rs
compiler/codegen/src/unparse.rs
vm/src/stdlib/ast/statement.rs
vm/src/stdlib/ast/type_parameters.rs
vm/src/stdlib/ast/other.rs
vm/src/stdlib/ast.rs
vm/src/stdlib/ast/exception.rs
vm/src/stdlib/ast/constant.rs
vm/src/stdlib/ast/expression.rs
compiler/codegen/src/compile.rs
vm/src/stdlib/ast/pattern.rs

🧠 Learnings (17)

📓 Common learnings

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: In most cases, Python code should not be edited. Bug fixes should be made through Rust code modifications only

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Applies to Lib/test/**/*.py : When tests fail due to unsupported syntax, keep the test as `@unittest.expectedFailure`, document that it requires PEP 695 support, and focus on tests that can be fixed through Rust code changes only

Cargo.toml (1)