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
8 changes: 8 additions & 0 deletions .changeset/fix-standards-violations.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@kidd-cli/core': patch
'@kidd-cli/cli': patch
---

Fix coding standards violations across packages

Replace `as` type assertions with type annotations, type guards, and documented exceptions. Replace `try/catch` blocks with `attempt`/`attemptAsync` from es-toolkit. Replace multi-branch `if/else` chains with `ts-pattern` `match` expressions. Rename `redactPaths` constant to `REDACT_PATHS`. Document intentional mutation and `throw` exceptions with inline comments.
19 changes: 6 additions & 13 deletions packages/cli/src/commands/doctor.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { loadConfig } from '@kidd-cli/config/loader'
import { command } from '@kidd-cli/core'
import type { Command, Context } from '@kidd-cli/core'
import { match } from '@kidd-cli/utils/fp'
import { readManifest } from '@kidd-cli/utils/manifest'
import pc from 'picocolors'
import { z } from 'zod'
Expand Down Expand Up @@ -216,19 +217,11 @@ function formatResultLine(
* @returns A colored string representation of the status.
*/
function formatDisplayStatus(status: CheckStatus | 'fix'): string {
if (status === 'pass') {
return pc.green('pass')
}

if (status === 'warn') {
return pc.yellow('warn')
}

if (status === 'fix') {
return pc.blue('fix ')
}

return pc.red('fail')
return match(status)
.with('pass', () => pc.green('pass'))
.with('warn', () => pc.yellow('warn'))
.with('fix', () => pc.blue('fix '))
.otherwise(() => pc.red('fail'))
}

/**
Expand Down
11 changes: 8 additions & 3 deletions packages/core/src/autoloader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ export async function autoload(options?: AutoloadOptions): Promise<CommandMap> {
const allResults = [...fileResults, ...dirResults]
const validPairs = allResults.filter((pair): pair is [string, Command] => pair !== undefined)

return Object.fromEntries(validPairs) as CommandMap
const commandMap: CommandMap = Object.fromEntries(validPairs)
return commandMap
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -121,7 +122,8 @@ async function buildSubCommands(dir: string, entries: Dirent[]): Promise<Command
const allResults = [...fileResults, ...dirResults]
const validPairs = allResults.filter((pair): pair is [string, Command] => pair !== undefined)

return Object.fromEntries(validPairs) as CommandMap
const commandMap: CommandMap = Object.fromEntries(validPairs)
return commandMap
}

/**
Expand Down Expand Up @@ -170,7 +172,10 @@ function isCommandExport(mod: unknown): mod is { default: Command } {
if (typeof mod !== 'object' || mod === null) {
return false
}
const def = (mod as Record<string, unknown>)['default']
if (!('default' in mod)) {
return false
}
const def: unknown = mod.default
if (!isPlainObject(def)) {
return false
}
Expand Down
27 changes: 12 additions & 15 deletions packages/core/src/cli.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { resolve } from 'node:path'

import { loadConfig } from '@kidd-cli/config/loader'
import { attemptAsync, isPlainObject, isString } from '@kidd-cli/utils/fp'
import { P, attemptAsync, isPlainObject, isString, match } from '@kidd-cli/utils/fp'
import yargs from 'yargs'
import type { z } from 'zod'

Expand Down Expand Up @@ -53,9 +53,9 @@ export async function cli<TSchema extends z.ZodType = z.ZodType>(
program.demandCommand(1, 'You must specify a command.')
}

const argv = await program.parseAsync()
const argv: Record<string, unknown> = await program.parseAsync()

applyCwd(argv as Record<string, unknown>)
applyCwd(argv)

if (!resolved.ref) {
return undefined
Expand All @@ -77,7 +77,7 @@ export async function cli<TSchema extends z.ZodType = z.ZodType>(
commandPath: resolved.ref.commandPath,
handler: resolved.ref.handler,
middleware: resolved.ref.middleware,
rawArgs: argv as Record<string, unknown>,
rawArgs: argv,
})

return executeError
Expand Down Expand Up @@ -121,7 +121,7 @@ async function resolveCommands(
return commands
}
if (isPlainObject(commands)) {
return commands as CommandMap
return commands
}
return resolveCommandsFromConfig()
}
Expand Down Expand Up @@ -172,14 +172,11 @@ function applyCwd(argv: Record<string, unknown>): void {
* @param logger - Logger with an error method for output.
*/
function exitOnError(error: unknown, logger: { error(msg: string): void }): void {
if (isContextError(error)) {
logger.error(error.message)
process.exit(error.exitCode)
} else if (error instanceof Error) {
logger.error(error.message)
process.exit(DEFAULT_EXIT_CODE)
} else {
logger.error(String(error))
process.exit(DEFAULT_EXIT_CODE)
}
const info = match(error)
.when(isContextError, (e) => ({ exitCode: e.exitCode, message: e.message }))
.with(P.instanceOf(Error), (e) => ({ exitCode: DEFAULT_EXIT_CODE, message: e.message }))
.otherwise((e) => ({ exitCode: DEFAULT_EXIT_CODE, message: String(e) }))

logger.error(info.message)
process.exit(info.exitCode)
}
2 changes: 2 additions & 0 deletions packages/core/src/context/create-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ export function createContext<TArgs extends AnyRecord, TConfig extends AnyRecord
args: options.args as Context<TArgs, TConfig>['args'],
config: options.config as Context<TArgs, TConfig>['config'],
fail(message: string, failOptions?: { code?: string; exitCode?: number }): never {
// Accepted exception: ctx.fail() is typed `never` and caught by the CLI boundary.
// This is the framework's halt mechanism — the runner catches the thrown ContextError.
throw createContextError(message, failOptions)
},
logger: ctxLogger,
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/context/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,12 @@ export function createContextError(
options?: { code?: string; exitCode?: number }
): ContextError {
const data = createContextErrorData(message, options)
// Accepted exception: Error construction requires `new Error()` then property decoration.
// The `as` cast and Object.defineProperty mutations are the only way to produce a
// Tagged Error subtype without using a class.
const error = new Error(data.message) as ContextError
error.name = 'ContextError'
// Intentional mutation: decorating an Error object with immutable properties.
Object.defineProperty(error, TAG, { enumerable: false, value: 'ContextError', writable: false })
Object.defineProperty(error, 'code', { enumerable: true, value: data.code, writable: false })
Object.defineProperty(error, 'exitCode', {
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/context/prompts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ export function createContextPrompts(): Prompts {
function unwrapCancelSignal<Type>(result: Type | symbol): Type {
if (clack.isCancel(result)) {
clack.cancel('Operation cancelled.')
// Accepted exception: prompt cancellation must propagate as an unwind.
// The runner catches the thrown ContextError at the CLI boundary.
throw createContextError('Prompt cancelled by user', {
code: 'PROMPT_CANCELLED',
exitCode: DEFAULT_EXIT_CODE,
Expand Down
26 changes: 20 additions & 6 deletions packages/core/src/context/redact.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const SPECIFIC_PATHS: readonly string[] = [
'env.LINEAR_API_KEY',
]

const redactPaths = pinoRedact({
const REDACT_PATHS = pinoRedact({
censor: resolveCensor,
paths: [...SPECIFIC_PATHS],
serialize: false,
Expand Down Expand Up @@ -64,7 +64,10 @@ function resolveCensor(value: unknown): unknown {
* @returns A deep clone with sensitive values replaced.
*/
export function redactObject<TObj extends object>(obj: TObj): TObj {
const pathRedacted = redactPaths(obj)
// Accepted exception: pinoRedact returns unknown and the recursive walk
// Requires Record<string, unknown>. TObj is preserved for the caller.
// eslint-disable-next-line new-cap -- REDACT_PATHS is a function from pinoRedact, not a constructor
const pathRedacted = REDACT_PATHS(obj)
return redactSensitiveKeys(pathRedacted as Record<string, unknown>) as TObj
}

Expand Down Expand Up @@ -101,8 +104,8 @@ function redactEntry(key: string, value: unknown): [string, unknown] {
return [key, value.map(redactArrayItem)]
}

if (value && typeof value === 'object') {
return [key, redactSensitiveKeys(value as Record<string, unknown>)]
if (isNonNullObject(value)) {
return [key, redactSensitiveKeys(value)]
}

return [key, value]
Expand All @@ -116,8 +119,19 @@ function redactEntry(key: string, value: unknown): [string, unknown] {
* @returns The element with sensitive keys redacted, or the original primitive.
*/
function redactArrayItem(item: unknown): unknown {
if (item && typeof item === 'object') {
return redactSensitiveKeys(item as Record<string, unknown>)
if (isNonNullObject(item)) {
return redactSensitiveKeys(item)
}
return item
}

/**
* Type guard that narrows an unknown value to a string-keyed record.
*
* @private
* @param value - The value to check.
* @returns True when the value is a non-null object.
*/
function isNonNullObject(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value)
}
2 changes: 2 additions & 0 deletions packages/core/src/lib/config/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ function parseJsoncContent(
content: string,
filePath: string
): ConfigOperationResult<unknown> {
// Intentional mutation: jsonc-parser API requires a mutable errors array.
// There is no immutable alternative — the parser populates it during parsing.
const errors: ParseError[] = []
const result = parseJsonc(content, errors, {
allowEmptyContent: false,
Expand Down
12 changes: 5 additions & 7 deletions packages/core/src/lib/project/root.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,11 @@ export function findProjectRoot(startDir: string = process.cwd()): ProjectRoot |
const nextVisited = new Set([...visited, currentDir])

const gitPath = join(currentDir, '.git')
try {
const result = checkGitPath(gitPath, currentDir)
if (result) {
return result
}
} catch {
// Race condition: file may have been deleted between existsSync and statSync
// Race condition: file may have been deleted between existsSync and statSync
const [checkError, result] = attempt(() => checkGitPath(gitPath, currentDir))

if (!checkError && result) {
return result
}

const parent = dirname(currentDir)
Expand Down
12 changes: 9 additions & 3 deletions packages/core/src/middleware/auth/credential.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { attemptAsync } from '@kidd-cli/utils/fp'

import type { BearerCredential } from './types.js'

/**
Expand Down Expand Up @@ -49,14 +51,18 @@ export async function postFormEncoded(
params: URLSearchParams,
signal?: AbortSignal
): Promise<Response | null> {
try {
return await fetch(url, {
const [fetchError, response] = await attemptAsync(() =>
fetch(url, {
body: params.toString(),
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
method: 'POST',
signal,
})
} catch {
)

if (fetchError) {
return null
}

return response
}
31 changes: 16 additions & 15 deletions packages/core/src/middleware/auth/oauth-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { IncomingMessage, Server, ServerResponse } from 'node:http'
import type { Socket } from 'node:net'
import { platform } from 'node:os'

import { match } from 'ts-pattern'
import { attempt, match } from '@kidd-cli/utils/fp'

// ---------------------------------------------------------------------------
// Constants
Expand Down Expand Up @@ -170,21 +170,21 @@ export function sendSuccessPage(res: ServerResponse): void {
* @returns True when the URL uses HTTPS or HTTP on a loopback address.
*/
export function isSecureAuthUrl(url: string): boolean {
try {
const parsed = new URL(url)
const [error, parsed] = attempt(() => new URL(url))

if (parsed.protocol === 'https:') {
return true
}
if (error || !parsed) {
return false
}

if (parsed.protocol !== 'http:') {
return false
}
if (parsed.protocol === 'https:') {
return true
}

return isLoopbackHost(parsed.hostname)
} catch {
if (parsed.protocol !== 'http:') {
return false
}

return isLoopbackHost(parsed.hostname)
}

/**
Expand Down Expand Up @@ -276,12 +276,13 @@ export function startLocalServer(options: {
* @returns True when the URL uses http: or https: protocol.
*/
function isHttpUrl(url: string): boolean {
try {
const parsed = new URL(url)
return parsed.protocol === 'https:' || parsed.protocol === 'http:'
} catch {
const [error, parsed] = attempt(() => new URL(url))

if (error || !parsed) {
return false
}

return parsed.protocol === 'https:' || parsed.protocol === 'http:'
}

/**
Expand Down
Loading
Loading