Skip to content
Merged
Show file tree
Hide file tree
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
6 changes: 3 additions & 3 deletions docs/technical/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ src/
generate.ts # Documentation generation and watch mode
validate.ts # Coverage validation with JSON output
diff.ts # DocIR diff against a Git ref and breaking-change gate
adr.ts # Architecture Decision Records (114 lines)
adr.ts # Architecture Decision Records (124 lines)
reporters/
index.ts # CI output formatters (82 lines)
```
Expand Down Expand Up @@ -433,7 +433,7 @@ Comparing documentation against HEAD~1...

### 4.5 `docgen adr <action> [title]`

**Source:** `src/commands/adr.ts` (114 lines)
**Source:** `src/commands/adr.ts` (124 lines)

**Signature:**

Expand Down Expand Up @@ -474,7 +474,7 @@ Creates a new Architecture Decision Record file.

**Validation:** If `title` is not provided, prints an error and exits with code 1.

**ID Assignment:** Reads all files in the ADR directory matching the pattern `/^ADR-\d+/`, counts them, and assigns the next sequential number. IDs are zero-padded to three digits: `ADR-001`, `ADR-002`, etc.
**ID Assignment:** Reads the names of the files in the ADR directory that match `/^ADR-(\d+)/` and assigns one more than the highest number found (`nextAdrNumber()`), so a gap or a deleted ADR never causes a number to be reused. Files that are not numbered ADRs, such as a `README.md` or `ADR-template.md`, are ignored. With none, the first ADR is `ADR-001`. IDs are zero-padded to three digits (`ADR-001`, `ADR-002`, ...) and grow past that without truncation (`ADR-1000`).

**File Naming:** The file name is constructed as:

Expand Down
27 changes: 15 additions & 12 deletions docs/technical/parser-typescript.md
Original file line number Diff line number Diff line change
Expand Up @@ -670,16 +670,18 @@ text.replace(/import\([^)]+\)\./g, "")

---

#### `extractParameters(params: ParameterDeclaration[]): ParamNode[]`
#### `extractParameters(params: ParameterDeclaration[], docNode: Node): ParamNode[]`

Maps an array of ts-morph `ParameterDeclaration` nodes to DocIR `ParamNode`
objects.
objects. `docNode` is the node that carries the JSDoc block: the function,
method, constructor or accessor itself, or the variable statement for an arrow
function assigned to a `const`.

```typescript
{
name: p.getName(),
type: this.buildTypeRef(p.getType()),
description: this.getParamDescription(p),
description: this.getParamDescription(p, docNode),
isOptional: p.isOptional(),
isRest: p.isRestParameter(),
defaultValue: p.getInitializer()?.getText(),
Expand All @@ -688,17 +690,18 @@ objects.

---

#### `getParamDescription(param: ParameterDeclaration): string`
#### `getParamDescription(param: ParameterDeclaration, docNode: Node): string`

Walks up to the parameter's parent function/method, extracts all `@param`
tags, and finds the one whose `value` starts with the parameter name. Then
strips the parameter name and any leading `- ` separator from the value.
Looks through the JSDoc blocks on `docNode` for the `@param` tag whose name is
exactly the parameter's name (so `id` never picks up the text written for
`idx`). The text is the tag's comment: the optional `{type}`, the name and a
leading `-` are not part of it, and line breaks inside a multi-line description
become single spaces.

```typescript
paramTag.value.replace(new RegExp(`^${param.getName()}\\s*-?\\s*`), "")
```

Returns `""` if no matching `@param` tag is found.
Returns `""` if no matching `@param` tag exists. Both `@param a - text` and
`@param a text` are accepted. Before this was corrected, `@param` text never
reached `ParamNode.description`, so documented parameters were reported as
undocumented by `docgen validate`.

---

Expand Down
69 changes: 67 additions & 2 deletions packages/cli/src/commands/adr.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,71 @@ describe("adrCommand", () => {
});
});

