From 63ae165652c8d152f3435c92d1d8b2f9f9427d47 Mon Sep 17 00:00:00 2001 From: benjamineckstein <13351939+benjamineckstein@users.noreply.github.com> Date: Wed, 3 Jun 2026 16:23:14 +0200 Subject: [PATCH 1/2] chore: resolve dead-code findings and fix stale fallow suppressions - xml-emit.ts: remove export and stale suppression from emitPstlAdr and emitPstlAdrSEPA (used only within the module); delete the dead emitNmElement function and its JSDoc entirely (never called anywhere) - arbitraries.ts: un-export IBAN_COUNTRIES, SEPA_CHARSET, arbSanitizedSepaText, MAX_AMOUNT_MINOR (all used only within the file) - pain008.ts: un-export FrequencyCodeSchema and MandateSetupReasonSchema (composed internally into MandateAmendmentSchema; not re-exported from index.ts); drop the now-unused FrequencyCode and MandateSetupReason type aliases - generate-snapshots.ts: add correct fallow-ignore-next-line unused-export suppression for writeSnapshots (a script entry point, not a test import) --- src/model/pain008.ts | 11 ++--------- src/writer/xml-emit.ts | 25 ++----------------------- test/arbitraries.ts | 9 ++++----- test/golden/generate-snapshots.ts | 2 ++ 4 files changed, 10 insertions(+), 37 deletions(-) diff --git a/src/model/pain008.ts b/src/model/pain008.ts index 17ce2dd..dc229b0 100644 --- a/src/model/pain008.ts +++ b/src/model/pain008.ts @@ -173,7 +173,7 @@ export type Creditor = z.infer * We support the Tp (code) branch only, which is XSD-valid; the Prd and PtInTm * branches are a documented follow-up. */ -export const FrequencyCodeSchema = z.enum([ +const FrequencyCodeSchema = z.enum([ 'YEAR', 'MNTH', 'QURT', @@ -184,8 +184,6 @@ export const FrequencyCodeSchema = z.enum([ 'INDA', 'FRTN', ]) -export type FrequencyCode = z.infer - /** * Original mandate setup reason (AmdmntInfDtls/OrgnlRsn), MandateSetupReason1Choice: Cd XOR Prtry. * @@ -200,12 +198,7 @@ const MandateReasonProprietarySchema = z.object({ /** Proprietary reason value (Prtry), max 70 chars, SEPA charset. */ proprietary: sepaText(70), }) -export const MandateSetupReasonSchema = z.union([ - MandateReasonCodeSchema, - MandateReasonProprietarySchema, -]) -export type MandateSetupReason = z.infer - +const MandateSetupReasonSchema = z.union([MandateReasonCodeSchema, MandateReasonProprietarySchema]) /** * Original creditor scheme identification (AmdmntInfDtls/OrgnlCdtrSchmeId). * diff --git a/src/writer/xml-emit.ts b/src/writer/xml-emit.ts index bf70744..abefa55 100644 --- a/src/writer/xml-emit.ts +++ b/src/writer/xml-emit.ts @@ -138,12 +138,7 @@ function emitCtryAdrLinesClose(lines: string[], indent: string, address: PostalA * @param indent - leading spaces for the PstlAdr tag (one level deeper than the party tag) * @param address - optional PostalAddress from the model; nothing emitted when absent */ -// fallow-ignore-next-line unused-exports -export function emitPstlAdr( - lines: string[], - indent: string, - address: PostalAddress | undefined -): void { +function emitPstlAdr(lines: string[], indent: string, address: PostalAddress | undefined): void { if (address === undefined) { return } @@ -194,21 +189,6 @@ export function emitPartyWithAddress( lines.push(`${indent}`) } -/** - * Emit a party element containing only a Nm child. - * Used for Dbtr, Cdtr blocks that carry just the name (legacy/DK variants). - * - * @param indent - leading spaces for the outer tag (e.g. " " for 6 spaces) - * @param tag - element name, e.g. "Dbtr" or "Cdtr" - * @param name - party name (will be XML-escaped) - */ -// fallow-ignore-next-line unused-exports -export function emitNmElement(lines: string[], indent: string, tag: string, name: string): void { - lines.push(`${indent}<${tag}>`) - lines.push(`${indent} ${xe(name)}`) - lines.push(`${indent}`) -} - /** * Emit a PstlAdr element for the DK SEPA variants (pain.001.003.03 and pain.008.003.02). * @@ -229,8 +209,7 @@ export function emitNmElement(lines: string[], indent: string, tag: string, name * @param address - optional PostalAddress from the model; nothing emitted when absent * @param variant - variant name used in error messages */ -// fallow-ignore-next-line unused-exports -export function emitPstlAdrSEPA( +function emitPstlAdrSEPA( lines: string[], indent: string, address: PostalAddress | undefined, diff --git a/test/arbitraries.ts b/test/arbitraries.ts index 64bb75f..045d083 100644 --- a/test/arbitraries.ts +++ b/test/arbitraries.ts @@ -22,7 +22,7 @@ import type { PostalAddress } from '../src/model/schema.js' * Countries with BBAN structures that fit [a-zA-Z0-9]{1,30}. * Each entry: [countryCode, bbanLength] where all digits are used (simple). */ -export const IBAN_COUNTRIES: Array<[string, number]> = [ +const IBAN_COUNTRIES: Array<[string, number]> = [ ['DE', 18], ['FR', 23], ['NL', 14], @@ -56,8 +56,7 @@ export function arbIban(): fc.Arbitrary { // SEPA text helpers // --------------------------------------------------------------------------- -export const SEPA_CHARSET = - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 /-?:().,'+" +const SEPA_CHARSET = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 /-?:().,'+" /** Arbitrary for a clean SEPA text string of the given max length. */ export function arbSepaText(minLen: number, maxLen: number): fc.Arbitrary { @@ -92,7 +91,7 @@ export function arbSepaIdentifier(minLen: number, maxLen: number): fc.Arbitrary< * Uses a wider character set including full Latin-1 Supplement, emoji, and * a small CJK sample to stress the drop-and-transliterate path. */ -export function arbSanitizedSepaText(minLen: number, maxLen: number): fc.Arbitrary { +function arbSanitizedSepaText(minLen: number, maxLen: number): fc.Arbitrary { const extendedLatin = 'äöüÄÖÜßàáâãåæèéêëìíîïðñòóôõøùúûýÿÀÁÂÃÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕØÙÚÛÝþÞçÇ' const droppedSamples = '🎉🥳🌍你好مرحباПривет' const mixedCharset = SEPA_CHARSET + extendedLatin + droppedSamples @@ -160,7 +159,7 @@ export function arbBic(): fc.Arbitrary { * EPC AT-06 cap: 999,999,999.99 EUR = 99,999,999,999 cents. * All boundary values must be at or below this cap. */ -export const MAX_AMOUNT_MINOR = 99_999_999_999n +const MAX_AMOUNT_MINOR = 99_999_999_999n export function arbMoney(): fc.Arbitrary<{ currencyCode: 'EUR'; minorUnits: bigint }> { return fc.oneof( diff --git a/test/golden/generate-snapshots.ts b/test/golden/generate-snapshots.ts index 5f9bd04..b5556a2 100644 --- a/test/golden/generate-snapshots.ts +++ b/test/golden/generate-snapshots.ts @@ -154,6 +154,8 @@ export const GOLDEN_DIRECT_DEBIT: DirectDebitDocument = { // Write snapshot files // --------------------------------------------------------------------------- +// Script entry point: invoked directly via node, not imported by any test file. +// fallow-ignore-next-line unused-export export function writeSnapshots(): void { const ctXml = writeCreditTransfer(GOLDEN_CREDIT_TRANSFER) writeFileSync(join(SNAPSHOTS_DIR, 'pain.001.001.09.xml'), ctXml, 'utf-8') From 0a6c05e50a76dc9db691bbc036da4f9399d9abba Mon Sep 17 00:00:00 2001 From: benjamineckstein <13351939+benjamineckstein@users.noreply.github.com> Date: Wed, 3 Jun 2026 16:26:38 +0200 Subject: [PATCH 2/2] refactor(validation): decompose checkDirectDebitRules into per-rule helpers Extract R1, R2, R3, R4 checks into focused private functions: - checkR1SignatureBeforeCollection: returns one issue or null - checkR2Issues: OOFF single-use mandate check - checkR3Issues: mandate scheme consistency check - checkR4SmndaRequiresFirst: SMNDA requires FRST batch - accumulateMandateUsage: pure map accumulator The orchestrator becomes a thin coordinator. Issue ordering is identical to before: R1 in collection-traversal order, R2 then R3 per mandate in map-insertion order, R4 at the end. Cognitive complexity: 58 (CRITICAL) -> 15 on the orchestrator. All 668 tests pass; guard suites stress-ran 8x with zero failures. --- src/model/dd-rules.ts | 253 +++++++++++++++++++++++++----------------- 1 file changed, 150 insertions(+), 103 deletions(-) diff --git a/src/model/dd-rules.ts b/src/model/dd-rules.ts index e02fc62..52cb15a 100644 --- a/src/model/dd-rules.ts +++ b/src/model/dd-rules.ts @@ -27,143 +27,190 @@ import type { DirectDebitDocument } from './pain008.js' import type { ProfileIssue } from '../profile/profile.js' +/** Internal record of one mandate usage within a document, used for R2/R3 checks. */ +interface MandateUsage { + batchIdx: number + colIdx: number + collectionDate: string + signatureDate: string + sequenceType: string + localInstrument: string +} + /** - * Check cross-field SEPA rulebook constraints on a DirectDebitDocument. - * Returns an empty array when the document passes all rules. - * Returns one ProfileIssue per violation with a precise path and message. - * - * @param doc A DirectDebitDocument that has already passed Zod schema validation. - * @returns An array of ProfileIssue describing any violations (empty when valid). + * Append a mandate usage record to the index map, creating a new entry if needed. + * This is a pure accumulator with no side effects beyond the map mutation. */ -export function checkDirectDebitRules(doc: DirectDebitDocument): ProfileIssue[] { +function accumulateMandateUsage( + index: Map, + mandateId: string, + usage: MandateUsage +): void { + const existing = index.get(mandateId) + if (existing !== undefined) { + existing.push(usage) + } else { + index.set(mandateId, [usage]) + } +} + +/** + * R1: mandate signatureDate must not be after the batch collectionDate. + * Returns a ProfileIssue if violated, null otherwise. + */ +function checkR1SignatureBeforeCollection( + bIdx: number, + cIdx: number, + signatureDate: string, + collectionDate: string +): ProfileIssue | null { + if (signatureDate <= collectionDate) return null + return { + path: `batches.${bIdx}.collections.${cIdx}.mandate.signatureDate`, + message: + `R1: mandate signatureDate (${signatureDate}) is after` + + ` the batch collectionDate (${collectionDate}).` + + ` A debit cannot precede the mandate signature.`, + } +} + +/** + * R2: a mandate id used in an OOFF batch must appear in exactly one collection + * across the whole document. Returns issues for all occurrences beyond the first. + */ +function checkR2Issues(mandateId: string, usages: MandateUsage[]): ProfileIssue[] { + const hasOoff = usages.some((u) => u.sequenceType === 'OOFF') + if (!hasOoff) return [] + + // The mandate appears more than once and at least one occurrence is OOFF. + // Report all occurrences beyond the first to make the error precise. const issues: ProfileIssue[] = [] + for (let i = 1; i < usages.length; i++) { + const u = usages[i] + if (u === undefined) continue + issues.push({ + path: `batches.${u.batchIdx}.collections.${u.colIdx}.mandate.id`, + message: + `R2: mandate "${mandateId}" is used in an OOFF batch and appears in` + + ` more than one collection in this document.` + + ` An OOFF mandate authorizes exactly one collection.`, + }) + } + return issues +} + +/** + * R3: a mandate id must not appear under both CORE and B2B local instruments + * in the same document. Returns issues for all conflicting occurrences. + */ +function checkR3Issues(mandateId: string, usages: MandateUsage[]): ProfileIssue[] { + const instruments = new Set(usages.map((u) => u.localInstrument)) + if (instruments.size <= 1) return [] - // Build an index: mandateId -> list of { batchIdx, colIdx, collectionDate, sequenceType, localInstrument } - interface MandateUsage { - batchIdx: number - colIdx: number - collectionDate: string - signatureDate: string - sequenceType: string - localInstrument: string + // Mandate appears under multiple instruments (e.g. CORE and B2B). Report from second occurrence. + const issues: ProfileIssue[] = [] + const first = usages[0] + if (first === undefined) return [] + + for (let i = 1; i < usages.length; i++) { + const u = usages[i] + if (u === undefined) continue + if (u.localInstrument !== first.localInstrument) { + issues.push({ + path: `batches.${u.batchIdx}.collections.${u.colIdx}.mandate.id`, + message: + `R3: mandate "${mandateId}" is used under local instrument "${u.localInstrument}"` + + ` in batch ${u.batchIdx} but under "${first.localInstrument}" in batch ${first.batchIdx}.` + + ` A mandate is bound to one scheme (CORE or B2B) and cannot be reused across schemes.`, + }) + } } + return issues +} - const mandateIndex = new Map() +/** + * R4: if any collection in a batch has mandate.amendment.sameMandateNewDebtorAccount === true + * (SMNDA), the batch's sequenceType must be 'FRST'. + * Returns issues for each violating collection. + */ +function checkR4SmndaRequiresFirst(doc: DirectDebitDocument): ProfileIssue[] { + const issues: ProfileIssue[] = [] for (let bIdx = 0; bIdx < doc.batches.length; bIdx++) { const batch = doc.batches[bIdx] - // doc is validated so batch is always defined here if (batch === undefined) continue - - const effectiveLocalInstrument = batch.localInstrument ?? 'CORE' + if (batch.sequenceType === 'FRST') continue for (let cIdx = 0; cIdx < batch.collections.length; cIdx++) { const col = batch.collections[cIdx] if (col === undefined) continue - const usage: MandateUsage = { - batchIdx: bIdx, - colIdx: cIdx, - collectionDate: batch.collectionDate, - signatureDate: col.mandate.signatureDate, - sequenceType: batch.sequenceType, - localInstrument: effectiveLocalInstrument, - } - - // R1: signatureDate must not be after collectionDate (YYYY-MM-DD lexicographic) - if (col.mandate.signatureDate > batch.collectionDate) { + if (col.mandate.amendment?.sameMandateNewDebtorAccount === true) { issues.push({ - path: `batches.${bIdx}.collections.${cIdx}.mandate.signatureDate`, + path: `batches.${bIdx}.collections.${cIdx}.mandate.amendment.sameMandateNewDebtorAccount`, message: - `R1: mandate signatureDate (${col.mandate.signatureDate}) is after` + - ` the batch collectionDate (${batch.collectionDate}).` + - ` A debit cannot precede the mandate signature.`, + `R4: batch ${bIdx} has sequenceType "${batch.sequenceType}" but collection ${cIdx}` + + ` has sameMandateNewDebtorAccount=true (SMNDA).` + + ` A batch containing SMNDA collections must have sequenceType FRST,` + + ` because SMNDA represents the first collection under the amended mandate.`, }) } - - // Accumulate usage for R2 and R3 - const existing = mandateIndex.get(col.mandate.id) - if (existing !== undefined) { - existing.push(usage) - } else { - mandateIndex.set(col.mandate.id, [usage]) - } } } - // R2 and R3: check each mandate that appears more than once - for (const [mandateId, usages] of mandateIndex) { - if (usages.length < 2) continue - - // R2: if any usage is under OOFF, the mandate may appear exactly once in total - const hasOoff = usages.some((u) => u.sequenceType === 'OOFF') - if (hasOoff) { - // The mandate appears more than once and at least one occurrence is OOFF. - // Report one issue pointing to the second (and later) OOFF or non-OOFF occurrences. - // We report all occurrences beyond the first to make the error precise. - for (let i = 1; i < usages.length; i++) { - const u = usages[i] - if (u === undefined) continue - issues.push({ - path: `batches.${u.batchIdx}.collections.${u.colIdx}.mandate.id`, - message: - `R2: mandate "${mandateId}" is used in an OOFF batch and appears in` + - ` more than one collection in this document.` + - ` An OOFF mandate authorizes exactly one collection.`, - }) - } - } + return issues +} - // R3: all usages of a mandate must share the same local instrument - const instruments = new Set(usages.map((u) => u.localInstrument)) - if (instruments.size > 1) { - // Mandate appears under multiple instruments (e.g. CORE and B2B). Report from second occurrence. - for (let i = 1; i < usages.length; i++) { - const u = usages[i] - if (u === undefined) continue - const first = usages[0] - if (first === undefined) continue - if (u.localInstrument !== first.localInstrument) { - issues.push({ - path: `batches.${u.batchIdx}.collections.${u.colIdx}.mandate.id`, - message: - `R3: mandate "${mandateId}" is used under local instrument "${u.localInstrument}"` + - ` in batch ${u.batchIdx} but under "${first.localInstrument}" in batch ${first.batchIdx}.` + - ` A mandate is bound to one scheme (CORE or B2B) and cannot be reused across schemes.`, - }) - } - } - } - } +/** + * Check cross-field SEPA rulebook constraints on a DirectDebitDocument. + * Returns an empty array when the document passes all rules. + * Returns one ProfileIssue per violation with a precise path and message. + * + * @param doc A DirectDebitDocument that has already passed Zod schema validation. + * @returns An array of ProfileIssue describing any violations (empty when valid). + */ +export function checkDirectDebitRules(doc: DirectDebitDocument): ProfileIssue[] { + const issues: ProfileIssue[] = [] + const mandateIndex = new Map() - // R4: SMNDA requires FRST sequenceType - // If any collection in a batch has mandate.amendment.sameMandateNewDebtorAccount === true, - // the batch's sequenceType must be 'FRST'. + // Build the mandate index and collect R1 issues in collection-traversal order. for (let bIdx = 0; bIdx < doc.batches.length; bIdx++) { const batch = doc.batches[bIdx] if (batch === undefined) continue - - if (batch.sequenceType === 'FRST') { - // Already FRST, no violation possible for this batch. - continue - } + const effectiveLocalInstrument = batch.localInstrument ?? 'CORE' for (let cIdx = 0; cIdx < batch.collections.length; cIdx++) { const col = batch.collections[cIdx] if (col === undefined) continue - if (col.mandate.amendment?.sameMandateNewDebtorAccount === true) { - issues.push({ - path: `batches.${bIdx}.collections.${cIdx}.mandate.amendment.sameMandateNewDebtorAccount`, - message: - `R4: batch ${bIdx} has sequenceType "${batch.sequenceType}" but collection ${cIdx}` + - ` has sameMandateNewDebtorAccount=true (SMNDA).` + - ` A batch containing SMNDA collections must have sequenceType FRST,` + - ` because SMNDA represents the first collection under the amended mandate.`, - }) - } + const r1Issue = checkR1SignatureBeforeCollection( + bIdx, + cIdx, + col.mandate.signatureDate, + batch.collectionDate + ) + if (r1Issue !== null) issues.push(r1Issue) + + accumulateMandateUsage(mandateIndex, col.mandate.id, { + batchIdx: bIdx, + colIdx: cIdx, + collectionDate: batch.collectionDate, + signatureDate: col.mandate.signatureDate, + sequenceType: batch.sequenceType, + localInstrument: effectiveLocalInstrument, + }) } } + // Collect R2 and R3 issues per mandate entry (R2 before R3 within each mandate, preserving original order). + for (const [mandateId, usages] of mandateIndex) { + if (usages.length < 2) continue + issues.push(...checkR2Issues(mandateId, usages)) + issues.push(...checkR3Issues(mandateId, usages)) + } + + // Collect R4 issues (SMNDA must be in a FRST batch). + issues.push(...checkR4SmndaRequiresFirst(doc)) + return issues }