Command Plugin for generating the source code

[MahdiBM]

Motivation

This PR adds the option to use the package as a Command plugin instead of a BuildTool plugin.

This benefits those who use heavy OpenAPI documents, and prefer not to have to wait for an extra round of OpenAPI code generation which can be accidentally triggered at times, for example if you clean your build folder.

The whole idea of creating this Command plugin came after @czechboy0 's comment here: https://github.com/apple/swift-openapi-generator/pull/96#issuecomment-1615969962

Modifications

Generally, add a Command plugin target to the package, plus modifying the functions etc... to match/allow this addition.

Result

There is a new Command plugin, and users can choose between the Command plugin and the BuildTool plugin at will.

Test Plan

As visible in the PR discussions below, we've done enough manual-testing of the Command plugin.

[MahdiBM]

I'm wondering if we should prefer to factor out the common bits into an internal target

That's a known limitation of plugins. They can't depend on non-executable targets, and i tried again too:

plugin 'OpenAPIGeneratorCommandPlugin' cannot depend on 'PluginsShared' of type 'library'; this dependency is unsupported

it appears some parts of https://github.com/apple/swift-openapi-generator/pull/99 have crept into this PR

I do try to not mix PRs together, but that was intentional. Notice how i didn't mix #96 into this, as i think that PR might be up for debate. I can of course still revert the #99 changes if it is needed by the time this is ready for review.

The reason was that i thought there is a low chance for #99 for rejection (as you've now approved it 🙂), and i also thought i can save myself some trouble by just merging that PR into this, since we're using this PR in production.

[simonjbeaumont]

I'm wondering if we should prefer to factor out the common bits into an internal target

That's a known limitation of plugins. They can't depend on non-executable targets, and i tried again too:

plugin 'OpenAPIGeneratorCommandPlugin' cannot depend on 'PluginsShared' of type 'library'; this dependency is unsupported

Of course—it's all coming back to me now 😅

it appears some parts of #99 have crept into this PR

I do try to not mix PRs together, but that was intentional. Notice how i didn't mix #96 into this, as i think that PR might be up for debate. I can of course still revert the #99 changes if it is needed by the time this is ready for review.

The reason was that i thought there is a low chance for #99 for rejection (as you've now approved it 🙂), and i also thought i can save myself some trouble by just merging that PR into this, since we're using this PR in production.

Makes total sense. And yes, I expect #99 will land before this PR so will likely become a moot point.

I'm just getting back from vacation so apologies for the delay in taking a look at this. When you're ready for a review proper, let us know.

Just looking at your checklist in the PR description:

• [ ] Better names e.g. for the Command plugin target

I'm not sure we'll do much better than OpenAPIGeneratorCommandPlugin (which you went for), given we already have the namespace-hogging OpenAPIGenerator to refer to the build tool plugin.