// Known gap found by the audit. `todo` keeps it visible without a red run.
it.todo("numbers a new ADR after the highest existing number, not after the file count (ADR-001 and ADR-003 present creates a second ADR-003)");
// The audit found that numbering used the file count, so gaps produced duplicate numbers.
describe("numbering with gaps and other files", () => {
const seed = (...names: string[]) => {
fs.mkdirSync(decisions(), { recursive: true });
for (const name of names) fs.writeFileSync(path.join(decisions(), name), `# ${name}\n`);
};

it("numbers after the highest existing number, not after the file count", async () => {
seed("ADR-001-first.md", "ADR-003-third.md");

await adrCommand("new", "Fourth", { status: "proposed" });

expect(fs.readdirSync(decisions()).sort()).toEqual([
"ADR-001-first.md",
"ADR-003-third.md",
"ADR-004-fourth.md",
]);
});

it("does not reuse a number after an earlier ADR was deleted", async () => {
await adrCommand("new", "One", { status: "proposed" });
await adrCommand("new", "Two", { status: "proposed" });
fs.rmSync(path.join(decisions(), fs.readdirSync(decisions()).sort()[0]));

await adrCommand("new", "Three", { status: "proposed" });

expect(fs.readdirSync(decisions()).sort()).toEqual([
expect.stringMatching(/^ADR-002-two/),
expect.stringMatching(/^ADR-003-three/),
]);
});

it("keeps counting past three digits instead of truncating or repeating", async () => {
seed("ADR-999-nine-nine-nine.md", "ADR-1000-one-thousand.md");

await adrCommand("new", "Next", { status: "proposed" });

expect(fs.readdirSync(decisions())).toContain("ADR-1001-next.md");
});

it("ignores files that are not numbered ADRs when choosing the number", async () => {
seed("README.md", "ADR-template.md", "notes-2024.md", "ADR-002-real.md");

await adrCommand("new", "After real", { status: "proposed" });

expect(fs.readdirSync(decisions())).toContain("ADR-003-after-real.md");
expect(fs.readdirSync(decisions())).toHaveLength(5);
});

it("starts at ADR-001 when the folder holds no numbered ADRs", async () => {
seed("README.md");

await adrCommand("new", "First", { status: "proposed" });

expect(fs.readdirSync(decisions())).toContain("ADR-001-first.md");
});

it("never overwrites an existing file", async () => {
seed("ADR-001-first.md");
const before = fs.readFileSync(path.join(decisions(), "ADR-001-first.md"), "utf-8");

await adrCommand("new", "First", { status: "proposed" });

expect(fs.readFileSync(path.join(decisions(), "ADR-001-first.md"), "utf-8")).toBe(before);
expect(fs.readdirSync(decisions())).toContain("ADR-002-first.md");
});
});
});
22 changes: 18 additions & 4 deletions packages/cli/src/commands/adr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,23 @@ export async function adrCommand(
}
}

/**
* Choose the number for a new ADR: one more than the highest `ADR-<n>` file name.
* Counting files would reuse a number after a deletion or a gap, and files that
* are not numbered ADRs (a README, a template) must not affect the result.
*
* @param fileNames - Names of the files in the ADR directory.
* @returns The number to give the next ADR; 1 when there are no numbered ADRs.
*/
export function nextAdrNumber(fileNames: string[]): number {
let highest = 0;
for (const name of fileNames) {
const match = /^ADR-(\d+)/.exec(name);
if (match) highest = Math.max(highest, Number(match[1]));
}
return highest + 1;
}

