Merge pull request #113 from strue36/escape-keywords

Escape keywords.

Author: mat-sop

Diff for: CHANGELOG.md

@@ -3,6 +3,7 @@
 ## UNRELEASED
 
 - Changed logic how custom scalar imports are generated. Deprecated `import_` key.
+- Added escaping of GraphQL names which are Python keywords by appending `_` to them.
 
 
 ## 0.5.0 (2023-04-05)

Diff for: ariadne_codegen/client_generators/arguments.py

@@ -25,7 +25,7 @@
 )
 from ..exceptions import ParsingError
 from ..plugins.manager import PluginManager
-from ..utils import str_to_snake_case
+from ..utils import process_name
 from .constants import ANY, OPTIONAL, SIMPLE_TYPE_MAP
 from .scalars import ScalarData
 
@@ -57,7 +57,7 @@ def generate(
         dict_ = generate_dict()
         for variable_definition in variable_definitions:
             org_name = variable_definition.variable.name.value
-            name = self._process_name(org_name)
+            name = process_name(org_name, self.convert_to_snake_case)
             annotation, used_custom_scalar = self._parse_type_node(
                 variable_definition.type
             )
@@ -94,11 +94,6 @@ def get_used_inputs(self) -> List[str]:
     def get_used_custom_scalars(self) -> List[str]:
         return self._used_custom_scalars
 
-    def _process_name(self, name: str) -> str:
-        if self.convert_to_snake_case:
-            return str_to_snake_case(name)
-        return name
-
     def _parse_type_node(
         self,
         node: Union[NamedTypeNode, ListTypeNode, NonNullTypeNode, TypeNode],

Diff for: ariadne_codegen/client_generators/input_types.py

@@ -20,7 +20,7 @@
     generate_module,
 )
 from ..plugins.manager import PluginManager
-from ..utils import str_to_snake_case
+from ..utils import process_name
 from .constants import (
     ANY,
     BASE_MODEL_CLASS_NAME,
@@ -109,7 +109,7 @@ def _parse_input_definition(
         )
 
         for lineno, (org_name, field) in enumerate(definition.fields.items(), start=1):
-            name = self._process_field_name(org_name)
+            name = process_name(org_name, self.convert_to_snake_case)
             annotation, field_type = parse_input_field_type(
                 field.type, custom_scalars=self.custom_scalars
             )
@@ -140,11 +140,6 @@ def _parse_input_definition(
 
         return class_def
 
-    def _process_field_name(self, name: str) -> str:
-        if self.convert_to_snake_case:
-            return str_to_snake_case(name)
-        return name
-
     def _process_field_value(
         self,
         field_implementation: ast.AnnAssign,

Diff for: ariadne_codegen/client_generators/package.py

@@ -8,7 +8,7 @@
 from ..codegen import generate_import_from
 from ..exceptions import ParsingError
 from ..plugins.manager import PluginManager
-from ..utils import ast_to_str, str_to_pascal_case, str_to_snake_case
+from ..utils import ast_to_str, process_name, str_to_pascal_case
 from .arguments import ArgumentsGenerator
 from .client import ClientGenerator
 from .constants import (
@@ -171,7 +171,7 @@ def add_operation(self, definition: OperationDefinitionNode):
             raise ParsingError("Query without name.")
 
         return_type_name = str_to_pascal_case(name.value)
-        method_name = str_to_snake_case(name.value)
+        method_name = process_name(name.value, convert_to_snake_case=True)
         module_name = method_name
         file_name = f"{module_name}.py"
 

Diff for: ariadne_codegen/client_generators/result_types.py

@@ -33,7 +33,7 @@
 )
 from ..exceptions import NotSupported, ParsingError
 from ..plugins.manager import PluginManager
-from ..utils import str_to_pascal_case, str_to_snake_case
+from ..utils import process_name, str_to_pascal_case
 from .constants import (
     ANY,
     BASE_MODEL_CLASS_NAME,
@@ -270,11 +270,9 @@ def _get_field_name(self, field: FieldNode) -> str:
         return field.name.value
 
     def _process_field_name(self, name: str) -> str:
-        if self.convert_to_snake_case:
-            if name == TYPENAME_FIELD_NAME:
-                return "__typename__"
-            return str_to_snake_case(name)
-        return name
+        if self.convert_to_snake_case and name == TYPENAME_FIELD_NAME:
+            return "__typename__"
+        return process_name(name, self.convert_to_snake_case)
 
     def _get_field_from_schema(self, type_name: str, field_name: str) -> GraphQLField:
         try:

Diff for: ariadne_codegen/utils.py

@@ -1,5 +1,6 @@
 import ast
 import re
+from keyword import iskeyword
 from textwrap import indent
 
 import isort
@@ -70,3 +71,14 @@ def format_multiline_strings(source: str) -> str:
             formatted = convert_to_multiline_string(orginal_str, variable_indent_size)
             formatted_source = formatted_source.replace(orginal_str, formatted)
     return formatted_source
+
+
+def process_name(name: str, convert_to_snake_case: bool) -> str:
+    """Processes the GraphQL name to remove keywords
+    and optionally convert to snake_case."""
+    processed_name = name
+    if convert_to_snake_case:
+        processed_name = str_to_snake_case(processed_name)
+    if iskeyword(processed_name):
+        processed_name += "_"
+    return processed_name

Diff for: tests/client_generators/input_types_generator/test_names.py

@@ -8,8 +8,9 @@
     FIELD_CLASS,
 )
 from ariadne_codegen.client_generators.input_types import InputTypesGenerator
+from ariadne_codegen.utils import ast_to_str
 
-from ...utils import compare_ast, get_class_def
+from ...utils import compare_ast, get_assignment_target_names, get_class_def
 
 
 @pytest.mark.parametrize(
@@ -137,3 +138,26 @@ def test_generate_returns_module_with_fields_names_converted_to_snake_case(
 
     class_def = get_class_def(module)
     assert compare_ast(class_def, expected_class_def)
+
+
+def test_generate_returns_module_with_valid_field_names():
+    schema = """
+    input KeywordInput {
+        in: String!
+        from: String!
+        and: String!
+    }
+    """
+
+    generator = InputTypesGenerator(
+        schema=build_ast_schema(parse(schema)), enums_module="enums"
+    )
+
+    module = generator.generate()
+
+    parsed = ast.parse(
+        ast_to_str(module)
+    )  # Round trip because invalid identifiers get picked up in parse
+    class_def = get_class_def(parsed)
+    field_names = get_assignment_target_names(class_def)
+    assert field_names == {"in_", "from_", "and_"}

Diff for: tests/client_generators/result_types_generator/test_names.py

@@ -5,8 +5,9 @@
 from graphql import OperationDefinitionNode, build_ast_schema, parse
 
 from ariadne_codegen.client_generators.result_types import ResultTypesGenerator
+from ariadne_codegen.utils import ast_to_str
 
-from ...utils import compare_ast, get_class_def
+from ...utils import compare_ast, get_assignment_target_names, get_class_def
 from .schema import SCHEMA_STR
 
 
@@ -104,3 +105,30 @@ def test_generate_returns_module_with_handled_graphql_alias(
     assert len(class_def.body) == 1
     field_implementation = class_def.body[0]
     assert compare_ast(field_implementation, expected_field_implementation)
+
+
+def test_generate_returns_module_with_valid_field_names():
+    query_str = """
+    query CustomQuery {
+        camelCaseQuery {
+            in: id
+        }
+    }
+    """
+    generator = ResultTypesGenerator(
+        schema=build_ast_schema(parse(SCHEMA_STR)),
+        operation_definition=cast(
+            OperationDefinitionNode, parse(query_str).definitions[0]
+        ),
+        enums_module_name="enums",
+        convert_to_snake_case=True,
+    )
+
+    module = generator.generate()
+
+    parsed = ast.parse(
+        ast_to_str(module)
+    )  # Round trip because invalid identifiers get picked up in parse
+    class_def = get_class_def(parsed, name_filter="CustomQueryCamelCaseQuery")
+    field_names = get_assignment_target_names(class_def)
+    assert field_names == {"in_"}

Diff for: tests/client_generators/test_arguments_generator.py

@@ -179,6 +179,38 @@ def test_generate_returns_arguments_and_dictionary_with_snake_case_names():
     assert compare_ast(arguments_dict, expected_arguments_dict)
 
 
+def test_generate_returns_arguments_and_dictionary_with_valid_names():
+    generator = ArgumentsGenerator(schema=GraphQLSchema(), convert_to_snake_case=True)
+    query = "query q($from: String!, $and: String!, $in: String!) {r}"
+    variable_definitions = _get_variable_definitions_from_query_str(query)
+
+    arguments, arguments_dict = generator.generate(variable_definitions)
+
+    expected_arguments = ast.arguments(
+        posonlyargs=[],
+        args=[
+            ast.arg(arg="self"),
+            ast.arg(arg="from_", annotation=ast.Name(id="str")),
+            ast.arg(arg="and_", annotation=ast.Name(id="str")),
+            ast.arg(arg="in_", annotation=ast.Name(id="str")),
+        ],
+        kwonlyargs=[],
+        kw_defaults=[],
+        defaults=[],
+    )
+    expected_arguments_dict = ast.Dict(
+        keys=[
+            ast.Constant(value="from"),
+            ast.Constant(value="and"),
+            ast.Constant(value="in"),
+        ],
+        values=[ast.Name(id="from_"), ast.Name(id="and_"), ast.Name(id="in_")],
+    )
+
+    assert compare_ast(arguments, expected_arguments)
+    assert compare_ast(arguments_dict, expected_arguments_dict)
+
+
 def test_generate_returns_arguments_with_not_mapped_custom_scalar():
     schema_str = """
         schema { query: Query }

Diff for: tests/client_generators/test_package_generator.py

@@ -1,3 +1,4 @@
+import ast
 from datetime import datetime
 from textwrap import dedent, indent
 
@@ -14,6 +15,8 @@
 from ariadne_codegen.client_generators.scalars import ScalarData
 from ariadne_codegen.exceptions import ParsingError
 
+from ..utils import get_class_def
+
 SCHEMA_STR = """
 schema {
     query: Query
@@ -308,6 +311,38 @@ async def custom_query(self, id: str, param: Optional[str] = None) -> CustomQuer
         assert "from .custom_query import CustomQuery" in client_content
 
 
+def test_generate_creates_client_with_valid_method_names(tmp_path):
+    package_name = "test_graphql_client"
+    generator = PackageGenerator(
+        package_name,
+        tmp_path.as_posix(),
+        build_ast_schema(parse(SCHEMA_STR)),
+        async_client=False,
+    )
+    query_str = """
+    query From($id: ID!, $param: String) {
+        query1(id: $id) {
+            field1
+            field2 {
+                fieldb
+            }
+            field3
+        }
+    }
+    """
+
+    generator.add_operation(parse(query_str).definitions[0])
+    generator.generate()
+
+    client_file_path = tmp_path / package_name / "client.py"
+    with client_file_path.open() as client_file:
+        client_content = client_file.read()
+        parsed = ast.parse(client_content)
+        class_def = get_class_def(parsed)
+        function = [x for x in class_def.body if isinstance(x, ast.FunctionDef)][0]
+        assert function.name == "from_"
+
+
 def test_generate_with_conflicting_query_name_raises_parsing_error(tmp_path):
     generator = PackageGenerator(
         "test_graphql_client",

Diff for: tests/utils.py

@@ -1,7 +1,7 @@
 import ast
 from itertools import zip_longest
 from textwrap import dedent
-from typing import List, Optional, Union, cast
+from typing import List, Optional, Set, Union, cast
 
 
 def compare_ast(
@@ -24,16 +24,28 @@ def compare_ast(
     return node1 == node2
 
 
-def get_class_def(module: ast.Module, class_index=0) -> Optional[ast.ClassDef]:
+def get_class_def(
+    module: ast.Module, class_index=0, name_filter=None
+) -> Optional[ast.ClassDef]:
     found = 0
     for expr in module.body:
-        if isinstance(expr, ast.ClassDef):
+        if isinstance(expr, ast.ClassDef) and (
+            not name_filter or expr.name == name_filter
+        ):
             found += 1
             if found - 1 == class_index:
                 return expr
     return None
 
 
+def get_assignment_target_names(class_def: ast.ClassDef) -> Set[str]:
+    return {
+        x.target.id
+        for x in class_def.body
+        if isinstance(x, ast.AnnAssign) and isinstance(x.target, ast.Name)
+    }
+
+
 def filter_ast_objects(module: ast.Module, ast_class) -> List[ast.AST]:
     return [expr for expr in module.body if isinstance(expr, ast_class)]