Skip to content

Commit d93e45b

Browse files
sverweijMcMeadow
andauthored
feat: adds support for [sections] - GitLab style (#204)
## Description - Adds support for sections as in use in GitLab They're not illegal on GitHub (CODEOWNERS with them do check out OK), but I'm not sure they're actually supported yet. ## Motivation and Context - They're 'legal' parts of a CODEOWNERS file both GitHub and on GitLab (and on GitLab they have a documented meaning). Before this PR the virtual-codeowners parser would most of the time reject lines with sections as 'unkown' syntax. - Sections can contain group names that need to be expanded in CODEOWNERS ## How Has This Been Tested? - [x] green ci - [x] Additional unit/ integration tests & expanded _corpus_ ## Types of changes - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] Documentation only change - [ ] Refactor (non-breaking change which fixes an issue without changing functionality) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) Co-authored-by: Lex McMeadow <McMeadow@users.noreply.github.com>
1 parent 9f61fb9 commit d93e45b

36 files changed

+741
-29
lines changed

.github/CODEOWNERS

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,4 @@
1515
# For things in .github also the eye of the admin group is useful
1616
.github/ @mcmeadow @sverweij
1717
tools/ @mcmeadow @sverweij
18-
README.md @sverweij
18+
README.md @sverweij

.github/VIRTUAL-CODEOWNERS.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,4 @@
77
# For things in .github also the eye of the admin group is useful
88
.github/ @vco-admins @admin
99
tools/ @vco-admins @tools
10-
README.md @vco @documentation
10+
README.md @vco @documentation

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ jobs:
5858
- run: npm test
5959
if: always() && matrix.node-version != '20.12.2'
6060
- name: run test & emit results to step summary
61-
# code coverage report on 20.x only, as --experimental-test-coverage on node > 20.12.2
61+
# code coverage report on 20.x only, as --experimental-test-coverage on node > 20.12.2
6262
# is not stable (gives varying results over identical runs)
6363
if: always() && matrix.node-version == '20.12.2'
6464
run: |

README.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,10 @@ libs/baarden/ jan@example.com korneel@example.com pier@example.com tjorus@
143143

144144
### Any gotcha's?
145145

146-
It won't check whether the users or teams you entered exist.
146+
- It won't check whether the users or teams you entered exist.
147+
- Only relevant when you're on GitLab: Section heading support is experimental
148+
and when generating labeler.yml default section owners aren't expanded to
149+
section rules.
147150

148151
### Do I have to run this each time I edit `VIRTUAL-CODEOWNERS.txt`?
149152

@@ -207,6 +210,10 @@ user/team names but doesn't verify their existence in the project.
207210

208211
- valid user/team names start with an `@` or are an e-mail address
209212
- valid rules have a file pattern and at least one user/team name
213+
- valid sections headings comply with the syntax described over at [GitLab](https://docs.gitlab.com/ee/user/project/codeowners/reference.html#sections)
214+
> different from GitLab's syntax the line `[bla @group` is not interpreted
215+
> as a rule, but as an erroneous section heading. This behaviour might change
216+
> to be the same as GitLab's in future releases without a major version bump.
210217

211218
### I want to specify different locations for the files (e.g. because I'm using GitLab)
212219

dist/codeowners/generate.js

Lines changed: 18 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/virtual-code-owners/parse.js

Lines changed: 37 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/virtual-code-owners/read.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
"prepack": "clean-pkg-json --dry --keep overrides --keep resolutions | jq '.scripts = {test: \"echo for test, build and static analysis scripts: see the github repository\"}' > smaller-package.json && mv smaller-package.json package.json && prettier --log-level warn --write --use-tabs package.json",
2727
"postpack": "git restore package.json",
2828
"scm:stage": "git add .",
29-
"test": "tsx --experimental-test-coverage --enable-source-maps --test-reporter ./tools/dot-with-summary.reporter.js --test src/*.test.ts src/**/*.test.ts",
29+
"test": "tsx --experimental-test-coverage --test-reporter ./tools/dot-with-summary.reporter.js --test src/*.test.ts src/**/*.test.ts",
3030
"update-dependencies": "npm run upem:update && npm run upem:install && npm run check",
3131
"upem-outdated": "npm outdated --json --long | upem --dry-run",
3232
"upem:install": "npm install",

src/codeowners/generate.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,19 @@ tools/ @team-tgif`;
6161
);
6262
});
6363

64+
it("replaces team names in section heads", () => {
65+
const lFixture = "[some section head] @team-sales @team-after-sales";
66+
const lTeamMapFixture = {
67+
"team-sales": ["jan", "pier", "tjorus"],
68+
"team-after-sales": ["wim", "zus", "jet"],
69+
};
70+
const lExpected = "[some section head] @jan @jet @pier @tjorus @wim @zus";
71+
equal(
72+
generateCodeOwnersFromString(lFixture, lTeamMapFixture, ""),
73+
lExpected,
74+
);
75+
});
76+
6477
it("correctly replaces a team name that is a substring of another one", () => {
6578
const lFixture = "tools/shared @substring";
6679
const lTeamMapFixture = {

src/codeowners/generate.ts

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,21 +38,34 @@ function generateLine(
3838
pTeamMap: ITeamMap,
3939
): string {
4040
if (pCSTLine.type === "rule") {
41-
const lUserNames = uniq(
42-
pCSTLine.users.flatMap((pUser) => expandTeamToUserNames(pUser, pTeamMap)),
43-
)
44-
.sort(compareUserNames)
45-
.join(" ");
4641
return (
4742
pCSTLine.filesPattern +
4843
pCSTLine.spaces +
49-
lUserNames +
44+
expandTeamsToUsersString(pCSTLine.users, pTeamMap) +
45+
(pCSTLine.inlineComment ? ` #${pCSTLine.inlineComment}` : "")
46+
);
47+
}
48+
if (pCSTLine.type === "section-heading") {
49+
return (
50+
(pCSTLine.optional ? "^" : "") +
51+
"[" +
52+
pCSTLine.sectionName +
53+
"]" +
54+
(pCSTLine.minApprovers ? `[${pCSTLine.minApprovers}]` : "") +
55+
pCSTLine.spaces +
56+
expandTeamsToUsersString(pCSTLine.users, pTeamMap) +
5057
(pCSTLine.inlineComment ? ` #${pCSTLine.inlineComment}` : "")
5158
);
5259
}
5360
return pCSTLine.raw;
5461
}
5562

