Fix pytest-parametrize-names-wrong-type (PT006) to edit both argnames and argvalues if both of them are single-element tuples/lists

[harupy]

Summary

Close #11243. Fix pytest-parametrize-names-wrong-type (PT006) to edit both argnames and argvalues if both of them are single-element tuples/lists.

# Before fix
@pytest.mark.parametrize(("x",), [(1,), (2,)])
def test_foo(x):
    ...

# After fix:
@pytest.mark.parametrize("x", [1, 2])
def test_foo(x):
    ...

Test Plan

New test cases

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[charliermarsh]

I'm sure you're right, but can you expand on why that's a problem? (Should we not be raising a diagnostic at all?)

[harupy]

@charliermarsh Thanks for the comment!

Should we not be raising a diagnostic at all?

This is another option.

[harupy]

I just found #11243. Should we unpack each item in argvalues as a fix?

[charliermarsh]

In this case, would this be correct?

@pytest.mark.parametrize("x,", [(1,), (2,)])

With the comma inside the string? I don't know enough about pytest.

[harupy]

@charliermarsh Let me check.

---

import pytest


@pytest.mark.parametrize("x,", [(1,), (2,)])
def test_foo(x):
    assert isinstance(x, int)

_____________________________________________________________________________________________________ test_foo[x0] ______________________________________________________________________________________________________

x = (1,)

    @pytest.mark.parametrize("x,", [(1,), (2,)])
    def test_foo(x):
>       assert isinstance(x, int)
E       assert False
E        +  where False = isinstance((1,), int)

a.py:6: AssertionError
_____________________________________________________________________________________________________ test_foo[x1] ______________________________________________________________________________________________________

x = (2,)

    @pytest.mark.parametrize("x,", [(1,), (2,)])
    def test_foo(x):
>       assert isinstance(x, int)
E       assert False
E        +  where False = isinstance((2,), int)

a.py:6: AssertionError

@pytest.mark.parametrize("x,", [(1,), (2,)]) is equivalent to @pytest.mark.parametrize("x", [(1,), (2,)]).

[MichaReiser]

Source of the relevant pytest code

https://github.com/pytest-dev/pytest/blob/9d4f36d87dae9a968fb527e2cb87e8a507b0beb3/src/_pytest/mark/structures.py#L124-L135

According to the rule. How should a user rewrite your example?

@pytest.mark.parametrize(("x",), [(1,), (2,)])
                       # ^^^^^^
def test_foo(x):
    ...

[harupy]

@MichaReiser

@pytest.mark.parametrize(("x",), [(1,), (2,)])
def test_foo(x):
    ...

should be rewritten to:

@pytest.mark.parametrize("x", [1, 2])
def test_foo(x):
    ...

# I'd prefer this style because I don't need to wonder what x is (tuple vs. int)

Both argnames and items in argvalues need to be fixed. PT006 only fixes argnames now.

(I implemented a fix to fix argvalues as well but didn't push it yet).

[MichaReiser]

(I implemented a fix to fix argvalues as well but didn't push it yet).

That sounds great. I'll have a look when you push the change

[harupy]

Should I add a test to ensure this doesn't conflict with PT007 which may also edit argvalues?

[MichaReiser]

This sounds like a great idea if we're concerned about whether the rules play nicely together.

[harupy]

@MichaReiser Manually confirmed PT006 and PT007 don't conflicts:

> cargo run -p ruff -- check --no-cache --select PT007,PT006 a.py --unsafe-fixes --fix
--- a.py
+++ a.py
@@ -1,6 +1,6 @@
 import pytest
 
 
-@pytest.mark.parametrize(("param",), [[1], [2], [3]])
+@pytest.mark.parametrize("param", [1, 2, 3])
 def test_foo(param):
     ...

Would fix 1 error.

[harupy]

Added a test case for this.

[MichaReiser]

This sounds like a great idea if we're concerned about whether the rules play nicely together.

[MichaReiser]

This is great. Thank you for following it thourgh