Actually lint/format the bulk of our swift code (#1093)

* Fix swiftformat issues, update swiftformat circleci

* Configure swiftlint to lint all of our swift code, disable annoying lints, and fix the reasonable ones

.circleci/config.yml

@@ -193,7 +193,7 @@ jobs:
       - checkout
       - run: brew install swiftlint swiftformat
       - run: swiftlint --strict
-      - run: swiftformat --lint --swiftversion 4 --verbose megazords
+      - run: swiftformat megazords components/*/ios --lint --swiftversion 4 --verbose
   Check Rust formatting:
     docker:
       - image: circleci/rust:latest

.github/pull_request_template.md

@@ -5,7 +5,7 @@
   - `cargo clippy --all --all-targets --all-features` runs without emitting any warnings
   - `cargo fmt` does not produce any changes to the code
   - `./gradlew ktlint detekt` runs without emitting any warnings
-  - `swiftformat --lint --swiftversion 4 megazords && swiftlint` runs without emitting any warnings
+  - `swiftformat --swiftversion 4 megazords components/*/ios && swiftlint` runs without emitting any warnings or producing changes
   - Note: For changes that need extra cross-platform testing, consider adding `[ci full]` to the PR title.
 - [ ] **Tests**: This PR includes thorough tests or an explanation of why it does not
 - [ ] **Changelog**: This PR includes a changelog entry or an explanation of why it does not need one

.swiftlint.yml

@@ -1,5 +1,29 @@
 included: # paths to include during linting. `--path` is ignored if present.
   - megazords
+  - components/places/ios
+  - components/support/ios
+  - components/logins/ios
+  - components/fxa-client/ios
 excluded:
   - Carthage
   - "megazords/ios/MozillaAppServicesTests"
+
+disabled_rules:
+  - file_length
+  # We're pretty careful about this already, but it's a pain to disable
+  # and reenable in the cases where we're sure.
+  - force_try
+  # `switch`es, like the ones we have in error conversion, get hit by
+  # this hard, and it's a dodgy metric to begin with.
+  - cyclomatic_complexity
+  # We'll get to these when we get to them!
+  - todo
+  # It disagrees with swiftformat on this, and thinks trailing commas are bad...
+  - trailing_comma
+
+identifier_name:
+  # Turn off it complaining about `id` or `let t = title`, etc, but keep
+  # warnings around e.g. enum names.
+  min_length:
+    warning: 0
+    error: 0

components/fxa-client/ios/FxAClient/Errors/FxAError.swift

@@ -4,6 +4,9 @@
 
 import Foundation
 
+// FIXME: these should be lower case.
+// swiftlint:disable identifier_name
+
 public enum FirefoxAccountError: Error {
     case Unauthorized(message: String)
     case Network(message: String)

components/fxa-client/ios/FxAClient/FirefoxAccount.swift

@@ -7,12 +7,16 @@ import os.log
 import UIKit
 
 open class FxAConfig {
+    // FIXME: these should be lower case.
+    // swiftlint:disable identifier_name
     public enum Server: String {
         case Release = "https://accounts.firefox.com"
         case Stable = "https://stable.dev.lcip.org"
         case Dev = "https://accounts.stage.mozaws.net"
     }
 
+    // swiftlint:enable identifier_name
+
     let contentUrl: String
     let clientId: String
     let redirectUri: String
@@ -24,21 +28,21 @@ open class FxAConfig {
     }
 
     public init(withServer server: Server, clientId: String, redirectUri: String) {
-        self.contentUrl = server.rawValue
+        contentUrl = server.rawValue
         self.clientId = clientId
         self.redirectUri = redirectUri
     }
 
     public static func release(clientId: String, redirectUri: String) -> FxAConfig {
-        return FxAConfig.init(withServer: FxAConfig.Server.Release, clientId: clientId, redirectUri: redirectUri)
+        return FxAConfig(withServer: FxAConfig.Server.Release, clientId: clientId, redirectUri: redirectUri)
     }
 
     public static func stable(clientId: String, redirectUri: String) -> FxAConfig {
-        return FxAConfig.init(withServer: FxAConfig.Server.Stable, clientId: clientId, redirectUri: redirectUri)
+        return FxAConfig(withServer: FxAConfig.Server.Stable, clientId: clientId, redirectUri: redirectUri)
     }
 
     public static func dev(clientId: String, redirectUri: String) -> FxAConfig {
-        return FxAConfig.init(withServer: FxAConfig.Server.Dev, clientId: clientId, redirectUri: redirectUri)
+        return FxAConfig(withServer: FxAConfig.Server.Dev, clientId: clientId, redirectUri: redirectUri)
     }
 }
 
@@ -61,59 +65,61 @@ open class FirefoxAccount {
     /// Please note that the `FxAConfig` provided will be consumed and therefore
     /// should not be re-used.
     public convenience init(config: FxAConfig) throws {
-        let pointer = try queue.sync(execute: {
-            return try FirefoxAccountError.unwrap({err in
+        let pointer = try queue.sync {
+            try FirefoxAccountError.unwrap { err in
                 fxa_new(config.contentUrl, config.clientId, config.redirectUri, err)
-            })
-        })
+            }
+        }
         self.init(raw: pointer)
     }
 
     /// Restore a previous instance of `FirefoxAccount` from a serialized state (obtained with `toJSON(...)`).
     open class func fromJSON(state: String) throws -> FirefoxAccount {
-        return try queue.sync(execute: {
-            let handle = try FirefoxAccountError.unwrap({ err in fxa_from_json(state, err) })
+        return try queue.sync {
+            let handle = try FirefoxAccountError.unwrap { err in fxa_from_json(state, err) }
             return FirefoxAccount(raw: handle)
-        })
+        }
     }
 
     deinit {
         if self.raw != 0 {
-            queue.sync(execute: {
-                try! FirefoxAccountError.unwrap({err in
+            queue.sync {
+                try! FirefoxAccountError.unwrap { err in
                     // Is `try!` the right thing to do? We should only hit an error here
                     // for panics and handle misuse, both inidicate bugs in our code
                     // (the first in the rust code, the 2nd in this swift wrapper).
                     fxa_free(self.raw, err)
-                })
-            })
+                }
+            }
         }
     }
 
-    /// Serializes the state of a `FirefoxAccount` instance. It can be restored later with `fromJSON(...)`.
-    /// It is the responsability of the caller to persist that serialized state regularly (after operations that mutate `FirefoxAccount`) in a **secure** location.
+    /// Serializes the state of a `FirefoxAccount` instance. It can be restored
+    /// later with `fromJSON(...)`. It is the responsability of the caller to
+    /// persist that serialized state regularly (after operations that mutate
+    /// `FirefoxAccount`) in a **secure** location.
     open func toJSON() throws -> String {
-        return try queue.sync(execute: {
-            return try self.toJSONInternal()
-        })
+        return try queue.sync {
+            try self.toJSONInternal()
+        }
     }
 
     private func toJSONInternal() throws -> String {
-        return String(freeingFxaString: try FirefoxAccountError.unwrap({err in
+        return String(freeingFxaString: try FirefoxAccountError.unwrap { err in
             fxa_to_json(self.raw, err)
-        }))
+        })
     }
 
     /// Registers a persistance callback. The callback will get called every time
     /// the `FirefoxAccount` state needs to be saved. The callback must
     /// persist the passed string in a secure location (like the keychain).
     public func registerPersistCallback(_ cb: PersistCallback) {
-        self.persistCallback = cb
+        persistCallback = cb
     }
 
     /// Unregisters a persistance callback.
     public func unregisterPersistCallback() {
-        self.persistCallback = nil
+        persistCallback = nil
     }
 
     private func tryPersistState() {
@@ -141,9 +147,9 @@ open class FirefoxAccount {
     open func getProfile(completionHandler: @escaping (Profile?, Error?) -> Void) {
         queue.async {
             do {
-                let profileBuffer = try FirefoxAccountError.unwrap({err in
+                let profileBuffer = try FirefoxAccountError.unwrap { err in
                     fxa_profile(self.raw, false, err)
-                })
+                }
                 let msg = try! MsgTypes_Profile(serializedData: Data(rustBuffer: profileBuffer))
                 fxa_bytebuffer_free(profileBuffer)
                 let profile = Profile(msg: msg)
@@ -155,35 +161,35 @@ open class FirefoxAccount {
     }
 
     open func getTokenServerEndpointURL() throws -> URL {
-        return try queue.sync(execute: {
-            return URL(string: String(freeingFxaString: try FirefoxAccountError.unwrap({err in
+        return try queue.sync {
+            URL(string: String(freeingFxaString: try FirefoxAccountError.unwrap { err in
                 fxa_get_token_server_endpoint_url(self.raw, err)
-            })))!
-        })
+            }))!
+        }
     }
 
     open func getConnectionSuccessURL() throws -> URL {
-        return try queue.sync(execute: {
-            return URL(string: String(freeingFxaString: try FirefoxAccountError.unwrap({err in
+        return try queue.sync {
+            URL(string: String(freeingFxaString: try FirefoxAccountError.unwrap { err in
                 fxa_get_connection_success_url(self.raw, err)
-            })))!
-        })
+            }))!
+        }
     }
 
     open func getManageAccountURL(entrypoint: String) throws -> URL {
-        return try queue.sync(execute: {
-            return URL(string: String(freeingFxaString: try FirefoxAccountError.unwrap({err in
+        return try queue.sync {
+            URL(string: String(freeingFxaString: try FirefoxAccountError.unwrap { err in
                 fxa_get_manage_account_url(self.raw, entrypoint, err)
-            })))!
-        })
+            }))!
+        }
     }
 
     open func getManageDevicesURL(entrypoint: String) throws -> URL {
-        return try queue.sync(execute: {
-            return URL(string: String(freeingFxaString: try FirefoxAccountError.unwrap({err in
+        return try queue.sync {
+            URL(string: String(freeingFxaString: try FirefoxAccountError.unwrap { err in
                 fxa_get_manage_devices_url(self.raw, entrypoint, err)
-            })))!
-        })
+            }))!
+        }
     }
 
     /// Request a OAuth token by starting a new OAuth flow.
@@ -199,9 +205,9 @@ open class FirefoxAccount {
         queue.async {
             do {
                 let scope = scopes.joined(separator: " ")
-                let url = URL(string: String(freeingFxaString: try FirefoxAccountError.unwrap({err in
+                let url = URL(string: String(freeingFxaString: try FirefoxAccountError.unwrap { err in
                     fxa_begin_oauth_flow(self.raw, scope, wantsKeys, err)
-                })))!
+                }))!
                 DispatchQueue.main.async { completionHandler(url, nil) }
             } catch {
                 DispatchQueue.main.async { completionHandler(nil, error) }
@@ -216,9 +222,9 @@ open class FirefoxAccount {
     open func completeOAuthFlow(code: String, state: String, completionHandler: @escaping (Void, Error?) -> Void) {
         queue.async {
             do {
-                try FirefoxAccountError.unwrap({err in
+                try FirefoxAccountError.unwrap { err in
                     fxa_complete_oauth_flow(self.raw, code, state, err)
-                })
+                }
                 DispatchQueue.main.async { completionHandler((), nil) }
                 self.tryPersistState()
             } catch {
@@ -235,9 +241,9 @@ open class FirefoxAccount {
     open func getAccessToken(scope: String, completionHandler: @escaping (AccessTokenInfo?, Error?) -> Void) {
         queue.async {
             do {
-                let infoBuffer = try FirefoxAccountError.unwrap({err in
+                let infoBuffer = try FirefoxAccountError.unwrap { err in
                     fxa_get_access_token(self.raw, scope, err)
-                })
+                }
                 let msg = try! MsgTypes_AccessTokenInfo(serializedData: Data(rustBuffer: infoBuffer))
                 fxa_bytebuffer_free(infoBuffer)
                 let tokenInfo = AccessTokenInfo(msg: msg)
@@ -256,10 +262,10 @@ public struct ScopedKey {
     public let kid: String
 
     internal init(msg: MsgTypes_ScopedKey) {
-        self.kty = msg.kty
-        self.scope = msg.scope
-        self.k = msg.k
-        self.kid = msg.kid
+        kty = msg.kty
+        scope = msg.scope
+        k = msg.k
+        kid = msg.kid
     }
 }
 
@@ -270,10 +276,10 @@ public struct AccessTokenInfo {
     public let expiresAt: Date
 
     internal init(msg: MsgTypes_AccessTokenInfo) {
-        self.scope = msg.scope
-        self.token = msg.token
-        self.key = msg.hasKey ? ScopedKey(msg: msg.key) : nil
-        self.expiresAt = Date.init(timeIntervalSince1970: Double(msg.expiresAt))
+        scope = msg.scope
+        token = msg.token
+        key = msg.hasKey ? ScopedKey(msg: msg.key) : nil
+        expiresAt = Date(timeIntervalSince1970: Double(msg.expiresAt))
     }
 }
 
@@ -289,9 +295,9 @@ public struct Profile {
     public let displayName: String?
 
     internal init(msg: MsgTypes_Profile) {
-        self.uid = msg.uid
-        self.email = msg.email
-        self.avatar = msg.hasAvatar ? Avatar(url: msg.avatar, isDefault: msg.avatarDefault) : nil
-        self.displayName = msg.hasDisplayName ? msg.displayName : nil
+        uid = msg.uid
+        email = msg.email
+        avatar = msg.hasAvatar ? Avatar(url: msg.avatar, isDefault: msg.avatarDefault) : nil
+        displayName = msg.hasDisplayName ? msg.displayName : nil
     }
 }

components/logins/ios/Logins/Errors/LoginStoreError.swift

@@ -4,9 +4,11 @@
 
 import Foundation
 
+// https://github.com/mozilla/application-services/issues/1042
+// swiftlint:disable identifier_name
+
 /// Indicates an error occurred while calling into the logins storage layer
 public enum LoginsStoreError: Error {
-
     /// This is a catch-all error code used for errors not yet exposed to consumers,
     /// typically since it doesn't seem like there's a sane way for them to be handled.
     case Unspecified(message: String)
@@ -89,9 +91,9 @@ public enum LoginsStoreError: Error {
     }
 
     @discardableResult
-    public static func unwrap<T>(_ callback: (UnsafeMutablePointer<Sync15PasswordsError>) throws -> T?) throws -> T {
+    public static func unwrap<T>(_ fn: (UnsafeMutablePointer<Sync15PasswordsError>) throws -> T?) throws -> T {
         var err = Sync15PasswordsError(code: Sync15Passwords_NoError, message: nil)
-        guard let result = try callback(&err) else {
+        guard let result = try fn(&err) else {
             if let loginErr = LoginsStoreError.fromConsuming(err) {
                 throw loginErr
             }
@@ -106,9 +108,9 @@ public enum LoginsStoreError: Error {
     }
 
     @discardableResult
-    public static func tryUnwrap<T>(_ callback: (UnsafeMutablePointer<Sync15PasswordsError>) throws -> T?) throws -> T? {
+    public static func tryUnwrap<T>(_ fn: (UnsafeMutablePointer<Sync15PasswordsError>) throws -> T?) throws -> T? {
         var err = Sync15PasswordsError(code: Sync15Passwords_NoError, message: nil)
-        guard let result = try callback(&err) else {
+        guard let result = try fn(&err) else {
             if let loginErr = LoginsStoreError.fromConsuming(err) {
                 throw loginErr
             }

components/logins/ios/Logins/LoginRecord.swift

@@ -22,21 +22,21 @@ open class LoginRecord {
     public var password: String
 
     /// This record's username, if any.
-    public var username: String? = nil
+    public var username: String?
 
     /// The challenge string for HTTP Basic authentication.
     ///
     /// Exactly one of `httpRealm` or `formSubmitURL` is allowed to be present,
     /// and attempting to insert or update a record to have both or neither will
     /// result in an `LoginsStoreError.InvalidLogin`.
-    public var httpRealm: String? = nil
+    public var httpRealm: String?
 
     /// The submission URL for the form where this login may be entered.
     ///
     /// As mentioned above, exactly one of `httpRealm` or `formSubmitURL` is allowed
     /// to be present, and attempting to insert or update a record to have
     /// both or neither will result in an `LoginsStoreError.InvalidLogin`.
-    public var formSubmitURL: String? = nil
+    public var formSubmitURL: String?
 
     /// A lower bound on the number of times this record has been "used".
     ///
@@ -67,10 +67,10 @@ open class LoginRecord {
     public var timePasswordChanged: Int64 = 0
 
     /// HTML field name of the username, if known.
-    public var usernameField: String? = nil
+    public var usernameField: String?
 
     /// HTML field name of the password, if known.
-    public var passwordField: String? = nil
+    public var passwordField: String?
 
     open func toJSONDict() -> [String: Any] {
         var dict: [String: Any] = [
@@ -108,7 +108,7 @@ open class LoginRecord {
 
     open func toJSON() throws -> String {
         // We need a String to pass back to rust.
-        let data: Data = try JSONSerialization.data(withJSONObject: self.toJSONDict())
+        let data: Data = try JSONSerialization.data(withJSONObject: toJSONDict())
         return String(data: data, encoding: String.Encoding.utf8)!
     }
 
@@ -136,44 +136,43 @@ open class LoginRecord {
         )
     }
 
-
-
     init(id: String,
-        password: String,
-        hostname: String,
-        username: String?,
-        formSubmitURL: String?,
-        httpRealm: String?,
-        timesUsed: Int?,
-        timeLastUsed: Int64?,
-        timeCreated: Int64?,
-        timePasswordChanged: Int64?,
-        usernameField: String?,
-        passwordField: String?) {
-            self.id = id
-            self.password = password
-            self.hostname = hostname
-            self.username = username
-            self.formSubmitURL = formSubmitURL
-            self.httpRealm = httpRealm
-            self.timesUsed = timesUsed ?? 0
-            self.timeLastUsed = timeLastUsed ?? 0
-            self.timeCreated = timeCreated ?? 0
-            self.timePasswordChanged = timePasswordChanged ?? 0
-            self.usernameField = usernameField
-            self.passwordField = passwordField
+         password: String,
+         hostname: String,
+         username: String?,
+         formSubmitURL: String?,
+         httpRealm: String?,
+         timesUsed: Int?,
+         timeLastUsed: Int64?,
+         timeCreated: Int64?,
+         timePasswordChanged: Int64?,
+         usernameField: String?,
+         passwordField: String?) {
+        self.id = id
+        self.password = password
+        self.hostname = hostname
+        self.username = username
+        self.formSubmitURL = formSubmitURL
+        self.httpRealm = httpRealm
+        self.timesUsed = timesUsed ?? 0
+        self.timeLastUsed = timeLastUsed ?? 0
+        self.timeCreated = timeCreated ?? 0
+        self.timePasswordChanged = timePasswordChanged ?? 0
+        self.usernameField = usernameField
+        self.passwordField = passwordField
     }
 
-
     public convenience init(fromJSONString json: String) throws {
-        let dict = try JSONSerialization.jsonObject(with: json.data(using: .utf8)!, options: []) as? [String: Any] ?? [String: Any]()
-        self.init(fromJSONDict: dict)
+        let dict = try JSONSerialization.jsonObject(with: json.data(using: .utf8)!,
+                                                    options: [])
+        self.init(fromJSONDict: dict as? [String: Any] ?? [String: Any]())
     }
 
     public static func fromJSONArray(_ jsonArray: String) throws -> [LoginRecord] {
-        if  let arr = try JSONSerialization.jsonObject(with: jsonArray.data(using: .utf8)!, options: []) as? [[String: Any]] {
+        if let arr = try JSONSerialization.jsonObject(with: jsonArray.data(using: .utf8)!,
+                                                      options: []) as? [[String: Any]] {
             return arr.map { (dict) -> LoginRecord in
-                return LoginRecord(fromJSONDict: dict)
+                LoginRecord(fromJSONDict: dict)
             }
         }
         return [LoginRecord]()

components/logins/ios/Logins/LoginsStorage.swift

@@ -5,19 +5,19 @@
 import Foundation
 import UIKit
 
-fileprivate let queue = DispatchQueue(label: "com.mozilla.logins-storage")
+private let queue = DispatchQueue(label: "com.mozilla.logins-storage")
 
 open class LoginsStorage {
     private var raw: UInt64 = 0
     let dbPath: String
-    private var interrupt_handle: LoginsInterruptHandle?
+    private var interruptHandle: LoginsInterruptHandle?
     // It's not 100% clear to me that this is necessary, but without it
-    // we might have a data race between reading `interrupt_handle` in
+    // we might have a data race between reading `interruptHandle` in
     // `interrupt()`, and writing it in `doDestroy` (or `doOpen`)
-    private let interrupt_handle_lock: NSLock = NSLock()
+    private let interruptHandleLock: NSLock = NSLock()
 
     public init(databasePath: String) {
-        self.dbPath = databasePath
+        dbPath = databasePath
     }
 
     deinit {
@@ -31,54 +31,54 @@ open class LoginsStorage {
             // Is `try!` the right thing to do? We should only hit an error here
             // for panics and handle misuse, both inidicate bugs in our code
             // (the first in the rust code, the 2nd in this swift wrapper).
-            try! LoginsStoreError.unwrap({ err in
+            try! LoginsStoreError.unwrap { err in
                 sync15_passwords_state_destroy(raw, err)
-            })
-            self.interrupt_handle_lock.lock()
-            self.interrupt_handle = nil
-            self.interrupt_handle_lock.unlock()
+            }
+            interruptHandleLock.lock()
+            defer { self.interruptHandleLock.unlock() }
+            interruptHandle = nil
         }
     }
 
     /// Manually close the database (this is automatically called from deinit(), so
     /// manually calling it is usually unnecessary).
     open func close() {
-        queue.sync(execute: {
+        queue.sync {
             self.doDestroy()
-        })
+        }
     }
 
     /// Test if the database is locked.
     open func isLocked() -> Bool {
-        return queue.sync(execute: {
-            return self.raw == 0
-        })
+        return queue.sync {
+            self.raw == 0
+        }
     }
 
     // helper to reduce boilerplate, we don't use queue.sync
     // since we expect the caller to do so.
     private func getUnlocked() throws -> UInt64 {
-        if self.raw == 0 {
+        if raw == 0 {
             throw LockError.mismatched
         }
-        return self.raw
+        return raw
     }
 
     private func doOpen(_ key: String) throws {
-        if self.raw != 0 {
+        if raw != 0 {
             return
         }
 
-        self.raw = try LoginsStoreError.unwrap({ err in
+        raw = try LoginsStoreError.unwrap({ err in
             sync15_passwords_state_new(self.dbPath, key, err)
         })
 
         do {
-            self.interrupt_handle_lock.lock()
-            defer { self.interrupt_handle_lock.unlock() }
-            self.interrupt_handle = LoginsInterruptHandle(ptr: try LoginsStoreError.unwrap({err in
+            interruptHandleLock.lock()
+            defer { self.interruptHandleLock.unlock() }
+            interruptHandle = LoginsInterruptHandle(ptr: try LoginsStoreError.unwrap { err in
                 sync15_passwords_new_interrupt_handle(self.raw, err)
-            }))
+            })
         } catch let e {
             // This should only happen on panic, but make sure we don't
             // leak a database in that case.
@@ -94,32 +94,32 @@ open class LoginsStorage {
     /// Throws a `LoginStoreError.InvalidKey` if the key is incorrect, or if dbPath does not point
     /// to a database, (may also throw `LoginStoreError.Unspecified` or `.Panic`).
     open func unlock(withEncryptionKey key: String) throws {
-        try queue.sync(execute: {
+        try queue.sync {
             if self.raw != 0 {
                 throw LockError.mismatched
             }
             try self.doOpen(key)
-        })
+        }
     }
 
     /// equivalent to `unlock(withEncryptionKey:)`, but does not throw if the
     /// database is already unlocked.
     open func ensureUnlocked(withEncryptionKey key: String) throws {
-        try queue.sync(execute: {
+        try queue.sync {
             try self.doOpen(key)
-        })
+        }
     }
 
     /// Lock the database.
     ///
     /// Throws `LockError.mismatched` if the database is already locked.
     open func lock() throws {
-        try queue.sync(execute: {
+        try queue.sync {
             if self.raw == 0 {
                 throw LockError.mismatched
             }
             self.doDestroy()
-        })
+        }
     }
 
     /// Locks the database, but does not throw in the case that the database is
@@ -131,76 +131,81 @@ open class LoginsStorage {
 
     /// Synchronize with the server.
     open func sync(unlockInfo: SyncUnlockInfo) throws {
-        try queue.sync(execute: {
+        try queue.sync {
             let engine = try self.getUnlocked()
-            try LoginsStoreError.unwrap({ err in
-                sync15_passwords_sync(engine, unlockInfo.kid, unlockInfo.fxaAccessToken, unlockInfo.syncKey, unlockInfo.tokenserverURL, err)
-            })
-        })
+            try LoginsStoreError.unwrap { err in
+                sync15_passwords_sync(engine,
+                                      unlockInfo.kid,
+                                      unlockInfo.fxaAccessToken,
+                                      unlockInfo.syncKey,
+                                      unlockInfo.tokenserverURL,
+                                      err)
+            }
+        }
     }
 
     /// Delete all locally stored login sync metadata. It's unclear if
     /// there's ever a reason for users to call this
     open func reset() throws {
-        try queue.sync(execute: {
+        try queue.sync {
             let engine = try self.getUnlocked()
-            try LoginsStoreError.unwrap({ err in
+            try LoginsStoreError.unwrap { err in
                 sync15_passwords_reset(engine, err)
-            })
-        })
+            }
+        }
     }
 
     /// Disable memory security, which prevents keys from being swapped to disk.
     /// This allows some esoteric attacks, but can have a performance benefit.
     open func disableMemSecurity() throws {
-        try queue.sync(execute: {
+        try queue.sync {
             let engine = try self.getUnlocked()
-            try LoginsStoreError.unwrap({ err in
+            try LoginsStoreError.unwrap { err in
                 sync15_passwords_disable_mem_security(engine, err)
-            })
-        })
+            }
+        }
     }
 
     /// Delete all locally stored login data.
     open func wipe() throws {
-        try queue.sync(execute: {
+        try queue.sync {
             let engine = try self.getUnlocked()
-            try LoginsStoreError.unwrap({ err in
+            try LoginsStoreError.unwrap { err in
                 sync15_passwords_wipe(engine, err)
-            })
-        })
+            }
+        }
     }
 
     open func wipeLocal() throws {
-        try queue.sync(execute: {
+        try queue.sync {
             let engine = try self.getUnlocked()
-            try LoginsStoreError.unwrap({ err in
+            try LoginsStoreError.unwrap { err in
                 sync15_passwords_wipe_local(engine, err)
-            })
-        })
+            }
+        }
     }
 
     /// Delete the record with the given ID. Returns false if no such record existed.
     open func delete(id: String) throws -> Bool {
-        return try queue.sync(execute: {
+        return try queue.sync {
             let engine = try self.getUnlocked()
-            let boolAsU8 = try LoginsStoreError.unwrap({ err in
+            let boolAsU8 = try LoginsStoreError.unwrap { err in
                 sync15_passwords_delete(engine, id, err)
-            })
+            }
             return boolAsU8 != 0
-        })
+        }
     }
 
     /// Bump the usage count for the record with the given id.
     ///
     /// Throws `LoginStoreError.NoSuchRecord` if there was no such record.
     open func touch(id: String) throws {
-        try queue.sync(execute: {
+        try queue.sync {
             let engine = try self.getUnlocked()
-            try LoginsStoreError.unwrap({ err in
+            try LoginsStoreError.unwrap { err in
                 sync15_passwords_touch(engine, id, err)
-            })
-        })
+            }
+        }
     }
 
     /// Insert `login` into the database. If `login.id` is not empty,
@@ -209,52 +214,52 @@ open class LoginsStorage {
     /// Returns the `id` of the newly inserted record.
     open func add(login: LoginRecord) throws -> String {
         let json = try login.toJSON()
-        return try queue.sync(execute: {
+        return try queue.sync {
             let engine = try self.getUnlocked()
-            let ptr = try LoginsStoreError.unwrap({ err in
+            let ptr = try LoginsStoreError.unwrap { err in
                 sync15_passwords_add(engine, json, err)
-            })
+            }
             return String(freeingRustString: ptr)
-        })
+        }
     }
 
     /// Update `login` in the database. If `login.id` does not refer to a known
     /// login, then this throws `LoginStoreError.NoSuchRecord`.
     open func update(login: LoginRecord) throws {
         let json = try login.toJSON()
-        return try queue.sync(execute: {
+        return try queue.sync {
             let engine = try self.getUnlocked()
-            return try LoginsStoreError.unwrap({ err in
+            return try LoginsStoreError.unwrap { err in
                 sync15_passwords_update(engine, json, err)
-            })
-        })
+            }
+        }
     }
 
     /// Get the record with the given id. Returns nil if there is no such record.
     open func get(id: String) throws -> LoginRecord? {
-        return try queue.sync(execute: {
+        return try queue.sync {
             let engine = try self.getUnlocked()
-            let ptr = try LoginsStoreError.tryUnwrap({ err in
+            let ptr = try LoginsStoreError.tryUnwrap { err in
                 sync15_passwords_get_by_id(engine, id, err)
-            })
+            }
             guard let rustStr = ptr else {
                 return nil
             }
             let jsonStr = String(freeingRustString: rustStr)
             return try LoginRecord(fromJSONString: jsonStr)
-        })
+        }
     }
 
     /// Get the entire list of records.
     open func list() throws -> [LoginRecord] {
-        return try queue.sync(execute: {
+        return try queue.sync {
             let engine = try self.getUnlocked()
-            let rustStr = try LoginsStoreError.unwrap({ err in
+            let rustStr = try LoginsStoreError.unwrap { err in
                 sync15_passwords_get_all(engine, err)
-            })
+            }
             let jsonStr = String(freeingRustString: rustStr)
             return try LoginRecord.fromJSONArray(jsonStr)
-        })
+        }
     }
 
     /// Interrupt a pending operation on another thread, causing it to fail with
@@ -267,17 +272,17 @@ open class LoginsStorage {
     ///
     /// Throws: `LoginsStoreError.Panic` if the rust code panics (please report this to us if it happens).
     open func interrupt() throws {
-        self.interrupt_handle_lock.lock()
-        defer { self.interrupt_handle_lock.unlock() }
-        // We don't throw mismatch in the case where `self.interrupt_handle` is nil,
+        interruptHandleLock.lock()
+        defer { self.interruptHandleLock.unlock() }
+        // We don't throw mismatch in the case where `self.interruptHandle` is nil,
         // because that would require users perform external synchronization.
-        if let h = self.interrupt_handle {
+        if let h = self.interruptHandle {
             try h.interrupt()
         }
     }
 }
 
-fileprivate class LoginsInterruptHandle {
+private class LoginsInterruptHandle {
     let ptr: OpaquePointer
     init(ptr: OpaquePointer) {
         self.ptr = ptr

components/places/ios/Places/Bookmark.swift

@@ -7,14 +7,12 @@ import Foundation
 /// Snarfed from firefox-ios, although we don't have the fake desktop root,
 /// and we only have the `All` Set.
 public struct BookmarkRoots {
-
-    public static let RootGUID =          "root________"
-    public static let MobileFolderGUID =  "mobile______"
-    public static let MenuFolderGUID =    "menu________"
+    public static let RootGUID = "root________"
+    public static let MobileFolderGUID = "mobile______"
+    public static let MenuFolderGUID = "menu________"
     public static let ToolbarFolderGUID = "toolbar_____"
     public static let UnfiledFolderGUID = "unfiled_____"
 
-
     public static let All = Set<String>([
         BookmarkRoots.RootGUID,
         BookmarkRoots.MobileFolderGUID,
@@ -24,7 +22,6 @@ public struct BookmarkRoots {
     ])
 }
 
-
 /**
  * Enumeration of the type of a bookmark item.
  */
@@ -39,7 +36,6 @@ public enum BookmarkNodeType: Int32 {
     // are not supported
 }
 
-
 /**
  * A base class containing the set of fields common to all nodes
  * in the bookmark tree.
@@ -78,10 +74,15 @@ public class BookmarkNode {
      */
     public let position: UInt32
 
-    fileprivate init(type: BookmarkNodeType, guid: String, dateAdded: Int64, lastModified: Int64, parentGUID: String?, position: UInt32) {
+    fileprivate init(type: BookmarkNodeType,
+                     guid: String,
+                     dateAdded: Int64,
+                     lastModified: Int64,
+                     parentGUID: String?,
+                     position: UInt32) {
         self.type = type
         self.guid = guid
-        self.dateAdded = dateAdded;
+        self.dateAdded = dateAdded
         self.lastModified = lastModified
         self.parentGUID = parentGUID
         self.position = position
@@ -93,7 +94,7 @@ public class BookmarkNode {
      * - Note: This is determined entirely by inspecting the GUID.
      */
     public var isRoot: Bool {
-        return BookmarkRoots.All.contains(self.guid)
+        return BookmarkRoots.All.contains(guid)
     }
 }
 
@@ -103,7 +104,7 @@ public class BookmarkNode {
  * It's type is always `BookmarkNodeType.separator`, and it has no fields
  * besides those defined by `BookmarkNode`.
  */
-public class BookmarkSeparator : BookmarkNode {
+public class BookmarkSeparator: BookmarkNode {
     public init(guid: String, dateAdded: Int64, lastModified: Int64, parentGUID: String?, position: UInt32) {
         super.init(
             type: .separator,
@@ -122,8 +123,7 @@ public class BookmarkSeparator : BookmarkNode {
  * It's type is always `BookmarkNodeType.bookmark`,  and in addition to the
  * fields provided by `BookmarkNode`, it has a `title` and a `url`.
  */
-public class BookmarkItem : BookmarkNode {
-
+public class BookmarkItem: BookmarkNode {
     /**
      * The URL of this bookmark.
      */
@@ -137,7 +137,13 @@ public class BookmarkItem : BookmarkNode {
      */
     public let title: String
 
-    public init(guid: String, dateAdded: Int64, lastModified: Int64, parentGUID: String?, position: UInt32, url: String, title: String) {
+    public init(guid: String,
+                dateAdded: Int64,
+                lastModified: Int64,
+                parentGUID: String?,
+                position: UInt32,
+                url: String,
+                title: String) {
         self.url = url
         self.title = title
         super.init(
@@ -158,7 +164,7 @@ public class BookmarkItem : BookmarkNode {
  * fields provided by `BookmarkNode`, it has a `title`, a list of `childGUIDs`,
  * and possibly a list of `children`.
  */
-public class BookmarkFolder : BookmarkNode {
+public class BookmarkFolder: BookmarkNode {
     /**
      * The title of this bookmark folder.
      *
@@ -182,7 +188,14 @@ public class BookmarkFolder : BookmarkNode {
      */
     public let children: [BookmarkNode]?
 
-    public init(guid: String, dateAdded: Int64, lastModified: Int64, parentGUID: String?, position: UInt32, title: String, childGUIDs: [String], children: [BookmarkNode]?) {
+    public init(guid: String,
+                dateAdded: Int64,
+                lastModified: Int64,
+                parentGUID: String?,
+                position: UInt32,
+                title: String,
+                childGUIDs: [String],
+                children: [BookmarkNode]?) {
         self.title = title
         self.childGUIDs = childGUIDs
         self.children = children
@@ -236,7 +249,7 @@ internal func unpackProtobuf(msg: MsgTypes_BookmarkNode) -> BookmarkNode {
         var childGUIDs = msg.childGuids
         // We don't bother sending both the guids and the child nodes over
         // the FFI as it's redundant.
-        if childGUIDs.isEmpty && !childNodes.isEmpty {
+        if childGUIDs.isEmpty, !childNodes.isEmpty {
             childGUIDs = childNodes.map { node in node.guid }
         }
         let childrenExpected = msg.hasHaveChildNodes ? msg.haveChildNodes : false

components/places/ios/Places/Errors/PlacesError.swift

@@ -7,7 +7,6 @@ import os.log
 
 /// Indicates an error occurred while calling into the places storage layer
 public enum PlacesError: Error {
-
     /// This indicates an attempt to use a connection after the PlacesAPI
     /// it came from is destroyed. This indicates a usage error of this library.
     case connUseAfterAPIClosed
@@ -135,7 +134,7 @@ public enum PlacesError: Error {
             return result
         } catch let e {
             // Can't log what the error is without jumping through hoops apparently, oh well...
-            os_log( "Hit places error when throwing is impossible %{public}@", type: .error, "\(e)")
+            os_log("Hit places error when throwing is impossible %{public}@", type: .error, "\(e)")
             return nil
         }
     }

components/places/ios/Places/Places.swift

@@ -18,7 +18,7 @@ internal typealias ConnectionHandle = UInt64
 public class PlacesAPI {
     private let handle: APIHandle
     private let writeConn: PlacesWriteConnection
-    fileprivate let queue = DispatchQueue(label: "com.mozilla.places.api")
+    private let queue = DispatchQueue(label: "com.mozilla.places.api")
 
     /**
      * Initialize a PlacesAPI
@@ -36,8 +36,8 @@ public class PlacesAPI {
             let writeHandle = try PlacesError.unwrap { error in
                 places_connection_new(handle, Int32(PlacesConn_ReadWrite), error)
             }
-            self.writeConn = try PlacesWriteConnection(handle: writeHandle)
-            self.writeConn.api = self
+            writeConn = try PlacesWriteConnection(handle: writeHandle)
+            writeConn.api = self
         } catch let e {
             // We failed to open the write connection, even though the
             // API was opened. This is... strange, but possible.
@@ -80,10 +80,10 @@ public class PlacesAPI {
      */
     open func openReader() throws -> PlacesReadConnection {
         return try queue.sync {
-            let h = try PlacesError.unwrap { error in
+            let conn = try PlacesError.unwrap { error in
                 places_connection_new(handle, Int32(PlacesConn_ReadOnly), error)
             }
-            return try PlacesReadConnection(handle: h, api: self)
+            return try PlacesReadConnection(handle: conn, api: self)
         }
     }
 
@@ -106,7 +106,12 @@ public class PlacesAPI {
     open func syncBookmarks(unlockInfo: SyncUnlockInfo) throws {
         return try queue.sync {
             try PlacesError.unwrap { err in
-                sync15_bookmarks_sync(handle, unlockInfo.kid, unlockInfo.fxaAccessToken, unlockInfo.syncKey, unlockInfo.tokenserverURL, err)
+                sync15_bookmarks_sync(handle,
+                                      unlockInfo.kid,
+                                      unlockInfo.fxaAccessToken,
+                                      unlockInfo.syncKey,
+                                      unlockInfo.tokenserverURL,
+                                      err)
             }
         }
     }
@@ -117,14 +122,14 @@ public class PlacesAPI {
  */
 public class PlacesReadConnection {
     fileprivate let queue = DispatchQueue(label: "com.mozilla.places.conn")
-    fileprivate var handle: ConnectionHandle;
+    fileprivate var handle: ConnectionHandle
     fileprivate weak var api: PlacesAPI?
     fileprivate let interruptHandle: InterruptHandle
 
     fileprivate init(handle: ConnectionHandle, api: PlacesAPI? = nil) throws {
         self.handle = handle
         self.api = api
-        self.interruptHandle = InterruptHandle(ptr: try PlacesError.unwrap { error in
+        interruptHandle = InterruptHandle(ptr: try PlacesError.unwrap { error in
             places_new_interrupt_handle(handle, error)
         })
     }
@@ -149,9 +154,9 @@ public class PlacesReadConnection {
         if handle != 0 {
             // In practice this can only fail if the rust code panics, which for this
             // function would be quite bad.
-            try! PlacesError.tryUnwrap({ err in
+            try! PlacesError.tryUnwrap { err in
                 places_connection_destroy(handle, err)
-            })
+            }
         }
     }
 
@@ -341,8 +346,7 @@ public class PlacesReadConnection {
 /**
  * A read-write connection to the places database.
  */
-public class PlacesWriteConnection : PlacesReadConnection {
-
+public class PlacesWriteConnection: PlacesReadConnection {
     /**
      * Run periodic database maintenance. This might include, but is
      * not limited to:
@@ -436,7 +440,9 @@ public class PlacesWriteConnection : PlacesReadConnection {
      *                            operation. (If this occurs, please let us know).
      */
     @discardableResult
-    open func createFolder(parentGUID: String, title: String, position: UInt32? = nil) throws -> String {
+    open func createFolder(parentGUID: String,
+                           title: String,
+                           position: UInt32? = nil) throws -> String {
         return try queue.sync {
             try self.checkApi()
             var msg = insertionMsg(type: .folder, parentGUID: parentGUID, position: position)
@@ -511,7 +517,10 @@ public class PlacesWriteConnection : PlacesReadConnection {
      *                            operation. (If this occurs, please let us know).
      */
     @discardableResult
-    open func createBookmark(parentGUID: String, url: String, title: String?, position: UInt32? = nil) throws -> String {
+    open func createBookmark(parentGUID: String,
+                             url: String,
+                             title: String?,
+                             position: UInt32? = nil) throws -> String {
         return try queue.sync {
             try self.checkApi()
             var msg = insertionMsg(type: .bookmark, parentGUID: parentGUID, position: position)
@@ -575,11 +584,10 @@ public class PlacesWriteConnection : PlacesReadConnection {
      *                            operation. (If this occurs, please let us know).
      */
     open func updateBookmarkNode(guid: String,
-                            parentGUID: String? = nil,
-                            position: UInt32? = nil,
-                            title: String? = nil,
-                            url: String? = nil) throws
-    {
+                                 parentGUID: String? = nil,
+                                 position: UInt32? = nil,
+                                 title: String? = nil,
+                                 url: String? = nil) throws {
         try queue.sync {
             try self.checkApi()
             var msg = MsgTypes_BookmarkNode()
@@ -621,7 +629,9 @@ public class PlacesWriteConnection : PlacesReadConnection {
     }
 
     // Remove the boilerplate common for all insertion messages
-    private func insertionMsg(type: BookmarkNodeType, parentGUID: String, position: UInt32?) -> MsgTypes_BookmarkNode {
+    private func insertionMsg(type: BookmarkNodeType,
+                              parentGUID: String,
+                              position: UInt32?) -> MsgTypes_BookmarkNode {
         var msg = MsgTypes_BookmarkNode()
         msg.nodeType = type.rawValue
         msg.parentGuid = parentGUID
@@ -633,7 +643,7 @@ public class PlacesWriteConnection : PlacesReadConnection {
 }
 
 // Wrapper around rust interrupt handle.
-fileprivate class InterruptHandle {
+private class InterruptHandle {
     let ptr: OpaquePointer
     init(ptr: OpaquePointer) {
         self.ptr = ptr

components/support/ios/SyncUnlockInfo.swift

@@ -12,7 +12,7 @@ open class SyncUnlockInfo {
     public var syncKey: String
     public var tokenserverURL: String
 
-    public init (kid: String, fxaAccessToken: String, syncKey: String, tokenserverURL: String) {
+    public init(kid: String, fxaAccessToken: String, syncKey: String, tokenserverURL: String) {
         self.kid = kid
         self.fxaAccessToken = fxaAccessToken
         self.syncKey = syncKey