Skip to content

Add a warning for TARGET_OS_* compilation conditions #2810

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 20, 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
25 changes: 24 additions & 1 deletion Sources/SwiftIfConfig/IfConfigDiagnostic.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ enum IfConfigDiagnostic: Error, CustomStringConvertible {
case ignoredTrailingComponents(version: VersionTuple, syntax: ExprSyntax)
case integerLiteralCondition(syntax: ExprSyntax, replacement: Bool)
case likelySimulatorPlatform(syntax: ExprSyntax)
case likelyTargetOS(syntax: ExprSyntax, replacement: ExprSyntax?)
case endiannessDoesNotMatch(syntax: ExprSyntax, argument: String)
case macabiIsMacCatalyst(syntax: ExprSyntax)
case expectedModuleName(syntax: ExprSyntax)
Expand Down Expand Up @@ -88,6 +89,12 @@ enum IfConfigDiagnostic: Error, CustomStringConvertible {
return
"platform condition appears to be testing for simulator environment; use 'targetEnvironment(simulator)' instead"

case .likelyTargetOS(syntax: _, replacement: let replacement?):
return "'TARGET_OS_*' preprocessor macros are not available in Swift; use '\(replacement)' instead"

case .likelyTargetOS(syntax: _, replacement: nil):
return "'TARGET_OS_*' preprocessor macros are not available in Swift; use 'os(...)' conditionals instead"

case .macabiIsMacCatalyst:
return "'macabi' has been renamed to 'macCatalyst'"

Expand Down Expand Up @@ -122,6 +129,7 @@ enum IfConfigDiagnostic: Error, CustomStringConvertible {
.ignoredTrailingComponents(version: _, syntax: let syntax),
.integerLiteralCondition(syntax: let syntax, replacement: _),
.likelySimulatorPlatform(syntax: let syntax),
.likelyTargetOS(syntax: let syntax, replacement: _),
.endiannessDoesNotMatch(syntax: let syntax, argument: _),
.macabiIsMacCatalyst(syntax: let syntax),
.expectedModuleName(syntax: let syntax),
Expand All @@ -145,7 +153,7 @@ extension IfConfigDiagnostic: DiagnosticMessage {
var severity: SwiftDiagnostics.DiagnosticSeverity {
switch self {
case .compilerVersionSecondComponentNotWildcard, .ignoredTrailingComponents,
.likelySimulatorPlatform, .endiannessDoesNotMatch, .macabiIsMacCatalyst:
.likelySimulatorPlatform, .likelyTargetOS, .endiannessDoesNotMatch, .macabiIsMacCatalyst:
return .warning
default: return .error
}
Expand Down Expand Up @@ -192,6 +200,21 @@ extension IfConfigDiagnostic: DiagnosticMessage {
)
}

// For the likely TARGET_OS_* condition we may have a Fix-It.
if case .likelyTargetOS(let syntax, let replacement?) = self {
return Diagnostic(
node: syntax,
message: self,
fixIt: .replace(
message: SimpleFixItMessage(
message: "replace with '\(replacement)'"
),
oldNode: syntax,
newNode: replacement
)
)
}

// For the targetEnvironment(macabi) -> macCatalyst rename we have a Fix-It.
if case .macabiIsMacCatalyst(syntax: let syntax) = self {
return Diagnostic(
Expand Down
48 changes: 48 additions & 0 deletions Sources/SwiftIfConfig/IfConfigEvaluation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ func evaluateIfConfig(
if let identExpr = condition.as(DeclReferenceExprSyntax.self),
let ident = identExpr.simpleIdentifier?.name
{
if let targetOSDiagnostic = diagnoseLikelyTargetOSTest(at: identExpr, name: ident) {
extraDiagnostics.append(targetOSDiagnostic)
}
// Evaluate the custom condition. If the build configuration cannot answer this query, fail.
return checkConfiguration(at: identExpr) {
(active: try configuration.isCustomConditionSet(name: ident), syntaxErrorsAllowed: false)
Expand Down Expand Up @@ -627,6 +630,51 @@ private func diagnoseLikelySimulatorEnvironmentTest(
return IfConfigDiagnostic.likelySimulatorPlatform(syntax: ExprSyntax(binOp)).asDiagnostic
}

/// If this identifier looks like it is a `TARGET_OS_*` compilation condition,
/// produce a diagnostic that suggests replacing it with the `os(*)` syntax.
///
/// For example, this checks for conditions like:
///
/// ```
/// #if TARGET_OS_IOS
/// ```
///
/// which should be replaced with
///
/// ```
/// #if os(iOS)
/// ```
private func diagnoseLikelyTargetOSTest(
at reference: DeclReferenceExprSyntax,
name: String
) -> Diagnostic? {
let prefix = "TARGET_OS_"
guard name.hasPrefix(prefix) else { return nil }
let osName = String(name.dropFirst(prefix.count))

if unmappedTargetOSNames.contains(osName) {
return IfConfigDiagnostic.likelyTargetOS(syntax: ExprSyntax(reference), replacement: nil).asDiagnostic
}

guard let replacement = targetOSNameMap[osName] else { return nil }

return IfConfigDiagnostic.likelyTargetOS(syntax: ExprSyntax(reference), replacement: replacement).asDiagnostic
}

// TARGET_OS_* macros that don’t have a direct Swift equivalent
private let unmappedTargetOSNames = ["WIN32", "UNIX", "MAC", "IPHONE", "EMBEDDED"]
private let targetOSNameMap: [String: ExprSyntax] = [
"WINDOWS": "os(Windows)",
"LINUX": "os(Linux)",
"OSX": "os(macOS)",
"IOS": "os(iOS)",
"MACCATALYST": "targetEnvironment(macCatalyst)",
"TV": "os(tvOS)",
"WATCH": "os(watchOS)",
"VISION": "os(visionOS)",
"SIMULATOR": "targetEnvironment(simulator)",
]

extension IfConfigClauseSyntax {
/// Fold the operators within an #if condition, turning sequence expressions
/// involving the various allowed operators (&&, ||, !) into well-structured
Expand Down
66 changes: 66 additions & 0 deletions Tests/SwiftIfConfigTest/EvaluateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,72 @@ public class EvaluateTests: XCTestCase {
)
}

func testTargetOS() throws {
assertIfConfig("TARGET_OS_DISHWASHER", .inactive)

assertIfConfig(
"TARGET_OS_IOS",
.inactive,
diagnostics: [
DiagnosticSpec(
message: "'TARGET_OS_*' preprocessor macros are not available in Swift; use 'os(iOS)' instead",
line: 1,
column: 1,
severity: .warning,
fixIts: [
FixItSpec(message: "replace with 'os(iOS)'")
]
)
]
)

assertIfConfig(
"TARGET_OS_OSX",
.inactive,
diagnostics: [
DiagnosticSpec(
message: "'TARGET_OS_*' preprocessor macros are not available in Swift; use 'os(macOS)' instead",
line: 1,
column: 1,
severity: .warning,
fixIts: [
FixItSpec(message: "replace with 'os(macOS)'")
]
)
]
)

assertIfConfig(
"TARGET_OS_MACCATALYST",
.inactive,
diagnostics: [
DiagnosticSpec(
message:
"'TARGET_OS_*' preprocessor macros are not available in Swift; use 'targetEnvironment(macCatalyst)' instead",
line: 1,
column: 1,
severity: .warning,
fixIts: [
FixItSpec(message: "replace with 'targetEnvironment(macCatalyst)'")
]
)
]
)

assertIfConfig(
"TARGET_OS_WIN32",
.inactive,
diagnostics: [
DiagnosticSpec(
message: "'TARGET_OS_*' preprocessor macros are not available in Swift; use 'os(...)' conditionals instead",
line: 1,
column: 1,
severity: .warning
)
]
)
}

func testVersions() throws {
assertIfConfig("swift(>=5.5)", .active)
assertIfConfig("swift(<6)", .active)
Expand Down