This commit is contained in:
Maxime Morille 2025-01-21 11:51:59 +01:00 committed by GitHub
commit ad1c7e5ef4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 216 additions and 21 deletions

View file

@ -0,0 +1,58 @@
import ignore from 'ignore';
import { regEx } from '../../../util/regex';
import type { FileOwnerRule } from '../types';
export class CodeOwnersParser {
private currentSection: { name: string; defaultUsers: string[] };
private internalRules: FileOwnerRule[];
constructor() {
this.currentSection = { name: '', defaultUsers: [] };
this.internalRules = [];
}
private changeCurrentSection(line: string): void {
const [name, ...usernames] = line.split(regEx(/\s+/));
this.currentSection = { name, defaultUsers: usernames };
}
private addRule(rule: FileOwnerRule): void {
this.internalRules.push(rule);
}
private extractOwnersFromLine(
line: string,
defaultUsernames: string[],
): FileOwnerRule {
const [pattern, ...usernames] = line.split(regEx(/\s+/));
const matchPattern = ignore().add(pattern);
return {
usernames: usernames.length > 0 ? usernames : defaultUsernames,
pattern,
score: pattern.length,
match: (path: string) => matchPattern.ignores(path),
};
}
parseLine(line: string): CodeOwnersParser {
if (CodeOwnersParser.isSectionHeader(line)) {
this.changeCurrentSection(line);
} else {
const rule = this.extractOwnersFromLine(
line,
this.currentSection.defaultUsers,
);
this.addRule(rule);
}
return this;
}
get rules(): FileOwnerRule[] {
return this.internalRules;
}
private static isSectionHeader(line: string): boolean {
return line.startsWith('[') || line.startsWith('^[');
}
}

View file

