From 07c6b13d93908081a5a4feb3a2ff5da52a6f4d40 Mon Sep 17 00:00:00 2001 From: Will Harney <62956339+wjhsf@users.noreply.github.com> Date: Fri, 15 Mar 2024 16:53:51 -0400 Subject: [PATCH] Enforce explicit function return type (#383) * Enforce explicit function return type It results in faster type checking and helps us identify usage of null vs undefined. * Use explicit return type in all files. --- .eslintrc.json | 1 + lib/__tests__/cookieJar.spec.ts | 6 ++--- lib/__tests__/jarSerialization.spec.ts | 6 ++--- lib/__tests__/regression.spec.ts | 4 ++-- lib/__tests__/util.spec.ts | 2 +- lib/cookie/canonicalDomain.ts | 2 +- lib/cookie/cookie.ts | 27 +++++++++++---------- lib/cookie/cookieCompare.ts | 2 +- lib/cookie/cookieJar.ts | 33 ++++++++++++++++---------- lib/cookie/formatDate.ts | 2 +- lib/cookie/parseDate.ts | 6 ++--- lib/memstore.ts | 4 ++-- lib/utils.ts | 12 +++++----- 13 files changed, 59 insertions(+), 48 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 52793e7c..80d4ed74 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -13,6 +13,7 @@ }, "reportUnusedDisableDirectives": true, "rules": { + "@typescript-eslint/explicit-function-return-type": "error", "max-lines": ["warn", 500] } } diff --git a/lib/__tests__/cookieJar.spec.ts b/lib/__tests__/cookieJar.spec.ts index 03b1648a..208698e9 100644 --- a/lib/__tests__/cookieJar.spec.ts +++ b/lib/__tests__/cookieJar.spec.ts @@ -741,7 +741,7 @@ describe('CookieJar', () => { beforeEach(async () => { const url = 'http://example.com/index.html' - const at = (timeFromNow: number) => ({ + const at = (timeFromNow: number): { now: Date } => ({ now: new Date(Date.now() + timeFromNow), }) @@ -838,7 +838,7 @@ describe('CookieJar', () => { beforeEach(async () => { const url = 'http://example.com/index.html' - const at = (timeFromNow: number) => ({ + const at = (timeFromNow: number): { now: Date } => ({ now: new Date(Date.now() + timeFromNow), }) @@ -1506,7 +1506,7 @@ function apiVariants( testName: string, apiVariants: ApiVariants, assertions: () => void, -) { +): void { it(`${testName} (callback)`, async () => { await new Promise((resolve) => apiVariants.callbackStyle(() => resolve(undefined)), diff --git a/lib/__tests__/jarSerialization.spec.ts b/lib/__tests__/jarSerialization.spec.ts index cf76f6e1..e189d0e6 100644 --- a/lib/__tests__/jarSerialization.spec.ts +++ b/lib/__tests__/jarSerialization.spec.ts @@ -299,7 +299,7 @@ describe('cookieJar serialization', () => { function expectDataToMatchSerializationSchema( serializedJar: SerializedCookieJar, -) { +): void { expect(serializedJar).not.toBeNull() expect(serializedJar).toBeInstanceOf(Object) expect(serializedJar.version).toBe(`tough-cookie@${version}`) @@ -326,7 +326,7 @@ const serializedCookiePropTypes: { [key: string]: string } = { sameSite: 'string', } -function validateSerializedCookie(cookie: SerializedCookie) { +function validateSerializedCookie(cookie: SerializedCookie): void { expect(typeof cookie).toBe('object') expect(cookie).not.toBeInstanceOf(Cookie) @@ -362,7 +362,7 @@ function validateSerializedCookie(cookie: SerializedCookie) { }) } -function isInteger(value: unknown) { +function isInteger(value: unknown): boolean { if (Number.isInteger) { return Number.isInteger(value) } diff --git a/lib/__tests__/regression.spec.ts b/lib/__tests__/regression.spec.ts index daf26a2e..62d0dccc 100644 --- a/lib/__tests__/regression.spec.ts +++ b/lib/__tests__/regression.spec.ts @@ -36,7 +36,7 @@ describe('Regression Tests', () => { expect.assertions(2) const cookieJar = new CookieJar() - const callback = function (err: null, cookie: Cookie) { + const callback = function (err: null, cookie: Cookie): void { expect(err).toBeNull() expect(cookie).toEqual( expect.objectContaining({ @@ -58,7 +58,7 @@ describe('Regression Tests', () => { expect.assertions(2) const cookieJar = new CookieJar() - const callback = function (err: null, cookie: Cookie) { + const callback = function (err: null, cookie: Cookie): void { expect(err).toBeNull() expect(cookie).toEqual([ expect.objectContaining({ diff --git a/lib/__tests__/util.spec.ts b/lib/__tests__/util.spec.ts index afcda6c2..a16bf063 100644 --- a/lib/__tests__/util.spec.ts +++ b/lib/__tests__/util.spec.ts @@ -11,7 +11,7 @@ describe('safeToString', () => { [123, '123'], [321n, '321'], [{ object: 'yes' }, '[object Object]'], - [(a: number, b: number) => a + b, '(a, b) => a + b'], + [(a: number, b: number): number => a + b, '(a, b) => a + b'], [Symbol('safeToString'), 'Symbol(safeToString)'], [Object.create(null), '[object Object]'], // eslint-disable-next-line no-sparse-arrays diff --git a/lib/cookie/canonicalDomain.ts b/lib/cookie/canonicalDomain.ts index 6029ed8d..0535e68e 100644 --- a/lib/cookie/canonicalDomain.ts +++ b/lib/cookie/canonicalDomain.ts @@ -2,7 +2,7 @@ import * as punycode from 'punycode/punycode.js' import { IP_V6_REGEX_OBJECT } from './constants' // S5.1.2 Canonicalized Host Names -export function canonicalDomain(str: string | null) { +export function canonicalDomain(str: string | null): string | null { if (str == null) { return null } diff --git a/lib/cookie/cookie.ts b/lib/cookie/cookie.ts index b261192d..fcc916d6 100644 --- a/lib/cookie/cookie.ts +++ b/lib/cookie/cookie.ts @@ -57,7 +57,7 @@ const CONTROL_CHARS = /[\x00-\x1F]/ // https://github.com/ChromiumWebApps/chromium/blob/b3d3b4da8bb94c1b2e061600df106d590fda3620/net/cookies/parsed_cookie.cc#L60 const TERMINATORS = ['\n', '\r', '\0'] -function trimTerminator(str: string) { +function trimTerminator(str: string): string { if (validators.isEmptyString(str)) return str for (let t = 0; t < TERMINATORS.length; t++) { const terminator = TERMINATORS[t] @@ -70,7 +70,10 @@ function trimTerminator(str: string) { return str } -function parseCookiePair(cookiePair: string, looseMode: boolean) { +function parseCookiePair( + cookiePair: string, + looseMode: boolean, +): Cookie | undefined { cookiePair = trimTerminator(cookiePair) validators.validate(validators.isString(cookiePair), cookiePair) @@ -273,7 +276,7 @@ function parse( return c } -function fromJSON(str: unknown) { +function fromJSON(str: unknown): Cookie | null { if (!str || validators.isEmptyString(str)) { return null } @@ -441,7 +444,7 @@ export class Cookie { this.creationIndex = Cookie.cookiesCreated } - [Symbol.for('nodejs.util.inspect.custom')]() { + [Symbol.for('nodejs.util.inspect.custom')](): string { const now = Date.now() const hostOnly = this.hostOnly != null ? this.hostOnly.toString() : '?' const createAge = @@ -525,11 +528,11 @@ export class Cookie { return obj } - clone() { + clone(): Cookie | null { return fromJSON(this.toJSON()) } - validate() { + validate(): boolean { if (this.value == null || !COOKIE_OCTETS.test(this.value)) { return false } @@ -565,7 +568,7 @@ export class Cookie { return true } - setExpires(exp: string | Date) { + setExpires(exp: string | Date): void { if (exp instanceof Date) { this.expires = exp } else { @@ -573,7 +576,7 @@ export class Cookie { } } - setMaxAge(age: number) { + setMaxAge(age: number): void { if (age === Infinity) { this.maxAge = 'Infinity' } else if (age === -Infinity) { @@ -583,7 +586,7 @@ export class Cookie { } } - cookieString() { + cookieString(): string { const val = this.value ?? '' if (this.key) { return `${this.key}=${val}` @@ -592,7 +595,7 @@ export class Cookie { } // gives Set-Cookie header format - toString() { + toString(): string { let str = this.cookieString() if (this.expires != 'Infinity') { @@ -690,14 +693,14 @@ export class Cookie { } // Mostly S5.1.2 and S5.2.3: - canonicalizedDomain() { + canonicalizedDomain(): string | null { if (this.domain == null) { return null } return canonicalDomain(this.domain) } - cdomain() { + cdomain(): string | null { return this.canonicalizedDomain() } diff --git a/lib/cookie/cookieCompare.ts b/lib/cookie/cookieCompare.ts index dd793f4e..f5a1a16a 100644 --- a/lib/cookie/cookieCompare.ts +++ b/lib/cookie/cookieCompare.ts @@ -17,7 +17,7 @@ import type { Cookie } from './cookie' const MAX_TIME = 2147483647000 /** Compares two cookies for sorting. */ -export function cookieCompare(a: Cookie, b: Cookie) { +export function cookieCompare(a: Cookie, b: Cookie): number { validators.validate(validators.isObject(a), safeToString(a)) validators.validate(validators.isObject(b), safeToString(b)) let cmp: number diff --git a/lib/cookie/cookieJar.ts b/lib/cookie/cookieJar.ts index 685f6ba0..a6aa6a8a 100644 --- a/lib/cookie/cookieJar.ts +++ b/lib/cookie/cookieJar.ts @@ -69,7 +69,7 @@ type CreateCookieJarOptions = { const SAME_SITE_CONTEXT_VAL_ERR = 'Invalid sameSiteContext option for getCookies(); expected one of "strict", "lax", or "none"' -function getCookieContext(url: unknown) { +function getCookieContext(url: unknown): URL | urlParse { if (url instanceof URL) { return url } else if (typeof url === 'string') { @@ -83,7 +83,8 @@ function getCookieContext(url: unknown) { } } -function checkSameSiteContext(value: string) { +type SameSiteLevel = keyof (typeof Cookie)['sameSiteLevel'] +function checkSameSiteContext(value: string): SameSiteLevel | null { validators.validate(validators.isNonEmptyString(value), value) const context = String(value).toLowerCase() if (context === 'none' || context === 'lax' || context === 'strict') { @@ -100,7 +101,7 @@ function checkSameSiteContext(value: string) { * @param cookie * @returns boolean */ -function isSecurePrefixConditionMet(cookie: Cookie) { +function isSecurePrefixConditionMet(cookie: Cookie): boolean { validators.validate(validators.isObject(cookie), safeToString(cookie)) const startsWithSecurePrefix = typeof cookie.key === 'string' && cookie.key.startsWith('__Secure-') @@ -118,20 +119,26 @@ function isSecurePrefixConditionMet(cookie: Cookie) { * @param cookie * @returns boolean */ -function isHostPrefixConditionMet(cookie: Cookie) { +function isHostPrefixConditionMet(cookie: Cookie): boolean { validators.validate(validators.isObject(cookie)) const startsWithHostPrefix = typeof cookie.key === 'string' && cookie.key.startsWith('__Host-') return ( !startsWithHostPrefix || - (cookie.secure && - cookie.hostOnly && - cookie.path != null && - cookie.path === '/') + Boolean( + cookie.secure && + cookie.hostOnly && + cookie.path != null && + cookie.path === '/', + ) ) } -function getNormalizedPrefixSecurity(prefixSecurity: string) { +type PrefixSecurityValue = + (typeof PrefixSecurityEnum)[keyof typeof PrefixSecurityEnum] +function getNormalizedPrefixSecurity( + prefixSecurity: string, +): PrefixSecurityValue { if (prefixSecurity != null) { const normalizedPrefixSecurity = prefixSecurity.toLowerCase() /* The three supported options */ @@ -561,7 +568,7 @@ export class CookieJar { const allPaths = options?.allPaths ?? false const store = this.store - function matchingCookie(c: Cookie) { + function matchingCookie(c: Cookie): boolean { // "Either: // The cookie's host-only-flag is true and the canonicalized // request-host is identical to the cookie's domain. @@ -846,12 +853,12 @@ export class CookieJar { return this.callSync((callback) => this.serialize(callback)) } - toJSON() { + toJSON(): SerializedCookieJar | undefined { return this.serializeSync() } // use the class method CookieJar.deserialize instead of calling this directly - _importCookies(serialized: unknown, callback: Callback) { + _importCookies(serialized: unknown, callback: Callback): void { let cookies: unknown[] | undefined = undefined if ( @@ -988,7 +995,7 @@ export class CookieJar { let completedCount = 0 const removeErrors: Error[] = [] - function removeCookieCb(removeErr: Error | null) { + function removeCookieCb(removeErr: Error | null): void { if (removeErr) { removeErrors.push(removeErr) } diff --git a/lib/cookie/formatDate.ts b/lib/cookie/formatDate.ts index c039c1da..faa93f7e 100644 --- a/lib/cookie/formatDate.ts +++ b/lib/cookie/formatDate.ts @@ -2,7 +2,7 @@ import * as validators from '../validators' import { safeToString } from '../utils' /** Converts a Date to a UTC string representation. */ -export function formatDate(date: Date) { +export function formatDate(date: Date): string { validators.validate(validators.isDate(date), safeToString(date)) return date.toUTCString() } diff --git a/lib/cookie/parseDate.ts b/lib/cookie/parseDate.ts index 57b193bc..f71ff307 100644 --- a/lib/cookie/parseDate.ts +++ b/lib/cookie/parseDate.ts @@ -32,7 +32,7 @@ function parseDigits( minDigits: number, maxDigits: number, trailingOK: boolean, -) { +): number | null { let count = 0 while (count < token.length) { const c = token.charCodeAt(count) @@ -55,7 +55,7 @@ function parseDigits( return parseInt(token.slice(0, count), 10) } -function parseTime(token: string) { +function parseTime(token: string): number[] | null { const parts = token.split(':') const result = [0, 0, 0] @@ -88,7 +88,7 @@ function parseTime(token: string) { return result } -function parseMonth(token: string) { +function parseMonth(token: string): number | null { token = String(token).slice(0, 3).toLowerCase() switch (token) { case 'jan': diff --git a/lib/memstore.ts b/lib/memstore.ts index b169da7b..c1e1c2f7 100644 --- a/lib/memstore.ts +++ b/lib/memstore.ts @@ -114,7 +114,7 @@ export class MemoryCookieStore extends Store { ) => void if (!path) { // null means "all paths" - pathMatcher = function matchAll(domainIndex) { + pathMatcher = function matchAll(domainIndex): void { for (const curPath in domainIndex) { const pathIndex = domainIndex[curPath] for (const key in pathIndex) { @@ -126,7 +126,7 @@ export class MemoryCookieStore extends Store { } } } else { - pathMatcher = function matchRFC(domainIndex) { + pathMatcher = function matchRFC(domainIndex): void { //NOTE: we should use path-match algorithm from S5.1.4 here //(see : https://github.com/ChromiumWebApps/chromium/blob/b3d3b4da8bb94c1b2e061600df106d590fda3620/net/cookies/canonical_cookie.cc#L299) for (const cookiePath in domainIndex) { diff --git a/lib/utils.ts b/lib/utils.ts index 83628f5c..d97a5ece 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -10,7 +10,7 @@ export interface ErrorCallback { } /** Wrapped `Object.prototype.toString`, so that you don't need to remember to use `.call()`. */ -export const objectToString = (obj: unknown) => +export const objectToString = (obj: unknown): string => Object.prototype.toString.call(obj) /** @@ -49,7 +49,7 @@ const safeToStringImpl = ( } /** Safely converts any value to string, using the value's own `toString` when available. */ -export const safeToString = (val: unknown) => safeToStringImpl(val) +export const safeToString = (val: unknown): string => safeToStringImpl(val) /** Utility object for promise/callback interop. */ export interface PromiseCallback { @@ -71,7 +71,7 @@ export function createPromiseCallback(cb?: Callback): PromiseCallback { }) if (typeof cb === 'function') { - callback = (err, result) => { + callback = (err, result): void => { try { if (err) cb(err) // If `err` is null, we know `result` must be `T` @@ -83,7 +83,7 @@ export function createPromiseCallback(cb?: Callback): PromiseCallback { } } } else { - callback = (err, result) => { + callback = (err, result): void => { try { // If `err` is null, we know `result` must be `T` // The assertion isn't *strictly* correct, as `T` could be nullish, but, ehh, good enough... @@ -98,11 +98,11 @@ export function createPromiseCallback(cb?: Callback): PromiseCallback { return { promise, callback, - resolve: (value: T) => { + resolve: (value: T): Promise => { callback(null, value) return promise }, - reject: (error: Error) => { + reject: (error: Error): Promise => { callback(error) return promise },