fix(parser): Attempt to deduplicate endpoint parameters based on name and location (fixes #305)

Author: dbanty

end_to_end_tests/golden-record/my_test_api_client/api/parameters/__init__.py

@@ -0,0 +1,77 @@
+from typing import Any, Dict, Union
+
+import httpx
+
+from ...client import Client
+from ...types import UNSET, Response, Unset
+
+
+def _get_kwargs(
+    *,
+    client: Client,
+    param_path: Union[Unset, str] = UNSET,
+    param: Union[Unset, str] = UNSET,
+) -> Dict[str, Any]:
+    url = "{}/same-name-multiple-locations/{param}".format(client.base_url, param=param_path)
+
+    headers: Dict[str, Any] = client.get_headers()
+    cookies: Dict[str, Any] = client.get_cookies()
+
+    params: Dict[str, Any] = {
+        "param": param,
+    }
+    params = {k: v for k, v in params.items() if v is not UNSET and v is not None}
+
+    return {
+        "url": url,
+        "headers": headers,
+        "cookies": cookies,
+        "timeout": client.get_timeout(),
+        "params": params,
+    }
+
+
+def _build_response(*, response: httpx.Response) -> Response[None]:
+    return Response(
+        status_code=response.status_code,
+        content=response.content,
+        headers=response.headers,
+        parsed=None,
+    )
+
+
+def sync_detailed(
+    *,
+    client: Client,
+    param_path: Union[Unset, str] = UNSET,
+    param: Union[Unset, str] = UNSET,
+) -> Response[None]:
+    kwargs = _get_kwargs(
+        client=client,
+        param_path=param_path,
+        param=param,
+    )
+
+    response = httpx.get(
+        **kwargs,
+    )
+
+    return _build_response(response=response)
+
+
+async def asyncio_detailed(
+    *,
+    client: Client,
+    param_path: Union[Unset, str] = UNSET,
+    param: Union[Unset, str] = UNSET,
+) -> Response[None]:
+    kwargs = _get_kwargs(
+        client=client,
+        param_path=param_path,
+        param=param,
+    )
+
+    async with httpx.AsyncClient() as _client:
+        response = await _client.get(**kwargs)
+
+    return _build_response(response=response)

end_to_end_tests/golden-record/my_test_api_client/api/parameters/get_same_name_multiple_locations_param.py

@@ -760,6 +760,33 @@
           "200": {"description": "Success"}
         }
       }
+    },
+    "/same-name-multiple-locations/{param}": {
+      "description": "Test that if you have a property of the same name in multiple locations, it produces valid code",
+      "get": {
+        "tags": ["parameters"],
+        "parameters": [
+          {
+            "name": "param",
+            "in": "query",
+            "schema": {
+              "type": "string"
+            }
+          },
+          {
+            "name": "param",
+            "in": "path",
+            "schema": {
+              "type": "string"
+            }
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": ""
+          }
+        }
+      }
     }
   },
   "components": {

end_to_end_tests/openapi.json

@@ -217,6 +217,7 @@ def _add_parameters(
         *, endpoint: "Endpoint", data: Union[oai.Operation, oai.PathItem], schemas: Schemas, config: Config
     ) -> Tuple[Union["Endpoint", ParseError], Schemas]:
         endpoint = deepcopy(endpoint)
+        used_python_names = set()
         if data.parameters is None:
             return endpoint, schemas
         for param in data.parameters:
@@ -232,6 +233,16 @@ def _add_parameters(
             )
             if isinstance(prop, ParseError):
                 return ParseError(detail=f"cannot parse parameter of endpoint {endpoint.name}", data=prop.data), schemas
+
+            if prop.python_name in used_python_names:
+                prop.set_python_name(f"{prop.python_name}_{param.param_in}")
+
+            if prop.python_name in used_python_names:
+                return (
+                    ParseError(detail=f"Encountered duplicate properties named {prop.python_name}", data=data),
+                    schemas,
+                )
+            used_python_names.add(prop.python_name)
             endpoint.relative_imports.update(prop.get_imports(prefix="..."))
 
             if param.param_in == ParameterLocation.QUERY:

openapi_python_client/parser/openapi.py

@@ -32,7 +32,10 @@ class Property:
     json_is_dict: ClassVar[bool] = False
 
     def __attrs_post_init__(self) -> None:
-        object.__setattr__(self, "python_name", utils.to_valid_python_identifier(utils.snake_case(self.name)))
+        self.set_python_name(self.name)
+
+    def set_python_name(self, new_name: str) -> None:
+        object.__setattr__(self, "python_name", utils.to_valid_python_identifier(utils.snake_case(new_name)))
 
     def get_base_type_string(self) -> str:
         return self._type_string

openapi_python_client/parser/properties/property.py

@@ -543,6 +543,47 @@ def test__add_parameters_happy(self, mocker):
         assert endpoint.header_parameters == [header_prop]
         assert schemas == schemas_3
 
+    def test__add_parameters_duplicate_properties(self, mocker):
+        from openapi_python_client.parser.openapi import Endpoint, Schemas
+
+        endpoint = self.make_endpoint()
+        parsed_schemas = mocker.MagicMock()
+        mocker.patch(
+            f"{MODULE_NAME}.property_from_data", return_value=(mocker.MagicMock(python_name="test"), parsed_schemas)
+        )
+        param = oai.Parameter.construct(name="test", required=True, param_schema=mocker.MagicMock(), param_in="path")
+        data = oai.Operation.construct(parameters=[param, param, param])
+        schemas = Schemas()
+        config = MagicMock()
+
+        result = Endpoint._add_parameters(endpoint=endpoint, data=data, schemas=schemas, config=config)
+        assert result == (ParseError(data=data, detail="Encountered duplicate properties named test"), parsed_schemas)
+
+    def test__add_parameters_duplicate_properties_different_location(self):
+        from openapi_python_client.parser.openapi import Endpoint, Schemas
+
+        endpoint = self.make_endpoint()
+        path_param = oai.Parameter.construct(
+            name="test", required=True, param_schema=oai.Schema.construct(type="string"), param_in="path"
+        )
+        query_param = oai.Parameter.construct(
+            name="test", required=True, param_schema=oai.Schema.construct(type="string"), param_in="query"
+        )
+        schemas = Schemas()
+        config = MagicMock()
+
+        result = Endpoint._add_parameters(
+            endpoint=endpoint,
+            data=oai.Operation.construct(parameters=[path_param, query_param]),
+            schemas=schemas,
+            config=config,
+        )[0]
+        assert isinstance(result, Endpoint)
+        assert result.path_parameters[0].python_name == "test"
+        assert result.path_parameters[0].name == "test"
+        assert result.query_parameters[0].python_name == "test_query"
+        assert result.query_parameters[0].name == "test"
+
     def test_from_data_bad_params(self, mocker):
         from openapi_python_client.parser.openapi import Endpoint