Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update swift-format version to 5.9 #175

Closed
takeshi-1000 opened this issue Aug 7, 2023 · 10 comments
Closed

Update swift-format version to 5.9 #175

takeshi-1000 opened this issue Aug 7, 2023 · 10 comments
Labels
area/generator Affects: plugin, CLI, config file. kind/enhancement Improvements to existing feature. size/S Small task. (A couple of hours of work.)
Milestone

Comments

@takeshi-1000
Copy link
Contributor

takeshi-1000 commented Aug 7, 2023

It seems that the following source code of Petstore was not properly formatted.

deserializer: { request, metadata in let path: Operations.listPets.Input.Path = .init()

The source code inside swift-openapi-generator seems fine and I've confirmed that if I raise the swift-format version to the current commit revision, it's formatted properly. There seems to be no release version currently available on the swift-format side, so I open this issue.

If you want to check, change the code as below,

--- a/Package.swift
+++ b/Package.swift
@@ -52,7 +52,7 @@ let package = Package(
         // Format Swift code
         .package(
             url: "https://github.com/apple/swift-format.git",
-            from: "508.0.1"
+            revision: "8337523533bbd153cdac74c8fff4a1ac79a0b805"
         ),
 
         // General algorithms

and when you build it, an error will occur due to the change in the swift-format interface. (around here)

/path/to/swift-openapi-generator/Sources/_OpenAPIGeneratorCore/Extensions/SwiftFormat.swift:44:100: error: value of type 'SourceLocation' has no member 'debugDescription'
                    Formatting the following code produced diagnostic at location \(sourceLocation.debugDescription) (see end):

If you change the above code appropriately so that the build passes, and run the test, the output will be as follows.

diff --git a/ReferenceSources/Petstore/Server.swift b/var/folders/nn/80g9wmmd37lc63c9wqtnspj40000gp/T/5705E10F-715B-4E33-8295-2090E9E2FA85/Server.swift
index cd60e0c..fc5d004 100644
--- a/ReferenceSources/Petstore/Server.swift
+++ b/var/folders/nn/80g9wmmd37lc63c9wqtnspj40000gp/T/5705E10F-715B-4E33-8295-2090E9E2FA85/Server.swift
@@ -81,11 +81,12 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol {
         try await handle(
             request: request,
             with: metadata,
             forOperation: Operations.listPets.id,
             using: { APIHandler.listPets($0) },
-            deserializer: { request, metadata in let path: Operations.listPets.Input.Path = .init()
+            deserializer: { request, metadata in
+                let path: Operations.listPets.Input.Path = .init()
                 let query: Operations.listPets.Input.Query = .init(
                     limit: try converter.getOptionalQueryItemAsText(
                         in: metadata.queryParameters,
                         name: "limit",
                         as: Swift.Int32.self
@@ -180,11 +181,12 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol {
         try await handle(
             request: request,
             with: metadata,
             forOperation: Operations.createPet.id,
             using: { APIHandler.createPet($0) },
-            deserializer: { request, metadata in let path: Operations.createPet.Input.Path = .init()
+            deserializer: { request, metadata in
+                let path: Operations.createPet.Input.Path = .init()
                 let query: Operations.createPet.Input.Query = .init()
                 let headers: Operations.createPet.Input.Headers = .init(
                     X_Extra_Arguments: try converter.getOptionalHeaderFieldAsJSON(
                         in: request.headerFields,
                         name: "X-Extra-Arguments",
@@ -273,11 +275,12 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol {
         try await handle(
             request: request,
             with: metadata,
             forOperation: Operations.getStats.id,
             using: { APIHandler.getStats($0) },
-            deserializer: { request, metadata in let path: Operations.getStats.Input.Path = .init()
+            deserializer: { request, metadata in
+                let path: Operations.getStats.Input.Path = .init()
                 let query: Operations.getStats.Input.Query = .init()
                 let headers: Operations.getStats.Input.Headers = .init()
                 let cookies: Operations.getStats.Input.Cookies = .init()
                 return Operations.getStats.Input(
                     path: path,
@@ -317,11 +320,12 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol {
         try await handle(
             request: request,
             with: metadata,
             forOperation: Operations.postStats.id,
             using: { APIHandler.postStats($0) },
-            deserializer: { request, metadata in let path: Operations.postStats.Input.Path = .init()
+            deserializer: { request, metadata in
+                let path: Operations.postStats.Input.Path = .init()
                 let query: Operations.postStats.Input.Query = .init()
                 let headers: Operations.postStats.Input.Headers = .init()
                 let cookies: Operations.postStats.Input.Cookies = .init()
                 let contentType = converter.extractContentTypeIfPresent(in: request.headerFields)
                 let body: Operations.postStats.Input.Body
@@ -367,11 +371,12 @@ fileprivate exte
nsion UniversalServer where APIHandler: APIProtocol {
         try await handle(
             request: request,
             with: metadata,
             forOperation: Operations.probe.id,
             using: { APIHandler.probe($0) },
-            deserializer: { request, metadata in let path: Operations.probe.Input.Path = .init()
+            deserializer: { request, metadata in
+                let path: Operations.probe.Input.Path = .init()
                 let query: Operations.probe.Input.Query = .init()
                 let headers: Operations.probe.Input.Headers = .init()
                 let cookies: Operations.probe.Input.Cookies = .init()
                 return Operations.probe.Input(
                     path: path,
diff --git a/ReferenceSources/Petstore_FF_MultipleContentTypes/Server.swift b/var/folders/nn/80g9wmmd37lc63c9wqtnspj40000gp/T/38A9C041-0D2B-4E2C-B842-F2A186367B2F/Server.swift
index 1a3753f..ffc0365 100644
--- a/ReferenceSources/Petstore_FF_MultipleContentTypes/Server.swift
+++ b/var/folders/nn/80g9wmmd37lc63c9wqtnspj40000gp/T/38A9C041-0D2B-4E2C-B842-F2A186367B2F/Server.swift
@@ -81,11 +81,12 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol {
         try await handle(
             request: request,
             with: metadata,
             forOperation: Operations.listPets.id,
             using: { APIHandler.listPets($0) },
-            deserializer: { request, metadata in let path: Operations.listPets.Input.Path = .init()
+            deserializer: { request, metadata in
+                let path: Operations.listPets.Input.Path = .init()
                 let query: Operations.listPets.Input.Query = .init(
                     limit: try converter.getOptionalQueryItemAsText(
                         in: metadata.queryParameters,
                         name: "limit",
                         as: Swift.Int32.self
@@ -180,11 +181,12 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol {
         try await handle(
             request: request,
             with: metadata,
             forOperation: Operations.createPet.id,
             using: { APIHandler.createPet($0) },
-            deserializer: { request, metadata in let path: Operations.createPet.Input.Path = .init()
+            deserializer: { request, metadata in
+                let path: Operations.createPet.Input.Path = .init()
                 let query: Operations.createPet.Input.Query = .init()
                 let headers: Operations.createPet.Input.Headers = .init(
                     X_Extra_Arguments: try converter.getOptionalHeaderFieldAsJSON(
                         in: request.headerFields,
                         name: "X-Extra-Arguments",
@@ -273,11 +275,12 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol {
         try await handle(
             request: request,
             with: metadata,
             forOperation: Operations.getStats.id,
             using: { APIHandler.getStats($0) },
-            deserializer: { request, metadata in let path: Operations.getStats.Input.Path = .init()
+            deserializer: { request, metadata in
+                let path: Operations.getStats.Input.Path = .init()
                 let query: Operations.getStats.Input.Query = .init()
                 let headers: Operations.getStats.Input.Headers = .init()
                 let cookies: Operations.getStats.Input.Cookies = .init()
                 return Operations.getStats.Input(
                     path: path,
@@ -337,11 +340,12 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol {
         try await handle(
             request: request,
             with: metadata,
             forOperation: Operations.postStats.id,
             using: { APIHandler.postStats($0) },
-            deserializer: { request, metadata in let path: Operations.postStats.Input.Path = .init()
+            deserializer: { request, metadata in
+                let path: Operations.postStats.Input.Path = .init()
                 let query: Operations.postStats.Input.Query = .init()
                 let headers: Operations.postStats.Input.Headers = .init()
                 let cookies: Operations.postStats.Input.Cookies = .init()
                 let contentType = converter.extractContentTypeIfPresent(in: request.headerFields)
let body: Operations.postStats.Input.Body
@@ -405,11 +409,12 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol {
         try await handle(
             request: request,
             with: metadata,
             forOperation: Operations.probe.id,
             using: { APIHandler.probe($0) },
-            deserializer: { request, metadata in let path: Operations.probe.Input.Path = .init()
+            deserializer: { request, metadata in
+                let path: Operations.probe.Input.Path = .init()
                 let query: Operations.probe.Input.Query = .init()
                 let headers: Operations.probe.Input.Headers = .init()
                 let cookies: Operations.probe.Input.Cookies = .init()
                 return Operations.probe.Input(
                     path: path,

Please note that the above behavior is the behavior of the current main branch of swift-format and is subject to change.

Thank you.

@czechboy0
Copy link
Contributor

Hi @takeshi-1000, this is a great catch - I don't personally like how it puts the code on the first line of the closure. Happy to hear it's fixed.

Marking the issue as blocked, and we'll update to the new version of swift-format once Swift 5.9 is released.

@czechboy0 czechboy0 added 🔨 semver/patch No public API change. area/generator Affects: plugin, CLI, config file. kind/enhancement Improvements to existing feature. size/S Small task. (A couple of hours of work.) status/blocked Waiting for another issue. labels Aug 8, 2023
@czechboy0 czechboy0 changed the title Update swift-format version to properly format PetStore Server.swift Update swift-format version to 5.9 Aug 8, 2023
@czechboy0 czechboy0 added this to the 1.0 milestone Aug 25, 2023
@czechboy0 czechboy0 removed the 🔨 semver/patch No public API change. label Sep 8, 2023
@czechboy0 czechboy0 modified the milestones: 1.0, 0.3.0 Sep 13, 2023
@czechboy0 czechboy0 modified the milestones: 0.3.0, 1.0 Sep 25, 2023
@missingmario
Copy link

@czechboy0

Hey! This is my first time interacting with the Open Source community on GitHub, so please let me know if I make any mistakes and/or come across as rude/annoying.

It looks like swift-format 509.0.0 is available as of last week.

I was playing around with swift-format 5.9 on my own project; and found out that having the swift-openapi-generator plugin active caused a conflict due to major version requirements of the package.

This issue seems to be related to #250 as well.

I would like to help if possible, would it be okay if I tried increasing the versions and opening a pull request?

Thank you for your time.

@czechboy0
Copy link
Contributor

Hi @missingmario, welcome!

We will require 5.9 at some point in the coming months, but aren't ready to do it yet.

Can you speak to how this is impacting your project? The generator uses the from: "508.0.1" version requirement, so 509.0.0 should still be able to resolve. What's the error you're seeing?

@missingmario
Copy link

missingmario commented Sep 25, 2023

Thank you @czechboy0 for the quick response!

I have a swift package which I use inside for my app: MyProjectNetwork.

I also have a separate swift package on a separate git repo (swift-myproject-openapi-client) which uses the generator as show in the code examples in this repo.

I then added both swift-myproject-openapi-client and swift-format (from: 509.0.0) as dependencies to MyProjectNetwork. Xcode shows me the following error:

Failed to resolve dependencies Dependencies could not be resolved because 'swift-myproject-openapi-client' depends on 'swift-openapi-generator' 0.2.2.
'swift-openapi-generator' 0.2.2 cannot be used because 'swift-openapi-generator' 0.2.2 depends on 'swift-format' 508.0.1..<509.0.0 and 'myprojectnetwork' depends on 'swift-format' 509.0.0..<510.0.0.

This makes sense to me as swift-format 508.y.z and 509.y.z are major versions and therefore may contain breaking changes according to SemVer.

@czechboy0
Copy link
Contributor

You're totally right, I didn't realize. Unfortunately, until we can bump to 5.9, you'll need to use swift-format 5.8.

@missingmario
Copy link

Oh, I'll wait then.

There seems to be a workaround to use swift-format 509 if the generator is used inside a separate package: using the OpenAPIGeneratorCommand instead of the OpenAPIGenerator plugin and committing the generated files to source control prevents the swift-format 508 requirement from being passed on to the main project, allowing you to use swift-format 509 if needed.

This might help anyone with the same problem as me.

Thank you for your help!

@czechboy0
Copy link
Contributor

Indeed, using ahead-of-time generation keeps the generator's dependencies out of your package's dependencies: https://swiftpackageindex.com/apple/swift-openapi-generator/0.2.2/documentation/swift-openapi-generator/manually-invoking-the-generator-cli

@czechboy0
Copy link
Contributor

Already done in #331.

@czechboy0 czechboy0 reopened this Oct 23, 2023
@czechboy0
Copy link
Contributor

Actually, we haven't yet bumped the repo itself to swift-format 5.9, reopening.

@czechboy0 czechboy0 removed the status/blocked Waiting for another issue. label Oct 23, 2023
czechboy0 added a commit that referenced this issue Oct 23, 2023
Bump swift-format to 5.9

### Motivation

As per #175.

### Modifications

Updated to swift-format 5.9, updated code to pass soundness.

### Result

Using swift-format 5.9.

### Test Plan

Soundness passed.


Reviewed by: dnadoba

Builds:
     ✔︎ pull request validation (5.10) - Build finished. 
     ✔︎ pull request validation (5.8) - Build finished. 
     ✔︎ pull request validation (5.9) - Build finished. 
     ✔︎ pull request validation (compatibility test) - Build finished. 
     ✔︎ pull request validation (docc test) - Build finished. 
     ✔︎ pull request validation (integration test) - Build finished. 
     ✔︎ pull request validation (nightly) - Build finished. 
     ✔︎ pull request validation (soundness) - Build finished. 

#340
czechboy0 added a commit to swift-server/swift-openapi-async-http-client that referenced this issue Oct 24, 2023
Bump swift-format to 5.9

### Motivation

As per apple/swift-openapi-generator#175.

### Modifications

Bump swift-format, update to make soundness pass.

### Result

Using swift-format 5.9.

### Test Plan

Soundness passed.


Reviewed by: dnadoba

Builds:
     ✔︎ pull request validation (5.10) - Build finished. 
     ✔︎ pull request validation (5.8) - Build finished. 
     ✔︎ pull request validation (5.9) - Build finished. 
     ✔︎ pull request validation (nightly) - Build finished. 
     ✔︎ pull request validation (soundness) - Build finished. 

#20
czechboy0 added a commit to apple/swift-openapi-urlsession that referenced this issue Oct 24, 2023
Bump swift-format to 5.9

### Motivation

As per apple/swift-openapi-generator#175.

### Modifications

Bumped swift-format for CI, no other changes needed.

### Result

Using swift-format 5.9.

### Test Plan

Soundness passed.


Reviewed by: dnadoba

Builds:
     ✔︎ pull request validation (5.10) - Build finished. 
     ✔︎ pull request validation (5.8) - Build finished. 
     ✔︎ pull request validation (5.9) - Build finished. 
     ✔︎ pull request validation (nightly) - Build finished. 
     ✔︎ pull request validation (soundness) - Build finished. 

#17
@czechboy0
Copy link
Contributor

Done now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/generator Affects: plugin, CLI, config file. kind/enhancement Improvements to existing feature. size/S Small task. (A couple of hours of work.)
Projects
None yet
Development

No branches or pull requests

3 participants