From 0dfb456b339b109d6654236da243c3a957795c9a Mon Sep 17 00:00:00 2001 From: Mikita Taukachou Date: Sun, 29 Mar 2026 19:09:32 +0200 Subject: [PATCH] refactor: clean up public API surface #24 Added `RollParserError` base class with typed `RollParserErrorCode` union and `isRollParserError()` type guard derived from a single `as const` array source of truth. Refactored `LexerError`, `ParseError`, and `EvaluatorError` to extend `RollParserError` with error codes and structured context (`nodeType` for evaluator errors). Narrowed `src/index.ts` exports by removing `Lexer`, `Parser`, `lex`, `TokenType`, `Token`, and mock RNG re-exports while keeping AST types required by `parse()` return type. Created `roll-parser/testing` subpath export with `createMockRng` and `MockRNGExhaustedError`, including dedicated ESM/CJS build scripts. Added bounds validation to `MockRNG.nextInt` that throws `RangeError` for out-of-range values. Replaced hardcoded `VERSION` constant with `package.json` import, migrated `bun-types` to `@types/bun`, and aligned line width to 100 chars. Co-Authored-By: Claude Opus 4.6 (1M context) --- .claude/rules/comments.md | 22 +++++-------- .claude/rules/rng.md | 3 ++ .claude/rules/testing.md | 2 ++ bun.lock | 6 ++-- package.json | 14 +++++--- src/cli/index.ts | 10 ++---- src/errors.ts | 67 ++++++++++++++++++++++++++++++++++++++ src/evaluator/evaluator.ts | 35 ++++++++++++++------ src/index.ts | 14 +++++--- src/lexer/lexer.ts | 21 +++++++----- src/parser/parser.ts | 56 +++++++++++++++++++++++-------- src/rng/index.ts | 1 - src/rng/mock.ts | 8 ++++- src/rng/rng.test.ts | 12 ++++++- src/testing.ts | 16 +++++++++ tsconfig.json | 2 +- 16 files changed, 220 insertions(+), 69 deletions(-) create mode 100644 src/errors.ts create mode 100644 src/testing.ts diff --git a/.claude/rules/comments.md b/.claude/rules/comments.md index ae366b7..f36c0a1 100644 --- a/.claude/rules/comments.md +++ b/.claude/rules/comments.md @@ -18,8 +18,16 @@ ```ts // - // * Parser Utils + // * Event Handlers // + + /* ... */ + + // + // * Validators + // + + /* ... */ ``` - `// TODO: ` — actionable future work; start with an imperative verb, reference issue if possible @@ -37,18 +45,6 @@ - Avoid commenting trivial code (obvious mappings, simple getters). - Prefer JSDoc/TSDoc for public API functions instead of inline prose. - Keep comments inside function bodies minimal — context belongs in tests or docs. -- Lines ≤ 80 characters, no emojis, complete sentences starting with a capital letter. - -```ts -function binarySearch(haystack: number[], needle: number): number { - // Guard against unsorted input arrays - if (!isSorted(haystack)) { - // ! Sorting here would hide the caller's bug - throw new Error('Input must be pre-sorted'); - } - // Standard binary-search implementation… -} -``` ## Maintenance diff --git a/.claude/rules/rng.md b/.claude/rules/rng.md index bfd7f48..3f4d808 100644 --- a/.claude/rules/rng.md +++ b/.claude/rules/rng.md @@ -29,12 +29,15 @@ mock.nextInt(1, 6); // Throws! (sequence exhausted) ``` `MockRNG` MUST throw on exhaustion — this catches incorrect roll counts in tests. +`MockRNG.nextInt` validates that returned values fall within `[min, max]` — throws `RangeError` if not. ## Usage in Tests ```typescript import { describe, it, expect } from 'bun:test'; +// Internal tests use relative imports: import { createMockRng } from '@/rng/mock'; +// npm consumers use: import { createMockRng } from 'roll-parser/testing'; describe('roll', () => { it('should roll exact values with MockRNG', () => { diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 0ff1987..2f3e468 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -21,7 +21,9 @@ describe('evaluate', () => { Use `createMockRng` for all deterministic dice tests. See `rng.md`. ```typescript +// Internal tests use relative imports: import { createMockRng } from '../rng/mock'; +// npm consumers use: import { createMockRng } from 'roll-parser/testing'; test('keeps highest die from pool', () => { const ast = parse('2d20kh1'); diff --git a/bun.lock b/bun.lock index b3e6ed3..6d28624 100644 --- a/bun.lock +++ b/bun.lock @@ -6,7 +6,7 @@ "name": "roll-parser", "devDependencies": { "@biomejs/biome": "^1.9.4", - "bun-types": "latest", + "@types/bun": "^1.2.0", "fast-check": "^3.23.2", "typescript": "^5.7.2", }, @@ -31,9 +31,11 @@ "@biomejs/cli-win32-x64": ["@biomejs/cli-win32-x64@1.9.4", "", { "os": "win32", "cpu": "x64" }, "sha512-8Y5wMhVIPaWe6jw2H+KlEm4wP/f7EW3810ZLmDlrEEy5KvBsb9ECEfu/kMWD484ijfQ8+nIi0giMgu9g1UAuuA=="], + "@types/bun": ["@types/bun@1.3.11", "", { "dependencies": { "bun-types": "1.3.11" } }, "sha512-5vPne5QvtpjGpsGYXiFyycfpDF2ECyPcTSsFBMa0fraoxiQyMJ3SmuQIGhzPg2WJuWxVBoxWJ2kClYTcw/4fAg=="], + "@types/node": ["@types/node@25.0.3", "", { "dependencies": { "undici-types": "~7.16.0" } }, "sha512-W609buLVRVmeW693xKfzHeIV6nJGGz98uCPfeXI1ELMLXVeKYZ9m15fAMSaUPBHYLGFsVRcMmSCksQOrZV9BYA=="], - "bun-types": ["bun-types@1.3.5", "", { "dependencies": { "@types/node": "*" } }, "sha512-inmAYe2PFLs0SUbFOWSVD24sg1jFlMPxOjOSSCYqUgn4Hsc3rDc7dFvfVYjFPNHtov6kgUeulV4SxbuIV/stPw=="], + "bun-types": ["bun-types@1.3.11", "", { "dependencies": { "@types/node": "*" } }, "sha512-1KGPpoxQWl9f6wcZh57LvrPIInQMn2TQ7jsgxqpRzg+l0QPOFvJVH7HmvHo/AiPgwXy+/Thf6Ov3EdVn1vOabg=="], "fast-check": ["fast-check@3.23.2", "", { "dependencies": { "pure-rand": "^6.1.0" } }, "sha512-h5+1OzzfCC3Ef7VbtKdcv7zsstUQwUDlYpUTvjeUsJAssPgLn7QzbboPtL5ro04Mq0rPOsMzl7q5hIbRs2wD1A=="], diff --git a/package.json b/package.json index fda6d98..b6113c8 100644 --- a/package.json +++ b/package.json @@ -13,13 +13,17 @@ "types": "./dist/index.d.ts", "import": "./dist/index.mjs", "require": "./dist/index.js" + }, + "./testing": { + "types": "./dist/testing.d.ts", + "import": "./dist/testing.mjs", + "require": "./dist/testing.js" } }, "files": [ "dist", "src", - "!src/**/*.test.ts", - "!src/rng/mock.ts" + "!src/**/*.test.ts" ], "keywords": [ "parser", @@ -51,9 +55,11 @@ "check": "bun typecheck && bun lint && bun format:check", "check:fix": "bun typecheck && bun lint:fix && bun format", "clean": "rm -rf dist coverage", - "build": "bun run clean && bun run build:esm && bun run build:cjs && bun run build:cli && bun run build:types", + "build": "bun run clean && bun run build:esm && bun run build:cjs && bun run build:esm:testing && bun run build:cjs:testing && bun run build:cli && bun run build:types", "build:esm": "bun build src/index.ts --outfile dist/index.mjs --target bun", "build:cjs": "bun build src/index.ts --outfile dist/index.js --target node", + "build:esm:testing": "bun build src/testing.ts --outfile dist/testing.mjs --target bun", + "build:cjs:testing": "bun build src/testing.ts --outfile dist/testing.js --target node", "build:cli": "bun build src/cli/index.ts --outfile dist/cli.js --target node", "build:types": "tsc --emitDeclarationOnly -p tsconfig.build.json", "test": "bun test", @@ -67,7 +73,7 @@ }, "devDependencies": { "@biomejs/biome": "^1.9.4", - "bun-types": "latest", + "@types/bun": "^1.2.0", "fast-check": "^3.23.2", "typescript": "^5.7.2" }, diff --git a/src/cli/index.ts b/src/cli/index.ts index d9fa337..0d26b74 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -6,10 +6,8 @@ * @module cli/index */ -import { EvaluatorError } from '../evaluator/evaluator'; +import { isRollParserError } from '../errors'; import { VERSION } from '../index'; -import { LexerError } from '../lexer/lexer'; -import { ParseError } from '../parser/parser'; import { roll } from '../roll'; import { parseArgs } from './args'; import { formatResult } from './format'; @@ -65,11 +63,7 @@ function main(): void { const output = formatResult(result, args.verbose); process.stdout.write(`${output}\n`); } catch (error) { - if ( - error instanceof LexerError || - error instanceof ParseError || - error instanceof EvaluatorError - ) { + if (isRollParserError(error)) { process.stderr.write(`Error: ${error.message}\n`); process.exitCode = 1; return; diff --git a/src/errors.ts b/src/errors.ts new file mode 100644 index 0000000..109b7ed --- /dev/null +++ b/src/errors.ts @@ -0,0 +1,67 @@ +/** + * Common error base class and error codes for roll-parser. + * + * @module errors + */ + +/** + * All known roll-parser error codes. Single source of truth — the + * `RollParserErrorCode` type and the runtime `VALID_CODES` set are + * both derived from this array. + * + * Lexer: `UNEXPECTED_CHARACTER`, `UNEXPECTED_IDENTIFIER` + * Parser: `UNEXPECTED_TOKEN`, `UNEXPECTED_END`, `EXPECTED_TOKEN` + * Evaluator: `INVALID_DICE_COUNT`, `INVALID_DICE_SIDES`, `DICE_LIMIT_EXCEEDED`, + * `DIVISION_BY_ZERO`, `MODULO_BY_ZERO`, `UNKNOWN_OPERATOR`, `UNKNOWN_NODE_TYPE`, + * `INVALID_MODIFIER_COUNT` + */ +const ROLL_PARSER_ERROR_CODES = [ + 'UNEXPECTED_CHARACTER', + 'UNEXPECTED_IDENTIFIER', + 'UNEXPECTED_TOKEN', + 'UNEXPECTED_END', + 'EXPECTED_TOKEN', + 'INVALID_DICE_COUNT', + 'INVALID_DICE_SIDES', + 'DICE_LIMIT_EXCEEDED', + 'DIVISION_BY_ZERO', + 'MODULO_BY_ZERO', + 'UNKNOWN_OPERATOR', + 'UNKNOWN_NODE_TYPE', + 'INVALID_MODIFIER_COUNT', +] as const; + +export type RollParserErrorCode = (typeof ROLL_PARSER_ERROR_CODES)[number]; + +/** + * Base error class for all roll-parser errors. + * + * Provides a typed `code` field for programmatic error handling. + * All library errors (`LexerError`, `ParseError`, `EvaluatorError`) + * extend this class. + */ +export class RollParserError extends Error { + readonly code: RollParserErrorCode; + + constructor(message: string, code: RollParserErrorCode) { + super(message); + this.name = 'RollParserError'; + this.code = code; + } +} + +const VALID_CODES: Set = new Set(ROLL_PARSER_ERROR_CODES); + +/** + * Type guard for roll-parser errors. Checks `instanceof` first, then + * falls back to duck-typing for cross-realm safety. + */ +export function isRollParserError(value: unknown): value is RollParserError { + if (value instanceof RollParserError) return true; + return ( + value instanceof Error && + 'code' in value && + typeof (value as RollParserError).code === 'string' && + VALID_CODES.has((value as RollParserError).code) + ); +} diff --git a/src/evaluator/evaluator.ts b/src/evaluator/evaluator.ts index 467edd9..4977b5d 100644 --- a/src/evaluator/evaluator.ts +++ b/src/evaluator/evaluator.ts @@ -4,6 +4,8 @@ * @module evaluator/evaluator */ +import type { RollParserErrorCode } from '../errors'; +import { RollParserError } from '../errors'; import type { ASTNode, BinaryOpNode, DiceNode, ModifierNode, UnaryOpNode } from '../parser/ast'; import { isModifier } from '../parser/ast'; import type { RNG } from '../rng/types'; @@ -20,10 +22,13 @@ import { /** * Error thrown during AST evaluation. */ -export class EvaluatorError extends Error { - constructor(message: string) { - super(message); +export class EvaluatorError extends RollParserError { + readonly nodeType: string | undefined; + + constructor(message: string, code: RollParserErrorCode, nodeType?: string) { + super(message, code); this.name = 'EvaluatorError'; + this.nodeType = nodeType ?? undefined; } } @@ -105,7 +110,11 @@ function evalNode(node: ASTNode, rng: RNG, ctx: EvalContext, env: EvalEnv): numb default: { const exhaustive: never = node; - throw new EvaluatorError(`Unknown node type: ${(exhaustive as ASTNode).type}`); + throw new EvaluatorError( + `Unknown node type: ${(exhaustive as ASTNode).type}`, + 'UNKNOWN_NODE_TYPE', + (exhaustive as ASTNode).type, + ); } } } @@ -131,15 +140,17 @@ function evalDice(node: DiceNode, rng: RNG, ctx: EvalContext, env: EvalEnv): num ); if (!Number.isInteger(count) || count < 0) { - throw new EvaluatorError(`Invalid dice count: ${count}`); + throw new EvaluatorError(`Invalid dice count: ${count}`, 'INVALID_DICE_COUNT', 'Dice'); } if (!Number.isInteger(sides) || sides < 1) { - throw new EvaluatorError(`Invalid dice sides: ${sides}`); + throw new EvaluatorError(`Invalid dice sides: ${sides}`, 'INVALID_DICE_SIDES', 'Dice'); } if (env.totalDiceRolled + count > env.maxDice) { throw new EvaluatorError( `Total dice count ${env.totalDiceRolled + count} exceeds limit of ${env.maxDice}`, + 'DICE_LIMIT_EXCEEDED', + 'Dice', ); } env.totalDiceRolled += count; @@ -188,19 +199,19 @@ function evalBinaryOp(node: BinaryOpNode, rng: RNG, ctx: EvalContext, env: EvalE return left * right; case '/': if (right === 0) { - throw new EvaluatorError('Division by zero'); + throw new EvaluatorError('Division by zero', 'DIVISION_BY_ZERO', 'BinaryOp'); } return left / right; case '%': if (right === 0) { - throw new EvaluatorError('Modulo by zero'); + throw new EvaluatorError('Modulo by zero', 'MODULO_BY_ZERO', 'BinaryOp'); } return left % right; case '**': return left ** right; default: { const exhaustive: never = node.operator; - throw new EvaluatorError(`Unknown operator: ${exhaustive}`); + throw new EvaluatorError(`Unknown operator: ${exhaustive}`, 'UNKNOWN_OPERATOR', 'BinaryOp'); } } } @@ -237,7 +248,11 @@ function flattenModifierChain( const modCount = evalNode(current.count, rng, countCtx, env); if (!Number.isInteger(modCount) || modCount < 0) { - throw new EvaluatorError(`Invalid modifier count: ${modCount}`); + throw new EvaluatorError( + `Invalid modifier count: ${modCount}`, + 'INVALID_MODIFIER_COUNT', + 'Modifier', + ); } const code = diff --git a/src/index.ts b/src/index.ts index f7b8e93..57e068a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,12 +4,15 @@ * @module roll-parser */ +// * Error hierarchy +export { RollParserError, isRollParserError } from './errors'; +export type { RollParserErrorCode } from './errors'; + // * Lexer exports -export { lex, Lexer, LexerError } from './lexer/lexer'; -export { TokenType, type Token } from './lexer/tokens'; +export { LexerError } from './lexer/lexer'; // * Parser exports -export { parse, Parser, ParseError } from './parser/parser'; +export { parse, ParseError } from './parser/parser'; export type { ASTNode, BinaryOpNode, @@ -29,7 +32,6 @@ export { // * RNG exports export type { RNG } from './rng/types'; export { SeededRNG } from './rng/seeded'; -export { createMockRng, MockRNGExhaustedError } from './rng/mock'; // * Evaluator exports export { DEFAULT_MAX_DICE, evaluate, EvaluatorError } from './evaluator/evaluator'; @@ -39,4 +41,6 @@ export type { DieModifier, DieResult, EvaluateOptions, RollResult } from './type export { roll } from './roll'; export type { RollOptions } from './roll'; -export const VERSION = '3.0.0-alpha.0'; +import pkg from '../package.json'; + +export const VERSION: string = pkg.version; diff --git a/src/lexer/lexer.ts b/src/lexer/lexer.ts index 7d148f2..683f0d1 100644 --- a/src/lexer/lexer.ts +++ b/src/lexer/lexer.ts @@ -4,19 +4,22 @@ * @module lexer/lexer */ +import type { RollParserErrorCode } from '../errors'; +import { RollParserError } from '../errors'; import { type Token, TokenType } from './tokens'; /** * Error thrown when the lexer encounters an invalid character. */ -export class LexerError extends Error { - constructor( - message: string, - public readonly position: number, - public readonly character: string, - ) { - super(`${message} at position ${position}: '${character}'`); +export class LexerError extends RollParserError { + readonly position: number; + readonly character: string; + + constructor(message: string, code: RollParserErrorCode, position: number, character: string) { + super(`${message} at position ${position}: '${character}'`, code); this.name = 'LexerError'; + this.position = position; + this.character = character; } } @@ -96,7 +99,7 @@ export class Lexer { case ')': return this.createTokenAt(TokenType.RPAREN, char, startPos); default: - throw new LexerError('Unexpected character', startPos, char); + throw new LexerError('Unexpected character', 'UNEXPECTED_CHARACTER', startPos, char); } } @@ -164,7 +167,7 @@ export class Lexer { return this.createTokenAt(TokenType.KEEP_HIGH, char, startPos); } - throw new LexerError('Unexpected identifier', startPos, char); + throw new LexerError('Unexpected identifier', 'UNEXPECTED_IDENTIFIER', startPos, char); } private peek(): string { diff --git a/src/parser/parser.ts b/src/parser/parser.ts index ebd0921..f28f677 100644 --- a/src/parser/parser.ts +++ b/src/parser/parser.ts @@ -4,6 +4,8 @@ * @module parser/parser */ +import type { RollParserErrorCode } from '../errors'; +import { RollParserError } from '../errors'; import { lex } from '../lexer/lexer'; import { type Token, TokenType } from '../lexer/tokens'; import type { @@ -18,14 +20,15 @@ import type { /** * Error thrown when the parser encounters invalid syntax. */ -export class ParseError extends Error { - constructor( - message: string, - public readonly position: number, - public readonly token?: Token, - ) { - super(`${message} at position ${position}`); +export class ParseError extends RollParserError { + readonly position: number; + readonly token: Token | undefined; + + constructor(message: string, code: RollParserErrorCode, position: number, token?: Token) { + super(`${message} at position ${position}`, code); this.name = 'ParseError'; + this.position = position; + this.token = token ?? undefined; } } @@ -79,7 +82,7 @@ export class Parser { */ parse(): ASTNode { if (this.peek().type === TokenType.EOF) { - throw new ParseError('Unexpected end of input', this.peek().position); + throw new ParseError('Unexpected end of input', 'UNEXPECTED_END', this.peek().position); } const ast = this.parseExpression(0); @@ -87,7 +90,12 @@ export class Parser { // Ensure we consumed all tokens if (this.peek().type !== TokenType.EOF) { const token = this.peek(); - throw new ParseError(`Unexpected token '${token.value}'`, token.position, token); + throw new ParseError( + `Unexpected token '${token.value}'`, + 'UNEXPECTED_TOKEN', + token.position, + token, + ); } return ast; @@ -133,10 +141,15 @@ export class Parser { return this.parseGrouped(); case TokenType.EOF: - throw new ParseError('Unexpected end of input', token.position); + throw new ParseError('Unexpected end of input', 'UNEXPECTED_END', token.position); default: - throw new ParseError(`Unexpected token '${token.value}'`, token.position, token); + throw new ParseError( + `Unexpected token '${token.value}'`, + 'UNEXPECTED_TOKEN', + token.position, + token, + ); } } @@ -164,7 +177,12 @@ export class Parser { return this.parseModifier(left, token); default: - throw new ParseError(`Unexpected infix token '${token.value}'`, token.position, token); + throw new ParseError( + `Unexpected infix token '${token.value}'`, + 'UNEXPECTED_TOKEN', + token.position, + token, + ); } } @@ -267,7 +285,12 @@ export class Parser { case TokenType.POWER: return '**'; default: - throw new ParseError(`Unknown operator '${token.value}'`, token.position, token); + throw new ParseError( + `Unknown operator '${token.value}'`, + 'UNEXPECTED_TOKEN', + token.position, + token, + ); } } @@ -330,7 +353,12 @@ export class Parser { const token = this.peek(); if (token.type !== type) { const expected = TokenType[type]; - throw new ParseError(`Expected ${expected} but got '${token.value}'`, token.position, token); + throw new ParseError( + `Expected ${expected} but got '${token.value}'`, + 'EXPECTED_TOKEN', + token.position, + token, + ); } return this.advance(); } diff --git a/src/rng/index.ts b/src/rng/index.ts index f11bd51..06b0a50 100644 --- a/src/rng/index.ts +++ b/src/rng/index.ts @@ -6,4 +6,3 @@ export type { RNG } from './types'; export { SeededRNG } from './seeded'; -export { createMockRng, MockRNGExhaustedError } from './mock'; diff --git a/src/rng/mock.ts b/src/rng/mock.ts index 91fdf6d..6f8bcbb 100644 --- a/src/rng/mock.ts +++ b/src/rng/mock.ts @@ -54,6 +54,12 @@ export function createMockRng(values: number[]): RNG { return { next: getNext, - nextInt: (_min: number, _max: number): number => getNext(), + nextInt: (min: number, max: number): number => { + const value = getNext(); + if (value < min || value > max) { + throw new RangeError(`MockRNG value ${value} is out of bounds [${min}, ${max}]`); + } + return value; + }, }; } diff --git a/src/rng/rng.test.ts b/src/rng/rng.test.ts index 7f6afca..ddf378f 100644 --- a/src/rng/rng.test.ts +++ b/src/rng/rng.test.ts @@ -461,7 +461,7 @@ describe('MockRNG', () => { expect(rng.next()).toBe(0.75); }); - it('should ignore min/max parameters (returns raw values)', () => { + it('should accept values within bounds', () => { const rng = createMockRng([4, 15, 20]); expect(rng.nextInt(1, 6)).toBe(4); @@ -469,6 +469,16 @@ describe('MockRNG', () => { expect(rng.nextInt(1, 100)).toBe(20); }); + it('should throw RangeError when value is out of bounds', () => { + const rng = createMockRng([100]); + + expect(() => rng.nextInt(1, 6)).toThrow(RangeError); + + const rng2 = createMockRng([0]); + + expect(() => rng2.nextInt(1, 6)).toThrow('out of bounds'); + }); + it('should allow mixing next and nextInt calls', () => { const rng = createMockRng([0.5, 4, 0.75, 6]); diff --git a/src/testing.ts b/src/testing.ts new file mode 100644 index 0000000..b94c0b2 --- /dev/null +++ b/src/testing.ts @@ -0,0 +1,16 @@ +/** + * Test utilities for roll-parser consumers. + * + * Import from `roll-parser/testing` for deterministic dice testing. + * + * @module testing + */ + +// Direct value exports force the bundler to inline the code +import { + MockRNGExhaustedError as _MockRNGExhaustedError, + createMockRng as _createMockRng, +} from './rng/mock'; + +export const createMockRng = _createMockRng; +export const MockRNGExhaustedError = _MockRNGExhaustedError; diff --git a/tsconfig.json b/tsconfig.json index 1118ffa..f0ee192 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -5,7 +5,7 @@ "module": "ESNext", "moduleResolution": "bundler", "lib": ["ES2022"], - "types": ["bun-types"], + "types": ["@types/bun"], "strict": true, "noEmit": true,