Update OpenAPIKit to v4

[iT0ny]

Context

Duplicate of #830. This PR implements the additional changes/fixes for issues surfaced in the original PR

Summary

• Move OpenAPIKit to 4.x
• Refresh handling of null values
• Fix tests

Details

• Bump OpenAPIKit version so we target 4.x, and raise the Yams requirement to 5.1+
• Update wording in testEmitsComplexOpenAPIParsingError and testSchemaWarningsForwardedToGeneratorDiagnostics (Tests/OpenAPIGeneratorCoreTests/Parser/Test_YamsParser.swift, Tests/OpenAPIGeneratorCoreTests/Translator/TypesTranslator/Test_translateSchemas.swift)
• Update checks for null values in Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateRawEnum.swift
• Update OpenAPI.Content.Encoding usage according to the OpenAPIKit v4 migration guide in Tests/OpenAPIGeneratorCoreTests/Translator/TypeAssignment/Test_TypeMatcher.swift

Testing

• ✅ swift test
• CI run

Fixes #821

[czechboy0]

Thank you @iT0ny, which of the changes were required when bumping the OpenAPIKit version, and which ones were nice to have?

[iT0ny]

Thank you @iT0ny, which of the changes were required when bumping the OpenAPIKit version, and which ones were nice to have?

All of them are required, tests mentioned in the PR message were failing. + there was an issue with nil checks after the version bump

[iT0ny]

updated user.email to correctly point to my GitHub email address

[mattpolzin]

Heads up that because of a change that merged into main between when this PR was opened and now, you'll need to require OpenAPIKit v4.3.1 or greater once you've rebased on latest main.

[iT0ny]

Thanks, done! @mattpolzin

[mattpolzin]

@czechboy0 more of a procedural question: what is your versioning strategy around bumping OpenAPIKit major versions?

Some potential considerations:

• Getting to version 4 of OpenAPIKit unlocks the ability to build external dereferencing into the generator, but getting to OpenAPIKit v5 will unlock OAS 3.2.0 features.
• I can't say how quickly I'll be able to get v5 out because OAS 3.2.0 actually represents a pretty substantial list of additions to the spec.

[czechboy0]

TBD, but it's possible we'll do a leap from 3 to 5 directly, as even moving to 4 would require a bit more qualification on my end that I don't have time for right now.

[heckj]

@czechboy0 I nerd sniped myself into this same path, closing the draft PR I created when I realized this was already here. I just tacked on the "semver/patch" label to the PR, but I wasn't sure how we were tracking versions for this when it generates code.

[mattpolzin]

OpenAPIKit v5 is out. https://github.com/mattpolzin/OpenAPIKit/releases/tag/5.0.0.

I wasn't rushing to make v4 migration defunct, of course, but since the preference was to move to v5 prior to releasing a new swift-openapi-generator version, here it is!

[mattpolzin]

@iT0ny here's a patch that fixes the "Compatibility test" CI suite. It's just a few more of the "Inconsistency" -> "Problem" verbiage changes. Just in case you've never applied a patch before, you can copy this (including the newline at the end) and then paste it into a file or pipe it into git apply.

git patch

