diff --git a/README.md b/README.md index fb59ffc..bb82015 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,8 @@ A CLI tool for managing tasks in the [firefox-ios](https://github.com/mozilla-mobile/firefox-ios) repository. +It is pronounced "fix-e-os". + **NOTE:** This tool is still in **BETA** ## Goals diff --git a/Sources/fxios/Commands/Nimbus/FeatureFlagsDebugViewControllerEditor.swift b/Sources/fxios/Commands/Nimbus/FeatureFlagsDebugViewControllerEditor.swift index 5e04c67..db3616b 100644 --- a/Sources/fxios/Commands/Nimbus/FeatureFlagsDebugViewControllerEditor.swift +++ b/Sources/fxios/Commands/Nimbus/FeatureFlagsDebugViewControllerEditor.swift @@ -24,7 +24,7 @@ enum FeatureFlagsDebugViewControllerEditor { var lastSettingEndIndex: Int? for (index, line) in lines.enumerated() { - if line.contains("var children: [Setting]") { + if line.contains("children: [Setting]") { inChildren = true continue } diff --git a/Sources/fxios/Commands/Nimbus/NimbusFeatureFlagLayerEditor.swift b/Sources/fxios/Commands/Nimbus/NimbusFeatureFlagLayerEditor.swift index 3952d64..59e0441 100644 --- a/Sources/fxios/Commands/Nimbus/NimbusFeatureFlagLayerEditor.swift +++ b/Sources/fxios/Commands/Nimbus/NimbusFeatureFlagLayerEditor.swift @@ -87,7 +87,7 @@ enum NimbusFeatureFlagLayerEditor { let caseCode = """ case .\(name): - return \(funcName)(from: nimbus) + return \(funcName)() """ lines.insert(caseCode, at: index) @@ -145,8 +145,8 @@ enum NimbusFeatureFlagLayerEditor { let funcName = "check\(StringUtils.capitalizeFirst(name))Feature" let funcCode = """ - private func \(funcName)(from nimbus: FxNimbus) -> Bool { - return nimbus.features.\(name).value().enabled + private func \(funcName)() -> Bool { + return nimbus.features.\(name)Feature.value().enabled } """ lines.insert(funcCode, at: index) diff --git a/Sources/fxios/Commands/Nimbus/NimbusFlaggableFeatureEditor.swift b/Sources/fxios/Commands/Nimbus/NimbusFlaggableFeatureEditor.swift index 807b971..010bb2f 100644 --- a/Sources/fxios/Commands/Nimbus/NimbusFlaggableFeatureEditor.swift +++ b/Sources/fxios/Commands/Nimbus/NimbusFlaggableFeatureEditor.swift @@ -10,7 +10,7 @@ enum NimbusFlaggableFeatureEditor { struct RemovalResult { let enumCaseRemoved: Bool let debugKeyRemoved: Bool? // nil = wasn't present, true = removed, false = failed to remove - let featureKeyRemoved: Bool + let userPrefsKeyRemoved: Bool? // nil = wasn't present, true = removed, false = failed to remove } static func addFeature( @@ -29,11 +29,9 @@ enum NimbusFlaggableFeatureEditor { content = try addToDebugKey(name: name, to: content) } - // 3. Add to featureKey + // 3. Add to userPrefsKey if user-toggleable if userToggleable { - content = try addUserToggleableCase(name: name, to: content) - } else { - content = try addToDefaultCase(name: name, to: content) + content = try addToUserPrefsKey(name: name, to: content) } try content.write(to: filePath, atomically: true, encoding: .utf8) @@ -56,17 +54,13 @@ enum NimbusFlaggableFeatureEditor { debugKeyRemoved = removed } - // 3. Determine if user-toggleable and remove from featureKey - let userToggleable = isUserToggleable(name: name, content: originalContent) - let featureKeyRemoved: Bool - if userToggleable { - let (contentAfterFeature, removed) = removeUserToggleableCase(name: name, from: content) - content = contentAfterFeature - featureKeyRemoved = removed - } else { - let (contentAfterFeature, removed) = removeFromDefaultCase(name: name, from: content) - content = contentAfterFeature - featureKeyRemoved = removed + // 3. Check if in userPrefsKey and try to remove + let wasInUserPrefsKey = isInUserPrefsKey(name: name, content: originalContent) + var userPrefsKeyRemoved: Bool? + if wasInUserPrefsKey { + let (contentAfterPrefs, removed) = removeFromUserPrefsKey(name: name, from: content) + content = contentAfterPrefs + userPrefsKeyRemoved = removed } try content.write(to: filePath, atomically: true, encoding: .utf8) @@ -74,7 +68,7 @@ enum NimbusFlaggableFeatureEditor { return RemovalResult( enumCaseRemoved: enumRemoved, debugKeyRemoved: debugKeyRemoved, - featureKeyRemoved: featureKeyRemoved + userPrefsKeyRemoved: userPrefsKeyRemoved ) } @@ -86,10 +80,25 @@ enum NimbusFlaggableFeatureEditor { content.contains("var debugKey: String?") } - private static func isUserToggleable(name: String, content: String) -> Bool { - // User-toggleable features have their own case in featureKey with fatalError or specific return - let userToggleablePattern = "case \\.\(name):\\s*\n\\s*(return FlagKeys\\.|fatalError)" - return content.range(of: userToggleablePattern, options: .regularExpression) != nil + private static func isInUserPrefsKey(name: String, content: String) -> Bool { + let lines = content.components(separatedBy: "\n") + var inUserPrefsKey = false + for line in lines { + if line.contains("var userPrefsKey: String?") { + inUserPrefsKey = true + continue + } + if inUserPrefsKey { + let trimmed = line.trimmingCharacters(in: .whitespaces) + if trimmed == "default:" || trimmed == "}" { + break + } + if trimmed.hasPrefix("case .\(name):") { + return true + } + } + } + return false } // MARK: - Enum Case Operations @@ -270,186 +279,73 @@ enum NimbusFlaggableFeatureEditor { return (content, false) } - // MARK: - featureKey Operations - - private static func addUserToggleableCase(name: String, to content: String) throws -> String { - var lines = content.components(separatedBy: "\n") - - // Find featureKey var and add a new case before the comment about non-toggleable cases - var inFeatureKey = false - var insertIndex: Int? - - for (index, line) in lines.enumerated() { - if line.contains("private var featureKey: String?") || line.contains("var featureKey: String?") { - inFeatureKey = true - continue - } - - if inFeatureKey { - // Insert before the comment about non-toggleable cases - if line.contains("Cases where users do not have the option") { - insertIndex = index - break - } - } - } - - guard let index = insertIndex else { - throw ValidationError("Could not find insertion point for featureKey user-toggleable case") - } - - let caseCode = """ - case .\(name): - fatalError("Please implement a key for this feature") - """ - lines.insert(caseCode, at: index) - - return lines.joined(separator: "\n") - } - - private static func removeUserToggleableCase(name: String, from content: String) -> (String, Bool) { - var lines = content.components(separatedBy: "\n") - - // Find and remove the case block - var removeStart: Int? - var removeEnd: Int? - - for (index, line) in lines.enumerated() - where line.trimmingCharacters(in: .whitespaces) == "case .\(name):" { - removeStart = index - // Find the end of this case (next case or default) - for i in (index + 1).. String { + private static func addToUserPrefsKey(name: String, to content: String) throws -> String { var lines = content.components(separatedBy: "\n") - // Find the default case in featureKey (the one with return nil) - var inFeatureKey = false - var inDefaultCase = false + var inUserPrefsKey = false var insertIndex: Int? + var lastCaseIndex: Int? for (index, line) in lines.enumerated() { - if line.contains("private var featureKey: String?") || line.contains("var featureKey: String?") { - inFeatureKey = true + if line.contains("var userPrefsKey: String?") { + inUserPrefsKey = true continue } - if inFeatureKey { - if line.contains("Cases where users do not have the option") { - inDefaultCase = true - continue - } - - if inDefaultCase { - let trimmed = line.trimmingCharacters(in: .whitespaces) + if inUserPrefsKey { + let trimmed = line.trimmingCharacters(in: .whitespaces) - if trimmed.hasPrefix(".") { - let featureName = String(trimmed.dropFirst().prefix(while: { $0.isLetter || $0.isNumber })) + if trimmed.hasPrefix("case .") { + let caseName = String(trimmed.dropFirst(6).prefix(while: { $0.isLetter || $0.isNumber })) + lastCaseIndex = index - if featureName > name && insertIndex == nil { - insertIndex = index - } + if caseName > name && insertIndex == nil { + insertIndex = index } + } - // End of the case list (return nil) - if trimmed.hasPrefix("return nil") { - if insertIndex == nil { - // Insert before the last .feature: line - for i in stride(from: index - 1, through: 0, by: -1) { - let prevLine = lines[i].trimmingCharacters(in: .whitespaces) - if prevLine.hasPrefix(".") && prevLine.hasSuffix(":") { - // Change : to , and insert after - lines[i] = lines[i].replacingOccurrences(of: ":", with: ",") - insertIndex = i + 1 - break - } - } - } - break + if trimmed == "default:" { + if insertIndex == nil { + insertIndex = lastCaseIndex.map { $0 + 1 } ?? index } + break } } } guard let index = insertIndex else { - throw ValidationError("Could not find insertion point for featureKey default case") + throw ValidationError("Could not find insertion point in userPrefsKey") } - // Determine suffix - let nextLine = lines[index].trimmingCharacters(in: .whitespaces) - let suffix = nextLine.hasPrefix("return") ? ":" : "," - - lines.insert(" .\(name)\(suffix)", at: index) - - // If we inserted with :, change the next line's : to , - if suffix == ":" { - let nextIdx = index + 1 - if nextIdx < lines.count && lines[nextIdx].contains(":") { - let trimmedNext = lines[nextIdx].trimmingCharacters(in: .whitespaces) - if trimmedNext.hasPrefix(".") && trimmedNext.hasSuffix(":") { - lines[nextIdx] = lines[nextIdx].replacingOccurrences(of: ":", with: ",") - } - } - } + let newCase = " case .\(name): fatalError(\"Please implement a preference key for this feature\")" + lines.insert(newCase, at: index) return lines.joined(separator: "\n") } - private static func removeFromDefaultCase(name: String, from content: String) -> (String, Bool) { + private static func removeFromUserPrefsKey(name: String, from content: String) -> (String, Bool) { var lines = content.components(separatedBy: "\n") - var inFeatureKey = false - var inDefaultCase = false + var inUserPrefsKey = false for (index, line) in lines.enumerated() { - if line.contains("private var featureKey: String?") || line.contains("var featureKey: String?") { - inFeatureKey = true + if line.contains("var userPrefsKey: String?") { + inUserPrefsKey = true continue } - if inFeatureKey { - if line.contains("Cases where users do not have the option") { - inDefaultCase = true - continue - } + if inUserPrefsKey { + let trimmed = line.trimmingCharacters(in: .whitespaces) - if inDefaultCase { - let trimmed = line.trimmingCharacters(in: .whitespaces) - - if trimmed == ".\(name)," || trimmed == ".\(name):" { - // If this was the last entry, make previous entry end with : - if trimmed.hasSuffix(":") { - for i in stride(from: index - 1, through: 0, by: -1) { - let prevLine = lines[i].trimmingCharacters(in: .whitespaces) - if prevLine.hasPrefix(".") && prevLine.hasSuffix(",") { - lines[i] = lines[i].replacingOccurrences(of: ",", with: ":") - break - } - } - } - lines.remove(at: index) - return (lines.joined(separator: "\n"), true) - } + if trimmed.hasPrefix("case .\(name):") { + lines.remove(at: index) + return (lines.joined(separator: "\n"), true) + } - if trimmed.hasPrefix("return nil") { - break - } + if trimmed == "default:" || trimmed == "}" { + break } } } diff --git a/Sources/fxios/Commands/Nimbus/NimbusRemove.swift b/Sources/fxios/Commands/Nimbus/NimbusRemove.swift index c3fa614..bbb36a5 100644 --- a/Sources/fxios/Commands/Nimbus/NimbusRemove.swift +++ b/Sources/fxios/Commands/Nimbus/NimbusRemove.swift @@ -165,11 +165,13 @@ extension Nimbus { } } - if result.featureKeyRemoved { - reportSuccess("Removed from featureKey") - } else { - reportFailure("Could not find/remove from featureKey") - hasFailures = true + if let userPrefsKeyRemoved = result.userPrefsKeyRemoved { + if userPrefsKeyRemoved { + reportSuccess("Removed from userPrefsKey") + } else { + reportFailure("Found in userPrefsKey but could not remove") + hasFailures = true + } } } catch { reportFailure("Failed to process file: \(error.localizedDescription)") diff --git a/Tests/fxiosTests/NimbusTests.swift b/Tests/fxiosTests/NimbusTests.swift index aae7aef..bd89929 100644 --- a/Tests/fxiosTests/NimbusTests.swift +++ b/Tests/fxiosTests/NimbusTests.swift @@ -50,6 +50,15 @@ struct NimbusTests { case alpha case zeta + var userPrefsKey: String? { + typealias FlagKeys = PrefsKeys.FeatureFlags + + switch self { + case .alpha: return FlagKeys.Alpha + default: return nil + } + } + var debugKey: String? { switch self { case .alpha, @@ -60,20 +69,6 @@ struct NimbusTests { } } } - - struct NimbusFlaggableFeature { - private var featureKey: String? { - typealias FlagKeys = PrefsKeys.FeatureFlags - - switch featureID { - case .alpha: - return FlagKeys.Alpha - // Cases where users do not have the option to manipulate a setting. - case .zeta: - return nil - } - } - } """ let flaggableFeaturePath = featureFlagsDir.appendingPathComponent("FeatureFlagID.swift") try flaggableFeatureContent.write(to: flaggableFeaturePath, atomically: true, encoding: .utf8) @@ -81,25 +76,24 @@ struct NimbusTests { // Create NimbusFeatureFlagLayer.swift let flagLayerContent = """ final class NimbusFeatureFlagLayer { - public func checkNimbusConfigFor( - _ featureID: FeatureFlagID, - from nimbus: FxNimbus = FxNimbus.shared - ) -> Bool { + private let nimbus: FxNimbus + + public func checkNimbusConfigFor(_ featureID: FeatureFlagID) -> Bool { switch featureID { case .alpha: - return checkAlphaFeature(from: nimbus) + return checkAlphaFeature() case .zeta: - return checkZetaFeature(from: nimbus) + return checkZetaFeature() } } - private func checkAlphaFeature(from nimbus: FxNimbus) -> Bool { - return nimbus.features.alpha.value().enabled + private func checkAlphaFeature() -> Bool { + return nimbus.features.alphaFeature.value().enabled } - private func checkZetaFeature(from nimbus: FxNimbus) -> Bool { - return nimbus.features.zeta.value().enabled + private func checkZetaFeature() -> Bool { + return nimbus.features.zetaFeature.value().enabled } } """ @@ -110,7 +104,7 @@ struct NimbusTests { let debugVCContent = """ final class FeatureFlagsDebugViewController { private func generateFeatureFlagToggleSettings() -> SettingSection { - var children: [Setting] = [ + let children: [Setting] = [ FeatureFlagsBoolSetting( with: .alpha, titleText: format(string: "Alpha"), @@ -467,8 +461,6 @@ struct NimbusTests { // Should have added the enum case #expect(content.contains("case beta")) - // Should have added to the default case in featureKey (since no --user-toggleable) - #expect(content.contains(".beta")) } @Test("add command updates NimbusFeatureFlagLayer.swift") @@ -490,10 +482,10 @@ struct NimbusTests { // Should have added the switch case #expect(content.contains("case .beta:")) - #expect(content.contains("checkBetaFeature(from: nimbus)")) + #expect(content.contains("checkBetaFeature()")) // Should have added the check function - #expect(content.contains("private func checkBetaFeature(from nimbus: FxNimbus) -> Bool")) - #expect(content.contains("nimbus.features.beta.value().enabled")) + #expect(content.contains("private func checkBetaFeature() -> Bool")) + #expect(content.contains("nimbus.features.betaFeature.value().enabled")) } @Test("add command with --debuggable updates debuggable settings") @@ -541,7 +533,7 @@ struct NimbusTests { let content = try String(contentsOf: filePath, encoding: .utf8) #expect(content.contains("case .beta:")) - #expect(content.contains("fatalError(\"Please implement a key for this feature\")")) + #expect(content.contains("fatalError(\"Please implement a preference key for this feature\")")) } @Test("add command with --description adds description to template")