Repository navigation
Conversation
czechboy0
left a comment
czechboy0
left a comment
There was a problem hiding this comment.
In Swift Crypto, they add the SourceWarningControl experimental feature in Package.swift, but in this PR we don't: apple/swift-crypto@15b0fb5#diff-f913940c58e8744a2af1c68b909bb6383e49007e6c5a12fb03104a9006ae677eR44
Can you check if it's required or not to make it correctly suppress the warnings?
## Summary Moves swift-openapi-generator from 1.10.2 to **1.13.0** and makes `make generate` notice when the pinned version changes. **Generator upgrade** | | Before | After | Why | |---|---|---|---| | Generator (`Makefile`) | 1.10.2 | **1.13.0** | Four minor versions behind | | swift-openapi-runtime floor (`Package.swift`) | 1.8.2 | **1.11.0** | The generator's own minimum since 1.12.0 | | Generator cache keys (`ci.yml`, `linux.yml`, `macos.yml`) | `1.10.2` | `1.13.0` | So CI builds the new generator | `Package.resolved` doesn't change. It already had runtime 1.12.1 from #35. OpenAPIKit v6, which arrived in 1.12.1 with stricter validation, accepts the spec with no warnings or errors. Apart from two changes, the generated code is the same as 1.10.2's: every import is now `public` (1.13.0, following `accessModifier`), and `Encoder`, `Decoder` and `Error` are written as `Swift.Encoder` and so on (1.11.0). **Why not 1.13.1:** its namespace file splitting is always on. With `accessModifier: public`, it produces 24 `UnusedImportAccess` warnings from imports that the small split files never use. The issue is tracked upstream in apple/swift-openapi-generator#946, and the fix, apple/swift-openapi-generator#947, is unreleased. Apart from the splitting, 1.13.1 only adds a clearer error for duplicate schema names. Worth bumping again once #947 ships. **Makefile: re-clone on a version change** Before this, the clone target had no prerequisites, so changing `SWIFT_OPENAPI_GENERATOR_GIT_TAG` left any existing clone in place and `make generate` kept running the old generator without saying so. The clone is now keyed on a `.cloned-<tag>` stamp file inside it. If the stamp is missing, `make generate` deletes the clone and `Sources/FreeAgentAPI/Generated`, then re-clones and rebuilds. An existing clone without a stamp is refreshed once. `Generated/` is cleared too because the generator never deletes files it no longer produces. Switching from 1.13.1 back to 1.13.0 left 1.13.1's split files behind, and the build failed with "invalid redeclaration of 'Components'". ## Testing - macOS: 92 API and 25 CLI unit tests pass, `make format` and `make lint` clean. Built with 1.13.0 and 1.13.1 and counted warnings from generated code: none with 1.13.0, 24 with 1.13.1. - `make generate` checked for real, not just dry runs: an existing unstamped clone refreshes once, an unchanged tag deletes and rebuilds nothing, and 1.13.0 -> 1.13.1 -> 1.13.0 leaves the right generator and only its own files each time, and the build passes. - Integration suites not run. They were skipped locally, and CI's Integration Tests job skips them because `openapi/` is unchanged. The generated code differs only in import modifiers and name qualification, so the requests and responses on the wire can't change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The generator emits the same imports into every file, so an access-level import is often unused in any given file, which triggers UnusedImportAccess and breaks builds that treat warnings as errors. Access-level imports now carry @diagnose behind hasFeature(SourceWarningControl), so output is unchanged on compilers without SE-0522.
5f184f9 to
6612af1
Compare
|
Hey @czechboy0 , sorry for the late reply! I don't think it's required. The Swift 6.4
So the flag was only needed while it was in review, and that swift-crypto line is probably just left over. I tried it locally without the flag too, and the unused import still errors without I've also rebased on main. Could you approve the workflow run when you get a chance or redirect to right approver? CI hasn't run on this yet. Thanks |
|
Hi @tirdesh, we still support generating code for Swift 6.2+, so we either need to guard the annotated imports with a Swift compiler version check, or this feature check. |
|
I'll be the approver here, I'll run the CI once the code looks ready 🙂 |
I've gone with the feature check, lmk if you want me to switch to the version check or any other changes to be done. Thanks for reviewing this!! |
|
@tirdesh Actually let's switch it to a Swift version check, so that we can remove it easily once Swift 6.6 ships. |
3 participants
Motivation
Fixes #946.
The same imports go into every generated file, so an access-level import is often unused in a
given one. Since 1.13.1 changed the file layout, that surfaces as:
which breaks any package built with
-warnings-as-errors.Modifications
The renderer emits
@diagnose(UnusedImportAccess, as: ignored)for access-level importsbehind
#if hasFeature(SourceWarningControl), following the swift-crypto pattern@simonjbeaumont pointed to on the issue. Only an access-level import can trip the diagnostic,
so the attribute follows the access modifier.
Result
On a compiler with SE-0522 the warning is gone, and nothing in the adopter's package has to
change. Older compilers take the
#elsebranch and get the imports they get today.Test Plan
swift test: 334 tests, 0 failures. The new test fails before the change, and the Petstorereferences are regenerated rather than hand-edited.
Both branches are checked against real toolchains.
hasFeature(SourceWarningControl)isfalse on 6.3.3, so released compilers take the
#elsebranch. Onswift-DEVELOPMENT-SNAPSHOT-2026-08-30-ait is true, and there an unusedpackage importunder
-warnings-as-errorsfails with the error above while the same import carrying theattribute compiles.