Reserved name populated by snake-case-field

[ahnjaeshin]

Describe the bug

When a property name is Global, and --snake-case-field option is given, the generator creates a pydantic model named global which fails, because global is a reserved keyword.

To Reproduce

Below is a command to generator models from docker engine api.
The docker engine api has a schema(Mode) that has Global as property name.

Used commandline:

$ datamodel-codegen \
    --url "https://converter.swagger.io/api/convert?url=https://docs.docker.com/engine/api/v1.30.yaml" \
    --output ex.py \
    --use-standard-collections \
    --use-generic-container-types \
    --reuse-model \
    --enable-faux-immutability \
    --strict-nullable \
    --use-schema-description \
    --snake-case-field \
    --allow-population-by-field-name \
    --target-python-version 3.7 \
    --enum-field-as-literal all

Expected behavior

I expect that code generator would select another name rather than global. such as global_.

Solution Idea

We can add an underscore if the lowercased string is reserved.

parser.base.py:camel_to_snake

@lru_cache()
def camel_to_snake(string: str) -> str:
    subbed = _UNDER_SCORE_1.sub(r'\1_\2', string)
    subbed = _UNDER_SCORE_2.sub(r'\1_\2', subbed).lower()  # +
    if keyword.iskeyword(subbed):                                                       # +
        subbed = subbed + "_"                                                                  # +
    return subbed                                                                                       # +

I've tested this out.

Version:

• OS: macOS 10.15.7
• Python version: 3.7.10
• datamodel-code-generator version: master branch (commit d5bc50c)

[koxudaxi]

@yuyupopo
Thank you for posting the suggestion.

I'm working on improve field_name.
We should use the method to get valid field_name.

self.model_resolver.get_valid_field_name_and_alias

[koxudaxi]

We may need to split the class to ModelResolver and FieldNameResolver?
Because we should care about duplicate field_name.
I will think about it at lunchtime...

[ahnjaeshin]

@yuyupopo
Thank you for posting the suggestion.

I'm working on improve field_name.
We should use the method to get valid field_name.

self.model_resolver.get_valid_field_name_and_alias

But it seems that field preprocessors are called after get_valid_field_name_and_alias.
Are you planning to merge the functionalities of get_valid_field_name_and_alias and field preprocessors?

[koxudaxi]

Yes.
Of course, We should confirm the effect of the changes.
But, The code-generator is used in a lot of productions.
We should change carefully.
I want to hear your opinion.

[ahnjaeshin]

First of all, I believe resolving this issue won't effect existing programs, because it is mentioning a corner case that generation fails.
The code already assumes the field names aren't a reserved name. (as checked viaget_valid_field_name_and_alias). The situation in this issue is a bug where the assumption fails; the field name becomes a reserved one after field_preprocessor.

Second, splitting the functionality of resolving/generating field names/values) into ModelResolver and FieldNameResolver. Seems great. The FieldNameResolver would resolve field names, and ModelResolver uses FieldNameResolver to resolve/generate field names. The Parser would use only the ModelResolver, like now.
The design where Parser holds both ModelResolver and FieldNameResolver may be possible, but I think it would create tight coupling between the two resolvers.

Third, I have a suggestion. This project have grown quite large. There are many control knobs that users can change (almost all arguments to parser.base.Parser). And it hard for newcomers like me to understand how each argument affects code generation or any side effects.
Example)

class ArgumentsThatEffectOutputModelConfig(BaseModel):
  allow_population_by_field_name: bool
  enable_faux_immutability: bool
  ...

class ArgumentsThatEffectOutputModelFieldName(BaseModel):
  snake_case_field: bool
  ...

class ArgumentsThatEffectOutputModelFieldType(BaseModel):
  ...

[koxudaxi]

OK, I will split the function to FieldNameResolver/ModelResolver.

Third, I have a suggestion. This project have grown quite large. There are many control knobs that users can change (almost all arguments to parser.base.Parser). And it hard for newcomers like me to understand how each argument affects code generation or any side effects.
Example)

I think the options should be explained in the documents. #133
But, I have not written it yet :(

Thank you

[koxudaxi]

@yuyupopo
I'm sorry for the late release.
I pushed the new version as 0.10.0

[ahnjaeshin]

No worries :) Thx for the fix