diff --git a/Tests/OpenAPIGeneratorReferenceTests/CompatabilityTest.swift b/Tests/OpenAPIGeneratorReferenceTests/CompatabilityTest.swift
index b99a4c8..62cd19a 100644
--- a/Tests/OpenAPIGeneratorReferenceTests/CompatabilityTest.swift
+++ b/Tests/OpenAPIGeneratorReferenceTests/CompatabilityTest.swift
@@ -88,7 +88,7 @@ final class CompatibilityTest: XCTestCase {
             "https://raw.githubusercontent.com/discourse/discourse_api_docs/8182f1b21ca62cc9ac85fd3a82cae8304033a672/openapi.yml",
             license: .apache,
             expectedDiagnostics: [
-                "Validation warning: Inconsistency encountered when parsing `OpenAPI Schema`: Found nothing but unsupported attributes..",
+                "Validation warning: Problem encountered when parsing `OpenAPI Schema`: Found nothing but unsupported attributes..",
                 "Multipart request bodies must always be required, but found an optional one - skipping. Mark as `required: true` to get this body generated.",
             ],
             skipBuild: compatibilityTestSkipBuild
@@ -100,15 +100,15 @@ final class CompatibilityTest: XCTestCase {
             "https://raw.githubusercontent.com/github/rest-api-description/322663c9c909974af16363b4dc0873c428bdbe34/descriptions-next/api.github.com/api.github.com.yaml",
             license: .mit,
             expectedDiagnostics: [
-                "Validation warning: Inconsistency encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: array. Specifically, attributes for these other types: [\"object\"].",
-                "Validation warning: Inconsistency encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: string. Specifically, attributes for these other types: [\"object\"].",
-                "Validation warning: Inconsistency encountered when parsing `OpenAPI Schema`: Found nothing but unsupported attributes..",
-                "Schema warning: Inconsistency encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: string. Specifically, attributes for these other types: [\"array\"].",
-                "Validation warning: Inconsistency encountered when parsing `Schema`: A schema contains properties for multiple types of schemas, namely: [\"array\", \"object\"]..",
-                "Validation warning: Inconsistency encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: string. Specifically, attributes for these other types: [\"array\"].",
+                "Validation warning: Problem encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: array. Specifically, attributes for these other types: [\"object\"].",
+                "Validation warning: Problem encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: string. Specifically, attributes for these other types: [\"object\"].",
+                "Validation warning: Problem encountered when parsing `OpenAPI Schema`: Found nothing but unsupported attributes..",
+                "Schema warning: Problem encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: string. Specifically, attributes for these other types: [\"array\"].",
+                "Validation warning: Problem encountered when parsing `Schema`: A schema contains properties for multiple types of schemas, namely: [\"array\", \"object\"]..",
+                "Validation warning: Problem encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: string. Specifically, attributes for these other types: [\"array\"].",
                 "A property name only appears in the required list, but not in the properties map - this is likely a typo; skipping this property.",
-                "Schema warning: Inconsistency encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: array. Specifically, attributes for these other types: [\"object\"].",
-                "Schema warning: Inconsistency encountered when parsing `Schema`: A schema contains properties for multiple types of schemas, namely: [\"array\", \"object\"]..",
+                "Schema warning: Problem encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: array. Specifically, attributes for these other types: [\"object\"].",
+                "Schema warning: Problem encountered when parsing `Schema`: A schema contains properties for multiple types of schemas, namely: [\"array\", \"object\"]..",
                 "Schema \"null\" is not supported, reason: \"schema type\", skipping",
             ],
             skipBuild: true
@@ -121,11 +121,11 @@ final class CompatibilityTest: XCTestCase {
             license: .mit,
             expectedDiagnostics: [
                 "Multipart request bodies must always be required, but found an optional one - skipping. Mark as `required: true` to get this body generated.",
-                "Validation warning: Inconsistency encountered when parsing `OpenAPI Schema`: Found nothing but unsupported attributes..",
-                "Validation warning: Inconsistency encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: string. Specifically, attributes for these other types: [\"array\"].",
+                "Validation warning: Problem encountered when parsing `OpenAPI Schema`: Found nothing but unsupported attributes..",
+                "Validation warning: Problem encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: string. Specifically, attributes for these other types: [\"array\"].",
                 "A property name only appears in the required list, but not in the properties map - this is likely a typo; skipping this property.",
-                "Validation warning: Inconsistency encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: string. Specifically, attributes for these other types: [\"object\"].",
-                "Schema warning: Inconsistency encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: string. Specifically, attributes for these other types: [\"array\"].",
+                "Validation warning: Problem encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: string. Specifically, attributes for these other types: [\"object\"].",
+                "Schema warning: Problem encountered when parsing `OpenAPI Schema`: Found schema attributes not consistent with the type specified: string. Specifically, attributes for these other types: [\"array\"].",
                 "Schema \"null\" is not supported, reason: \"schema type\", skipping",
             ],
             skipBuild: true

[mattpolzin]

@iT0ny to add a bit of context for what's going on around this PR and the one I am preparing now:

Honza and Si have agreed to merge both migration to OpenAPIKit v4 and OpenAPIKit v5 at the same time. I have based migration to v5 off of this branch. If we can get CI green on this branch then we should be in good shape to request code review on both your branch and mine and get that ball rolling again.

[iT0ny]

@mattpolzin hey, thanks for letting me know, I'll try to update the PR ASAP

[mattpolzin]

I feel bad not using this branch what with it being real effort on @iT0ny's part, but I also want to keep in mind that (as far as I know) there are no remaining blockers to moving to OpenAPIKit 5 other than getting this PR's CI to go green and getting maintainer review of this PR and the follow-on PR I created.

Perhaps @simonjbeaumont could push the patch I pasted in my previous comment to this branch as a maintainer and we could get things moving? I know this might be pretty inconvenient given other ongoing work, I'm just doing my part to suggest a path forward. If that doesn't happen, no big deal.

[iT0ny]

@mattpolzin hey, just pushed the changes, thank you for waiting. sorry it took so long, got caught up with irl stuff

[simonjbeaumont]

First off, thank you to both of you for helping push this project forwards. Your contributions are very welcome.

From my perspective I'd like to review something that takes us from where we are to OpenAPIKit v5 as there's little benefit to investing a lot of time reviewing an intermediate step.

I'm sympathetic though that this has been a combined effort and you both deserve credit for all the hard work. IIUC the v5 PR is currently based off this branch, right? So I'm pretty sure the merge would attribute both authors. Does anyone object to us working off a unified PR?

I'm also heading out on vacation so won't be able to get to this for a couple of weeks anyway.

[mattpolzin]

No worries. I appreciate the follow-up.

[mattpolzin]

@simonjbeaumont understood. I'll rebase my work off this branch once more and then my only request would be that you kick CI off before leaving if you have time so I know if we're all green or not.

If you don't get around to kicking CI off, I'll survive ;)

[mattpolzin]

Of course, I am currently working through a merge conflict on my OpenAPIKit 5 branch, so not ready to run CI there yet at the moment regardless.

[mattpolzin]

I know there's PTO going on for maintainers right now, but just one (final?) update; I've rebased and updated the OpenAPIKit v5 migration branch and marked the PR ready for review. It remains based off of the work done in this PR and retains commit attribution for iT0ny for the v4 work. Merging that PR will suffice to upgrade through the two major versions of OpenAPIKit (v4/v5).