Fix adding file reference to bundle and package files

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Fixed
 - Fix Carthage support https://github.com/xcodeswift/xcproj/pull/226 by @ileitch
+- Fix adding file reference to bundle and package files https://github.com/xcodeswift/xcproj/pull/234 by @fuzza
 
 ### Changed
 - Carthage minimum Deployment Target:  https://github.com/xcodeswift/xcproj/pull/229 by @olbrichj

Sources/xcproj/PBXProjObjects+Helpers.swift

@@ -99,10 +99,14 @@ public extension PBXProj.Objects {
         sourceTree: PBXSourceTree = .group,
         sourceRoot: Path) throws -> ObjectReference<PBXFileReference> {
 
-        guard filePath.isFile else {
-            throw XCodeProjEditingError.notAFile(path: filePath)
+        guard filePath.exists else {
+            throw XCodeProjEditingError.fileNotExists(path: filePath)
         }
 
+        guard let fileType = PBXFileReference.fileType(path: filePath) else {
+            throw XCodeProjEditingError.unsupportedFileType(path: filePath)
+        }
+
         guard let groupReference = groups.first(where: { $0.value == toGroup })?.key else {
             throw XCodeProjEditingError.groupNotFound(group: toGroup)
         }
@@ -133,8 +137,8 @@ public extension PBXProj.Objects {
         let fileReference = PBXFileReference(
             sourceTree: sourceTree,
             name: filePath.lastComponent,
-            explicitFileType: PBXFileReference.fileType(path: filePath),
-            lastKnownFileType: PBXFileReference.fileType(path: filePath),
+            explicitFileType: fileType,
+            lastKnownFileType: fileType,
             path: path?.string
         )
         let reference = generateReference(fileReference, filePath.string)
@@ -200,13 +204,16 @@ public struct GroupAddingOptions: OptionSet {
 }
 
 public enum XCodeProjEditingError: Error, CustomStringConvertible {
-    case notAFile(path: Path)
+    case unsupportedFileType(path: Path)
+    case fileNotExists(path: Path)
     case groupNotFound(group: PBXGroup)
 
     public var description: String {
         switch self {
-        case .notAFile(let path):
-            return "\(path) is not a file path"
+        case .unsupportedFileType(let path):
+            return "\(path) is not supported."
+        case .fileNotExists(let path):
+            return "\(path) doesn't exist."
         case .groupNotFound(let group):
             return "Group not found in project: \(group)"
         }

Tests/xcprojTests/XCTestCase+Assertions.swift

@@ -2,11 +2,25 @@ import Foundation
 import XCTest
 
 extension XCTestCase {
+    typealias EquatableError = Error & Equatable
+
     func XCTAssertNotNilAndUnwrap<T>(_ obj: T?, message: String = "") -> T {
         guard let unwrappedObj = obj else {
             XCTAssertNotNil(obj, message)
             fatalError() // Unreachable since the above assertion will fail
         }
         return unwrappedObj
     }
+
+    func XCTAssertThrowsSpecificError<T, E: EquatableError>(_ expression: @autoclosure () throws -> T, _ error: E, _ message: @autoclosure () -> String = "", file: StaticString = #file, line: UInt = #line) {
+        XCTAssertThrowsError(expression, message, file: file, line: line) { actualError in
+            let message = "Expected \(error) got \(actualError)"
+
+            guard let actualCastedError = actualError as? E else {
+                XCTFail(message, file: file, line: line)
+                return
+            }
+            XCTAssertEqual(actualCastedError, error, message, file: file, line: line)
+        }
+    }
 }

Tests/xcprojTests/XcodeProjIntegrationSpec.swift

@@ -138,34 +138,86 @@ final class XcodeProjIntegrationSpec: XCTestCase {
         XCTAssertEqual(project.pbxproj.objects.groups[newGroups[1].reference], newGroups[1].object)
     }
 
-    func test_add_new_file() throws {
+    func test_add_new_source_file() throws {
         let proj = projectiOS()!.pbxproj
         let filePath = fixturesPath() + "newfile.swift"
         let iOSGroup = proj.objects.group(named: "iOS", inGroup: proj.rootGroup)!
         let file = try proj.objects.addFile(at: filePath, toGroup: iOSGroup.object, sourceRoot: fixturesPath() + "iOS")
 
+        let expectedFile = PBXFileReference(sourceTree: .group,
+                                            name: "newfile.swift",
+                                            explicitFileType: "sourcecode.swift",
+                                            lastKnownFileType: "sourcecode.swift",
+                                            path: "../../newfile.swift")
+
         XCTAssertEqual(proj.objects.fileReferences[file.reference], file.object)
-        XCTAssertEqual(file.object.name, "newfile.swift")
-        XCTAssertEqual(file.object.sourceTree, PBXSourceTree.group)
-        XCTAssertEqual(file.object.path, "../../newfile.swift")
+        XCTAssertEqual(file.object, expectedFile)
         XCTAssertNotNil(iOSGroup.object.children.index(of: file.reference))
 
         let existingFile = try proj.objects.addFile(at: filePath, toGroup: proj.rootGroup, sourceRoot: fixturesPath() + "iOS")
 
         XCTAssertTrue(file == existingFile)
     }
-
-    func test_add_not_a_file() throws {
+    
+    func test_add_new_dynamic_framework() throws {
         let proj = projectiOS()!.pbxproj
-        do {
-            _ = try proj.objects.addFile(at: fixturesPath() + "iOS/iOS", toGroup: proj.rootGroup, sourceRoot: fixturesPath() + "iOS")
-            XCTFail("Adding not file path should throw error")
-        } catch {}
-
-        do {
-            _ = try proj.objects.addFile(at: fixturesPath() + "iOS/iOS/newfile.swift", toGroup: proj.rootGroup, sourceRoot: fixturesPath() + "iOS")
-            XCTFail("Adding not existing file should throw error")
-        } catch {}
+        let filePath = fixturesPath() + "dummy.framework"
+
+        let iOSGroup = proj.objects.group(named: "iOS", inGroup: proj.rootGroup)!
+        let file = try proj.objects.addFile(at: filePath,
+                                            toGroup: iOSGroup.object,
+                                            sourceRoot: fixtureiOSSourcePath())
+
+        let expectedFile = PBXFileReference(sourceTree: .group,
+                                            name: "dummy.framework",
+                                            explicitFileType: "wrapper.framework",
+                                            lastKnownFileType: "wrapper.framework",
+                                            path: "../../dummy.framework")
+
+
+        XCTAssertEqual(proj.objects.fileReferences[file.reference], file.object)
+        XCTAssertEqual(file.object, expectedFile)
+        XCTAssertNotNil(iOSGroup.object.children.index(of: file.reference))
+    }
+
+    func test_add_existing_file_returns_existing_object() throws {
+        let proj = projectiOS()!.pbxproj
+        let filePath = fixturesPath() + "newfile.swift"
+        let iOSGroup = proj.objects.group(named: "iOS", inGroup: proj.rootGroup)!.object
+
+        let file = try proj.objects.addFile(at: filePath, toGroup: iOSGroup, sourceRoot: fixtureiOSSourcePath())
+        let existingFile = try proj.objects.addFile(at: filePath, toGroup: proj.rootGroup, sourceRoot: fixtureiOSSourcePath())
+        XCTAssertTrue(file == existingFile)
+    }
+
+    func test_add_unsupported_file_throws() {
+        let proj = projectiOS()!.pbxproj
+        let filePath = fixturesPath() + "unsupported.xyz"
+        XCTAssertThrowsSpecificError(
+            try proj.objects.addFile(at: filePath, toGroup: proj.rootGroup, sourceRoot: fixtureiOSSourcePath()),
+            XCodeProjEditingError.unsupportedFileType(path: filePath),
+            "Adding file reference to unsupported file type should throw an error"
+        )
+    }
+
+    func test_add_directory_throws() {
+        let proj = projectiOS()!.pbxproj
+        let filePath = fixturesPath() + "directory"
+        XCTAssertThrowsSpecificError(
+            try proj.objects.addFile(at: filePath, toGroup: proj.rootGroup, sourceRoot: fixtureiOSSourcePath()),
+            XCodeProjEditingError.unsupportedFileType(path: filePath),
+            "Adding a directory as a file reference should throw an error"
+        )
+    }
+
+    func test_add_nonexisting_file_throws() {
+        let proj = projectiOS()!.pbxproj
+        let filePath = fixturesPath() + "nonexisting.swift"
+        XCTAssertThrowsSpecificError(
+            try proj.objects.addFile(at: filePath, toGroup: proj.rootGroup, sourceRoot: fixtureiOSSourcePath()),
+            XCodeProjEditingError.fileNotExists(path: filePath),
+            "Adding a reference to non existing file should throw an error"
+        )
     }
 
     func test_add_new_build_file() throws {
@@ -255,6 +307,10 @@ final class XcodeProjIntegrationSpec: XCTestCase {
         return fixturesPath() + "iOS/Project.xcodeproj"
     }
 
+    private func fixtureiOSSourcePath() -> Path {
+        return fixturesPath() + "iOS"
+    }
+
     private func projectiOS() -> XcodeProj? {
         return try? XcodeProj(path: fixtureiOSProjectPath())
     }
@@ -263,3 +319,20 @@ final class XcodeProjIntegrationSpec: XCTestCase {
         return try XcodeProj(path: fixtureWithoutWorkspaceProjectPath())
     }
 }
+
+// This could be code generated (e.g. using sourcery)
+extension XCodeProjEditingError: Equatable {
+
+    static public func == (lhs: XCodeProjEditingError, rhs: XCodeProjEditingError) -> Bool {
+        switch (lhs, rhs) {
+        case (.unsupportedFileType(let path1), .unsupportedFileType(let path2)):
+            return path1 == path2
+        case (.fileNotExists(let path1), .fileNotExists(let path2)):
+            return path1 == path2
+        case (.groupNotFound(let group1), .groupNotFound(let group2)):
+            return group1 == group2
+        default:
+            return false
+        }
+    }
+}