From a83576e1ade95ecbf46fd390c804ee424dc1ec7e Mon Sep 17 00:00:00 2001 From: naresh-FD Date: Sat, 17 Oct 2026 16:41:35 +0530 Subject: [PATCH 1/2] Carry @param descriptions into ParamNode for TypeScript sources getParamDescription looked for a tag whose text started with the parameter name, but ts-morph returns the comment without the name, so no description was ever found and `docgen validate` listed every documented parameter as undocumented. Match the tag by its own name instead, read the JSDoc from the node that carries it (the variable statement for const arrow functions), and strip the hyphen and line breaks. Eight tests, five deliberate breakages each caught. Self-validation: 567 -> 557 violations, none added. CHANGE_ID: AUDIT-001 Co-Authored-By: Claude Sonnet 5.5 --- docs/technical/parser-typescript.md | 27 ++-- packages/parser-typescript/src/index.ts | 51 ++++---- .../parser-typescript/src/param-docs.test.ts | 115 ++++++++++++++++++ 3 files changed, 159 insertions(+), 34 deletions(-) create mode 100644 packages/parser-typescript/src/param-docs.test.ts diff --git a/docs/technical/parser-typescript.md b/docs/technical/parser-typescript.md index c55fafe..b486388 100644 --- a/docs/technical/parser-typescript.md +++ b/docs/technical/parser-typescript.md @@ -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(), @@ -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`. --- diff --git a/packages/parser-typescript/src/index.ts b/packages/parser-typescript/src/index.ts index e4270ec..fb30bb6 100644 --- a/packages/parser-typescript/src/index.ts +++ b/packages/parser-typescript/src/index.ts @@ -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, @@ -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, @@ -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), @@ -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), @@ -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, @@ -787,7 +787,8 @@ export class TypeScriptParser implements ParserPlugin { parameters: kind === "setter" ? this.extractParameters( - (accessor as SetAccessorDeclaration).getParameters() + (accessor as SetAccessorDeclaration).getParameters(), + accessor ) : [], returnType: @@ -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 ""; } diff --git a/packages/parser-typescript/src/param-docs.test.ts b/packages/parser-typescript/src/param-docs.test.ts new file mode 100644 index 0000000..e54aa16 --- /dev/null +++ b/packages/parser-typescript/src/param-docs.test.ts @@ -0,0 +1,115 @@ +import * as fs from "fs"; +import * as path from "path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@docgen/core", async () => await import("../../core/src")); + +// Each parse builds a ts-morph project; the first one in a cold process is slow. +vi.setConfig({ testTimeout: 30_000 }); + +// AUDIT-001 follow-up: `@param name - text` tags did not reach ParamNode.description, so RepoScribe's own +// validation listed every documented parameter as undocumented. These tests pin the intended behaviour. + +const langConfig = { + name: "typescript", + source: "src", + include: ["**/*.ts"], + exclude: [], + parser: "@docgen/parser-typescript", + options: {}, +}; + +describe("TypeScriptParser @param descriptions", () => { + let dir: string; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(process.cwd(), ".tmp-param-docs-")); + fs.mkdirSync(path.join(dir, "src"), { recursive: true }); + }); + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + async function parameters(source: string, memberName: string): Promise> { + const file = path.join(dir, "src", "sample.ts"); + fs.writeFileSync(file, source); + const { TypeScriptParser } = await import("./index"); + const docir = await new TypeScriptParser().parse([file], langConfig); + const members = docir.modules.flatMap((module) => module.members); + const member = members.find((candidate) => candidate.name === memberName); + if (!member) throw new Error(`member ${memberName} not found in ${members.map((m) => m.name).join(", ")}`); + return Object.fromEntries(member.parameters.map((parameter) => [parameter.name, parameter.description])); + } + + it("reads a description written with a hyphen after the name", async () => { + const result = await parameters( + ["/**", " * Add.", " * @param a - The first number.", " * @param b - The second number.", " */", + "export function add(a: number, b: number): number { return a + b; }"].join("\n"), + "add" + ); + expect(result).toEqual({ a: "The first number.", b: "The second number." }); + }); + + it("reads a description written without a hyphen", async () => { + const result = await parameters( + ["/**", " * Add.", " * @param a The first number.", " */", "export function add(a: number): number { return a; }"].join("\n"), + "add" + ); + expect(result).toEqual({ a: "The first number." }); + }); + + it("ignores a {type} annotation in the tag", async () => { + const result = await parameters( + ["/**", " * Add.", " * @param {number} a - The first number.", " */", "export function add(a: number): number { return a; }"].join("\n"), + "add" + ); + expect(result).toEqual({ a: "The first number." }); + }); + + it("gives each parameter its own text when names share a prefix", async () => { + const result = await parameters( + ["/**", " * Find.", " * @param idx - The index.", " * @param id - The identifier.", " */", + "export function find(id: string, idx: number): string { return id + idx; }"].join("\n"), + "find" + ); + expect(result).toEqual({ id: "The identifier.", idx: "The index." }); + }); + + it("leaves an undocumented parameter empty instead of borrowing another tag", async () => { + const result = await parameters( + ["/**", " * Add.", " * @param a - The first number.", " */", + "export function add(a: number, b: number): number { return a + b; }"].join("\n"), + "add" + ); + expect(result).toEqual({ a: "The first number.", b: "" }); + }); + + it("keeps a multi-line description together", async () => { + const result = await parameters( + ["/**", " * Add.", " * @param a - The first number,", " * which may be negative.", " */", + "export function add(a: number): number { return a; }"].join("\n"), + "add" + ); + expect(result.a).toContain("The first number,"); + expect(result.a).toContain("which may be negative."); + }); + + it("reads descriptions on arrow functions assigned to a const", async () => { + const result = await parameters( + ["/**", " * Double.", " * @param n - The number to double.", " */", + "export const double = (n: number): number => n * 2;"].join("\n"), + "double" + ); + expect(result).toEqual({ n: "The number to double." }); + }); + + it("reads descriptions on class methods", async () => { + const result = await parameters( + ["export class Calculator {", " /**", " * Add.", " * @param a - Left side.", " * @param b - Right side.", " */", + " add(a: number, b: number): number { return a + b; }", "}"].join("\n"), + "add" + ); + expect(result).toEqual({ a: "Left side.", b: "Right side." }); + }); +}); From 5b422428a8be52ec91baf8f92bcb6ba0b67bdb6c Mon Sep 17 00:00:00 2001 From: naresh-FD Date: Sat, 17 Oct 2026 16:44:12 +0530 Subject: [PATCH 2/2] Number new ADRs after the highest existing number `docgen adr new` took the file count plus one, so a gap or a deleted ADR made it reuse a number (ADR-001 and ADR-003 present produced a second ADR-003). Use the highest `ADR-` found instead, ignore files that are not numbered ADRs, and keep counting past three digits. The it.todo from the audit becomes six real tests; five deliberate breakages are each caught. CHANGE_ID: AUDIT-001 Co-Authored-By: Claude Sonnet 5.5 --- docs/technical/cli.md | 6 +-- packages/cli/src/commands/adr.test.ts | 69 ++++++++++++++++++++++++++- packages/cli/src/commands/adr.ts | 22 +++++++-- 3 files changed, 88 insertions(+), 9 deletions(-) diff --git a/docs/technical/cli.md b/docs/technical/cli.md index fb3240c..ae76e3c 100644 --- a/docs/technical/cli.md +++ b/docs/technical/cli.md @@ -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) ``` @@ -433,7 +433,7 @@ Comparing documentation against HEAD~1... ### 4.5 `docgen adr [title]` -**Source:** `src/commands/adr.ts` (114 lines) +**Source:** `src/commands/adr.ts` (124 lines) **Signature:** @@ -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: diff --git a/packages/cli/src/commands/adr.test.ts b/packages/cli/src/commands/adr.test.ts index c1416a7..707e46d 100644 --- a/packages/cli/src/commands/adr.test.ts +++ b/packages/cli/src/commands/adr.test.ts @@ -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"); + }); + }); }); diff --git a/packages/cli/src/commands/adr.ts b/packages/cli/src/commands/adr.ts index 79cb989..1608b81 100644 --- a/packages/cli/src/commands/adr.ts +++ b/packages/cli/src/commands/adr.ts @@ -33,6 +33,23 @@ export async function adrCommand( } } +/** + * Choose the number for a new ADR: one more than the highest `ADR-` 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, @@ -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);