From 7f2de6e2c3d17e8502bb2523e8f6c6ba02b17123 Mon Sep 17 00:00:00 2001 From: sverweij Date: Sat, 11 May 2024 19:50:17 +0200 Subject: [PATCH 1/5] feat: adds support for [sections] - GitLab style --- .github/CODEOWNERS | 15 +- .github/VIRTUAL-CODEOWNERS.txt | 15 +- .github/labeler.yml | 4 + README.md | 9 +- dist/codeowners/generate.js | 17 ++ dist/virtual-code-owners/parse.js | 36 +++ dist/virtual-code-owners/read.js | 2 +- package.json | 2 +- src/codeowners/generate.test.ts | 13 + src/codeowners/generate.ts | 17 ++ src/labeler-yml/generate.ts | 11 +- src/team-map/read.ts | 2 +- .../empty-teams/015-section-required.txt | 2 + .../empty-teams/015-section-required.yml | 26 ++ .../021-section-required-min-approvers.txt | 2 + .../021-section-required-min-approvers.yml | 26 ++ .../empty-teams/022-section-optional.txt | 5 + .../empty-teams/022-section-optional.yml | 62 ++++ ...3-section-required-with-default-owners.txt | 2 + ...3-section-required-with-default-owners.yml | 49 +++ ...ired-with-default-owners-min-approvers.txt | 2 + ...ired-with-default-owners-min-approvers.yml | 50 +++ .../empty-teams/025-section-without-end.txt | 2 + .../empty-teams/025-section-without-end.yml | 20 ++ ...ual-team.txt => 040-rule-virtual-team.txt} | 0 ...ual-team.yml => 040-rule-virtual-team.yml} | 0 ...-teams.txt => 041-rule-multiple-teams.txt} | 0 ...-teams.yml => 041-rule-multiple-teams.yml} | 0 ...chen-sink.txt => 050-the-kitchen-sink.txt} | 0 ...chen-sink.yml => 050-the-kitchen-sink.yml} | 0 .../051-the-kitchen-sink-with-sections.txt | 29 ++ .../051-the-kitchen-sink-with-sections.yml | 289 ++++++++++++++++++ src/virtual-code-owners/cst.d.ts | 22 +- src/virtual-code-owners/parse.test.ts | 1 + src/virtual-code-owners/parse.ts | 47 +++ src/virtual-code-owners/read.ts | 2 +- 36 files changed, 765 insertions(+), 16 deletions(-) create mode 100644 src/virtual-code-owners/__fixtures__/corpus/empty-teams/015-section-required.txt create mode 100644 src/virtual-code-owners/__fixtures__/corpus/empty-teams/015-section-required.yml create mode 100644 src/virtual-code-owners/__fixtures__/corpus/empty-teams/021-section-required-min-approvers.txt create mode 100644 src/virtual-code-owners/__fixtures__/corpus/empty-teams/021-section-required-min-approvers.yml create mode 100644 src/virtual-code-owners/__fixtures__/corpus/empty-teams/022-section-optional.txt create mode 100644 src/virtual-code-owners/__fixtures__/corpus/empty-teams/022-section-optional.yml create mode 100644 src/virtual-code-owners/__fixtures__/corpus/empty-teams/023-section-required-with-default-owners.txt create mode 100644 src/virtual-code-owners/__fixtures__/corpus/empty-teams/023-section-required-with-default-owners.yml create mode 100644 src/virtual-code-owners/__fixtures__/corpus/empty-teams/024-section-required-with-default-owners-min-approvers.txt create mode 100644 src/virtual-code-owners/__fixtures__/corpus/empty-teams/024-section-required-with-default-owners-min-approvers.yml create mode 100644 src/virtual-code-owners/__fixtures__/corpus/empty-teams/025-section-without-end.txt create mode 100644 src/virtual-code-owners/__fixtures__/corpus/empty-teams/025-section-without-end.yml rename src/virtual-code-owners/__fixtures__/corpus/teams/{020-rule-virtual-team.txt => 040-rule-virtual-team.txt} (100%) rename src/virtual-code-owners/__fixtures__/corpus/teams/{020-rule-virtual-team.yml => 040-rule-virtual-team.yml} (100%) rename src/virtual-code-owners/__fixtures__/corpus/teams/{021-rule-multiple-teams.txt => 041-rule-multiple-teams.txt} (100%) rename src/virtual-code-owners/__fixtures__/corpus/teams/{021-rule-multiple-teams.yml => 041-rule-multiple-teams.yml} (100%) rename src/virtual-code-owners/__fixtures__/corpus/teams/{030-the-kitchen-sink.txt => 050-the-kitchen-sink.txt} (100%) rename src/virtual-code-owners/__fixtures__/corpus/teams/{030-the-kitchen-sink.yml => 050-the-kitchen-sink.yml} (100%) create mode 100644 src/virtual-code-owners/__fixtures__/corpus/teams/051-the-kitchen-sink-with-sections.txt create mode 100644 src/virtual-code-owners/__fixtures__/corpus/teams/051-the-kitchen-sink-with-sections.yml diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 824ca73..8cd13c5 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -15,4 +15,17 @@ # For things in .github also the eye of the admin group is useful .github/ @mcmeadow @sverweij tools/ @mcmeadow @sverweij -README.md @sverweij \ No newline at end of file +README.md @sverweij + +[section] +bla @sverweij + +[section-with-owners][(1)] @mcmeadow @sverweij +# blabla # Patronen ohne Nutzernahmen sind erlaubt in Sectionen mit Nutzernamen im Titel +bladiebla @other-owner-still + +[section-with-approvers][3] +# Diese Section ist leer. Is das erlaubt? + +^[optional-section] +stonks/ @mcmeadow @sverweij diff --git a/.github/VIRTUAL-CODEOWNERS.txt b/.github/VIRTUAL-CODEOWNERS.txt index 577dcaf..0825bd1 100644 --- a/.github/VIRTUAL-CODEOWNERS.txt +++ b/.github/VIRTUAL-CODEOWNERS.txt @@ -7,4 +7,17 @@ # 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 \ No newline at end of file +README.md @vco @documentation + +[section] +bla @vco + +[section-with-owners] @vco @vco-admins +# blabla # Patronen ohne Nutzernahmen sind erlaubt in Sectionen mit Nutzernamen im Titel +bladiebla @other-owner-still + +[section-with-approvers][3] +# Diese Section ist leer. Is das erlaubt? + +^[optional-section] +stonks/ @vco-admins @tools diff --git a/.github/labeler.yml b/.github/labeler.yml index 9c6463c..1b5a1bc 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -12,15 +12,18 @@ vco: - changed-files: - any-glob-to-any-file: "**" - any-glob-to-any-file: README.md + - any-glob-to-any-file: bla vco-admins: - changed-files: - any-glob-to-any-file: .github/** - any-glob-to-any-file: tools/** + - any-glob-to-any-file: stonks/** tools: - changed-files: - any-glob-to-any-file: tools/** + - any-glob-to-any-file: stonks/** admin: - changed-files: @@ -29,3 +32,4 @@ admin: documentation: - changed-files: - any-glob-to-any-file: README.md + diff --git a/README.md b/README.md index e5c6736..5d023ef 100644 --- a/README.md +++ b/README.md @@ -143,7 +143,10 @@ libs/baarden/ jan@example.com korneel@example.com pier@example.com 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) diff --git a/dist/codeowners/generate.js b/dist/codeowners/generate.js index 454117b..19a7742 100644 --- a/dist/codeowners/generate.js +++ b/dist/codeowners/generate.js @@ -38,6 +38,23 @@ function generateLine(pCSTLine, pTeamMap) { (pCSTLine.inlineComment ? ` #${pCSTLine.inlineComment}` : "") ); } + if (pCSTLine.type === "section") { + const lUserNames = uniq( + pCSTLine.users.flatMap((pUser) => expandTeamToUserNames(pUser, pTeamMap)), + ) + .sort(compareUserNames) + .join(" "); + return ( + (pCSTLine.optional ? "^" : "") + + "[" + + pCSTLine.sectionName + + "]" + + (pCSTLine.minApprovers ? `[(${pCSTLine.minApprovers})]` : "") + + pCSTLine.spaces + + lUserNames + + (pCSTLine.inlineComment ? ` #${pCSTLine.inlineComment}` : "") + ); + } return pCSTLine.raw; } function expandTeamToUserNames(pUser, pTeamMap) { diff --git a/dist/virtual-code-owners/parse.js b/dist/virtual-code-owners/parse.js index 302e7d1..40b6433 100644 --- a/dist/virtual-code-owners/parse.js +++ b/dist/virtual-code-owners/parse.js @@ -19,6 +19,9 @@ function parseLine(pUntreatedLine, pTeamMap, pLineNo) { if (lTrimmedLine.startsWith("#")) { return { type: "comment", line: pLineNo, raw: pUntreatedLine }; } + if (lTrimmedLine.startsWith("[") || lTrimmedLine.startsWith("^[")) { + return parseSection(pUntreatedLine, pLineNo, pTeamMap); + } if (!lRule?.groups) { if (lTrimmedLine === "") { return { type: "empty", line: pLineNo, raw: pUntreatedLine }; @@ -35,6 +38,39 @@ function parseLine(pUntreatedLine, pTeamMap, pLineNo) { raw: pUntreatedLine, }; } +function parseSection(pUntreatedLine, pLineNo, pTeamMap) { + const lTrimmedLine = pUntreatedLine.trim(); + const lCommentSplitLine = lTrimmedLine.split(/\s*#/); + const lSection = lCommentSplitLine[0]?.match( + /^(?\^)?\[(?[^\]]+)\](\[(?[0-9]+)\])?(?\s+)(?.*)$/, + ); + if (!lSection?.groups) { + return lTrimmedLine.endsWith("]") + ? { + type: "section-without-users", + line: pLineNo, + raw: pUntreatedLine, + } + : { + type: "unknown", + line: pLineNo, + raw: pUntreatedLine, + }; + } + return { + type: "section", + line: pLineNo, + optional: lSection.groups.optionalIndicator === "^", + sectionName: lSection.groups.sectionName, + minApprovers: lSection.groups.minApprovers + ? parseInt(lSection.groups.minApprovers, 10) + : 1, + spaces: lSection.groups.spaces, + users: parseUsers(lSection.groups.userNames, pTeamMap), + inlineComment: lTrimmedLine.split(/\s*#/)[1] ?? "", + raw: pUntreatedLine, + }; +} function parseUsers(pUserNamesString, pTeamMap) { const lUserNames = pUserNamesString.split(/\s+/); return lUserNames.map((pUserName, pIndex) => { diff --git a/dist/virtual-code-owners/read.js b/dist/virtual-code-owners/read.js index cc185ca..8611926 100644 --- a/dist/virtual-code-owners/read.js +++ b/dist/virtual-code-owners/read.js @@ -25,7 +25,7 @@ function reportAnomalies(pFileName, pAnomalies) { return pAnomalies .map((pAnomaly) => { if (pAnomaly.type === "invalid-line") { - return `${pFileName}:${pAnomaly.line}:1 invalid line - neither a rule, comment nor empty: "${pAnomaly.raw}"`; + return `${pFileName}:${pAnomaly.line}:1 invalid line - neither a rule, section, comment nor empty: "${pAnomaly.raw}"`; } else { return ( `${pFileName}:${pAnomaly.line}:1 invalid user or team name "${pAnomaly.raw}" (#${pAnomaly.userNumberWithinLine} on this line). ` + diff --git a/package.json b/package.json index 074dd38..4f10ee4 100644 --- a/package.json +++ b/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", diff --git a/src/codeowners/generate.test.ts b/src/codeowners/generate.test.ts index 5aac1fb..80d2e75 100644 --- a/src/codeowners/generate.test.ts +++ b/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 = { diff --git a/src/codeowners/generate.ts b/src/codeowners/generate.ts index 27cdc4c..cd470eb 100644 --- a/src/codeowners/generate.ts +++ b/src/codeowners/generate.ts @@ -50,6 +50,23 @@ function generateLine( (pCSTLine.inlineComment ? ` #${pCSTLine.inlineComment}` : "") ); } + if (pCSTLine.type === "section") { + const lUserNames = uniq( + pCSTLine.users.flatMap((pUser) => expandTeamToUserNames(pUser, pTeamMap)), + ) + .sort(compareUserNames) + .join(" "); + return ( + (pCSTLine.optional ? "^" : "") + + "[" + + pCSTLine.sectionName + + "]" + + (pCSTLine.minApprovers ? `[${pCSTLine.minApprovers}]` : "") + + pCSTLine.spaces + + lUserNames + + (pCSTLine.inlineComment ? ` #${pCSTLine.inlineComment}` : "") + ); + } return pCSTLine.raw; } diff --git a/src/labeler-yml/generate.ts b/src/labeler-yml/generate.ts index 9fbed17..317a89e 100644 --- a/src/labeler-yml/generate.ts +++ b/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, diff --git a/src/team-map/read.ts b/src/team-map/read.ts index fff6d6a..cb54bac 100644 --- a/src/team-map/read.ts +++ b/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`, ); diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/015-section-required.txt b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/015-section-required.txt new file mode 100644 index 0000000..e9153da --- /dev/null +++ b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/015-section-required.txt @@ -0,0 +1,2 @@ +[required-section] +aap/ @some-user @some-other-user \ No newline at end of file diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/015-section-required.yml b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/015-section-required.yml new file mode 100644 index 0000000..5215bc7 --- /dev/null +++ b/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, + }, + ], + }, +] diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/021-section-required-min-approvers.txt b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/021-section-required-min-approvers.txt new file mode 100644 index 0000000..42f02ad --- /dev/null +++ b/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 \ No newline at end of file diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/021-section-required-min-approvers.yml b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/021-section-required-min-approvers.yml new file mode 100644 index 0000000..6f3c26d --- /dev/null +++ b/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, + }, + ], + }, +] diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/022-section-optional.txt b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/022-section-optional.txt new file mode 100644 index 0000000..72d2003 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/022-section-optional.yml b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/022-section-optional.yml new file mode 100644 index 0000000..57285b0 --- /dev/null +++ b/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", + 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, + }, + ], + }, +] diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/023-section-required-with-default-owners.txt b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/023-section-required-with-default-owners.txt new file mode 100644 index 0000000..299ffde --- /dev/null +++ b/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 \ No newline at end of file diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/023-section-required-with-default-owners.yml b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/023-section-required-with-default-owners.yml new file mode 100644 index 0000000..e70956e --- /dev/null +++ b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/023-section-required-with-default-owners.yml @@ -0,0 +1,49 @@ +[ + { + inlineComment: "", + line: 1, + optional: false, + raw: "[required-section] @default-user @default-group", + sectionName: "required-section", + spaces: " ", + type: "section", + users: + [ + { + bareName: "default-user", + raw: "@default-user", + type: "other-user-or-team", + userNumberWithinLine: 1, + }, + { + bareName: "default-group", + raw: "@default-group", + type: "other-user-or-team", + userNumberWithinLine: 2, + }, + ], + }, + { + 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, + }, + ], + }, +] diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/024-section-required-with-default-owners-min-approvers.txt b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/024-section-required-with-default-owners-min-approvers.txt new file mode 100644 index 0000000..4429d0c --- /dev/null +++ b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/024-section-required-with-default-owners-min-approvers.txt @@ -0,0 +1,2 @@ +[required-section][42] @default-user @default-group +aap/ @some-user @some-other-user \ No newline at end of file diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/024-section-required-with-default-owners-min-approvers.yml b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/024-section-required-with-default-owners-min-approvers.yml new file mode 100644 index 0000000..c64add4 --- /dev/null +++ b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/024-section-required-with-default-owners-min-approvers.yml @@ -0,0 +1,50 @@ +[ + { + inlineComment: "", + line: 1, + minApprovers: 42, + optional: false, + raw: "[required-section][42] @default-user @default-group", + sectionName: "required-section", + spaces: " ", + type: "section", + users: + [ + { + bareName: "default-user", + raw: "@default-user", + type: "other-user-or-team", + userNumberWithinLine: 1, + }, + { + bareName: "default-group", + raw: "@default-group", + type: "other-user-or-team", + userNumberWithinLine: 2, + }, + ], + }, + { + 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, + }, + ], + }, +] diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/025-section-without-end.txt b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/025-section-without-end.txt new file mode 100644 index 0000000..9b4a929 --- /dev/null +++ b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/025-section-without-end.txt @@ -0,0 +1,2 @@ +[required-section bladiebla +aap/ @some-user \ No newline at end of file diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/025-section-without-end.yml b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/025-section-without-end.yml new file mode 100644 index 0000000..fe1ab57 --- /dev/null +++ b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/025-section-without-end.yml @@ -0,0 +1,20 @@ +[ + { line: 1, raw: "[required-section bladiebla", type: "unknown" }, + { + filesPattern: "aap/", + inlineComment: "", + line: 2, + raw: "aap/ @some-user", + spaces: " ", + type: "rule", + users: + [ + { + bareName: "some-user", + raw: "@some-user", + type: "other-user-or-team", + userNumberWithinLine: 1, + }, + ], + }, +] diff --git a/src/virtual-code-owners/__fixtures__/corpus/teams/020-rule-virtual-team.txt b/src/virtual-code-owners/__fixtures__/corpus/teams/040-rule-virtual-team.txt similarity index 100% rename from src/virtual-code-owners/__fixtures__/corpus/teams/020-rule-virtual-team.txt rename to src/virtual-code-owners/__fixtures__/corpus/teams/040-rule-virtual-team.txt diff --git a/src/virtual-code-owners/__fixtures__/corpus/teams/020-rule-virtual-team.yml b/src/virtual-code-owners/__fixtures__/corpus/teams/040-rule-virtual-team.yml similarity index 100% rename from src/virtual-code-owners/__fixtures__/corpus/teams/020-rule-virtual-team.yml rename to src/virtual-code-owners/__fixtures__/corpus/teams/040-rule-virtual-team.yml diff --git a/src/virtual-code-owners/__fixtures__/corpus/teams/021-rule-multiple-teams.txt b/src/virtual-code-owners/__fixtures__/corpus/teams/041-rule-multiple-teams.txt similarity index 100% rename from src/virtual-code-owners/__fixtures__/corpus/teams/021-rule-multiple-teams.txt rename to src/virtual-code-owners/__fixtures__/corpus/teams/041-rule-multiple-teams.txt diff --git a/src/virtual-code-owners/__fixtures__/corpus/teams/021-rule-multiple-teams.yml b/src/virtual-code-owners/__fixtures__/corpus/teams/041-rule-multiple-teams.yml similarity index 100% rename from src/virtual-code-owners/__fixtures__/corpus/teams/021-rule-multiple-teams.yml rename to src/virtual-code-owners/__fixtures__/corpus/teams/041-rule-multiple-teams.yml diff --git a/src/virtual-code-owners/__fixtures__/corpus/teams/030-the-kitchen-sink.txt b/src/virtual-code-owners/__fixtures__/corpus/teams/050-the-kitchen-sink.txt similarity index 100% rename from src/virtual-code-owners/__fixtures__/corpus/teams/030-the-kitchen-sink.txt rename to src/virtual-code-owners/__fixtures__/corpus/teams/050-the-kitchen-sink.txt diff --git a/src/virtual-code-owners/__fixtures__/corpus/teams/030-the-kitchen-sink.yml b/src/virtual-code-owners/__fixtures__/corpus/teams/050-the-kitchen-sink.yml similarity index 100% rename from src/virtual-code-owners/__fixtures__/corpus/teams/030-the-kitchen-sink.yml rename to src/virtual-code-owners/__fixtures__/corpus/teams/050-the-kitchen-sink.yml diff --git a/src/virtual-code-owners/__fixtures__/corpus/teams/051-the-kitchen-sink-with-sections.txt b/src/virtual-code-owners/__fixtures__/corpus/teams/051-the-kitchen-sink-with-sections.txt new file mode 100644 index 0000000..fa3e7f8 --- /dev/null +++ b/src/virtual-code-owners/__fixtures__/corpus/teams/051-the-kitchen-sink-with-sections.txt @@ -0,0 +1,29 @@ +#! this is not the CODEOWNERS file - to get that one run +#! npx virtual-code-owners +#! +# catch-all to ensure there at least _is_ a code owner, even when +# it's _everyone_ + +* @baarden + +[admin & ci stuff] +.github/ @ch/transversal + +[generic stuff][3] @baarden +apps/framework/ @leren-lezen framework@example.com +apps/ux-portal/ @ch/ux @ch/transversal +libs/components/ @ch/ux + +[specific functionality] +libs/ubc-sales/ @ch/sales +libs/ubc-after-sales/ @ch/after-sales +libs/ubc-pre-sales/ @ch/pre-sales +libs/ubc-refund/ @ch/sales @ch/after-sales # shared responsibility +libs/ubc-baarden/ @baarden + +^[non critical stuff] @baarden +libs/ubc-foo/ @baardlozen +#! +#! The VIRTUAL-CODEOWNERS.txt file has the .txt extension because untamed vscode +#! installations might otherwise interpret it as markdown and auto-correct +#! and auto-format it as such diff --git a/src/virtual-code-owners/__fixtures__/corpus/teams/051-the-kitchen-sink-with-sections.yml b/src/virtual-code-owners/__fixtures__/corpus/teams/051-the-kitchen-sink-with-sections.yml new file mode 100644 index 0000000..90dfc91 --- /dev/null +++ b/src/virtual-code-owners/__fixtures__/corpus/teams/051-the-kitchen-sink-with-sections.yml @@ -0,0 +1,289 @@ +[ + { + "type": "ignorable-comment", + "line": 1, + "raw": "#! this is not the CODEOWNERS file - to get that one run", + }, + { + "type": "ignorable-comment", + "line": 2, + "raw": "#! npx virtual-code-owners", + }, + { "type": "ignorable-comment", "line": 3, "raw": "#!" }, + { + "type": "comment", + "line": 4, + "raw": "# catch-all to ensure there at least _is_ a code owner, even when", + }, + { "type": "comment", "line": 5, "raw": "# it's _everyone_" }, + { "type": "empty", "line": 6, "raw": "" }, + { + "type": "rule", + "line": 7, + "filesPattern": "*", + "spaces": " ", + "users": + [ + { + "type": "virtual-team-name", + "userNumberWithinLine": 1, + "bareName": "baarden", + "raw": "@baarden", + }, + ], + "inlineComment": "", + "raw": "* @baarden", + }, + { "type": "empty", "line": 8, "raw": "" }, + { "type": "section-without-users", "line": 9, "raw": "[admin & ci stuff]" }, + { + "type": "rule", + "line": 10, + "filesPattern": ".github/", + "spaces": " ", + "users": + [ + { + "type": "other-user-or-team", + "userNumberWithinLine": 1, + "bareName": "ch/transversal", + "raw": "@ch/transversal", + }, + ], + "inlineComment": "", + "raw": ".github/ @ch/transversal", + }, + { "type": "empty", "line": 11, "raw": "" }, + { + "type": "section", + "line": 12, + "optional": false, + "sectionName": "generic stuff", + "minApprovers": 3, + "spaces": " ", + "users": + [ + { + "type": "virtual-team-name", + "userNumberWithinLine": 1, + "bareName": "baarden", + "raw": "@baarden", + }, + ], + "inlineComment": "", + "raw": "[generic stuff][3] @baarden", + }, + { + "type": "rule", + "line": 13, + "filesPattern": "apps/framework/", + "spaces": " ", + "users": + [ + { + "type": "virtual-team-name", + "userNumberWithinLine": 1, + "bareName": "leren-lezen", + "raw": "@leren-lezen", + }, + { + "type": "e-mail-address", + "userNumberWithinLine": 2, + "bareName": "framework@example.com", + "raw": "framework@example.com", + }, + ], + "inlineComment": "", + "raw": "apps/framework/ @leren-lezen framework@example.com", + }, + { + "type": "rule", + "line": 14, + "filesPattern": "apps/ux-portal/", + "spaces": " ", + "users": + [ + { + "type": "other-user-or-team", + "userNumberWithinLine": 1, + "bareName": "ch/ux", + "raw": "@ch/ux", + }, + { + "type": "other-user-or-team", + "userNumberWithinLine": 2, + "bareName": "ch/transversal", + "raw": "@ch/transversal", + }, + ], + "inlineComment": "", + "raw": "apps/ux-portal/ @ch/ux @ch/transversal", + }, + { + "type": "rule", + "line": 15, + "filesPattern": "libs/components/", + "spaces": " ", + "users": + [ + { + "type": "other-user-or-team", + "userNumberWithinLine": 1, + "bareName": "ch/ux", + "raw": "@ch/ux", + }, + ], + "inlineComment": "", + "raw": "libs/components/ @ch/ux", + }, + { "type": "empty", "line": 16, "raw": "" }, + { + "type": "section-without-users", + "line": 17, + "raw": "[specific functionality]", + }, + { + "type": "rule", + "line": 18, + "filesPattern": "libs/ubc-sales/", + "spaces": " ", + "users": + [ + { + "type": "other-user-or-team", + "userNumberWithinLine": 1, + "bareName": "ch/sales", + "raw": "@ch/sales", + }, + ], + "inlineComment": "", + "raw": "libs/ubc-sales/ @ch/sales", + }, + { + "type": "rule", + "line": 19, + "filesPattern": "libs/ubc-after-sales/", + "spaces": " ", + "users": + [ + { + "type": "other-user-or-team", + "userNumberWithinLine": 1, + "bareName": "ch/after-sales", + "raw": "@ch/after-sales", + }, + ], + "inlineComment": "", + "raw": "libs/ubc-after-sales/ @ch/after-sales", + }, + { + "type": "rule", + "line": 20, + "filesPattern": "libs/ubc-pre-sales/", + "spaces": " ", + "users": + [ + { + "type": "other-user-or-team", + "userNumberWithinLine": 1, + "bareName": "ch/pre-sales", + "raw": "@ch/pre-sales", + }, + ], + "inlineComment": "", + "raw": "libs/ubc-pre-sales/ @ch/pre-sales", + }, + { + "type": "rule", + "line": 21, + "filesPattern": "libs/ubc-refund/", + "spaces": " ", + "users": + [ + { + "type": "other-user-or-team", + "userNumberWithinLine": 1, + "bareName": "ch/sales", + "raw": "@ch/sales", + }, + { + "type": "other-user-or-team", + "userNumberWithinLine": 2, + "bareName": "ch/after-sales", + "raw": "@ch/after-sales", + }, + ], + "inlineComment": " shared responsibility", + "raw": "libs/ubc-refund/ @ch/sales @ch/after-sales # shared responsibility", + }, + { + "type": "rule", + "line": 22, + "filesPattern": "libs/ubc-baarden/", + "spaces": " ", + "users": + [ + { + "type": "virtual-team-name", + "userNumberWithinLine": 1, + "bareName": "baarden", + "raw": "@baarden", + }, + ], + "inlineComment": "", + "raw": "libs/ubc-baarden/ @baarden", + }, + { "type": "empty", "line": 23, "raw": "" }, + { + "type": "section", + "line": 24, + "optional": true, + "sectionName": "non critical stuff", + "spaces": " ", + "users": + [ + { + "type": "virtual-team-name", + "userNumberWithinLine": 1, + "bareName": "baarden", + "raw": "@baarden", + }, + ], + "inlineComment": "", + "raw": "^[non critical stuff] @baarden", + }, + { + "type": "rule", + "line": 25, + "filesPattern": "libs/ubc-foo/", + "spaces": " ", + "users": + [ + { + "type": "other-user-or-team", + "userNumberWithinLine": 1, + "bareName": "baardlozen", + "raw": "@baardlozen", + }, + ], + "inlineComment": "", + "raw": "libs/ubc-foo/ @baardlozen", + }, + { "type": "ignorable-comment", "line": 26, "raw": "#!" }, + { + "type": "ignorable-comment", + "line": 27, + "raw": "#! The VIRTUAL-CODEOWNERS.txt file has the .txt extension because untamed vscode", + }, + { + "type": "ignorable-comment", + "line": 28, + "raw": "#! installations might otherwise interpret it as markdown and auto-correct", + }, + { + "type": "ignorable-comment", + "line": 29, + "raw": "#! and auto-format it as such", + }, + { "type": "empty", "line": 30, "raw": "" }, +] diff --git a/src/virtual-code-owners/cst.d.ts b/src/virtual-code-owners/cst.d.ts index e4abb2a..a7551a3 100644 --- a/src/virtual-code-owners/cst.d.ts +++ b/src/virtual-code-owners/cst.d.ts @@ -1,11 +1,17 @@ export type IVirtualCodeOwnersCST = IVirtualCodeOwnerLine[]; export type IVirtualCodeOwnerLine = IBoringCSTLine | IInterestingCSTLine; +export type IInterestingCSTLine = IRuleCSTLine | ISectionCSTLine; export interface IBoringCSTLine { - type: "comment" | "ignorable-comment" | "empty" | "unknown"; + type: + | "comment" + | "ignorable-comment" + | "empty" + | "section-without-users" + | "unknown"; line: number; raw: string; } -export interface IInterestingCSTLine { +export interface IRuleCSTLine { type: "rule"; line: number; filesPattern: string; @@ -14,6 +20,18 @@ export interface IInterestingCSTLine { inlineComment: string; raw: string; } +export interface ISectionCSTLine { + type: "section"; + line: number; + optional: boolean; + sectionName: string; + minApprovers?: number; + spaces: string; + users: IUser[]; + inlineComment: string; + raw: string; +} + export type UserType = | "virtual-team-name" | "e-mail-address" diff --git a/src/virtual-code-owners/parse.test.ts b/src/virtual-code-owners/parse.test.ts index be9199b..eb027bf 100644 --- a/src/virtual-code-owners/parse.test.ts +++ b/src/virtual-code-owners/parse.test.ts @@ -51,6 +51,7 @@ describe("parses VIRTUAL-CODEOWNERS.txt - with 'virtual teams'", () => { rel(getOutputFileName(pFileName)), "utf-8", ); + // console.log(JSON.stringify(parse(lInput, TEAMS), null, 2)); it(`parses ${pFileName}`, () => { deepEqual(parse(lInput, TEAMS), parseYaml(lExpected)); }); diff --git a/src/virtual-code-owners/parse.ts b/src/virtual-code-owners/parse.ts index f96e0c2..55d19f6 100644 --- a/src/virtual-code-owners/parse.ts +++ b/src/virtual-code-owners/parse.ts @@ -1,6 +1,7 @@ import { EOL } from "node:os"; import type { ITeamMap } from "../team-map/team-map.js"; import type { + ISectionCSTLine, IUser, IVirtualCodeOwnerLine, IVirtualCodeOwnersCST, @@ -36,6 +37,9 @@ function parseLine( if (lTrimmedLine.startsWith("#")) { return { type: "comment", line: pLineNo, raw: pUntreatedLine }; } + if (lTrimmedLine.startsWith("[") || lTrimmedLine.startsWith("^[")) { + return parseSection(pUntreatedLine, pLineNo, pTeamMap); + } if (!lRule?.groups) { if (lTrimmedLine === "") { return { type: "empty", line: pLineNo, raw: pUntreatedLine }; @@ -54,6 +58,49 @@ function parseLine( }; } +function parseSection( + pUntreatedLine: string, + pLineNo: number, + pTeamMap: ITeamMap, +): IVirtualCodeOwnerLine { + const lTrimmedLine = pUntreatedLine.trim(); + const lCommentSplitLine = lTrimmedLine.split(/\s*#/); + const lSection = lCommentSplitLine[0]?.match( + /^(?\^)?\[(?[^\]]+)\](\[(?[0-9]+)\])?(?\s+)(?.*)$/, + ); + if (!lSection?.groups) { + return lTrimmedLine.endsWith("]") + ? { + type: "section-without-users", + line: pLineNo, + raw: pUntreatedLine, + } + : { + type: "unknown", + line: pLineNo, + raw: pUntreatedLine, + }; + } + const lReturnValue: ISectionCSTLine = { + type: "section", + line: pLineNo, + optional: lSection.groups.optionalIndicator === "^", + sectionName: lSection.groups.sectionName as string, + spaces: lSection.groups.spaces as string, + users: parseUsers(lSection.groups.userNames as string, pTeamMap), + inlineComment: lTrimmedLine.split(/\s*#/)[1] ?? "", + raw: pUntreatedLine, + }; + + if (lSection.groups.minApprovers) { + lReturnValue.minApprovers = parseInt( + lSection.groups.minApprovers as string, + ); + } + + return lReturnValue; +} + function parseUsers(pUserNamesString: string, pTeamMap: ITeamMap): IUser[] { const lUserNames = pUserNamesString.split(/\s+/); return lUserNames.map((pUserName, pIndex) => { diff --git a/src/virtual-code-owners/read.ts b/src/virtual-code-owners/read.ts index c2e8791..574066b 100644 --- a/src/virtual-code-owners/read.ts +++ b/src/virtual-code-owners/read.ts @@ -32,7 +32,7 @@ function reportAnomalies(pFileName: string, pAnomalies: IAnomaly[]): string { return pAnomalies .map((pAnomaly) => { if (pAnomaly.type === "invalid-line") { - return `${pFileName}:${pAnomaly.line}:1 invalid line - neither a rule, comment nor empty: "${pAnomaly.raw}"`; + return `${pFileName}:${pAnomaly.line}:1 invalid line - neither a rule, section, comment nor empty: "${pAnomaly.raw}"`; } else { return ( `${pFileName}:${pAnomaly.line}:1 invalid user or team name "${pAnomaly.raw}" (#${pAnomaly.userNumberWithinLine} on this line). ` + From f1a1598a62a292dd7219959c89b9980ddd1a52d7 Mon Sep 17 00:00:00 2001 From: sverweij Date: Sat, 11 May 2024 20:16:45 +0200 Subject: [PATCH 2/5] fix: clean up own codeowners file --- .github/CODEOWNERS | 13 ------------- .github/VIRTUAL-CODEOWNERS.txt | 13 ------------- .github/labeler.yml | 3 --- dist/codeowners/generate.js | 2 +- dist/virtual-code-owners/parse.js | 9 +++++---- 5 files changed, 6 insertions(+), 34 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 8cd13c5..4885cdb 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -16,16 +16,3 @@ .github/ @mcmeadow @sverweij tools/ @mcmeadow @sverweij README.md @sverweij - -[section] -bla @sverweij - -[section-with-owners][(1)] @mcmeadow @sverweij -# blabla # Patronen ohne Nutzernahmen sind erlaubt in Sectionen mit Nutzernamen im Titel -bladiebla @other-owner-still - -[section-with-approvers][3] -# Diese Section ist leer. Is das erlaubt? - -^[optional-section] -stonks/ @mcmeadow @sverweij diff --git a/.github/VIRTUAL-CODEOWNERS.txt b/.github/VIRTUAL-CODEOWNERS.txt index 0825bd1..dcd2dcf 100644 --- a/.github/VIRTUAL-CODEOWNERS.txt +++ b/.github/VIRTUAL-CODEOWNERS.txt @@ -8,16 +8,3 @@ .github/ @vco-admins @admin tools/ @vco-admins @tools README.md @vco @documentation - -[section] -bla @vco - -[section-with-owners] @vco @vco-admins -# blabla # Patronen ohne Nutzernahmen sind erlaubt in Sectionen mit Nutzernamen im Titel -bladiebla @other-owner-still - -[section-with-approvers][3] -# Diese Section ist leer. Is das erlaubt? - -^[optional-section] -stonks/ @vco-admins @tools diff --git a/.github/labeler.yml b/.github/labeler.yml index 1b5a1bc..cf0e584 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -12,18 +12,15 @@ vco: - changed-files: - any-glob-to-any-file: "**" - any-glob-to-any-file: README.md - - any-glob-to-any-file: bla vco-admins: - changed-files: - any-glob-to-any-file: .github/** - any-glob-to-any-file: tools/** - - any-glob-to-any-file: stonks/** tools: - changed-files: - any-glob-to-any-file: tools/** - - any-glob-to-any-file: stonks/** admin: - changed-files: diff --git a/dist/codeowners/generate.js b/dist/codeowners/generate.js index 19a7742..779b191 100644 --- a/dist/codeowners/generate.js +++ b/dist/codeowners/generate.js @@ -49,7 +49,7 @@ function generateLine(pCSTLine, pTeamMap) { "[" + pCSTLine.sectionName + "]" + - (pCSTLine.minApprovers ? `[(${pCSTLine.minApprovers})]` : "") + + (pCSTLine.minApprovers ? `[${pCSTLine.minApprovers}]` : "") + pCSTLine.spaces + lUserNames + (pCSTLine.inlineComment ? ` #${pCSTLine.inlineComment}` : "") diff --git a/dist/virtual-code-owners/parse.js b/dist/virtual-code-owners/parse.js index 40b6433..cb52544 100644 --- a/dist/virtual-code-owners/parse.js +++ b/dist/virtual-code-owners/parse.js @@ -57,19 +57,20 @@ function parseSection(pUntreatedLine, pLineNo, pTeamMap) { raw: pUntreatedLine, }; } - return { + const lReturnValue = { type: "section", line: pLineNo, optional: lSection.groups.optionalIndicator === "^", sectionName: lSection.groups.sectionName, - minApprovers: lSection.groups.minApprovers - ? parseInt(lSection.groups.minApprovers, 10) - : 1, spaces: lSection.groups.spaces, users: parseUsers(lSection.groups.userNames, pTeamMap), inlineComment: lTrimmedLine.split(/\s*#/)[1] ?? "", raw: pUntreatedLine, }; + if (lSection.groups.minApprovers) { + lReturnValue.minApprovers = parseInt(lSection.groups.minApprovers); + } + return lReturnValue; } function parseUsers(pUserNamesString, pTeamMap) { const lUserNames = pUserNamesString.split(/\s+/); From 07eb6c4860aaf49bd51e7867d71c1d65873ebbc5 Mon Sep 17 00:00:00 2001 From: Sander Verweij Date: Sat, 11 May 2024 20:35:43 +0200 Subject: [PATCH 3/5] fix: apply suggestions from code review @McMeadow Co-authored-by: Lex McMeadow --- dist/virtual-code-owners/parse.js | 2 +- dist/virtual-code-owners/read.js | 2 +- src/virtual-code-owners/parse.ts | 1 + src/virtual-code-owners/read.ts | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dist/virtual-code-owners/parse.js b/dist/virtual-code-owners/parse.js index cb52544..2c7bda9 100644 --- a/dist/virtual-code-owners/parse.js +++ b/dist/virtual-code-owners/parse.js @@ -68,7 +68,7 @@ function parseSection(pUntreatedLine, pLineNo, pTeamMap) { raw: pUntreatedLine, }; if (lSection.groups.minApprovers) { - lReturnValue.minApprovers = parseInt(lSection.groups.minApprovers); + lReturnValue.minApprovers = parseInt(lSection.groups.minApprovers, 10); } return lReturnValue; } diff --git a/dist/virtual-code-owners/read.js b/dist/virtual-code-owners/read.js index 8611926..1ed908f 100644 --- a/dist/virtual-code-owners/read.js +++ b/dist/virtual-code-owners/read.js @@ -25,7 +25,7 @@ function reportAnomalies(pFileName, pAnomalies) { return pAnomalies .map((pAnomaly) => { if (pAnomaly.type === "invalid-line") { - return `${pFileName}:${pAnomaly.line}:1 invalid line - neither a rule, section, comment nor empty: "${pAnomaly.raw}"`; + return `${pFileName}:${pAnomaly.line}:1 invalid line - neither a rule, section head, comment nor empty: "${pAnomaly.raw}"`; } else { return ( `${pFileName}:${pAnomaly.line}:1 invalid user or team name "${pAnomaly.raw}" (#${pAnomaly.userNumberWithinLine} on this line). ` + diff --git a/src/virtual-code-owners/parse.ts b/src/virtual-code-owners/parse.ts index 55d19f6..8cd4f5c 100644 --- a/src/virtual-code-owners/parse.ts +++ b/src/virtual-code-owners/parse.ts @@ -95,6 +95,7 @@ function parseSection( if (lSection.groups.minApprovers) { lReturnValue.minApprovers = parseInt( lSection.groups.minApprovers as string, + 10 ); } diff --git a/src/virtual-code-owners/read.ts b/src/virtual-code-owners/read.ts index 574066b..50355e4 100644 --- a/src/virtual-code-owners/read.ts +++ b/src/virtual-code-owners/read.ts @@ -32,7 +32,7 @@ function reportAnomalies(pFileName: string, pAnomalies: IAnomaly[]): string { return pAnomalies .map((pAnomaly) => { if (pAnomaly.type === "invalid-line") { - return `${pFileName}:${pAnomaly.line}:1 invalid line - neither a rule, section, comment nor empty: "${pAnomaly.raw}"`; + return `${pFileName}:${pAnomaly.line}:1 invalid line - neither a rule, section head, comment nor empty: "${pAnomaly.raw}"`; } else { return ( `${pFileName}:${pAnomaly.line}:1 invalid user or team name "${pAnomaly.raw}" (#${pAnomaly.userNumberWithinLine} on this line). ` + From b89fc0457ad7fd41d49d2d2c76f4c98312305b73 Mon Sep 17 00:00:00 2001 From: sverweij Date: Sat, 11 May 2024 20:55:29 +0200 Subject: [PATCH 4/5] refactor(codeowners/generate): slightly de-duplicate --- .github/labeler.yml | 1 - .github/workflows/ci.yml | 2 +- dist/codeowners/generate.js | 19 +++++++------------ src/codeowners/generate.ts | 20 ++++++++------------ src/virtual-code-owners/parse.ts | 2 +- 5 files changed, 17 insertions(+), 27 deletions(-) diff --git a/.github/labeler.yml b/.github/labeler.yml index cf0e584..9c6463c 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -29,4 +29,3 @@ admin: documentation: - changed-files: - any-glob-to-any-file: README.md - diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c688821..c6d7444 100644 --- a/.github/workflows/ci.yml +++ b/.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: | diff --git a/dist/codeowners/generate.js b/dist/codeowners/generate.js index 779b191..7d5f7ae 100644 --- a/dist/codeowners/generate.js +++ b/dist/codeowners/generate.js @@ -26,24 +26,14 @@ export default function generateCodeOwners( } function generateLine(pCSTLine, pTeamMap) { 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") { - const lUserNames = uniq( - pCSTLine.users.flatMap((pUser) => expandTeamToUserNames(pUser, pTeamMap)), - ) - .sort(compareUserNames) - .join(" "); return ( (pCSTLine.optional ? "^" : "") + "[" + @@ -51,12 +41,17 @@ function generateLine(pCSTLine, pTeamMap) { "]" + (pCSTLine.minApprovers ? `[${pCSTLine.minApprovers}]` : "") + pCSTLine.spaces + - lUserNames + + expandTeamsToUsersString(pCSTLine.users, pTeamMap) + (pCSTLine.inlineComment ? ` #${pCSTLine.inlineComment}` : "") ); } return pCSTLine.raw; } +function expandTeamsToUsersString(pUsers, pTeamMap) { + return uniq(pUsers.flatMap((pUser) => expandTeamToUserNames(pUser, pTeamMap))) + .sort(compareUserNames) + .join(" "); +} function expandTeamToUserNames(pUser, pTeamMap) { if (pUser.type === "virtual-team-name") { return stringifyTeamMembers(pTeamMap, pUser.bareName); diff --git a/src/codeowners/generate.ts b/src/codeowners/generate.ts index cd470eb..bdb633f 100644 --- a/src/codeowners/generate.ts +++ b/src/codeowners/generate.ts @@ -38,24 +38,14 @@ 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") { - const lUserNames = uniq( - pCSTLine.users.flatMap((pUser) => expandTeamToUserNames(pUser, pTeamMap)), - ) - .sort(compareUserNames) - .join(" "); return ( (pCSTLine.optional ? "^" : "") + "[" + @@ -63,13 +53,19 @@ function generateLine( "]" + (pCSTLine.minApprovers ? `[${pCSTLine.minApprovers}]` : "") + pCSTLine.spaces + - lUserNames + + 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); diff --git a/src/virtual-code-owners/parse.ts b/src/virtual-code-owners/parse.ts index 8cd4f5c..a65f47e 100644 --- a/src/virtual-code-owners/parse.ts +++ b/src/virtual-code-owners/parse.ts @@ -95,7 +95,7 @@ function parseSection( if (lSection.groups.minApprovers) { lReturnValue.minApprovers = parseInt( lSection.groups.minApprovers as string, - 10 + 10, ); } From c225876c811fe68829423220f075daef73b23089 Mon Sep 17 00:00:00 2001 From: sverweij Date: Sat, 11 May 2024 21:11:37 +0200 Subject: [PATCH 5/5] fix: 'section' -> 'section-heading' --- dist/codeowners/generate.js | 2 +- dist/virtual-code-owners/parse.js | 2 +- dist/virtual-code-owners/read.js | 2 +- src/codeowners/generate.ts | 2 +- .../corpus/empty-teams/022-section-optional.yml | 2 +- .../023-section-required-with-default-owners.yml | 2 +- ...4-section-required-with-default-owners-min-approvers.yml | 2 +- .../corpus/teams/051-the-kitchen-sink-with-sections.yml | 4 ++-- src/virtual-code-owners/cst.d.ts | 6 +++--- src/virtual-code-owners/parse.ts | 6 +++--- src/virtual-code-owners/read.ts | 2 +- 11 files changed, 16 insertions(+), 16 deletions(-) diff --git a/dist/codeowners/generate.js b/dist/codeowners/generate.js index 7d5f7ae..21d12a0 100644 --- a/dist/codeowners/generate.js +++ b/dist/codeowners/generate.js @@ -33,7 +33,7 @@ function generateLine(pCSTLine, pTeamMap) { (pCSTLine.inlineComment ? ` #${pCSTLine.inlineComment}` : "") ); } - if (pCSTLine.type === "section") { + if (pCSTLine.type === "section-heading") { return ( (pCSTLine.optional ? "^" : "") + "[" + diff --git a/dist/virtual-code-owners/parse.js b/dist/virtual-code-owners/parse.js index 2c7bda9..aee63b1 100644 --- a/dist/virtual-code-owners/parse.js +++ b/dist/virtual-code-owners/parse.js @@ -58,7 +58,7 @@ function parseSection(pUntreatedLine, pLineNo, pTeamMap) { }; } const lReturnValue = { - type: "section", + type: "section-heading", line: pLineNo, optional: lSection.groups.optionalIndicator === "^", sectionName: lSection.groups.sectionName, diff --git a/dist/virtual-code-owners/read.js b/dist/virtual-code-owners/read.js index 1ed908f..ffc1e06 100644 --- a/dist/virtual-code-owners/read.js +++ b/dist/virtual-code-owners/read.js @@ -25,7 +25,7 @@ function reportAnomalies(pFileName, pAnomalies) { return pAnomalies .map((pAnomaly) => { if (pAnomaly.type === "invalid-line") { - return `${pFileName}:${pAnomaly.line}:1 invalid line - neither a rule, section head, comment nor empty: "${pAnomaly.raw}"`; + return `${pFileName}:${pAnomaly.line}:1 invalid line - neither a rule, section heading, comment nor empty: "${pAnomaly.raw}"`; } else { return ( `${pFileName}:${pAnomaly.line}:1 invalid user or team name "${pAnomaly.raw}" (#${pAnomaly.userNumberWithinLine} on this line). ` + diff --git a/src/codeowners/generate.ts b/src/codeowners/generate.ts index bdb633f..95fa74b 100644 --- a/src/codeowners/generate.ts +++ b/src/codeowners/generate.ts @@ -45,7 +45,7 @@ function generateLine( (pCSTLine.inlineComment ? ` #${pCSTLine.inlineComment}` : "") ); } - if (pCSTLine.type === "section") { + if (pCSTLine.type === "section-heading") { return ( (pCSTLine.optional ? "^" : "") + "[" + diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/022-section-optional.yml b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/022-section-optional.yml index 57285b0..4554529 100644 --- a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/022-section-optional.yml +++ b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/022-section-optional.yml @@ -31,7 +31,7 @@ raw: "^[optional-section-with-default] @koos-koets", sectionName: "optional-section-with-default", spaces: " ", - type: "section", + type: "section-heading", users: [ { diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/023-section-required-with-default-owners.yml b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/023-section-required-with-default-owners.yml index e70956e..9e5361a 100644 --- a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/023-section-required-with-default-owners.yml +++ b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/023-section-required-with-default-owners.yml @@ -6,7 +6,7 @@ raw: "[required-section] @default-user @default-group", sectionName: "required-section", spaces: " ", - type: "section", + type: "section-heading", users: [ { diff --git a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/024-section-required-with-default-owners-min-approvers.yml b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/024-section-required-with-default-owners-min-approvers.yml index c64add4..f31d356 100644 --- a/src/virtual-code-owners/__fixtures__/corpus/empty-teams/024-section-required-with-default-owners-min-approvers.yml +++ b/src/virtual-code-owners/__fixtures__/corpus/empty-teams/024-section-required-with-default-owners-min-approvers.yml @@ -7,7 +7,7 @@ raw: "[required-section][42] @default-user @default-group", sectionName: "required-section", spaces: " ", - type: "section", + type: "section-heading", users: [ { diff --git a/src/virtual-code-owners/__fixtures__/corpus/teams/051-the-kitchen-sink-with-sections.yml b/src/virtual-code-owners/__fixtures__/corpus/teams/051-the-kitchen-sink-with-sections.yml index 90dfc91..f67f137 100644 --- a/src/virtual-code-owners/__fixtures__/corpus/teams/051-the-kitchen-sink-with-sections.yml +++ b/src/virtual-code-owners/__fixtures__/corpus/teams/051-the-kitchen-sink-with-sections.yml @@ -55,7 +55,7 @@ }, { "type": "empty", "line": 11, "raw": "" }, { - "type": "section", + "type": "section-heading", "line": 12, "optional": false, "sectionName": "generic stuff", @@ -235,7 +235,7 @@ }, { "type": "empty", "line": 23, "raw": "" }, { - "type": "section", + "type": "section-heading", "line": 24, "optional": true, "sectionName": "non critical stuff", diff --git a/src/virtual-code-owners/cst.d.ts b/src/virtual-code-owners/cst.d.ts index a7551a3..5c2e24e 100644 --- a/src/virtual-code-owners/cst.d.ts +++ b/src/virtual-code-owners/cst.d.ts @@ -1,6 +1,6 @@ export type IVirtualCodeOwnersCST = IVirtualCodeOwnerLine[]; export type IVirtualCodeOwnerLine = IBoringCSTLine | IInterestingCSTLine; -export type IInterestingCSTLine = IRuleCSTLine | ISectionCSTLine; +export type IInterestingCSTLine = IRuleCSTLine | ISectionHeadingCSTLine; export interface IBoringCSTLine { type: | "comment" @@ -20,8 +20,8 @@ export interface IRuleCSTLine { inlineComment: string; raw: string; } -export interface ISectionCSTLine { - type: "section"; +export interface ISectionHeadingCSTLine { + type: "section-heading"; line: number; optional: boolean; sectionName: string; diff --git a/src/virtual-code-owners/parse.ts b/src/virtual-code-owners/parse.ts index a65f47e..b700073 100644 --- a/src/virtual-code-owners/parse.ts +++ b/src/virtual-code-owners/parse.ts @@ -1,7 +1,7 @@ import { EOL } from "node:os"; import type { ITeamMap } from "../team-map/team-map.js"; import type { - ISectionCSTLine, + ISectionHeadingCSTLine, IUser, IVirtualCodeOwnerLine, IVirtualCodeOwnersCST, @@ -81,8 +81,8 @@ function parseSection( raw: pUntreatedLine, }; } - const lReturnValue: ISectionCSTLine = { - type: "section", + const lReturnValue: ISectionHeadingCSTLine = { + type: "section-heading", line: pLineNo, optional: lSection.groups.optionalIndicator === "^", sectionName: lSection.groups.sectionName as string, diff --git a/src/virtual-code-owners/read.ts b/src/virtual-code-owners/read.ts index 50355e4..798dbc3 100644 --- a/src/virtual-code-owners/read.ts +++ b/src/virtual-code-owners/read.ts @@ -32,7 +32,7 @@ function reportAnomalies(pFileName: string, pAnomalies: IAnomaly[]): string { return pAnomalies .map((pAnomaly) => { if (pAnomaly.type === "invalid-line") { - return `${pFileName}:${pAnomaly.line}:1 invalid line - neither a rule, section head, comment nor empty: "${pAnomaly.raw}"`; + return `${pFileName}:${pAnomaly.line}:1 invalid line - neither a rule, section heading, comment nor empty: "${pAnomaly.raw}"`; } else { return ( `${pFileName}:${pAnomaly.line}:1 invalid user or team name "${pAnomaly.raw}" (#${pAnomaly.userNumberWithinLine} on this line). ` +