Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions org/allPRs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,97 @@ export const releaseAndHotfixBranchBSKChangeWarning = async () => {
warn(`Please check whether the BSK changes on this branch need to be merged to the other platform's release/hotfix branch`);
}

export const featureFlagAsanaLink = async () => {
const featureFlagFiles = [
"iOS/Core/FeatureFlag.swift",
"macOS/LocalPackages/FeatureFlags/Sources/FeatureFlags/FeatureFlag.swift"
];

const changedFiles = [
...danger.git.modified_files,
...danger.git.created_files
].filter(file => featureFlagFiles.includes(file));

if (changedFiles.length === 0) return;

const asanaTaskUrlRegex = /^\/\/\/\s*https:\/\/app\.asana\.com\/1\/137249556945\/project\/1211834678943996\/task\/\d+\s*$/;
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated

for (const file of changedFiles) {
const diff = await danger.git.diffForFile(file);
if (!diff?.diff) continue;

const diffLines = diff.diff.split(/\n/);
let insideFeatureFlagEnum = false;
let braceDepth = 0;

for (let i = 0; i < diffLines.length; i++) {
const line = diffLines[i];

// Skip diff metadata lines
if (line.startsWith("---") || line.startsWith("+++") || line.startsWith("\\")) continue;

// Reset context on new hunk headers
if (line.startsWith("@@")) {
insideFeatureFlagEnum = false;
braceDepth = 0;
// Check if the hunk header context mentions FeatureFlag enum
if (/enum\s+FeatureFlag\b/.test(line)) {
insideFeatureFlagEnum = true;
braceDepth = 1;
}
continue;
}

// Skip removed lines – they don't exist in the new file
if (line.startsWith("-")) continue;

// Get the actual content (strip the leading + or space)
const content = line.length > 0 ? line.substring(1) : "";

// Track enum FeatureFlag declaration
if (/\benum\s+FeatureFlag\b/.test(content)) {
insideFeatureFlagEnum = true;
braceDepth = 0;
}

// Track brace depth when inside FeatureFlag enum
if (insideFeatureFlagEnum) {
braceDepth += (content.match(/{/g) || []).length;
braceDepth -= (content.match(/}/g) || []).length;
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated

if (braceDepth <= 0) {
insideFeatureFlagEnum = false;
continue;
}
}

if (!insideFeatureFlagEnum) continue;

// Only check added lines for new case declarations
if (!line.startsWith("+")) continue;
if (!/^\s*case\s+\w+/.test(content)) continue;

// Found an added case line – walk upward through added comment lines looking for the Asana URL
let foundAsanaLink = false;
for (let j = i - 1; j >= 0; j--) {
const prevLine = diffLines[j];
if (!prevLine.startsWith("+")) break;
const prevContent = prevLine.substring(1).trim();
if (!prevContent.startsWith("///")) break;
if (asanaTaskUrlRegex.test(prevContent)) {
foundAsanaLink = true;
break;
}
}

if (!foundAsanaLink) {
warn(`New FeatureFlag case in \`${file}\` is missing a valid Feature Flag link in the comment.\nAdd a task in https://app.asana.com/1/137249556945/project/1211834678943996/list/1211838475578067 and use it in the comment.`);
return;
}
}
}
}