• [x] (Won't Do) Work with all targets without needing to specify a specific target through Xcode UI or CLI arguments. Will worsen the error handling ... seems to not be worth it.

Could you elaborate here? I assume the original intention is that it would run once per target for which it found an OpenAPI document and config file. In this case, was your concern it exiting on the first error would lead to a game of whack-an-error for users?

• [ ] Unit tests? If any?

For what we can unit test, great. Otherwise, we could probably add a CI that runs the ahead-of-time generation workflow and diffs against the reference code. We'll have to think about whether it's worth it. WDYT, @czechboy0?

[MahdiBM]

I'm just getting back from vacation so apologies for the delay in taking a look at this. When you're ready for a review proper, let us know.

No worries. Hopefully i didn't interrupt your vacation 😅

Could you elaborate here? I assume the original intention is that it would run once per target for which it found an OpenAPI document and config file. In this case, was your concern it exiting on the first error would lead to a game of whack-an-error for users?

Basically yes ... i haven't actually tried implementing it since it didn't work out in my mind. I can try if you think it might be worth it. It was kind of a hard decision that i'm still not sure about. I can't even properly remember what situations i was thinking about when i decided it might not be worth it... I guess i'll give it an actual try and see what happens ...

[czechboy0]

Hi @MahdiBM – I'm happy to see this change, added a few high level comments, but like @simonjbeaumont, I'll respect your the Draft status of the PR and will not dive into detailed code review. I'll say, #99 might need a bit more work, so I think it's best if you remove those changes from this PR.

I do think we just need to use symlinks to share code between the two plugins, and for the code that doesn't need to also be used by the CLI, you can just add it as a separate file in one of the plugins, and symlink it to the other.

So, like:

Plugins
    OpenAPIGenerator
        plugin.swift
        GeneratorMode.swift -> ../../Sources/_OpenAPIGeneratorCore/GeneratorMode.swift
        PluginUtils.swift
    OpenAPIGeneratorCommand
        plugin.swift
        GeneratorMode.swift -> ../../Sources/_OpenAPIGeneratorCore/GeneratorMode.swift
        PluginUtils.swift -> ../OpenAPIGenerator/PluginUtils.swift
Sources
    _OpenAPIGeneratorCore
        GeneratorMode.swift

[MahdiBM]

So i finally managed to settle on the general form of how this is going to work (comments, docs etc... are not done yet) Some notes:

• Removed the ability to ask the generator to only run on 1 target. Because:
  • In short it'll be buggy.
  • To explain:
  • Xcode always passes arguments of form ["--target", "TARGET1", "--target", "TARGET2"] to the command plugin.
  • For some reason, this triggers the CommandPlugin instead of the XcodeCommandPlugin. Maybe my expectations are off.
  • The passed targets are not all targets. Xcode won't pass some of the targets sometimes, maybe due to scheme settings, which means if you rely on the arguments, you might miss the actual target that needs OpenAPI generation, as was the case with my tests on Penny.
  • So this means we need to check dependencies of targets too, but at that point we're not trusting the arguments anyway.
  • Before, the code was to act on 1 specified target if and only if arguments include 1 target, but that can also be buggy. Just imagine if there are 5 targets and Xcode decides to only pass 1 of the targets.
• Changed the error handling part to first check for existence of both config and document files, and throw only after that.
  • The benefit is that the command plugin can better identify if a target does not have any OpenAPI files and so it might not be supposed to have generated OpenAPI stuff at all, or if the target is supposed to have OpenAPI generation but one of the files isn't available.
  • Generally so far i've handled the errors in a complicated way, and that's only because i want the thrown user-facing errors to be as accurate as possible.

[MahdiBM]

@simonjbeaumont @czechboy0

The PR is now ready for review. If it seems acceptable, i can work on the docs part too.

From @simonjbeaumont:

For what we can unit test, great. Otherwise, we could probably add a CI that runs the ahead-of-time generation workflow and diffs against the reference code. We'll have to think about whether it's worth it. WDYT, @czechboy0?

I think we'll need that?

From @czechboy0:

I do think we just need to use symlinks to share code between the two plugins, and for the code that doesn't need to also be used by the CLI, you can just add it as a separate file in one of the plugins, and symlink it to the other.

I'll do that if you insist, but it looks cleaner to me this way because there are 4 files (GeneratorMode, InvocationSource, PluginError, PluginUtils) and 3 of them are required in both plugins. GeneratorMode is only required for the BuildTool plugin, but i still moved it to the PluginsShared directory to make it look cleaner. The PluginsShared directory is only used by plugins.

Also i renamed the directory from GeneratedSources to OpenAPISources to basically eliminate chances of name collisions with another plugin in the command-plugin considering both GeneratedSources and names like Types/Client/Server are common.

Please feel free to make naming suggestions, comment corrections etc...

[MahdiBM]

I also have paid attention to the soundness CI. I'm just not sure what-to-do/how-to-do with the remainder of the warnings.

• It emits some warnings for code that wasn't even edited by me. For example it expects me to not use long lines for error descriptions, which is weird. I can use multiline strings, but that doesn't sound right.
• It says /code/Package.swift:169:10: warning: [TrailingComma] add trailing comma to the last element in multiline collection literal while the whole Package.swift file is only 168 lines.

[MahdiBM]

@czechboy0 @simonjbeaumont Take 3!

As "bullet points" since that's easier to write for me:

• I tried to make the Command plugin work with only one target, but who knew there would be even more problems!
• The main problem this time around was that i noticed Xcode's command plugin run UI doesn't include all targets to pass to a command plugin.
  • It only includes products.
• So this means if we have a OpenAPI-generated target which is not e.g. an executable target and is only used by some other executable targets, we will not be able to choose the OpenAPI-generated target from Xcode's UI.
• To solve this problem, what i did was to find local dependencies of a package and run the generator on both the main target and its local dependencies.
  • Luckily the normal CommandPlugin protocol does provide enough context to be able to accurately find a target's local dependencies.
  • But the XcodeCommandPlugin protocol does not provide that info, so i had to get rid of it.
  • To be honest, as i mentioned before, i'm not sure if it used to do anything in the first place.
  • Even Xcode seemed to run the CommandPlugin's performCommand() function instead of XcodeCommandPlugin, not 100% sure why.
• The problem in all this, is that similar to what i tried before with running the command plugin automatically on all targets, it makes the error handling worse than if we could get a singular correct/exact target name from users.
• Though as always, i've tried to throw proper useful errors to users so i don't think this is a deal breaker.

If you wanted my opinion, i'd say yeah it's suboptimal that Xcode UI doesn't allow passing of any target names you want through the UI itself, but i still think the command plugin is still good enough to be worth it.

You may also ask why not just go back to doing it for all targets of the package at least? And to answer, i'm not 100% sure but i'm fine with this small level of explicitly choosing a product from Xcode's UI although it's one more click, considering it allows us to throw slightly more meaningful/accurate errors. Again, not sure, i can set it back to just run on all of a package's targets at once if you think that's better.

[czechboy0]

The main problem this time around was that i noticed Xcode's command plugin run UI doesn't include all targets to pass to a command plugin.

Well, that's unfortunate. We have at least these two options:

• live with that limitation, and ask adopters to wrap any target with OpenAPI documents in a product
• do what you did, and crawl the local dependency graph and try to run it on all of the targets with the right files

I'm on the fence, but at this point leaning towards what you did, and please file a FB on Xcode to support any targets, not just products, and we'll link it here to ensure that we can clean up the behavior in the future once Xcode improves that behavior.

Also curious which direction @simonjbeaumont is leaning.

Ok, let me re-review the code now 🙂

[MahdiBM]

live with that limitation, and ask adopters to wrap any target with OpenAPI documents in a product

Honestly that idea did not occur to me. It does sound like a good idea. I just tried adding the generator-related target to the products array of the package. It worked seamlessly and made Xcode show the target to choose for the command plugin.

[czechboy0]

live with that limitation, and ask adopters to wrap any target with OpenAPI documents in a product

Honestly that idea did not occur to me. It does sound like a good idea. I just tried adding the generator-related target to the products array of the package. It worked seamlessly and made Xcode show the target to choose for the command plugin.

Right, the question is whether this is a reasonable thing to ask people, to basically publicize any target where they use the plugin. Maybe that goes against our documented recommendations. Idk, @simonjbeaumont what do you think?

[MahdiBM]

One thing that i forgot ... should i make the command-plugin-invocation delete the build-plugin-invocation generated files? So users don't need to manually clean-build or modify DerivedData.

The build plugin won't be able to do the same though, considering it doesn't have write access to the source code.

[czechboy0]

One thing that i forgot ... should i make the command-plugin-invocation delete the build-plugin-invocation generated files? So users don't need to manually clean-build or modify DerivedData.

The build plugin won't be able to do the same though, considering it doesn't have write access to the source code.

I don't think so, they each produce files into different locations. The build plugin produces them in GeneratedSources in Derived Data, the command plugin writes them back to the package source directory. I wouldn't want them to try to delete each other's files, seems it could go wrong very badly very quickly, even if the sandbox allowed it (I don't think it would).

