Add access modifiers to generated imports#883
Conversation
simonjbeaumont
left a comment
There was a problem hiding this comment.
Thanks for taking the time to open the PR.
I'm sympathetic to the problem and open to a solution here. But I think this has been solved at the wrong layer.
The PR currently does this at the concrete implementation of a renderer and just unilaterally updates all imports to use a given access modifier.
This needs to be done in the types that conform to FileTranslator, e.g. see ClientFileTranslator, which is where the structured Swift representation is determined. You'll see there how access modifiers are used to generate the declarations in the file and where the import statements are defined.
This allows for two things, which we'll need before we can land this PR:
- Other renderer implementations that may come in the future will get consistent behaviour because we removed the layering violation.
- We can treat the imports used for the generated API differently from the
additionalImports.
d4b05f7 to
7d9b310
Compare
|
Support access modifiers on generated imports MotivationWith the introduction of Swift 6.0's Modifications
ResultGenerated Swift files can now properly emit Test PlanUnit tests added in |
|
@arpit-garg-1995 just a heads up that this package only supports Swift 6.1+ now so you don't need to do anything special in the generator to handle older compilers. |
Support access modifiers on generated imports ### Motivation With the introduction of Swift 6.0's `InternalImportsByDefault` flag, generated declarations need a way to re-export the symbols they depend on. Supporting the configured access modifier on imports ensures they remain visible to consumers of the generated code when necessary. ### Modifications - Add `accessModifier` property to `ImportDescription`. - Update `TextBasedRenderer` to conditionally render `public` and `package` access modifiers on imports, wrapped within an `#if compiler(>=6.0)` block to maintain compatibility with older compilers. - Introduce `importDescriptions(adding:)` in `FileTranslator` to automatically propagate the configured access modifier to both built-in and additional imports. - Adopt the new import generation logic in `ClientFileTranslator`, `ServerFileTranslator`, and `TypesFileTranslator`. - Add unit tests in `Test_TextBasedRenderer` to verify the correct rendering of imports with different access modifiers, attributes, and OS conditions. ### Result Generated Swift files can now properly emit `public import` or `package import` statements when compiled with Swift `6.0` and above, based on the provided configuration. ### Test Plan Unit tests added in `Test_TextBasedRenderer`.
7d9b310 to
7b8907f
Compare
|
Addressed the latest feedback in this commit. Modifications
ResultThis simplifies the generator logic and aligns it with the package’s current Swift version support. Test Plan
@swift-server-bot test this please |
Are you sure you got the tests passing locally? They're failing for every Swift version and platform in CI.
Just FYI — this does nothing. |
|
Let me check and confirm. Will update soon. |
### MotivationStarting in Swift 6.0, users can enable the
InternalImportsByDefaultupcoming feature flag, which makes all importsinternalby default. However, when users generate code and specify anaccessModifierofpackageorpublicin their swift-openapi-generator configuration, the generated code fails to build. This occurs because the generated public/package declarations are trying to expose types from modules that are only imported internally. We need a way to cascade the appropriate access modifiers down to the imports so the generated code compiles successfully under Swift 6.0.Modifications
Modified
TextBasedRenderer.swiftto propagate the configuredaccessModifierwhen generating import statements.accessModifieris either.publicor.package, the generator now wraps the import statement in an#if compiler(>=6.0)conditional compilation block.public import Foo).#elsebranch is provided to output the plain import statement (e.g.,import Foo) for older compilers, thereby ensuring backward compatibility without requiring additional feature flags.@preconcurrencyand@_spi) so they are correctly attached regardless of the compiler version.Result
After these changes, code generated with an access modifier of
publicorpackagewill compile successfully in Swift 6.0 when theInternalImportsByDefaultflag is enabled. The imports will correctly match the access level of the generated code, preventing module visibility mismatch errors, while seamlessly supporting developers on older Swift versions.Test Plan
Test_TextBasedRenderer.swift(testImportsWithAccessModifiers) to verify that import statements correctly render the#if compiler(>=6.0)blocks when.publicand.packagemodifiers are supplied.@preconcurrency,@_spi, and specific struct imports likestruct Foundation.URL) render their attributes correctly in both the#ifand#elsebranches.accessModifier: publicin theopenapi-generator-config.yaml, and build the resulting Swift package using the Swift 6 compiler with theInternalImportsByDefaultfeature flag enabled to ensure no build warnings or errors are thrown.Testing with Act: Passed local medium act pull_request workflow