Skip to content

Commit

Permalink
Fix target attributes value mismatch
Browse files Browse the repository at this point in the history
SystemCapabilities attributes triggered value mismatch when the attributes where listed in different order.
Ordered both project and target attributes to ensure the comparison is not affected by the order of the attributes.

Test Plan:
- Ensure all CI checks pass
- Inspect changes in snapshots files
- Copy the same project with SystemCapabilities attributes, re-order manually attributes in one of the projects and do the diff, verify xcdiff does not report any differences

Signed-off-by: Marcin Iwanicki <[email protected]>
  • Loading branch information
marciniwanicki committed Jan 9, 2023
1 parent acd6146 commit c055f68
Show file tree
Hide file tree
Showing 19 changed files with 352 additions and 34 deletions.
2 changes: 1 addition & 1 deletion CommandTests/Generated/html_format_verbose.2.3ef640ed.md

Large diffs are not rendered by default.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions CommandTests/Generated/p1_p2_json_format.2.e54c06ce.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion CommandTests/Generated/p1_p2_verbose.2.fe666557.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 31 additions & 0 deletions Fixtures/ios_project_1/Project.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,37 @@
2E26637322B7F52500BA59BC = {
CreatedOnToolsVersion = 10.2.1;
LastSwiftMigration = 1140;
SystemCapabilities = {
com.apple.ApplicationGroups.iOS = {
enabled = 1;
};
com.apple.BackgroundModes = {
enabled = 1;
};
com.apple.DataProtection = {
enabled = 1;
};
com.apple.Keychain = {
enabled = 1;
};
com.apple.Push = {
enabled = 1;
};
com.apple.SafariKeychain = {
enabled = 1;
};
com.apple.Siri = {
enabled = 1;
};
};
TestOrder = {
com.apple.Siri = {
enabled = 1;
};
com.apple.SafariKeychain = {
enabled = 1;
};
};
};
2E26638722B7F52600BA59BC = {
CreatedOnToolsVersion = 10.2.1;
Expand Down
25 changes: 25 additions & 0 deletions Fixtures/ios_project_2/Project.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,31 @@
2E26637322B7F52500BA59BC = {
CreatedOnToolsVersion = 10.2.1;
LastSwiftMigration = 1020;
SystemCapabilities = {
com.apple.SafariKeychain = {
enabled = 1;
};
com.apple.Siri = {
enabled = 1;
};
com.apple.ApplicationGroups.iOS = {
enabled = 1;
};
com.apple.Keychain = {
enabled = 1;
};
com.apple.Push = {
enabled = 0;
};
};
TestOrder = {
com.apple.SafariKeychain = {
enabled = 1;
};
com.apple.Siri = {
enabled = 1;
};
};
};
2E26638722B7F52600BA59BC = {
CreatedOnToolsVersion = 10.2.1;
Expand Down
12 changes: 6 additions & 6 deletions Sources/XCDiffCore/Comparator/AttributesComparator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ final class AttributesComparator: Comparator {

private func compareValues(
context: [String],
first: [String: String],
second: [String: String]
first: [String: AttributeValue],
second: [String: AttributeValue]
) -> CompareResult {
let firstKeys = Array(first.keys)
let secondKeys = Array(second.keys)
Expand All @@ -87,8 +87,8 @@ final class AttributesComparator: Comparator {
let secondAttribute = second[name]
guard firstAttribute == secondAttribute else {
return .init(context: name,
first: firstAttribute,
second: secondAttribute)
first: firstAttribute?.description,
second: secondAttribute?.description)
}

return nil
Expand All @@ -103,7 +103,7 @@ final class AttributesComparator: Comparator {
)
}

private func keyAndValue(_ key: String, attributes: [String: String]) -> String {
return "\(key) = \(attributes[key] ?? "nil")"
private func keyAndValue(_ key: String, attributes: [String: AttributeValue]) -> String {
return "\(key) = \(attributes[key]?.description ?? "nil")"
}
}
66 changes: 49 additions & 17 deletions Sources/XCDiffCore/Library/TargetsHelper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,37 @@ enum DependencyDescriptorType: String {
case optional
}

enum AttributeValue: Equatable, CustomStringConvertible {
case dictionary([String: AttributeValue])
case string(String)

var description: String {
switch self {
case let .string(string):
return "\(string)"
case let .dictionary(dictionary):
return string(from: dictionary)
}
}

private func string(from dictionary: [String: AttributeValue]) -> String {
let value = dictionary
.sorted { $0.key < $1.key }
.map { "\"\($0.key)\": \(string(from: $0.value))" }
.joined(separator: ", ")
return "[\(value)]"
}

private func string(from value: AttributeValue) -> String {
switch value {
case let .dictionary(dictionary):
return string(from: dictionary)
case let .string(string):
return "\"\(string)\""
}
}
}

final class TargetsHelper {
private let pathHelper = PathHelper()

Expand Down Expand Up @@ -154,17 +185,15 @@ final class TargetsHelper {
}
}

func attributes(from pbxproj: PBXProj) throws -> [String: String] {
func attributes(from pbxproj: PBXProj) throws -> [String: AttributeValue] {
guard let rootProject = try pbxproj.rootProject() else {
return [:]
}

return rootProject.attributes.mapValues {
"\($0)"
}
return rootProject.attributes.mapValues(attributeValue)
}

func targetAttributes(pbxproj: PBXProj, target: PBXTarget) throws -> [String: String] {
func targetAttributes(pbxproj: PBXProj, target: PBXTarget) throws -> [String: AttributeValue] {
guard let rootProject = try pbxproj.rootProject(),
let attributes = rootProject.targetAttributes[target] else {
return [:]
Expand All @@ -178,19 +207,12 @@ final class TargetsHelper {
)

return attributes.mapValues { value in
switch value {
case let pbxTarget as PBXTarget:
return pbxTarget.name
default:
let stringValue = "\(value)"

// Some attributes like `TestTargetID` store references to other targets
// as such those need to be resolved to allow semantic comparisons
if let target = targetsLookup[stringValue] {
return target.name
}
return stringValue
// Some attributes like `TestTargetID` store references to other targets
// as such those need to be resolved to allow semantic comparisons
if let target = targetsLookup["\(value)"] {
return .string(target.name)
}
return attributeValue(value)
}
}

Expand All @@ -215,6 +237,16 @@ final class TargetsHelper {
private func path(from fileElement: PBXFileElement?, sourceRoot: Path) throws -> String? {
return try pathHelper.fullPath(from: fileElement, sourceRoot: sourceRoot) ?? fileElement?.path
}

private func attributeValue(_ value: Any) -> AttributeValue {
if let pbxTarget = value as? PBXTarget {
return .string(pbxTarget.name)
}
if let dictionary = value as? [String: Any] {
return .dictionary(dictionary.mapValues(attributeValue))
}
return .string(String(describing: value))
}
}

// MARK: - XcodeProj Extensions
Expand Down
Loading

0 comments on commit c055f68

Please sign in to comment.