[simonjbeaumont]

Catching up a bit here...

live with that limitation, and ask adopters to wrap any target with OpenAPI documents in a product

Honestly that idea did not occur to me. It does sound like a good idea. I just tried adding the generator-related target to the products array of the package. It worked seamlessly and made Xcode show the target to choose for the command plugin.

Right, the question is whether this is a reasonable thing to ask people, to basically publicize any target where they use the plugin. Maybe that goes against our documented recommendations. Idk, @simonjbeaumont what do you think?

Just confirming my understanding here: are we saying that it works as expected with swift package ... but with Xcode there's some issues, that require this workaround?

In that event I don't think we should hold back the utility of having a supported workflow for Swift PM users.

We then are free to choose how to approach the Xcode flow, in light of any limitations it might have:

1. Actively disable it in code.
2. Let people use it but call out it's not supported in docs.
3. Actively support it and call out how to use it in docs, with all the limitations, and consequences of any workarounds.

One thing that i forgot ... should i make the command-plugin-invocation delete the build-plugin-invocation generated files? So users don't need to manually clean-build or modify DerivedData.

The build plugin won't be able to do the same though, considering it doesn't have write access to the source code.

I think I'd be OK with it removing things from DerivedData. But I don't feel strongly about it. What are the other options?

• Let the build fail?
• Emit warnings?
• Abort and tell the user to clean?

