Merge pull request #37 from viennadd/main

Normalise names of Python identifiers

Author: MarcoMuellner

src/openapi_python_generator/language_converters/python/common.py

@@ -1,4 +1,8 @@
+import keyword
+import re
+
 _use_orjson: bool = False
+_symbol_ascii_strip_re = re.compile(r"[^A-Za-z0-9_]")
 
 
 def set_use_orjson(value: bool) -> None:
@@ -17,3 +21,16 @@ def get_use_orjson() -> bool:
     """
     global _use_orjson
     return _use_orjson
+
+
+def normalize_symbol(symbol: str) -> str:
+    """
+    Remove invalid characters & keywords in Python symbol names
+    :param symbol: name of the identifier
+    :return: normalized identifier name
+    """
+    symbol = symbol.replace("-", "_")
+    normalized_symbol = _symbol_ascii_strip_re.sub('', symbol)
+    if normalized_symbol in keyword.kwlist:
+        normalized_symbol = normalized_symbol + '_'
+    return normalized_symbol

src/openapi_python_generator/language_converters/python/service_generator.py

@@ -83,16 +83,17 @@ def _generate_params_from_content(content: Union[Reference, Schema]):
                 continue  # pragma: no cover
             converted_result = ""
             required = False
+            param_name_cleaned = common.normalize_symbol(param.name)
 
             if isinstance(param.param_schema, Schema):
                 converted_result = (
-                    f"{param.name} : {type_converter(param.param_schema, param.required).converted_type}"
+                    f"{param_name_cleaned} : {type_converter(param.param_schema, param.required).converted_type}"
                     + ("" if param.required else " = None")
                 )
                 required = param.required
             elif isinstance(param.param_schema, Reference):
                 converted_result = (
-                    f"{param.name} : {param.param_schema.ref.split('/')[-1] }"
+                    f"{param_name_cleaned} : {param.param_schema.ref.split('/')[-1] }"
                     + (
                         ""
                         if isinstance(param, Reference) or param.required
@@ -153,7 +154,7 @@ def _generate_params_from_content(content: Union[Reference, Schema]):
 
 def generate_operation_id(operation: Operation, http_op: str) -> str:
     if operation.operationId is not None:
-        return f"{operation.operationId.replace('-', '_')}"
+        return common.normalize_symbol(operation.operationId)
     else:
         raise Exception(f"OperationId is not defined for {http_op}")  # pragma: no cover
 
@@ -165,8 +166,7 @@ def _generate_params(operation: Operation, param_in : Literal["query", "header"]
     params = []
     for param in operation.parameters:
         if isinstance(param, Parameter) and param.param_in == param_in:
-            param_name_cleaned = param.name.replace("-", "_")
-
+            param_name_cleaned = common.normalize_symbol(param.name)
             params.append(f"'{param.name}' : {param_name_cleaned}")
 
     return params

tests/regression/test_issue_illegal_py_symbols.py

@@ -0,0 +1,47 @@
+import pytest
+from click.testing import CliRunner
+
+from openapi_python_generator.__main__ import main
+from openapi_python_generator.common import HTTPLibrary
+from tests.conftest import test_data_folder
+from tests.conftest import test_result_path
+
+
+@pytest.fixture
+def runner() -> CliRunner:
+    """Fixture for invoking command-line interfaces."""
+    return CliRunner()
+
+
+@pytest.mark.parametrize(
+    "library",
+    [HTTPLibrary.httpx, HTTPLibrary.requests, HTTPLibrary.aiohttp],
+)
+def test_issue_keyword_parameter_name(runner: CliRunner, model_data_with_cleanup, library) -> None:
+    result = runner.invoke(
+        main,
+        [
+            str(test_data_folder / "issue_keyword_parameter_name.json"),
+            str(test_result_path),
+            "--library",
+            library.value,
+        ],
+    )
+    assert result.exit_code == 0
+
+
+@pytest.mark.parametrize(
+    "library",
+    [HTTPLibrary.httpx, HTTPLibrary.requests, HTTPLibrary.aiohttp],
+)
+def test_issue_illegal_character_in_operation_id(runner: CliRunner, model_data_with_cleanup, library) -> None:
+    result = runner.invoke(
+        main,
+        [
+            str(test_data_folder / "issue_illegal_character_in_operation_id.json"),
+            str(test_result_path),
+            "--library",
+            library.value,
+        ],
+    )
+    assert result.exit_code == 0

tests/test_data/issue_illegal_character_in_operation_id.json

@@ -0,0 +1,46 @@
+{
+	"openapi": "3.0.2",
+	"info": {
+		"title": "Test case for ",
+		"description": "Provide hooks for GetCourse processes and export/import of GetCourse data.",
+		"version": "0.1.0"
+	},
+	"servers": [],
+	"paths": {
+		"/v1/diffs": {
+			"post": {
+				"tags": [
+				],
+				"summary": "Get Groups",
+				"operationId": "get_grou##ps_$v1_ex%%ports_gr.oups__post",
+				"parameters": [
+				],
+				"requestBody": {
+					"content": {
+						"application/json": {
+							"schema": {
+								"title": "AccountGroupsRequest",
+								"required": [
+									"domain"
+								],
+								"type": "object",
+								"properties": {
+									"from": {
+										"title": "Domain",
+										"type": "string"
+									}
+								}
+							}
+						}
+					},
+					"required": true
+				},
+				"responses": {
+				}
+			}
+		}
+	},
+	"components": {
+		"schemas": {}
+	}
+}

tests/test_data/issue_keyword_parameter_name.json

@@ -0,0 +1,55 @@
+{
+	"openapi": "3.0.2",
+	"info": {
+		"title": "Test case for ",
+		"description": "Provide hooks for GetCourse processes and export/import of GetCourse data.",
+		"version": "0.1.0"
+	},
+	"servers": [],
+	"paths": {
+		"/v1/diffs": {
+			"post": {
+				"tags": [
+				],
+				"summary": "Get Groups",
+				"operationId": "get_groups_v1_exports_groups__post",
+				"parameters": [
+					{
+						"required": true,
+						"schema": {
+							"title": "from_sha of a diff",
+							"type": "string"
+						},
+						"name": "from",
+						"in": "query"
+					}
+				],
+				"requestBody": {
+					"content": {
+						"application/json": {
+							"schema": {
+								"title": "AccountGroupsRequest",
+								"required": [
+									"domain"
+								],
+								"type": "object",
+								"properties": {
+									"from": {
+										"title": "Domain",
+										"type": "string"
+									}
+								}
+							}
+						}
+					},
+					"required": true
+				},
+				"responses": {
+				}
+			}
+		}
+	},
+	"components": {
+		"schemas": {}
+	}
+}