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 <[email protected]>

.github/CODEOWNERS

@@ -15,4 +15,4 @@
 # For things in .github also the eye of the admin group is useful
 .github/  @mcmeadow @sverweij
 tools/    @mcmeadow @sverweij
-README.md @sverweij
+README.md @sverweij

.github/VIRTUAL-CODEOWNERS.txt

@@ -7,4 +7,4 @@
 # For things in .github also the eye of the admin group is useful
 .github/  @vco-admins @admin
 tools/    @vco-admins @tools
-README.md @vco @documentation
+README.md @vco @documentation

.github/workflows/ci.yml

@@ -58,7 +58,7 @@ jobs:
       - run: npm test
         if: always() && matrix.node-version != '20.12.2'
       - name: run test & emit results to step summary
-        # code coverage report on 20.x only, as --experimental-test-coverage on node > 20.12.2 
+        # code coverage report on 20.x only, as --experimental-test-coverage on node > 20.12.2
         # is not stable (gives varying results over identical runs)
         if: always() && matrix.node-version == '20.12.2'
         run: |

README.md

@@ -143,7 +143,10 @@ libs/baarden/       [email protected] [email protected] [email protected] tjorus@
 
 ### Any gotcha's?
 
-It won't check whether the users or teams you entered exist.
+- It won't check whether the users or teams you entered exist.
+- Only relevant when you're on GitLab: Section heading support is experimental
+  and when generating labeler.yml default section owners aren't expanded to
+  section rules.
 
 ### Do I have to run this each time I edit `VIRTUAL-CODEOWNERS.txt`?
 
@@ -207,6 +210,10 @@ user/team names but doesn't verify their existence in the project.
 
 - valid user/team names start with an `@` or are an e-mail address
 - valid rules have a file pattern and at least one user/team name
+- valid sections headings comply with the syntax described over at [GitLab](https://docs.gitlab.com/ee/user/project/codeowners/reference.html#sections)
+  > different from GitLab's syntax the line `[bla @group` is not interpreted
+  > as a rule, but as an erroneous section heading. This behaviour might change
+  > to be the same as GitLab's in future releases without a major version bump.
 
 ### I want to specify different locations for the files (e.g. because I'm using GitLab)
 

package.json

@@ -26,7 +26,7 @@
     "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",
     "postpack": "git restore package.json",
     "scm:stage": "git add .",
-    "test": "tsx --experimental-test-coverage --enable-source-maps --test-reporter ./tools/dot-with-summary.reporter.js --test src/*.test.ts src/**/*.test.ts",
+    "test": "tsx --experimental-test-coverage --test-reporter ./tools/dot-with-summary.reporter.js --test src/*.test.ts src/**/*.test.ts",
     "update-dependencies": "npm run upem:update && npm run upem:install && npm run check",
     "upem-outdated": "npm outdated --json --long | upem --dry-run",
     "upem:install": "npm install",

src/codeowners/generate.test.ts

@@ -61,6 +61,19 @@ tools/ @team-tgif`;
     );
   });
 
+  it("replaces team names in section heads", () => {
+    const lFixture = "[some section head] @team-sales @team-after-sales";
+    const lTeamMapFixture = {
+      "team-sales": ["jan", "pier", "tjorus"],
+      "team-after-sales": ["wim", "zus", "jet"],
+    };
+    const lExpected = "[some section head] @jan @jet @pier @tjorus @wim @zus";
+    equal(
+      generateCodeOwnersFromString(lFixture, lTeamMapFixture, ""),
+      lExpected,
+    );
+  });
+
   it("correctly replaces a team name that is a substring of another one", () => {
     const lFixture = "tools/shared @substring";
     const lTeamMapFixture = {

src/codeowners/generate.ts

@@ -38,21 +38,34 @@ function generateLine(
   pTeamMap: ITeamMap,
 ): string {
   if (pCSTLine.type === "rule") {
-    const lUserNames = uniq(
-      pCSTLine.users.flatMap((pUser) => expandTeamToUserNames(pUser, pTeamMap)),
-    )
-      .sort(compareUserNames)
-      .join(" ");
     return (
       pCSTLine.filesPattern +
       pCSTLine.spaces +
-      lUserNames +
+      expandTeamsToUsersString(pCSTLine.users, pTeamMap) +
+      (pCSTLine.inlineComment ? ` #${pCSTLine.inlineComment}` : "")
+    );
+  }
+  if (pCSTLine.type === "section-heading") {
+    return (
+      (pCSTLine.optional ? "^" : "") +
+      "[" +
+      pCSTLine.sectionName +
+      "]" +
+      (pCSTLine.minApprovers ? `[${pCSTLine.minApprovers}]` : "") +
+      pCSTLine.spaces +
+      expandTeamsToUsersString(pCSTLine.users, pTeamMap) +
       (pCSTLine.inlineComment ? ` #${pCSTLine.inlineComment}` : "")
     );
   }
   return pCSTLine.raw;
 }
 
