-
Notifications
You must be signed in to change notification settings - Fork 349
Adopt upcoming Swift language feature MemberImportVisibility
#803
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
Conversation
28c0940
to
24c3dbd
Compare
#else | ||
@testable import struct ArgumentParser.CompletionShell | ||
#endif | ||
@testable import ArgumentParser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this file need @testable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/Users/allan/Downloads/swift-argument-parser/Tests/ArgumentParserExampleTests/MathExampleTests.swift:252:23: error: 'format' is inaccessible due to 'internal' protection level
expected: shell.format(completions: [
^~~~~~
ArgumentParser.CompletionShell.format:2:15: note: 'format(completions:)' declared here
internal func format(completions: [String]) -> String}
^
/Users/allan/Downloads/swift-argument-parser/Tests/ArgumentParserExampleTests/MathExampleTests.swift:258:25: error: 'shellEnvironmentVariableName' is inaccessible due to 'internal' protection level
CompletionShell.shellEnvironmentVariableName: shell.rawValue
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
ArgumentParser.CompletionShell.shellEnvironmentVariableName:2:21: note: 'shellEnvironmentVariableName' declared here
internal static let shellEnvironmentVariableName: String}
^
/Users/allan/Downloads/swift-argument-parser/Tests/ArgumentParserExampleTests/MathExampleTests.swift:264:23: error: 'format' is inaccessible due to 'internal' protection level
expected: shell.format(completions: [
^~~~~~
ArgumentParser.CompletionShell.format:2:15: note: 'format(completions:)' declared here
internal func format(completions: [String]) -> String}
^
/Users/allan/Downloads/swift-argument-parser/Tests/ArgumentParserExampleTests/MathExampleTests.swift:270:25: error: 'shellEnvironmentVariableName' is inaccessible due to 'internal' protection level
CompletionShell.shellEnvironmentVariableName: shell.rawValue
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
ArgumentParser.CompletionShell.shellEnvironmentVariableName:2:21: note: 'shellEnvironmentVariableName' declared here
internal static let shellEnvironmentVariableName: String}
^
/Users/allan/Downloads/swift-argument-parser/Tests/ArgumentParserExampleTests/MathExampleTests.swift:276:23: error: 'format' is inaccessible due to 'internal' protection level
expected: shell.format(completions: [
^~~~~~
ArgumentParser.CompletionShell.format:2:15: note: 'format(completions:)' declared here
internal func format(completions: [String]) -> String}
^
/Users/allan/Downloads/swift-argument-parser/Tests/ArgumentParserExampleTests/MathExampleTests.swift:281:25: error: 'shellEnvironmentVariableName' is inaccessible due to 'internal' protection level
CompletionShell.shellEnvironmentVariableName: shell.rawValue
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
ArgumentParser.CompletionShell.shellEnvironmentVariableName:2:21: note: 'shellEnvironmentVariableName' declared here
internal static let shellEnvironmentVariableName: String}
^
#if swift(>=6.0) | ||
#if compiler(>=6.0) | ||
internal import ArgumentParserToolInfo | ||
internal import Foundation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im curious what is using foundation from this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/Users/allan/Downloads/swift-argument-parser/Sources/ArgumentParser/Completions/CompletionsGenerator.swift:160:9: error: instance method 'replacingOccurrences(of:with:options:range:)' is not available due to missing import of defining module 'Foundation'
: replacingOccurrences(of: "'", with: "'\\''")
^
/Users/allan/Downloads/swift-argument-parser/Sources/ArgumentParser/Completions/CompletionsGenerator.swift:14:1: note: add import of module 'Foundation'
//internal import Foundation
^
/Users/allan/Downloads/swift-argument-parser/Sources/ArgumentParser/Completions/CompletionsGenerator.swift:165:5: error: instance method 'replacingOccurrences(of:with:options:range:)' is not available due to missing import of defining module 'Foundation'
replacingOccurrences(of: "-", with: "_")
^
/Users/allan/Downloads/swift-argument-parser/Sources/ArgumentParser/Completions/CompletionsGenerator.swift:14:1: note: add import of module 'Foundation'
//internal import Foundation
^
@swift-ci test |
Enable the language feature flag for SE-0444. This instructs the compiler to be stricter about requiring import declarations in order to access member declarations.
All uses of `internal import` were previously garuded with `#if swift(>=6.0)`, presumably to handle differences in compiler support for `internal import`. However, `#if swift(>=)` is not the correct directive to use for this purpose, as it primarily tests the language mode that is enabled. These existing conditionals never evaluated true when building this package since it is not configured to build with Swift 6 enabled. Instead, use `#if compiler(>=6.0)` which correctly predicates the imports on only the compiler version.
24c3dbd
to
33ae082
Compare
@swift-ci test |
@tshortli Quick questions: the SE-0444 doc indicates that If 6.0- compilers won't be affected, would enabling this flag for a project that can be built by Swift 6.0- compilers allow for certain code to compile on 6.1+, but not compile on 6.0- due to clashing implicitly imported symbols? If that's the case, why even enable it now for Swift Argument Parser (SAP)? Would it make sense to enable it only after SAP requires 6.1+ for compilation? Thanks for any insight you can provide. Sorry if I'm mistaken, but I've only enabled an experimental (though not upcoming) feature when all Swift compilers that should be able to compile my code support the feature. |
If the compiler does not recognize the feature identifier specified with
The source code will still be compatible with the older toolchains, because
No, I don't think it makes sense. Practically speaking that requirement would be very far away, and in the meantime we won't get the benefit in build environments that use newer compilers. |
Enable the language feature flag for SE-0444. This instructs the compiler to be stricter about requiring import declarations in order to access member declarations.
Also, replace
#if swift(>=6.0)
guards with#if compiler(>=6.0)
. All uses ofinternal import
were previously guarded with#if swift(>=6.0)
, presumably to handle differences in compiler support forinternal import
. However,#if swift(>=)
is not the correct directive to use for this purpose, as it primarily tests the language mode that is enabled. These existing conditionals never evaluated true when building this package since it is not configured to build with Swift 6 enabled. Instead, use#if compiler(>=6.0)
which correctly predicates the imports on only the compiler version.