Fix handling of multi-line XCConfig assignments (#559)

neonichu · web-flow · commit b78e04ad7c35 · 2025-06-04T18:31:57.000-07:00
Originally only one line was tracked here (the end line effectively), but we also have to track the start line separately for correct multi-line ranges.
diff --git a/Sources/SWBCore/MacroConfigFileLoader.swift b/Sources/SWBCore/MacroConfigFileLoader.swift
@@ -242,7 +242,7 @@ final class MacroConfigFileLoader: Sendable {
                 return MacroConfigFileParser(byteString: data, path: path, delegate: delegate)
             }
 
-            mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, line: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {
+            mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {
                 // Look up the macro name, creating it as a user-defined macro if it isn’t already known.
                 let macro = table.namespace.lookupOrDeclareMacro(UserDefinedMacroDeclaration.self, macroName)
 
@@ -253,7 +253,7 @@ final class MacroConfigFileLoader: Sendable {
                 }
 
                 // Parse the value in a manner consistent with the macro definition.
-                let location = MacroValueAssignmentLocation(path: path, line: line, startColumn: startColumn, endColumn: endColumn)
+                let location = MacroValueAssignmentLocation(path: path, startLine: startLine, endLine: endLine, startColumn: startColumn, endColumn: endColumn)
                 table.push(macro, table.namespace.parseForMacro(macro, value: value), conditions: conditionSet, location: location)
             }
 
diff --git a/Sources/SWBMacro/MacroConfigFileParser.swift b/Sources/SWBMacro/MacroConfigFileParser.swift
@@ -362,6 +362,7 @@ public final class MacroConfigFileParser {
         // Skip over the equals sign.
         assert(currChar == /* '=' */ 61)
         advance()
+        let startLine = currLine
         let startColumn = currIdx - startOfLine
 
         var chunks : [String] = []
@@ -385,7 +386,7 @@ public final class MacroConfigFileParser {
         }
         // Finally, now that we have the name, conditions, and value, we tell the delegate about it.
         let value = chunks.joined(separator: " ")
-        delegate?.foundMacroValueAssignment(name, conditions: conditions, value: value, path: path, line: currLine, startColumn: startColumn, endColumn: currIdx - startOfLine, parser: self)
+        delegate?.foundMacroValueAssignment(name, conditions: conditions, value: value, path: path, startLine: startLine, endLine: currLine, startColumn: startColumn, endColumn: currIdx - startOfLine, parser: self)
     }
 
     public func parseNonListAssignmentRHS() -> String? {
@@ -520,7 +521,7 @@ public final class MacroConfigFileParser {
             }
             func endPreprocessorInclusion() {
             }
-            func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, line: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {
+            func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {
                 self.macroName = macroName
                 self.conditions = conditions.isEmpty ? nil : conditions
             }
@@ -567,7 +568,7 @@ public protocol MacroConfigFileParserDelegate {
     func endPreprocessorInclusion()
 
     /// Invoked once for each macro value assignment.  The `macroName` is guaranteed to be non-empty, but `value` may be empty.  Any macro conditions are passed as tuples in the `conditions`; parameters are guaranteed to be non-empty strings, but patterns may be empty.
-    mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, line: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser)
+    mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser)
 
     /// Invoked if an error, warning, or other diagnostic is detected.
     func handleDiagnostic(_ diagnostic: MacroConfigFileDiagnostic, parser: MacroConfigFileParser)
diff --git a/Sources/SWBMacro/MacroValueAssignmentTable.swift b/Sources/SWBMacro/MacroValueAssignmentTable.swift
@@ -337,7 +337,8 @@ public final class MacroValueAssignment: Serializable, CustomStringConvertible,
         if let _location {
             return .init(
                 path: Self.macroConfigPaths.withLock { $0[_location.pathRef] },
-                line: _location.line,
+                startLine: _location.startLine,
+                endLine: _location.endLine,
                 startColumn: _location.startColumn,
                 endColumn: _location.endColumn
             )
@@ -355,7 +356,8 @@ public final class MacroValueAssignment: Serializable, CustomStringConvertible,
         if let location {
             self._location = InternedMacroValueAssignmentLocation(
                 pathRef: Self.macroConfigPaths.withLock({ $0.append(location.path).index }),
-                line: location.line,
+                startLine: location.startLine,
+                endLine: location.endLine,
                 startColumn: location.startColumn,
                 endColumn: location.endColumn
             )
@@ -432,44 +434,50 @@ public final class MacroValueAssignment: Serializable, CustomStringConvertible,
 
 public struct MacroValueAssignmentLocation: Sendable, Equatable {
     public let path: Path
-    public let line: Int
+    public let startLine: Int
+    public let endLine: Int
     public let startColumn: Int
     public let endColumn: Int
 
-    public init(path: Path, line: Int, startColumn: Int, endColumn: Int) {
+    public init(path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int) {
         self.path = path
-        self.line = line
+        self.startLine = startLine
+        self.endLine = endLine
         self.startColumn = startColumn
         self.endColumn = endColumn
     }
 }
 
 private struct InternedMacroValueAssignmentLocation: Serializable, Sendable {
     let pathRef: OrderedSet<Path>.Index
-    let line: Int
+    public let startLine: Int
+    public let endLine: Int
     let startColumn: Int
     let endColumn: Int
 
-    init(pathRef: OrderedSet<Path>.Index, line: Int, startColumn: Int, endColumn: Int) {
+    init(pathRef: OrderedSet<Path>.Index, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int) {
         self.pathRef = pathRef
-        self.line = line
+        self.startLine = startLine
+        self.endLine = endLine
         self.startColumn = startColumn
         self.endColumn = endColumn
     }
 
     public func serialize<T>(to serializer: T) where T : SWBUtil.Serializer {
-        serializer.beginAggregate(4)
+        serializer.beginAggregate(5)
         serializer.serialize(pathRef)
-        serializer.serialize(line)
+        serializer.serialize(startLine)
+        serializer.serialize(endLine)
         serializer.serialize(startColumn)
         serializer.serialize(endColumn)
         serializer.endAggregate()
     }
 
     public init(from deserializer: any SWBUtil.Deserializer) throws {
-        try deserializer.beginAggregate(4)
+        try deserializer.beginAggregate(5)
         self.pathRef = try deserializer.deserialize()
-        self.line = try deserializer.deserialize()
+        self.startLine = try deserializer.deserialize()
+        self.endLine = try deserializer.deserialize()
         self.startColumn = try deserializer.deserialize()
         self.endColumn = try deserializer.deserialize()
     }
diff --git a/Tests/SWBCoreTests/SettingsTests.swift b/Tests/SWBCoreTests/SettingsTests.swift
@@ -134,7 +134,7 @@ import SWBMacro
         // Verify that the settings from the xcconfig were added.
         let XCCONFIG_USER_SETTING = try #require(settings.userNamespace.lookupMacroDeclaration("XCCONFIG_USER_SETTING"))
         #expect(settings.tableForTesting.lookupMacro(XCCONFIG_USER_SETTING)?.expression.stringRep == "from-xcconfig")
-        #expect(settings.tableForTesting.location(of: XCCONFIG_USER_SETTING) == MacroValueAssignmentLocation(path: .init("/tmp/xcconfigs/Base0.xcconfig"), line: 1, startColumn: 24, endColumn: 38))
+        #expect(settings.tableForTesting.location(of: XCCONFIG_USER_SETTING) == MacroValueAssignmentLocation(path: .init("/tmp/xcconfigs/Base0.xcconfig"), startLine: 1, endLine: 1, startColumn: 24, endColumn: 38))
 
         // Verify the user project settings.
         let USER_PROJECT_SETTING = try #require(settings.userNamespace.lookupMacroDeclaration("USER_PROJECT_SETTING"))
diff --git a/Tests/SWBMacroTests/MacroParsingTests.swift b/Tests/SWBMacroTests/MacroParsingTests.swift
@@ -790,7 +790,7 @@ fileprivate let testFileData = [
         }
         func endPreprocessorInclusion() {
         }
-        func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, line: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {
+        func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {
         }
 
         func handleDiagnostic(_ diagnostic: MacroConfigFileDiagnostic, parser: MacroConfigFileParser) {
@@ -816,10 +816,10 @@ fileprivate let testFileData = [
                                   ],
                                   expectedDiagnostics: [],
                                   expectedLocations: [
-                                    (macro: "FEATURE_DEFINES_A", path: .init("Multiline.xcconfig"), line: 2, startColumn: 20, endColumn: 37),
-                                    (macro: "FEATURE_DEFINES_B", path: .init("Multiline.xcconfig"), line: 5, startColumn: 20, endColumn: 87),
-                                    (macro: "FEATURE_DEFINES_C", path: .init("Multiline.xcconfig"), line: 9, startColumn: 20, endColumn: 61),
-                                    (macro: "FEATURE_DEFINES_D", path: .init("Multiline.xcconfig"), line: 11, startColumn: 20, endColumn: 45),
+                                    (macro: "FEATURE_DEFINES_A", path: .init("Multiline.xcconfig"), startLine: 1, endLine: 2, startColumn: 20, endColumn: 37),
+                                    (macro: "FEATURE_DEFINES_B", path: .init("Multiline.xcconfig"), startLine: 3, endLine: 5, startColumn: 20, endColumn: 87),
+                                    (macro: "FEATURE_DEFINES_C", path: .init("Multiline.xcconfig"), startLine: 6, endLine: 9, startColumn: 20, endColumn: 61),
+                                    (macro: "FEATURE_DEFINES_D", path: .init("Multiline.xcconfig"), startLine: 10, endLine: 11, startColumn: 20, endColumn: 45),
                                   ],
                                   expectedIncludeDirectivesCount: 1
         )
@@ -830,7 +830,7 @@ fileprivate let testFileData = [
 typealias ConditionInfo = (param: String, pattern: String)
 typealias AssignmentInfo = (macro: String, conditions: [ConditionInfo], value: String)
 typealias DiagnosticInfo = (level: MacroConfigFileDiagnostic.Level, kind: MacroConfigFileDiagnostic.Kind, line: Int)
-typealias LocationInfo = (macro: String, path: Path, line: Int, startColumn: Int, endColumn: Int)
+typealias LocationInfo = (macro: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int)
 
 private func TestMacroConfigFileParser(_ string: String, expectedAssignments: [AssignmentInfo], expectedDiagnostics: [DiagnosticInfo], expectedLocations: [LocationInfo]? = nil, expectedIncludeDirectivesCount: Int, sourceLocation: SourceLocation = #_sourceLocation) {
 
@@ -856,10 +856,10 @@ private func TestMacroConfigFileParser(_ string: String, expectedAssignments: [A
         func endPreprocessorInclusion() {
             self.includeDirectivesCount += 1
         }
-        func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, line: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {
+        func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {
             // print("\(parser.lineNumber): \(macroName)\(conditions.map({ "[\($0.param)=\($0.pattern)]" }).joinWithSeparator(""))=\(value)")
             assignments.append((macro: macroName, conditions: conditions, value: value))
-            locations.append((macro: macroName, path: path, line: line, startColumn: startColumn, endColumn: endColumn))
+            locations.append((macro: macroName, path: path, startLine: startLine, endLine: endLine, startColumn: startColumn, endColumn: endColumn))
         }
         func handleDiagnostic(_ diagnostic: MacroConfigFileDiagnostic, parser: MacroConfigFileParser) {
             // print("\(parser.lineNumber): \(diagnostic)")
@@ -913,7 +913,7 @@ func ==(lhs: [DiagnosticInfo], rhs: [DiagnosticInfo]) -> Bool {
 }
 
 func ==(lhs: LocationInfo, rhs: LocationInfo) -> Bool {
-    return (lhs.macro == rhs.macro) && (lhs.path == rhs.path) && (lhs.line == rhs.line) && (lhs.startColumn == rhs.startColumn) && (lhs.endColumn == rhs.endColumn)
+    return (lhs.macro == rhs.macro) && (lhs.path == rhs.path) && (lhs.startLine == rhs.startLine) && (lhs.endLine == rhs.endLine) && (lhs.startColumn == rhs.startColumn) && (lhs.endColumn == rhs.endColumn)
 }
 
 func ==(lhs: [LocationInfo], rhs: [LocationInfo]) -> Bool {

Original file line number	Diff line number	Diff line change
`@@ -242,7 +242,7 @@ final class MacroConfigFileLoader: Sendable {`
`242`	`242`	`return MacroConfigFileParser(byteString: data, path: path, delegate: delegate)`
`243`	`243`	`}`
`244`	`244`
`245`		`- mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, line: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {`
	`245`	`+ mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {`
`246`	`246`	`// Look up the macro name, creating it as a user-defined macro if it isn’t already known.`
`247`	`247`	`let macro = table.namespace.lookupOrDeclareMacro(UserDefinedMacroDeclaration.self, macroName)`
`248`	`248`
`@@ -253,7 +253,7 @@ final class MacroConfigFileLoader: Sendable {`
`253`	`253`	`}`
`254`	`254`
`255`	`255`	`// Parse the value in a manner consistent with the macro definition.`
`256`		`- let location = MacroValueAssignmentLocation(path: path, line: line, startColumn: startColumn, endColumn: endColumn)`
	`256`	`+ let location = MacroValueAssignmentLocation(path: path, startLine: startLine, endLine: endLine, startColumn: startColumn, endColumn: endColumn)`
`257`	`257`	`table.push(macro, table.namespace.parseForMacro(macro, value: value), conditions: conditionSet, location: location)`
`258`	`258`	`}`
`259`	`259`
Original file line number	Diff line number	Diff line change
`@@ -362,6 +362,7 @@ public final class MacroConfigFileParser {`
`362`	`362`	`// Skip over the equals sign.`
`363`	`363`	`assert(currChar == /* '=' */ 61)`
`364`	`364`	`advance()`
	`365`	`+ let startLine = currLine`
`365`	`366`	`let startColumn = currIdx - startOfLine`
`366`	`367`
`367`	`368`	`var chunks : [String] = []`
`@@ -385,7 +386,7 @@ public final class MacroConfigFileParser {`
`385`	`386`	`}`
`386`	`387`	`// Finally, now that we have the name, conditions, and value, we tell the delegate about it.`
`387`	`388`	`let value = chunks.joined(separator: " ")`
`388`		`- delegate?.foundMacroValueAssignment(name, conditions: conditions, value: value, path: path, line: currLine, startColumn: startColumn, endColumn: currIdx - startOfLine, parser: self)`
	`389`	`+ delegate?.foundMacroValueAssignment(name, conditions: conditions, value: value, path: path, startLine: startLine, endLine: currLine, startColumn: startColumn, endColumn: currIdx - startOfLine, parser: self)`
`389`	`390`	`}`
`390`	`391`
`391`	`392`	`public func parseNonListAssignmentRHS() -> String? {`
`@@ -520,7 +521,7 @@ public final class MacroConfigFileParser {`
`520`	`521`	`}`
`521`	`522`	`func endPreprocessorInclusion() {`
`522`	`523`	`}`
`523`		`- func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, line: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {`
	`524`	`+ func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) {`
`524`	`525`	`self.macroName = macroName`
`525`	`526`	`self.conditions = conditions.isEmpty ? nil : conditions`
`526`	`527`	`}`
`@@ -567,7 +568,7 @@ public protocol MacroConfigFileParserDelegate {`
`567`	`568`	`func endPreprocessorInclusion()`
`568`	`569`
`569`	`570`	/// Invoked once for each macro value assignment. The `macroName` is guaranteed to be non-empty, but `value` may be empty. Any macro conditions are passed as tuples in the `conditions`; parameters are guaranteed to be non-empty strings, but patterns may be empty.
`570`		`- mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, line: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser)`
	`571`	`+ mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser)`
`571`	`572`
`572`	`573`	`/// Invoked if an error, warning, or other diagnostic is detected.`
`573`	`574`	`func handleDiagnostic(_ diagnostic: MacroConfigFileDiagnostic, parser: MacroConfigFileParser)`