![]() ## Summary At present, when we store a binding, we include a `TextRange` alongside it. The `TextRange` _sometimes_ matches the exact range of the identifier to which the `Binding` is linked, but... not always. For example, given: ```python x = 1 ``` The binding we create _will_ use the range of `x`, because the left-hand side is an `Expr::Name`, which has a valid range on it. However, given: ```python try: pass except ValueError as e: pass ``` When we create a binding for `e`, we don't have a `TextRange`... The AST doesn't give us one. So we end up extracting it via lexing. This PR extends that pattern to the rest of the binding kinds, to ensure that whenever we create a binding, we always use the range of the bound name. This leads to better diagnostics in cases like pattern matching, whereby the diagnostic for "unused variable `x`" here used to include `*x`, instead of just `x`: ```python def f(provided: int) -> int: match provided: case [_, *x]: pass ``` This is _also_ required for symbol renames, since we track writes as bindings -- so we need to know the ranges of the bound symbols. By storing these bindings precisely, we can also remove the `binding.trimmed_range` abstraction -- since bindings already use the "trimmed range". To implement this behavior, I took some of our existing utilities (like the code we had for `except ValueError as e` above), migrated them from a full lexer to a zero-allocation lexer that _only_ identifies "identifiers", and moved the behavior into a trait, so we can now do `stmt.identifier(locator)` to get the range for the identifier. Honestly, we might end up discarding much of this if we decide to put ranges on all identifiers (https://github.com/astral-sh/RustPython-Parser/pull/8). But even if we do, this will _still_ be a good change, because the lexer introduced here is useful beyond names (e.g., we use it find the `except` keyword in an exception handler, to find the `else` after a `for` loop, and so on). So, I'm fine committing this even if we end up changing our minds about the right approach. Closes #5090. ## Benchmarks No significant change, with one statistically significant improvement (-2.1654% on `linter/all-rules/large/dataset.py`): ``` linter/default-rules/numpy/globals.py time: [73.922 µs 73.955 µs 73.986 µs] thrpt: [39.882 MiB/s 39.898 MiB/s 39.916 MiB/s] change: time: [-0.5579% -0.4732% -0.3980%] (p = 0.00 < 0.05) thrpt: [+0.3996% +0.4755% +0.5611%] Change within noise threshold. Found 6 outliers among 100 measurements (6.00%) 4 (4.00%) low severe 1 (1.00%) low mild 1 (1.00%) high mild linter/default-rules/pydantic/types.py time: [1.4909 ms 1.4917 ms 1.4926 ms] thrpt: [17.087 MiB/s 17.096 MiB/s 17.106 MiB/s] change: time: [+0.2140% +0.2741% +0.3392%] (p = 0.00 < 0.05) thrpt: [-0.3380% -0.2734% -0.2136%] Change within noise threshold. Found 4 outliers among 100 measurements (4.00%) 3 (3.00%) high mild 1 (1.00%) high severe linter/default-rules/numpy/ctypeslib.py time: [688.97 µs 691.34 µs 694.15 µs] thrpt: [23.988 MiB/s 24.085 MiB/s 24.168 MiB/s] change: time: [-1.3282% -0.7298% -0.1466%] (p = 0.02 < 0.05) thrpt: [+0.1468% +0.7351% +1.3461%] Change within noise threshold. Found 15 outliers among 100 measurements (15.00%) 1 (1.00%) low mild 2 (2.00%) high mild 12 (12.00%) high severe linter/default-rules/large/dataset.py time: [3.3872 ms 3.4032 ms 3.4191 ms] thrpt: [11.899 MiB/s 11.954 MiB/s 12.011 MiB/s] change: time: [-0.6427% -0.2635% +0.0906%] (p = 0.17 > 0.05) thrpt: [-0.0905% +0.2642% +0.6469%] No change in performance detected. Found 20 outliers among 100 measurements (20.00%) 1 (1.00%) low severe 2 (2.00%) low mild 4 (4.00%) high mild 13 (13.00%) high severe linter/all-rules/numpy/globals.py time: [148.99 µs 149.21 µs 149.42 µs] thrpt: [19.748 MiB/s 19.776 MiB/s 19.805 MiB/s] change: time: [-0.7340% -0.5068% -0.2778%] (p = 0.00 < 0.05) thrpt: [+0.2785% +0.5094% +0.7395%] Change within noise threshold. Found 2 outliers among 100 measurements (2.00%) 1 (1.00%) low mild 1 (1.00%) high severe linter/all-rules/pydantic/types.py time: [3.0362 ms 3.0396 ms 3.0441 ms] thrpt: [8.3779 MiB/s 8.3903 MiB/s 8.3997 MiB/s] change: time: [-0.0957% +0.0618% +0.2125%] (p = 0.45 > 0.05) thrpt: [-0.2121% -0.0618% +0.0958%] No change in performance detected. Found 11 outliers among 100 measurements (11.00%) 1 (1.00%) low severe 3 (3.00%) low mild 5 (5.00%) high mild 2 (2.00%) high severe linter/all-rules/numpy/ctypeslib.py time: [1.6879 ms 1.6894 ms 1.6909 ms] thrpt: [9.8478 MiB/s 9.8562 MiB/s 9.8652 MiB/s] change: time: [-0.2279% -0.0888% +0.0436%] (p = 0.18 > 0.05) thrpt: [-0.0435% +0.0889% +0.2284%] No change in performance detected. Found 5 outliers among 100 measurements (5.00%) 4 (4.00%) low mild 1 (1.00%) high severe linter/all-rules/large/dataset.py time: [7.1520 ms 7.1586 ms 7.1654 ms] thrpt: [5.6777 MiB/s 5.6831 MiB/s 5.6883 MiB/s] change: time: [-2.5626% -2.1654% -1.7780%] (p = 0.00 < 0.05) thrpt: [+1.8102% +2.2133% +2.6300%] Performance has improved. Found 2 outliers among 100 measurements (2.00%) 1 (1.00%) low mild 1 (1.00%) high mild ``` |
||
---|---|---|
.. | ||
resources/test/fixtures | ||
src | ||
Cargo.toml | ||
generate.py | ||
orphan_rules_in_the_formatter.svg | ||
README.md |
Rust Python Formatter
The goal of our formatter is to be compatible with Black except for rare edge cases (mostly involving comment placement).
Implementing a node
Formatting each node follows roughly the same structure. We start with a Format{{Node}}
struct
that implements Default (and AsFormat
/IntoFormat
impls in generated.rs
, see orphan rules below).
#[derive(Default)]
pub struct FormatStmtReturn;
We implement FormatNodeRule<{{Node}}> for Format{{Node}}
. Inside, we destructure the item to make
sure we're not missing any field. If we want to write multiple items, we use an efficient write!
call, for single items .format().fmt(f)
or .fmt(f)
is sufficient.
impl FormatNodeRule<StmtReturn> for FormatStmtReturn {
fn fmt_fields(&self, item: &StmtReturn, f: &mut PyFormatter) -> FormatResult<()> {
// Here we destructure item and make sure each field is listed.
// We generally don't need range is it's underscore-ignored
let StmtReturn { range: _, value } = item;
// Implement some formatting logic, in this case no space (and no value) after a return with
// no value
if let Some(value) = value {
write!(
f,
[
text("return"),
// There are multiple different space and newline types (e.g.
// `soft_line_break_or_space()`, check the builders module), this one will
// always be translate to a normal ascii whitespace character
space(),
// `return a, b` is valid, but if it wraps we'd need parentheses.
// This is different from `(a, b).count(1)` where the parentheses around the
// tuple are mandatory
value.format().with_options(Parenthesize::IfBreaks)
]
)
} else {
text("return").fmt(f)
}
}
}
Check the builders
module for the primitives that you can use.
If something such as list or a tuple can break into multiple lines if it is too long for a single
line, wrap it into a group
. Ignoring comments, we could format a tuple with two items like this:
write!(
f,
[group(&format_args![
text("("),
soft_block_indent(&format_args![
item1.format()
text(","),
soft_line_break_or_space(),
item2.format(),
if_group_breaks(&text(","))
]),
text(")")
])]
)
If everything fits on a single line, the group doesn't break and we get something like ("a", "b")
.
If it doesn't, we get something like
(
"a",
"b",
)
For a list of expression, you don't need to format it manually but can use the JoinBuilder
util,
accessible through .join_with
. Finish will write to the formatter internally.
f.join_with(&format_args!(text(","), soft_line_break_or_space()))
.entries(self.elts.iter().formatted())
.finish()?;
// Here we need a trailing comma on the last entry of an expanded group since we have more
// than one element
write!(f, [if_group_breaks(&text(","))])
If you need avoid second mutable borrows with a builder, you can use format_with(|f| { ... })
as
a formattable element similar to text()
or group()
.
The generic comment formatting in FormatNodeRule
handles comments correctly for most nodes, e.g.
preceding and end-of-line comments depending on the node range. Sometimes however, you may have
dangling comments that are not before or after a node but inside of it, e.g.
[
# here we use an empty list
]
Here, you have to call dangling_comments
manually and stubbing out fmt_dangling_comments
in list
formatting.
impl FormatNodeRule<ExprList> for FormatExprList {
fn fmt_fields(&self, item: &ExprList, f: &mut PyFormatter) -> FormatResult<()> {
// ...
write!(
f,
[group(&format_args![
text("["),
dangling_comments(dangling),
soft_block_indent(&items),
text("]")
])]
)
}
fn fmt_dangling_comments(&self, _node: &ExprList, _f: &mut PyFormatter) -> FormatResult<()> {
// Handled as part of `fmt_fields`
Ok(())
}
}
Comments are categorized into Leading
, Trailing
and Dangling
, you can override this in
place_comment
.
Development notes
Handling parentheses and comments are two major challenges in a Python formatter.
We have copied the majority of tests over from Black and use insta for
snapshot testing with the diff between Ruff and Black, Black output and Ruff output. We put
additional test cases in resources/test/fixtures/ruff
.
The full Ruff test suite is slow, cargo test -p ruff_python_formatter
is a lot faster.
There is a ruff_python_formatter
binary that avoid building and linking the main ruff
crate.
You can use scratch.py
as a playground, e.g.
cargo run --bin ruff_python_formatter -- --emit stdout scratch.py
, which additional --print-ir
and --print-comments
options.
The origin of Ruff's formatter is the Rome formatter, e.g. the ruff_formatter crate is forked from the rome_formatter crate. The Rome repository can be a helpful reference when implementing something in the Ruff formatter
The orphan rules and trait structure
For the formatter, we would like to implement Format
from the rust_formatter crate for all AST
nodes, defined in the rustpython_parser crate. This violates Rust's orphan rules. We therefore
generate in generate.py
a newtype for each AST node with implementations of FormatNodeRule
,
FormatRule
, AsFormat
and IntoFormat
on it.