Merge pull request #697 from rgoldberg/696-lint-cleanup

Improve Swift linting, build warnings & code

.swift-format

@@ -1,5 +1,7 @@
 {
+  "indentBlankLines": false,
   "indentConditionalCompilationBlocks": false,
+  "indentSwitchCaseLabels": false,
   "indentation": {
     "spaces": 4
   },
@@ -12,10 +14,11 @@
   "maximumBlankLines": 1,
   "multiElementCollectionTrailingCommas": true,
   "prioritizeKeepingFunctionOutputTogether": true,
+  "reflowMultilineStringLiterals": true,
   "respectsExistingLineBreaks": true,
   "rules": {
     "AllPublicDeclarationsHaveDocumentation": true,
-    "AlwaysUseLiteralForEmptyCollectionInit": true,
+    "AlwaysUseLiteralForEmptyCollectionInit": false,
     "AlwaysUseLowerCamelCase": true,
     "AmbiguousTrailingClosureOverload": true,
     "BeginDocumentationCommentWithOneLineSummary": true,
@@ -57,6 +60,6 @@
   },
   "spacesAroundRangeFormationOperators": false,
   "spacesBeforeEndOfLineComments": 1,
-  "TrailingComma": false,
+  "tabWidth": 4,
   "version": 1
 }

.swiftlint.yml

@@ -8,30 +8,26 @@
 opt_in_rules:
   - all
 disabled_rules:
-  - closure_body_length
+  # eventually enable
+  - file_header
+  - one_declaration_per_file
+  - trailing_comma
+  # never enable
   - contrasted_opening_brace
   - explicit_acl
   - explicit_enum_raw_value
   - explicit_top_level_acl
   - explicit_type_interface
-  - file_header
-  - file_name
-  - force_unwrapping
-  - function_body_length
   - inert_defer
-  - legacy_objc_type
   - no_extension_access_modifier
   - no_grouping_extension
-  - number_separator
-  - one_declaration_per_file
+  - no_magic_numbers
   - prefixed_toplevel_constant
-  - quick_discouraged_call
-  - quick_discouraged_pending_test
-  - required_deinit
   - sorted_enum_cases
-  - trailing_comma
   - unused_capture_list
   - vertical_whitespace_between_cases
+file_name:
+  excluded: [Finder.swift, Utilities.swift]
 file_types_order:
   order:
     [
@@ -41,3 +37,23 @@ file_types_order:
       [preview_provider],
       [library_content_provider],
     ]
+number_separator:
+  minimum_length: 6
+type_contents_order:
+  order:
+    [
+      [case],
+      [type_alias, associated_type],
+      [subtype],
+      [type_property],
+      [instance_property],
+      [ib_inspectable],
+      [ib_outlet],
+      [initializer],
+      [deinitializer],
+      [type_method],
+      [view_life_cycle_method],
+      [ib_action],
+      [other_method],
+      [subscript],
+    ]

Sources/mas/AppStore/ISStoreAccount.swift

@@ -12,7 +12,7 @@ import StoreFoundation
 
 private let timeout = 30.0
 
