From 4bbcf1def1f863c58ce2c7a0e9e75ab724e4b4e6 Mon Sep 17 00:00:00 2001 From: Jonathan Crooke Date: Tue, 29 Mar 2022 17:44:29 +0100 Subject: [PATCH 1/7] Add API to search for files in a group with relative path, handling non-unique names --- .../XcodeProj/Objects/Files/PBXGroup.swift | 26 ++++++++++++++ .../Objects/Files/PBXGroupTests.swift | 34 +++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/Sources/XcodeProj/Objects/Files/PBXGroup.swift b/Sources/XcodeProj/Objects/Files/PBXGroup.swift index c2bded056..08250e1c1 100644 --- a/Sources/XcodeProj/Objects/Files/PBXGroup.swift +++ b/Sources/XcodeProj/Objects/Files/PBXGroup.swift @@ -120,12 +120,26 @@ public extension PBXGroup { /// /// - Parameter name: file name. /// - Returns: file with the given name contained in the given parent group. + @available(*, deprecated, message: "Please use file(with:relativeTo:). This method assumes all names in the group are unique") func file(named name: String) -> PBXFileReference? { childrenReferences .objects() .first(where: { $0.name == name }) } + /// Returns the file in the group with the given path, relative to a base path. + /// Will use the root path to generate normalized absolute paths for the search path + /// and also the file paths in the group. + /// + /// - Parameter path: a file path. + /// - Parameter basePath: a base path to search from. + /// - Returns: file with the given absolute path contained in the given parent group. + func file(with path: Path, relativeTo basePath: Path) -> PBXFileReference? { + let normalized = path.normalized(relativeTo: basePath) + return children + .first { $0.normalizedPath(relativeTo: basePath) == normalized } as? PBXFileReference + } + /// Creates a group with the given name and returns it. /// /// - Parameters: @@ -233,3 +247,15 @@ public extension PBXGroup { return fileReference } } + +private extension PBXFileElement { + func normalizedPath(relativeTo sourceRoot: Path) -> Path? { + path.map { Path($0).normalized(relativeTo: sourceRoot) } + } +} + +private extension Path { + func normalized(relativeTo path: Path) -> Path { + (path + self).normalize() + } +} diff --git a/Tests/XcodeProjTests/Objects/Files/PBXGroupTests.swift b/Tests/XcodeProjTests/Objects/Files/PBXGroupTests.swift index 6fd5360d5..1772c6a16 100644 --- a/Tests/XcodeProjTests/Objects/Files/PBXGroupTests.swift +++ b/Tests/XcodeProjTests/Objects/Files/PBXGroupTests.swift @@ -199,6 +199,40 @@ final class PBXGroupTests: XCTestCase { XCTAssertEqual(file!.sourceTree, .developerDir) } + func test_whenSearchingForFiles_thenTheAbsolutePathIsUsedForUniqueness() throws { + let project = makeEmptyPBXProj() + let group = PBXGroup(children: [], + sourceTree: .absolute, + name: "group") + project.add(object: group) + + let rootPath = try Path.uniqueTemporary() + let subdir = rootPath + "subdir" + try subdir.mkdir() + + let file1 = rootPath + "file" + let file2 = rootPath + "subdir/file" + + let filesToRefs = try [file1, file2].reduce([Path: PBXFileReference]()) { + try Data().write(to: $1.url) + let file = try group.addFile(at: $1, sourceTree: .absolute, sourceRoot: rootPath) + return $0.merging([$1: file]) { _, new in new } + } + + XCTAssertEqual( + group.file(with: "file", relativeTo: rootPath), + try XCTUnwrap(filesToRefs[file1]) + ) + XCTAssertEqual( + group.file(with: "file", relativeTo: rootPath + "subdir"), + try XCTUnwrap(filesToRefs[file2]) + ) + XCTAssertEqual( + group.file(with: "subdir/file", relativeTo: rootPath), + try XCTUnwrap(filesToRefs[file2]) + ) + } + private func makeEmptyPBXProj() -> PBXProj { PBXProj( rootObject: nil, From 8cd16c9694edc6068575be808956f80104f29607 Mon Sep 17 00:00:00 2001 From: Jonathan Crooke Date: Thu, 31 Mar 2022 10:00:09 +0100 Subject: [PATCH 2/7] Remove deprecation, improve docs --- Sources/XcodeProj/Objects/Files/PBXGroup.swift | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Sources/XcodeProj/Objects/Files/PBXGroup.swift b/Sources/XcodeProj/Objects/Files/PBXGroup.swift index 08250e1c1..3d5cab629 100644 --- a/Sources/XcodeProj/Objects/Files/PBXGroup.swift +++ b/Sources/XcodeProj/Objects/Files/PBXGroup.swift @@ -117,22 +117,26 @@ public extension PBXGroup { } /// Returns the file in the group with the given name. + /// - Note: Performs a linear search based on the file's name _only_. To search for a specific file + /// based on file-system location, use the `file(with path:)` method. /// /// - Parameter name: file name. /// - Returns: file with the given name contained in the given parent group. - @available(*, deprecated, message: "Please use file(with:relativeTo:). This method assumes all names in the group are unique") func file(named name: String) -> PBXFileReference? { childrenReferences .objects() .first(where: { $0.name == name }) } - /// Returns the file in the group with the given path, relative to a base path. - /// Will use the root path to generate normalized absolute paths for the search path - /// and also the file paths in the group. + /// Returns the file in the group with the given path, relative to a common base path. + /// - Note: Unlike the `file(named:)` API, considers the files's actual location on disk; therefore + /// can handle groups/projects that contain files with duplicated names. + /// `basePath` is likely to be the project's `$(SRCROOT)`, but can be any common root + /// between all files. Both arguments support traversal; i.e. `../` /// - /// - Parameter path: a file path. - /// - Parameter basePath: a base path to search from. + /// - Parameters: + /// - path: a file path to search for. + /// - basePath: a base path to search from. /// - Returns: file with the given absolute path contained in the given parent group. func file(with path: Path, relativeTo basePath: Path) -> PBXFileReference? { let normalized = path.normalized(relativeTo: basePath) From 51abda85fdf7f496956e646b57cd254b8f1c2e26 Mon Sep 17 00:00:00 2001 From: Jonathan Crooke Date: Thu, 31 Mar 2022 11:14:18 +0100 Subject: [PATCH 3/7] Simplify API, always consider absolute paths internally --- .../XcodeProj/Objects/Files/PBXGroup.swift | 39 ++++----- .../Objects/Files/PBXGroupTests.swift | 83 ++++++++++++++----- 2 files changed, 76 insertions(+), 46 deletions(-) diff --git a/Sources/XcodeProj/Objects/Files/PBXGroup.swift b/Sources/XcodeProj/Objects/Files/PBXGroup.swift index 3d5cab629..3174d8b14 100644 --- a/Sources/XcodeProj/Objects/Files/PBXGroup.swift +++ b/Sources/XcodeProj/Objects/Files/PBXGroup.swift @@ -128,20 +128,25 @@ public extension PBXGroup { .first(where: { $0.name == name }) } - /// Returns the file in the group with the given path, relative to a common base path. - /// - Note: Unlike the `file(named:)` API, considers the files's actual location on disk; therefore - /// can handle groups/projects that contain files with duplicated names. - /// `basePath` is likely to be the project's `$(SRCROOT)`, but can be any common root - /// between all files. Both arguments support traversal; i.e. `../` + /// Returns the file in the group with the given path, based on locations on-disk. + /// - Note: Paths function as-in Xcode; with respect to the group's `PBXSourceTree`. + /// Internally, identity is calculated based on normalized absolute paths for all files involved; i.e. actual + /// location in the file system. /// /// - Parameters: /// - path: a file path to search for. - /// - basePath: a base path to search from. - /// - Returns: file with the given absolute path contained in the given parent group. - func file(with path: Path, relativeTo basePath: Path) -> PBXFileReference? { - let normalized = path.normalized(relativeTo: basePath) - return children - .first { $0.normalizedPath(relativeTo: basePath) == normalized } as? PBXFileReference + /// - Returns: an existing reference to that file, or `nil` if not found + func file(with path: Path) -> PBXFileReference? { + func absolutePath(for fileRef: PBXFileElement) -> Path? { + try? fileRef.fullPath(sourceRoot: ".") + } + + guard let groupAbsPath = absolutePath(for: self) else { return nil } + let fileNormalizedAbsolutePath = (groupAbsPath + path).normalize() + return children.first { + guard let candidateAbsPath = absolutePath(for: $0) else { return false } + return candidateAbsPath.normalize() == fileNormalizedAbsolutePath + } as? PBXFileReference } /// Creates a group with the given name and returns it. @@ -251,15 +256,3 @@ public extension PBXGroup { return fileReference } } - -private extension PBXFileElement { - func normalizedPath(relativeTo sourceRoot: Path) -> Path? { - path.map { Path($0).normalized(relativeTo: sourceRoot) } - } -} - -private extension Path { - func normalized(relativeTo path: Path) -> Path { - (path + self).normalize() - } -} diff --git a/Tests/XcodeProjTests/Objects/Files/PBXGroupTests.swift b/Tests/XcodeProjTests/Objects/Files/PBXGroupTests.swift index 1772c6a16..8abaee73e 100644 --- a/Tests/XcodeProjTests/Objects/Files/PBXGroupTests.swift +++ b/Tests/XcodeProjTests/Objects/Files/PBXGroupTests.swift @@ -199,37 +199,74 @@ final class PBXGroupTests: XCTestCase { XCTAssertEqual(file!.sourceTree, .developerDir) } - func test_whenSearchingForFiles_thenTheAbsolutePathIsUsedForUniqueness() throws { + func test_whenSearchingForFilesByPath_thenNormalizedAbsolutePathsAreUsedInternallyForUniqueness() throws { let project = makeEmptyPBXProj() - let group = PBXGroup(children: [], - sourceTree: .absolute, - name: "group") - project.add(object: group) + let subDirectoryName = UUID().uuidString + + func prepareDirectories() throws -> (projectDir: Path, groupDir: Path, group: PBXGroup) { + let absolutePath = try Path.uniqueTemporary() + let splitSize = 2 + let projectDir = Path(components: absolutePath.components[0.. Date: Thu, 31 Mar 2022 16:29:33 +0100 Subject: [PATCH 4/7] Refactor tests to test all possible sourceRoot types --- .../XcodeProj/Objects/Files/PBXGroup.swift | 5 +- .../Files/PBXGroupPathSearchTests.swift | 120 ++++++++++++++++++ .../Objects/Files/PBXGroupTests.swift | 71 ----------- 3 files changed, 123 insertions(+), 73 deletions(-) create mode 100644 Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift diff --git a/Sources/XcodeProj/Objects/Files/PBXGroup.swift b/Sources/XcodeProj/Objects/Files/PBXGroup.swift index 3174d8b14..fd0ba0c96 100644 --- a/Sources/XcodeProj/Objects/Files/PBXGroup.swift +++ b/Sources/XcodeProj/Objects/Files/PBXGroup.swift @@ -135,10 +135,11 @@ public extension PBXGroup { /// /// - Parameters: /// - path: a file path to search for. + /// - sourceRoot: path to project's source root. /// - Returns: an existing reference to that file, or `nil` if not found - func file(with path: Path) -> PBXFileReference? { + func file(with path: Path, sourceRoot: Path) -> PBXFileReference? { func absolutePath(for fileRef: PBXFileElement) -> Path? { - try? fileRef.fullPath(sourceRoot: ".") + try? fileRef.fullPath(sourceRoot: sourceRoot) } guard let groupAbsPath = absolutePath(for: self) else { return nil } diff --git a/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift b/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift new file mode 100644 index 000000000..5aedf0f2f --- /dev/null +++ b/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift @@ -0,0 +1,120 @@ +import Foundation +import PathKit +import XcodeProj +import XCTest + +final class PBXGroupPathSearchTests: XCTestCase { + + struct Files { + let fileName: String + let file1Path: Path + let file2Path: Path + } + + typealias PathToFileReferenceMap = [Path: PBXFileReference] + + let groupPathComponentsCount = 2 + + let sourceRootName = "src" + let subDirName = "subDir" + let fileName = "theFile" + + var projectDir: Path! + var sourceRoot: Path { projectDir + sourceRootName } + var project: PBXProj! + var group: PBXGroup! + + var files: Files! + + override func setUpWithError() throws { + project = PBXProj( + rootObject: nil, + objectVersion: 0, + archiveVersion: 0, + classes: [:], + objects: [] + ) + projectDir = try Path.uniqueTemporary() + try (projectDir + sourceRootName + subDirName).mkpath() + try createGroup() + files = Files( + fileName: fileName, + file1Path: Path(fileName), + file2Path: Path(components: [subDirName, fileName]) + ) + } + + func test_whenFilesHaveAbsoluteSourceTree_thenCanBeFoundByPath() throws { + try checkAssertions(for: .absolute) + } + + func test_whenFilesHaveGroupSourceTree_thenCanBeFoundByPath() throws { + try checkAssertions(for: .group) + } + func test_whenFilesHaveSourceRootSourceTree_thenCanBeFoundByPath() throws { + try checkAssertions(for: .sourceRoot) + } +} + +private extension PBXGroupPathSearchTests { + + func createGroup() throws { + let group = PBXGroup(children: [], sourceTree: .group, name: "group", path: sourceRootName) + let parent = PBXGroup(children: [group], sourceTree: .absolute, name: "parent", path: projectDir.string) + project.add(object: parent) + project.add(object: group) + self.group = group + } + + func addToGroup(testFiles: Files, sourceTree: PBXSourceTree) throws -> PathToFileReferenceMap { + try [testFiles.file1Path, testFiles.file2Path] + .reduce([Path: PBXFileReference]()) { map, filePath in + let absolutePath = sourceRoot + filePath + try Data().write(to: absolutePath.url) + let file = try group.addFile( + at: absolutePath, + sourceTree: sourceTree, + sourceRoot: sourceRoot, + validatePresence: true + ) + return map.merging([filePath: file]) { _, new in new } + } + } + + func checkAssertions(for sourceTree: PBXSourceTree, line: UInt = #line) throws { + func assert(filePath: Path, hasReference: PBXFileReference?) { + let actual = group.file(with: filePath, sourceRoot: sourceRoot) + if let expected = hasReference { + XCTAssertEqual(actual, expected, file: #file, line: line) + } else { + XCTAssertNil(actual, file: #file, line: line) + } + } + + let fileReferenceFor = try addToGroup( + testFiles: files, + sourceTree: sourceTree + ) + + assert( + filePath: files.file1Path, + hasReference: fileReferenceFor[files.file1Path] + ) + assert( + filePath: files.file2Path, + hasReference: fileReferenceFor[files.file2Path] + ) + assert( + filePath: files.file2Path + "../.." + fileName, + hasReference: fileReferenceFor[files.file1Path] + ) + assert( + filePath: files.file1Path + "../foo/.." + subDirName + fileName, + hasReference: fileReferenceFor[files.file2Path] + ) + assert( + filePath: Path(UUID().uuidString), + hasReference: nil + ) + } +} diff --git a/Tests/XcodeProjTests/Objects/Files/PBXGroupTests.swift b/Tests/XcodeProjTests/Objects/Files/PBXGroupTests.swift index 8abaee73e..6fd5360d5 100644 --- a/Tests/XcodeProjTests/Objects/Files/PBXGroupTests.swift +++ b/Tests/XcodeProjTests/Objects/Files/PBXGroupTests.swift @@ -199,77 +199,6 @@ final class PBXGroupTests: XCTestCase { XCTAssertEqual(file!.sourceTree, .developerDir) } - func test_whenSearchingForFilesByPath_thenNormalizedAbsolutePathsAreUsedInternallyForUniqueness() throws { - let project = makeEmptyPBXProj() - let subDirectoryName = UUID().uuidString - - func prepareDirectories() throws -> (projectDir: Path, groupDir: Path, group: PBXGroup) { - let absolutePath = try Path.uniqueTemporary() - let splitSize = 2 - let projectDir = Path(components: absolutePath.components[0.. PBXProj { PBXProj( rootObject: nil, From 87d18b9ae77c29e18fad06e249f969df1bce5ad1 Mon Sep 17 00:00:00 2001 From: Jonathan Crooke Date: Thu, 31 Mar 2022 16:46:19 +0100 Subject: [PATCH 5/7] Wip --- .../Files/PBXGroupPathSearchTests.swift | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift b/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift index 5aedf0f2f..872ba65ad 100644 --- a/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift +++ b/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift @@ -16,6 +16,7 @@ final class PBXGroupPathSearchTests: XCTestCase { let groupPathComponentsCount = 2 let sourceRootName = "src" + let groupDirName = "theGroup" let subDirName = "subDir" let fileName = "theFile" @@ -35,7 +36,7 @@ final class PBXGroupPathSearchTests: XCTestCase { objects: [] ) projectDir = try Path.uniqueTemporary() - try (projectDir + sourceRootName + subDirName).mkpath() +// try (projectDir + sourceRootName + groupDirName + subDirName).mkpath() try createGroup() files = Files( fileName: fileName, @@ -59,8 +60,18 @@ final class PBXGroupPathSearchTests: XCTestCase { private extension PBXGroupPathSearchTests { func createGroup() throws { - let group = PBXGroup(children: [], sourceTree: .group, name: "group", path: sourceRootName) - let parent = PBXGroup(children: [group], sourceTree: .absolute, name: "parent", path: projectDir.string) + let group = PBXGroup( + children: [], + sourceTree: .group, + name: "group", + path: Path(components: [sourceRootName, groupDirName]).string + ) + let parent = PBXGroup( + children: [group], + sourceTree: .absolute, + name: "parent", + path: projectDir.string + ) project.add(object: parent) project.add(object: group) self.group = group @@ -69,20 +80,20 @@ private extension PBXGroupPathSearchTests { func addToGroup(testFiles: Files, sourceTree: PBXSourceTree) throws -> PathToFileReferenceMap { try [testFiles.file1Path, testFiles.file2Path] .reduce([Path: PBXFileReference]()) { map, filePath in - let absolutePath = sourceRoot + filePath - try Data().write(to: absolutePath.url) +// let absolutePath = sourceRoot + filePath +// try Data().write(to: absolutePath.url) let file = try group.addFile( - at: absolutePath, + at: filePath, sourceTree: sourceTree, sourceRoot: sourceRoot, - validatePresence: true + validatePresence: false ) return map.merging([filePath: file]) { _, new in new } } } - func checkAssertions(for sourceTree: PBXSourceTree, line: UInt = #line) throws { - func assert(filePath: Path, hasReference: PBXFileReference?) { + func checkAssertions(for sourceTree: PBXSourceTree) throws { + func assert(filePath: Path, hasReference: PBXFileReference?, line: UInt = #line) { let actual = group.file(with: filePath, sourceRoot: sourceRoot) if let expected = hasReference { XCTAssertEqual(actual, expected, file: #file, line: line) From e3b1fe1c10db7d4043a1cd018f0ffd9d7b578ca5 Mon Sep 17 00:00:00 2001 From: Jonathan Crooke Date: Thu, 31 Mar 2022 17:18:43 +0100 Subject: [PATCH 6/7] Tests --- .../Files/PBXGroupPathSearchTests.swift | 78 ++++++++++++------- 1 file changed, 48 insertions(+), 30 deletions(-) diff --git a/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift b/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift index 872ba65ad..878868485 100644 --- a/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift +++ b/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift @@ -15,18 +15,21 @@ final class PBXGroupPathSearchTests: XCTestCase { let groupPathComponentsCount = 2 - let sourceRootName = "src" - let groupDirName = "theGroup" - let subDirName = "subDir" - let fileName = "theFile" + enum Names { + static let fileName = "theFile" + static let sourceRoot = "src" + static let group = "theGroup" + static let subDir = "subDir" + } + + var projectPath: Path! + var sourceRoot: Path { projectPath + Names.sourceRoot } + var groupPath: Path { sourceRoot + Names.group } + var subDirPath: Path { groupPath + Names.subDir } - var projectDir: Path! - var sourceRoot: Path { projectDir + sourceRootName } var project: PBXProj! var group: PBXGroup! - var files: Files! - override func setUpWithError() throws { project = PBXProj( rootObject: nil, @@ -35,25 +38,38 @@ final class PBXGroupPathSearchTests: XCTestCase { classes: [:], objects: [] ) - projectDir = try Path.uniqueTemporary() -// try (projectDir + sourceRootName + groupDirName + subDirName).mkpath() + projectPath = try Path.uniqueTemporary() + try (projectPath + Names.sourceRoot + Names.group + Names.subDir).mkpath() try createGroup() - files = Files( - fileName: fileName, - file1Path: Path(fileName), - file2Path: Path(components: [subDirName, fileName]) - ) } - func test_whenFilesHaveAbsoluteSourceTree_thenCanBeFoundByPath() throws { - try checkAssertions(for: .absolute) + func test_whenFilesHaveAbsoluteSourceTree_thenCanBeFound() throws { + try checkAssertions(for: .absolute) { + Files( + fileName: Names.fileName, + file1Path: groupPath + Names.fileName, + file2Path: subDirPath + Names.fileName + ) + } } - func test_whenFilesHaveGroupSourceTree_thenCanBeFoundByPath() throws { - try checkAssertions(for: .group) + func test_whenFilesHaveGroupSourceTree_thenCanBeFound() throws { + try checkAssertions(for: .group) { + Files( + fileName: Names.fileName, + file1Path: Path(Names.fileName), + file2Path: Path(components: [Names.subDir, Names.fileName]) + ) + } } - func test_whenFilesHaveSourceRootSourceTree_thenCanBeFoundByPath() throws { - try checkAssertions(for: .sourceRoot) + func test_whenFilesHaveSourceRootSourceTree_thenCanBeFound() throws { + try checkAssertions(for: .sourceRoot) { + Files( + fileName: Names.fileName, + file1Path: Path(components: [Names.sourceRoot, Names.fileName]), + file2Path: Path(components: [Names.sourceRoot, Names.subDir, Names.fileName]) + ) + } } } @@ -64,24 +80,24 @@ private extension PBXGroupPathSearchTests { children: [], sourceTree: .group, name: "group", - path: Path(components: [sourceRootName, groupDirName]).string + path: Path(components: [Names.sourceRoot, Names.group]).string ) let parent = PBXGroup( children: [group], sourceTree: .absolute, name: "parent", - path: projectDir.string + path: projectPath.string ) project.add(object: parent) project.add(object: group) self.group = group } - func addToGroup(testFiles: Files, sourceTree: PBXSourceTree) throws -> PathToFileReferenceMap { + func addToGroup( + testFiles: Files, sourceTree: PBXSourceTree, sourceRoot: Path + ) throws -> PathToFileReferenceMap { try [testFiles.file1Path, testFiles.file2Path] .reduce([Path: PBXFileReference]()) { map, filePath in -// let absolutePath = sourceRoot + filePath -// try Data().write(to: absolutePath.url) let file = try group.addFile( at: filePath, sourceTree: sourceTree, @@ -92,7 +108,7 @@ private extension PBXGroupPathSearchTests { } } - func checkAssertions(for sourceTree: PBXSourceTree) throws { + func checkAssertions(for sourceTree: PBXSourceTree, with files: () -> Files) throws { func assert(filePath: Path, hasReference: PBXFileReference?, line: UInt = #line) { let actual = group.file(with: filePath, sourceRoot: sourceRoot) if let expected = hasReference { @@ -102,9 +118,11 @@ private extension PBXGroupPathSearchTests { } } + let files = files() let fileReferenceFor = try addToGroup( testFiles: files, - sourceTree: sourceTree + sourceTree: sourceTree, + sourceRoot: projectPath ) assert( @@ -116,11 +134,11 @@ private extension PBXGroupPathSearchTests { hasReference: fileReferenceFor[files.file2Path] ) assert( - filePath: files.file2Path + "../.." + fileName, + filePath: files.file2Path + "../.." + Names.fileName, hasReference: fileReferenceFor[files.file1Path] ) assert( - filePath: files.file1Path + "../foo/.." + subDirName + fileName, + filePath: files.file1Path + "../foo/.." + Names.subDir + Names.fileName, hasReference: fileReferenceFor[files.file2Path] ) assert( From a3188c3be4cbf2638d429d124222dc3a4c00f741 Mon Sep 17 00:00:00 2001 From: Jonathan Crooke Date: Thu, 31 Mar 2022 17:19:09 +0100 Subject: [PATCH 7/7] Just extract existing functionality --- .../XcodeProj/Objects/Files/PBXGroup.swift | 48 +++--- .../Files/PBXGroupPathSearchTests.swift | 149 ------------------ 2 files changed, 27 insertions(+), 170 deletions(-) delete mode 100644 Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift diff --git a/Sources/XcodeProj/Objects/Files/PBXGroup.swift b/Sources/XcodeProj/Objects/Files/PBXGroup.swift index fd0ba0c96..e943db6ba 100644 --- a/Sources/XcodeProj/Objects/Files/PBXGroup.swift +++ b/Sources/XcodeProj/Objects/Files/PBXGroup.swift @@ -138,16 +138,7 @@ public extension PBXGroup { /// - sourceRoot: path to project's source root. /// - Returns: an existing reference to that file, or `nil` if not found func file(with path: Path, sourceRoot: Path) -> PBXFileReference? { - func absolutePath(for fileRef: PBXFileElement) -> Path? { - try? fileRef.fullPath(sourceRoot: sourceRoot) - } - - guard let groupAbsPath = absolutePath(for: self) else { return nil } - let fileNormalizedAbsolutePath = (groupAbsPath + path).normalize() - return children.first { - guard let candidateAbsPath = absolutePath(for: $0) else { return false } - return candidateAbsPath.normalize() == fileNormalizedAbsolutePath - } as? PBXFileReference + try? existingFileReference(with: path, sourceRoot: sourceRoot)?.value } /// Creates a group with the given name and returns it. @@ -211,17 +202,11 @@ public extension PBXGroup { } let groupPath = try fullPath(sourceRoot: sourceRoot) - if override, let existingFileReference = try projectObjects.fileReferences.first(where: { - // Optimization: compare lastComponent before fullPath compare - guard let fileRefPath = $0.value.path else { - return try filePath == $0.value.fullPath(sourceRoot: sourceRoot) - } - let fileRefLastPathComponent = fileRefPath.split(separator: "/").last! - if filePath.lastComponent == fileRefLastPathComponent { - return try filePath == $0.value.fullPath(sourceRoot: sourceRoot) - } - return false - }) { + if override, let existingFileReference = try self.existingFileReference( + from: projectObjects, + with: filePath, + sourceRoot: sourceRoot + ) { if !childrenReferences.contains(existingFileReference.key) { existingFileReference.value.path = groupPath.flatMap { filePath.relative(to: $0) }?.string childrenReferences.append(existingFileReference.key) @@ -257,3 +242,24 @@ public extension PBXGroup { return fileReference } } + +private extension PBXGroup { + + func existingFileReference( + from objects: PBXObjects? = nil, + with filePath: Path, + sourceRoot: Path + ) throws -> (key: PBXObjectReference, value: PBXFileReference)? { + try (objects ?? self.objects()).fileReferences.first(where: { + // Optimization: compare lastComponent before fullPath compare + guard let fileRefPath = $0.value.path else { + return try filePath == $0.value.fullPath(sourceRoot: sourceRoot) + } + let fileRefLastPathComponent = fileRefPath.split(separator: "/").last! + if filePath.lastComponent == fileRefLastPathComponent { + return try filePath == $0.value.fullPath(sourceRoot: sourceRoot) + } + return false + }) + } +} diff --git a/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift b/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift deleted file mode 100644 index 878868485..000000000 --- a/Tests/XcodeProjTests/Objects/Files/PBXGroupPathSearchTests.swift +++ /dev/null @@ -1,149 +0,0 @@ -import Foundation -import PathKit -import XcodeProj -import XCTest - -final class PBXGroupPathSearchTests: XCTestCase { - - struct Files { - let fileName: String - let file1Path: Path - let file2Path: Path - } - - typealias PathToFileReferenceMap = [Path: PBXFileReference] - - let groupPathComponentsCount = 2 - - enum Names { - static let fileName = "theFile" - static let sourceRoot = "src" - static let group = "theGroup" - static let subDir = "subDir" - } - - var projectPath: Path! - var sourceRoot: Path { projectPath + Names.sourceRoot } - var groupPath: Path { sourceRoot + Names.group } - var subDirPath: Path { groupPath + Names.subDir } - - var project: PBXProj! - var group: PBXGroup! - - override func setUpWithError() throws { - project = PBXProj( - rootObject: nil, - objectVersion: 0, - archiveVersion: 0, - classes: [:], - objects: [] - ) - projectPath = try Path.uniqueTemporary() - try (projectPath + Names.sourceRoot + Names.group + Names.subDir).mkpath() - try createGroup() - } - - func test_whenFilesHaveAbsoluteSourceTree_thenCanBeFound() throws { - try checkAssertions(for: .absolute) { - Files( - fileName: Names.fileName, - file1Path: groupPath + Names.fileName, - file2Path: subDirPath + Names.fileName - ) - } - } - - func test_whenFilesHaveGroupSourceTree_thenCanBeFound() throws { - try checkAssertions(for: .group) { - Files( - fileName: Names.fileName, - file1Path: Path(Names.fileName), - file2Path: Path(components: [Names.subDir, Names.fileName]) - ) - } - } - func test_whenFilesHaveSourceRootSourceTree_thenCanBeFound() throws { - try checkAssertions(for: .sourceRoot) { - Files( - fileName: Names.fileName, - file1Path: Path(components: [Names.sourceRoot, Names.fileName]), - file2Path: Path(components: [Names.sourceRoot, Names.subDir, Names.fileName]) - ) - } - } -} - -private extension PBXGroupPathSearchTests { - - func createGroup() throws { - let group = PBXGroup( - children: [], - sourceTree: .group, - name: "group", - path: Path(components: [Names.sourceRoot, Names.group]).string - ) - let parent = PBXGroup( - children: [group], - sourceTree: .absolute, - name: "parent", - path: projectPath.string - ) - project.add(object: parent) - project.add(object: group) - self.group = group - } - - func addToGroup( - testFiles: Files, sourceTree: PBXSourceTree, sourceRoot: Path - ) throws -> PathToFileReferenceMap { - try [testFiles.file1Path, testFiles.file2Path] - .reduce([Path: PBXFileReference]()) { map, filePath in - let file = try group.addFile( - at: filePath, - sourceTree: sourceTree, - sourceRoot: sourceRoot, - validatePresence: false - ) - return map.merging([filePath: file]) { _, new in new } - } - } - - func checkAssertions(for sourceTree: PBXSourceTree, with files: () -> Files) throws { - func assert(filePath: Path, hasReference: PBXFileReference?, line: UInt = #line) { - let actual = group.file(with: filePath, sourceRoot: sourceRoot) - if let expected = hasReference { - XCTAssertEqual(actual, expected, file: #file, line: line) - } else { - XCTAssertNil(actual, file: #file, line: line) - } - } - - let files = files() - let fileReferenceFor = try addToGroup( - testFiles: files, - sourceTree: sourceTree, - sourceRoot: projectPath - ) - - assert( - filePath: files.file1Path, - hasReference: fileReferenceFor[files.file1Path] - ) - assert( - filePath: files.file2Path, - hasReference: fileReferenceFor[files.file2Path] - ) - assert( - filePath: files.file2Path + "../.." + Names.fileName, - hasReference: fileReferenceFor[files.file1Path] - ) - assert( - filePath: files.file1Path + "../foo/.." + Names.subDir + Names.fileName, - hasReference: fileReferenceFor[files.file2Path] - ) - assert( - filePath: Path(UUID().uuidString), - hasReference: nil - ) - } -}