+function expandTeamsToUsersString(pUsers: IUser[], pTeamMap: ITeamMap): string {
+  return uniq(pUsers.flatMap((pUser) => expandTeamToUserNames(pUser, pTeamMap)))
+    .sort(compareUserNames)
+    .join(" ");
+}
+
 function expandTeamToUserNames(pUser: IUser, pTeamMap: ITeamMap): string[] {
   if (pUser.type === "virtual-team-name") {
     return stringifyTeamMembers(pTeamMap, pUser.bareName);

src/labeler-yml/generate.ts

@@ -1,7 +1,7 @@
 import { EOL } from "node:os";
 import type { ITeamMap } from "../team-map/team-map.js";
 import type {
-  IInterestingCSTLine,
+  IRuleCSTLine,
   IVirtualCodeOwnersCST,
 } from "../virtual-code-owners/cst.js";
 
@@ -48,12 +48,12 @@ function getPatternsForTeam(
       .filter((pLine) => {
         return (
           pLine.type === "rule" &&
-          lineContainsTeamName(pLine as IInterestingCSTLine, pTeamName)
+          lineContainsTeamName(pLine as IRuleCSTLine, pTeamName)
         );
       })
       // @ts-expect-error ts thinks it can still be an IBoringCSTLine,
       // but with the filter above we've ruled that out
-      .map((pLine: IInterestingCSTLine) => pLine.filesPattern)
+      .map((pLine: IRuleCSTLine) => pLine.filesPattern)
   );
 }
 
@@ -93,10 +93,7 @@ function transformForYamlAndMinimatch(pOriginalString: string): string {
   return lReturnValue;
 }
 
-function lineContainsTeamName(
-  pLine: IInterestingCSTLine,
-  pTeamName: string,
-): boolean {
+function lineContainsTeamName(pLine: IRuleCSTLine, pTeamName: string): boolean {
   return pLine.users.some(
     (pUser) =>
       pUser.type === "virtual-team-name" && pUser.bareName === pTeamName,

src/team-map/read.ts

@@ -26,7 +26,7 @@ function assertTeamMapValid(pTeamMap: ITeamMap, pVirtualTeamsFileName: string) {
   if (!ajv.validate(virtualTeamsSchema, pTeamMap)) {
     throw new Error(
       `This is not a valid virtual-teams.yml:${EOL}${formatAjvErrors(
-        ajv.errors,
+        ajv.errors as any[],
         pVirtualTeamsFileName,
       )}.\n`,
     );

src/virtual-code-owners/__fixtures__/corpus/empty-teams/015-section-required.txt

@@ -0,0 +1,2 @@
+[required-section]
+aap/ @some-user @some-other-user

src/virtual-code-owners/__fixtures__/corpus/empty-teams/015-section-required.yml

@@ -0,0 +1,26 @@
+[
+  { line: 1, raw: "[required-section]", type: "section-without-users" },
+  {
+    filesPattern: "aap/",
+    inlineComment: "",
+    line: 2,
+    raw: "aap/ @some-user @some-other-user",
+    spaces: " ",
+    type: "rule",
+    users:
+      [
+        {
+          bareName: "some-user",
+          raw: "@some-user",
+          type: "other-user-or-team",
+          userNumberWithinLine: 1,
+        },
+        {
+          bareName: "some-other-user",
+          raw: "@some-other-user",
+          type: "other-user-or-team",
+          userNumberWithinLine: 2,
+        },
+      ],
+  },
+]

src/virtual-code-owners/__fixtures__/corpus/empty-teams/021-section-required-min-approvers.txt

@@ -0,0 +1,2 @@
+[required-section][42]
+aap/ @some-user @some-other-user

src/virtual-code-owners/__fixtures__/corpus/empty-teams/021-section-required-min-approvers.yml

@@ -0,0 +1,26 @@
+[
+  { line: 1, raw: "[required-section][42]", type: "section-without-users" },
+  {
+    filesPattern: "aap/",
+    inlineComment: "",
+    line: 2,
+    raw: "aap/ @some-user @some-other-user",
+    spaces: " ",
+    type: "rule",
+    users:
+      [
+        {
+          bareName: "some-user",
+          raw: "@some-user",
+          type: "other-user-or-team",
+          userNumberWithinLine: 1,
+        },
+        {
+          bareName: "some-other-user",
+          raw: "@some-other-user",
+          type: "other-user-or-team",
+          userNumberWithinLine: 2,
+        },
+      ],
+  },
+]

src/virtual-code-owners/__fixtures__/corpus/empty-teams/022-section-optional.txt

@@ -0,0 +1,5 @@
+^[optional-section]
+aap/ @some-user @some-other-user
+
+^[optional-section-with-default] @koos-koets
+noot/ @robbie

src/virtual-code-owners/__fixtures__/corpus/empty-teams/022-section-optional.yml

@@ -0,0 +1,62 @@
+[
+  { line: 1, raw: "^[optional-section]", type: "section-without-users" },
+  {
+    filesPattern: "aap/",
+    inlineComment: "",
+    line: 2,
+    raw: "aap/ @some-user @some-other-user",
+    spaces: " ",
+    type: "rule",
+    users:
+      [
+        {
+          bareName: "some-user",
+          raw: "@some-user",
+          type: "other-user-or-team",
+          userNumberWithinLine: 1,
+        },
+        {
+          bareName: "some-other-user",
+          raw: "@some-other-user",
+          type: "other-user-or-team",
+          userNumberWithinLine: 2,
+        },
+      ],
+  },
+  { line: 3, raw: "", type: "empty" },
+  {
+    inlineComment: "",
+    line: 4,
+    optional: true,
+    raw: "^[optional-section-with-default] @koos-koets",
+    sectionName: "optional-section-with-default",
+    spaces: " ",
+    type: "section-heading",
+    users:
+      [
+        {
+          bareName: "koos-koets",
+          raw: "@koos-koets",
+          type: "other-user-or-team",
+          userNumberWithinLine: 1,
+        },
+      ],
+  },
+  {
+    filesPattern: "noot/",
+    inlineComment: "",
+    line: 5,
+    raw: "noot/ @robbie",
+    spaces: " ",
+    type: "rule",
+    users:
+      [
+        {
+          bareName: "robbie",
+          raw: "@robbie",
+          type: "other-user-or-team",
+          userNumberWithinLine: 1,
+        },
+      ],
+  },
+]

src/virtual-code-owners/__fixtures__/corpus/empty-teams/023-section-required-with-default-owners.txt

@@ -0,0 +1,2 @@
+[required-section] @default-user @default-group
+aap/ @some-user @some-other-user