-extension ISStoreAccount: StoreAccount {
+extension ISStoreAccount {
     static var primaryAccount: Promise<ISStoreAccount> {
         race(
             Promise { seal in

Sources/mas/AppStore/PurchaseDownloadObserver.swift

@@ -7,23 +7,27 @@
 //
 
 import CommerceKit
+import PromiseKit
 import StoreFoundation
 
-private let downloadingPhase: Int64 = 0
-private let installingPhase: Int64 = 1
-private let downloadedPhase: Int64 = 5
+private let downloadingPhase = 0 as Int64
+private let installingPhase = 1 as Int64
+private let downloadedPhase = 5 as Int64
 
-@objc
-class PurchaseDownloadObserver: NSObject, CKDownloadQueueObserver {
+class PurchaseDownloadObserver: CKDownloadQueueObserver {
     private let purchase: SSPurchase
-    var completionHandler: (() -> Void)?
-    var errorHandler: ((MASError) -> Void)?
+    private var completionHandler: (() -> Void)?
+    private var errorHandler: ((MASError) -> Void)?
     private var priorPhaseType: Int64?
 
     init(purchase: SSPurchase) {
         self.purchase = purchase
     }
 
+    deinit {
+        // do nothing
+    }
+
     func downloadQueue(_ queue: CKDownloadQueue, statusChangedFor download: SSDownload) {
         guard
             download.metadata.itemIdentifier == purchase.itemIdentifier,
@@ -82,17 +86,16 @@ class PurchaseDownloadObserver: NSObject, CKDownloadQueueObserver {
     }
 }
 
-struct ProgressState {
+private struct ProgressState {
     let percentComplete: Float
     let phase: String
 
     var percentage: String {
-        // swiftlint:disable:next no_magic_numbers
         String(format: "%.1f%%", floor(percentComplete * 1000) / 10)
     }
 }
 
-func progress(_ state: ProgressState) {
+private func progress(_ state: ProgressState) {
     // Don't display the progress bar if we're not on a terminal
     guard isatty(fileno(stdout)) != 0 else {
         return
@@ -112,13 +115,13 @@ private extension SSDownload {
     }
 }
 
-extension SSDownloadStatus {
+private extension SSDownloadStatus {
     var progressState: ProgressState {
         ProgressState(percentComplete: percentComplete, phase: activePhase.phaseDescription)
     }
 }
 
-extension SSDownloadPhase {
+private extension SSDownloadPhase {
     var phaseDescription: String {
         switch phaseType {
         case downloadingPhase:
@@ -130,3 +133,17 @@ extension SSDownloadPhase {
         }
     }
 }
+
+extension PurchaseDownloadObserver {
+    func observeDownloadQueue(_ downloadQueue: CKDownloadQueue = CKDownloadQueue.shared()) -> Promise<Void> {
+        let observerID = downloadQueue.add(self)
+
+        return Promise<Void> { seal in
+            errorHandler = seal.reject
+            completionHandler = seal.fulfill_
+        }
+        .ensure {
+            downloadQueue.remove(observerID)
+        }
+    }
+}

Sources/mas/AppStore/SSPurchase.swift

@@ -12,13 +12,14 @@ import StoreFoundation
 
 extension SSPurchase {
     func perform(appID: AppID, purchasing: Bool) -> Promise<Void> {
-        var parameters: [String: Any] = [
-            "productType": "C",
-            "price": 0,
-            "salableAdamId": appID,
-            "pg": "default",
-            "appExtVrsId": 0,
-        ]
+        var parameters =
+            [
+                "productType": "C",
+                "price": 0,
+                "salableAdamId": appID,
+                "pg": "default",
+                "appExtVrsId": 0,
+            ] as [String: Any]
 
         if purchasing {
             parameters["macappinstalledconfirmed"] = 1
@@ -75,17 +76,7 @@ extension SSPurchase {
                 }
         }
         .then { purchase in
-            let observer = PurchaseDownloadObserver(purchase: purchase)
-            let downloadQueue = CKDownloadQueue.shared()
-            let observerID = downloadQueue.add(observer)
-
-            return Promise<Void> { seal in
-                observer.errorHandler = seal.reject
-                observer.completionHandler = seal.fulfill_
-            }
-            .ensure {
-                downloadQueue.remove(observerID)
-            }
+            PurchaseDownloadObserver(purchase: purchase).observeDownloadQueue()
         }
     }
 }

Sources/mas/Commands/SignIn.swift

@@ -21,7 +21,7 @@ extension MAS {
         @Argument(help: "Apple ID")
         var appleID: String
         @Argument(help: "Password")
-        var password: String = ""
+        var password = ""
 
         /// Runs the command.
         func run() throws {

Sources/mas/Commands/Upgrade.swift

@@ -18,44 +18,43 @@ extension MAS {
         )
 
         @Argument(help: ArgumentHelp("App ID/app name", valueName: "app-id-or-name"))
-        var appIDOrNames: [String] = []
+        var appIDOrNames = [String]()
 
         /// Runs the command.
         func run() throws {
             try run(appLibrary: SoftwareMapAppLibrary(), searcher: ITunesSearchAppStoreSearcher())
         }
 
         func run(appLibrary: AppLibrary, searcher: AppStoreSearcher) throws {
-            let apps: [(installedApp: SoftwareProduct, storeApp: SearchResult)]
             do {
-                apps = try findOutdatedApps(appLibrary: appLibrary, searcher: searcher)
-            } catch {
-                throw error as? MASError ?? .searchFailed
-            }
-
-            guard !apps.isEmpty else {
-                return
-            }
+                let apps = try findOutdatedApps(appLibrary: appLibrary, searcher: searcher)
 
-            print("Upgrading \(apps.count) outdated application\(apps.count > 1 ? "s" : ""):")
-            print(
-                apps.map { installedApp, storeApp in
-                    "\(storeApp.trackName) (\(installedApp.bundleVersion)) -> (\(storeApp.version))"
+                guard !apps.isEmpty else {
+                    return
                 }
-                .joined(separator: "\n")
-            )
 
-            do {
-                try downloadApps(withAppIDs: apps.map(\.storeApp.trackId)).wait()
+                print("Upgrading \(apps.count) outdated application\(apps.count > 1 ? "s" : ""):")
+                print(
+                    apps.map { installedApp, storeApp in
+                        "\(storeApp.trackName) (\(installedApp.bundleVersion)) -> (\(storeApp.version))"
+                    }
+                    .joined(separator: "\n")
+                )
+
+                do {
+                    try downloadApps(withAppIDs: apps.map(\.storeApp.trackId)).wait()
+                } catch {
+                    throw error as? MASError ?? .downloadFailed(error: error as NSError)
+                }
             } catch {
-                throw error as? MASError ?? .downloadFailed(error: error as NSError)
+                throw error as? MASError ?? .searchFailed
             }
         }
 
         private func findOutdatedApps(
             appLibrary: AppLibrary,
             searcher: AppStoreSearcher
-        ) throws -> [(SoftwareProduct, SearchResult)] {
+        ) throws -> [(installedApp: SoftwareProduct, storeApp: SearchResult)] {
             let apps =
                 appIDOrNames.isEmpty
                 ? appLibrary.installedApps

Sources/mas/Controllers/AppLibrary.swift

@@ -27,6 +27,7 @@ extension AppLibrary {
     /// - Parameter appID: app ID for app(s).
     /// - Returns: [SoftwareProduct] of matching apps.
     func installedApps(withAppID appID: AppID) -> [SoftwareProduct] {
+        // swiftlint:disable:next legacy_objc_type
         let appID = NSNumber(value: appID)
         return installedApps.filter { $0.itemIdentifier == appID }
     }

Sources/mas/Controllers/SoftwareMapAppLibrary.swift

@@ -10,17 +10,24 @@ import CommerceKit
 import ScriptingBridge
 
 /// Utility for managing installed apps.
-struct SoftwareMapAppLibrary: AppLibrary {
+class SoftwareMapAppLibrary: AppLibrary {
+    /// CommerceKit's singleton manager of installed software.
+    private let softwareMap: SoftwareMap
+
     /// Array of installed software products.
-    let installedApps: [SoftwareProduct]
+    lazy var installedApps = softwareMap.allSoftwareProducts()
+        .filter { product in
+            product.bundlePath.starts(with: "/Applications/")
+        }
 
     /// Internal initializer for providing a mock software map.
     /// - Parameter softwareMap: SoftwareMap to use
     init(softwareMap: SoftwareMap = CKSoftwareMap.shared()) {
-        installedApps = softwareMap.allSoftwareProducts()
-            .filter { product in
-                product.bundlePath.starts(with: "/Applications/")
-            }
+        self.softwareMap = softwareMap
+    }
+
+    deinit {
+        // do nothing
     }
 
     /// Uninstalls all apps located at any of the elements of `appPaths`.
@@ -58,7 +65,7 @@ private func chown(paths: [String]) throws -> [String: (uid_t, gid_t)] {
         dict[path] = try getOwnerAndGroupOfItem(atPath: path)
     }
 
-    var chownedIDsByPath: [String: (uid_t, gid_t)] = [:]
+    var chownedIDsByPath = [String: (uid_t, gid_t)]()
     for (path, ownerIDs) in ownerIDsByPath {
         guard chown(path, sudoUID, sudoGID) == 0 else {
             for (chownedPath, chownedIDs) in chownedIDsByPath

Sources/mas/Formatters/Utilities.swift

@@ -13,15 +13,21 @@ import Foundation
 /// Terminal Control Sequence Indicator.
 private let csi = "\u{001B}["
 
-private var standardError = FileHandle.standardError
+private var standardError = FileHandleTextOutputStream(FileHandle.standardError)
+
+private struct FileHandleTextOutputStream: TextOutputStream {
+    private let fileHandle: FileHandle
+
+    init(_ fileHandle: FileHandle) {
+        self.fileHandle = fileHandle
+    }
 
-extension FileHandle: TextOutputStream {
     /// Appends the given string to the stream.
-    public func write(_ string: String) {
+    func write(_ string: String) {
         guard let data = string.data(using: .utf8) else {
             return
         }
-        write(data)
+        fileHandle.write(data)
     }
 }