Aside: Does the build plugin have read access to the source tree? If it did, it might be able to provide sensible errors/warnings, even if it cannot delete things automatically.

[czechboy0]

Just confirming my understanding here: are we saying that it works as expected with swift package ... but with Xcode there's some issues, that require this workaround?

Right, from CLI it always works, but to support Xcode, which doesn't allow selecting a target, only a product, we had to do a bunch more implementation complexity. So with this PR, we support both, it's just a little more complex than if Xcode allowed selecting targets to run commands on. But it's done now, and should work, so I wouldn't advocate to go and remove it.

Regarding removing other files: just to clarify, this would only be needed for people who are switching between using the build plugin and the command plugin. I wouldn't worry about it, as long as people follow our docs, they'll get into a good state. I wouldn't want to open the can of worms of tracking user's local state and try to get them out of having a bad cache, as not even the build system tries to do that – so I'm okay with them cleaning their build folder and trying again, at which point it'll work.

[MahdiBM]

@czechboy0 one more thing i remembered: If a project has been using the build plugin and switches to the command plugin, Xcode/Swift would throw the error that says "there are files with same names". Though i think it'll still be fine, because even if we change file names, then Xcode will just throw "duplicate type name" errors instead. So i think the solution is to just point this out in the docs. Possibly have a "Switching between plugins" tutorial.

[czechboy0]

@czechboy0 one more thing i remembered: If a project has been using the build plugin and switches to the command plugin, Xcode/Swift would throw the error that says "there are files with same names". Though i think it'll still be fine, because even if we change file names, then Xcode will just throw "duplicate type name" errors instead. So i think the solution is to just point this out in the docs. Possibly have a "Switching between plugins" tutorial.

We'll see if people actually hit it and struggle to figure it out, but I'm fine with us not adding complexity just to catch this one case and try to provide a better error. It seems it'd be too fragile anyway, as it'd rely on a bunch of implementation details of the build system and Xcode, so I'd like to avoid it.

[MahdiBM]

@czechboy0

please file a FB on Xcode to support any targets, not just products, and we'll link it here to ensure that we can clean up the behavior in the future once Xcode improves that behavior.

FB12674830 🙂

[MahdiBM]

@czechboy0 one more try 🙂 :

Not choosing any product from Xcode UI:

Choosing 'AutoPingsLambda' which doesn't have OpenAPI files, and its local dependencies too:

Choosing the exact product:

[simonjbeaumont]

This seems good to me. To summarise:

1. If you don't select any target, it tries them all and "skips" targets that aren't deemed appropriate.
2. If you explicitly select an appropriate target, it succeeds.
3. If you explicitly select an inappropriate target, it fails.

