zijianhuang / openapiclientgen

Generate strongly typed C# and TypeScript client codes from Open API / Swagger definitions supporting jQuery, Angular, AXIOS, Fetch API, Aurelia and Angular Strictly Typed Forms
MIT License
64 stars 13 forks source link

Add option to use PascalCase usage for property and class names. #37

Closed codertimu closed 2 years ago

codertimu commented 2 years ago

Hi,

I attempted to add two optional features to the CS code generator:

1- Add the option to use PascalCase usage for property and class names: Settings.UsePascalCase

e.g. created_date to CreatedDate

2- Add the option to opt-out prefixing type names with the enclosing type: Settings.PrefixWithTypeName Currently, a class property is being named by prefixing the enclosing type name: e.g. Instead of

public class Data {
  public DataOrder Order { get; set;}
}

public class DataOrder {
  public DataOrderAdditionalInfo AdditionalInfo { get; set;}
}

public class DataOrderAdditionalInfo {
  public string Info { get; set;}
}

it will be like this:

public class Data {
  public Order Order { get; set;}
}

public class Order {
  public AdditionalInfo AdditionalInfo { get; set;}
}

public class AdditionalInfo {
  public string Info { get; set;}
}

I tested the features locally but am open to adding tests to the repo if guided.

zijianhuang commented 2 years ago

Can you provide a YAML or JSON definition file for such scenario?

And it will be even better that you write a test case for this. For example, SwagTests.ComponentsToCsTypesTests.TestRequired targets Settings/DateToDateOnly alone.

When you write your test case

codertimu commented 2 years ago

Added tests for both UsePascalCase and PrefixWithTypeName. Sample YAML file was also added. I preferred PrefixWithTypeName=False by default because I believe this should be the default behavior.

codertimu commented 2 years ago

Also, I adjusted post-build commands based on the host OS since I am working on a mac.

zijianhuang commented 2 years ago
  1. Created local branch
  2. Pull the pull request without commit to the local branch

See some minor conflicts:


<<<<<<< HEAD
            string s = TranslateJsonToCode("SwagMock\\Required.json");
            Assert.Equal(expected, s);
        }
=======
            string s = TranslateJsonToCode("SwagMock" + Path.DirectorySeparatorChar + "Required.json");
            Assert.Equal(expected, s);
        }
>>>>>>> 7bcce48b3d6a30b8494d6eabfc90729ab27376bf
``

Manually resolved the conflicts. 
zijianhuang commented 2 years ago

SwagTests.CodeGenNG2Tests.TestUspto failed

Assert.Equal() Failure
                                          ↓ (pos 247)
    Expected: ···\ttotal?: number;\r\n\t\tDataSetListApis?: Array<DataSetListApis>;···
    Actual:   ···\ttotal?: number;\r\n\t\tApis?: Array<Apis>;\r\n\t}\r\n\r\n\texport interf···
                                          ↑ (pos 247)

DataSetListApis is expected since it is defined inside DataSetList.

@codertimu : "I preferred PrefixWithTypeName=False by default because I believe this should be the default behavior."

I have seen quite a lot examples with types defined inside another type, but with multiple identical type name instances across multiple types, however with different detailed definitions. Bad design of the Swagger/OpenApi, I would regard. So to live with the inconvenient reality, it is slightly more convenient to have PrefixWithTypeName default true.

My guess is that some designers of YAML is basically walking through existing RESTful Web services which had been badly designed with sub-types of the same name scattering across, but with different detailed definitions.

codertimu commented 2 years ago

@zijianhuang Sure, it makes sense. I can make it True by default.

I think I haven't run the tests other than ComponentsToCsTypesTests. Let me make the above change and run all the tests and fix if any issues happen.

codertimu commented 2 years ago

@zijianhuang I can't run all the tests because the openapi-directory does not seem to be committed in the openapiclientgen/Tests/openapi-directory path. Can you please commit it so that I can run the tests?

zijianhuang commented 2 years ago

don't worry. I am running all the test. And this round I will run the Angular build tests which take hours. Your changes are basically all merged already.

zijianhuang commented 2 years ago

Ready to release soon.

zijianhuang commented 2 years ago

@codertimu

v2.2 released. Things look working well. Thanks. I just did code review on string ToPascalCase(this string original), just wonder the reasons why not using TextInfo.ToTitleCase(String) @ https://docs.microsoft.com/en-us/dotnet/api/system.globalization.textinfo.totitlecase ?

I had tested:

public static class StringExtensions
{
    static readonly TextInfo myTI = new CultureInfo("en-US", false).TextInfo;
    public static string ToPascalCase(this string original)
    {
        return myTI.ToTitleCase(original);
    }
}

Things look fine to me. And since you are in Germany, presumably you have seen some examples of using German to name identifiers. Can you further look at such issue? or even have a check on other European languages if non-English naming of identifiers is not rare?

This is obviously not something urgent, please feel free to do it anytime and create a new PR and this PR had been merged to master already.

codertimu commented 2 years ago

The current implementation handles lots of cases such as words with underscores(snake case), letters followed by numbers, and even words already in PascalCase(ToTitle fails this).

zijianhuang commented 2 years ago

thanks. case closed completely.