63+
function expandTeamsToUsersString(pUsers: IUser[], pTeamMap: ITeamMap): string {
64+
return uniq(pUsers.flatMap((pUser) => expandTeamToUserNames(pUser, pTeamMap)))
65+
.sort(compareUserNames)
66+
.join(" ");
67+
}
68+
5669
function expandTeamToUserNames(pUser: IUser, pTeamMap: ITeamMap): string[] {
5770
if (pUser.type === "virtual-team-name") {
5871
return stringifyTeamMembers(pTeamMap, pUser.bareName);

src/labeler-yml/generate.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { EOL } from "node:os";
22
import type { ITeamMap } from "../team-map/team-map.js";
33
import type {
4-
IInterestingCSTLine,
4+
IRuleCSTLine,
55
IVirtualCodeOwnersCST,
66
} from "../virtual-code-owners/cst.js";
77

@@ -48,12 +48,12 @@ function getPatternsForTeam(
4848
.filter((pLine) => {
4949
return (
5050
pLine.type === "rule" &&
51-
lineContainsTeamName(pLine as IInterestingCSTLine, pTeamName)
51+
lineContainsTeamName(pLine as IRuleCSTLine, pTeamName)
5252
);
5353
})
5454
// @ts-expect-error ts thinks it can still be an IBoringCSTLine,
5555
// but with the filter above we've ruled that out
56-
.map((pLine: IInterestingCSTLine) => pLine.filesPattern)
56+
.map((pLine: IRuleCSTLine) => pLine.filesPattern)
5757
);
5858
}
5959

@@ -93,10 +93,7 @@ function transformForYamlAndMinimatch(pOriginalString: string): string {
9393
return lReturnValue;
9494
}
9595

96-
function lineContainsTeamName(
97-
pLine: IInterestingCSTLine,
98-
pTeamName: string,
99-
): boolean {
96+
function lineContainsTeamName(pLine: IRuleCSTLine, pTeamName: string): boolean {
10097
return pLine.users.some(
10198
(pUser) =>
10299
pUser.type === "virtual-team-name" && pUser.bareName === pTeamName,

src/team-map/read.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ function assertTeamMapValid(pTeamMap: ITeamMap, pVirtualTeamsFileName: string) {
2626
if (!ajv.validate(virtualTeamsSchema, pTeamMap)) {
2727
throw new Error(
2828
`This is not a valid virtual-teams.yml:${EOL}${formatAjvErrors(
29-
ajv.errors,
29+
ajv.errors as any[],
3030
pVirtualTeamsFileName,
3131
)}.\n`,
3232
);
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[required-section]
2+
aap/ @some-user @some-other-user
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
[
2+
{ line: 1, raw: "[required-section]", type: "section-without-users" },
3+
{
4+
filesPattern: "aap/",
5+
inlineComment: "",
6+
line: 2,
7+
raw: "aap/ @some-user @some-other-user",
8+
spaces: " ",
9+
type: "rule",
10+
users:
11+
[
12+
{
13+
bareName: "some-user",
14+
raw: "@some-user",
15+
type: "other-user-or-team",
16+
userNumberWithinLine: 1,
17+
},
18+
{
19+
bareName: "some-other-user",
20+
raw: "@some-other-user",
21+
type: "other-user-or-team",
22+
userNumberWithinLine: 2,
23+
},
24+
],
25+
},
26+
]
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[required-section][42]
2+
aap/ @some-user @some-other-user
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
[
2+
{ line: 1, raw: "[required-section][42]", type: "section-without-users" },
3+
{
4+
filesPattern: "aap/",
5+
inlineComment: "",
6+
line: 2,
7+
raw: "aap/ @some-user @some-other-user",
8+
spaces: " ",
9+
type: "rule",
10+
users:
11+
[
12+
{
13+
bareName: "some-user",
14+
raw: "@some-user",
15+
type: "other-user-or-team",
16+
userNumberWithinLine: 1,
17+
},
18+
{
19+
bareName: "some-other-user",
20+
raw: "@some-other-user",
21+
type: "other-user-or-team",
22+
userNumberWithinLine: 2,
23+
},
24+
],
25+
},
26+
]
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
^[optional-section]
2+
aap/ @some-user @some-other-user
3+
4+
^[optional-section-with-default] @koos-koets
5+
noot/ @robbie
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
[
2+
{ line: 1, raw: "^[optional-section]", type: "section-without-users" },
3+
{
4+
filesPattern: "aap/",
5+
inlineComment: "",
6+
line: 2,
7+
raw: "aap/ @some-user @some-other-user",
8+
spaces: " ",
9+
type: "rule",
10+
users:
11+
[
12+
{
13+
bareName: "some-user",
14+
raw: "@some-user",
15+
type: "other-user-or-team",
16+
userNumberWithinLine: 1,
17+
},
18+
{
19+
bareName: "some-other-user",
20+
raw: "@some-other-user",
21+
type: "other-user-or-team",
22+
userNumberWithinLine: 2,
23+
},
24+
],
25+
},
26+
{ line: 3, raw: "", type: "empty" },
27+
{
28+
inlineComment: "",
29+
line: 4,
30+
optional: true,
31+
raw: "^[optional-section-with-default] @koos-koets",
32+
sectionName: "optional-section-with-default",
33+
spaces: " ",
34+
type: "section-heading",
35+
users:
36+
[
37+
{
38+
bareName: "koos-koets",
39+
raw: "@koos-koets",
40+
type: "other-user-or-team",
41+
userNumberWithinLine: 1,
42+
},
43+
],
44+
},
45+
{
46+
filesPattern: "noot/",
47+
inlineComment: "",
48+
line: 5,
49+
raw: "noot/ @robbie",
50+
spaces: " ",
51+
type: "rule",
52+
users:
53+
[
54+
{
55+
bareName: "robbie",
56+
raw: "@robbie",
57+
type: "other-user-or-team",
58+
userNumberWithinLine: 1,
59+
},
60+
],
61+
},
62+
]
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[required-section] @default-user @default-group
2+
aap/ @some-user @some-other-user

0 commit comments

Comments
 (0)