fix: say out loud what every sort was already doing

Sonar's S2871 flags a bare .sort(), which orders by UTF-16 code unit rather than
by value. All 30 remaining sites turned out to sort ASCII strings — ISO dates,
currency and country codes, setting keys, route names, OAuth scopes, zip entry
names — where that order is already the intended one. Each says so explicitly
now, with a comparator that reproduces it exactly rather than a localeCompare
that would quietly reorder things.

That mattered more than it sounds. Several of these feed something other than a
display: useAtlas joins the sorted codes into the signature that decides whether
to rebuild the region layer, daysSlice re-pins sorted dates positionally onto
reordered days, and three timeline views pick a day's colour by indexOf into the
sorted array. Switching any of them to a locale collator would have been a silent
regression.

Two smaller ones came with it. The .map(placeBtn) calls in FileManagerAssignModal
pass the index as a second argument, which placeBtn declares and uses for its
React key — correct already, now written out as .map((p, i) => placeBtn(p, i)).

And budgetSlice's optimistic paid-toggle stored the raw boolean behind
`paid as unknown as number`, while the schema declares z.number() and both the
REST response and the WebSocket event carry 0/1. So the optimistic state and the
confirmed state disagreed on type until a refresh. It normalises now, and the two
tests that asserted `true` assert the contract.

