Fix code completion in header files (#426)

Previously, header files would have their compiler flags inferred
from source files which include them. However, code completion
didn't work properly since the file path in the arguments was
incorrect, pointing to the source file instead of the header file.

This also includes support for source files which include other source files, although that's not a common use case.

Author: DavidGoldman

Sources/SKCore/BuildSystemManager.swift

@@ -156,17 +156,36 @@ extension BuildSystemManager: BuildSystem {
         self.watchedFiles[uri] = (mainFile, language)
       }
 
-      let newStatus = self.cachedStatusOrRegisterForSettings( for: mainFile, language: language)
+      let newStatus = self.cachedStatusOrRegisterForSettings(for: mainFile, language: language)
 
-      if let change = newStatus.buildSettingsChange,
+      if let mainChange = newStatus.buildSettingsChange,
          let delegate = self._delegate {
+        let change = self.convert(change: mainChange, ofMainFile: mainFile, to: uri)
         self.notifyQueue.async {
           delegate.fileBuildSettingsChanged([uri: change])
         }
       }
     }
   }
 
+  /// Return settings for `file` based on  the `change` settings for `mainFile`.
+  ///
+  /// This is used when inferring arguments for header files (e.g. main file is a `.m`  while file is  a` .h`).
+  func convert(
+    change: FileBuildSettingsChange,
+    ofMainFile mainFile: DocumentURI,
+    to file: DocumentURI
+  ) -> FileBuildSettingsChange {
+    guard mainFile != file else { return change }
+    switch change {
+    case .removedOrUnavailable: return .removedOrUnavailable
+    case .fallback(let settings):
+      return .fallback(settings.patching(newFile: file.pseudoPath, originalFile: mainFile.pseudoPath))
+    case .modified(let settings):
+      return .modified(settings.patching(newFile: file.pseudoPath, originalFile: mainFile.pseudoPath))
+    }
+  }
+
   /// *Must be called on queue*. Handle a request for `FileBuildSettings` on
   /// `mainFile`. Updates and returns the new `MainFileStatus` for `mainFile`.
   func cachedStatusOrRegisterForSettings(
@@ -233,7 +252,9 @@ extension BuildSystemManager: BuildSystem {
       }
       if let change = status.buildSettingsChange {
         for watch in watches {
-          changedWatchedFiles[watch.key] = change
+          let newChange =
+            self.convert(change: change, ofMainFile: mainFile, to: watch.key)
+          changedWatchedFiles[watch.key] = newChange
         }
       }
     }
@@ -413,7 +434,10 @@ extension BuildSystemManager: MainFilesDelegate {
 
           let newStatus = self.cachedStatusOrRegisterForSettings(
               for: newMainFile, language: language)
-          buildSettingsChanges[uri] = newStatus.buildSettingsChange
+          if let change = newStatus.buildSettingsChange {
+            let newChange = self.convert(change: change, ofMainFile: newMainFile, to: uri)
+            buildSettingsChanges[uri] = newChange
+          }
         }
       }
 

Sources/SKCore/FileBuildSettings.swift

@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+import Foundation
 import LanguageServerProtocol
 
 /// Build settings for a single file.
@@ -29,3 +30,50 @@ public struct FileBuildSettings: Equatable {
     self.workingDirectory = workingDirectory
   }
 }
