diff --git a/packages/cli/src/commands/check/index.ts b/packages/cli/src/commands/check/index.ts index 20cdc74..1392208 100644 --- a/packages/cli/src/commands/check/index.ts +++ b/packages/cli/src/commands/check/index.ts @@ -1,15 +1,20 @@ import { Command } from "clipanion"; +import { isEmpty } from "es-toolkit/compat"; +import path from "path"; import { loadConfig } from "../../config/load-config.js"; -import { createPackageManager } from "../../package-manager/create-package-manager.js"; -import { isTargetPackage } from "../../package-manager/utils/is-target-package.js"; +import { getPackageEntryPoints } from "../../core/entry-point.js"; import { getTsConfigPath } from "../../core/get-ts-config-path.js"; import { getTsProject } from "../../core/get-ts-project.js"; -import { isExportSourceFile } from "../../core/parser/source/is-export-source-file.js"; -import { getExportedDeclarationsBySourceFile } from "../../core/parser/source/get-exported-declarations-by-sourcefile.js"; import { excludeBarrelReExports } from "../../core/parser/source/exclude-barrel-re-exports.js"; -import { hasJSDocTag } from "../../core/parser/jsdoc/jsdoc-utils.js"; -import { getPackageEntryPoints } from "../../core/entry-point.js"; -import path from "path"; +import { getExportedDeclarationsBySourceFile } from "../../core/parser/source/get-exported-declarations-by-sourcefile.js"; +import { isExportSourceFile } from "../../core/parser/source/is-export-source-file.js"; +import { createPackageManager } from "../../package-manager/create-package-manager.js"; +import { isTargetPackage } from "../../package-manager/utils/is-target-package.js"; +import { + PackageValidationResult, + validateExports, +} from "./validate/validate-exports.js"; +import { ValidationError } from "./validate/validate.types.js"; export class CheckCommand extends Command { static paths = [[`check`]]; @@ -17,7 +22,7 @@ export class CheckCommand extends Command { async execute(): Promise { const { checkConfig, projectConfig, targetPackages } = await loadContext(); - if (targetPackages.length === 0) { + if (isEmpty(targetPackages)) { printNoPackagesFound( projectConfig.workspace.include, projectConfig.root, @@ -26,6 +31,7 @@ export class CheckCommand extends Command { return 1; } + let hasErrors = false; for (const pkg of targetPackages) { console.log(`📝 ${pkg.name} processing...`); @@ -46,28 +52,19 @@ export class CheckCommand extends Command { const exportDeclarationsBySourceFiles = exportSourceFiles.flatMap( getExportedDeclarationsBySourceFile ); - const excludeBarrelReExport = excludeBarrelReExports( - exportDeclarationsBySourceFiles - ); - const missingJSDocExports = excludeBarrelReExport.filter((target) => { - return !hasJSDocTag(target.declaration, "public"); - }); + const exportDeclarations = excludeBarrelReExports(exportDeclarationsBySourceFiles); + const result = validateExports(exportDeclarations, projectConfig.root); + if (!isEmpty(result.issues)) { + printValidationErrors(pkg.name, result); + hasErrors = true; - if (missingJSDocExports.length > 0) { - console.log(`❌ ${pkg.name} has missing JSDoc:`); - missingJSDocExports.forEach((exportInfo) => { - const relativePath = path.relative( - projectConfig.root, - exportInfo.filePath - ); - console.log(` - ${relativePath}:${exportInfo.symbolName}`); - }); - } else { - console.log(`✅ ${pkg.name} has JSDoc for all exports`); + continue; } + + console.log(`✅ ${pkg.name} has JSDoc for all exports`); } - return 0; + return hasErrors ? 1 : 0; } } @@ -108,3 +105,31 @@ function printNoPackagesFound( console.error(` - project root: ${root}`); console.error(` - package manager: ${packageManager}`); } + + + +function printValidationErrors(packageName: string, result: PackageValidationResult): void { + console.log(`❌ ${packageName} has missing JSDoc:`); + + result.issues.forEach((issue) => { + issue.errors.forEach((error) => { + const message = formatErrorMessage(error); + console.log(` - ${issue.relativePath}:${issue.exportDeclaration.symbolName} - ${message}`); + }); + }); +} + +function formatErrorMessage(error: ValidationError): string { + switch (error.type) { + case "missing_public": + return "missing @public"; + case "missing_param": + return `missing @param for '${error.target}'`; + case "unused_param": + return `unused @param '${error.target}'`; + case "missing_returns": + return "missing @returns"; + case "invalid_returns": + return error.message ?? "invalid @returns"; + } +} diff --git a/packages/cli/src/commands/check/validate/validate-exports.ts b/packages/cli/src/commands/check/validate/validate-exports.ts new file mode 100644 index 0000000..c850a08 --- /dev/null +++ b/packages/cli/src/commands/check/validate/validate-exports.ts @@ -0,0 +1,49 @@ +import { isEmpty } from "es-toolkit/compat"; +import path from "path"; +import { ExportDeclaration } from "../../../core/types/parser.types.js"; +import { validate } from "./validate-jsdoc.js"; +import { validatePublic } from "./validate-public.js"; +import { ValidationError } from "./validate.types.js"; + +export interface ValidationIssue { + exportDeclaration: ExportDeclaration; + relativePath: string; + errors: ValidationError[]; +} + +export interface PackageValidationResult { + issues: ValidationIssue[]; +} + +export function validateExports( + exportDeclarations: ExportDeclaration[], + projectRoot: string +): PackageValidationResult { + const issues = exportDeclarations + .map((target) => { + const relativePath = path.relative(projectRoot, target.filePath); + const isPublic = validatePublic(target.declaration); + + if (!isPublic) { + return { + exportDeclaration: target, + relativePath, + errors: [{ type: "missing_public" as const, target: target.symbolName }], + }; + } + + const { errors } = validate(target.declaration); + if (isEmpty(errors)) { + return null; + } + + return { + exportDeclaration: target, + relativePath, + errors, + }; + }) + .filter((issue) => issue != null); + + return { issues }; +} \ No newline at end of file diff --git a/packages/cli/src/commands/check/validate/validate-jsdoc.ts b/packages/cli/src/commands/check/validate/validate-jsdoc.ts new file mode 100644 index 0000000..3abe3f3 --- /dev/null +++ b/packages/cli/src/commands/check/validate/validate-jsdoc.ts @@ -0,0 +1,69 @@ +import { Node } from 'ts-morph'; +import { JSDocParser } from '../../../core/parser/jsdoc/jsdoc-parser.js'; +import { getJSDoc } from '../../../core/parser/jsdoc/jsdoc-utils.js'; +import { ParsedJSDoc } from '../../../core/types/parser.types.js'; +import { FunctionValidator } from './validator/function-validator.js'; +import { InterfaceValidator } from './validator/interface-validator.js'; + + +export function validate(node: Node) { + const validator = createValidator(node); + + if (validator != null) { + return validator.validate(); + } + + return { errors: [], isValid: true }; +} + +const EMPTY_PARSED_JSDOC: ParsedJSDoc = { + examples: [], + parameters: [], + throws: [], + typedef: [], + see: [], + version: [], +}; + +function createValidator(node: Node) { + const jsDocParser = new JSDocParser(); + const jsDoc = getJSDoc(node); + const parsedJSDoc = jsDoc != null ? jsDocParser.parse(jsDoc) : EMPTY_PARSED_JSDOC; + + if (Node.isFunctionDeclaration(node)) { + return new FunctionValidator(node, parsedJSDoc); + } + + if (Node.isVariableDeclaration(node)) { + const initializer = node.getInitializer(); + + if (Node.isArrowFunction(initializer)) { + return new FunctionValidator(node, parsedJSDoc); + } + + // TODO: + // if (Node.isObjectLiteralExpression(initializer)) { + // return new ObjectLiteralValidator(node, parsedJSDoc); + // } + } + + + if (Node.isInterfaceDeclaration(node)) { + return new InterfaceValidator(node, parsedJSDoc); + } + + // if (Node.isTypeAliasDeclaration(node)) { + // return new TypeAliasValidator(node, parsedJSDoc); + // } + + // if (Node.isEnumDeclaration(node)) { + // return new EnumValidator(node, parsedJSDoc); + // } + + + // if (Node.isClassDeclaration(node)) { + // return new ClassValidator(node, parsedJSDoc); + // } + + return undefined; +} \ No newline at end of file diff --git a/packages/cli/src/commands/check/validate/validate-public.ts b/packages/cli/src/commands/check/validate/validate-public.ts new file mode 100644 index 0000000..7bf43c0 --- /dev/null +++ b/packages/cli/src/commands/check/validate/validate-public.ts @@ -0,0 +1,6 @@ +import { hasJSDocTag } from "../../../core/parser/jsdoc/jsdoc-utils.js"; +import { Node } from "ts-morph"; + +export function validatePublic(node: Node) { + return hasJSDocTag(node, "public"); +} diff --git a/packages/cli/src/commands/check/validate/validate.types.ts b/packages/cli/src/commands/check/validate/validate.types.ts new file mode 100644 index 0000000..9595b25 --- /dev/null +++ b/packages/cli/src/commands/check/validate/validate.types.ts @@ -0,0 +1,17 @@ +type ValidationErrorType = + | "missing_public" + | "missing_param" + | "unused_param" + | "missing_returns" + | "invalid_returns"; + +export interface ValidationError { + type: ValidationErrorType; + target: string; + message?: string; +} + +export interface ValidationResult { + errors: ValidationError[]; + isValid: boolean; +} diff --git a/packages/cli/src/commands/check/validate/validator/function-validator.ts b/packages/cli/src/commands/check/validate/validator/function-validator.ts new file mode 100644 index 0000000..6c1920a --- /dev/null +++ b/packages/cli/src/commands/check/validate/validator/function-validator.ts @@ -0,0 +1,162 @@ +import { isEmpty } from "es-toolkit/compat"; +import { getJSDocParameterNames } from "../../../../core/parser/jsdoc/jsdoc-utils.js"; +import { + FunctionDeclaration, + Node, + ParameterDeclaration, + Type, + VariableDeclaration, +} from "ts-morph"; +import { ValidationError } from "../validate.types.js"; +import { Validator } from "./validator.js"; + +type FunctionLikeNode = FunctionDeclaration | VariableDeclaration; + +export class FunctionValidator extends Validator { + validate() { + const errors = [...this.validateParams(), ...this.validateReturns()]; + + return { errors, isValid: isEmpty(errors) }; + } + + private validateParams(): ValidationError[] { + const parameters = this.getParameters(); + const jsDocParamNames = getJSDocParameterNames(this.parsedJSDoc.parameters ?? []); + const tsParamNames = parameters.map((p) => p.getName()); + + const missingParams = this.findMissingParams(tsParamNames, jsDocParamNames); + const unusedParams = this.findUnusedParams(jsDocParamNames, tsParamNames, parameters); + + return [...missingParams, ...unusedParams]; + } + + private validateReturns(): ValidationError[] { + const jsDocReturns = this.parsedJSDoc.returns; + + if (jsDocReturns == null) { + return [{ type: "missing_returns" as const, target: "returns" }]; + } + + if (isEmpty(jsDocReturns.type)) { + return []; + } + + const actualReturnType = this.getReturnType(); + if (actualReturnType == null) { + return []; + } + + if (!this.areTypesCompatible(jsDocReturns.type, actualReturnType)) { + return [ + { + type: "invalid_returns" as const, + target: "returns", + message: `Expected ${jsDocReturns.type}, got ${actualReturnType.getText()}`, + }, + ]; + } + + return []; + } + + private getParameters(): ParameterDeclaration[] { + if (Node.isFunctionDeclaration(this.node)) { + return this.node.getParameters(); + } + + if (Node.isVariableDeclaration(this.node)) { + const initializer = this.node.getInitializer(); + if (initializer && Node.isArrowFunction(initializer)) { + return initializer.getParameters(); + } + } + + return []; + } + + private getReturnType(): Type | undefined { + if (Node.isFunctionDeclaration(this.node)) { + return this.node.getReturnType(); + } + + if (Node.isVariableDeclaration(this.node)) { + const initializer = this.node.getInitializer(); + const isFunctionLike = + initializer && (Node.isArrowFunction(initializer) || Node.isFunctionExpression(initializer)); + + return isFunctionLike ? initializer.getReturnType() : undefined; + } + + return undefined; + } + + + private findMissingParams(tsParamNames: string[], jsDocParamNames: string[]): ValidationError[] { + return tsParamNames + .filter((name) => !jsDocParamNames.includes(name)) + .map((name) => ({ type: "missing_param" as const, target: name })); + } + + private findUnusedParams( + jsDocParamNames: string[], + tsParamNames: string[], + parameters: ParameterDeclaration[] + ): ValidationError[] { + return jsDocParamNames + .filter((name) => this.isParamUnused(name, tsParamNames, parameters)) + .map((name) => ({ type: "unused_param" as const, target: name })); + } + + private isParamUnused( + jsDocParamName: string, + tsParamNames: string[], + parameters: ParameterDeclaration[] + ): boolean { + if (!jsDocParamName.includes(".")) { + return !tsParamNames.includes(jsDocParamName); + } + + return this.isNestedParamUnused(jsDocParamName, parameters); + } + + private isNestedParamUnused(jsDocParamName: string, parameters: ParameterDeclaration[]): boolean { + const [parentName, ...propertyPath] = jsDocParamName.split("."); + const param = parameters.find((p) => p.getName() === parentName); + + if (param == null) { + return true; + } + + return !this.hasPropertyPath(param.getType(), propertyPath, param); + } + + private hasPropertyPath(type: Type, path: string[], locationNode: ParameterDeclaration): boolean { + let currentType = type; + + for (const propertyName of path) { + const property = currentType.getProperty(propertyName); + if (property == null) { + return false; + } + currentType = property.getValueDeclaration()?.getType() ?? property.getTypeAtLocation(locationNode); + } + + return true; + } + + private areTypesCompatible(jsDocTypeText: string, actualType: Type): boolean { + // import('@mylib/math').Math -> Math + const normalize = (t: string) => + t.replace(/import\([^)]+\)\./g, "").replace(/\s+/g, " ").trim(); + + const expected = normalize(jsDocTypeText); + const actual = normalize(actualType.getText()); + + return expected === actual || this.areVoidUndefinedEquivalent(expected, actual); + } + + private areVoidUndefinedEquivalent(a: string, b: string): boolean { + const voidTypes = ["void", "undefined"]; + return voidTypes.includes(a) && voidTypes.includes(b); + } +} diff --git a/packages/cli/src/commands/check/validate/validator/interface-validator.ts b/packages/cli/src/commands/check/validate/validator/interface-validator.ts new file mode 100644 index 0000000..a18e050 --- /dev/null +++ b/packages/cli/src/commands/check/validate/validator/interface-validator.ts @@ -0,0 +1,54 @@ +import { difference, flatMap } from "es-toolkit"; +import { isEmpty } from "es-toolkit/compat"; +import { InterfaceDeclaration, Node, PropertySignature } from "ts-morph"; +import { getJSDocParameterNames } from "../../../../core/parser/jsdoc/jsdoc-utils.js"; +import { Validator } from "./validator.js"; + +export class InterfaceValidator extends Validator { + validate() { + const properties = this.node + .getProperties() + .filter((p) => Node.isPropertySignature(p)); + + const allPropertyPaths = flatMap(properties, (p) => collectPropertyPaths(p)); + const methods = this.node.getMethods().map((m) => m.getName()); + + const jsDocParams = getJSDocParameterNames(this.parsedJSDoc.parameters ?? []); + + // TODO: The original implementation treated params, members, and properties all as params, + // but each of them should be handled separately + const missing = difference(allPropertyPaths, jsDocParams).map((target) => ({ + type: "missing_param" as const, + target, + })); + + const allValidPaths = [...allPropertyPaths, ...methods]; + const unused = difference(jsDocParams, allValidPaths).map((target) => ({ + type: "unused_param" as const, + target, + })); + + const errors = [...missing, ...unused]; + return { errors, isValid: isEmpty(errors) }; + } +} + +function collectPropertyPaths(prop: PropertySignature, prefix = ""): string[] { + const name = prop.getName(); + const fullPath = prefix !== "" ? `${prefix}.${name}` : name; + const typeNode = prop.getTypeNode(); + + if (!Node.isTypeLiteral(typeNode)) { + return [fullPath]; + } + + const nestedProps = typeNode + .getProperties() + .filter((p) => Node.isPropertySignature(p)); + + if (isEmpty(nestedProps)) { + return [fullPath]; + } + + return [fullPath, ...flatMap(nestedProps, (p) => collectPropertyPaths(p, fullPath))]; +} diff --git a/packages/cli/src/commands/check/validate/validator/validator.ts b/packages/cli/src/commands/check/validate/validator/validator.ts new file mode 100644 index 0000000..4425bf7 --- /dev/null +++ b/packages/cli/src/commands/check/validate/validator/validator.ts @@ -0,0 +1,12 @@ +import { Node } from "ts-morph"; +import { ParsedJSDoc } from "../../../../core/types/parser.types.js"; +import { ValidationResult } from "../validate.types.js"; + +export abstract class Validator { + constructor( + readonly node: TSNode, + protected readonly parsedJSDoc: ParsedJSDoc + ) { } + + abstract validate(): ValidationResult; +} diff --git a/packages/cli/src/core/parser/jsdoc/jsdoc-parser.ts b/packages/cli/src/core/parser/jsdoc/jsdoc-parser.ts index 92127e7..8d16598 100644 --- a/packages/cli/src/core/parser/jsdoc/jsdoc-parser.ts +++ b/packages/cli/src/core/parser/jsdoc/jsdoc-parser.ts @@ -1,18 +1,28 @@ -import { JSDoc } from "ts-morph"; import * as commentParser from "comment-parser"; +import { isEmpty } from "es-toolkit/compat"; +import { JSDoc } from "ts-morph"; import { - ParsedJSDoc, + ExampleData, ParameterData, + ParsedJSDoc, ReturnData, + SeeData, ThrowsData, TypedefData, - ExampleData, - SeeData, VersionData, } from "../../types/parser.types.js"; const LANGUAGE = "tsx"; +const EMPTY_PARSED_JSDOC: ParsedJSDoc = { + examples: [], + parameters: [], + throws: [], + typedef: [], + see: [], + version: [], +}; + export class JSDocParser { parse(jsDoc: JSDoc): ParsedJSDoc { if (!jsDoc) { @@ -51,14 +61,7 @@ export class JSDocParser { } private createEmptyJSDocData(): ParsedJSDoc { - return { - examples: [], - parameters: [], - throws: [], - typedef: [], - see: [], - version: [], - }; + return EMPTY_PARSED_JSDOC; } private extractDescription(block: commentParser.Block): string | undefined { @@ -140,28 +143,63 @@ export class JSDocParser { paramMap: Map, parameters: ParameterData[], ): void { - const parts = param.name.split("."); - const parentName = parts[0]; - if (!parentName) return; - - let parent = paramMap.get(parentName); - - if (!parent) { - parent = { - name: parentName, - type: "Object", - description: "", - required: true, - defaultValue: undefined, - nested: [], - }; - paramMap.set(parentName, parent); - parameters.push(parent); + const [rootName, ...nestedPath] = param.name.split("."); + if (rootName == null || isEmpty(nestedPath)) { + return; } - const nestedParam = { ...param, name: parts.slice(1).join(".") }; - parent.nested = parent.nested || []; - parent.nested.push(nestedParam); + const existingRoot = paramMap.get(rootName); + const rootParam = existingRoot ?? this.createPlaceholderObjectParam(rootName); + if (existingRoot == null) { + paramMap.set(rootName, rootParam); + parameters.push(rootParam); + } + + this.insertParamAtPath({ + into: rootParam.nested ?? [], + path: nestedPath, + param, + }); + } + + private insertParamAtPath({ into, path, param }: { + into: ParameterData[]; + path: string[]; + param: ParameterData; + }): void { + const [currentSegment, ...remainingPath] = path; + if (currentSegment == null) { + return; + } + + if (isEmpty(remainingPath)) { + into.push({ ...param, name: currentSegment, nested: [] }); + + return; + } + + const existingChild = into.find((n) => n.name === currentSegment); + const child = existingChild ?? this.createPlaceholderObjectParam(currentSegment); + if (existingChild == null) { + into.push(child); + } + + this.insertParamAtPath({ + into: child.nested ?? [], + path: remainingPath, + param, + }); + } + + private createPlaceholderObjectParam(name: string): ParameterData { + return { + name, + type: "Object", + description: "", + required: true, + defaultValue: undefined, + nested: [], + }; } private extractReturns(block: commentParser.Block): ReturnData | undefined { diff --git a/packages/cli/src/core/parser/jsdoc/jsdoc-utils.ts b/packages/cli/src/core/parser/jsdoc/jsdoc-utils.ts index b9e0e88..4d57f87 100644 --- a/packages/cli/src/core/parser/jsdoc/jsdoc-utils.ts +++ b/packages/cli/src/core/parser/jsdoc/jsdoc-utils.ts @@ -1,4 +1,7 @@ import { Node, JSDocableNode, JSDoc } from "ts-morph"; +import { flatMap } from "es-toolkit"; +import { isEmpty } from "es-toolkit/compat"; +import { ParameterData } from "../../types/parser.types.js"; export function hasJSDocTag(node: Node, tagName: string): boolean { const jsDocableNode = getJSDocableNode(node); @@ -39,3 +42,18 @@ function getJSDocableNode(node: Node): JSDocableNode | undefined { return undefined; } + +export function getJSDocParameterNames(parameters: ParameterData[]): string[] { + const collectNames = (params: ParameterData[], prefix = ""): string[] => { + return flatMap(params, (param) => { + const fullName = isEmpty(prefix) ? param.name : `${prefix}.${param.name}`; + + return [ + fullName, + ...(param.nested != null ? collectNames(param.nested, fullName) : []), + ]; + }); + }; + + return collectNames(parameters); +} diff --git a/packages/cli/src/tests/commands/check/function-validator.spec.ts b/packages/cli/src/tests/commands/check/function-validator.spec.ts new file mode 100644 index 0000000..45347c0 --- /dev/null +++ b/packages/cli/src/tests/commands/check/function-validator.spec.ts @@ -0,0 +1,245 @@ +import { describe, it, expect } from "vitest"; +import { assert } from "es-toolkit"; +import { FunctionValidator } from "../../../commands/check/validate/validator/function-validator.js"; +import { createTSSourceFile } from "../../utils/create-ts-source-file.js"; +import { parseJSDocFromNode } from "../../utils/parse-jsdoc-from-node.js"; + +describe("FunctionValidator", () => { + describe("function declarations", () => { + describe("parameter validation", () => { + it("should return valid when all params are documented", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param name - The name + * @param age - The age + * @returns The greeting + */ + export function greet(name: string, age: number): string { + return name + age; + } + `); + + const fn = sourceFile.getFunctions()[0]; + assert(fn != null, "Expected function"); + + const validator = new FunctionValidator(fn, parseJSDocFromNode(fn)); + const result = validator.validate(); + + expect(result.isValid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it("should detect missing param documentation", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param name - The name + * @returns The greeting + */ + export function greet(name: string, age: number): string { + return name + age; + } + `); + + const fn = sourceFile.getFunctions()[0]; + assert(fn != null, "Expected function"); + + const validator = new FunctionValidator(fn, parseJSDocFromNode(fn)); + const result = validator.validate(); + + expect(result.isValid).toBe(false); + expect(result.errors).toContainEqual({ + type: "missing_param", + target: "age", + }); + }); + + it("should detect unused param documentation", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param name - The name + * @param oldParam - No longer exists + * @returns The greeting + */ + export function greet(name: string): string { + return name; + } + `); + + const fn = sourceFile.getFunctions()[0]; + assert(fn != null, "Expected function"); + + const validator = new FunctionValidator(fn, parseJSDocFromNode(fn)); + const result = validator.validate(); + + expect(result.isValid).toBe(false); + expect(result.errors).toContainEqual({ + type: "unused_param", + target: "oldParam", + }); + }); + + it("should handle object parameter with nested JSDoc", () => { + const sourceFile = createTSSourceFile(` + interface Options { + a: string; + b: string; + } + + /** + * @public + * @param options - The options object + * @param options.a - The a value + * @param options.b - The b value + * @returns The result + */ + export function foo(options: Options): string { + return options.a + options.b; + } + `); + + const fn = sourceFile.getFunctions()[0]; + assert(fn != null, "Expected function"); + + const validator = new FunctionValidator(fn, parseJSDocFromNode(fn)); + const result = validator.validate(); + + expect(result.isValid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it("should detect unused nested param that does not exist on type", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param options - The options object + * @param options.timeout - Timeout in ms + * @param options.retries - Number of retries + * @param options.something - This property does not exist + * @returns The result + */ + export function fetch(options: { timeout: number; retries: number }): string { + return "done"; + } + `); + + const fn = sourceFile.getFunctions()[0]; + assert(fn != null, "Expected function"); + + const validator = new FunctionValidator(fn, parseJSDocFromNode(fn)); + const result = validator.validate(); + + expect(result.isValid).toBe(false); + expect(result.errors).toContainEqual({ + type: "unused_param", + target: "options.something", + }); + }); + }); + + describe("return validation", () => { + it("should detect missing @returns when @public is present", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param name - The name + */ + export function greet(name: string): string { + return name; + } + `); + + const fn = sourceFile.getFunctions()[0]; + assert(fn != null, "Expected function"); + + const validator = new FunctionValidator(fn, parseJSDocFromNode(fn)); + const result = validator.validate(); + + expect(result.isValid).toBe(false); + expect(result.errors).toContainEqual({ + type: "missing_returns", + target: "returns", + }); + }); + + it("should detect invalid @returns type", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param name - The name + * @returns {number} The greeting + */ + export function greet(name: string): string { + return name; + } + `); + + const fn = sourceFile.getFunctions()[0]; + assert(fn != null, "Expected function"); + + const validator = new FunctionValidator(fn, parseJSDocFromNode(fn)); + const result = validator.validate(); + + expect(result.isValid).toBe(false); + expect(result.errors).toContainEqual( + expect.objectContaining({ + type: "invalid_returns", + target: "returns", + }) + ); + }); + }); + + }); + + describe("arrow functions (variable declarations)", () => { + it("should validate arrow function params", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param name - The name + * @returns The greeting + */ + export const greet = (name: string): string => name; + `); + + const variable = sourceFile.getVariableDeclarations()[0]; + assert(variable != null, "Expected variable declaration"); + + const validator = new FunctionValidator(variable, parseJSDocFromNode(variable)); + const result = validator.validate(); + + expect(result.isValid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it("should detect missing param in arrow function", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @returns The sum + */ + export const add = (a: number, b: number): number => a + b; + `); + + const variable = sourceFile.getVariableDeclarations()[0]; + assert(variable != null, "Expected variable declaration"); + + const validator = new FunctionValidator(variable, parseJSDocFromNode(variable)); + const result = validator.validate(); + + expect(result.isValid).toBe(false); + expect(result.errors).toContainEqual({ + type: "missing_param", + target: "a", + }); + expect(result.errors).toContainEqual({ + type: "missing_param", + target: "b", + }); + }); + + }); +}); diff --git a/packages/cli/src/tests/commands/check/interface-validator.spec.ts b/packages/cli/src/tests/commands/check/interface-validator.spec.ts new file mode 100644 index 0000000..f32fba2 --- /dev/null +++ b/packages/cli/src/tests/commands/check/interface-validator.spec.ts @@ -0,0 +1,270 @@ +import { describe, it, expect } from "vitest"; +import { assert } from "es-toolkit"; +import { InterfaceValidator } from "../../../commands/check/validate/validator/interface-validator.js"; +import { createTSSourceFile } from "../../utils/create-ts-source-file.js"; +import { parseJSDocFromNode } from "../../utils/parse-jsdoc-from-node.js"; + +describe("InterfaceValidator", () => { + describe("property validation", () => { + it("should return valid when all properties are documented", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param name - The user's name + * @param age - The user's age + */ + export interface User { + name: string; + age: number; + } + `); + + const iface = sourceFile.getInterfaces()[0]; + assert(iface != null, "Expected interface"); + + const validator = new InterfaceValidator(iface, parseJSDocFromNode(iface)); + const result = validator.validate(); + + expect(result.isValid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it("should detect missing property documentation", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param name - The user's name + */ + export interface User { + name: string; + age: number; + } + `); + + const iface = sourceFile.getInterfaces()[0]; + assert(iface != null, "Expected interface"); + + const validator = new InterfaceValidator(iface, parseJSDocFromNode(iface)); + const result = validator.validate(); + + expect(result.isValid).toBe(false); + expect(result.errors).toContainEqual({ + type: "missing_param", + target: "age", + }); + }); + + it("should detect unused property documentation", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param name - The user's name + * @param deletedField - No longer exists + */ + export interface User { + name: string; + } + `); + + const iface = sourceFile.getInterfaces()[0]; + assert(iface != null, "Expected interface"); + + const validator = new InterfaceValidator(iface, parseJSDocFromNode(iface)); + const result = validator.validate(); + + expect(result.isValid).toBe(false); + expect(result.errors).toContainEqual({ + type: "unused_param", + target: "deletedField", + }); + }); + + it("should detect multiple missing and unused params", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param oldField1 - Removed + * @param oldField2 - Also removed + */ + export interface User { + name: string; + email: string; + } + `); + + const iface = sourceFile.getInterfaces()[0]; + assert(iface != null, "Expected interface"); + + const validator = new InterfaceValidator(iface, parseJSDocFromNode(iface)); + const result = validator.validate(); + + expect(result.isValid).toBe(false); + expect(result.errors).toContainEqual({ + type: "missing_param", + target: "name", + }); + expect(result.errors).toContainEqual({ + type: "missing_param", + target: "email", + }); + expect(result.errors).toContainEqual({ + type: "unused_param", + target: "oldField1", + }); + expect(result.errors).toContainEqual({ + type: "unused_param", + target: "oldField2", + }); + }); + }); + + describe("nested property validation", () => { + it("should accept nested property documentation", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param address - The address + * @param address.street - The street + * @param address.city - The city + */ + export interface User { + address: { + street: string; + city: string; + }; + } + `); + + const iface = sourceFile.getInterfaces()[0]; + assert(iface != null, "Expected interface"); + + const validator = new InterfaceValidator(iface, parseJSDocFromNode(iface)); + const result = validator.validate(); + + expect(result.isValid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it("should detect missing nested property documentation", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param address - The address + * @param address.street - The street + */ + export interface User { + address: { + street: string; + city: string; + }; + } + `); + + const iface = sourceFile.getInterfaces()[0]; + assert(iface != null, "Expected interface"); + + const validator = new InterfaceValidator(iface, parseJSDocFromNode(iface)); + const result = validator.validate(); + + expect(result.isValid).toBe(false); + expect(result.errors).toContainEqual({ + type: "missing_param", + target: "address.city", + }); + }); + + it("should detect unused nested property documentation", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param address - The address + * @param address.zipCode - Removed property + */ + export interface User { + address: { + street: string; + }; + } + `); + + const iface = sourceFile.getInterfaces()[0]; + assert(iface != null, "Expected interface"); + + const validator = new InterfaceValidator(iface, parseJSDocFromNode(iface)); + const result = validator.validate(); + + expect(result.isValid).toBe(false); + expect(result.errors).toContainEqual({ + type: "unused_param", + target: "address.zipCode", + }); + }); + }); + + describe("interface methods", () => { + it("should allow documented method names as valid params", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param name - The name + * @param greet - Method to greet + */ + export interface Person { + name: string; + greet(): void; + } + `); + + const iface = sourceFile.getInterfaces()[0]; + assert(iface != null, "Expected interface"); + + const validator = new InterfaceValidator(iface, parseJSDocFromNode(iface)); + const result = validator.validate(); + + expect(result.isValid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it("should not require method documentation", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + * @param name - The name + */ + export interface Person { + name: string; + greet(): void; + } + `); + + const iface = sourceFile.getInterfaces()[0]; + assert(iface != null, "Expected interface"); + + const validator = new InterfaceValidator(iface, parseJSDocFromNode(iface)); + const result = validator.validate(); + + expect(result.isValid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + }); + + describe("empty interface", () => { + it("should return valid for empty interface with JSDoc", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + */ + export interface Empty {} + `); + + const iface = sourceFile.getInterfaces()[0]; + assert(iface != null, "Expected interface"); + + const validator = new InterfaceValidator(iface, parseJSDocFromNode(iface)); + const result = validator.validate(); + + expect(result.isValid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + }); +}); diff --git a/packages/cli/src/tests/commands/check/validate-public.spec.ts b/packages/cli/src/tests/commands/check/validate-public.spec.ts new file mode 100644 index 0000000..e46fc0c --- /dev/null +++ b/packages/cli/src/tests/commands/check/validate-public.spec.ts @@ -0,0 +1,196 @@ +import { describe, it, expect } from "vitest"; +import { assert } from "es-toolkit"; +import { validatePublic } from "../../../commands/check/validate/validate-public.js"; +import { createTSSourceFile } from '../../utils/create-ts-source-file.js'; + +describe("validatePublic - @public tag detection", () => { + describe("function declarations", () => { + it("should return true when @public tag exists", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + */ + export function greet(name: string) {} + `); + + const functions = sourceFile.getFunctions(); + const target = functions[0]; + assert(target != null, "Expected function"); + + expect(validatePublic(target)).toBe(true); + }); + + it("should return false when @public tag is missing", () => { + const sourceFile = createTSSourceFile(` + /** + * Internal function + */ + export function internal(a: number): number { + return a; + } + `); + + const functions = sourceFile.getFunctions(); + const target = functions[0]; + assert(target != null, "Expected function"); + + expect(validatePublic(target)).toBe(false); + }); + + it("should return false when no JSDoc exists", () => { + const sourceFile = createTSSourceFile(` + export function noDoc(a: number): number { + return a; + } + `); + + const functions = sourceFile.getFunctions(); + const target = functions[0]; + assert(target != null, "Expected function"); + + expect(validatePublic(target)).toBe(false); + }); + }); + + describe("variable declarations (arrow functions)", () => { + it("should return true when @public tag exists", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + */ + export const greet = (name: string): string => name; + `); + + const variables = sourceFile.getVariableDeclarations(); + const target = variables[0]; + assert(target != null, "Expected variable declaration"); + + expect(validatePublic(target)).toBe(true); + }); + + it("should return false when @public tag is missing", () => { + const sourceFile = createTSSourceFile(` + /** + * Internal arrow function + */ + export const internal = (a: number): number => a; + `); + + const variables = sourceFile.getVariableDeclarations(); + const target = variables[0]; + assert(target != null, "Expected variable declaration"); + + expect(validatePublic(target)).toBe(false); + }); + }); + + describe("interface declarations", () => { + it("should return true when @public tag exists", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + */ + export interface User { + name: string; + } + `); + + const interfaces = sourceFile.getInterfaces(); + const target = interfaces[0]; + assert(target != null, "Expected interface"); + + expect(validatePublic(target)).toBe(true); + }); + + it("should return false when @public tag is missing", () => { + const sourceFile = createTSSourceFile(` + /** + * Internal interface + */ + export interface InternalUser { + name: string; + } + `); + + const interfaces = sourceFile.getInterfaces(); + const target = interfaces[0]; + assert(target != null, "Expected interface"); + + expect(validatePublic(target)).toBe(false); + }); + }); + + describe("type alias declarations", () => { + it("should return true when @public tag exists", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + */ + export type Point = { + x: number; + y: number; + }; + `); + + const typeAliases = sourceFile.getTypeAliases(); + const target = typeAliases[0]; + assert(target != null, "Expected type alias"); + + expect(validatePublic(target)).toBe(true); + }); + + it("should return false when @public tag is missing", () => { + const sourceFile = createTSSourceFile(` + /** + * Internal type + */ + export type InternalPoint = { + x: number; + y: number; + }; + `); + + const typeAliases = sourceFile.getTypeAliases(); + const target = typeAliases[0]; + assert(target != null, "Expected type alias"); + + expect(validatePublic(target)).toBe(false); + }); + }); + + describe("class declarations", () => { + it("should return true when @public tag exists", () => { + const sourceFile = createTSSourceFile(` + /** + * @public + */ + export class User { + name: string; + } + `); + + const classes = sourceFile.getClasses(); + const target = classes[0]; + assert(target != null, "Expected class"); + + expect(validatePublic(target)).toBe(true); + }); + + it("should return false when @public tag is missing", () => { + const sourceFile = createTSSourceFile(` + /** + * Internal class + */ + export class InternalUser { + name: string; + } + `); + + const classes = sourceFile.getClasses(); + const target = classes[0]; + assert(target != null, "Expected class"); + + expect(validatePublic(target)).toBe(false); + }); + }); +}); diff --git a/packages/cli/src/tests/core/parser/jsdoc/jsdoc-parser.spec.ts b/packages/cli/src/tests/core/parser/jsdoc/jsdoc-parser.spec.ts index 28629a7..99883e5 100644 --- a/packages/cli/src/tests/core/parser/jsdoc/jsdoc-parser.spec.ts +++ b/packages/cli/src/tests/core/parser/jsdoc/jsdoc-parser.spec.ts @@ -263,6 +263,45 @@ export function processOptions(options: { name: string; age: number; active?: bo expect(result.parameters?.[0].nested?.[2].defaultValue).toBe("true"); }); + it("should parse deeply nested parameter properties (3+ levels)", () => { + const sourceFile = project.createSourceFile( + "test.ts", + ` +/** + * @param {Object} config - Configuration object + * @param {Object} config.database - Database settings + * @param {Object} config.database.connection - Connection settings + * @param {string} config.database.connection.host - Database host + * @param {number} config.database.connection.port - Database port + * @param {string} config.database.name - Database name + */ +export function configure(config: any): void { + // implementation +} + ` + ); + + const func = sourceFile.getFunction("configure")!; + const jsDoc = func.getJsDocs()[0]!; + + const result = parser.parse(jsDoc); + + expect(result.parameters).toMatchObject([ + { + name: "config", + nested: [ + { + name: "database", + nested: [ + { name: "connection", nested: [{ name: "host" }, { name: "port" }] }, + { name: "name" }, + ], + }, + ], + }, + ]); + }); + it("should handle JSDoc without any documentation", () => { const result = parser.parse(null as unknown as JSDoc); diff --git a/packages/cli/src/tests/utils/create-ts-source-file.ts b/packages/cli/src/tests/utils/create-ts-source-file.ts new file mode 100644 index 0000000..17a49c4 --- /dev/null +++ b/packages/cli/src/tests/utils/create-ts-source-file.ts @@ -0,0 +1,6 @@ +import { Project, SourceFile } from 'ts-morph'; + +export function createTSSourceFile(code: string): SourceFile { + const project = new Project({ useInMemoryFileSystem: true }); + return project.createSourceFile("test.ts", code); +} diff --git a/packages/cli/src/tests/utils/parse-jsdoc-from-node.ts b/packages/cli/src/tests/utils/parse-jsdoc-from-node.ts new file mode 100644 index 0000000..6dce446 --- /dev/null +++ b/packages/cli/src/tests/utils/parse-jsdoc-from-node.ts @@ -0,0 +1,20 @@ +import { Node } from "ts-morph"; +import { JSDocParser } from "../../core/parser/jsdoc/jsdoc-parser.js"; +import { getJSDoc } from "../../core/parser/jsdoc/jsdoc-utils.js"; +import { ParsedJSDoc } from "../../core/types/parser.types.js"; + +const EMPTY_PARSED_JSDOC: ParsedJSDoc = { + examples: [], + parameters: [], + throws: [], + typedef: [], + see: [], + version: [], +}; + +export function parseJSDocFromNode(node: Node): ParsedJSDoc { + const jsDocParser = new JSDocParser(); + const jsDoc = getJSDoc(node); + + return jsDoc != null ? jsDocParser.parse(jsDoc) : EMPTY_PARSED_JSDOC; +}