[swift5] Add useSPMFileStructure

[aymanbagabas]

Signed-off-by: Ayman Bagabas ayman.bagabas@gmail.com

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master, 5.1.x, 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

[4brunu]

I left a question, but otherwise looks good to me 👍
Could you please create a new sample project to ensure this works and keep working as expected? Or change a current one to use this flag?
Thanks

[4brunu]

One last thing that I noticed is that the options USE_SPM_FILE_STRUCTURE and SWIFT_PACKAGE_PATH don't apear in the docs.

To make them apear, you make something like

 cliOptions.add(new CliOption(PROJECT_NAME, "Project name in Xcode"));
 cliOptions.add(CliOption.newBoolean(USE_COROUTINES, "Whether to use the Coroutines adapter with the retrofit2 library.")); 

[4brunu]

@aymanbagabas thanks for you patience!
Looks greats, thanks for taking the time to make this great contribution 👍
If CI passes, this is ready to merge for me 🙂

[aymanbagabas]

@4brunu do you have any idea why this is failing? I couldn't tell what I'm missing in the last commit.

[ERROR] Tests run: 1437, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 66.084 s <<< FAILURE! - in TestSuite
[ERROR] checkOptionsProcessing(org.openapitools.codegen.swift5.Swift5OptionsTest)  Time elapsed: 0.019 s  <<< FAILURE!
java.lang.ClassCastException: java.lang.String cannot be cast to java.lang.Boolean
[ERROR] Failures: 
[ERROR]   Swift5OptionsTest>AbstractOptionsTest.checkOptionsProcessing:51 » ClassCast ja...
[ERROR] Tests run: 1437, Failures: 1, Errors: 0, Skipped: 0
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.0.0-M3:test (default-test) on project openapi-generator: There are test failures.

[4brunu]

Looks like you already fix it.
Can you run the following commands to update the sample projects and docs?
That's why circle ci is failing.

./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh

[4brunu]

CI is now passing.
@wing328 for me you can merge this PR

[wing328]

@aymanbagabas thanks for taking my feedback.

Shouldn't just swiftPackagePath be enough? In other words, we want to avoid duplicated options do very similar things.

[4brunu]

I would like to give you some context regarding this PR.
The current file structure is Classes/OpenAPIs/* because it was like this since the beginning and it’s fine, there is nothing wrong with this.
But then Apple introduced Swift Package Manager(SPM), that is an official package manager for Swift.
With that a standard for the file structure was introduced, Sources/projectName/*.
Since that was introduced, the libraries in the community are being migrated to this new structure.
I think that we should also migrate for this file structure, for good practice reasons.
The only problem is that that change would introduce a breaking change, for people that integrate the generated code manually. For those that use the SPM, Cocoapods or Carthage integration it will work automatically.
I think that a custom path isn’t needed here, because we are implementing a community standard.

My suggestion is that we go with Three of the two options:
1 - Make the breaking change, because this is a community standard, it's very easy to fix, and Swift 5 generator is still in beta.
2 - Make the breaking change, but also introduce a boolean that allows the user go back to the old behaviour.
3 - Don't make the breaking change, by introducing a boolean, and keep the legacy behaviour on by default.

@aymanbagabas @wing328 what do you think of this?

[aymanbagabas]

TBH, I'm all for making the breaking change since the generator is still in beta and users don't expect it to be stable. But instead of introducing a boolean to allow the old behavior, users could use swiftPackagePath to set the old behavior back.

[wing328]

Only experimental generators allows breaking changes in the upcoming patch release (5.1.1)

I prefer 3

3 - Don't make the breaking change, by introducing a boolean, and keep the legacy behaviour on by default.

In 5.2.0, we can change the option to use the new behaviour by default.

[4brunu]

Looks option 3 also looks good to me.
And what about the option swiftPackagePath should we keep it along side with useSPMFileStructure?
Or should we just keep useSPMFileStructure?
For me I think we can just keep useSPMFileStructure.
@aymanbagabas @wing328 what do you think?

[4brunu]

@aymanbagabas another place that I remembered that I think we need to update the path is in the XcodeGen.mustache file, the sources property

[wing328]

what do you think?

Prefer one option instead of two for simplicity.

[aymanbagabas]

I think this discussion should've taken place earlier. Anyway, since both options have been implemented already and work as expected, I don't see the point of dropping one of them.

As @wing328 mentioned, we can change this behavior and make the breaking changes in the 5.2.0 release. Drop useSPMFileStructure, and set the default source path to Sources/{{projectname}} and use swiftPackagePath to have the old behavior back.

[4brunu]

Looks good to me.
@aymanbagabas could you please fix the merge conflicts and the XcodeGen.mustache sources property?

[4brunu]

@aymanbagabas thanks for the changes.
Looks good to me.
@wing328 for me this PR is ready to merge 👍

[wing328]

Sorry for the delay. PR merged.