Update UP032 to handle repeated indices/keywords

[harupy]

Example:

x = 1
print("{0} and {0}".format(x))
print("{a} and {a}".format(a=x))

Expected behavior:

UP032 is raised and the code is fixed to:

x = 1
print(f"{x} and {x}")
print(f"{x} and {x}")

Actual behavior:

UP032 is not raised (playground)

[dhruvmanila]

Hey, thanks for reporting. This matches the behavior of pyupgrade but I do think we could update the rule to handle such cases. \cc @charliermarsh

[harupy]

@dhruvmanila Thanks for the comment! I found this code:

https://github.com/asottile/pyupgrade/blob/e982da96c48181856859c0eba57c88ef72f916ac/tests/features/format_literals_test.py#L18

        # Don't touch non-incrementing integers
        "'{0} {0}'.format(1)",

I'm investigating why pyupgrade made this choice.

[harupy]

This comment might be the reason:

https://github.com/asottile/pyupgrade/blob/e982da96c48181856859c0eba57c88ef72f916ac/pyupgrade/_plugins/fstrings.py#L121

                # timid: could make the f-string longer
                if candidate and candidate in seen:
                    break

For example:

"{0} and {0} and {0}".format(xxxxxxxxxxxxxxxxxxx)

is obviously shorter than:

f"{xxxxxxxxxxxxxxxxxxx} and {xxxxxxxxxxxxxxxxxxx} and {xxxxxxxxxxxxxxxxxxx}"

[dhruvmanila]

Hey, thanks for looking. Yes, the first example is shorter than the fix but if the fix is under the line-length then it's a valid, otherwise we just ignore it:

ruff/crates/ruff/src/rules/pyupgrade/rules/f_strings.rs

Lines 343 to 357 in d9e84c5

	// Avoid refactors that exceed the line length limit.
	let col_offset = template.start() - checker.locator().line_start(template.start());
	if contents.lines().enumerate().any(|(idx, line)| {
	// If `template` is a multiline string, `col_offset` should only be applied to the first
	// line:
	// ```
	// a = """{} -> offset = col_offset (= 4)
	// {} -> offset = 0
	// """.format(0, 1) -> offset = 0
	// ```
	let offset = if idx == 0 { col_offset.to_usize() } else { 0 };
	offset + line.chars().count() > line_length.get()
	}) {
	return;
	}

[harupy]

Another advantage of the original code is modifiability? Suppose you want to rename xxxxxxxxxxxxxxxxxxx, you can replace a single xxxxxxxxxxxxxxxxxxx in the original code, but three xxxxxxxxxxxxxxxxxxx in the fixed code.

[dhruvmanila]

That's a good point although the LSP rename method can help with that (which most editors have). I think we should implement this. We already have the guard rail in place which avoids auto-fixing if the updated code becomes longer than the line-length.

[harupy]

@dhruvmanila Makes sense. I'll file a PR!

[dhruvmanila]

Closed by #6266