@ -25,7 +25,7 @@ import { setBaseUrl } from '../../../util/http/gitlab';
import type { HttpResponse } from '../../../util/http/types';
import { parseInteger } from '../../../util/number';
import * as p from '../../../util/promises';
import { regEx } from '../../../util/regex';
import { newlineRegex, regEx } from '../../../util/regex';
import { sanitize } from '../../../util/sanitize';
import {
ensureTrailingSlash,
@ -39,6 +39,7 @@ import type {
EnsureCommentConfig,
EnsureCommentRemovalConfig,
EnsureIssueConfig,
FileOwnerRule,
FindPRConfig,
GitUrlOption,
Issue,
@ -54,6 +55,7 @@ import type {
} from '../types';
import { repoFingerprint } from '../util';
import { smartTruncate } from '../utils/pr-body';
import { CodeOwnersParser } from './code-owners';
import {
getMemberUserIDs,
getMemberUsernames,
@ -1478,3 +1480,23 @@ export async function expandGroupMembers(
}
return expandedReviewersOrAssignees;
}
export function extractRulesFromCodeOwnersFile(
codeOwnersFile: string,
): FileOwnerRule[] {
const parser = new CodeOwnersParser();
const cleanedLines = codeOwnersFile
.split(newlineRegex)
// Remove comments
.map((line) => line.split('#')[0])
// Remove empty lines
.map((line) => line.trim())
.filter(is.nonEmptyString);
for (const line of cleanedLines) {
parser.parseLine(line);
}
return parser.rules;
}

View file

@ -221,6 +221,13 @@ export interface AutodiscoverConfig {
projects?: string[];
}
export interface FileOwnerRule {
usernames: string[];
pattern: string;
score: number;
match: (path: string) => boolean;
}
export interface Platform {
findIssue(title: string): Promise<Issue | null>;
getIssueList(): Promise<Issue[]>;
@ -280,6 +287,7 @@ export interface Platform {
filterUnavailableUsers?(users: string[]): Promise<string[]>;
commitFiles?(config: CommitFilesConfig): Promise<LongCommitSha | null>;
expandGroupMembers?(reviewersOrAssignees: string[]): Promise<string[]>;
extractRulesFromCodeOwnersFile?(fileContent: string): FileOwnerRule[];
maxBodyLength(): number;
labelCharLimit?(): number;

View file

@ -2,12 +2,19 @@ import { codeBlock } from 'common-tags';
import { mock } from 'jest-mock-extended';
import { fs, git } from '../../../../../test/util';
import type { Pr } from '../../../../modules/platform';
import { platform } from '../../../../modules/platform';
import * as gitlab from '../../../../modules/platform/gitlab';
import { codeOwnersForPr } from './code-owners';
jest.mock('../../../../util/fs');
jest.mock('../../../../util/git');
jest.mock('../../../../modules/platform');
describe('workers/repository/update/pr/code-owners', () => {
beforeAll(() => {
platform.extractRulesFromCodeOwnersFile = undefined;
});
describe('codeOwnersForPr', () => {
let pr: Pr;
@ -170,6 +177,102 @@ describe('workers/repository/update/pr/code-owners', () => {
});
});
describe('supports Gitlab sections', () => {
beforeAll(() => {
platform.extractRulesFromCodeOwnersFile =
gitlab.extractRulesFromCodeOwnersFile;
});
it('returns section code owner', async () => {
fs.readLocalFile.mockResolvedValueOnce(
['[team] @jimmy', '*'].join('\n'),
);
git.getBranchFiles.mockResolvedValueOnce(['README.md']);
const codeOwners = await codeOwnersForPr(pr);
expect(codeOwners).toEqual(['@jimmy']);
});
const codeOwnerFileWithDefaultApproval = codeBlock`
# Required for all files
* @general-approvers
[Documentation] @docs-team
docs/
README.md
*.txt
# Invalid section
Something before [Tests] @tests-team
tests/
# Optional section
^[Optional] @optional-team
optional/
[Database] @database-team
model/db/
config/db/database-setup.md @docs-team
`;
it('returns code owners of multiple sections', async () => {
fs.readLocalFile.mockResolvedValueOnce(
codeOwnerFileWithDefaultApproval,
);
git.getBranchFiles.mockResolvedValueOnce([
'config/db/database-setup.md',
]);
const codeOwners = await codeOwnersForPr(pr);
expect(codeOwners).toEqual(['@docs-team', '@general-approvers']);
});
it('returns default owners when none is explicitly set', async () => {
fs.readLocalFile.mockResolvedValueOnce(
codeOwnerFileWithDefaultApproval,
);
git.getBranchFiles.mockResolvedValueOnce(['model/db/CHANGELOG.txt']);
const codeOwners = await codeOwnersForPr(pr);
expect(codeOwners).toEqual([
'@database-team',
'@docs-team',
'@general-approvers',
]);
});
it('parses only sections that start at the beginning of a line', async () => {
fs.readLocalFile.mockResolvedValueOnce(
codeOwnerFileWithDefaultApproval,
);
git.getBranchFiles.mockResolvedValueOnce(['tests/setup.ts']);
const codeOwners = await codeOwnersForPr(pr);
expect(codeOwners).not.toInclude('@tests-team');
});
it('returns code owners for optional sections', async () => {
fs.readLocalFile.mockResolvedValueOnce(
codeOwnerFileWithDefaultApproval,
);
git.getBranchFiles.mockResolvedValueOnce([
'optional/optional-file.txt',
]);
const codeOwners = await codeOwnersForPr(pr);
expect(codeOwners).toEqual([
'@optional-team',
'@docs-team',
'@general-approvers',
]);
});
});
it('does not require all files to match a single rule, regression test for #12611', async () => {
fs.readLocalFile.mockResolvedValueOnce(
codeBlock`

View file

@ -1,16 +1,15 @@
import is from '@sindresorhus/is';
import ignore from 'ignore';
import { logger } from '../../../../logger';
import type { Pr } from '../../../../modules/platform';
import type { FileOwnerRule, Pr } from '../../../../modules/platform';
import { platform } from '../../../../modules/platform';
import { readLocalFile } from '../../../../util/fs';
import { getBranchFiles } from '../../../../util/git';
import { newlineRegex, regEx } from '../../../../util/regex';
interface FileOwnerRule {
usernames: string[];
pattern: string;
score: number;
match: (path: string) => boolean;
interface FileOwnersScore {
file: string;
userScoreMap: Map<string, number>;
}
function extractOwnersFromLine(line: string): FileOwnerRule {
@ -24,11 +23,6 @@ function extractOwnersFromLine(line: string): FileOwnerRule {
};
}
interface FileOwnersScore {
file: string;
userScoreMap: Map<string, number>;
}
function matchFileToOwners(
file: string,
rules: FileOwnerRule[],
@ -75,6 +69,22 @@ function getOwnerList(filesWithOwners: FileOwnersScore[]): OwnerFileScore[] {
}));
}
function extractRulesFromCodeOwnersFile(
codeOwnersFile: string,
): FileOwnerRule[] {
return (
codeOwnersFile
.split(newlineRegex)
// Remove comments
.map((line) => line.split('#')[0])
// Remove empty lines
.map((line) => line.trim())
.filter(is.nonEmptyString)
// Extract pattern & usernames
.map(extractOwnersFromLine)
);
}
export async function codeOwnersForPr(pr: Pr): Promise<string[]> {
logger.debug('Searching for CODEOWNERS file');
try {
@ -102,15 +112,9 @@ export async function codeOwnersForPr(pr: Pr): Promise<string[]> {
}
// Convert CODEOWNERS file into list of matching rules
const fileOwnerRules = codeOwnersFile
.split(newlineRegex)
// Remove comments
.map((line) => line.split('#')[0])
// Remove empty lines
.map((line) => line.trim())
.filter(is.nonEmptyString)
// Extract pattern & usernames
.map(extractOwnersFromLine);
const fileOwnerRules = platform.extractRulesFromCodeOwnersFile
? platform.extractRulesFromCodeOwnersFile(codeOwnersFile)
: extractRulesFromCodeOwnersFile(codeOwnersFile);
logger.debug(
{ prFiles, fileOwnerRules },