Fix hyphens in path parameters (#986)

Fixes #976 and #578, and replaces #978.

@dbanty please choose your preferred approach between this and PR #987.

The original issue is that `openapi-python-client` throws `incorrect
path templating` warnings when the path has a parameter with a hyphen
and consequently fails to generate the endpoints.

---

The first commit ensures that hyphens are recognised as allowed
delimiters in parameter path names. This allows the endpoints to be
generated.

However, this generates lines like these:

```python
def _get_kwargs(
    user_id: int,
) -> Dict[str, Any]:
    _kwargs: Dict[str, Any] = {
        "method": "post",
        "url": "/activitypub/user-id/{user-id}/inbox".format(user-id=user_id,),
    }
    return _kwargs
```

Since Python variable names cannot contain hyphens, the `user-id`
parameter name here will trigger errors (starting with `ruff`).

---

The second commit replaces parameter names with their `python_name` in
`__init__.py` and passes the modified path to
`templates/endpoint_module.py.jinja`.

This fixes the issue and allows endpoints to be generated correctly. 

---

#987 is a different option for the second commit which instead creates a
custom Jinja filter in `utils.py` and so that the parameter names in
`endpoint.path` can be converted to their python names directly in
`templates/endpoint_module.py.jinja`.

Both approaches are equivalent and have been tested with different
parameter names (snake case, camel case, kebab case, mixed).

---------

Co-authored-by: harabat <[email protected]>
Co-authored-by: Dylan Anthony <[email protected]>
Co-authored-by: Dylan Anthony <[email protected]>

Author: 3 people

Diff for: .changeset/allow_hyphens_in_path_parameters.md

@@ -0,0 +1,15 @@
+---
+default: patch
+---
+
+# Allow hyphens in path parameters
+
+Before now, path parameters which were invalid Python identifiers were not allowed, and would fail generation with an
+"Incorrect path templating" error. In particular, this meant that path parameters with hyphens were not allowed.
+This has now been fixed!
+
+PR #986 fixed issue #976. Thanks @harabat!
+
+> [!WARNING]
+> This change may break custom templates, see [this diff](https://github.com/openapi-generators/openapi-python-client/pull/986/files#diff-0de8437b26075d8fe8454cf47d8d95d4835c7f827fa87328e03f690412be803e)
+> if you have trouble upgrading.

Diff for: end_to_end_tests/baseline_openapi_3.0.json

@@ -1457,6 +1457,27 @@
         }
       }
     },
+    "/naming/{hyphen-in-path}": {
+      "get": {
+        "tags": ["naming"],
+        "operationId": "hyphen_in_path",
+        "parameters": [
+          {
+            "name": "hyphen-in-path",
+            "in": "path",
+            "required": true,
+            "schema": {
+              "type": "string"
+            }
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": "Successful response"
+          }
+        }
+      }
+    },
     "/parameter-references/{path_param}": {
       "get": {
         "tags": [

Diff for: end_to_end_tests/baseline_openapi_3.1.yaml

@@ -1451,6 +1451,27 @@ info:
       }
     }
   },
