Skip to content

Commit 150e569

Browse files
committed
Properly translate import path expressions to an import path
My old hack of taking the text of the whole expression and splitting it on a period was showing its age by failing to reject invalid code. Perform the proper translation, throwing errors if the import path doesn't match the proper A.B.C form.
1 parent f3c3f14 commit 150e569

File tree

3 files changed

+60
-4
lines changed

3 files changed

+60
-4
lines changed

Sources/SwiftIfConfig/IfConfigError.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ enum IfConfigError: Error, CustomStringConvertible {
3333
case likelySimulatorPlatform(syntax: ExprSyntax)
3434
case endiannessDoesNotMatch(syntax: ExprSyntax, argument: String)
3535
case macabiIsMacCatalyst(syntax: ExprSyntax)
36+
case expectedModuleName(syntax: ExprSyntax)
3637

3738
var description: String {
3839
switch self {
@@ -89,6 +90,9 @@ enum IfConfigError: Error, CustomStringConvertible {
8990

9091
case .endiannessDoesNotMatch:
9192
return "unknown endianness for build configuration '_endian' (must be 'big' or 'little')"
93+
94+
case .expectedModuleName:
95+
return "expected module name"
9296
}
9397
}
9498

@@ -110,7 +114,8 @@ enum IfConfigError: Error, CustomStringConvertible {
110114
.integerLiteralCondition(syntax: let syntax, replacement: _),
111115
.likelySimulatorPlatform(syntax: let syntax),
112116
.endiannessDoesNotMatch(syntax: let syntax, argument: _),
113-
.macabiIsMacCatalyst(syntax: let syntax):
117+
.macabiIsMacCatalyst(syntax: let syntax),
118+
.expectedModuleName(syntax: let syntax):
114119
return Syntax(syntax)
115120

116121
case .unsupportedVersionOperator(name: _, operator: let op):

Sources/SwiftIfConfig/IfConfigEvaluation.swift

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -405,9 +405,13 @@ func evaluateIfConfig(
405405
return recordError(.canImportTwoParameters(syntax: ExprSyntax(call)))
406406
}
407407

408-
// FIXME: This is a gross hack. Actually look at the sequence of
409-
// `MemberAccessExprSyntax` nodes and pull out the identifiers.
410-
let importPath = firstArg.expression.trimmedDescription.split(separator: ".")
408+
// Extract the import path.
409+
let importPath: [String]
410+
do {
411+
importPath = try extractImportPath(firstArg.expression)
412+
} catch {
413+
return recordError(error, at: firstArg.expression)
414+
}
411415

412416
// If there is a second argument, it shall have the label _version or
413417
// _underlyingVersion.
@@ -473,6 +477,40 @@ func evaluateIfConfig(
473477
return recordError(.unknownExpression(condition))
474478
}
475479

480+
/// Given an expression with the expected form A.B.C, extract the import path
481+
/// ["A", "B", "C"] from it. Throws an error if the expression doesn't match
482+
/// this form.
483+
private func extractImportPath(_ expression: some ExprSyntaxProtocol) throws -> [String] {
484+
// Member access.
485+
if let memberAccess = expression.as(MemberAccessExprSyntax.self),
486+
let base = memberAccess.base,
487+
let memberName = memberAccess.declName.simpleIdentifier
488+
{
489+
return try extractImportPath(base) + [memberName]
490+
}
491+
492+
// Declaration reference.
493+
if let declRef = expression.as(DeclReferenceExprSyntax.self),
494+
let name = declRef.simpleIdentifier
495+
{
496+
return [name]
497+
}
498+
499+
throw IfConfigError.expectedModuleName(syntax: ExprSyntax(expression))
500+
}
501+
502+
extension DeclReferenceExprSyntax {
503+
/// If this declaration reference is a simple identifier, return that
504+
/// string.
505+
fileprivate var simpleIdentifier: String? {
506+
guard argumentNames == nil else {
507+
return nil
508+
}
509+
510+
return baseName.text
511+
}
512+
}
513+
476514
/// Determine whether the given condition only involves disjunctions that
477515
/// check the given config function against one of the provided values.
478516
///

Tests/SwiftIfConfigTest/EvaluateTests.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,19 @@ public class EvaluateTests: XCTestCase {
345345
)
346346
]
347347
)
348+
349+
assertIfConfig(
350+
"canImport(A(b: 1, c: 2).B.C)",
351+
.unparsed,
352+
diagnostics: [
353+
DiagnosticSpec(
354+
message: "expected module name",
355+
line: 1,
356+
column: 11,
357+
severity: .error
358+
)
359+
]
360+
)
348361
}
349362

350363
func testLikelySimulatorEnvironment() throws {

0 commit comments

Comments
 (0)