Skip to content

Address more code review feedback for the SwiftIfConfig library #2770

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

Merged
merged 3 commits into from
Aug 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Sources/SwiftIfConfig/BuildConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ public enum CanImportVersion {
/// be imported is a complicated task only implemented in the Swift compiler.
/// Therefore, queries are permitted to throw an error to report when they
/// cannot answer a query, in which case this error will be reported to
/// the caller.
/// the caller and the condition will be treated as being "false", so the
/// code covered by the condition will be inactive.
public protocol BuildConfiguration {
/// Determine whether a given custom build condition has been set.
///
Expand Down
11 changes: 7 additions & 4 deletions Sources/SwiftIfConfig/ConfiguredRegions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,18 @@ extension SyntaxProtocol {
/// func f()
/// #elseif B
/// func g()
/// #elseif compiler(>= 12.0)
/// please print the number after 41
/// #endif
/// #else
/// #endif
///
/// If the configuration options `DEBUG` and `B` are provided, but `A` is not,
/// the results will be contain:
/// - Active region for the `#if DEBUG`
/// - Inactive region for the `#if A`
/// - Active region for the `#elseif B`
/// and the compiler version is less than 12.0, the results will be contain:
/// - Active region for the `#if DEBUG`.
/// - Inactive region for the `#if A`.
/// - Active region for the `#elseif B`.
/// - Unparsed region for the `#elseif compiler(>= 12.0)`.
/// - Inactive region for the final `#else`.
public func configuredRegions(
in configuration: some BuildConfiguration
Expand Down
10 changes: 8 additions & 2 deletions Sources/SwiftIfConfig/SyntaxProtocol+IfConfig.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,19 @@ extension SyntaxProtocol {
/// #if DEBUG
/// #if A
/// func f()
/// #elseif B
/// func g()
/// #elseif B
/// func g()
/// #elseif compiler(>= 12.0)
/// please print the number after 41
/// #endif
/// #endif
///
/// a call to `isActive` on the syntax node for the function `g` would return `active` when the
/// configuration options `DEBUG` and `B` are provided, but `A` is not.
///
/// If the compiler version is smaller than 12.0, then `isActive` on any of the tokens within
/// that `#elseif` block would return "unparsed", because that syntax should not (conceptually)
/// be parsed.
public func isActive(
in configuration: some BuildConfiguration
) -> (state: IfConfigRegionState, diagnostics: [Diagnostic]) {
Expand Down
28 changes: 26 additions & 2 deletions Tests/SwiftIfConfigTest/EvaluateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,9 @@ public class EvaluateTests: XCTestCase {
assertIfConfig("canImport(SwiftSyntax, _version: 5.10)", .inactive)
assertIfConfig(#"canImport(SwiftSyntax, _version: "5.9")"#, .active)
assertIfConfig("canImport(SwiftSyntax, _underlyingVersion: 5009)", .active)
assertIfConfig("canImport(SwiftSyntax, _underlyingVersion: 5009.10", .inactive)
assertIfConfig("canImport(SwiftSyntax, _underlyingVersion: 5009.10)", .inactive)
assertIfConfig(
"canImport(SwiftSyntax, _underlyingVersion: 5009.10.5.4.2.3.5",
"canImport(SwiftSyntax, _underlyingVersion: 5009.10.5.4.2.3.5)",
.inactive,
diagnostics: [
DiagnosticSpec(
Expand All @@ -229,6 +229,30 @@ public class EvaluateTests: XCTestCase {
)
]
)
assertIfConfig(
"canImport(SwiftSyntax, _version: 20A301)",
.unparsed,
diagnostics: [
DiagnosticSpec(
message: "'canImport' version check has invalid version '20A301'",
line: 1,
column: 34,
severity: .error
)
]
)
assertIfConfig(
#"canImport(SwiftSyntax, _version: "20A301")"#,
.unparsed,
diagnostics: [
DiagnosticSpec(
message: #"'canImport' version check has invalid version '"20A301"'"#,
line: 1,
column: 34,
severity: .error
)
]
)
}
}

Expand Down