Four `| 0` truncations are listed in sonar-project.properties instead of changed.
Two are a djb2 string hash where the 32-bit wrap is the algorithm. The other two
look like plain truncation of a row id, but `| 0` is also coercing undefined to
0 there — the settlement flows the API returns carry no user_id, so that is the
normal path, and Math.trunc returns NaN and crashes the lookup.
This commit is contained in:
Maurice
2026-08-25 16:03:01 +02:00
committed by Maurice
parent 4ba407aead
commit 52d1a0720f
28 changed files with 70 additions and 39 deletions
@@ -123,13 +123,13 @@ export function AssignModal(S: FileManagerState) {
) : null
})()}
</div>
{dayPlaces.map(placeBtn)}
{dayPlaces.map((p, i) => placeBtn(p, i))}
</div>
))}
{unassigned.length > 0 && (
<div>
{dayGroups.length > 0 && <div style={{ fontSize: 'calc(11px * var(--fs-scale-caption, 1))', fontWeight: 600, color: 'var(--text-muted)', padding: '8px 10px 2px' }}>{t('files.unassigned')}</div>}
{unassigned.map(placeBtn)}
{unassigned.map((p, i) => placeBtn(p, i))}
</div>
)}
</div>
@@ -23,7 +23,7 @@ export function MapView({ entries, mapEntries, sortedDates, activeLocationId, fu
if (!byDate.has(d)) byDate.set(d, [])
byDate.get(d)!.push({ entry: e, globalIdx: i })
})
const dates = [...byDate.keys()].sort()
const dates = [...byDate.keys()].sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))
const mapItems = useMemo(() => mapEntries.map(e => ({
id: String(e.id),
@@ -46,7 +46,7 @@ export default function MobileMapTimeline({
const [activeIndex, setActiveIndex] = useState(0)
const entryDayMeta = useMemo(() => {
const uniqueDates = [...new Set(entries.map((e: any) => e.entry_date).sort())]
const uniqueDates = [...new Set(entries.map((e: any) => e.entry_date).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)))]
const counters = new Map<string, number>()
return entries.map((e: any) => {
const dayIdx = uniqueDates.indexOf(e.entry_date)
@@ -118,7 +118,7 @@ export function ReservationModal({ isOpen, onClose, onSave, reservation, days, p
// constrain to trip days via their day dropdowns. Falls back to no limit when
// the trip has no dated days.
const tripDateRange = useMemo(() => {
const dates = (days || []).map(d => d.date).filter((d): d is string => !!d).sort()
const dates = (days || []).map(d => d.date).filter((d): d is string => !!d).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))
return { min: dates[0], max: dates[dates.length - 1] }
}, [days])
+1 -1
View File
@@ -1450,7 +1450,7 @@ function backSpread(input: AutoInput): BookSpread {
const dates = input.entries
.map(e => e.date)
.filter((d): d is string => !!d)
.sort()
.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))
const span = dates.length
? [formatBookDate(dates[0], input.locale), formatBookDate(dates[dates.length - 1], input.locale)].filter(Boolean).join(' — ')
: ''
+1 -1
View File
@@ -249,7 +249,7 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu
const eps = (r.endpoints || []).map(e => `${e.role}@${e.lat ?? ''},${e.lng ?? ''}`).join(';')
return `${r.id}:${r.day_id ?? ''}:${r.end_day_id ?? ''}:${r.reservation_time ?? ''}:${pos ?? ''}:${eps}`
})
.sort()
.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))
.join('|')
}, [reservationsForSignature, selectedDayId])
@@ -151,7 +151,7 @@ function MCurrencyWidget(): React.ReactElement {
}).catch(() => { /* keep localStorage; retry on next load */ })
}, [isLoaded, updateSetting])
const currencies = rates ? Object.keys(rates).sort() : [...CURRENCIES]
const currencies = rates ? Object.keys(rates).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)) : [...CURRENCIES]
const rate = rates?.[to] ?? null
const converted = rate != null ? (Number.parseFloat(amount.replace(',', '.')) || 0) * rate : null
const swap = () => { setFrom(to); setTo(from) }
@@ -196,7 +196,7 @@ export default function MReservationSheet({ planner, onOpenExpense }: MReservati
// Restrict non-hotel booking dates to the trip's span (#1662); hotels already
// constrain to trip days via their day dropdowns.
const tripDates = days.map(d => d.date).filter((d): d is string => !!d).sort()
const tripDates = days.map(d => d.date).filter((d): d is string => !!d).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))
const tripMinDate = tripDates[0]
const tripMaxDate = tripDates[tripDates.length - 1]
+1 -1
View File
@@ -633,7 +633,7 @@ function CurrencyTool(): React.ReactElement {
}).catch(() => { /* keep localStorage; retry on next load */ })
}, [isLoaded, updateSetting])
const currencies = rates ? Object.keys(rates).sort() : CURRENCIES
const currencies = rates ? Object.keys(rates).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)) : CURRENCIES
const ccyOptions = currencies.map(c => ({ value: c, label: c }))
const rate = rates?.[to] ?? null
const converted = rate != null ? (Number.parseFloat(amount.replace(',', '.')) || 0) * rate : null
+1 -1
View File
@@ -63,7 +63,7 @@ function JourneyDetailPageDesktop() {
const timelineEntries = current.entries.filter(e => (!hideSkeletons || e.type !== 'skeleton'))
const dayGroups = groupByDate(timelineEntries)
const sortedDates = [...dayGroups.keys()].sort()
const sortedDates = [...dayGroups.keys()].sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))
const tripDateMin = current.trips.length
? current.trips.reduce((min: string, t: any) => t.start_date && (!min || t.start_date < min) ? t.start_date : min, '')
+1 -1
View File
@@ -649,7 +649,7 @@ export function useAtlas() {
// layer meant a continent's worth of admin-1 polygons stayed in the DOM, and every
// newly loaded country tore the whole thing down and built it again (#1950).
const bounds = viewportBounds()
const inViewCodes = Object.keys(regionGeoCache.current).filter(code => countryInView(code, bounds)).sort()
const inViewCodes = Object.keys(regionGeoCache.current).filter(code => countryInView(code, bounds)).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))
const sig = inViewCodes.join('|')
if (!force && sig === renderedRegionSigRef.current) return
@@ -300,7 +300,7 @@ export function useJourneyDetail() {
(current?.entries || [])
.filter(e => e.title !== 'Gallery' && e.title !== '[Trip Photos]')
.map(e => e.entry_date)
.sort()
.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))
)]
const sorted = [...mapEntries].sort((a, b) => a.entry_date.localeCompare(b.entry_date))
const dayCounters = new Map<string, number>()
@@ -55,7 +55,7 @@ export function useJourneyPublic() {
const groupedEntries = useMemo(() => groupByDate(timelineEntries), [timelineEntries])
// Chronological throughout: this is what the day colours and the stop numbers are
// derived from, so flipping the reading order must not renumber the trip.
const sortedDates = useMemo(() => [...groupedEntries.keys()].sort(), [groupedEntries])
const sortedDates = useMemo(() => [...groupedEntries.keys()].sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)), [groupedEntries])
const mapEntries = useMemo(
() => timelineEntries.filter(e => e.location_lat && e.location_lng),
[timelineEntries],
+1 -1
View File
@@ -144,7 +144,7 @@ describe('budgetSlice', () => {
);
await useTripStore.getState().toggleBudgetMemberPaid(1, 8, 3, true);
const stored = useTripStore.getState().budgetItems.find(i => i.id === 8);
expect(stored?.members?.[0]?.paid).toBe(true);
expect(stored?.members?.[0]?.paid).toBe(1);
});
it('FE-STORE-BUDGET-010: reorderBudgetItems reorders optimistically and reloads on error', async () => {
+4 -4
View File
@@ -82,10 +82,10 @@ export const createBudgetSlice = (set: SetState, get: GetState): BudgetSlice =>
set(state => ({
budgetItems: state.budgetItems.map(item =>
item.id === itemId
// The server persists `paid` as 0/1; the optimistic update stores the
// boolean toggle value (truthy-compatible) — narrow it to the member's
// numeric type without changing the stored runtime value.
? { ...item, members: (item.members || []).map(m => m.user_id === userId ? { ...m, paid: paid as unknown as number } : m) }
// The server persists `paid` as 0/1 and broadcasts the same 0/1 over
// WebSocket, so the optimistic write normalises the boolean toggle to
// that numeric contract instead of parking a boolean under a cast.
? { ...item, members: (item.members || []).map(m => m.user_id === userId ? { ...m, paid: paid ? 1 : 0 } : m) }
: item
)
}));
+1 -1
View File
@@ -20,7 +20,7 @@ export const createDaysSlice = (set: SetState, get: GetState): DaysSlice => ({
reorderDays: async (tripId, orderedIds) => {
const prevDays = get().days
const byId = new Map(prevDays.map(d => [d.id, d]))
const sortedDates = prevDays.map(d => d.date).filter((d): d is string => !!d).sort()
const sortedDates = prevDays.map(d => d.date).filter((d): d is string => !!d).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))
const optimistic = orderedIds
.map((id, i) => {
const d = byId.get(id)
+1 -1
View File
@@ -285,7 +285,7 @@ export function formatMoneySum(
// Breakdown: base first, then the rest in stable code order.
const parts = [
...(groups.has(baseCur) ? [baseCur] : []),
...foreign.sort(),
...foreign.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)),
]
return parts.map(cur => formatMoney(groups.get(cur)!, cur, locale, opts)).join(' + ')
}
+1 -1
View File
@@ -170,7 +170,7 @@ describe('budgetSlice', () => {
const updatedItem = useTripStore.getState().budgetItems.find(i => i.id === 10);
const updatedMember = updatedItem?.members.find(m => m.user_id === 5);
expect(updatedMember?.paid).toBe(true);
expect(updatedMember?.paid).toBe(1);
});
});
});
+2 -2
View File
@@ -141,12 +141,12 @@ const manifestAtCommit: NetworkCheck = {
if ((m.operatorEgress === true) !== (v.operatorEgress === true)) {
p(`manifest operatorEgress ${m.operatorEgress === true} != entry ${v.operatorEgress === true}`);
}
const normAddons = (a: unknown) => (Array.isArray(a) ? [...a].map(String).sort() : []);
const normAddons = (a: unknown) => (Array.isArray(a) ? [...a].map(String).sort((x, y) => (x < y ? -1 : x > y ? 1 : 0)) : []);
if (JSON.stringify(normAddons(m.requiredAddons)) !== JSON.stringify(normAddons(v.requiredAddons))) {
p('manifest requiredAddons != entry requiredAddons');
}
const normDeps = (d: unknown) =>
(Array.isArray(d) ? d.map((x) => `${(x as { id?: string })?.id}@${(x as { version?: string })?.version}`).sort() : []);
(Array.isArray(d) ? d.map((x) => `${(x as { id?: string })?.id}@${(x as { version?: string })?.version}`).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)) : []);
if (JSON.stringify(normDeps(m.pluginDependencies)) !== JSON.stringify(normDeps(v.pluginDependencies))) {
p('manifest pluginDependencies != entry pluginDependencies');
}
+2 -2
View File
@@ -530,12 +530,12 @@ const releaseEntryParity: OfflineCheck = {
problems.push(`• manifest operatorEgress ${m.operatorEgress === true} != entry ${v.operatorEgress === true}`);
}
const normAddons = (a: unknown) => [...arr(a)].sort();
const normAddons = (a: unknown) => [...arr(a)].sort((x, y) => (x < y ? -1 : x > y ? 1 : 0));
if (JSON.stringify(normAddons(m.requiredAddons)) !== JSON.stringify(normAddons(v.requiredAddons))) {
problems.push(`• manifest requiredAddons [${normAddons(m.requiredAddons)}] != entry [${normAddons(v.requiredAddons)}]`);
}
const normDeps = (d: unknown) =>
(Array.isArray(d) ? d.map((x) => `${(x as { id?: string })?.id}@${(x as { version?: string })?.version}`).sort() : []);
(Array.isArray(d) ? d.map((x) => `${(x as { id?: string })?.id}@${(x as { version?: string })?.version}`).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)) : []);
if (JSON.stringify(normDeps(m.pluginDependencies)) !== JSON.stringify(normDeps(v.pluginDependencies))) {
problems.push(`• manifest pluginDependencies [${normDeps(m.pluginDependencies)}] != entry [${normDeps(v.pluginDependencies)}]`);
}
+1 -1
View File
@@ -100,6 +100,6 @@ export function packPluginDir(dir: string, outPath: string): PackResult {
artifact: outAbs,
sha256: createHash('sha256').update(buf).digest('hex'),
size: buf.length,
files: filtered.map((f) => f.name).sort(),
files: filtered.map((f) => f.name).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)),
};
}
+1 -1
View File
@@ -254,7 +254,7 @@ export class AtlasService {
const dates = countryTrips
.map((t) => t.start_date)
.filter(Boolean)
.sort();
.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0));
return {
code: c.code,
placeCount: c.places.length,
+1 -1
View File
@@ -166,5 +166,5 @@ export function splitManagedKeys<T extends Record<string, unknown>>(
if (LOCKED.has(key)) blocked.push(key);
else (allowed as Record<string, unknown>)[key] = value;
}
return { allowed, blocked: blocked.sort() };
return { allowed, blocked: blocked.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)) };
}
@@ -86,14 +86,14 @@ export function validateBodyContracts(
problems.push(
`${violations.length} mutation handler(s) read @Body() without a createZodDto class ` +
`(add a <domain>.dto.ts wrapper over the @trek/shared schema — do NOT extend the allow-list): ` +
violations.sort().join(', '),
violations.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)).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().join(', '),
stale.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)).join(', '),
);
}
if (problems.length) {
+2 -2
View File
@@ -398,7 +398,7 @@ export class DaysService {
const oldDateById = new Map(rows.map(r => [r.id, r.date]));
// Dates stay pinned to slots: position i keeps the i-th date (ascending).
const sortedDates = rows.map(r => r.date).filter((d): d is string => !!d).sort();
const sortedDates = rows.map(r => r.date).filter((d): d is string => !!d).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0));
const isDated = sortedDates.length > 0;
const setDayNumber = this.db.prepare('UPDATE days SET day_number = ? WHERE id = ?');
@@ -452,7 +452,7 @@ export class DaysService {
}
// Dated trip: rebuild N+1 contiguous dates from the earliest date.
const start = datedRows.map(r => r.date).sort()[0];
const start = datedRows.map(r => r.date).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))[0];
const dates = Array.from({ length: n + 1 }, (_, i) => addDays(start, i));
const oldDateById = new Map(rows.map(r => [r.id, r.date]));
const setDayNumberAndDate = this.db.prepare('UPDATE days SET day_number = ?, date = ? WHERE id = ?');
+3 -3
View File
@@ -179,10 +179,10 @@ export interface StatsInput {
*/
export function computeJourneyStats(input: StatsInput): JourneyStats {
const points = thinRoute(input.points);
const dated = points.map(p => isoDay(p.date)).filter((d): d is string => !!d).sort();
const dated = points.map(p => isoDay(p.date)).filter((d): d is string => !!d).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0));
const tripStarts = input.tripDates.map(t => isoDay(t.start)).filter((d): d is string => !!d).sort();
const tripEnds = input.tripDates.map(t => isoDay(t.end)).filter((d): d is string => !!d).sort();
const tripStarts = input.tripDates.map(t => isoDay(t.start)).filter((d): d is string => !!d).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0));
const tripEnds = input.tripDates.map(t => isoDay(t.end)).filter((d): d is string => !!d).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0));
const start = dated[0] ?? tripStarts[0] ?? null;
const end = dated[dated.length - 1] ?? tripEnds[tripEnds.length - 1] ?? null;
@@ -82,8 +82,8 @@ export function jsonRpcError(message: string, code = -32000): object {
export function sameScopes(a: string[] | null, b: string[] | null): boolean {
if (a === null || b === null) return a === b;
if (a.length !== b.length) return false;
const left = [...a].sort();
const right = [...b].sort();
const left = [...a].sort((x, y) => (x < y ? -1 : x > y ? 1 : 0));
const right = [...b].sort((x, y) => (x < y ? -1 : x > y ? 1 : 0));
return left.every((scope, i) => scope === right[i]);
}
+32 -1
View File
@@ -62,7 +62,7 @@ sonar.cpd.exclusions=shared/src/i18n/**
# Locale files are UI copy — strings like "Password", "API key", "token" are labels,
# not credentials, and the secret detectors false-positive on them constantly.
# Ignore the secrets analyzer and the hard-coded-credential rules on i18n only.
sonar.issue.ignore.multicriteria=i18nsec1,i18nsec2,i18nsec3,rand1,rand2,rand3,rand4,rand5,cli1,hash1,hash2,hash3,hash4,ns1,ns2,frame1
sonar.issue.ignore.multicriteria=i18nsec1,i18nsec2,i18nsec3,rand1,rand2,rand3,rand4,rand5,rand6,cli1,hash1,hash2,hash3,hash4,hash5,hash6,hash7,hash8,ns1,ns2,frame1
sonar.issue.ignore.multicriteria.i18nsec1.ruleKey=secrets:*
sonar.issue.ignore.multicriteria.i18nsec1.resourceKey=shared/src/i18n/**
sonar.issue.ignore.multicriteria.i18nsec2.ruleKey=typescript:S2068
@@ -145,3 +145,34 @@ sonar.issue.ignore.multicriteria.ns2.resourceKey=server/src/nest/plugins/host/rp
# event.source instead.
sonar.issue.ignore.multicriteria.frame1.ruleKey=typescript:S2819
sonar.issue.ignore.multicriteria.frame1.resourceKey=client/src/components/Plugins/PluginFrame.tsx
# randomId is the module that MINTS idempotency keys, and its first two rungs are
# crypto.randomUUID and crypto.getRandomValues. The Math.random line is the third,
# reached only by a runtime with no Web Crypto at all, and the value it produces is
# a deduplication key the server scopes per (user, method, path) — not a token, not
# a secret. Replacing it is not possible: there is no stronger primitive left at
# that point, and deleting the rung would leave such a runtime with no key.
sonar.issue.ignore.multicriteria.rand6.ruleKey=typescript:S2245
sonar.issue.ignore.multicriteria.rand6.resourceKey=client/src/utils/randomId.ts
# S7767 (bitwise truncation) — Math.trunc is NOT equivalent to `| 0`, and all four
# sites depend on a difference the rule does not model.
#
# packingListPanel.helpers and todoListModel hold the same djb2-variant string
# hash, `h = ((h << 5) - h + chr) | 0`, whose only job is to stay bounded while it
# folds a category name into a colour index. Math.trunc would let h grow past 2^53,
# lose precision, and hand out different colours for the same names.
#
# BudgetPanel.helpers and TravelerPicker look like plain truncation of a user row
# id, but `| 0` is also coercing there: it turns undefined and NaN into 0, while
# Math.trunc returns NaN and the array lookup then yields undefined. That is not
# theoretical — the settlement `flows` the API returns carry a username and an
# avatar but no user_id, so colorForUserId(undefined) is the normal path for that
# section. Swapping it crashed two BudgetPanel tests on `.gradient` of undefined.
sonar.issue.ignore.multicriteria.hash5.ruleKey=typescript:S7767
sonar.issue.ignore.multicriteria.hash5.resourceKey=client/src/components/Packing/packingListPanel.helpers.ts
sonar.issue.ignore.multicriteria.hash6.ruleKey=typescript:S7767
sonar.issue.ignore.multicriteria.hash6.resourceKey=client/src/components/Todo/todoListModel.ts
sonar.issue.ignore.multicriteria.hash7.ruleKey=typescript:S7767
sonar.issue.ignore.multicriteria.hash7.resourceKey=client/src/components/Budget/BudgetPanel.helpers.ts
sonar.issue.ignore.multicriteria.hash8.ruleKey=typescript:S7767
sonar.issue.ignore.multicriteria.hash8.resourceKey=client/src/components/Planner/TravelerPicker.tsx