RUF010 auto-fix only once per line

[LotemAm]

With 6db05d8, output of --show-fixes:

cargo run -p ruff_cli -- check crates/ruff/resources/test/fixtures/ruff/RUF010.py --no-cache --select RUF010 --show-source --show-fixes

crates/ruff/resources/test/fixtures/ruff/RUF010.py:9:4: RUF010 [*] Use conversion in f-string
   |
 9 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
   |    ^^^^^^^^ RUF010
10 | 
11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
   |
   = help: Replace f-string function call with conversion

crates/ruff/resources/test/fixtures/ruff/RUF010.py:9:16: RUF010 [*] Use conversion in f-string
   |
 9 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
   |                ^^^^^^^^^ RUF010
10 | 
11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
   |
   = help: Replace f-string function call with conversion

crates/ruff/resources/test/fixtures/ruff/RUF010.py:9:29: RUF010 [*] Use conversion in f-string
   |
 9 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
   |                             ^^^^^^^^^^ RUF010
10 | 
11 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
   |
   = help: Replace f-string function call with conversion

crates/ruff/resources/test/fixtures/ruff/RUF010.py:11:4: RUF010 [*] Use conversion in f-string
   |
11 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
12 | 
13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
   |    ^^^^^^^^^^^ RUF010
14 | 
15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
   |
   = help: Replace f-string function call with conversion

crates/ruff/resources/test/fixtures/ruff/RUF010.py:11:19: RUF010 [*] Use conversion in f-string
   |
11 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
12 | 
13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
   |                   ^^^^^^^^^^^^ RUF010
14 | 
15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
   |
   = help: Replace f-string function call with conversion

crates/ruff/resources/test/fixtures/ruff/RUF010.py:11:35: RUF010 [*] Use conversion in f-string
   |
11 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
12 | 
13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
   |                                   ^^^^^^^^^^^^^ RUF010
14 | 
15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
   |
   = help: Replace f-string function call with conversion

crates/ruff/resources/test/fixtures/ruff/RUF010.py:13:5: RUF010 [*] Use conversion in f-string
   |
13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
14 | 
15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
   |     ^^^^^^^^ RUF010
16 | 
17 | f"{foo(bla)}"  # OK
   |
   = help: Replace f-string function call with conversion

crates/ruff/resources/test/fixtures/ruff/RUF010.py:13:19: RUF010 [*] Use conversion in f-string
   |
13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
14 | 
15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
   |                   ^^^^^^^^^ RUF010
16 | 
17 | f"{foo(bla)}"  # OK
   |
   = help: Replace f-string function call with conversion

crates/ruff/resources/test/fixtures/ruff/RUF010.py:13:34: RUF010 [*] Use conversion in f-string
   |
13 | f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
14 | 
15 | f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
   |                                  ^^^^^^^^^^ RUF010
16 | 
17 | f"{foo(bla)}"  # OK
   |
   = help: Replace f-string function call with conversion

Found 9 errors.
[*] 9 potentially fixable with the --fix option.

But, actual fixes are only one per line, this is the output of --diff:

cargo run -p ruff_cli -- check crates/ruff/resources/test/fixtures/ruff/RUF010.py --no-cache --select RUF010 --diff

--- crates/ruff/resources/test/fixtures/ruff/RUF010.py
+++ crates/ruff/resources/test/fixtures/ruff/RUF010.py
@@ -6,11 +6,11 @@
     pass
 
 
-f"{str(bla)}, {repr(bla)}, {ascii(bla)}"  # RUF010
+f"{bla!s}, {repr(bla)}, {ascii(bla)}"  # RUF010
 
-f"{str(d['a'])}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
+f"{d['a']!s}, {repr(d['b'])}, {ascii(d['c'])}"  # RUF010
 
-f"{(str(bla))}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
+f"{bla!s}, {(repr(bla))}, {(ascii(bla))}"  # RUF010
 
 f"{foo(bla)}"  # OK
 

Would fix 3 errors

Note that reverting #4524 seems to fix the issue.