In scenario (1), I can see in the screenshot two things that would be nice to improve:

• The order of the logs: would be nice to log the target being operated on before the rest of the logging. This reads clearer IMO.
• Would be nice for these "skipped" targets to appear as "skipped" or "warnings" rather than "failed" in the logs.

I don't mind deferring that to another PR though. WDYT @czechboy0 @MahdiBM?

[MahdiBM]

@simonjbeaumont I know this PR has been going for long enough that you probably don't want to bother me anymore, but those changes are small enough, and even if they weren't, i still wouldn't mind them 😅

The order of the logs: would be nice to log the target being operated on before the rest of the logging. This reads clearer IMO.

Good idea but will basically duplicate the logs because we'll need to say running on target '----' then run failed on target '-----'. (Unless the target is just straight up not a swift source module in which case no need for duplication.)

Should i go for it? or did you mean something else?

Would be nice for these "skipped" targets to appear as "skipped" or "warnings" rather than "failed" in the logs.

No objections, but can you mention some examples so i know what you exactly have in mind? Like just changing the words, or kind of flagging the print statements by starting them with e.g. Skipped target '----':, Warnings for target '-----':? EDIT: Ah reading your message again, you probably just meant changing the word. Though i still like the other idea 🤔

[MahdiBM]

What i do prefer though, is for the Docs part to be deferred to another PR considering this has taken enough time/length 😅 would prefer this to be merged sooner than later, when possible, so i can also start using the main Apple repo in my Package.swifts 🙂

[simonjbeaumont]

@simonjbeaumont I know this PR has been going for long enough that you probably don't want to bother me anymore, but those changes are small enough, and even if they weren't, i still wouldn't mind them 😅

OK, great!

Both of these comments pertain to this logging from your screenshot above, so I'll paste the textual version here before continuing to discuss your specific questions:

✅ Invoking plug-in command "OpenAPIGeneratorCommand" 18.5 seconds
- Target 'PennyTests': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'Models': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'Penny': OpenAPI code generation failed. No expected config or OpenAPI document files.
Swift OpenAPI Generator is running with the following configuration:
- OpenAPI document path: /Users/mahdibm/Github/penny-bot/Sources/GitHubAP|/openapi.yaml
- Configuration path: /Users/mahdibm/Github/penny-bot/Sources/GitHubAP|/openapi-generator-config.yml
- Generator modes: types, client
- Output file names: Types.swift, Client.swift
- Output directory: /Users/mahdibm/Github/penny-bot/Sources/GitHubAP|/GeneratedSources
- Diagnostics output path: <none - logs to stderr>
- Current directory: /Users/mahdibm/Github/penny-bot
- Plugin source: command
- Additional imports: <none>
File Types.swift: unchanged
File Client.swift: unchanged
- Target 'GitHubAPI': OpenAPI code generation successfully completed.
- Target 'Extensions': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'GHHooksLambda': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'Fake': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'SponsorsLambda': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'SharedServices': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'CoinsLambda': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'FaqsLambda': OpenAPI code generation failed. No expected config or OpenAPI document files.
- Target 'AutoPingsLambda': OpenAPI code generation failed. No expected config or OpenAPI document files.
✅ Activity Log Complete 7/20/23, 10:50 AM 17.8 seconds
No issues

The order of the logs: would be nice to log the target being operated on before the rest of the logging. This reads clearer IMO.

Good idea but will basically duplicate the logs because we'll need to say running on target '----' then run failed on target '-----'. (Unless the target is just straight up not a swift source module in which case no need for duplication.)

Should i go for it? or did you mean something else?

