[BUG][csharp] Incorrect property name Varlock for lock in generated code

[fujieda]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• Have you tested with the latest master to confirm the issue still exists?
• Have you searched for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

The csharp generator currently generates the property name VarLock for the property lock in the following definition since version 7.0.0:

TestResponse:
  type: object
  properties:
    lock:
      type: boolean

Generated C# code:

/// <summary>
/// Gets or Sets VarLock
/// </summary>
[DataMember(Name = "lock", EmitDefaultValue = true)]
public bool VarLock { get; set; }

However, the property name Lock is not reserved word and should not be changed. The correct property name should be Lock.

openapi-generator version

version 7.0.0 and later versions, including the latest master.

OpenAPI declaration file content or url

openapi: 3.0.0
info:
  title: property name test
  version: 1.0.0
paths: {}
components:
  schemas:
    TestResponse:
      type: object
      properties:
        lock:
          type: boolean

Generation Details

java -jar openapi-generator-cli-7.2.0.jar generate -g csharp -i simple.yaml

Related issues/PRs

#13681

[wing328]

However, the property name Lock is not reserved word and should not be changed. The correct property name should be Lock.

ideally that should be the case but Lock seems to be a reserved word that cannot be used in other places (e.g. method name).

as a workaround, can you use the name mapping option to map the property name "lock to "Lock" instead for the time being?

https://github.com/OpenAPITools/openapi-generator/blob/master/docs/customization.md#name-mapping

[fujieda]

The suggested workaround unfortunately does not solve the problem.
Despite the name mapping attempt to Lock, the generator still treats it as a reserved word and converts it to VarLock.
The root cause of this issue is the case-insensitive nature of reserved word processing .

[wing328]

            {{#vars}}
            {{^isInherited}}
            {{^isReadOnly}}
            {{#required}}
            {{^conditionalSerialization}}
            {{^vendorExtensions.x-csharp-value-type}}
            // to ensure "{{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}}" is required (not null)
            if ({{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}} == null)
            {
                throw new ArgumentNullException("{{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}} is a required property for {{classname}} and cannot be null");
            }
            this.{{name}} = {{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}};
            {{/vendorExtensions.x-csharp-value-type}}
            {{#vendorExtensions.x-csharp-value-type}}
            this.{{name}} = {{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}};
            {{/vendorExtensions.x-csharp-value-type}}

i think it's due to the use of camelcase_param which is set to true to handle reserved word:

        return super.addMustacheLambdas()
                .put("camelcase_param", new CamelCaseLambda().generator(this).escapeAsParamName(true))

(in abstract csharp codegen)

personally I prefer to remove {{#lambda.camelcase_param}} from the template and replace it with x-csharp-name-camelcase instead (created in postProcessModels)

[fujieda]

Actually, the aspnetcore generator has the same problem. This template tweak does not fix it.
I do have a solution to this problem, but this solution reverts some of the recent changes in reserved word handling back to version 6.x.
So I'm hesitant to submit it as a pull request.

diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractCSharpCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractCSharpCodegen.java
index 576a254cc3f..ff27be9d471 100644
--- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractCSharpCodegen.java
+++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractCSharpCodegen.java
@@ -1297,10 +1297,7 @@ public abstract class AbstractCSharpCodegen extends DefaultCodegen implements Co

     @Override
     public String escapeReservedWord(String name) {
-        if (reservedWords().contains(name) ||
-                reservedWords().contains(name.toLowerCase(Locale.ROOT)) ||
-                reservedWords().contains(camelize(sanitizeName(name))) ||
-                isReservedWord(name) ||
+        if (isReservedWord(name) ||
                 name.matches("^\\d.*")) {
             name = this.invalidNamePrefix + camelize(name);
         }

[RookY2K]

Actually, the aspnetcore generator has the same problem. This template tweak does not fix it. I do have a solution to this problem, but this solution reverts some of the recent changes in reserved word handling back to version 6.x. So I'm hesitant to submit it as a pull request.

diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractCSharpCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractCSharpCodegen.java
index 576a254cc3f..ff27be9d471 100644
--- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractCSharpCodegen.java
+++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractCSharpCodegen.java
@@ -1297,10 +1297,7 @@ public abstract class AbstractCSharpCodegen extends DefaultCodegen implements Co

     @Override
     public String escapeReservedWord(String name) {
-        if (reservedWords().contains(name) ||
-                reservedWords().contains(name.toLowerCase(Locale.ROOT)) ||
-                reservedWords().contains(camelize(sanitizeName(name))) ||
-                isReservedWord(name) ||
+        if (isReservedWord(name) ||
                 name.matches("^\\d.*")) {
             name = this.invalidNamePrefix + camelize(name);
         }

I'm curious why we're doing case insensitive matching? Is there a desire to maintain a case insensitive list of reserved words?

[fujieda]

I don't think there is any justification for implementing case-insensitive matching.
@devhl-labs
Could you please provide the rationale for this decision?

[devhl-labs]

@fujieda It was something to do how CamelCaseLambda calls the escapeReservedWord method. Sadly my PR does not go into detail about it, but I think the reason was so that the model constructor parameters can be named the same as the property casing aside.

[devhl-labs]

The issue I was fixing is specific to System.Text.Json. The parameter and related constructor parameter have to be named the same or else STJ throws an error. Newtonsoft can handle it. Instead of prefixing var on keywords like lock, I can use the @ symbol to escape the reserved word. That will allow us to revert the case insensitive matching on reserved words.