[refurb] Fix FURB103 fixes

[chirizxc]

Fixes #20785

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+10 -10 violations, +0 -0 fixes in 2 projects; 53 projects unchanged)

bokeh/bokeh (+9 -9 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ examples/models/buttons.py:75:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(doc, title="Button widgets"))`
- examples/models/buttons.py:75:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(title="Button widgets", doc))`
+ examples/models/calendars.py:104:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(doc, title="Calendar 2014"))`
- examples/models/calendars.py:104:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(title="Calendar 2014", doc))`
+ examples/models/daylight.py:107:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(doc, title="Daylight Plot"))`
- examples/models/daylight.py:107:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(title="Daylight Plot", doc))`
+ examples/models/gauges.py:110:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(doc, title="Gauges"))`
- examples/models/gauges.py:110:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(title="Gauges", doc))`
+ examples/models/glyphs.py:115:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(doc, title="Glyphs"))`
- examples/models/glyphs.py:115:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(title="Glyphs", doc))`
+ examples/models/sliders.py:74:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(doc, title="sliders"))`
- examples/models/sliders.py:74:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(title="sliders", doc))`
+ examples/models/twin_axis.py:57:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(doc, title="Twin Axis Plot"))`
- examples/models/twin_axis.py:57:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(title="Twin Axis Plot", doc))`
+ examples/models/widgets.py:299:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(doc, title="Widgets"))`
- examples/models/widgets.py:299:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(title="Widgets", doc))`
+ examples/output/apis/file_html.py:122:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(doc, title=plot.title.text))`
- examples/output/apis/file_html.py:122:10: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text(file_html(title=plot.title.text, doc))`

astropy/astropy (+1 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ astropy/io/fits/tests/test_hdulist.py:1134:14: FURB103 [*] `open` and `write` should be replaced by `Path(filename)....`
- astropy/io/fits/tests/test_hdulist.py:1134:14: FURB103 [*] `open` and `write` should be replaced by `Path(filename).write_text("Ceçi ne marche pas", encoding="utf=8")`

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
FURB103	20	10	10	0	0

[ntBre]

Wow, thank you! I should have noticed this in the ecosystem check from the previous PR. Thanks for following up!

This fix obviously works and even seems a bit simpler than the old code, but I'm a bit confused how we were ending up in this state.

[ntBre]

If I'm following this correctly, the bug is that we're relocating the argument to a function call within the f.write call when we're supposed to be relocating the argument to f.write itself. Is that accurate?

If so, it seems like the root problem might actually be the arg we're passing here and extracting in match_write_call. It doesn't seem like we should be rewriting the arguments to json.dumps at all.

[chirizxc]

I didn't quite get it, but in make_suggestion we actually just cut the source code for the argument (locator.slice(arg.range())) and put it directly, condition with open.keywords is needed to keep all arguments with names that were passed to the original open(...)

[MichaReiser]

The issue here is that the generator emits the arguments (and keyword arguments) in source order. See here

ruff/crates/ruff_python_codegen/src/generator.rs

Lines 1191 to 1209 in 48ada2d

	for arg_or_keyword in arguments.arguments_source_order() {
	match arg_or_keyword {
	ArgOrKeyword::Arg(arg) => {
	self.p_delim(&mut first, ", ");
	self.unparse_expr(arg, precedence::COMMA);
	}
	ArgOrKeyword::Keyword(keyword) => {
	self.p_delim(&mut first, ", ");
	if let Some(arg) = &keyword.arg {
	self.p_id(arg);
	self.p("=");
	self.unparse_expr(&keyword.value, precedence::COMMA);
	} else {
	self.p("**");
	self.unparse_expr(&keyword.value, precedence::MAX);
	}
	}
	}
	}

This is obviously very surprising and probably another reason why we should change arguments to be a single vec containing both keywoard and normal arguments, so that we don't have to do this guessing game inside generator.

For this rule, I think the fix is to use a more proper range for arg that isn't TextRange::default and ensures that the argument comes in the right order.

[chirizxc]

The issue here is that the generator emits the arguments (and keyword arguments) in source order. See here

ruff/crates/ruff_python_codegen/src/generator.rs

Lines 1191 to 1209 in 48ada2d

	for arg_or_keyword in arguments.arguments_source_order() {
	match arg_or_keyword {
	ArgOrKeyword::Arg(arg) => {
	self.p_delim(&mut first, ", ");
	self.unparse_expr(arg, precedence::COMMA);
	}
	ArgOrKeyword::Keyword(keyword) => {
	self.p_delim(&mut first, ", ");
	if let Some(arg) = &keyword.arg {
	self.p_id(arg);
	self.p("=");
	self.unparse_expr(&keyword.value, precedence::COMMA);
	} else {
	self.p("**");
	self.unparse_expr(&keyword.value, precedence::MAX);
	}
	}
	}
	}

This is obviously very surprising and probably another reason why we should change arguments to be a single vec containing both keywoard and normal arguments, so that we don't have to do this guessing game inside generator.

For this rule, I think the fix is to use a more proper range for arg that isn't TextRange::default and ensures that the argument comes in the right order.

I think this solution should be optimal, since we get rid of AST transformations and arg.clone(), and we also preserve the original spaces, please correct if this is wrong