+  "/naming/{hyphen-in-path}": {
+    "get": {
+      "tags": [ "naming" ],
+      "operationId": "hyphen_in_path",
+      "parameters": [
+        {
+          "name": "hyphen-in-path",
+          "in": "path",
+          "required": true,
+          "schema": {
+            "type": "string"
+          }
+        }
+      ],
+      "responses": {
+        "200": {
+          "description": "Successful response"
+        }
+      }
+    }
+  },
   "/parameter-references/{path_param}": {
     "get": {
       "tags": [

Diff for: end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/naming/__init__.py

@@ -2,7 +2,7 @@
 
 import types
 
-from . import mixed_case, post_naming_property_conflict_with_import
+from . import hyphen_in_path, mixed_case, post_naming_property_conflict_with_import
 
 
 class NamingEndpoints:
@@ -13,3 +13,7 @@ def post_naming_property_conflict_with_import(cls) -> types.ModuleType:
     @classmethod
     def mixed_case(cls) -> types.ModuleType:
         return mixed_case
+
+    @classmethod
+    def hyphen_in_path(cls) -> types.ModuleType:
+        return hyphen_in_path

Diff for: end_to_end_tests/golden-record/my_test_api_client/api/naming/hyphen_in_path.py

@@ -0,0 +1,91 @@
+from http import HTTPStatus
+from typing import Any, Dict, Optional, Union
+
+import httpx
+
+from ... import errors
+from ...client import AuthenticatedClient, Client
+from ...types import Response
+
+
+def _get_kwargs(
+    hyphen_in_path: str,
+) -> Dict[str, Any]:
+    _kwargs: Dict[str, Any] = {
+        "method": "get",
+        "url": f"/naming/{hyphen_in_path}",
+    }
+
+    return _kwargs
+
+
+def _parse_response(*, client: Union[AuthenticatedClient, Client], response: httpx.Response) -> Optional[Any]:
+    if response.status_code == HTTPStatus.OK:
+        return None
+    if client.raise_on_unexpected_status:
+        raise errors.UnexpectedStatus(response.status_code, response.content)
+    else:
+        return None
+
+
+def _build_response(*, client: Union[AuthenticatedClient, Client], response: httpx.Response) -> Response[Any]:
+    return Response(
+        status_code=HTTPStatus(response.status_code),
+        content=response.content,
+        headers=response.headers,
+        parsed=_parse_response(client=client, response=response),
+    )
+
+
+def sync_detailed(
+    hyphen_in_path: str,
+    *,
+    client: Union[AuthenticatedClient, Client],
+) -> Response[Any]:
+    """
+    Args:
+        hyphen_in_path (str):
+
+    Raises:
+        errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
+        httpx.TimeoutException: If the request takes longer than Client.timeout.
+
+    Returns:
+        Response[Any]
+    """
+
+    kwargs = _get_kwargs(
+        hyphen_in_path=hyphen_in_path,
+    )
+
+    response = client.get_httpx_client().request(
+        **kwargs,
+    )
+
+    return _build_response(client=client, response=response)
+
+
+async def asyncio_detailed(
+    hyphen_in_path: str,
+    *,
+    client: Union[AuthenticatedClient, Client],
+) -> Response[Any]:
+    """
+    Args:
+        hyphen_in_path (str):
+
+    Raises:
+        errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
+        httpx.TimeoutException: If the request takes longer than Client.timeout.
+
+    Returns:
+        Response[Any]
+    """
+
+    kwargs = _get_kwargs(
+        hyphen_in_path=hyphen_in_path,
+    )
+
+    response = await client.get_async_httpx_client().request(**kwargs)
+
+    return _build_response(client=client, response=response)

Diff for: openapi_python_client/parser/openapi.py

@@ -26,7 +26,7 @@
 from .properties.schemas import parameter_from_reference
 from .responses import Response, response_from_data
 
-_PATH_PARAM_REGEX = re.compile("{([a-zA-Z_][a-zA-Z0-9_]*)}")
+_PATH_PARAM_REGEX = re.compile("{([a-zA-Z_-][a-zA-Z0-9_-]*)}")
 
 
 def import_string_from_class(class_: Class, prefix: str = "") -> str:
@@ -379,6 +379,8 @@ def sort_parameters(*, endpoint: "Endpoint") -> Union["Endpoint", ParseError]:
             return ParseError(
                 detail=f"Incorrect path templating for {endpoint.path} (Path parameters do not match with path)",
             )
+        for parameter in endpoint.path_parameters:
+            endpoint.path = endpoint.path.replace(f"{{{parameter.name}}}", f"{{{parameter.python_name}}}")
         return endpoint
 
     @staticmethod

Diff for: openapi_python_client/templates/endpoint_module.py.jinja

@@ -31,7 +31,7 @@ def _get_kwargs(
         {% if endpoint.path_parameters %}
         "url": "{{ endpoint.path }}".format(
         {%- for parameter in endpoint.path_parameters -%}
-        {{parameter.name}}={{parameter.python_name}},
+        {{parameter.python_name}}={{parameter.python_name}},
         {%- endfor -%}
         ),
         {% else %}