From 0762271c9085c3778df4b179f6aafcf767a06ec8 Mon Sep 17 00:00:00 2001 From: Jacob Hearst Date: Wed, 29 Jan 2025 13:52:28 -0600 Subject: [PATCH 1/2] Add assoc value to unknown enum cases --- .../ScryfallKit/Models/Card/Card+enums.swift | 67 +++++---- Sources/ScryfallKit/Models/Card/Card.swift | 4 +- Sources/ScryfallKit/Models/MTGSet.swift | 36 ++--- .../ScryfallKit/Models/UnknownDecodable.swift | 27 ++++ .../ScryfallKitTests/CaseIterableTests.swift | 139 ++++++++++++++++++ Tests/ScryfallKitTests/SmokeTests.swift | 8 +- 6 files changed, 224 insertions(+), 57 deletions(-) create mode 100644 Sources/ScryfallKit/Models/UnknownDecodable.swift create mode 100644 Tests/ScryfallKitTests/CaseIterableTests.swift diff --git a/Sources/ScryfallKit/Models/Card/Card+enums.swift b/Sources/ScryfallKit/Models/Card/Card+enums.swift index 56b1468..432b34b 100644 --- a/Sources/ScryfallKit/Models/Card/Card+enums.swift +++ b/Sources/ScryfallKit/Models/Card/Card+enums.swift @@ -124,27 +124,25 @@ extension Card { /// Layouts for a Magic card /// /// [Scryfall documentation](https://scryfall.com/docs/api/layouts) - public enum Layout: String, CaseIterable, Codable, Sendable { + public enum Layout: RawRepresentable, CaseIterable, Codable, Sendable, Equatable, Hashable { case normal, split, flip, transform, meld, leveler, saga, adventure, planar, scheme, vanguard, - token, emblem, augment, host, `class`, battle, `case`, mutate, prototype, unknown - case modalDfc = "modal_dfc" - case doubleSided = "double_sided" - case doubleFacedToken = "double_faced_token" - case artSeries = "art_series" - case reversibleCard = "reversible_card" - - /// Codable initializer - /// - /// If this initializer fails to decode a value, instead of throwing an error, it will decode as the ``ScryfallKit/Card/Layout-swift.enum/unknown`` type and print a message to the logs. - /// - Parameter decoder: The Decoder to try decoding a ``ScryfallKit/Card/Layout-swift.enum`` from - public init(from decoder: Decoder) throws { - self = (try? Self(rawValue: decoder.singleValueContainer().decode(RawValue.self))) ?? .unknown - if self == .unknown, let rawValue = try? String(from: decoder) { - if #available(iOS 14.0, macOS 11.0, *) { - Logger.decoder.error("Decoded unknown Layout: \(rawValue)") - } else { - print("Decoded unknown Layout: \(rawValue)") - } + token, emblem, augment, host, `class`, battle, `case`, mutate, prototype, modalDfc, doubleSided, doubleFacedToken, artSeries, reversibleCard + case unknown(String) + + /// All known Magic: the Gathering card layouts + public static let allCases: [Card.Layout] = [ + .normal, .split, .flip, .transform, .meld, .leveler, .saga, .adventure, .planar, .scheme, .vanguard, .token, .emblem, .augment, .host, .class, .battle, .case, .mutate, .prototype, .modalDfc, .doubleSided, .doubleFacedToken, .artSeries, .reversibleCard, + ] + + public var rawValue: String { + switch self { + case .modalDfc: "modal_dfc" + case .doubleSided: "double_sided" + case .doubleFacedToken: "double_faced_token" + case .artSeries: "art_series" + case .reversibleCard: "reversible_card" + case .unknown(let string): string + default: String(describing: self) } } } @@ -200,21 +198,22 @@ extension Card { /// Effects applied to a Magic card frame /// /// [Scryfall documentation](https://scryfall.com/docs/api/frames#frame-effects) - public enum FrameEffect: String, Codable, CaseIterable, Sendable { + public enum FrameEffect: RawRepresentable, Codable, Sendable, CaseIterable, Equatable, Hashable { case legendary, miracle, nyxtouched, draft, devoid, tombstone, colorshifted, inverted, - sunmoondfc, compasslanddfc, originpwdfc, mooneldrazidfc, waxingandwaningmoondfc, showcase, - extendedart, companion, etched, snow, lesson, convertdfc, fandfc, battle, gravestone, fullart, - vehicle, borderless, extended, spree, textless, unknown, enchantment, shatteredglass, upsidedowndfc - - public init(from decoder: Decoder) throws { - self = - try FrameEffect(rawValue: decoder.singleValueContainer().decode(RawValue.self)) ?? .unknown - if self == .unknown, let rawValue = try? String(from: decoder) { - if #available(iOS 14.0, macOS 11.0, *) { - Logger.decoder.error("Decoded unknown FrameEffect: \(rawValue)") - } else { - print("Decoded unknown FrameEffect: \(rawValue)") - } + sunmoondfc, compasslanddfc, originpwdfc, mooneldrazidfc, waxingandwaningmoondfc, showcase, + extendedart, companion, etched, snow, lesson, convertdfc, fandfc, battle, gravestone, fullart, + vehicle, borderless, extended, spree, textless, enchantment, shatteredglass, upsidedowndfc + case unknown(String) + + /// All known Magic: the Gathering frame effects + public static let allCases: [Card.FrameEffect] = [ + .legendary, .miracle, .nyxtouched, .draft, .devoid, .tombstone, .colorshifted, .inverted, .sunmoondfc, .compasslanddfc, .originpwdfc, .mooneldrazidfc, .waxingandwaningmoondfc, .showcase, .extendedart, .companion, .etched, .snow, .lesson, .convertdfc, .fandfc, .battle, .gravestone, .fullart, .vehicle, .borderless, .extended, .spree, .textless, .enchantment, + ] + + public var rawValue: String { + switch self { + case .unknown(let unknownRawValue): unknownRawValue + default: String(describing: self) } } } diff --git a/Sources/ScryfallKit/Models/Card/Card.swift b/Sources/ScryfallKit/Models/Card/Card.swift index 306c1e1..c7d2ccb 100644 --- a/Sources/ScryfallKit/Models/Card/Card.swift +++ b/Sources/ScryfallKit/Models/Card/Card.swift @@ -166,7 +166,7 @@ public struct Card: Codable, Identifiable, Hashable, Sendable { /// A link to this card's set on Scryfall public var setSearchUri: URL /// The type of set this card was printed in - public var setType: MTGSet.`Type` + public var setType: MTGSet.Kind /// A link to this card's set object on the Scryfall API public var setUri: String /// This card's set code @@ -254,7 +254,7 @@ public struct Card: Codable, Identifiable, Hashable, Sendable { scryfallSetUri: String, setName: String, setSearchUri: URL, - setType: MTGSet.`Type`, + setType: MTGSet.Kind, setUri: String, set: String, storySpotlight: Bool, diff --git a/Sources/ScryfallKit/Models/MTGSet.swift b/Sources/ScryfallKit/Models/MTGSet.swift index 20e0a79..f5bcfea 100644 --- a/Sources/ScryfallKit/Models/MTGSet.swift +++ b/Sources/ScryfallKit/Models/MTGSet.swift @@ -30,25 +30,27 @@ public struct MTGSet: Codable, Identifiable, Hashable, Sendable { /// A machine-readable value describing the type of set this is. /// /// See [Scryfall's docs](https://scryfall.com/docs/api/sets#set-types) for more information on set types - public enum `Type`: String, Codable, Sendable { + public enum Kind: RawRepresentable, Codable, Sendable, CaseIterable, Hashable, Equatable { // While "masters" is in fact not inclusive, it's also a name that we can't control // swiftlint:disable:next inclusive_language case core, expansion, masters, masterpiece, spellbook, commander, planechase, archenemy, - vanguard, funny, starter, box, promo, token, memorabilia, arsenal, alchemy, minigame, unknown - case fromTheVault = "from_the_vault" - case premiumDeck = "premium_deck" - case duelDeck = "duel_deck" - case draftInnovation = "draft_innovation" - case treasureChest = "treasure_chest" + vanguard, funny, starter, box, promo, token, memorabilia, arsenal, alchemy, minigame, fromTheVault, premiumDeck, duelDeck, draftInnovation, treasureChest + case unknown(String) - public init(from decoder: Decoder) throws { - self = try Self(rawValue: decoder.singleValueContainer().decode(RawValue.self)) ?? .unknown - if self == .unknown, let rawValue = try? String(from: decoder) { - if #available(iOS 14.0, macOS 11.0, *) { - Logger.main.warning("Decoded unknown MTGSet Type: \(rawValue)") - } else { - print("Decoded unknown MTGSet Type: \(rawValue)") - } + public static let allCases: [Kind] = [ + .core, .expansion, .masters, .masterpiece, .spellbook, .commander, .planechase, .archenemy, + .vanguard, .funny, .starter, .box, .promo, .token, .memorabilia, .arsenal, .alchemy, .minigame, .fromTheVault, .premiumDeck, .duelDeck, .draftInnovation, .treasureChest + ] + + public var rawValue: String { + switch self { + case .fromTheVault: "from_the_vault" + case .premiumDeck: "premium_deck" + case .duelDeck: "duel_deck" + case .draftInnovation: "draft_innovation" + case .treasureChest: "treasure_chest" + case .unknown(let unknownValue): unknownValue + default: String(describing: self) } } } @@ -64,7 +66,7 @@ public struct MTGSet: Codable, Identifiable, Hashable, Sendable { /// The English name of the set. public var name: String /// A computer-readable classification for this set. - public var setType: MTGSet.`Type` + public var setType: Kind /// The date the set was released or the first card was printed in the set (in GMT-8 Pacific time). public var releasedAt: String? /// The block code for this set, if any. @@ -100,7 +102,7 @@ public struct MTGSet: Codable, Identifiable, Hashable, Sendable { mtgoCode: String? = nil, tcgplayerId: Int? = nil, name: String, - setType: MTGSet.`Type`, + setType: Kind, releasedAt: String? = nil, blockCode: String? = nil, block: String? = nil, diff --git a/Sources/ScryfallKit/Models/UnknownDecodable.swift b/Sources/ScryfallKit/Models/UnknownDecodable.swift new file mode 100644 index 0000000..59b147a --- /dev/null +++ b/Sources/ScryfallKit/Models/UnknownDecodable.swift @@ -0,0 +1,27 @@ +// +// UnknownDecodable.swift +// + +/// A convenience protocol to reduce duplication of RawRepresentable and Decodable initializers +protocol UnknownDecodable: Decodable, CaseIterable, RawRepresentable where RawValue == String { + static func unknown(_ rawValue: String) -> Self +} + +extension UnknownDecodable { + public init?(rawValue: String) { + guard let match = Self.allCases.first(where: { $0.rawValue == rawValue }) else { + return nil + } + + self = match + } + + public init(from decoder: any Decoder) throws { + let rawValue = try decoder.singleValueContainer().decode(String.self) + self = .init(rawValue: rawValue) ?? .unknown(rawValue) + } +} + +extension Card.FrameEffect: UnknownDecodable {} +extension Card.Layout: UnknownDecodable {} +extension MTGSet.Kind: UnknownDecodable {} diff --git a/Tests/ScryfallKitTests/CaseIterableTests.swift b/Tests/ScryfallKitTests/CaseIterableTests.swift new file mode 100644 index 0000000..21a925e --- /dev/null +++ b/Tests/ScryfallKitTests/CaseIterableTests.swift @@ -0,0 +1,139 @@ +// +// CaseIterableTests.swift +// + +import XCTest +import ScryfallKit + +/// A test suite to remind maintainers to make sure that all the known cases for +/// an enum's `allCases` property. +/// +/// Some of the enums in ScryfallKit have to get updated a lot because WOTC +/// is constantly playing with card design. To fill the gap between the release +/// of a new enum case and the release of a supporting ScryfallKit version, +/// the `unknown(String)` case was introduced. +/// +/// Unfortunately, adding an associated value to an enum prevents the compiler +/// from automatically synthesizing the `CaseIterable` conformance. +/// +/// The manual conformance of `CaseIterable` for the affected types MUST include all cases +/// _except_ the `unknown(String)` case. Manually providing this conformance introduces the +/// risk that a new case will be added to one of these types but NOT added to the `allCases` +/// array. This test suite aims to prevent that via (ab)use of switch exhaustivity. By switching +/// on an arbitrary enum case in a unit test, the compiler will error out if a new case is added +/// to the enum but not added to the switch statement in the test. This should hopefully +/// remind maintainers to keep `allCases` up to date. +final class CaseIterableTests: XCTestCase { + func testFrameEffect() { + let stub = Card.FrameEffect.battle + let contains = switch stub { + case .legendary: Card.FrameEffect.allCases.contains(.legendary) + case .miracle: Card.FrameEffect.allCases.contains(.miracle) + case .nyxtouched: Card.FrameEffect.allCases.contains(.nyxtouched) + case .draft: Card.FrameEffect.allCases.contains(.draft) + case .devoid: Card.FrameEffect.allCases.contains(.devoid) + case .tombstone: Card.FrameEffect.allCases.contains(.tombstone) + case .colorshifted: Card.FrameEffect.allCases.contains(.colorshifted) + case .inverted: Card.FrameEffect.allCases.contains(.inverted) + case .sunmoondfc: Card.FrameEffect.allCases.contains(.sunmoondfc) + case .compasslanddfc: Card.FrameEffect.allCases.contains(.compasslanddfc) + case .originpwdfc: Card.FrameEffect.allCases.contains(.originpwdfc) + case .mooneldrazidfc: Card.FrameEffect.allCases.contains(.mooneldrazidfc) + case .waxingandwaningmoondfc: Card.FrameEffect.allCases.contains(.waxingandwaningmoondfc) + case .showcase: Card.FrameEffect.allCases.contains(.showcase) + case .extendedart: Card.FrameEffect.allCases.contains(.extendedart) + case .companion: Card.FrameEffect.allCases.contains(.companion) + case .etched: Card.FrameEffect.allCases.contains(.etched) + case .snow: Card.FrameEffect.allCases.contains(.snow) + case .lesson: Card.FrameEffect.allCases.contains(.lesson) + case .convertdfc: Card.FrameEffect.allCases.contains(.convertdfc) + case .fandfc: Card.FrameEffect.allCases.contains(.fandfc) + case .battle: Card.FrameEffect.allCases.contains(.battle) + case .gravestone: Card.FrameEffect.allCases.contains(.gravestone) + case .fullart: Card.FrameEffect.allCases.contains(.fullart) + case .vehicle: Card.FrameEffect.allCases.contains(.vehicle) + case .borderless: Card.FrameEffect.allCases.contains(.borderless) + case .extended: Card.FrameEffect.allCases.contains(.extended) + case .spree: Card.FrameEffect.allCases.contains(.spree) + case .textless: Card.FrameEffect.allCases.contains(.textless) + case .enchantment: Card.FrameEffect.allCases.contains(.enchantment) + case .shatteredglass: Card.FrameEffect.allCases.contains(.shatteredglass) + case .upsidedowndfc: Card.FrameEffect.allCases.contains(.upsidedowndfc) + case .unknown(let string): + // Unknown case shouldn't be in allCases + !Card.FrameEffect.allCases.contains(.unknown(string)) + } + + XCTAssertTrue(contains) + } + + func testLayout() { + let stub = Card.Layout.adventure + let contains = switch stub { + case .normal: Card.Layout.allCases.contains(.normal) + case .split: Card.Layout.allCases.contains(.split) + case .flip: Card.Layout.allCases.contains(.flip) + case .transform: Card.Layout.allCases.contains(.transform) + case .meld: Card.Layout.allCases.contains(.meld) + case .leveler: Card.Layout.allCases.contains(.leveler) + case .saga: Card.Layout.allCases.contains(.saga) + case .adventure: Card.Layout.allCases.contains(.adventure) + case .planar: Card.Layout.allCases.contains(.planar) + case .scheme: Card.Layout.allCases.contains(.scheme) + case .vanguard: Card.Layout.allCases.contains(.vanguard) + case .token: Card.Layout.allCases.contains(.token) + case .emblem: Card.Layout.allCases.contains(.emblem) + case .augment: Card.Layout.allCases.contains(.augment) + case .host: Card.Layout.allCases.contains(.host) + case .class: Card.Layout.allCases.contains(.class) + case .battle: Card.Layout.allCases.contains(.battle) + case .case: Card.Layout.allCases.contains(.case) + case .mutate: Card.Layout.allCases.contains(.mutate) + case .prototype: Card.Layout.allCases.contains(.prototype) + case .modalDfc: Card.Layout.allCases.contains(.modalDfc) + case .doubleSided: Card.Layout.allCases.contains(.doubleSided) + case .doubleFacedToken: Card.Layout.allCases.contains(.doubleFacedToken) + case .artSeries: Card.Layout.allCases.contains(.artSeries) + case .reversibleCard: Card.Layout.allCases.contains(.reversibleCard) + case .unknown(let string): + // Unknown case shouldn't be in allCases + !Card.Layout.allCases.contains(.unknown(string)) + } + + XCTAssertTrue(contains) + } + + func testSetType() { + let stub = MTGSet.Kind.funny + let contains = switch stub { + case .core: MTGSet.Kind.allCases.contains(.core) + case .expansion: MTGSet.Kind.allCases.contains(.expansion) + case .masters: MTGSet.Kind.allCases.contains(.masters) + case .masterpiece: MTGSet.Kind.allCases.contains(.masterpiece) + case .spellbook: MTGSet.Kind.allCases.contains(.spellbook) + case .commander: MTGSet.Kind.allCases.contains(.commander) + case .planechase: MTGSet.Kind.allCases.contains(.planechase) + case .archenemy: MTGSet.Kind.allCases.contains(.archenemy) + case .vanguard: MTGSet.Kind.allCases.contains(.vanguard) + case .funny: MTGSet.Kind.allCases.contains(.funny) + case .starter: MTGSet.Kind.allCases.contains(.starter) + case .box: MTGSet.Kind.allCases.contains(.box) + case .promo: MTGSet.Kind.allCases.contains(.promo) + case .token: MTGSet.Kind.allCases.contains(.token) + case .memorabilia: MTGSet.Kind.allCases.contains(.memorabilia) + case .arsenal: MTGSet.Kind.allCases.contains(.arsenal) + case .alchemy: MTGSet.Kind.allCases.contains(.alchemy) + case .minigame: MTGSet.Kind.allCases.contains(.minigame) + case .fromTheVault: MTGSet.Kind.allCases.contains(.fromTheVault) + case .premiumDeck: MTGSet.Kind.allCases.contains(.premiumDeck) + case .duelDeck: MTGSet.Kind.allCases.contains(.duelDeck) + case .draftInnovation: MTGSet.Kind.allCases.contains(.draftInnovation) + case .treasureChest: MTGSet.Kind.allCases.contains(.treasureChest) + case .unknown(let string): + // Unknown case shouldn't be in allCases + !MTGSet.Kind.allCases.contains(.unknown(string)) + } + + XCTAssertTrue(contains) + } +} diff --git a/Tests/ScryfallKitTests/SmokeTests.swift b/Tests/ScryfallKitTests/SmokeTests.swift index b1ed21a..fc18324 100644 --- a/Tests/ScryfallKitTests/SmokeTests.swift +++ b/Tests/ScryfallKitTests/SmokeTests.swift @@ -17,7 +17,7 @@ final class SmokeTests: XCTestCase { func testLayouts() async throws { // Verify that we can handle all layout types // Skip double sided because there aren't any double_sided or battle cards being returned by Scryfall - for layout in Card.Layout.allCases where ![.doubleSided, .unknown, .battle].contains(layout) { + for layout in Card.Layout.allCases where ![.doubleSided, .battle].contains(layout) { let cards = try await client.searchCards(query: "layout:\(layout.rawValue)") checkForUnknowns(in: cards.data) } @@ -175,11 +175,11 @@ final class SmokeTests: XCTestCase { private func checkForUnknowns(in cards: [Card]) { for card in cards { - XCTAssertNotEqual(card.layout, .unknown, "Unknown layout on \(card.name)") - XCTAssertNotEqual(card.setType, .unknown, "Unknown set type on \(card.name)") if let frameEffects = card.frameEffects { for effect in frameEffects { - XCTAssertNotEqual(effect, .unknown, "Unknown frame effect on \(card.name) [\(card.set)]") + if case .unknown(let string) = effect { + XCTFail("Unknown frame effect: \(string)") + } } } } From 5be007de427e4effbc2d3caa6fa2b6aeaf47c671 Mon Sep 17 00:00:00 2001 From: Jacob Hearst Date: Mon, 10 Feb 2025 14:56:19 -0600 Subject: [PATCH 2/2] Add docstrings --- Sources/ScryfallKit/Models/Card/Card+enums.swift | 5 ++++- Sources/ScryfallKit/Models/MTGSet.swift | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Sources/ScryfallKit/Models/Card/Card+enums.swift b/Sources/ScryfallKit/Models/Card/Card+enums.swift index 432b34b..4d963e2 100644 --- a/Sources/ScryfallKit/Models/Card/Card+enums.swift +++ b/Sources/ScryfallKit/Models/Card/Card+enums.swift @@ -127,6 +127,8 @@ extension Card { public enum Layout: RawRepresentable, CaseIterable, Codable, Sendable, Equatable, Hashable { case normal, split, flip, transform, meld, leveler, saga, adventure, planar, scheme, vanguard, token, emblem, augment, host, `class`, battle, `case`, mutate, prototype, modalDfc, doubleSided, doubleFacedToken, artSeries, reversibleCard + + /// A layout that hasn't been added to ScryfallKit yet case unknown(String) /// All known Magic: the Gathering card layouts @@ -196,13 +198,14 @@ extension Card { } /// Effects applied to a Magic card frame - /// + /// /// [Scryfall documentation](https://scryfall.com/docs/api/frames#frame-effects) public enum FrameEffect: RawRepresentable, Codable, Sendable, CaseIterable, Equatable, Hashable { case legendary, miracle, nyxtouched, draft, devoid, tombstone, colorshifted, inverted, sunmoondfc, compasslanddfc, originpwdfc, mooneldrazidfc, waxingandwaningmoondfc, showcase, extendedart, companion, etched, snow, lesson, convertdfc, fandfc, battle, gravestone, fullart, vehicle, borderless, extended, spree, textless, enchantment, shatteredglass, upsidedowndfc + /// A layout that hasn't been added to ScryfallKit yet case unknown(String) /// All known Magic: the Gathering frame effects diff --git a/Sources/ScryfallKit/Models/MTGSet.swift b/Sources/ScryfallKit/Models/MTGSet.swift index f5bcfea..105c387 100644 --- a/Sources/ScryfallKit/Models/MTGSet.swift +++ b/Sources/ScryfallKit/Models/MTGSet.swift @@ -35,6 +35,7 @@ public struct MTGSet: Codable, Identifiable, Hashable, Sendable { // swiftlint:disable:next inclusive_language case core, expansion, masters, masterpiece, spellbook, commander, planechase, archenemy, vanguard, funny, starter, box, promo, token, memorabilia, arsenal, alchemy, minigame, fromTheVault, premiumDeck, duelDeck, draftInnovation, treasureChest + /// A layout that hasn't been added to ScryfallKit yet case unknown(String) public static let allCases: [Kind] = [