Return compiler arguments for invalid package manifests (#8138)

Currently, when there‘s a syntax error in a package manifest, we don’t
get any build settings from it in SourceKit-LSP and thus loose almost
all semantic functionality. If we can’t parse the package manifest, fall
back to providing build settings by assuming it has the current Swift
tools version.

Fixes swiftlang/sourcekit-lsp#1704
rdar://136423767

Author: ahoppen

Sources/Workspace/Workspace+Manifests.swift

@@ -384,33 +384,23 @@ extension Workspace {
     }
 
     /// Returns manifest interpreter flags for a package.
-    // TODO: should this be throwing instead?
-    public func interpreterFlags(for packagePath: AbsolutePath) -> [String] {
-        do {
-            guard let manifestLoader = self.manifestLoader as? ManifestLoader else {
-                throw StringError("unexpected manifest loader kind")
-            }
+    public func interpreterFlags(for manifestPath: AbsolutePath) throws -> [String] {
+        guard let manifestLoader = self.manifestLoader as? ManifestLoader else {
+            throw StringError("unexpected manifest loader kind")
+        }
 
-            let manifestPath = try ManifestLoader.findManifest(
-                packagePath: packagePath,
-                fileSystem: self.fileSystem,
-                currentToolsVersion: self.currentToolsVersion
-            )
-            let manifestToolsVersion = try ToolsVersionParser.parse(
-                manifestPath: manifestPath,
-                fileSystem: self.fileSystem
-            )
+        let manifestToolsVersion = (try? ToolsVersionParser.parse(
+            manifestPath: manifestPath,
+            fileSystem: self.fileSystem
+        )) ?? self.currentToolsVersion
 
-            guard self.currentToolsVersion >= manifestToolsVersion || SwiftVersion.current.isDevelopment,
-                  manifestToolsVersion >= ToolsVersion.minimumRequired
-            else {
-                throw StringError("invalid tools version")
-            }
-            return manifestLoader.interpreterFlags(for: manifestToolsVersion)
-        } catch {
-            // We ignore all failures here and return empty array.
-            return []
+
+        guard self.currentToolsVersion >= manifestToolsVersion || SwiftVersion.current.isDevelopment,
+                manifestToolsVersion >= ToolsVersion.minimumRequired
+        else {
+            throw StringError("invalid tools version")
         }
+        return manifestLoader.interpreterFlags(for: manifestToolsVersion)
     }
 
     /// Load the manifests for the current dependency tree.

Tests/WorkspaceTests/WorkspaceTests.swift

@@ -159,9 +159,10 @@ final class WorkspaceTests: XCTestCase {
 
         try testWithTemporaryDirectory { path in
             let foo = path.appending("foo")
+            let packageManifest = foo.appending("Package.swift")
 
             func createWorkspace(_ content: String) throws -> Workspace {
-                try fs.writeFileContents(foo.appending("Package.swift"), string: content)
+                try fs.writeFileContents(packageManifest, string: content)
 
                 let manifestLoader = ManifestLoader(toolchain: try UserToolchain.default)
 
@@ -185,7 +186,7 @@ final class WorkspaceTests: XCTestCase {
                     """
                 )
 
-                XCTAssertMatch(ws.interpreterFlags(for: foo), [.equal("-swift-version"), .equal("4")])
+                XCTAssertMatch(try ws.interpreterFlags(for: packageManifest), [.equal("-swift-version"), .equal("4")])
             }
 
             do {
@@ -199,7 +200,7 @@ final class WorkspaceTests: XCTestCase {
                     """
                 )
 
-                XCTAssertEqual(ws.interpreterFlags(for: foo), [])
+                XCTAssertThrowsError(try ws.interpreterFlags(for: packageManifest))
             }
 
             do {
@@ -213,7 +214,19 @@ final class WorkspaceTests: XCTestCase {
                     """
                 )
 
-                XCTAssertMatch(ws.interpreterFlags(for: foo), [.equal("-swift-version"), .equal("6")])
+                XCTAssertMatch(try ws.interpreterFlags(for: packageManifest), [.equal("-swift-version"), .equal("6")])
+            }
+
+            do {
+                // Invalid package manifest should still produce build settings.
+                let ws = try createWorkspace(
+                    """
+                    // swift-tools-version:999.0
+                    import PackageDescription
+                    """
+                )
+
+                XCTAssertMatch(try ws.interpreterFlags(for: packageManifest), [.equal("-package-description-version")])
             }
         }
     }