Merge pull request #43 from SwiftGen/feature/remove-actool

Refactor asset catalog code to use our own parsing instead of `actool`.

CHANGELOG.md

@@ -38,6 +38,9 @@ Due to the removal of legacy code, there are a few breaking changes in this new
   [David Jennes](https://github.com/djbe)
   [#18](https://github.com/SwiftGen/templates/issues/18)
   [#38](https://github.com/SwiftGen/templates/issues/38)
+* Images: switch back from `actool` to an internal parser to fix numerous issues with the former.  
+  [David Jennes](https://github.com/djbe)
+  [#43](https://github.com/SwiftGen/templates/issues/43)
 
 ## 1.1.0
 

Sources/Parsers/AssetsCatalogParser.swift

@@ -7,166 +7,134 @@
 import Foundation
 import PathKit
 
+struct Catalog {
+  enum Entry {
+    case group(name: String, items: [Entry])
+    case image(name: String, value: String)
+  }
+
+  let name: String
+  let entries: [Entry]
+}
+
 public final class AssetsCatalogParser {
-  typealias Catalog = [Entry]
-  var catalogs = [String: Catalog]()
+  public enum Error: Swift.Error, CustomStringConvertible {
+    case invalidFile
+
+    public var description: String {
+      switch self {
+      case .invalidFile:
+        return "error: File must be an asset catalog"
+      }
+    }
+  }
+
+  var catalogs = [Catalog]()
 
   public init() {}
 
-  public func parseCatalog(at path: Path) {
-    guard let items = loadAssetCatalog(at: path) else { return }
+  public func parseCatalog(at path: Path) throws {
+    guard path.extension == AssetCatalog.extension else {
+      throw AssetsCatalogParser.Error.invalidFile
+    }
+
     let name = path.lastComponentWithoutExtension
+    let entries = process(folder: path)
 
-    // process recursively (and append if already exists)
-    var catalog = catalogs[name] ?? Catalog()
-    catalog += process(items: items)
-    catalogs[name] = catalog
-  }
-
-  enum Entry {
-  case group(name: String, items: [Entry])
-  case image(name: String, value: String)
+    catalogs += [Catalog(name: name, entries: entries)]
   }
 }
 
 // MARK: - Plist processing
 
 private enum AssetCatalog {
-  static let children = "children"
-  static let filename = "filename"
-  static let providesNamespace = "provides-namespace"
-  static let root = "com.apple.actool.catalog-contents"
+  static let `extension` = "xcassets"
+
+  enum Contents {
+    static let path = "Contents.json"
+    static let properties = "properties"
+    static let providesNamespace = "provides-namespace"
+  }
 
-  enum Extension {
+  enum Item {
     static let imageSet = "imageset"
 
 	/**
 	 * This is a list of supported asset catalog item types, for now we just
 	 * support `image set`s. If you want to add support for new types, just add
 	 * it to this whitelist, and add the necessary code to the
 	 * `process(items:withPrefix:)` method.
-	 *
-	 * Note: The `actool` utility that we use for parsing hase some issues. Check
-	 * this issue for more information:
-	 * https://github.com/SwiftGen/SwiftGen/issues/228
 	 */
     static let supported = [imageSet]
   }
 }
 
 extension AssetsCatalogParser {
   /**
-   This method recursively parses a tree of nodes (similar to a directory structure)
-   resulting from the `actool` utility.
-   
+   This method recursively parses a directory structure, processing each folder (files are ignored).
+  */
+  func process(folder: Path, withPrefix prefix: String = "") -> [Catalog.Entry] {
+    return (try? folder.children().flatMap {
+      process(item: $0, withPrefix: prefix)
+    }) ?? []
+  }
+
+  /**
    Each node in an asset catalog is either (there are more types, but we ignore those):
-   - An imageset, which is essentially a group containing a list of files (the latter is ignored).
-
-         <dict>
-           <key>children</key>
-           <array>
-             ...actual file items here (for example the 1x, 2x and 3x images)...
-           </array>
-           <key>filename</key>
-           <string>Tomato.imageset</string>
-         </dict>
-
-   - A group, containing sub items such as imagesets or groups. A group can provide a namespaced,
-     which means that all the sub items will have to be prefixed with their parent's name.
-
-         <dict>
-           <key>children</key>
-           <array>
-             ...sub items such as groups or imagesets...
-           </array>
-           <key>filename</key>
-           <string>Round</string>
-           <key>provides-namespace</key>
-           <true/>
-         </dict>
+     - An imageset, which is essentially a group containing a list of files (the latter is ignored).
    
-   - Parameter items: The array of items to recursively process.
+     - A group, containing sub items such as imagesets or groups. A group can provide a namespaced,
+       which means that all the sub items will have to be prefixed with their parent's name.
+
+         {
+           "properties" : {
+             "provides-namespace" : true
+           }
+         }
+
+   - Parameter folder: The directory path to recursively process.
    - Parameter prefix: The prefix to prepend values with (from namespaced groups).
    - Returns: An array of processed Entry items (a catalog).
   */
-  fileprivate func process(items: [[String: Any]], withPrefix prefix: String = "") -> Catalog {
-    var result = Catalog()
-
-    for item in items {
-      guard let filename = item[AssetCatalog.filename] as? String else { continue }
-      let path = Path(filename)
-
-      if path.extension == AssetCatalog.Extension.imageSet {
-        // this is a simple imageset
-        let imageName = path.lastComponentWithoutExtension
-
-        result += [.image(name: imageName, value: "\(prefix)\(imageName)")]
-      } else if path.extension == nil || AssetCatalog.Extension.supported.contains(path.extension ?? "") {
-        // this is a group/folder
-        let children = item[AssetCatalog.children] as? [[String: Any]] ?? []
-
-        if let providesNamespace = item[AssetCatalog.providesNamespace] as? Bool,
-          providesNamespace {
-          let processed = process(items: children, withPrefix: "\(prefix)\(filename)/")
-          result += [.group(name: filename, items: processed)]
-        } else {
-          let processed = process(items: children, withPrefix: prefix)
-          result += [.group(name: filename, items: processed)]
-        }
-      }
+  func process(item: Path, withPrefix prefix: String) -> Catalog.Entry? {
+    guard item.isDirectory else { return nil }
+    let type = item.extension ?? ""
+
+    switch (type, AssetCatalog.Item.supported.contains(type)) {
+    case (AssetCatalog.Item.imageSet, _):
+      let name = item.lastComponentWithoutExtension
+      return .image(name: name, value: "\(prefix)\(name)")
+    case ("", _), (_, true):
+      let filename = item.lastComponent
+      let subPrefix = isNamespaced(path: item) ? "\(prefix)\(filename)/" : prefix
+
+      return .group(
+        name: filename,
+        items: process(folder: item, withPrefix: subPrefix)
+      )
+    default:
+      return nil
     }
-
-    return result
   }
-}
 
-// MARK: - ACTool
-
-extension AssetsCatalogParser {
-  /**
-   Try to parse an asset catalog using the `actool` utilty. While it supports parsing
-   multiple catalogs at once, we only use it to parse one at a time.
-
-   The output of the utility is a Plist and should be similar to this:
-   ```xml
-   <?xml version="1.0" encoding="UTF-8"?>
-   <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
-   <plist version="1.0">
-   <dict>
-     <key>com.apple.actool.catalog-contents</key>
-     <array>
-       <dict>
-         <key>children</key>
-         <array>
-           ...
-         </array>
-         <key>filename</key>
-         <string>Images.xcassets</string>
-       </dict>
-     </array>
-   </dict>
-   </plist>
-   ```
-   
-   - Parameter path: The path to the catalog to parse.
-   - Returns: An array of dictionaries, representing the tree of nodes in the catalog.
-  */
-  fileprivate func loadAssetCatalog(at path: Path) -> [[String: Any]]? {
-    let command = Command("xcrun", arguments: "actool", "--print-contents", path.string)
-    let output = command.execute() as Data
-
-    // try to parse plist
-    guard let plist = try? PropertyListSerialization
-        .propertyList(from: output, format: nil) else { return nil }
+  private func isNamespaced(path: Path) -> Bool {
+    if let contents = self.contents(for: path),
+      let properties = contents[AssetCatalog.Contents.properties] as? [String: Any],
+      let providesNamespace = properties[AssetCatalog.Contents.providesNamespace] as? Bool {
+      return providesNamespace
+    } else {
+      return false
+    }
+  }
 
-    // get first parsed catalog
-    guard let contents = plist as? [String: Any],
-      let catalogs = contents[AssetCatalog.root] as? [[String: Any]],
-      let catalog = catalogs.first else { return nil }
+  private func contents(for path: Path) -> [String: Any]? {
+    let contents = path + Path(AssetCatalog.Contents.path)
 
-    // get list of children
-    guard let children = catalog[AssetCatalog.children] as? [[String: Any]] else { return nil }
+    guard let data = try? contents.read(),
+      let json = try? JSONSerialization.jsonObject(with: data, options: []) else {
+        return nil
+    }
 
-    return children
+    return json as? [String: Any]
   }
 }

Sources/Stencil/AssetsCatalogContext.swift

@@ -19,19 +19,21 @@ import Foundation
 */
 extension AssetsCatalogParser {
   public func stencilContext() -> [String: Any] {
-    let catalogs = self.catalogs.keys.sorted(by: <).map { name -> [String: Any] in
-      return [
-        "name": name,
-        "assets": structure(entries: self.catalogs[name] ?? [])
-      ]
+    let catalogs = self.catalogs
+      .sorted { lhs, rhs in lhs.name < rhs.name }
+      .map { catalog -> [String: Any] in
+        return [
+          "name": catalog.name,
+          "assets": structure(entries: catalog.entries)
+        ]
     }
 
     return [
       "catalogs": catalogs
     ]
   }
 
-  private func structure(entries: [Entry]) -> [[String: Any]] {
+  private func structure(entries: [Catalog.Entry]) -> [[String: Any]] {
     return entries.map { entry in
       switch entry {
       case let .group(name: name, items: items):

Tests/SwiftGenKitTests/ImagesTests.swift

@@ -22,9 +22,9 @@ class ImagesTests: XCTestCase {
     XCTDiffContexts(result, expected: "empty.plist", sub: .images)
   }
 
-  func testDefaults() {
+  func testDefaults() throws {
     let parser = AssetsCatalogParser()
-    parser.parseCatalog(at: Fixtures.path(for: "Images.xcassets", sub: .images))
+    try parser.parseCatalog(at: Fixtures.path(for: "Images.xcassets", sub: .images))
 
     let result = parser.stencilContext()
     XCTDiffContexts(result, expected: "defaults.plist", sub: .images)