astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.56k stars 1.09k forks source link

Allow empty `tuple()` call with C408 (unnecessary-collection-call) #13861

Open LordAro opened 2 weeks ago

LordAro commented 2 weeks ago

It's totally valid but () is a bit of a strange syntax to actually spawn an empty tuple, especially when it's used within another call. e.g.

action = core.ClassName(core.ActCon.STUB, tuple())  # noqa: C408
action = core.ClassName(core.ActCon.STUB, ())

my_list.append(tuple(1, 2, 3))  # perfectly valid C408 warning
my_list.append((1, 2, 3))
my_list.append(tuple())  # noqa: C408
my_list.append(())

I suggest adding a setting to allow empty tuples, but possibly just allowing it generally.

Ruff version: 0.7.0

MichaReiser commented 2 weeks ago

Hi @LordAro

I'm struggling to understand how your question relates to C409 because the rule doesn't trigger for me with the given code example (see this playground). However, your example raises C408 and the rule recommends using literal syntax over function calls.

Raising C408 for tuple is exactly the intent of the rule. That's why I don't think they should be excluded regardless of their position (see the rule documentation for the motivation). You can disable C408 when you prefer the method form.

LordAro commented 2 weeks ago

Ah, I screwed up the numbers when constructing this example didn't I? I've updated the original issue

C408 pertains to all collection types - I'm saying that empty tuples specifically could be a special case because the syntax ( () ) is used for so much else

LordAro commented 2 weeks ago

Although tbh maybe it's more relevant for 1-length tuples, as (foo) isn't a tuple but (foo, ) is

MichaReiser commented 2 weeks ago

Thanks for updating the issue. Glad to hear that we're talking about the same :)

I agree, () is very unambiguous. I can see how (foo,) is harder to spot. But the rule isn't just about readability:

It's unnecessary to call, e.g., dict() as opposed to using an empty literal ({}). The former is slower because the name dict must be looked up in the global scope in case it has been rebound.

That's why I still think we should keep flagging all tuples, including zero and one element tuples because it applies to them as well.