diff --git a/.swiftlint.yml b/.swiftlint.yml index 3cb70db..ddcc588 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -70,10 +70,10 @@ custom_rules: regex: "@objcMembers" message: "Explicitly use @objc on each member you want to expose to Objective-C" severity: error - # no_direct_standard_out_logs: - # name: "Writing log messages directly to standard out is disallowed" - # regex: "(\\bprint|\\bdebugPrint|\\bdump|Swift\\.print|Swift\\.debugPrint|Swift\\.dump)\\s*\\(" - # match_kinds: - # - identifier - # message: "Don't commit `print(…)`, `debugPrint(…)`, or `dump(…)` as they write to standard out in release. Either log to a dedicated logging system or silence this warning in debug-only scenarios explicitly using `// swiftlint:disable:next no_direct_standard_out_logs`" - # severity: warning + no_direct_standard_out_logs: + name: "Writing log messages directly to standard out is disallowed" + regex: "(\\bprint|\\bdebugPrint|\\bdump|Swift\\.print|Swift\\.debugPrint|Swift\\.dump)\\s*\\(" + match_kinds: + - identifier + message: "Don't commit `print(…)`, `debugPrint(…)`, or `dump(…)` as they write to standard out in release. Either log to a dedicated logging system or silence this warning in debug-only scenarios explicitly using `// swiftlint:disable:next no_direct_standard_out_logs`" + severity: warning diff --git a/Doughnut.xcodeproj/project.pbxproj b/Doughnut.xcodeproj/project.pbxproj index c36908e..fd64191 100644 --- a/Doughnut.xcodeproj/project.pbxproj +++ b/Doughnut.xcodeproj/project.pbxproj @@ -23,6 +23,7 @@ 6B816BE927E71A1A00EED863 /* ModelTestCase.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B816BE827E71A1A00EED863 /* ModelTestCase.swift */; }; 6B816BEB27E71A2700EED863 /* DoughnutTestCase.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B816BEA27E71A2700EED863 /* DoughnutTestCase.swift */; }; 6B816BED27E71A6700EED863 /* PodcastModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B816BEC27E71A6700EED863 /* PodcastModelTests.swift */; }; + 6B8745502842015000AB07CF /* OSLog+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B87454F2842015000AB07CF /* OSLog+Extensions.swift */; }; 6B94DF4C278968F500BCB149 /* NSTableView+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B94DF4B278968F500BCB149 /* NSTableView+Extensions.swift */; }; 6B96F45D27CE6F10001941BA /* PodcastSearchField.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B96F45C27CE6F10001941BA /* PodcastSearchField.swift */; }; 6B9C30BB27B5708300D462BE /* BaseTableView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B9C30BA27B5708300D462BE /* BaseTableView.swift */; }; @@ -129,6 +130,7 @@ 6B816BE827E71A1A00EED863 /* ModelTestCase.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ModelTestCase.swift; sourceTree = ""; }; 6B816BEA27E71A2700EED863 /* DoughnutTestCase.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DoughnutTestCase.swift; sourceTree = ""; }; 6B816BEC27E71A6700EED863 /* PodcastModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PodcastModelTests.swift; sourceTree = ""; }; + 6B87454F2842015000AB07CF /* OSLog+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OSLog+Extensions.swift"; sourceTree = ""; }; 6B94DF4B278968F500BCB149 /* NSTableView+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "NSTableView+Extensions.swift"; sourceTree = ""; }; 6B96F45C27CE6F10001941BA /* PodcastSearchField.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PodcastSearchField.swift; sourceTree = ""; }; 6B9C30BA27B5708300D462BE /* BaseTableView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BaseTableView.swift; sourceTree = ""; }; @@ -258,6 +260,7 @@ children = ( 6B0605C32788626F00A8A91E /* AppKit */, 6BC6396227A68E0C00535897 /* CoreMedia */, + 6B209FD8284331BC006F3C23 /* OSLog */, ); name = Extensions; sourceTree = ""; @@ -275,6 +278,14 @@ name = AppKit; sourceTree = ""; }; + 6B209FD8284331BC006F3C23 /* OSLog */ = { + isa = PBXGroup; + children = ( + 6B87454F2842015000AB07CF /* OSLog+Extensions.swift */, + ); + name = OSLog; + sourceTree = ""; + }; 6B571B4627E74C31008E1534 /* Fixtures */ = { isa = PBXGroup; children = ( @@ -896,6 +907,7 @@ 837D52BE1F8E622200C17514 /* TasksViewController.swift in Sources */, 8379899B1F81616C00234577 /* SeekSlider.swift in Sources */, 6B36624627CFB339008E1CA5 /* NSImage+Extensions.swift in Sources */, + 6B8745502842015000AB07CF /* OSLog+Extensions.swift in Sources */, 837068131F7BBD63007FE973 /* EpisodeCellView.swift in Sources */, 83667DF41F76D22600F1ABC0 /* DetailViewController.swift in Sources */, 6BC6396427A68E2100535897 /* CMTime+Extensions.swift in Sources */, diff --git a/Doughnut/CrashReport/CrashReporter.swift b/Doughnut/CrashReport/CrashReporter.swift index 9ebb1bf..a54b028 100644 --- a/Doughnut/CrashReport/CrashReporter.swift +++ b/Doughnut/CrashReport/CrashReporter.swift @@ -17,6 +17,7 @@ */ import Foundation +import OSLog import CrashReporter @@ -27,13 +28,14 @@ final class CrashReporter { } private static var sharedInstance = CrashReporter() + private static let log = OSLog.main(category: "CrashReporter") private var plCrashReporter: PLCrashReporter? private init() { let config = PLCrashReporterConfig(signalHandlerType: .BSD, symbolicationStrategy: .symbolTable) guard let plCrashReporter = PLCrashReporter(configuration: config) else { - print("CrashReporter: could not create an instance of PLCrashReporter") + Self.log(level: .error, "could not create an instance of PLCrashReporter") return } self.plCrashReporter = plCrashReporter @@ -41,7 +43,7 @@ final class CrashReporter { do { try plCrashReporter.enableAndReturnError() } catch { - print("CrashReporter: failed to enable PLCrashReporter: \(error)") + Self.log(level: .error, "failed to enable PLCrashReporter: \(error)") } } @@ -67,17 +69,17 @@ final class CrashReporter { if let text = PLCrashReportTextFormatter.stringValue(for: report, with: PLCrashReportTextFormatiOS) { return text } else { - print("CrashReporter: can't convert the report to text") + Self.log(level: .error, "can't convert the report to text") } } catch { - print("CrashReporter failed to load and parse crash report: \(error)") + Self.log(level: .error, "failed to load and parse crash report: \(error)") } return nil } func forceCrash() { - fatalError("Force crashd in \(#function)") + fatalError("Force crash in \(#function)") } } diff --git a/Doughnut/Library/Episode.swift b/Doughnut/Library/Episode.swift index fe97a32..f55ea99 100644 --- a/Doughnut/Library/Episode.swift +++ b/Doughnut/Library/Episode.swift @@ -198,7 +198,7 @@ class Episode: Record { if let url = url() { NSWorkspace.shared.recycle([url], completionHandler: { _, error in if let error = error { - print("Failed to move to trash \(error)") + Library.log(level: .error, "Failed to move to trash \(error)") } else { if let completion = completion { completion(url) @@ -228,7 +228,7 @@ class Episode: Record { if copyToLibrary { // Perform the copy guard let storagePath = podcast.storagePath() else { - print("Could not determine podcast storage location") + Library.log(level: .error, "Could not determine podcast storage location") return } @@ -245,7 +245,7 @@ class Episode: Record { episode.fileName = fileName } catch { - print("Failed to copy \(url.path) to library. Output destination \(outputPath)") + Library.log(level: .error, "Failed to copy \(url.path) to library. Output destination \(outputPath)") return } } else { diff --git a/Doughnut/Library/Library.swift b/Doughnut/Library/Library.swift index 1857e48..db30699 100644 --- a/Doughnut/Library/Library.swift +++ b/Doughnut/Library/Library.swift @@ -17,6 +17,7 @@ */ import Foundation +import OSLog import FeedKit import GRDB @@ -45,6 +46,8 @@ class Library: NSObject { static var global = Library() static let databaseFilename = "Doughnut Library.dnl" + static let log = OSLog.main(category: "Library") + enum Events: String { case Subscribed case Unsubscribed @@ -94,7 +97,10 @@ class Library: NSObject { var configuration = Configuration() if Preference.bool(for: Preference.Key.debugSQLTraceEnabled) { configuration.prepareDatabase { db in - db.trace { print($0) } + db.trace { + // swiftlint:disable:next no_direct_standard_out_logs + print($0) + } } } dbQueue = try DatabaseQueue(path: databaseFile().path, configuration: configuration) @@ -103,7 +109,7 @@ class Library: NSObject { try LibraryMigrations.migrate(db: dbQueue) if !Preference.testEnv() { - print("Connected to Doughnut library at \(path.path)") + Self.log(level: .info, "Connected to Doughnut library at \(path.path)") } try dbQueue.inDatabase({ db in @@ -113,7 +119,7 @@ class Library: NSObject { podcast.loadEpisodes(db: db) #if DEBUG - print("Loading \(podcast.title) with \(podcast.episodes.count) episodes") + Self.log(level: .debug, "Loading \(podcast.title) with \(podcast.episodes.count) episodes") #endif } @@ -188,7 +194,7 @@ class Library: NSObject { } static func handleDatabaseError(_ error: Error) { - print("Library: error \(error), stack trace: \(Thread.callStackSymbols)") + Self.log(level: .error, "Library: error \(error), stack trace: \(Thread.callStackSymbols)") } static func sanitizePath(_ path: String) -> String { @@ -303,13 +309,13 @@ class Library: NSObject { if let storagePath = podcast.storagePath() { NSWorkspace.shared.recycle([storagePath], completionHandler: { (trashedFiles, error) in if let error = error { - print("Failed to move podcast data to trash: \(error.localizedDescription)") + Self.log(level: .error, "Failed to move podcast data to trash: \(error.localizedDescription)") let alert = NSAlert() alert.messageText = "Failed to trash data" alert.informativeText = error.localizedDescription } else { - print("Moved podcast data stored at \(trashedFiles) to trash") + Self.log(level: .info, "Moved podcast data stored at \(trashedFiles) to trash") } }) } diff --git a/Doughnut/Library/Podcast.swift b/Doughnut/Library/Podcast.swift index 334219f..075b94d 100644 --- a/Doughnut/Library/Podcast.swift +++ b/Doughnut/Library/Podcast.swift @@ -162,7 +162,7 @@ class Podcast: Record { do { try FileManager.default.createDirectory(at: pathUrl, withIntermediateDirectories: true, attributes: nil) } catch { - print("Failed to create directory \(error)") + Library.log(level: .error, "Failed to create directory \(error)") } } @@ -295,7 +295,7 @@ class Podcast: Record { guard let rssFeed = feed.rssFeed else { return [] } return self.parse(feed: rssFeed) case .failure(let error): - print("Error reloading \(self.title): \(String(describing: error.localizedDescription))") + Library.log(level: .error, "Error reloading \(self.title): \(String(describing: error.localizedDescription))") return [] } } diff --git a/Doughnut/Library/Utils.swift b/Doughnut/Library/Utils.swift index 41e17a5..fde3d4b 100644 --- a/Doughnut/Library/Utils.swift +++ b/Doughnut/Library/Utils.swift @@ -56,7 +56,7 @@ class Utils { } } } catch let error { - print(error) + Library.log(level: .error, "Failed to parse iTunes feed with: \(error)") } completion(nil) diff --git a/Doughnut/Player/Player+NowPlayingInfoCenter.swift b/Doughnut/Player/Player+NowPlayingInfoCenter.swift index a3e7f77..050df99 100644 --- a/Doughnut/Player/Player+NowPlayingInfoCenter.swift +++ b/Doughnut/Player/Player+NowPlayingInfoCenter.swift @@ -37,8 +37,7 @@ extension Player { } func updateNowPlayingEpisodeInfo() { - // TODO: convert print statemenets to logs with levels. - print("[NowPlayingInfo]: updateNowPlayingEpisodeInfo") + Self.log(level: .debug, "[NowPlayingInfo]: updateNowPlayingEpisodeInfo called") guard let currentEpisode = currentEpisode else { nowPlayingEpisodeInfoDictionary = [:] @@ -68,8 +67,7 @@ extension Player { } func updateNowPlayingPlaybackInfo() { - // TODO: convert print statemenets to logs with levels. - // print("[NowPlayingInfo]: updateNowPlayingPlaybackInfo") + // Self.logger.debug("[NowPlayingInfo]: updateNowPlayingPlaybackInfo called") // TODO: Limit the rate of sending the following values. let nowPlayingInfoDictionary = nowPlayingEpisodeInfoDictionary.merging([ diff --git a/Doughnut/Player/Player+RemoteCommandCenter.swift b/Doughnut/Player/Player+RemoteCommandCenter.swift index cc96858..cd56120 100644 --- a/Doughnut/Player/Player+RemoteCommandCenter.swift +++ b/Doughnut/Player/Player+RemoteCommandCenter.swift @@ -29,25 +29,25 @@ extension Player { // Playback Commands remoteCommandCenter.pauseCommand.addTarget { [weak self] _ in - print("[RemoteCommand]: Receive pauseCommand") + Self.log(level: .debug, "[RemoteCommand]: Receive pauseCommand") self?.pause() return .success } remoteCommandCenter.playCommand.addTarget { [weak self] _ in - print("[RemoteCommand]: Receive playCommand") + Self.log(level: .debug, "[RemoteCommand]: Receive playCommand") self?.play() return .success } remoteCommandCenter.stopCommand.addTarget { [weak self] _ in - print("[RemoteCommand]: Receive stopCommand") + Self.log(level: .debug, "[RemoteCommand]: Receive stopCommand") self?.stop() return .success } remoteCommandCenter.togglePlayPauseCommand.addTarget { [weak self] _ in - print("[RemoteCommand]: Receive togglePlayPauseCommand") + Self.log(level: .debug, "[RemoteCommand]: Receive togglePlayPauseCommand") self?.togglePlay() return .success } @@ -66,14 +66,14 @@ extension Player { // Skip Interval Commands remoteCommandCenter.skipForwardCommand.addTarget { [weak self] event in - print("[RemoteCommand]: Receive skipForwardCommand") + Self.log(level: .debug, "[RemoteCommand]: Receive skipForwardCommand") guard let event = event as? MPSkipIntervalCommandEvent else { return .commandFailed } self?.skipAhead(seconds: event.interval) return .success } remoteCommandCenter.skipBackwardCommand.addTarget { [weak self] event in - print("[RemoteCommand]: Receive skipBackwardCommand") + Self.log(level: .debug, "[RemoteCommand]: Receive skipBackwardCommand") guard let event = event as? MPSkipIntervalCommandEvent else { return .commandFailed } self?.skipBack(seconds: event.interval) return .success @@ -86,7 +86,7 @@ extension Player { remoteCommandCenter.seekBackwardCommand.isEnabled = false remoteCommandCenter.changePlaybackPositionCommand.addTarget { [weak self] event in - print("[RemoteCommand]: Receive changePlaybackPositionCommand") + Self.log(level: .debug, "[RemoteCommand]: Receive changePlaybackPositionCommand") guard let event = event as? MPChangePlaybackPositionCommandEvent else { return .commandFailed } self?.seek(seconds: event.positionTime) return .success diff --git a/Doughnut/Player/Player.swift b/Doughnut/Player/Player.swift index 21baae4..f1b21f4 100644 --- a/Doughnut/Player/Player.swift +++ b/Doughnut/Player/Player.swift @@ -18,6 +18,7 @@ import AVFoundation import Cocoa +import OSLog protocol PlayerDelegate: AnyObject { func update(forEpisode episode: Episode?) @@ -33,6 +34,8 @@ enum PlayerLoadStatus { final class Player: NSObject { static var global = Player() + static let log = OSLog.main(category: "Player") + weak var delegate: PlayerDelegate? private static let playedThreshold: Double = 0.9 @@ -131,7 +134,7 @@ final class Player: NSObject { assert(false, "'.loading' should not be returned in the completionHandler of loadValuesAsynchronously(forKeys:).") break default: - print("Failed to load the AVAsset failed with status: \(status), error: \(error?.localizedDescription ?? "nil").") + Player.log(level: .error, "Failed to load the AVAsset failed with status: \(status), error: \(error?.localizedDescription ?? "nil")") break } cleanupLoadStatusOnFail() @@ -251,7 +254,7 @@ final class Player: NSObject { loadStatus = .none } - print("Playing") + Self.log(level: .debug, "Playing") postPlaybackStatusUpdates() } } @@ -340,9 +343,9 @@ final class Player: NSObject { if #available(macOS 11.0, *) { AVAudioRoutingArbiter.shared.begin(category: .playback) { defaultDeviceChanged, error in if let error = error { - print("begins routing arbitration failed, defaultDeviceChanged: \(defaultDeviceChanged), error: \(error)") + Self.log(level: .error, "begins routing arbitration failed, defaultDeviceChanged: \(defaultDeviceChanged), error: \(error)") } else { - print("begins routing arbitration, defaultDeviceChanged: \(defaultDeviceChanged)") + Self.log(level: .debug, "begins routing arbitration, defaultDeviceChanged: \(defaultDeviceChanged)") } } } @@ -351,7 +354,7 @@ final class Player: NSObject { private func leaveRoutingArbitration() { if #available(macOS 11.0, *) { AVAudioRoutingArbiter.shared.leave() - print("leaves routing arbitration") + Self.log(level: .debug, "leaves routing arbitration") } } @@ -409,6 +412,7 @@ final class Player: NSObject { // If there are channels, it's an input device + // swiftlint:disable:next no_direct_standard_out_logs Swift.print("Found output device '\(name)' with \(channelCount) channels [\(id)]") inputDevices.append(id) } diff --git a/Doughnut/Preference/Preference.swift b/Doughnut/Preference/Preference.swift index 64a2731..821c6ca 100644 --- a/Doughnut/Preference/Preference.swift +++ b/Doughnut/Preference/Preference.swift @@ -202,7 +202,7 @@ class Preference { do { try FileManager.default.createDirectory(at: url, withIntermediateDirectories: true, attributes: nil) } catch { - print("Failed to create directory \(error)") + Library.log(level: .error, "Failed to create directory \(error)") } } } diff --git a/Doughnut/Utilities/OSLog+Extensions.swift b/Doughnut/Utilities/OSLog+Extensions.swift new file mode 100644 index 0000000..e37dc1d --- /dev/null +++ b/Doughnut/Utilities/OSLog+Extensions.swift @@ -0,0 +1,36 @@ +/* + * Doughnut Podcast Client + * Copyright (C) 2017 - 2022 Chris Dyer + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +import Foundation +import OSLog + +extension OSLog { + + static func main(category: String) -> OSLog { + return OSLog(subsystem: Bundle.main.bundleIdentifier!, category: category) + } + + // This is a compromised approach since `os.Logger` and `os.OSLogMessage` + // requires macOS 11.0. + // See also: https://stackoverflow.com/questions/53025698#62488271 + // TODO: Migrate to `os.Logger` when we drop the support for 10.15 (Catalina). + func callAsFunction(level: OSLogType, _ s: String) { + os_log(level, log: self, "%{public}s", s) + } + +} diff --git a/DoughnutTests/.swiftlint.yml b/DoughnutTests/.swiftlint.yml new file mode 100644 index 0000000..409d261 --- /dev/null +++ b/DoughnutTests/.swiftlint.yml @@ -0,0 +1,2 @@ +disabled_rules: + - no_direct_standard_out_logs