I'm guessing the issue is that these fixes are now considered overlapping in crates/ruff/src/autofix/mod.rs:53,60 though I'm not sure how to fix this. Best idea I could think of is moving the next fixes range by the difference of char count from the last fix (as long as they don't overlap in the first place)

Maybe it would be good to add snapshot tests for --diff?

[charliermarsh]

I’m wondering if we should allow overlapping fixes to be output in diff mode.

[LotemAm]

I think that, as a user, something seems wrong when running without --fix shows there are 9 fixes available, but with --fix only 3 were fixed and there are no error/warnings to indicate why.

[JonathanPlasse]

It would be possible fix this if the range for ExprFormattedValue was not the all f-string.

f"{str(bla)}, {repr(bla)}, {ascii(bla)}"

[
    Expr(
        StmtExpr {
            range: 0..40,
            value: JoinedStr(
                ExprJoinedStr {
                    range: 0..40,
                    values: [
                        FormattedValue(
                            ExprFormattedValue {
                                range: 0..40,
                                value: Call(
                                    ExprCall {
                                        range: 3..11,
                                        func: Name(
                                            ExprName {
                                                range: 3..6,
                                                id: Identifier(
                                                    "str",
                                                ),
                                                ctx: Load,
                                            },
                                        ),
                                        args: [
                                            Name(
                                                ExprName {
                                                    range: 7..10,
                                                    id: Identifier(
                                                        "bla",
                                                    ),
                                                    ctx: Load,
                                                },
                                            ),
                                        ],
                                        keywords: [],
                                    },
                                ),
                                conversion: None,
                                format_spec: None,
                            },
                        ),
                        Constant(
                            ExprConstant {
                                range: 0..40,
                                value: Str(
                                    ", ",
                                ),
                                kind: None,
                            },
                        ),
                        FormattedValue(
                            ExprFormattedValue {
                                range: 0..40,
                                value: Call(
                                    ExprCall {
                                        range: 15..24,
                                        func: Name(
                                            ExprName {
                                                range: 15..19,
                                                id: Identifier(
                                                    "repr",
                                                ),
                                                ctx: Load,
                                            },
                                        ),
                                        args: [
                                            Name(
                                                ExprName {
                                                    range: 20..23,
                                                    id: Identifier(
                                                        "bla",
                                                    ),
                                                    ctx: Load,
                                                },
                                            ),
                                        ],
                                        keywords: [],
                                    },
                                ),
                                conversion: None,
                                format_spec: None,
                            },
                        ),
                        Constant(
                            ExprConstant {
                                range: 0..40,
                                value: Str(
                                    ", ",
                                ),
                                kind: None,
                            },
                        ),
                        FormattedValue(
                            ExprFormattedValue {
                                range: 0..40,
                                value: Call(
                                    ExprCall {
                                        range: 28..38,
                                        func: Name(
                                            ExprName {
                                                range: 28..33,
                                                id: Identifier(
                                                    "ascii",
                                                ),
                                                ctx: Load,
                                            },
                                        ),
                                        args: [
                                            Name(
                                                ExprName {
                                                    range: 34..37,
                                                    id: Identifier(
                                                        "bla",
                                                    ),
                                                    ctx: Load,
                                                },
                                            ),
                                        ],
                                        keywords: [],
                                    },
                                ),
                                conversion: None,
                                format_spec: None,
                            },
                        ),
                    ],
                },
            ),
        },
    ),
]

[charliermarsh]

Yeah, I looked at this briefly, but there isn't really a straightforward way to fix this right now.

[JonathanPlasse]

Could you explain why the range is incorrect?

[MichaReiser]

A node's range should enclose all its content but not more. E.g. it should be impossible for siblings to have an overlapping range as it is the case here for the Constant and the ExprFormattedValue that both have the 0..40 range.

Constant(
                            ExprConstant {
                                range: 0..40,
                                value: Str(
                                    ", ",
                                ),
                                kind: None,
                            },
                        ),
                        FormattedValue(
                            ExprFormattedValue {
                                range: 0..40,

I have to take a look at RustPython on why it ends up with these ranges but this might be a problem for the formatter too.

[charliermarsh]

This was actually just a bug in the rule.