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/AppStore/StoreAccount.swift

@@ -1,15 +0,0 @@
-//
-//  StoreAccount.swift
-//  mas
-//
-//  Created by Ben Chatelain on 4/3/18.
-//  Copyright © 2018 Andrew Naylor. All rights reserved.
-//
-
-import Foundation
-
-// periphery:ignore - save for future use in testing
-protocol StoreAccount {
-    var identifier: String { get set }
-    var dsID: NSNumber { get set }
-}

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,7 +18,7 @@ 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 {
@@ -26,36 +26,35 @@ extension MAS {
         }
 
         func run(appLibrary: AppLibrary, searcher: AppStoreSearcher) throws {
-            let apps: [(installedApp: SoftwareProduct, storeApp: SearchResult)]
             do {
-                apps = try findOutdatedApps(appLibrary: appLibrary, searcher: searcher)
+                let apps = try findOutdatedApps(appLibrary: appLibrary, searcher: searcher)
+
+                guard !apps.isEmpty else {
+                    return
+                }
+
+                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 ?? .searchFailed
             }
-
-            guard !apps.isEmpty else {
-                return
-            }
-
-            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)
-            }
         }
 
         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)
     }
 }
 

Sources/mas/Models/AppID.swift

@@ -16,6 +16,7 @@ extension AppID {
     }
 }
 
+// swiftlint:disable:next legacy_objc_type
 extension NSNumber {
     var appIDValue: AppID {
         uint64Value

Sources/mas/Models/SearchResult.swift

@@ -9,11 +9,11 @@
 struct SearchResult: Decodable {
     var currentVersionReleaseDate = ""
     var fileSizeBytes = "0"
-    var formattedPrice: String? = "0"
+    var formattedPrice = "0" as String?
     var minimumOsVersion = ""
     var sellerName = ""
-    var sellerUrl: String? = ""
-    var trackId: AppID = 0
+    var sellerUrl = "" as String?
+    var trackId = 0 as AppID
     var trackName = ""
     var trackViewUrl = ""
     var version = ""

Sources/mas/Models/SoftwareProduct.swift

@@ -16,6 +16,7 @@ protocol SoftwareProduct {
     var bundleIdentifier: String { get set }
     var bundlePath: String { get set }
     var bundleVersion: String { get set }
+    // swiftlint:disable:next legacy_objc_type
     var itemIdentifier: NSNumber { get set }
 }
 

Sources/mas/Utilities/Finder.swift

@@ -2,7 +2,7 @@
 // swiftlint:disable:next blanket_disable_command
 // swiftlint:disable attributes discouraged_none_name file_length file_types_order identifier_name
 // swiftlint:disable:next blanket_disable_command
-// swiftlint:disable implicitly_unwrapped_optional line_length missing_docs
+// swiftlint:disable implicitly_unwrapped_optional legacy_objc_type line_length missing_docs
 import AppKit
 import ScriptingBridge
 

Tests/masTests/.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": false,
-    "AlwaysUseLiteralForEmptyCollectionInit": true,
+    "AlwaysUseLiteralForEmptyCollectionInit": false,
     "AlwaysUseLowerCamelCase": true,
     "AmbiguousTrailingClosureOverload": true,
     "BeginDocumentationCommentWithOneLineSummary": true,
@@ -25,7 +28,7 @@
     "FullyIndirectEnum": true,
     "GroupNumericLiterals": true,
     "IdentifiersMustBeASCII": true,
-    "NeverForceUnwrap": false,
+    "NeverForceUnwrap": true,
     "NeverUseForceTry": false,
     "NeverUseImplicitlyUnwrappedOptionals": true,
     "NoAccessLevelOnExtensionDeclaration": false,
@@ -57,6 +60,6 @@
   },
   "spacesAroundRangeFormationOperators": false,
   "spacesBeforeEndOfLineComments": 1,
-  "TrailingComma": false,
+  "tabWidth": 4,
   "version": 1
 }

Tests/masTests/.swiftlint.yml

@@ -6,8 +6,13 @@
 #
 ---
 disabled_rules:
+  - closure_body_length
   - force_cast
   - force_try
+  - function_body_length
   - implicitly_unwrapped_optional
   - large_tuple
-  - no_magic_numbers
+  - legacy_objc_type
+  - quick_discouraged_call
+  - quick_discouraged_pending_test
+  - required_deinit

Tests/masTests/Commands/UninstallSpec.swift

@@ -14,7 +14,7 @@ import Quick
 
 public final class UninstallSpec: QuickSpec {
     override public func spec() {
-        let appID: AppID = 12345
+        let appID = 12345 as AppID
         let app = MockSoftwareProduct(
             appName: "Some App",
             bundleIdentifier: "com.some.app",

Tests/masTests/Controllers/ITunesSearchAppStoreSearcherSpec.swift

@@ -71,7 +71,7 @@ public final class ITunesSearchAppStoreSearcherSpec: QuickSpec {
 
             context("when lookup used") {
                 it("can find slack") {
-                    let appID: AppID = 803_453_959
+                    let appID = 803_453_959 as AppID
                     let networkSession = MockFromFileNetworkSession(responseFile: "lookup/slack.json")
                     let searcher = ITunesSearchAppStoreSearcher(networkManager: NetworkManager(session: networkSession))