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

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

* refactor: Use location prefix for all duplicate params

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_query: 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_query,
+    }
+    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_query: Union[Unset, str] = UNSET,
+) -> Response[None]:
+    kwargs = _get_kwargs(
+        client=client,
+        param_path=param_path,
+        param_query=param_query,
+    )
+
+    response = httpx.get(
+        **kwargs,
+    )
+
+    return _build_response(response=response)
+
+
+async def asyncio_detailed(
+    *,
+    client: Client,
+    param_path: Union[Unset, str] = UNSET,
+    param_query: Union[Unset, str] = UNSET,
+) -> Response[None]:
+    kwargs = _get_kwargs(
+        client=client,
+        param_path=param_path,
+        param_query=param_query,
+    )
+
+    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

@@ -1,6 +1,6 @@
+import itertools
 from copy import deepcopy
 from dataclasses import dataclass, field
-from enum import Enum
 from typing import Any, Dict, Iterator, List, Optional, Set, Tuple, Union
 
 from pydantic import ValidationError
@@ -13,15 +13,6 @@
 from .responses import Response, response_from_data
 
 
-class ParameterLocation(str, Enum):
-    """The places Parameters can be put when calling an Endpoint"""
-
-    QUERY = "query"
-    PATH = "path"
-    HEADER = "header"
-    COOKIE = "cookie"
-
-
 def import_string_from_class(class_: Class, prefix: str = "") -> str:
     """Create a string which is used to import a reference"""
     return f"from {prefix}.{class_.module_name} import {class_.name}"
@@ -217,6 +208,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: Dict[str, Tuple[Property, oai.ParameterLocation]] = {}
         if data.parameters is None:
             return endpoint, schemas
         for param in data.parameters:
@@ -232,18 +224,39 @@ 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:
+                duplicate, duplicate_location = used_python_names[prop.python_name]
+                if duplicate.python_name == prop.python_name:  # Existing should be converted too for consistency
+                    duplicate.set_python_name(f"{duplicate.python_name}_{duplicate_location}")
+                prop.set_python_name(f"{prop.python_name}_{param.param_in}")
+            else:
+                used_python_names[prop.python_name] = (prop, param.param_in)
+
             endpoint.relative_imports.update(prop.get_imports(prefix="..."))
 
-            if param.param_in == ParameterLocation.QUERY:
+            if param.param_in == oai.ParameterLocation.QUERY:
                 endpoint.query_parameters.append(prop)
-            elif param.param_in == ParameterLocation.PATH:
+            elif param.param_in == oai.ParameterLocation.PATH:
                 endpoint.path_parameters.append(prop)
-            elif param.param_in == ParameterLocation.HEADER:
+            elif param.param_in == oai.ParameterLocation.HEADER:
                 endpoint.header_parameters.append(prop)
-            elif param.param_in == ParameterLocation.COOKIE:
+            elif param.param_in == oai.ParameterLocation.COOKIE:
                 endpoint.cookie_parameters.append(prop)
             else:
                 return ParseError(data=param, detail="Parameter must be declared in path or query"), schemas
+
+        name_check = set()
+        for prop in itertools.chain(
+            endpoint.query_parameters, endpoint.path_parameters, endpoint.header_parameters, endpoint.cookie_parameters
+        ):
+            if prop.python_name in name_check:
+                return (
+                    ParseError(data=data, detail=f"Could not reconcile duplicate parameters named {prop.python_name}"),
+                    schemas,
+                )
+            name_check.add(prop.python_name)
+
         return endpoint, schemas
 
     @staticmethod

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

@@ -3,6 +3,7 @@
     "OpenAPI",
     "Operation",
     "Parameter",
+    "ParameterLocation",
     "PathItem",
     "Reference",
     "RequestBody",
@@ -18,6 +19,7 @@
 from .openapi_schema_pydantic import MediaType
 from .openapi_schema_pydantic import OpenAPI as _OpenAPI
 from .openapi_schema_pydantic import Operation, Parameter, PathItem, Reference, RequestBody, Response, Responses, Schema
+from .parameter_location import ParameterLocation
 
 regex = re.compile(r"(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)")
 

openapi_python_client/schema/__init__.py

@@ -1,5 +1,6 @@
 from pydantic import Field
 
+from ..parameter_location import ParameterLocation
 from .parameter import Parameter
 
 
@@ -14,7 +15,7 @@ class Header(Parameter):
     """
 
     name = Field(default="", const=True)
-    param_in = Field(default="header", const=True, alias="in")
+    param_in = Field(default=ParameterLocation.HEADER, const=True, alias="in")
 
     class Config:
         allow_population_by_field_name = True

openapi_python_client/schema/openapi_schema_pydantic/header.py

@@ -2,6 +2,7 @@
 
 from pydantic import BaseModel, Field
 
+from ..parameter_location import ParameterLocation
 from .example import Example
 from .media_type import MediaType
 from .reference import Reference
@@ -30,7 +31,7 @@ class Parameter(BaseModel):
     - For all other cases, the `name` corresponds to the parameter name used by the [`in`](#parameterIn) property.
     """
 
-    param_in: str = Field(alias="in")
+    param_in: ParameterLocation = Field(alias="in")
     """
     **REQUIRED**. The location of the parameter. Possible values are `"query"`, `"header"`, `"path"` or `"cookie"`.
     """

openapi_python_client/schema/openapi_schema_pydantic/parameter.py

@@ -0,0 +1,10 @@
+from enum import Enum
+
+
+class ParameterLocation(str, Enum):
+    """The places Parameters can be put when calling an Endpoint"""
+
+    QUERY = "query"
+    PATH = "path"
+    HEADER = "header"
+    COOKIE = "cookie"

openapi_python_client/schema/parameter_location.py

@@ -543,6 +543,48 @@ 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()
+        param = oai.Parameter.construct(
+            name="test", required=True, param_schema=oai.Schema.construct(type="string"), param_in="path"
+        )
+        data = oai.Operation.construct(parameters=[param, param])
+        schemas = Schemas()
+        config = MagicMock()
+
+        result = Endpoint._add_parameters(endpoint=endpoint, data=data, schemas=schemas, config=config)
+        assert result == (
+            ParseError(data=data, detail="Could not reconcile duplicate parameters named test_path"),
+            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_path"
+        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