fix: generate valid code when a path parameter with a default precedes one without

Author: saadkhantareen

end_to_end_tests/functional_tests/generated_code_execution/test_path_parameters.py

@@ -149,3 +149,79 @@ def test_path_params_with_hash_are_encoded(self, sync_detailed, Client):
         # Hash should be encoded as %23
         expected_url = "/items/item%231/details/detail%23id"
         assert call_kwargs["url"] == expected_url
+
+
+@with_generated_client_fixture(
+    """
+paths:
+  "/test/{has_default}/{no_default}":
+    get:
+      operationId: getTest
+      parameters:
+        - name: has_default
+          in: path
+          required: true
+          schema:
+            type: string
+            default: default_value
+        - name: no_default
+          in: path
+          required: true
+          schema:
+            type: integer
+      responses:
+        "200":
+          description: Success
+          content:
+            application/json:
+              schema:
+                type: object
+                properties:
+                  result:
+                    type: string
+"""
+)
+@with_generated_code_import(".api.default.get_test.sync_detailed")
+@with_generated_code_import(".client.Client")
+class TestPathParameterWithDefaultBeforeRequired:
+    """A path parameter with a default may precede one without a default in the path.
+
+    The generated function must still be valid Python, and the default must be used
+    when the parameter is not supplied.
+    """
+
+    def test_default_is_used_when_parameter_is_omitted(self, sync_detailed, Client):
+        mock_httpx_client = MagicMock(spec=httpx.Client)
+        mock_response = MagicMock(spec=httpx.Response)
+        mock_response.status_code = 200
+        mock_response.json.return_value = {"result": "ok"}
+        mock_response.content = b'{"result": "ok"}'
+        mock_response.headers = {}
+        mock_httpx_client.request.return_value = mock_response
+
+        client = Client(base_url="https://api.example.com")
+        client.set_httpx_client(mock_httpx_client)
+
+        sync_detailed(no_default=42, client=client)
+
+        mock_httpx_client.request.assert_called_once()
+        call_kwargs = mock_httpx_client.request.call_args[1]
+        assert call_kwargs["url"] == "/test/default_value/42"
+
+    def test_explicit_value_overrides_default(self, sync_detailed, Client):
+        mock_httpx_client = MagicMock(spec=httpx.Client)
+        mock_response = MagicMock(spec=httpx.Response)
+        mock_response.status_code = 200
+        mock_response.json.return_value = {"result": "ok"}
+        mock_response.content = b'{"result": "ok"}'
+        mock_response.headers = {}
+        mock_httpx_client.request.return_value = mock_response
+
+        client = Client(base_url="https://api.example.com")
+        client.set_httpx_client(mock_httpx_client)
+
+        sync_detailed(no_default=42, has_default="custom", client=client)
+
+        mock_httpx_client.request.assert_called_once()
+        call_kwargs = mock_httpx_client.request.call_args[1]
+        assert call_kwargs["url"] == "/test/custom/42"

openapi_python_client/parser/openapi.py

@@ -376,6 +376,11 @@ def sort_parameters(*, endpoint: "Endpoint") -> "Endpoint | ParseError":
         """
         Sorts the path parameters of an `endpoint` so that they match the order declared in `endpoint.path`.
 
+        Parameters which declare a default are then moved after parameters which do not, preserving the
+        path order within each group. Generated endpoint functions declare path parameters as
+        positional-or-keyword arguments, so a parameter with a default followed by one without would be
+        a `SyntaxError` in the generated code.
+
         Args:
             endpoint: The endpoint to sort the parameters of.
 
@@ -402,6 +407,8 @@ def sort_parameters(*, endpoint: "Endpoint") -> "Endpoint | ParseError":
                     f"{{{parameter.name.get_untrusted_value()}}}", f"{{{parameter.python_name}}}"
                 )
             )
+        # Stable sort: parameters without a default must be declared before those with one
+        endpoint.path_parameters.sort(key=lambda param: param.default is not None)
         return endpoint
 
     @staticmethod

tests/test_parser/test_openapi.py

@@ -7,6 +7,7 @@
 from openapi_python_client.parser.errors import ParseError
 from openapi_python_client.parser.openapi import Endpoint, EndpointCollection, import_string_from_class
 from openapi_python_client.parser.properties import Class, IntProperty, Parameters, Schemas
+from openapi_python_client.parser.properties.protocol import Value
 from openapi_python_client.schema import DataType
 from openapi_python_client.schema.ref import Ref
 from openapi_python_client.schema.untrusted_string import UntrustedString
@@ -268,6 +269,22 @@ def test_sort_parameters(self, string_property_factory):
 
         assert result_names == expected_names
 
+    def test_sort_parameters_moves_parameters_with_defaults_last(self, string_property_factory):
+        endpoint = self.make_endpoint()
+        endpoint.path = UntrustedString("/multiple-path-parameters/{param1}/{param2}/{param3}/{param4}")
+
+        endpoint.path_parameters.append(string_property_factory(name="param1", default=Value(PythonCode("'a'"), "a")))
+        endpoint.path_parameters.append(string_property_factory(name="param2"))
+        endpoint.path_parameters.append(string_property_factory(name="param3", default=Value(PythonCode("'c'"), "c")))
+        endpoint.path_parameters.append(string_property_factory(name="param4"))
+
+        result = Endpoint.sort_parameters(endpoint=endpoint)
+
+        assert isinstance(result, Endpoint)
+        result_names = [param.name for param in result.path_parameters]
+        # Parameters without a default come first; path order is preserved within each group
+        assert result_names == ["param2", "param4", "param1", "param3"]
+
     def test_sort_parameters_missing_param(self, string_property_factory):
         endpoint = self.make_endpoint()
         endpoint.path = UntrustedString("/multiple-path-parameters/{param1}/{param2}")