+
+fileprivate let cExtensions = ["c"]
+fileprivate let cppExtensions = ["cpp", "cc"]
+fileprivate let objcExtensions = ["m"]
+fileprivate let objcppExtensions = ["mm"]
+
+private extension String {
+  var pathExtension: String {
+    return (self as NSString).pathExtension
+  }
+  var pathBasename: String {
+    return (self as NSString).lastPathComponent
+  }
+}
+
+public extension FileBuildSettings {
+  /// Return arguments suitable for use by `newFile`.
+  ///
+  /// This patches the arguments by searching for the argument corresponding to
+  /// `originalFile` and replacing it.
+  func patching(newFile: String, originalFile: String) -> FileBuildSettings {
+    var arguments = self.compilerArguments
+    let basename = originalFile.pathBasename
+    let fileExtension = originalFile.pathExtension
+    if let index = arguments.lastIndex(where: {
+      // It's possible the arguments use relative paths while the `originalFile` given
+      // is an absolute/real path value. We guess based on suffixes instead of hitting
+      // the file system.
+      $0.hasSuffix(basename) && originalFile.hasSuffix($0)
+    }) {
+      arguments[index] = newFile
+      // The `-x<lang>` flag needs to be before the possible `-c <header file>`
+      // argument in order for Clang to respect it. If there is a pre-existing `-x`
+      // flag though, Clang will honor that one instead since it comes after.
+      if cExtensions.contains(fileExtension) {
+        arguments.insert("-xc", at: 0)
+      } else if cppExtensions.contains(fileExtension) {
+        arguments.insert("-xc++", at: 0)
+      } else if objcExtensions.contains(fileExtension) {
+        arguments.insert("-xobjective-c", at: 0)
+      } else if (objcppExtensions.contains(fileExtension)) {
+        arguments.insert("-xobjective-c++", at: 0)
+      }
+    }
+    return FileBuildSettings(compilerArguments: arguments, workingDirectory: self.workingDirectory)
+  }
+}

Tests/SKCoreTests/BuildSystemManagerTests.swift

@@ -325,13 +325,16 @@ final class BuildSystemManagerTests: XCTestCase {
     defer { withExtendedLifetime(bsm) {} } // Keep BSM alive for callbacks.
     let del = BSMDelegate(bsm)
 
-    bs.map[cpp] = FileBuildSettings(compilerArguments: ["C++ Main File"])
+    let cppArg = "C++ Main File"
+    bs.map[cpp] = FileBuildSettings(compilerArguments: [cppArg, cpp.pseudoPath])
 
     let initial1 = expectation(description: "initial settings h1 via cpp")
     let initial2 = expectation(description: "initial settings h2 via cpp")
+    let expectedArgsH1 = FileBuildSettings(compilerArguments: ["-xc++", cppArg, h1.pseudoPath])
+    let expectedArgsH2 = FileBuildSettings(compilerArguments: ["-xc++", cppArg, h2.pseudoPath])
     del.expected = [
-      (h1, bs.map[cpp]!, initial1, #file, #line),
-      (h2, bs.map[cpp]!, initial2, #file, #line),
+      (h1, expectedArgsH1, initial1, #file, #line),
+      (h2, expectedArgsH2, initial2, #file, #line),
     ]
 
     bsm.registerForChangeNotifications(for: h1, language: .c)
@@ -341,12 +344,15 @@ final class BuildSystemManagerTests: XCTestCase {
     // since they are backed by the same underlying cpp file.
     wait(for: [initial1, initial2], timeout: 10, enforceOrder: false)
 
-    bs.map[cpp] = FileBuildSettings(compilerArguments: ["New C++ Main File"])
+    let newCppArg = "New C++ Main File"
+    bs.map[cpp] = FileBuildSettings(compilerArguments: [newCppArg, cpp.pseudoPath])
     let changed1 = expectation(description: "initial settings h1 via cpp")
     let changed2 = expectation(description: "initial settings h2 via cpp")
+    let newArgsH1 = FileBuildSettings(compilerArguments: ["-xc++", newCppArg, h1.pseudoPath])
+    let newArgsH2 = FileBuildSettings(compilerArguments: ["-xc++", newCppArg, h2.pseudoPath])
     del.expected = [
-      (h1, bs.map[cpp]!, changed1, #file, #line),
-      (h2, bs.map[cpp]!, changed2, #file, #line),
+      (h1, newArgsH1, changed1, #file, #line),
+      (h2, newArgsH2, changed2, #file, #line),
     ]
     bsm.fileBuildSettingsChanged([cpp: .modified(bs.map[cpp]!)])