fix: resolve all strict TypeScript errors#69
Conversation
- Use `.charAt()` instead of bracket indexing on strings to avoid `string | undefined` return type - Add `?? 0` fallback for readonly tuple/array weight lookups in bounded loops - Add explicit undefined guard for `cy/vat` checkLetter - Fix `de/idnr` digit distribution counter to avoid unchecked indexed increment - Remove unused imports: `randomDigits` (ba/jmbg, ro/cnp), `isdigits` (mu/brn) All 4184 tests pass. The tsconfig already had `strict: true` and `noUncheckedIndexedAccess: true` enabled; these fixes make the codebase actually clean under those settings.
…Returns, useUnknownInCatchVariables)
|
| Filename | Overview |
|---|---|
| src/is/kennitala.ts | Loop converted to v.charAt(i) + ?? 0, but lines 48 and 59 still use v[8]/v[9] bracket notation — passes tsc only because Number() accepts any. |
| src/cy/vat.ts | Mixed fix strategy: checkLetter = v[8] kept as bracket notation with an added === undefined guard, while every other access in the file and PR uses .charAt(). |
| src/de/idnr.ts | Digit distribution counter correctly refactored to avoid unchecked increment — reads prev, guards on undefined, then assigns; logically equivalent and TypeScript-clean. |
| src/ba/jmbg.ts | Removed unused randomDigits import; generate now builds components via randomInt, so the removal is correct. |
| src/ro/cnp.ts | Removed unused randomDigits import; generate already builds each component via randomInt, so removal is correct. |
| src/mu/brn.ts | Removed unused isdigits import; validation uses regex patterns exclusively, so removal is correct. |
| tsconfig.json | Added noImplicitOverride and noImplicitReturns (two undocumented additions beyond the stated scope of the PR); both are sensible strictness flags and the test suite confirms no breakage. |
| src/au/abn.ts | Standard .charAt(i) / ?? 0 fix applied consistently to all weight and digit accesses. |
| src/au/acn.ts | .charAt(i) and ?? 0 applied to both validate and generate loops; check digit comparison also updated. |
| src/es/cif.ts | String bracket accesses (first, last, CIF_LETTERS[check]) replaced with v.charAt() and CIF_LETTERS.charAt(check) consistently. |
| src/es/vat.ts | All five DNI_LETTERS[…] and CIF_LETTERS[…] lookup sites replaced with .charAt(); first and last character reads also updated. |
| src/br/cnpj.ts | Both weight-sum loops and the out-of-loop w2[12] access guarded with ?? 0; .charAt() applied to all digit reads. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["String character access\n(was: v[i] → string | undefined)"] --> B{Access Pattern}
B -->|"Character from string"| C["v.charAt(i)\n→ always string"]
B -->|"Weight from const tuple"| D["WEIGHTS[i] ?? 0\n→ always number"]
B -->|"Letter from string constant"| E["LETTERS.charAt(idx)\n→ always string"]
C --> F["No undefined risk\nPassed to Number()"]
D --> G["No undefined risk\nUsed in arithmetic"]
E --> H["No undefined risk\nUsed in comparison"]
F & G & H --> I["tsc --noEmit passes\nnoUncheckedIndexedAccess satisfied"]
style A fill:#f88,color:#000
style I fill:#8f8,color:#000
Comments Outside Diff (1)
-
src/cy/vat.ts, line 49 (link)The line
const checkLetter = v[8]still uses bracket notation, givingcheckLetterthe typestring | undefined. The PR addressed this by adding an explicit=== undefinedguard in theifcondition below (line 56), which is correct and type-safe. However, this is the only place in the entire PR that uses the "add a guard" strategy — all 20+ other sites converted to.charAt()instead.If you'd prefer full consistency, the straightforward change is:
(and then the
checkLetter === undefinedguard on line 56 could be dropped, sincecharAtalways returns astring). Either approach is fine, but mixing both patterns across the codebase may confuse future readers.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (2): Last reviewed commit: "fix: remove redundant strictNullChecks a..." | Re-trigger Greptile
Both are already implied by strict: true. Keeping them explicitly was misleading, as Greptile flagged in review.
| sum += Number(v.charAt(i)) * (WEIGHTS[i] ?? 0); | ||
| } | ||
| const remainder = (11 - (sum % 11)) % 11; | ||
| if (remainder === 10 || remainder !== Number(v[8])) { |
There was a problem hiding this comment.
Inconsistent bracket-index conversion
The loop immediately above (line 45) was converted to use v.charAt(i), but the two remaining character accesses on lines 48 and 59 still use bracket notation (v[8] / v[9]). Under noUncheckedIndexedAccess these return string | undefined; the only reason they don't produce TypeScript errors here is that they are immediately wrapped in Number(), which is typed as accepting any.
For consistency with every other changed file in this PR—and to avoid silent NaN if the guard is ever removed—they should also be converted:
| if (remainder === 10 || remainder !== Number(v[8])) { | |
| if (remainder === 10 || remainder !== Number(v.charAt(8))) { |
Summary
.charAt()instead of bracket indexing on strings to avoidstring | undefinedundernoUncheckedIndexedAccess?? 0fallback for readonly tuple/array weight lookups in bounded loopsundefinedguard forcy/vatcheckLetterde/idnrdigit distribution counter to avoid unchecked indexed incrementrandomDigitsin ba/jmbg and ro/cnp,isdigitsin mu/brn)The tsconfig already had
strict: trueandnoUncheckedIndexedAccess: trueenabled; these 25 files had latent type errors. Nowtsc --noEmitpasses cleanly.Test plan
tsc --noEmitreports zero errorsbun testpasses all 4184 tests (1 pre-existing failure inde.handelsregunrelated to this change)