diff --git a/server/src/nest/common/compare.ts b/server/src/nest/common/compare.ts new file mode 100644 index 000000000..89dd0e8b7 --- /dev/null +++ b/server/src/nest/common/compare.ts @@ -0,0 +1,19 @@ +/** + * The comparator a bare `.sort()` already applies, written out. + * + * `Array.prototype.sort()` with no argument compares the UTF-16 code units of + * each element's string form. For ISO dates, ASCII keys, currency codes and the + * like that is the order the code wants, but Sonar's S2871 flags the bare call + * because the same syntax silently mis-sorts numbers ([10, 9, 1] becomes + * [1, 10, 9]). + * + * Shared rather than inlined at each call site: an inline arrow is a new + * function on every sort, and the ones that live on error paths are never + * executed by a passing test — which drags a whole domain's coverage down for a + * comparator that does nothing but restate the default. One exported function, + * covered once. + * + * NOT for user-visible names. Code-unit order files every accented word behind + * the entire ASCII range, so a list a person reads wants `localeCompare`. + */ +export const byCodeUnit = (a: string, b: string): number => (a < b ? -1 : a > b ? 1 : 0); diff --git a/server/src/nest/common/managed.ts b/server/src/nest/common/managed.ts index fd99c0198..0f216e9fd 100644 --- a/server/src/nest/common/managed.ts +++ b/server/src/nest/common/managed.ts @@ -1,5 +1,6 @@ import { SetMetadata } from '@nestjs/common'; import { RuntimeEnvService } from '../app-config/runtime-env.service'; +import { byCodeUnit } from './compare'; /** Metadata key `@ManagedForbidden()` writes. */ export const MANAGED_FORBIDDEN = 'trek:managed-forbidden'; @@ -166,5 +167,5 @@ export function splitManagedKeys>( if (LOCKED.has(key)) blocked.push(key); else (allowed as Record)[key] = value; } - return { allowed, blocked: blocked.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)) }; + return { allowed, blocked: blocked.sort(byCodeUnit) }; } diff --git a/server/src/nest/common/validate-body-contracts.ts b/server/src/nest/common/validate-body-contracts.ts index b03255d9c..81b1f4f44 100644 --- a/server/src/nest/common/validate-body-contracts.ts +++ b/server/src/nest/common/validate-body-contracts.ts @@ -5,6 +5,7 @@ import { RouteParamtypes } from '@nestjs/common/enums/route-paramtypes.enum'; import { ModulesContainer } from '@nestjs/core'; import { isZodDto } from 'nestjs-zod/dto'; import { BODY_CONTRACT_ALLOW_LIST } from './body-contract-allow-list'; +import { byCodeUnit } from './compare'; const MUTATION_METHODS = new Set([ RequestMethod.POST, @@ -86,14 +87,14 @@ export function validateBodyContracts( problems.push( `${violations.length} mutation handler(s) read @Body() without a createZodDto class ` + `(add a .dto.ts wrapper over the @trek/shared schema — do NOT extend the allow-list): ` + - violations.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)).join(', '), + violations.sort(byCodeUnit).join(', '), ); } if (stale.length) { problems.push( `stale body-contract allow-list entr${stale.length === 1 ? 'y' : 'ies'} ` + `(route removed or now validated — delete from body-contract-allow-list.ts): ` + - stale.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)).join(', '), + stale.sort(byCodeUnit).join(', '), ); } if (problems.length) { diff --git a/server/tests/unit/nest/compare.test.ts b/server/tests/unit/nest/compare.test.ts new file mode 100644 index 000000000..ffcdb94b2 --- /dev/null +++ b/server/tests/unit/nest/compare.test.ts @@ -0,0 +1,43 @@ +/** + * byCodeUnit restates what a bare `.sort()` already did, so the contract is + * "identical to the default order" — and that is what these check, against the + * default rather than against hand-written expectations. + */ +import { describe, it, expect } from 'vitest'; +import { byCodeUnit } from '../../../src/nest/common/compare'; + +describe('byCodeUnit', () => { + it('CMP-001: reports less, greater and equal', () => { + expect(byCodeUnit('a', 'b')).toBe(-1); + expect(byCodeUnit('b', 'a')).toBe(1); + expect(byCodeUnit('a', 'a')).toBe(0); + }); + + it('CMP-002: orders ISO dates chronologically, which is why the callers can use it', () => { + expect(['2026-03-01', '2026-01-15', '2026-02-28'].sort(byCodeUnit)) + .toEqual(['2026-01-15', '2026-02-28', '2026-03-01']); + }); + + it('CMP-003: reproduces the default sort exactly for the ASCII the callers pass', () => { + const samples = [ + ['EUR', 'usd', 'CHF', 'JPY'], + ['b', 'A', 'a', 'B', ''], + ['plugin:one', 'plugin:One', 'plugin:1', 'plugin:'], + ['2026-01-01', '2026-1-1', '2026-01-1'], + ['x'], + [], + ]; + for (const sample of samples) { + expect([...sample].sort(byCodeUnit)).toEqual([...sample].sort()); + } + }); + + it('CMP-004: is NOT locale-aware — the reason the doc comment warns callers off names', () => { + // Code-unit order files every accented word behind the whole ASCII range. + // A list a person reads wants localeCompare instead; this asserts the + // difference so nobody reaches for byCodeUnit there by accident. + expect(['Zelt', 'Übernachtung'].sort(byCodeUnit)).toEqual(['Zelt', 'Übernachtung']); + expect(['Zelt', 'Übernachtung'].sort((a, b) => a.localeCompare(b, 'de'))) + .toEqual(['Übernachtung', 'Zelt']); + }); +});