From ea90e0a99e2562d200e07b1db5602d32c9abba21 Mon Sep 17 00:00:00 2001 From: Jack Date: Fri, 16 May 2025 16:04:19 +0200 Subject: [PATCH] refactor: dont allow nullable numbers (@miodec) (#6564) Enables strict boolean expressions rule for nullable numbers --- backend/src/api/controllers/result.ts | 14 ++++- backend/src/workers/later-worker.ts | 6 +- frontend/src/ts/elements/monkey-power.ts | 9 +-- frontend/src/ts/elements/profile.ts | 2 +- frontend/src/ts/elements/psa.ts | 3 +- .../src/ts/elements/test-activity-calendar.ts | 3 +- frontend/src/ts/elements/test-activity.ts | 6 +- frontend/src/ts/elements/xp-bar.ts | 24 ++++---- frontend/src/ts/modals/quote-rate.ts | 23 +++++--- frontend/src/ts/pages/account.ts | 2 +- frontend/src/ts/pages/leaderboards.ts | 11 +++- frontend/src/ts/test/caret.ts | 5 +- frontend/src/ts/test/result.ts | 2 +- packages/eslint-config/index.js | 2 +- packages/funbox/src/validation.ts | 4 +- packages/util/__test__/numbers.spec.ts | 56 +++++++++++++++++++ packages/util/src/numbers.ts | 26 +++++++++ 17 files changed, 156 insertions(+), 42 deletions(-) diff --git a/backend/src/api/controllers/result.ts b/backend/src/api/controllers/result.ts index c80f15581..47166ec1f 100644 --- a/backend/src/api/controllers/result.ts +++ b/backend/src/api/controllers/result.ts @@ -52,7 +52,12 @@ import { XpBreakdown, } from "@monkeytype/contracts/schemas/results"; import { Mode } from "@monkeytype/contracts/schemas/shared"; -import { mapRange, roundTo2, stdDev } from "@monkeytype/util/numbers"; +import { + isSafeNumber, + mapRange, + roundTo2, + stdDev, +} from "@monkeytype/util/numbers"; import { getCurrentDayTimestamp, getStartOfDayTimestamp, @@ -321,7 +326,10 @@ export async function addResult( const earliestPossible = (lastResult?.timestamp ?? 0) + testDurationMilis + incompleteTestsMilis; const nowNoMilis = Math.floor(Date.now() / 1000) * 1000; - if (lastResult?.timestamp && nowNoMilis < earliestPossible - 1000) { + if ( + isSafeNumber(lastResult?.timestamp) && + nowNoMilis < earliestPossible - 1000 + ) { void addLog( "invalid_result_spacing", { @@ -777,7 +785,7 @@ async function calculateXp( Logger.error(`Could not fetch last result: ${getLastResultError}`); } - if (lastResult?.timestamp) { + if (isSafeNumber(lastResult?.timestamp)) { const lastResultDay = getStartOfDayTimestamp(lastResult.timestamp); const today = getCurrentDayTimestamp(); if (lastResultDay !== today) { diff --git a/backend/src/workers/later-worker.ts b/backend/src/workers/later-worker.ts index b7b83d549..39d9e6a0d 100644 --- a/backend/src/workers/later-worker.ts +++ b/backend/src/workers/later-worker.ts @@ -16,7 +16,7 @@ import LaterQueue, { import { recordTimeToCompleteJob } from "../utils/prometheus"; import { WeeklyXpLeaderboard } from "../services/weekly-xp-leaderboard"; import { MonkeyMail } from "@monkeytype/contracts/schemas/users"; -import { mapRange } from "@monkeytype/util/numbers"; +import { isSafeNumber, mapRange } from "@monkeytype/util/numbers"; async function handleDailyLeaderboardResults( ctx: LaterTaskContexts["daily-leaderboard-results"] @@ -74,7 +74,7 @@ async function handleDailyLeaderboardResults( ) .max(); - if (!xpReward) return; + if (!isSafeNumber(xpReward)) return; const rewardMail = buildMonkeyMail({ subject: "Daily leaderboard placement", @@ -164,7 +164,7 @@ async function handleWeeklyXpLeaderboardResults( ) .max(); - if (!xpReward) return; + if (!isSafeNumber(xpReward)) return; const rewardMail = buildMonkeyMail({ subject: "Weekly XP Leaderboard placement", diff --git a/frontend/src/ts/elements/monkey-power.ts b/frontend/src/ts/elements/monkey-power.ts index 5fcb258a9..7904caef6 100644 --- a/frontend/src/ts/elements/monkey-power.ts +++ b/frontend/src/ts/elements/monkey-power.ts @@ -1,6 +1,7 @@ import * as ThemeColors from "./theme-colors"; import * as SlowTimer from "../states/slow-timer"; import Config from "../config"; +import { isSafeNumber } from "@monkeytype/util/numbers"; type Particle = { x: number; @@ -85,7 +86,7 @@ function createParticle(x: number, y: number, color: string): Particle { * @param {Particle} particle */ function updateParticle(particle: Particle): void { - if (!ctx.canvas || !ctx.deltaTime) return; + if (!ctx.canvas || !isSafeNumber(ctx.deltaTime)) return; particle.prev.x = particle.x; particle.prev.y = particle.y; @@ -123,7 +124,7 @@ export function init(): void { } function render(): void { - if (!ctx.lastFrame || !ctx.context2d || !ctx.canvas) return; + if (!isSafeNumber(ctx.lastFrame) || !ctx.context2d || !ctx.canvas) return; ctx.rendering = true; const time = Date.now(); ctx.deltaTime = (time - ctx.lastFrame) / 1000; @@ -163,7 +164,7 @@ function render(): void { } export function reset(immediate = false): void { - if (!ctx.resetTimeOut) return; + if (!isSafeNumber(ctx.resetTimeOut)) return; delete ctx.resetTimeOut; clearTimeout(ctx.resetTimeOut); @@ -213,7 +214,7 @@ export async function addPower(good = true, extra = false): Promise { "transform", `translate(${shake[0]}px, ${shake[1]}px)` ); - if (ctx.resetTimeOut) clearTimeout(ctx.resetTimeOut); + if (isSafeNumber(ctx.resetTimeOut)) clearTimeout(ctx.resetTimeOut); ctx.resetTimeOut = setTimeout(reset, 2000) as unknown as number; } diff --git a/frontend/src/ts/elements/profile.ts b/frontend/src/ts/elements/profile.ts index d46fb09c6..bb13402c7 100644 --- a/frontend/src/ts/elements/profile.ts +++ b/frontend/src/ts/elements/profile.ts @@ -172,7 +172,7 @@ export async function update( console.debug("isToday", isToday); console.debug("isYesterday", isYesterday); - const offsetString = streakOffset + const offsetString = Numbers.isSafeNumber(streakOffset) ? `(${streakOffset > 0 ? "+" : ""}${streakOffset} offset)` : ""; diff --git a/frontend/src/ts/elements/psa.ts b/frontend/src/ts/elements/psa.ts index 8446fa9d9..95507069c 100644 --- a/frontend/src/ts/elements/psa.ts +++ b/frontend/src/ts/elements/psa.ts @@ -9,6 +9,7 @@ import { z } from "zod"; import { LocalStorageWithSchema } from "../utils/local-storage-with-schema"; import { IdSchema } from "@monkeytype/contracts/schemas/util"; import { tryCatch } from "@monkeytype/util/trycatch"; +import { isSafeNumber } from "@monkeytype/util/numbers"; const confirmedPSAs = new LocalStorageWithSchema({ key: "confirmedPSAs", @@ -136,7 +137,7 @@ export async function show(): Promise { } const localmemory = getMemory(); latest.forEach((psa) => { - if (psa.date) { + if (isSafeNumber(psa.date)) { const dateObj = new Date(psa.date); const diff = psa.date - Date.now(); const string = secondsToString( diff --git a/frontend/src/ts/elements/test-activity-calendar.ts b/frontend/src/ts/elements/test-activity-calendar.ts index 42d352648..4b839b393 100644 --- a/frontend/src/ts/elements/test-activity-calendar.ts +++ b/frontend/src/ts/elements/test-activity-calendar.ts @@ -1,4 +1,5 @@ import { UTCDateMini } from "@date-fns/utc/date/mini"; +import { safeNumber } from "@monkeytype/util/numbers"; import { format, endOfMonth, @@ -219,7 +220,7 @@ export class ModifiableTestActivityCalendar const lastDay = new UTCDateMini(this.lastDay); if (isSameDay(date, lastDay)) { const last = this.data.length - 1; - this.data[last] = (this.data[last] || 0) + 1; + this.data[last] = (safeNumber(this.data[last]) ?? 0) + 1; } else if (isBefore(date, lastDay)) { throw new Error("cannot alter data in the past."); } else { diff --git a/frontend/src/ts/elements/test-activity.ts b/frontend/src/ts/elements/test-activity.ts index d51a9e89d..f49f484cb 100644 --- a/frontend/src/ts/elements/test-activity.ts +++ b/frontend/src/ts/elements/test-activity.ts @@ -7,6 +7,7 @@ import { TestActivityCalendar, TestActivityMonth, } from "./test-activity-calendar"; +import { safeNumber } from "@monkeytype/util/numbers"; let yearSelector: SlimSelect | undefined = undefined; @@ -21,7 +22,10 @@ export function init( $("#testActivity").removeClass("hidden"); yearSelector = getYearSelector(); - initYearSelector("current", userSignUpDate?.getFullYear() || 2022); + initYearSelector( + "current", + safeNumber(userSignUpDate?.getFullYear()) ?? 2022 + ); updateLabels(calendar.firstDayOfWeek); update(calendar); } diff --git a/frontend/src/ts/elements/xp-bar.ts b/frontend/src/ts/elements/xp-bar.ts index e2292cc07..6404b5872 100644 --- a/frontend/src/ts/elements/xp-bar.ts +++ b/frontend/src/ts/elements/xp-bar.ts @@ -3,7 +3,7 @@ import * as Levels from "../utils/levels"; import { getAll } from "./theme-colors"; import * as SlowTimer from "../states/slow-timer"; import { XpBreakdown } from "@monkeytype/contracts/schemas/results"; -import { mapRange } from "@monkeytype/util/numbers"; +import { isSafeNumber, mapRange } from "@monkeytype/util/numbers"; let breakdownVisible = false; let skip = false; @@ -268,12 +268,12 @@ async function animateXpBreakdown( await Misc.sleep(delay); - if (breakdown.fullAccuracy) { + if (isSafeNumber(breakdown.fullAccuracy)) { await Misc.sleep(delay); total += breakdown.fullAccuracy; void flashTotalXp(total); await addBreakdownListItem("perfect", breakdown.fullAccuracy); - } else if (breakdown.corrected) { + } else if (isSafeNumber(breakdown.corrected)) { await Misc.sleep(delay); total += breakdown.corrected; void flashTotalXp(total); @@ -282,19 +282,19 @@ async function animateXpBreakdown( if (skip) return; - if (breakdown.quote) { + if (isSafeNumber(breakdown.quote)) { await Misc.sleep(delay); total += breakdown.quote; void flashTotalXp(total); await addBreakdownListItem("quote", breakdown.quote); } else { - if (breakdown.punctuation) { + if (isSafeNumber(breakdown.punctuation)) { await Misc.sleep(delay); total += breakdown.punctuation; void flashTotalXp(total); await addBreakdownListItem("punctuation", breakdown.punctuation); } - if (breakdown.numbers) { + if (isSafeNumber(breakdown.numbers)) { await Misc.sleep(delay); total += breakdown.numbers; void flashTotalXp(total); @@ -304,7 +304,7 @@ async function animateXpBreakdown( if (skip) return; - if (breakdown.funbox) { + if (isSafeNumber(breakdown.funbox)) { await Misc.sleep(delay); total += breakdown.funbox; void flashTotalXp(total); @@ -313,7 +313,7 @@ async function animateXpBreakdown( if (skip) return; - if (breakdown.streak) { + if (isSafeNumber(breakdown.streak)) { await Misc.sleep(delay); total += breakdown.streak; void flashTotalXp(total); @@ -322,7 +322,7 @@ async function animateXpBreakdown( if (skip) return; - if (breakdown.accPenalty) { + if (isSafeNumber(breakdown.accPenalty)) { await Misc.sleep(delay); total -= breakdown.accPenalty; void flashTotalXp(total); @@ -331,7 +331,7 @@ async function animateXpBreakdown( if (skip) return; - if (breakdown.incomplete) { + if (isSafeNumber(breakdown.incomplete)) { await Misc.sleep(delay); total += breakdown.incomplete; void flashTotalXp(total); @@ -340,7 +340,7 @@ async function animateXpBreakdown( if (skip) return; - if (breakdown.configMultiplier) { + if (isSafeNumber(breakdown.configMultiplier)) { await Misc.sleep(delay); total *= breakdown.configMultiplier; void flashTotalXp(total); @@ -352,7 +352,7 @@ async function animateXpBreakdown( if (skip) return; - if (breakdown.daily) { + if (isSafeNumber(breakdown.daily)) { await Misc.sleep(delay); total += breakdown.daily; void flashTotalXp(total); diff --git a/frontend/src/ts/modals/quote-rate.ts b/frontend/src/ts/modals/quote-rate.ts index 99b53bbee..921182a4d 100644 --- a/frontend/src/ts/modals/quote-rate.ts +++ b/frontend/src/ts/modals/quote-rate.ts @@ -5,6 +5,7 @@ import * as DB from "../db"; import * as Loader from "../elements/loader"; import * as Notifications from "../elements/notifications"; import AnimatedModal, { ShowOptions } from "../utils/animated-modal"; +import { isSafeNumber } from "@monkeytype/util/numbers"; let rating = 0; @@ -33,11 +34,16 @@ function reset(): void { } function getRatingAverage(quoteStats: QuoteStats): number { - if (!quoteStats.totalRating || !quoteStats.ratings) { - return 0; + if ( + isSafeNumber(quoteStats.ratings) && + isSafeNumber(quoteStats.totalRating) && + quoteStats.ratings > 0 && + quoteStats.totalRating > 0 + ) { + return Math.round((quoteStats.totalRating / quoteStats.ratings) * 10) / 10; } - return Math.round((quoteStats.totalRating / quoteStats.ratings) * 10) / 10; + return 0; } export async function getQuoteStats( @@ -66,7 +72,7 @@ export async function getQuoteStats( } quoteStats = response.body.data as QuoteStats; - if (quoteStats !== undefined && !quoteStats.average) { + if (quoteStats !== undefined && quoteStats.average === undefined) { quoteStats.average = getRatingAverage(quoteStats); } @@ -118,7 +124,7 @@ export function show(quote: Quote, showOptions?: ShowOptions): void { const snapshot = DB.getSnapshot(); const alreadyRated = snapshot?.quoteRatings?.[currentQuote.language]?.[currentQuote.id]; - if (alreadyRated) { + if (isSafeNumber(alreadyRated)) { rating = alreadyRated; } refreshStars(); @@ -163,7 +169,7 @@ async function submit(): Promise { const languageRatings = quoteRatings?.[currentQuote.language] ?? {}; - if (languageRatings?.[currentQuote.id]) { + if (isSafeNumber(languageRatings?.[currentQuote.id])) { const oldRating = quoteRatings[currentQuote.language]?.[ currentQuote.id ] as number; @@ -180,7 +186,10 @@ async function submit(): Promise { Notifications.add("Rating updated", 1); } else { languageRatings[currentQuote.id] = rating; - if (quoteStats?.ratings && quoteStats.totalRating) { + if ( + isSafeNumber(quoteStats?.ratings) && + isSafeNumber(quoteStats.totalRating) + ) { quoteStats.ratings++; quoteStats.totalRating += rating; } else { diff --git a/frontend/src/ts/pages/account.ts b/frontend/src/ts/pages/account.ts index d0903589f..3e13cca35 100644 --- a/frontend/src/ts/pages/account.ts +++ b/frontend/src/ts/pages/account.ts @@ -55,7 +55,7 @@ let visibleTableLines = 0; function loadMoreLines(lineIndex?: number): void { if (filteredResults === undefined || filteredResults.length === 0) return; let newVisibleLines; - if (lineIndex && lineIndex > visibleTableLines) { + if (Numbers.isSafeNumber(lineIndex) && lineIndex > visibleTableLines) { newVisibleLines = Math.ceil(lineIndex / 10) * 10; } else { newVisibleLines = visibleTableLines + 10; diff --git a/frontend/src/ts/pages/leaderboards.ts b/frontend/src/ts/pages/leaderboards.ts index dd532dd97..b51ca86f9 100644 --- a/frontend/src/ts/pages/leaderboards.ts +++ b/frontend/src/ts/pages/leaderboards.ts @@ -46,6 +46,7 @@ import { Language, LanguageSchema, } from "@monkeytype/contracts/schemas/languages"; +import { isSafeNumber } from "@monkeytype/util/numbers"; const LeaderboardTypeSchema = z.enum(["allTime", "weekly", "daily"]); type LeaderboardType = z.infer; @@ -479,7 +480,9 @@ function buildTableRow(entry: LeaderboardEntry, me = false): string { }?isUid" class="entryName" uid=${entry.uid} router-link>${entry.name}
${getHtmlByUserFlags(entry)} - ${entry.badgeId ? getBadgeHTMLbyId(entry.badgeId) : ""} + ${ + isSafeNumber(entry.badgeId) ? getBadgeHTMLbyId(entry.badgeId) : "" + }
@@ -530,7 +533,9 @@ function buildWeeklyTableRow(entry: XpLeaderboardEntry, me = false): string { }?isUid" class="entryName" uid=${entry.uid} router-link>${entry.name}
${getHtmlByUserFlags(entry)} - ${entry.badgeId ? getBadgeHTMLbyId(entry.badgeId) : ""} + ${ + isSafeNumber(entry.badgeId) ? getBadgeHTMLbyId(entry.badgeId) : "" + }
@@ -1071,7 +1076,7 @@ function handleJumpButton(action: Action, page?: number): void { const user = Auth?.currentUser; if (user) { const rank = state.userData?.rank; - if (rank) { + if (isSafeNumber(rank)) { // - 1 to make sure position 50 with page size 50 is on the first page (page 0) const page = Math.floor((rank - 1) / state.pageSize); diff --git a/frontend/src/ts/test/caret.ts b/frontend/src/ts/test/caret.ts index c03c00ad2..bd9f5c89e 100644 --- a/frontend/src/ts/test/caret.ts +++ b/frontend/src/ts/test/caret.ts @@ -7,6 +7,7 @@ import * as TestWords from "./test-words"; import { prefersReducedMotion } from "../utils/misc"; import { convertRemToPixels } from "../utils/numbers"; import { splitIntoCharacters } from "../utils/strings"; +import { safeNumber } from "@monkeytype/util/numbers"; export let caretAnimating = true; const caret = document.querySelector("#caret") as HTMLElement; @@ -163,8 +164,8 @@ export async function updatePosition(noAnim = false): Promise { // offsetHeight is the same for all visible letters // so is offsetTop (for same line letters) const letterHeight = - currentLetter?.offsetHeight || - lastWordLetter?.offsetHeight || + (safeNumber(currentLetter?.offsetHeight) ?? 0) || + (safeNumber(lastWordLetter?.offsetHeight) ?? 0) || Config.fontSize * convertRemToPixels(1); const letterPosTop = diff --git a/frontend/src/ts/test/result.ts b/frontend/src/ts/test/result.ts index ee34206e9..f79abc773 100644 --- a/frontend/src/ts/test/result.ts +++ b/frontend/src/ts/test/result.ts @@ -780,7 +780,7 @@ export function updateRateQuote(randomQuote: Quote | null): void { const userqr = DB.getSnapshot()?.quoteRatings?.[randomQuote.language]?.[randomQuote.id]; - if (userqr) { + if (Numbers.isSafeNumber(userqr)) { $(".pageTest #result #rateQuoteButton .icon") .removeClass("far") .addClass("fas"); diff --git a/packages/eslint-config/index.js b/packages/eslint-config/index.js index 7bef58ef0..6647cb312 100644 --- a/packages/eslint-config/index.js +++ b/packages/eslint-config/index.js @@ -121,7 +121,7 @@ module.exports = { "@typescript-eslint/no-floating-promises": "error", "@typescript-eslint/strict-boolean-expressions": [ "error", - { allowNullableBoolean: true, allowNullableNumber: true }, + { allowNullableBoolean: true }, ], "@typescript-eslint/non-nullable-type-assertion-style": "off", "@typescript-eslint/no-unnecessary-condition": "off", diff --git a/packages/funbox/src/validation.ts b/packages/funbox/src/validation.ts index eb384d851..d646f40d0 100644 --- a/packages/funbox/src/validation.ts +++ b/packages/funbox/src/validation.ts @@ -2,6 +2,7 @@ import { intersect } from "@monkeytype/util/arrays"; import { FunboxForcedConfig } from "./types"; import { getFunbox } from "./list"; import { FunboxName } from "@monkeytype/contracts/schemas/configs"; +import { safeNumber } from "@monkeytype/util/numbers"; export function checkCompatibility( funboxNames: FunboxName[], @@ -110,7 +111,8 @@ export function checkCompatibility( .filter((f) => f !== undefined) .flat() .reduce>((counts, cssModification) => { - counts[cssModification] = (counts[cssModification] || 0) + 1; + counts[cssModification] = + (safeNumber(counts[cssModification]) ?? 0) + 1; return counts; }, {}) ).every((c) => c <= 1); diff --git a/packages/util/__test__/numbers.spec.ts b/packages/util/__test__/numbers.spec.ts index 5ad69b73d..d8372be27 100644 --- a/packages/util/__test__/numbers.spec.ts +++ b/packages/util/__test__/numbers.spec.ts @@ -97,4 +97,60 @@ describe("numbers", () => { }); }); }); + describe("isSafeNumber", () => { + describe("should correctly identify safe numbers", () => { + const testCases = [ + //safe + { input: 0, expected: true }, + { input: 1, expected: true }, + { input: -1, expected: true }, + { input: 0.5, expected: true }, + { input: -0.5, expected: true }, + //not safe + { input: NaN, expected: false }, + { input: Infinity, expected: false }, + { input: -Infinity, expected: false }, + { input: "string", expected: false }, + { input: null, expected: false }, + { input: undefined, expected: false }, + { input: true, expected: false }, + { input: false, expected: false }, + ]; + + it.for(testCases)( + "should return $expected for $input", + ({ input, expected }) => { + expect(Numbers.isSafeNumber(input)).toEqual(expected); + } + ); + }); + }); + describe("safeNumber", () => { + describe("should correctly identify safe numbers", () => { + const testCases = [ + //safe + { input: 0, expected: 0 }, + { input: 1, expected: 1 }, + { input: -1, expected: -1 }, + { input: 0.5, expected: 0.5 }, + { input: -0.5, expected: -0.5 }, + //not safe + { input: NaN, expected: undefined }, + { input: Infinity, expected: undefined }, + { input: -Infinity, expected: undefined }, + { input: "string", expected: undefined }, + { input: null, expected: undefined }, + { input: undefined, expected: undefined }, + { input: true, expected: undefined }, + { input: false, expected: undefined }, + ]; + + it.for(testCases)( + "should return $expected for $input", + ({ input, expected }) => { + expect(Numbers.safeNumber(input as number)).toEqual(expected); + } + ); + }); + }); }); diff --git a/packages/util/src/numbers.ts b/packages/util/src/numbers.ts index aa090ca6b..240558732 100644 --- a/packages/util/src/numbers.ts +++ b/packages/util/src/numbers.ts @@ -125,3 +125,29 @@ export function mapRange( return result; } + +/** + * Checks if a value is a safe number. Safe numbers are finite and not NaN. + * @param value The value to check. + * @returns True if the value is a safe number, false otherwise. + */ +export function isSafeNumber(value: unknown): value is number { + if (typeof value === "number") { + return !isNaN(value) && isFinite(value); + } + return false; +} + +/** + * Converts a number to a safe number or undefined. NaN, Infinity, and -Infinity are converted to undefined. + * @param value The value to convert. + * @returns The input number if it is safe, undefined otherwise. + */ +export function safeNumber( + value: number | undefined | null +): number | undefined { + if (isSafeNumber(value)) { + return value; + } + return undefined; +}