function createAdr(
adrDir: string,
title: string | undefined,
Expand All @@ -47,10 +64,7 @@ function createAdr(
fs.mkdirSync(adrDir, { recursive: true });
}

// Determine next ADR number
const existing = fs.readdirSync(adrDir).filter((f) => f.match(/^ADR-\d+/));
const nextNum = existing.length + 1;
const id = `ADR-${String(nextNum).padStart(3, "0")}`;
const id = `ADR-${String(nextAdrNumber(fs.readdirSync(adrDir))).padStart(3, "0")}`;
const fileName = `${id}-${title.toLowerCase().replace(/[^a-z0-9]+/g, "-")}.md`;
const filePath = path.join(adrDir, fileName);

Expand Down
51 changes: 29 additions & 22 deletions packages/parser-typescript/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ export class TypeScriptParser implements ParserPlugin {
isAbstract: false,
signature: method.getText(),
description: this.extractDescription(method),
parameters: this.extractParameters(method.getParameters()),
parameters: this.extractParameters(method.getParameters(), method),
returnType: this.buildTypeRef(method.getReturnType()),
throws: [],
deprecated: null,
Expand Down Expand Up @@ -440,7 +440,7 @@ export class TypeScriptParser implements ParserPlugin {
isAbstract: false,
signature: this.buildFunctionSignature(func),
description: this.extractDescription(func),
parameters: this.extractParameters(func.getParameters()),
parameters: this.extractParameters(func.getParameters(), func),
returnType: this.buildTypeRef(func.getReturnType()),
throws: this.extractThrows(func),
deprecated: null,
Expand Down Expand Up @@ -498,7 +498,7 @@ export class TypeScriptParser implements ParserPlugin {
isAbstract: false,
signature: this.buildVariableFunctionSignature(declaration, initializer),
description: this.extractDescription(docNode),
parameters: this.extractParameters(initializer.getParameters()),
parameters: this.extractParameters(initializer.getParameters(), docNode),
returnType: this.buildTypeRef(initializer.getReturnType()),
throws: this.extractThrows(docNode),
deprecated: this.extractDeprecation(docNode),
Expand Down Expand Up @@ -715,7 +715,7 @@ export class TypeScriptParser implements ParserPlugin {
isAbstract: method.isAbstract(),
signature: this.buildMethodSignature(method),
description: this.extractDescription(method),
parameters: this.extractParameters(method.getParameters()),
parameters: this.extractParameters(method.getParameters(), method),
returnType: this.buildTypeRef(method.getReturnType()),
throws: this.extractThrows(method),
deprecated: this.extractDeprecation(method),
Expand Down Expand Up @@ -761,7 +761,7 @@ export class TypeScriptParser implements ParserPlugin {
.map((p) => p.getText())
.join(", ")})`,
description: this.extractDescription(ctor),
parameters: this.extractParameters(ctor.getParameters()),
parameters: this.extractParameters(ctor.getParameters(), ctor),
returnType: null,
throws: [],
deprecated: null,
Expand All @@ -787,7 +787,8 @@ export class TypeScriptParser implements ParserPlugin {
parameters:
kind === "setter"
? this.extractParameters(
(accessor as SetAccessorDeclaration).getParameters()
(accessor as SetAccessorDeclaration).getParameters(),
accessor
)
: [],
returnType:
Expand Down Expand Up @@ -904,32 +905,38 @@ export class TypeScriptParser implements ParserPlugin {
return text.replace(/import\([^)]+\)\./g, "");
}

private extractParameters(params: ParameterDeclaration[]): ParamNode[] {
/**
* Build parameter nodes for a callable.
* @param params - The declared parameters, in order.
* @param docNode - The node that carries the JSDoc block: the function itself, or the variable
* statement for an arrow function assigned to a const.
*/
private extractParameters(params: ParameterDeclaration[], docNode: Node): ParamNode[] {
return params.map((p) => ({
name: p.getName(),
type: this.buildTypeRef(p.getType()),
description: this.getParamDescription(p),
description: this.getParamDescription(p, docNode),
isOptional: p.isOptional(),
isRest: p.isRestParameter(),
defaultValue: p.getInitializer()?.getText(),
}));
}

private getParamDescription(param: ParameterDeclaration): string {
// Walk up to the function/method and find matching @param tag
const parent = param.getParent();
if (!parent) return "";

const tags = this.extractTags(parent);
const paramTag = tags.find(
(t) => t.tag === "param" && t.description.startsWith(param.getName())
);

if (paramTag) {
// Strip the param name from the value
return paramTag.description.replace(new RegExp(`^${param.getName()}\\s*-?\\s*`), "");
/**
* Find the `@param` text for one parameter. A tag's name is matched exactly (so `id` never picks up
* `idx`'s text), and the optional `{type}`, the name and a leading hyphen are not part of the text.
*/
private getParamDescription(param: ParameterDeclaration, docNode: Node): string {
const name = param.getName();
for (const doc of this.getJsDocs(docNode)) {
for (const tag of doc.getTags()) {
if (!Node.isJSDocParameterTag(tag) || tag.getName() !== name) continue;
return (tag.getCommentText() ?? "")
.replace(/^\s*-\s*/, "")
.replace(/\s*\n\s*/g, " ")
.trim();
}
}

return "";
}

Expand Down
Loading