Skip to content

Require IDs to be unique (even with drafts) #3185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all 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
23 changes: 22 additions & 1 deletion index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ const draft = Symbol('draft');
// This must match the definition in docs/guidelines.md
const identifierPattern = /^[a-z][a-z0-9]*(-[a-z0-9]+)*$/;

// All identifiers (including drafts) must be unique with respect to their
// siblings. These maps track them with respect to file names, for clearer error
// mesesages.
const uniqueIdMaps = {
features: new Map<string, string>(),
groups: new Map<string, string>(),
snapshots: new Map<string, string>(),
}

function* yamlEntries(root: string): Generator<[string, any]> {
const filePaths = new fdir()
.withBasePath()
Expand All @@ -38,6 +47,18 @@ function* yamlEntries(root: string): Generator<[string, any]> {
for (const fp of filePaths) {
// The feature identifier/key is the filename without extension.
const { name: key } = path.parse(fp);
const pathParts = fp.split(path.sep);

// Assert ID uniqueness
for (const [pool, map] of Object.entries(uniqueIdMaps)) {
if (!pathParts.includes("spec") && pathParts.includes(pool)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the !pathParts.includes("spec") condition about the features/draft/spec/ directory? If so, should we instead give all drafts the same treatment? We set data[draft] in this loop so we might be able to use that in the condition.

const otherFile: string | undefined = map.get(key);
if (otherFile) {
throw new Error(`ID collision between ${fp} and ${otherFile}`);
}
map.set(key, fp);
}
}

if (!identifierPattern.test(key)) {
throw new Error(`${key} is not a valid identifier (see guidelines)`);
Expand All @@ -50,7 +71,7 @@ function* yamlEntries(root: string): Generator<[string, any]> {
Object.assign(data, dist);
}

if (fp.split(path.sep).includes('draft')) {
if (pathParts.includes('draft')) {
data[draft] = true;
}

Expand Down