Simplify positional arguments compatibility code in pytest.fixture()

The dynamic scope feature added in 10bf6aac76
necessitated some wrangling of arguments in pytest.fixture(). In
particular, it deprecated positional arguments in favor of keyword-only
arguments, while keeping backward compatibility.

The way it did this avoided some code duplication but ended up being
quite hard to follow and to annotate with types.

Replace it with some straightforward code, which is not very DRY but is
simple and easy to remove when the time comes.

src/_pytest/fixtures.py

@@ -1031,50 +1031,8 @@ class FixtureFunctionMarker:
         return function
 
 
-FIXTURE_ARGS_ORDER = ("scope", "params", "autouse", "ids", "name")
-
-
-def _parse_fixture_args(callable_or_scope, *args, **kwargs):
-    arguments = {
-        "scope": "function",
-        "params": None,
-        "autouse": False,
-        "ids": None,
-        "name": None,
-    }
-    kwargs = {
-        key: value for key, value in kwargs.items() if arguments.get(key) != value
-    }
-
-    fixture_function = None
-    if isinstance(callable_or_scope, str):
-        args = list(args)
-        args.insert(0, callable_or_scope)
-    else:
-        fixture_function = callable_or_scope
-
-    positionals = set()
-    for positional, argument_name in zip(args, FIXTURE_ARGS_ORDER):
-        arguments[argument_name] = positional
-        positionals.add(argument_name)
-
-    duplicated_kwargs = {kwarg for kwarg in kwargs.keys() if kwarg in positionals}
-    if duplicated_kwargs:
-        raise TypeError(
-            "The fixture arguments are defined as positional and keyword: {}. "
-            "Use only keyword arguments.".format(", ".join(duplicated_kwargs))
-        )
-
-    if positionals:
-        warnings.warn(FIXTURE_POSITIONAL_ARGUMENTS, stacklevel=2)
-
-    arguments.update(kwargs)
-
-    return fixture_function, arguments
-
-
 def fixture(
-    callable_or_scope=None,
+    fixture_function=None,
     *args,
     scope="function",
     params=None,
@@ -1131,24 +1089,56 @@ def fixture(
                 ``fixture_<fixturename>`` and then use
                 ``@pytest.fixture(name='<fixturename>')``.
     """
+    # Positional arguments backward compatibility.
+    # If a kwarg is equal to its default, assume it was not explicitly
+    # passed, i.e. not duplicated. The more correct way is to use a
+    # **kwargs and check `in`, but that obfuscates the function signature.
+    if isinstance(fixture_function, str):
+        # It's actually the first positional argument, scope.
+        args = (fixture_function, *args)
+        fixture_function = None
+    duplicated_args = []
+    if len(args) > 0:
+        if scope == "function":
+            scope = args[0]
+        else:
+            duplicated_args.append("scope")
+    if len(args) > 1:
+        if params is None:
+            params = args[1]
+        else:
+            duplicated_args.append("params")
+    if len(args) > 2:
+        if autouse is False:
+            autouse = args[2]
+        else:
+            duplicated_args.append("autouse")
+    if len(args) > 3:
+        if ids is None:
+            ids = args[3]
+        else:
+            duplicated_args.append("ids")
+    if len(args) > 4:
+        if name is None:
+            name = args[4]
+        else:
+            duplicated_args.append("name")
+    if len(args) > 5:
+        raise TypeError(
+            "fixture() takes 5 positional arguments but {} were given".format(len(args))
+        )
+    if duplicated_args:
+        raise TypeError(
+            "The fixture arguments are defined as positional and keyword: {}. "
+            "Use only keyword arguments.".format(", ".join(duplicated_args))
+        )
+    if args:
+        warnings.warn(FIXTURE_POSITIONAL_ARGUMENTS, stacklevel=2)
+    # End backward compatiblity.
+
     if params is not None:
         params = list(params)
 
-    fixture_function, arguments = _parse_fixture_args(
-        callable_or_scope,
-        *args,
-        scope=scope,
-        params=params,
-        autouse=autouse,
-        ids=ids,
-        name=name,
-    )
-    scope = arguments.get("scope")
-    params = arguments.get("params")
-    autouse = arguments.get("autouse")
-    ids = arguments.get("ids")
-    name = arguments.get("name")
-
     if fixture_function and params is None and autouse is False:
         # direct decoration
         return FixtureFunctionMarker(scope, params, autouse, name=name)(
@@ -1159,7 +1149,7 @@ def fixture(
 
 
 def yield_fixture(
-    callable_or_scope=None,
+    fixture_function=None,
     *args,
     scope="function",
     params=None,
@@ -1173,7 +1163,7 @@ def yield_fixture(
         Use :py:func:`pytest.fixture` directly instead.
     """
     return fixture(
-        callable_or_scope,
+        fixture_function,
         *args,
         scope=scope,
         params=params,

testing/python/fixtures.py

@@ -4155,7 +4155,7 @@ def test_fixture_named_request(testdir):
     )
 
 
-def test_fixture_duplicated_arguments():
+def test_fixture_duplicated_arguments() -> None:
     """Raise error if there are positional and keyword arguments for the same parameter (#1682)."""
     with pytest.raises(TypeError) as excinfo:
 
@@ -4169,8 +4169,31 @@ def test_fixture_duplicated_arguments():
         "Use only keyword arguments."
     )
 
+    with pytest.raises(TypeError) as excinfo:
 
-def test_fixture_with_positionals():
+        @pytest.fixture(
+            "function",
+            ["p1"],
+            True,
+            ["id1"],
+            "name",
+            scope="session",
+            params=["p1"],
+            autouse=True,
+            ids=["id1"],
+            name="name",
+        )
+        def arg2(request):
+            pass
+
+    assert (
+        str(excinfo.value)
+        == "The fixture arguments are defined as positional and keyword: scope, params, autouse, ids, name. "
+        "Use only keyword arguments."
+    )
+
+
+def test_fixture_with_positionals() -> None:
     """Raise warning, but the positionals should still works (#1682)."""
     from _pytest.deprecated import FIXTURE_POSITIONAL_ARGUMENTS
 
@@ -4187,6 +4210,18 @@ def test_fixture_with_positionals():
     assert fixture_with_positionals._pytestfixturefunction.autouse
 
 
+def test_fixture_with_too_many_positionals() -> None:
+    with pytest.raises(TypeError) as excinfo:
+
+        @pytest.fixture("function", [0], True, ["id"], "name", "extra")
+        def fixture_with_positionals():
+            pass
+
+    assert (
+        str(excinfo.value) == "fixture() takes 5 positional arguments but 6 were given"
+    )
+
+
 def test_indirect_fixture_does_not_break_scope(testdir):
     """Ensure that fixture scope is respected when using indirect fixtures (#570)"""
     testdir.makepyfile(