Avoid using arguments (#316)

* Ignore max-lines where appropriate.

* Change callback error signature to null instead of undefined.

* Remove unnecessary helper.

* Split error callback and success callback for better type narrowing.

* Be more strict about resolving/rejecting values.

* Change `promiseCallback` to only accept a callback, not `arguments`.

* Hide annoying warnings that pop up in IDE.

* Handle different function signatures for callback argument.

* Use more normal callback definition.

* Modern syntax!

* Revert "Remove unnecessary helper." Turns out it's necessary.

This reverts commit f2df0bb.

.eslintrc.json

@@ -13,10 +13,6 @@
   },
   "reportUnusedDisableDirectives": true,
   "rules": {
-    // this is only needed because `createPromiseCallback` uses arguments to support the callback-style api
-    // we should strongly consider dropping the callback and sync api variants (in say v6) to reduce the
-    // surface area and complexity of tough-cookie
-    "prefer-rest-params": "warn",
     "max-lines": ["warn", 500]
   }
 }

lib/__tests__/cookieJar.spec.ts

@@ -49,7 +49,7 @@ describe('CookieJar', () => {
   })
 
   describe('setCookie', () => {
-    let cookie: Cookie
+    let cookie: Cookie | undefined
 
     apiVariants(
       'should resolve to a Cookie',
@@ -83,8 +83,8 @@ describe('CookieJar', () => {
       },
       () => {
         expect(cookie).toBeInstanceOf(Cookie)
-        expect(cookie.key).toBe('foo')
-        expect(cookie.value).toBe('bar')
+        expect(cookie?.key).toBe('foo')
+        expect(cookie?.value).toBe('bar')
       },
     )
 
@@ -185,8 +185,8 @@ describe('CookieJar', () => {
           lastAccessed: t1,
         }),
       )
-      expect(cookie.TTL()).toBe(Infinity)
-      expect(cookie.isPersistent()).toBe(false)
+      expect(cookie?.TTL()).toBe(Infinity)
+      expect(cookie?.isPersistent()).toBe(false)
 
       // updates the last access when retrieving a cookie
       jest.advanceTimersByTime(10000)
@@ -279,7 +279,7 @@ describe('CookieJar', () => {
         'a=b; Domain=example.com; Path=/',
         'http://www.app.example.com/index.html',
       )
-      expect(cookie.domain).toBe('example.com')
+      expect(cookie?.domain).toBe('example.com')
     })
 
     it('should allow a sub-path cookie on a super-domain', async () => {
@@ -348,8 +348,8 @@ describe('CookieJar', () => {
           lastAccessed: t0,
         }),
       )
-      expect(cookie.TTL()).toBe(Infinity)
-      expect(cookie.isPersistent()).toBe(false)
+      expect(cookie?.TTL()).toBe(Infinity)
+      expect(cookie?.isPersistent()).toBe(false)
     })
   })
 
@@ -800,7 +800,7 @@ describe('CookieJar', () => {
   })
 
   describe('getSetCookieStrings', () => {
-    let cookieHeaders: string[]
+    let cookieHeaders: string[] | undefined
 
     describe('api', () => {
       beforeEach(async () => {

lib/__tests__/data/parser.ts

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

lib/__tests__/removeAll.spec.ts

@@ -138,23 +138,23 @@ class StoreWithoutRemoveAll extends Store {
     domain: string,
     path: string,
     key: string,
-  ): Promise<Cookie>
+  ): Promise<Cookie | undefined>
   override findCookie(
     domain: string,
     path: string,
     key: string,
-    callback: Callback<Cookie>,
+    callback: Callback<Cookie | undefined>,
   ): void
   override findCookie(
     _domain: string,
     _path: string,
     _key: string,
-    callback?: Callback<Cookie>,
+    callback?: Callback<Cookie | undefined>,
   ): unknown {
     if (!callback) {
       throw new Error('This should not be undefined')
     }
-    return callback(undefined, undefined)
+    return callback(null, undefined)
   }
 
   override findCookies(
@@ -177,7 +177,7 @@ class StoreWithoutRemoveAll extends Store {
     if (!callback) {
       throw new Error('This should not be undefined')
     }
-    return callback(undefined, [])
+    return callback(null, [])
   }
 
   override putCookie(cookie: Cookie): Promise<void>
@@ -188,7 +188,7 @@ class StoreWithoutRemoveAll extends Store {
     if (!callback) {
       throw new Error('This should not be undefined')
     }
-    return callback(undefined)
+    return callback(null)
   }
 
   override getAllCookies(): Promise<Cookie[]>
@@ -198,7 +198,7 @@ class StoreWithoutRemoveAll extends Store {
     if (!callback) {
       throw new Error('This should not be undefined')
     }
-    return callback(undefined, this.cookies.slice())
+    return callback(null, this.cookies.slice())
   }
 
   override removeCookie(
@@ -222,7 +222,7 @@ class StoreWithoutRemoveAll extends Store {
     if (!callback) {
       throw new Error('This should not be undefined')
     }
-    return callback(undefined)
+    return callback(null)
   }
 }
 

lib/cookie/cookie.ts

@@ -29,6 +29,9 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
+// This file was too big before we added max-lines, and it's ongoing work to reduce its size.
+/* eslint max-lines: [1, 750] */
+
 import * as pubsuffix from '../pubsuffix-psl'
 import * as validators from '../validators'
 import { getCustomInspectSymbol } from '../utilHelper'
@@ -540,7 +543,11 @@ export class Cookie {
     ) {
       return false
     }
-    if (this.maxAge != null && this.maxAge <= 0) {
+    if (
+      this.maxAge != null &&
+      this.maxAge !== 'Infinity' &&
+      (this.maxAge === '-Infinity' || this.maxAge <= 0)
+    ) {
       return false // "Max-Age=" non-zero-digit *DIGIT
     }
     if (this.path != null && !PATH_VALUE.test(this.path)) {