Personally, I prefer logging to be before an operation, and, looking through the build logs in Xcode for a target I happen to have open I see this pattern. Examples include Compiling ..., and Discovering Swift tasks after .... When an operation is failed, there is some "duplicate" logging; here is a textual version of what I see in the report navigator when I replace print with pint (note, the emojis aren't actually in the log—they are a representation of how Xcode renders the icons):

...
✅ Planning Swift module swift_openapi_generator (arm64)
ℹ️ Compiling runGenerator.swift 
    ℹ️ Compile runGenerator.swift (arm64)
        ❌ Cannot find 'pint' in scope
❌ Build failed ...

When the issue if fixed, I see:

✅ Planning Swift module swift_openapi_generator (arm64)
✅ Compiling runGenerator.swift 
    ✅ Compile runGenerator.swift (arm64)
...
✅ Build succeeded ...

Would be nice for these "skipped" targets to appear as "skipped" or "warnings" rather than "failed" in the logs.

No objections, but can you mention some examples so i know what you exactly have in mind? Like just changing the words, or kind of flagging the print statement by starting them with e.g. Skipped target '----':, Warnings for target '-----':?

I think it's odd to see big green check marks and lots of <some operation> failed in the logs.

I'm not suggesting we go for emojis, but I'd be happy with us finding some terminology that is clear about what's going on, for example:

Considering target for OpenAPI generation: InappropriateTarget
Skipped target (no OpenAPI document): InappropriateTarget
Considering target for OpenAPI generation: AppropriateTarget
Running Swift OpenAPI Generator on target AppropriateTarget with the following configuration:
...

I'd be happy if it then printed a line explicitly saying whether it was succeeded or failed.

IMHO this small amount of "duplicate" logging is a small price to pay for clarity when debugging a failed build.

[MahdiBM]

@simonjbeaumont This is what i came up with.

Please let me know if i can improve any of the sentences.

I used the ✅ emoji for a successful code generation because i think it makes long logs like this more skimmable.

The - Stopping because target isn't configured for OpenAPI code generation. logs don't feel satisfying in terms of being different from the theme/mood of the rest of the logs, but that's the best i could come up with.

[simonjbeaumont]

@simonjbeaumont This is what i came up with. Thanks for going the extra mile on this one.

I used the ✅ emoji for a successful code generation because i think it makes long logs like this more skimmable.

While I initially hinted at not doing this, I think I agree; it does make it easier to read at a glance so I'm fine with it 👍

Please let me know if i can improve any of the sentences.

The - Stopping because target isn't configured for OpenAPI code generation. logs don't feel satisfying in terms of being different from the theme/mood of the rest of the logs, but that's the best i could come up with.

Aside from tone, this one in particular isn't informative—why isn't it "configured" and what does it mean for it to be configured? I this case we could have something like: - Skipped target: Target does not contain OpenAPI document or something similar?

Other than that I'm pleased with how this is looking.

@czechboy0 do you have anything to add before we push this over the line?

[MahdiBM]

@simonjbeaumont i made it non-informative because i suspected @czechboy0 would not like the duplication of "there are no files" messages as he requested some changes about them in the past reviews.

Though i think both views are completely valid. It's just a question of if the information is worth the duplication. I'm personally fine with both, maybe slightly leaning towards the current non-informative message.

[simonjbeaumont]

@simonjbeaumont i made it non-informative because i suspected @czechboy0 would not like the duplication of "there are no files" messages as he requested some changes about them in the past reviews.

Though i think both views are completely valid. It's just a question of if the information is worth the duplication. I'm personally fine with both, maybe slightly leaning towards the current non-informative message.

OK, that's fair enough, and sorry for missing that historical review context.

I'm happy with things the way they are now (subject to the soundness check passing again).

[MahdiBM]

Honestly i had no idea what some of the warnings were requesting:

** ✅ Found no unacceptable language.
** Running /code/scripts/check-license-headers.sh...
** ERROR: Unsupported file extension for file (exclude or update this script): Plugins/OpenAPIGenerator/PluginsShared
** Running /code/scripts/run-swift-format.sh...
Plugins/PluginsShared/PluginUtils.swift:4:92: warning: [UseEarlyExits] replace the `if/else` block with a `guard` statement containing the early exit
Plugins/PluginsShared/PluginUtils.swift:4:92: warning: [UseEarlyExits] replace the `if/else` block with a `guard` statement containing the early exit
/code/Package.swift:173:10: warning: [TrailingComma] add trailing comma to the last element in multiline collection literal
Package.swift:164:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginError.swift:14:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginError.swift:21:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginError.swift:97:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginError.swift:99:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginError.swift:104:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginError.swift:106:1: warning: [LineLength] line is too long
Plugins/PluginsShared/PluginUtils.swift:4:1: warning: [LineLength] line is too long
Plugins/OpenAPIGeneratorCommand/plugin.swift:58:1: warning: [LineLength] line is too long
Plugins/OpenAPIGeneratorCommand/plugin.swift:59:1: warning: [LineLength] line is too long
Plugins/OpenAPIGeneratorCommand/plugin.swift:62:1: warning: [LineLength] line is too long
** ERROR: ❌ Running swift-format produced errors.

  To fix, run the following command:

    % git ls-files -z '*.swift' | xargs -0 swift-format --in-place --parallel

A few of the warnings were also pointing at non-existent (or wrong) lines (like Package.swift being 172 lines and swift-format complaining about the 173 rd line!) But luckily i noticed the CI also mentions git ls-files -z '*.swift' | xargs -0 swift-format --in-place --parallel which automatically solved the problems.

EDIT: Not to mention the line is too long warnings which were in short complaining about something that couldn't have been solved, and even the command didn't really fix anything, it just moved the long lines to the line after the returns which is the best it could do.

[MahdiBM]

@simonjbeaumont @czechboy0 what should i do to resolve this last soundness error?

** ✅ Found no unacceptable language.
** Running /code/scripts/check-license-headers.sh...
** ERROR: Unsupported file extension for file (exclude or update this script): Plugins/OpenAPIGenerator/PluginsShared
** Running /code/scripts/run-swift-format.sh...
** ✅ Ran swift-format with no errors.

[MahdiBM]

Had to exclude 2 to directories from check-license-headers.sh. These are the symlinked directories, and the actual files are still checked for headers, as it threw errors for the files that didn't have the headers in the second to last CI run.

read -ra PATHS_TO_CHECK_FOR_LICENSE <<< "$( \
  git -C "${REPO_ROOT}" ls-files -z \
  ":(exclude).gitignore" \
  ":(exclude).spi.yml" \
  ":(exclude).swift-format" \
  ":(exclude).github/*" \
  ":(exclude)CODE_OF_CONDUCT.md" \
  ":(exclude)CONTRIBUTING.md" \
  ":(exclude)CONTRIBUTORS.txt" \
  ":(exclude)LICENSE.txt" \
  ":(exclude)NOTICE.txt" \
  ":(exclude)Package.swift" \
  ":(exclude)Package.resolved" \
  ":(exclude)README.md" \
  ":(exclude)SECURITY.md" \
  ":(exclude)scripts/unacceptable-language.txt" \
  ":(exclude)Tests/PetstoreConsumerTests/Generated" \
  ":(exclude)Tests/OpenAPIGeneratorReferenceTests/Resources/ReferenceSources/*" \
  ":(exclude)docker/*" \
  ":(exclude)**/*.docc/*" \
  ":(exclude)**/.gitignore" \
  ":(exclude)**/Package.swift" \
  ":(exclude)**/Package.resolved" \
  ":(exclude)**/README.md" \
  ":(exclude)**/openapi.yaml" \
  ":(exclude)**/openapi.yml" \
  ":(exclude)**/petstore.yaml" \
  ":(exclude)**/openapi-generator-config.yaml" \
  ":(exclude)**/openapi-generator-config.yml" \
+  ":(exclude)Plugins/OpenAPIGenerator/PluginsShared" \
+  ":(exclude)Plugins/OpenAPIGeneratorCommand/PluginsShared" \
  | xargs -0 \
)"

[czechboy0]

Thanks @MahdiBM for bearing with us during this marathon of a PR 🙂 Just updated from main and will land once CI passes.