unnecessary-literal-within-tuple-call (C409) rule transforms single element list literals incorrectly

[bardiharborow]

The unnecessary-literal-within-tuple-call (C409) rule detects the violation here:

tuple([1])

ruff check --fix --unsafe-fixes fails to transform the code correctly, resulting in this, which is not a tuple:

(1)

Instead it should transform the code to:

(1,)

Environment:

% ruff --version
ruff 0.3.2

% cat pyproject.toml
[tool.ruff]
lint.select = [
    "C4"
]

[zanieb]

Thanks for the report!

[WindowGenerator]

@zanieb Hi!
Can I take it too?

[charliermarsh]

Go for it @WindowGenerator.

[bardiharborow]

I don't know much about Ruff internals but it feels like it might be safer to take the approach of transforming and then rendering the AST, rather than replacing specific string indexes, since the latter is vulnerable to producing invalid or incorrect code in this fashion.

[dhruvmanila]

Yeah, we have a Generator struct which does exactly that. Moving the fix to use that would solve the issue as it takes one element tuple into consideration.

[charliermarsh]

We should just add this case to the fix. Text-based fixes are preferable because they preserve all trivia. (This fix used to use LibCST but we intentionally changed it because it’s slow and we want to remove our LibCST dependency altogether eventually.)

I think there was just a misunderstanding on what the fix is supposed to do based on the comment above it in the code, which led to this oversight.

[bardiharborow]

Fixed by #10491.