chore: enforce lint, cleanup, cut v1.0.0#99
Conversation
The package.json `imports` map referenced `./dist/_*/*.js` for
subpath imports. dist is built with relative paths (no `#util/*`
references survive bundling), and `bun test` resolves `#util/*`
via tsconfig paths, so the map was a no-op for both consumers and
local dev.
The Validator type previously used a conditional intersection
that exposed `parse: undefined` on the unparametrized form. That
forced consumers (and the parse-test discovery loop) to fall back
to ad-hoc `(value as { parse?: unknown }).parse` casts. Move
`parse?` onto the base shape with a widened
`ParsedIdentifier | null` return; the conditional intersection
still narrows it to the precise `TParsed` for typed validators.
Replace 127 non-null assertions across validators and scripts with either narrowed accessors or `// SAFETY:` comments paired with a local eslint disable. The most common patterns: - Weighted-sum loops `for (let i = 0; i < weights.length; i++)` rewritten to `for (const [i, weight] of weights.entries())` so the weight is intrinsically narrowed. - `arr[randomInt(0, arr.length - 1)]` and `str[randomInt(...)]` routed through new `randomPick(arr)` and `randomChar(str)` helpers in `_util/generate`, which throw on empty input instead of returning `undefined`. - Length-checked accesses (`v[0]!` after `v.length === N`) and required regex captures (`match[1]!` after `if (match)`) keep the assertion but pick up an explicit `// SAFETY:` comment so the rule can be enabled globally without losing the audit trail. Behavior is preserved: 4228 tests still pass.
The lint rule was silently disabled in `.oxlintrc.json`, so `bun run lint` reported clean while 127 forbidden assertions sat in source. Flip the rule to `error`, run `lint` with `--deny-warnings`, add a `format:check` script, and run both `bun run lint` and `bun run format:check` in CI so the gates fail loudly on regressions. oxfmt also touched 22 source/markdown files that had drifted from the configured style. Those are reformatting-only changes.
The npm registry pins `^0.0.1` to exactly `0.0.1` (a long-standing semver quirk for major 0/minor 0 ranges). Anyone who installed the package at `^0.0.1` is stuck on the very first publish, even though the codebase has had public-facing changes since. Skipping straight to 1.0.0 frees consumers from that lock and gives later releases a normal semver runway. No public-API breakage compared to the pre-1.0 line; the changes described in CHANGELOG are improvements on top of the same surface.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request bumps the project version to 1.0.0 and enforces the no-non-null-assertion lint rule. Key changes include refactoring manual loops to use array iterators, adding explicit undefined checks, and introducing utility functions for random selection. The Validator interface was also updated to include an optional parse method. Review feedback identified a potential runtime crash in the Panama RUC validator due to an insufficient length check in the NT branch and noted an inaccurate safety comment regarding buffer length.
| // NT designation (xxNT-xxx-xxx) | ||
| // SAFETY: NT branch requires len === 4 (checked above). | ||
| // eslint-disable-next-line no-non-null-assertion |
There was a problem hiding this comment.
The NT branch in computeDV can crash if segments.length is 3. The safety comment is incorrect because the initial check at the start of the function (line 121) only ensures a length of 3 or 4. If the length is 3 and the second segment is "NT", segments[3] will be undefined, and accessing its length property on line 155 will throw a runtime error. You should explicitly check for the existence of the fourth segment to avoid a runtime error.
| // NT designation (xxNT-xxx-xxx) | |
| // SAFETY: NT branch requires len === 4 (checked above). | |
| // eslint-disable-next-line no-non-null-assertion | |
| const s3 = segments[3]; | |
| if (s3 === undefined) { | |
| return null; | |
| } |
There was a problem hiding this comment.
Fixed in 6045b06. You're right — the early guard at the top of computeDV only rejected len === 4 && segments[1] !== "NT", so a 3-segment input with segments[1] === "NT" slipped through and crashed on segments[3]!.length. Rather than adding a runtime s3 === undefined check inside the NT branch, I tightened the guard to make the NT branch and len === 4 exactly equivalent ((len === 4) !== (segments[1] === "NT") → return null), which lets the existing s3 assertion stay sound and updates the SAFETY note to match. Regression test added in __test__/pa.test.ts ("1-NT-100 DV00" → invalid).
CC on behalf of @jan-kubica
| s1 + | ||
| z(6 - s2.length) + | ||
| s2; | ||
| // SAFETY: juridical branch produces buf of length 20. |
There was a problem hiding this comment.
The comment stating that the juridical branch produces a buffer of length 20 is inaccurate. Based on the construction logic (lines 217-222), the length is typically around 11-12 characters. While index 5 is safe to access because the length is at least 11, the documentation should be corrected to avoid confusion.
| // SAFETY: juridical branch produces buf of length 20. | |
| // SAFETY: juridical branch produces buf of sufficient length (>= 11). |
There was a problem hiding this comment.
Fixed in 6045b06. You're right that "length 20" was wrong for the juridical buf — z(10 - s0.length) + s0 only collapses to 10 chars when s0.length <= 10, otherwise it's longer, and the s1/s2 parts vary the same way. Replaced the comment with the actual invariant used at the access site: s0 is zero-padded to at least 10 chars, so buf[3..6] are always defined regardless of the segment lengths.
CC on behalf of @jan-kubica
The early format guard only rejected 4-segment inputs whose second segment was not "NT", but allowed a 3-segment input with "NT" in position 1 (e.g. "1-NT-100"). The NT branch then accessed `segments[3]` and called `.length` on `undefined`, crashing instead of returning null/INVALID_FORMAT. Tighten the guard so the NT branch and a 4-segment shape are exactly equivalent, drop the misleading "len === 4 (checked above)" SAFETY note in favour of one matching the new guard, and replace the juridical "buf of length 20" SAFETY note with the actual invariant: s0 is zero-padded to at least 10 chars, so buf[3..6] are defined. Add a regression test for the previously-crashing 3-segment NT input.
1 participant
importsmap inpackage.json(built output uses relative paths; dev/test resolution uses tsconfigpaths).parse?on the baseValidatortype so consumers no longer need an unsafe cast to detect parsable validators. GenericValidator<T>still narrows the return type.no-non-null-assertionin.oxlintrc.json(was silentlyoff) and runoxlint --deny-warningsplusoxfmt --checkin CI so both gates fail loudly.for (const [i, w] of WEIGHTS.entries()), random-element pickers routed through newrandomPick/randomCharhelpers, and length-checked or required-capture cases carry an explicit// SAFETY:comment.oxfmt.0.0.1→1.0.0; refreshCHANGELOG.mdandSECURITY.mdsupported-versions accordingly.Test plan
bun run lintcleanbun run format:checkcleanbun run typecheckcleanbun testgreen (4228 tests)CHANGELOG.mdentry for v1.0.0 present,SECURITY.mdupdated to 1.x