Merge pull request #18 from duckduckgo/dominik/user-defaults-wrapper

Add Danger check for @UserDefaultsWrapper additions

Author: ayoy

org/allPRs.ts

@@ -111,11 +111,6 @@ export const singletons = async () => {
         ...danger.git.created_files
     ].filter(file => file.endsWith(".swift"));
 
-    // If no files to check after filtering, exit early
-    if (changedFiles.length === 0) {
-        return;
-    }
-
     for (const file of changedFiles) {
         let diff = await danger.git.diffForFile(file);
         let addedLines = diff?.added.split(/\n/);
@@ -129,15 +124,30 @@ export const singletons = async () => {
     }
 }
 
-export const remoteReleasableFeatureWarning = async () => {
+export const userDefaultsWrapper = async () => {
     const changedFiles = [
         ...danger.git.modified_files,
         ...danger.git.created_files
     ].filter(file => file.endsWith(".swift"));
 
-    if (changedFiles.length === 0) {
-        return;
+    for (const file of changedFiles) {
+        let diff = await danger.git.diffForFile(file);
+        let addedLines = diff?.added.split(/\n/);
+        const foundOccurrence = addedLines?.find(value => /^\+(?!\s*\/\/)\s*@UserDefaultsWrapper(?:\((?:.*\))?)?$/.test(value));
+        if (foundOccurrence) {
+            // trim leading + and whitespace
+            const cleanLine = foundOccurrence.replace(/^\+\s*/, '').trim();
+            fail(`New \`@UserDefaultsWrapper\` definitions are not allowed. Please use \`KeyedStoring\` protocol instead.\nFound this line:\n\`\`\`swift\n${cleanLine}\n\`\`\``);
+            return;
+        }
     }
+}
+
+export const remoteReleasableFeatureWarning = async () => {
+    const changedFiles = [
+        ...danger.git.modified_files,
+        ...danger.git.created_files
+    ].filter(file => file.endsWith(".swift"));
 
     for (const file of changedFiles) {
         let diff = await danger.git.diffForFile(file);
@@ -284,6 +294,7 @@ export default async () => {
     await xcodeprojConfiguration_macOS()
     await xcodeprojObjectVersion_macOS()
     await singletons()
+    await userDefaultsWrapper()
     await remoteReleasableFeatureWarning()
     await localizedStrings()
     await licensedFonts()

tests/userDefaultsWrapper.allPRs.test.ts

@@ -0,0 +1,127 @@
+jest.mock("danger", () => jest.fn())
+import danger from 'danger'
+const dm = danger as any;
+
+import { userDefaultsWrapper } from '../org/allPRs'
+
+beforeEach(() => {
+    dm.addedLines = ""
+    dm.fail = jest.fn().mockReturnValue(true);
+
+    dm.danger = {
+        git: {
+            diffForFile: async (_filename) => {
+                return { added: dm.addedLines }
+            },
+            modified_files: [
+                "ModifiedFile.swift"
+            ],
+            created_files: [
+                "CreatedFile.swift"
+            ]
+        }
+    }
+})
+
+describe("@UserDefaultsWrapper checks", () => {
+    it("does not fail with no changes to Swift files", async () => {
+        dm.danger.git.modified_files = []
+
+        await userDefaultsWrapper()
+        expect(dm.fail).not.toHaveBeenCalled()
+    })
+
+    it("does not fail with no diff in Swift files", async () => {
+        dm.danger.git.diffForFile = async (_filename) => {}
+
+        await userDefaultsWrapper()
+        expect(dm.fail).not.toHaveBeenCalled()
+    })
+
+    it("does not fail with no additions", async () => {
+        await userDefaultsWrapper()
+        expect(dm.fail).not.toHaveBeenCalled()
+    })
+
+    it("does not fail with non-@UserDefaultsWrapper additions", async () => {
+        dm.addedLines = `
++		let subscription = PrivacyProSubscription(status: .inactive)
++       let isSubscribed: Bool
++       var billingPeriod: String?
++       public let startedAt: Int?
++       internal let expiresOrRenewsAt: Int?
++       private var paymentPlatform: String?
++       let status: String?
+        `
+
+        await userDefaultsWrapper()
+        expect(dm.fail).not.toHaveBeenCalled()
+    })
+
+    it("does not fail with UserDefaultsWrapper usage additions", async () => {
+        dm.addedLines = `
++       let userDefaults: UserDefaults = UserDefaultsWrapper<Any>.sharedDefaults
+        `
+
+        await userDefaultsWrapper()
+        expect(dm.fail).not.toHaveBeenCalled()
+    })
+
+    it("does not fail with commented out @UserDefaultsWrapper additions", async () => {
+        dm.addedLines = `
++		// @UserDefaultsWrapper(key: .homePageIsFavoriteVisible, defaultValue: true)
+        `
+
+        await userDefaultsWrapper()
+        expect(dm.fail).not.toHaveBeenCalled()
+    })
+
+    it("fails with @UserDefaultsWrapper additions", async () => {
+        dm.addedLines = `
++		@UserDefaultsWrapper(key: .homePageIsFavoriteVisible, defaultValue: true)
+        `
+
+        await userDefaultsWrapper()
+        expect(dm.fail).toHaveBeenCalled()
+    })
+
+    it("fails with @UserDefaultsWrapper additions with open bracket only", async () => {
+        dm.addedLines = `
++		@UserDefaultsWrapper(
++           key: .homePageIsFavoriteVisible,
++           defaultValue: true
++       )
+        `
+
+        await userDefaultsWrapper()
+        expect(dm.fail).toHaveBeenCalled()
+    })
+
+    it("fails with @UserDefaultsWrapper additions without parameters", async () => {
+        dm.addedLines = `
++		@UserDefaultsWrapper
++       private var didCrashDuringCrashHandlersSetUp: Bool
+        `
+
+        await userDefaultsWrapper()
+        expect(dm.fail).toHaveBeenCalled()
+    })
+
+    it("does not fail with @UserDefaultsWrapper deletions", async () => {
+        dm.addedLines = `
+-		@UserDefaultsWrapper
+-       private var didCrashDuringCrashHandlersSetUp: Bool
+-
+-		@UserDefaultsWrapper(
+-           key: .homePageIsFavoriteVisible,
+-           defaultValue: true
+-       )
+-
+-		@UserDefaultsWrapper(key: .homePageIsFavoriteVisible, defaultValue: true)
+-       var isFavoriteVisible: Bool
+        `
+
+        await userDefaultsWrapper()
+        expect(dm.fail).not.toHaveBeenCalled()
+    })
+})