Skip to content

Commit

Permalink
Enforce explicit function return type (#383)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
wjhsf committed Mar 15, 2024
1 parent b775575 commit 07c6b13
Show file tree
Hide file tree
Showing 13 changed files with 59 additions and 48 deletions.
1 change: 1 addition & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
},
"reportUnusedDisableDirectives": true,
"rules": {
"@typescript-eslint/explicit-function-return-type": "error",
"max-lines": ["warn", 500]
}
}
6 changes: 3 additions & 3 deletions lib/__tests__/cookieJar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
})

Expand Down Expand Up @@ -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),
})

Expand Down Expand Up @@ -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)),
Expand Down
6 changes: 3 additions & 3 deletions lib/__tests__/jarSerialization.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`)
Expand All @@ -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)

Expand Down Expand Up @@ -362,7 +362,7 @@ function validateSerializedCookie(cookie: SerializedCookie) {
})
}

function isInteger(value: unknown) {
function isInteger(value: unknown): boolean {
if (Number.isInteger) {
return Number.isInteger(value)
}
Expand Down
4 changes: 2 additions & 2 deletions lib/__tests__/regression.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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({
Expand Down
2 changes: 1 addition & 1 deletion lib/__tests__/util.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/cookie/canonicalDomain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
27 changes: 15 additions & 12 deletions lib/cookie/cookie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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)

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -565,15 +568,15 @@ export class Cookie {
return true
}

setExpires(exp: string | Date) {
setExpires(exp: string | Date): void {
if (exp instanceof Date) {
this.expires = exp
} else {
this.expires = parseDate(exp) || 'Infinity'
}
}

setMaxAge(age: number) {
setMaxAge(age: number): void {
if (age === Infinity) {
this.maxAge = 'Infinity'
} else if (age === -Infinity) {
Expand All @@ -583,7 +586,7 @@ export class Cookie {
}
}

cookieString() {
cookieString(): string {
const val = this.value ?? ''
if (this.key) {
return `${this.key}=${val}`
Expand All @@ -592,7 +595,7 @@ export class Cookie {
}

// gives Set-Cookie header format
toString() {
toString(): string {
let str = this.cookieString()

if (this.expires != 'Infinity') {
Expand Down Expand Up @@ -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()
}

Expand Down
2 changes: 1 addition & 1 deletion lib/cookie/cookieCompare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 20 additions & 13 deletions lib/cookie/cookieJar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> {
if (url instanceof URL) {
return url
} else if (typeof url === 'string') {
Expand All @@ -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') {
Expand All @@ -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-')
Expand All @@ -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 */
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<CookieJar>) {
_importCookies(serialized: unknown, callback: Callback<CookieJar>): void {
let cookies: unknown[] | undefined = undefined

if (
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion lib/cookie/formatDate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
6 changes: 3 additions & 3 deletions lib/cookie/parseDate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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]

Expand Down Expand Up @@ -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':
Expand Down
4 changes: 2 additions & 2 deletions lib/memstore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down

0 comments on commit 07c6b13

Please sign in to comment.