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.

Sign up for GitHub

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

Jump to bottom

Use the same .swift-format in the repo and in the generated code #40

Closed
czechboy0 opened this issue May 25, 2023 · 8 comments
Closed

Use the same .swift-format in the repo and in the generated code #40

czechboy0 opened this issue May 25, 2023 · 8 comments
Labels
area/generator Affects: plugin, CLI, config file. kind/enhacement Improvements to existing feature. size/S Small task. (A couple of hours of work.) status/blocked Waiting for another issue.
Milestone
Post-1.0

Comments

@czechboy0
Copy link
Collaborator

We use swift-format both for formatting the generator code, and the generated code. However, the configuration is duplicated in both places, we should have one source of truth for it instead.
@czechboy0 czechboy0 added area/generator Affects: plugin, CLI, config file. kind/enhacement Improvements to existing feature. size/S Small task. (A couple of hours of work.) labels May 25, 2023
@denil-ct denil-ct mentioned this issue Jun 18, 2023
Closed
@denil-ct
Copy link
Contributor

denil-ct commented Jun 18, 2023 •
edited

I've opened a PR for this. Please have a look.

@czechboy0
Copy link
Collaborator Author

Well, this is good timing: apple/swift-package-manager#6067

Unfortunately, it's only supported in Swift 5.9, so I propose we punt this issue until later, and adopt it when we bump the minimum tools version to 5.9.

@denil-ct
Copy link
Contributor

Sounds good to me. So sometime in the fall when Xcode 15 comes out of beta I assume?

@czechboy0 czechboy0 added the status/blocked Waiting for another issue. label Jun 19, 2023
@czechboy0
Copy link
Collaborator Author

To be determined, but marking as blocked for now. You can close your other PR for now, but we'll definitely appreciate the contribution when Swift OpenAPI Generator requires Swift tools version 5.9!

@czechboy0
Copy link
Collaborator Author

Filed #75 to properly relate the two.

@czechboy0 czechboy0 mentioned this issue Jul 17, 2023
Closed
@czechboy0 czechboy0 mentioned this issue Aug 1, 2023
Merged
3 tasks
@czechboy0 czechboy0 added this to the Post-1.0 milestone Aug 25, 2023
This was referenced Aug 28, 2023
Merged
Closed
czechboy0 added a commit that referenced this issue Aug 30, 2023
@czechboy0
[Generator] Integrate the new URI and String coders (#226)
7ddef95 
[Generator] Integrate the new URI and String coders

### Motivation

Depends on runtime changes from apple/swift-openapi-runtime#45.

Up until now, we relied on a series of marker and helper protocols `_StringConvertible` and `_AutoLosslessStringConvertible` to handle converting between various types and their string representation.

This has been very manual and required a non-trivial amount of work to support any extra type, especially `Date` and generated string enums.

Well, turns out this was an unnecessarily difficult way to approach the problem - we had a better solution available for a long time - `Codable`.

Since all the generated types and all the built-in types we reference are already `Codable`, there is no need to reinvent a way to serialize and deserialize types, and we should just embrace it.

While a JSON encoder and decoder already exists in Foundation, we didn't have one handy for encoding to and from URIs (used by headers, query and path parameters), and raw string representation (using `LosslessStringConvertible`). We created those in the runtime library in PRs apple/swift-openapi-runtime#44 and apple/swift-openapi-runtime#41, and integrated them into our helper functions (which got significantly simplified this way) in apple/swift-openapi-runtime#45.

Out of scope of this PR, but this also opens the door to supporting URL form encoded bodies (#182), multipart (#36), and base64 (#11).

While this should be mostly invisible to our adopters, this refactoring creates space for implementing more complex features and overall simplifies our serialization story.

### Modifications

- Updated the generator to use the new helper functions.
- Updated the article about serialization, shows how we reduced the number of helper functions by moving to `Codable`.
- Set the `lineLength` to 120 on the formatter configuration, it was inconsistent with our `.swift-format` file, and lead to the soundness script trying to update the reference files, but then the reference tests were failing. Since we're planning to sync these in #40, this is a step closer to it, but it means that it's probably best to review this PR's diff with whitespace ignored.

### Result

Now the generated code uses the new helper functions, allowing us to delete all the deprecated helpers in 0.2.0.

### Test Plan

Updated file-based reference, snippet, and unit tests.


Reviewed by: glbrntt

Builds:
     ✔︎ pull request validation (5.8) - Build finished. 
     ✔︎ pull request validation (5.9) - 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. 

#226
@denil-ct
Copy link
Contributor

denil-ct commented Sep 2, 2023

@czechboy0 if #75 is 1.0, can't this be under the same milestone as well?

@czechboy0
Copy link
Collaborator Author

@denil-ct the milestone doesn't mean when it'll land, it means which release it's blocking. It's likely this will land around the same time as #75, the milestone just means this wouldn't be considered a 1.0 blocker.

@czechboy0
Copy link
Collaborator Author

We don't use swift-format as a library anymore, so this is obsolete.

@czechboy0 czechboy0 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/generator Affects: plugin, CLI, config file. kind/enhacement Improvements to existing feature. size/S Small task. (A couple of hours of work.) status/blocked Waiting for another issue.
Projects
None yet
Milestone
Post-1.0
Development

No branches or pull requests
2 participants
@czechboy0 @denil-ct