Skip to content

Commit 5104319

Browse files
ayoyayoy
authored
Add a check for adding singletons
Task URL: https://app.asana.com/1/137249556945/project/1203301625297703/task/1210209652200970?focus=true Description: This change adds a new check that should fail when new singletons are defined in Swift code. It checks for static let/var shared.
2 parents 478a494 + 5f98170 commit 5104319

4 files changed

Lines changed: 545 additions & 520 deletions

File tree

‎org/allPRs.ts‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ export const prSize = async () => {
55
const excludedExtensions = ['.xcodeproj', '.xcassets', '.xcworkspace'];
66

77
// Get all modified and added files (unique)
8-
const changedFiles = [...new Set([
8+
const changedFiles = [
99
...danger.git.modified_files,
1010
...danger.git.created_files
11-
])];
11+
];
1212

1313
// Filter out excluded file types
1414
const filesToCheck = changedFiles.filter(file =>
@@ -77,6 +77,30 @@ export const xcodeprojConfiguration_macOS = async () => {
7777
}
7878
}
7979

80+
export const singletons = async () => {
81+
const changedFiles = [
82+
...danger.git.modified_files,
83+
...danger.git.created_files
84+
].filter(file => file.endsWith(".swift"));
85+
86+
// If no files to check after filtering, exit early
87+
if (changedFiles.length === 0) {
88+
return;
89+
}
90+
91+
for (const file of changedFiles) {
92+
let diff = await danger.git.diffForFile(file);
93+
let addedLines = diff?.added.split(/\n/);
94+
const foundSingleton = addedLines?.find(value => /^\+(?!\s*\/\/)\s*(?:public|private|internal)?\s*static\s*(?:let|var)\s*shared(?:\s*:.+)?\s*=.*$/.test(value));
95+
if (foundSingleton) {
96+
// trim leading + and whitespace
97+
const cleanLine = foundSingleton.replace(/^\+\s*/, '').trim();
98+
fail(`New singleton definitions are not allowed. Found this line:\n\`\`\`swift\n${cleanLine}\n\`\`\``);
99+
return;
100+
}
101+
}
102+
}
103+
80104
export const localizedStrings = async () => {
81105
for (let file of danger.git.modified_files) {
82106
let diff = await danger.git.diffForFile(file);
@@ -193,11 +217,11 @@ export const releaseAndHotfixBranchBSKChangeWarning = async () => {
193217
const branchName = danger.github.pr.head.ref;
194218
if (!branchName.startsWith('release/') && !branchName.startsWith('hotfix/')) return;
195219

196-
const changedFiles = [...new Set([
220+
const changedFiles = [
197221
...danger.git.modified_files,
198222
...danger.git.created_files,
199223
...danger.git.deleted_files
200-
])];
224+
];
201225

202226
const bskFiles = changedFiles.filter(file => file.startsWith('BrowserServicesKit'));
203227
if (bskFiles.length === 0) return;
@@ -210,6 +234,7 @@ export default async () => {
210234
await prSize()
211235
await internalLink()
212236
await xcodeprojConfiguration_macOS()
237+
await singletons()
213238
await localizedStrings()
214239
await licensedFonts()
215240
await newColors()

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"@babel/preset-typescript": "^7.21.4",
1111
"@types/jest": "^26.0.23",
1212
"@types/node": "^15.0.2",
13-
"danger": "^11.2.0",
13+
"danger": "^13.0.4",
1414
"jest": "^29.0.0",
1515
"jest-junit": "^12.0.0",
1616
"ts-jest": "^29.1.0",

‎tests/singletons.allPRs.test.ts‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
jest.mock("danger", () => jest.fn())
2+
import danger from 'danger'
3+
const dm = danger as any;
4+
5+
import { singletons } from '../org/allPRs'
6+
7+
beforeEach(() => {
8+
dm.addedLines = ""
9+
dm.fail = jest.fn().mockReturnValue(true);
10+
11+
dm.danger = {
12+
git: {
13+
diffForFile: async (_filename) => {
14+
return { added: dm.addedLines }
15+
},
16+
modified_files: [
17+
"ModifiedFile.swift"
18+
],
19+
created_files: [
20+
"CreatedFile.swift"
21+
]
22+
}
23+
}
24+
})
25+
26+
describe("Singletons checks", () => {
27+
it("does not fail with no changes to Swift files", async () => {
28+
dm.danger.git.modified_files = []
29+
30+
await singletons()
31+
expect(dm.fail).not.toHaveBeenCalled()
32+
})
33+
34+
it("does not fail with no diff in Swift files", async () => {
35+
dm.danger.git.diffForFile = async (_filename) => {}
36+
37+
await singletons()
38+
expect(dm.fail).not.toHaveBeenCalled()
39+
})
40+
41+
it("does not fail with no additions", async () => {
42+
await singletons()
43+
expect(dm.fail).not.toHaveBeenCalled()
44+
})
45+
46+
it("does not fail with non-singleton additions", async () => {
47+
dm.addedLines = `
48+
+ let subscription = PrivacyProSubscription(status: .inactive)
49+
+ let isSubscribed: Bool
50+
+ var billingPeriod: String?
51+
+ public let startedAt: Int?
52+
+ internal let expiresOrRenewsAt: Int?
53+
+ private var paymentPlatform: String?
54+
+ let status: String?
55+
`
56+
57+
await singletons()
58+
expect(dm.fail).not.toHaveBeenCalled()
59+
})
60+
61+
it("does not fail with singleton usage additions", async () => {
62+
dm.addedLines = `
63+
+ let favicon = FaviconManager.shared.favicon(for: url)
64+
`
65+
66+
await singletons()
67+
expect(dm.fail).not.toHaveBeenCalled()
68+
})
69+
70+
it("does not fail with commented out singleton additions", async () => {
71+
dm.addedLines = `
72+
+ // static let shared = FaviconManager()
73+
`
74+
75+
await singletons()
76+
expect(dm.fail).not.toHaveBeenCalled()
77+
})
78+
79+
it("fails with singleton additions", async () => {
80+
dm.addedLines = `
81+
+ static let shared = FaviconManager()
82+
`
83+
84+
await singletons()
85+
expect(dm.fail).toHaveBeenCalled()
86+
})
87+
88+
it("fails with singleton additions with type specifier", async () => {
89+
dm.addedLines = `
90+
+ static let shared: FaviconManager = .init()
91+
`
92+
93+
await singletons()
94+
expect(dm.fail).toHaveBeenCalled()
95+
})
96+
97+
it("fails with private singleton additions", async () => {
98+
dm.addedLines = `
99+
+ private static let shared = FaviconManager()
100+
`
101+
102+
await singletons()
103+
expect(dm.fail).toHaveBeenCalled()
104+
})
105+
106+
it("fails with internal singleton additions", async () => {
107+
dm.addedLines = `
108+
+ internal static let shared = FaviconManager()
109+
`
110+
111+
await singletons()
112+
expect(dm.fail).toHaveBeenCalled()
113+
})
114+
115+
it("fails with public singleton additions", async () => {
116+
dm.addedLines = `
117+
+ public static let shared = FaviconManager()
118+
`
119+
120+
await singletons()
121+
expect(dm.fail).toHaveBeenCalled()
122+
})
123+
124+
it("fails with var singleton additions", async () => {
125+
dm.addedLines = `
126+
+ static var shared = FaviconManager()
127+
`
128+
129+
await singletons()
130+
expect(dm.fail).toHaveBeenCalled()
131+
})
132+
133+
it("fails with private var singleton additions", async () => {
134+
dm.addedLines = `
135+
+ private static var shared = FaviconManager()
136+
`
137+
138+
await singletons()
139+
expect(dm.fail).toHaveBeenCalled()
140+
})
141+
142+
it("fails with internal var singleton additions", async () => {
143+
dm.addedLines = `
144+
+ internal static var shared = FaviconManager()
145+
`
146+
147+
await singletons()
148+
expect(dm.fail).toHaveBeenCalled()
149+
})
150+
151+
it("fails with public var singleton additions", async () => {
152+
dm.addedLines = `
153+
+ public static var shared = FaviconManager()
154+
`
155+
156+
await singletons()
157+
expect(dm.fail).toHaveBeenCalled()
158+
})
159+
160+
it("does not fail with singleton deletions", async () => {
161+
dm.addedLines = `
162+
- static let shared = FaviconManager()
163+
- private static let shared = FaviconManager()
164+
- internal static let shared = FaviconManager()
165+
- public static let shared = FaviconManager()
166+
- static var shared = FaviconManager()
167+
- private static var shared = FaviconManager()
168+
- internal static var shared = FaviconManager()
169+
- public static var shared = FaviconManager()
170+
`
171+
172+
await singletons()
173+
expect(dm.fail).not.toHaveBeenCalled()
174+
})
175+
})

0 commit comments

Comments
 (0)