// Default run
export default async () => {
await prSize()
Expand All @@ -301,4 +392,5 @@ export default async () => {
await newColors()
await embeddedFilesURLMismatch()
await releaseAndHotfixBranchBSKChangeWarning()
await featureFlagAsanaLink()
}
254 changes: 254 additions & 0 deletions tests/featureFlagAsanaLink.allPRs.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,254 @@
jest.mock("danger", () => jest.fn())
import danger from 'danger'
const dm = danger as any;

import { featureFlagAsanaLink } from '../org/allPRs'

beforeEach(() => {
dm.diffContent = ""
dm.warn = jest.fn().mockReturnValue(true);

dm.danger = {
git: {
diffForFile: async (_filename) => {
return { added: dm.diffContent, diff: dm.diffContent }
},
modified_files: [
"iOS/Core/FeatureFlag.swift"
],
created_files: []
}
}
})

describe("Feature flag Asana link checks", () => {
it("does not fail when no feature flag files are changed", async () => {
dm.danger.git.modified_files = ["SomeOtherFile.swift"]

await featureFlagAsanaLink()
expect(dm.warn).not.toHaveBeenCalled()
})

it("does not fail when diff has no added case lines", async () => {
dm.diffContent = `@@ -10,6 +10,8 @@ enum FeatureFlag {
+ var rawValue: String
+ var source: FeatureFlagSource
`

await featureFlagAsanaLink()
expect(dm.warn).not.toHaveBeenCalled()
})

it("does not fail when added case has valid Asana link above it", async () => {
dm.diffContent = `@@ -10,6 +10,8 @@ enum FeatureFlag {
+ /// https://app.asana.com/1/137249556945/project/1211834678943996/task/123456789
+ case myNewFeature
`

await featureFlagAsanaLink()
expect(dm.warn).not.toHaveBeenCalled()
})

it("does not fail when added case with assignment has valid Asana link above it", async () => {
dm.diffContent = `@@ -10,6 +10,8 @@ enum FeatureFlag {
+ /// https://app.asana.com/1/137249556945/project/1211834678943996/task/999888777
+ case anotherFeature = "anotherFeature"
`

await featureFlagAsanaLink()
expect(dm.warn).not.toHaveBeenCalled()
})

it("fails when added case has no comment above it", async () => {
dm.diffContent = `@@ -10,6 +10,7 @@ enum FeatureFlag {
+ case myNewFeature
`

await featureFlagAsanaLink()
expect(dm.warn).toHaveBeenCalled()
})

it("fails when added case has a non-Asana comment above it", async () => {
dm.diffContent = `@@ -10,6 +10,8 @@ enum FeatureFlag {
+ /// Some description of the feature
+ case myNewFeature
`

await featureFlagAsanaLink()
expect(dm.warn).toHaveBeenCalled()
})

it("fails when added case has an Asana link with wrong project ID", async () => {
dm.diffContent = `@@ -10,6 +10,8 @@ enum FeatureFlag {
+ /// https://app.asana.com/1/137249556945/project/9999999999999999/task/123456789
+ case myNewFeature
`

await featureFlagAsanaLink()
expect(dm.warn).toHaveBeenCalled()
})

it("fails when comment above is a context line (not added)", async () => {
dm.diffContent = `@@ -10,6 +10,7 @@ enum FeatureFlag {
/// https://app.asana.com/1/137249556945/project/1211834678943996/task/123456789
+ case myNewFeature
`

await featureFlagAsanaLink()
expect(dm.warn).toHaveBeenCalled()
})

it("works with macOS feature flag file", async () => {
dm.danger.git.modified_files = [
"macOS/LocalPackages/FeatureFlags/Sources/FeatureFlags/FeatureFlag.swift"
]
dm.diffContent = `@@ -10,6 +10,8 @@ enum FeatureFlag {
+ /// https://app.asana.com/1/137249556945/project/1211834678943996/task/123456789
+ case macOSFeature
`

await featureFlagAsanaLink()
expect(dm.warn).not.toHaveBeenCalled()
})

it("fails for macOS feature flag file with missing link", async () => {
dm.danger.git.modified_files = [
"macOS/LocalPackages/FeatureFlags/Sources/FeatureFlags/FeatureFlag.swift"
]
dm.diffContent = `@@ -10,6 +10,7 @@ enum FeatureFlag {
+ case macOSFeature
`

await featureFlagAsanaLink()
expect(dm.warn).toHaveBeenCalled()
})

it("does not fail when diff is empty", async () => {
dm.danger.git.diffForFile = async (_filename) => { return null }

await featureFlagAsanaLink()
expect(dm.warn).not.toHaveBeenCalled()
})

it("does not fail for removed case lines", async () => {
dm.diffContent = `@@ -10,6 +10,5 @@ enum FeatureFlag {
- case removedFeature
`

await featureFlagAsanaLink()
expect(dm.warn).not.toHaveBeenCalled()
})

it("does not fail for case added in a different enum", async () => {
dm.diffContent = `@@ -50,6 +50,7 @@ enum SomeOtherEnum {
+ case otherEnumCase
`

await featureFlagAsanaLink()
expect(dm.warn).not.toHaveBeenCalled()
})

it("fails for case in FeatureFlag enum but not in other enum in same diff", async () => {
dm.diffContent = `@@ -10,6 +10,7 @@ enum SomeOtherEnum {
+ case otherEnumCase
@@ -50,6 +50,7 @@ enum FeatureFlag {
+ case featureFlagCase
`

await featureFlagAsanaLink()
expect(dm.warn).toHaveBeenCalled()
})

it("does not fail for case in other enum when FeatureFlag cases are valid", async () => {
dm.diffContent = `@@ -10,6 +10,7 @@ enum SomeOtherEnum {
+ case otherEnumCase
@@ -50,6 +50,8 @@ enum FeatureFlag {
+ /// https://app.asana.com/1/137249556945/project/1211834678943996/task/123456789
+ case featureFlagCase
`

await featureFlagAsanaLink()
expect(dm.warn).not.toHaveBeenCalled()
})

it("tracks FeatureFlag enum declared in the diff itself", async () => {
dm.diffContent = `@@ -10,6 +10,10 @@
+enum FeatureFlag: String {
+ case myNewFeature
+}
`

await featureFlagAsanaLink()
expect(dm.warn).toHaveBeenCalled()
})

it("does not fail when Asana link is above other comment lines", async () => {
dm.diffContent = `@@ -10,6 +10,10 @@ enum FeatureFlag {
+ /// https://app.asana.com/1/137249556945/project/1211834678943996/task/123456789
+ /// Some description of the feature
+ case myNewFeature
`

await featureFlagAsanaLink()
expect(dm.warn).not.toHaveBeenCalled()
})

it("does not fail when Asana link is several comment lines above case", async () => {
dm.diffContent = `@@ -10,6 +10,11 @@ enum FeatureFlag {
+ /// https://app.asana.com/1/137249556945/project/1211834678943996/task/123456789
+ /// First line of description
+ /// Second line of description
+ case myNewFeature
`

await featureFlagAsanaLink()
expect(dm.warn).not.toHaveBeenCalled()
})

it("fails when comment block has no Asana link at all", async () => {
dm.diffContent = `@@ -10,6 +10,10 @@ enum FeatureFlag {
+ /// First line of description
+ /// Second line of description
+ case myNewFeature
`

await featureFlagAsanaLink()
expect(dm.warn).toHaveBeenCalled()
})

it("does not check cases after FeatureFlag enum closing brace", async () => {
dm.diffContent = `@@ -10,6 +10,12 @@
+enum FeatureFlag: String {
+ /// https://app.asana.com/1/137249556945/project/1211834678943996/task/123456789
+ case validFeature
+}
+enum AnotherEnum {
+ case nolinkNeeded
+}
`

await featureFlagAsanaLink()
expect(dm.warn).not.toHaveBeenCalled()
})

it("ignores braces from removed lines when tracking FeatureFlag enum scope", async () => {
dm.diffContent = `@@ -10,6 +10,8 @@ enum FeatureFlag {
- }
+ /// https://app.asana.com/1/137249556945/project/1211834678943996/task/123456789
+ case myNewFeature
`

await featureFlagAsanaLink()
expect(dm.warn).not.toHaveBeenCalled()
})

it("warns when removed braces would have hidden a missing link", async () => {
dm.diffContent = `@@ -10,6 +10,8 @@ enum FeatureFlag {
- }
+ case myNewFeature
`

await featureFlagAsanaLink()
expect(dm.warn).toHaveBeenCalled()
})
})