Enable strict type checked rules (#392)

* Make usage of null/undefined more consistent.

Primarily allowing either value for user input.

* add explicit return type to canonicalDomain

* add note for weird parse behavior

* use @typescript-eslint/strict-type-checked

* eslint auto fixes

* remove unnecessary polyfill

* manual fixes for strict rules

* Update lib/cookie/cookie.ts

Co-authored-by: Colin Casey <[email protected]>

* Replace `Nullable<T>` with more precise `T | undefined`

* add a bit of breathing room to the file size

* Change `parse` to only return undefined on failure, nut null | undefined.

* standardize helpers on returning undefined

* update doc for return type

* Fix a few more errors.

* appease linter

* change fromJSON to return undefined instead of null

* fix incorrect comparison

* change date helpers to avoid null, for consistency

* update fromJSON tests for new return type

* change internal null value to undefined

* add type annotations

* move from .eslintrc to flat config

* linter fixes for vows tests

* fixes for restrict-template-expressions

* Avoid type linting eslint.config.mjs

* fix package-lock.json

* bump deps

* fix package-lock.json again

* restore missing dependency

* revert changes to test files

* disable no-unused-vars lint rule

* fix no-case-declarations violation

* remove max-lines enforcement

we've decided it doesn't really add value

* fix types and disable false positive eslint errors

* fix package-lock.json

I always mangle it :(

* don't need to disable a rule that's never enabled

missed a spot

---------

Co-authored-by: Colin Casey <[email protected]>

Author: wjhsf

.eslintignore

@@ -0,0 +1,41 @@
+// @ts-check
+
+import eslint from '@eslint/js'
+import prettierRecommended from 'eslint-plugin-prettier/recommended'
+import tseslint from 'typescript-eslint'
+import globals from 'globals'
+
+export default tseslint.config(
+  {
+    ignores: ['dist', 'jest.config.ts'],
+  },
+  eslint.configs.recommended,
+  ...tseslint.configs.strictTypeChecked,
+  prettierRecommended,
+  {
+    languageOptions: {
+      globals: globals.node,
+      parserOptions: {
+        project: './tsconfig.json',
+        tsconfigDirName: import.meta.dirname,
+      },
+    },
+    linterOptions: {
+      reportUnusedDisableDirectives: 'warn',
+    },
+    rules: {
+      '@typescript-eslint/explicit-function-return-type': 'error',
+    },
+  },
+  {
+    // Once we remove the legacy vows tests in ./test, we can remove these JS-specific rules
+    files: ['test/**/*.js', 'eslint.config.mjs'],
+    ...tseslint.configs.disableTypeChecked,
+    rules: {
+      ...tseslint.configs.disableTypeChecked.rules,
+      '@typescript-eslint/explicit-function-return-type': 'off',
+      '@typescript-eslint/no-var-requires': 'off',
+      '@typescript-eslint/no-unused-vars': 'off',
+    },
+  },
+)

.eslintrc.json

@@ -909,7 +909,9 @@ describe('CookieJar', () => {
       'should remove all the stored cookies',
       {
         callbackStyle(done) {
-          cookieJar.removeAllCookies(() => done())
+          cookieJar.removeAllCookies(() => {
+            done()
+          })
         },
         async asyncStyle() {
           await cookieJar.removeAllCookies()
@@ -1348,7 +1350,7 @@ describe.each(['local', 'example', 'invalid', 'localhost', 'test'])(
           expect.objectContaining({
             key: 'settingThisShouldPass',
             value: 'true',
-            domain: `${specialUseDomain}`,
+            domain: specialUseDomain,
           }),
         )
         const cookies = await cookieJar.getCookies(
@@ -1393,7 +1395,7 @@ describe.each(['local', 'example', 'invalid', 'localhost', 'test'])(
           expect.objectContaining({
             key: 'settingThisShouldPass',
             value: 'true',
-            domain: `${specialUseDomain}`,
+            domain: specialUseDomain,
           }),
         )
         const cookies = await cookieJar.getCookies(
@@ -1480,9 +1482,9 @@ describe('Synchronous API on async CookieJar', () => {
 
   it('should throw an error when calling `removeAllCookiesSync` if store is not synchronous', () => {
     const cookieJar = new CookieJar(store)
-    expect(() => cookieJar.removeAllCookiesSync()).toThrow(
-      'CookieJar store is not synchronous; use async API instead.',
-    )
+    expect(() => {
+      cookieJar.removeAllCookiesSync()
+    }).toThrow('CookieJar store is not synchronous; use async API instead.')
   })
 })
 
@@ -1496,7 +1498,7 @@ function createCookie(
   if (!cookie) {
     throw new Error('This should not be undefined')
   }
-  if (options?.hostOnly) {
+  if (options.hostOnly) {
     cookie.hostOnly = options.hostOnly
   }
   return cookie
@@ -1508,9 +1510,11 @@ function apiVariants(
   assertions: () => void,
 ): void {
   it(`${testName} (callback)`, async () => {
-    await new Promise((resolve) =>
-      apiVariants.callbackStyle(() => resolve(undefined)),
-    )
+    await new Promise((resolve) => {
+      apiVariants.callbackStyle(() => {
+        resolve(undefined)
+      })
+    })
     assertions()
   })
 

eslint.config.mjs

@@ -1,4 +1,3 @@
-// This file just contains test data, so we don't care about the number of lines.
 export default [
   {
     test: '0001',

lib/__tests__/cookieJar.spec.ts

@@ -306,7 +306,9 @@ function expectDataToMatchSerializationSchema(
   expect(serializedJar.storeType).toBe('MemoryCookieStore')
   expect(serializedJar.rejectPublicSuffixes).toBe(true)
   expect(serializedJar.cookies).toBeInstanceOf(Array)
-  serializedJar.cookies.forEach((cookie) => validateSerializedCookie(cookie))
+  serializedJar.cookies.forEach((cookie) => {
+    validateSerializedCookie(cookie)
+  })
 }
 
 const serializedCookiePropTypes: { [key: string]: string } = {
@@ -345,7 +347,7 @@ function validateSerializedCookie(cookie: SerializedCookie): void {
 
       case 'intOrInf':
         if (cookie[prop] !== 'Infinity' && cookie[prop] !== '-Infinity') {
-          expect(isInteger(cookie[prop])).toBe(true)
+          expect(Number.isInteger(cookie[prop])).toBe(true)
         }
         break
 
@@ -361,14 +363,3 @@ function validateSerializedCookie(cookie: SerializedCookie): void {
     }
   })
 }
-
-function isInteger(value: unknown): boolean {
-  if (Number.isInteger) {
-    return Number.isInteger(value)
-  }
-  // Node 0.10 (still supported) doesn't have Number.isInteger
-  // from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isInteger
-  return (
-    typeof value === 'number' && isFinite(value) && Math.floor(value) === value
-  )
-}

lib/__tests__/data/parser.ts

@@ -33,18 +33,18 @@ describe('store removeAllCookies API', () => {
       // replace remove cookie behavior to throw an error on the 4th invocation
       const _removeCookie = store.removeCookie.bind(store)
       const spy = jest.spyOn(store, 'removeCookie')
-      spy.mockImplementationOnce((domain, path, key, callback) =>
-        _removeCookie.call(store, domain, path, key, callback),
-      )
-      spy.mockImplementationOnce((domain, path, key, callback) =>
-        _removeCookie.call(store, domain, path, key, callback),
-      )
-      spy.mockImplementationOnce((domain, path, key, callback) =>
-        _removeCookie.call(store, domain, path, key, callback),
-      )
-      spy.mockImplementationOnce((_domain, _path, _key, callback) =>
-        callback(new Error('something happened 4')),
-      )
+      spy.mockImplementationOnce((domain, path, key, callback) => {
+        _removeCookie.call(store, domain, path, key, callback)
+      })
+      spy.mockImplementationOnce((domain, path, key, callback) => {
+        _removeCookie.call(store, domain, path, key, callback)
+      })
+      spy.mockImplementationOnce((domain, path, key, callback) => {
+        _removeCookie.call(store, domain, path, key, callback)
+      })
+      spy.mockImplementationOnce((_domain, _path, _key, callback) => {
+        callback(new Error('something happened 4'))
+      })
 
       await expect(jar.removeAllCookies()).rejects.toThrow(
         'something happened 4',
@@ -71,11 +71,12 @@ describe('store removeAllCookies API', () => {
       const spy = jest.spyOn(store, 'removeCookie')
       spy.mockImplementation((domain, path, key, callback) => {
         if (spy.mock.calls.length % 2 === 1) {
-          return callback(
-            new Error(`something happened ${spy.mock.calls.length}`),
+          callback(
+            new Error(`something happened ${String(spy.mock.calls.length)}`),
           )
+          return
         }
-        return _removeCookie.call(store, domain, path, key, callback)
+        _removeCookie.call(store, domain, path, key, callback)
       })
 
       await expect(jar.removeAllCookies()).rejects.toThrowError(
@@ -143,7 +144,8 @@ class StoreWithoutRemoveAll extends Store {
     if (!callback) {
       throw new Error('This should not be undefined')
     }
-    return callback(null, undefined)
+    callback(null, undefined)
+    return
   }
 
   override findCookies(
@@ -166,7 +168,8 @@ class StoreWithoutRemoveAll extends Store {
     if (!callback) {
       throw new Error('This should not be undefined')
     }
-    return callback(null, [])
+    callback(null, [])
+    return
   }
 
   override putCookie(cookie: Cookie): Promise<void>
@@ -177,7 +180,8 @@ class StoreWithoutRemoveAll extends Store {
     if (!callback) {
       throw new Error('This should not be undefined')
     }
-    return callback(null)
+    callback(null)
+    return
   }
 
   override getAllCookies(): Promise<Cookie[]>
@@ -187,7 +191,8 @@ class StoreWithoutRemoveAll extends Store {
     if (!callback) {
       throw new Error('This should not be undefined')
     }
-    return callback(null, this.cookies.slice())
+    callback(null, this.cookies.slice())
+    return
   }
 
   override removeCookie(
@@ -211,7 +216,8 @@ class StoreWithoutRemoveAll extends Store {
     if (!callback) {
       throw new Error('This should not be undefined')
     }
-    return callback(null)
+    callback(null)
+    return
   }
 }
 
@@ -234,7 +240,8 @@ class MemoryStoreExtension extends MemoryCookieStore {
     if (!callback) {
       throw new Error('This should not be undefined')
     }
-    return super.getAllCookies(callback)
+    super.getAllCookies(callback)
+    return
   }
 
   override removeCookie(
@@ -258,7 +265,8 @@ class MemoryStoreExtension extends MemoryCookieStore {
     if (!callback) {
       throw new Error('This should not be undefined')
     }
-    return super.removeCookie(domain, path, key, callback)
+    super.removeCookie(domain, path, key, callback)
+    return
   }
 
   override removeAllCookies(): Promise<void>
@@ -268,6 +276,7 @@ class MemoryStoreExtension extends MemoryCookieStore {
     if (!callback) {
       throw new Error('This should not be undefined')
     }
-    return super.removeAllCookies(callback)
+    super.removeAllCookies(callback)
+    return
   }
 }

lib/__tests__/jarSerialization.spec.ts

@@ -164,7 +164,7 @@ function parse(str: string, options?: ParseCookieOptions): Cookie | undefined {
       continue
     }
     const av_sep = av.indexOf('=')
-    let av_key, av_value
+    let av_key: string, av_value: string | null
 
     if (av_sep === -1) {
       av_key = av
@@ -551,11 +551,11 @@ export class Cookie {
     const hostOnly = this.hostOnly != null ? this.hostOnly.toString() : '?'
     const createAge =
       this.creation && this.creation !== 'Infinity'
-        ? `${now - this.creation.getTime()}ms`
+        ? `${String(now - this.creation.getTime())}ms`
         : '?'
     const accessAge =
       this.lastAccessed && this.lastAccessed !== 'Infinity'
-        ? `${now - this.lastAccessed.getTime()}ms`
+        ? `${String(now - this.lastAccessed.getTime())}ms`
         : '?'
     return `Cookie="${this.toString()}; hostOnly=${hostOnly}; aAge=${accessAge}; cAge=${createAge}"`
   }
@@ -657,7 +657,7 @@ export class Cookie {
    * @beta
    */
   validate(): boolean {
-    if (this.value == null || !COOKIE_OCTETS.test(this.value)) {
+    if (!this.value || !COOKIE_OCTETS.test(this.value)) {
       return false
     }
     if (
@@ -732,7 +732,7 @@ export class Cookie {
    * @public
    */
   cookieString(): string {
-    const val = this.value ?? ''
+    const val = this.value || ''
     if (this.key) {
       return `${this.key}=${val}`
     }
@@ -753,7 +753,7 @@ export class Cookie {
     }
 
     if (this.maxAge != null && this.maxAge != Infinity) {
-      str += `; Max-Age=${this.maxAge}`
+      str += `; Max-Age=${String(this.maxAge)}`
     }
 
     if (this.domain && !this.hostOnly) {

lib/__tests__/removeAll.spec.ts

@@ -88,7 +88,7 @@ export function cookieCompare(a: Cookie, b: Cookie): number {
   }
 
   // break ties for the same millisecond (precision of JavaScript's clock)
-  cmp = (a.creationIndex ?? 0) - (b.creationIndex ?? 0)
+  cmp = (a.creationIndex || 0) - (b.creationIndex || 0)
 
   return cmp
 }