[refurb] Make the fix for FURB163 unsafe for log2, log10, *args, and deleted comments

[chirizxc]

Summary

/closes #18639

Test Plan

update snapshots

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[chirizxc]

do we have to update doc as well? because examples from the ussie and the doc are different

i mean

[chirizxc]

although it should only work for float

[chirizxc]

i'm not sure, but maybe such a is_float already exists somewhere in the helper or something like that

[ntBre]

There's this:

ruff/crates/ruff_python_semantic/src/analyze/typing.rs

Lines 981 to 984 in 4cfaa39

	/// Test whether the given binding can be considered an instance of `float`.
	pub fn is_float(binding: &Binding, semantic: &SemanticModel) -> bool {
	check_type::<FloatChecker>(binding, semantic)
	}

which would work on a Binding if you wanted to resolve variables too, but do we need to check if the argument is a float? You can see the same floating point behavior with less exotic arguments:

>>> math.log2(10)
3.321928094887362
>>> math.log(10, 2)
3.3219280948873626

and I think we're already checking for number literals.

[chirizxc]

there's also a question about ruff_python_ast/src/generated.rs file

rustrover writes that crate:: is not needed in 834 places, should the crates/ruff_python_ast/generate.py itself be corrected?

[ntBre]

Thanks! I just had a couple of suggestions.

And yes, we need to update the documentation with a ## Fix safety section because the fix is now unsafe in many cases.

[ntBre]

There's this:

ruff/crates/ruff_python_semantic/src/analyze/typing.rs

Lines 981 to 984 in 4cfaa39

	/// Test whether the given binding can be considered an instance of `float`.
	pub fn is_float(binding: &Binding, semantic: &SemanticModel) -> bool {
	check_type::<FloatChecker>(binding, semantic)
	}

which would work on a Binding if you wanted to resolve variables too, but do we need to check if the argument is a float? You can see the same floating point behavior with less exotic arguments:

>>> math.log2(10)
3.321928094887362
>>> math.log(10, 2)
3.3219280948873626

and I think we're already checking for number literals.

[ntBre]

nit: I think you can use arg.is_starred_expr here.

[ntBre]

there's also a question about ruff_python_ast/src/generated.rs file

rustrover writes that crate:: is not needed in 834 places, should the crates/ruff_python_ast/generate.py itself be corrected?

This is interesting, I guess clippy doesn't warn about this. We could probably open a separate issue or PR for this, if you wanted!

[ntBre]

Nice! One more docs nit, but this is otherwise good to go.

[ntBre]

I think I would treat these two cases separately. My understanding is that the fix is unsafe when comments are in the replacement range because they will be deleted, which seems a bit separate from either semantics or readability.

And I don't think the *args case has any effect on readability, it "just" changes the semantics of the code by raising an error when a different number of arguments are passed.

[chirizxc]

if write it like this, i think it would make more sense:

This fix is marked unsafe when the argument is a starred expression, as this changes the call semantics and may raise runtime errors. It is also unsafe if comments are present within the call, as they will be removed. Additionally, math.log(x, base) and math.log2(x) / math.log10(x) may differ due to floating-point rounding.

[ntBre]

Thanks!