fix(NODE-3058): accept null or undefined anywhere we permit nullish values (#2921)

nbbeeken · web-flow · commit b42a1b417e8a · 2021-07-30T13:03:33.000-04:00
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -27,7 +27,7 @@
     "tsdoc/syntax": "warn",
 
     "no-console": "error",
-    "eqeqeq": ["error", "always", { "null": "ignore" }],
+    "eqeqeq": ["error", "always", {"null": "ignore"}],
     "strict": ["error", "global"],
 
     "@typescript-eslint/no-explicit-any": "off",
@@ -36,15 +36,30 @@
       "error",
       {
         "selector": "TSEnumDeclaration",
-        "message": "Don't declare enums"
+        "message": "Do not declare enums"
+      },
+      {
+        "selector": "BinaryExpression[operator=/[=!]==/] Identifier[name='undefined']",
+        "message": "Do not strictly check undefined"
+      },
+      {
+        "selector": "BinaryExpression[operator=/[=!]==/] Literal[raw='null']",
+        "message": "Do not strictly check null"
+      },
+      {
+        "selector": "BinaryExpression[operator=/[=!]==?/] Literal[value='undefined']",
+        "message": "Do not strictly check typeof undefined (NOTE: currently this rule only detects the usage of 'undefined' string literal so this could be a misfire)"
       }
     ]
   },
   "overrides": [{
     "files": ["*.d.ts"],
     "rules": {
       "prettier/prettier": "off",
-      "@typescript-eslint/no-empty-interface": "off"
+      "@typescript-eslint/no-empty-interface": "off",
+      "@typescript-eslint/no-misused-new": "off",
+      "@typescript-eslint/ban-types": "off",
+      "@typescript-eslint/no-unused-vars": "off"
     }
   },
   {
diff --git a/src/cmap/auth/scram.ts b/src/cmap/auth/scram.ts
@@ -311,7 +311,7 @@ const hiLengthMap = {
 function HI(data: string, salt: Buffer, iterations: number, cryptoMethod: CryptoMethod) {
   // omit the work if already generated
   const key = [data, salt.toString('base64'), iterations].join('_');
-  if (_hiCache[key] !== undefined) {
+  if (_hiCache[key] != null) {
     return _hiCache[key];
   }
 
diff --git a/src/cmap/command_monitoring_events.ts b/src/cmap/command_monitoring_events.ts
@@ -212,15 +212,15 @@ function extractCommand(command: WriteProtocolMessageType): Document {
       // up-convert legacy find command
       result = { find: collectionName(command) };
       Object.keys(LEGACY_FIND_QUERY_MAP).forEach(key => {
-        if (typeof command.query[key] !== 'undefined') {
+        if (command.query[key] != null) {
           result[LEGACY_FIND_QUERY_MAP[key]] = deepCopy(command.query[key]);
         }
       });
     }
 
     Object.keys(LEGACY_FIND_OPTIONS_MAP).forEach(key => {
       const legacyKey = key as keyof typeof LEGACY_FIND_OPTIONS_MAP;
-      if (typeof command[legacyKey] !== 'undefined') {
+      if (command[legacyKey] != null) {
         result[LEGACY_FIND_OPTIONS_MAP[legacyKey]] = deepCopy(command[legacyKey]);
       }
     });
@@ -232,7 +232,7 @@ function extractCommand(command: WriteProtocolMessageType): Document {
       }
     });
 
-    if (typeof command.pre32Limit !== 'undefined') {
+    if (command.pre32Limit != null) {
       result.limit = command.pre32Limit;
     }
 
@@ -286,7 +286,7 @@ function extractReply(command: WriteProtocolMessageType, reply?: Document) {
   }
 
   // is this a legacy find command?
-  if (command.query && typeof command.query.$query !== 'undefined') {
+  if (command.query && command.query.$query != null) {
     return {
       ok: 1,
       cursor: {
diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts
@@ -343,9 +343,9 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
     options: CommandOptions | undefined,
     callback: Callback
   ): void {
-    if (typeof ns.db === 'undefined' || typeof ns === 'string') {
+    if (!(ns instanceof MongoDBNamespace)) {
       // TODO(NODE-3483): Replace this with a MongoCommandError
-      throw new MongoDriverError('Namespace cannot be a string');
+      throw new MongoDriverError('Must provide a MongoDBNamespace instance');
     }
 
     const readPreference = getReadPreference(cmd, options);
@@ -453,7 +453,7 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
 
   /** @internal */
   query(ns: MongoDBNamespace, cmd: Document, options: QueryOptions, callback: Callback): void {
-    const isExplain = typeof cmd.$explain !== 'undefined';
+    const isExplain = cmd.$explain != null;
     const readPreference = options.readPreference ?? ReadPreference.primary;
     const batchSize = options.batchSize || 0;
     const limit = options.limit;
diff --git a/src/cmap/stream_description.ts b/src/cmap/stream_description.ts
@@ -51,7 +51,7 @@ export class StreamDescription {
   receiveResponse(response: Document): void {
     this.type = parseServerType(response);
     RESPONSE_FIELDS.forEach(field => {
-      if (typeof response[field] !== 'undefined') {
+      if (typeof response[field] != null) {
         this[field] = response[field];
       }
 
diff --git a/src/collection.ts b/src/collection.ts
@@ -699,7 +699,7 @@ export class Collection<TSchema extends Document = Document> {
     options?: FindOptions<TSchema> | Callback<TSchema | undefined>,
     callback?: Callback<TSchema>
   ): Promise<TSchema | undefined> | void {
-    if (callback !== undefined && typeof callback !== 'function') {
+    if (callback != null && typeof callback !== 'function') {
       throw new MongoInvalidArgumentError(
         'Third parameter to `findOne()` must be a callback or undefined'
       );
@@ -1088,7 +1088,7 @@ export class Collection<TSchema extends Document = Document> {
     options?: CountDocumentsOptions | Callback<number>,
     callback?: Callback<number>
   ): Promise<number> | void {
-    if (typeof filter === 'undefined') {
+    if (filter == null) {
       (filter = {}), (options = {}), (callback = undefined);
     } else if (typeof filter === 'function') {
       (callback = filter as Callback<number>), (filter = {}), (options = {});
diff --git a/src/connection_string.ts b/src/connection_string.ts
@@ -177,7 +177,7 @@ function toRecord(value: string): Record<string, any> {
   const keyValuePairs = value.split(',');
   for (const keyValue of keyValuePairs) {
     const [key, value] = keyValue.split(':');
-    if (typeof value === 'undefined') {
+    if (value == null) {
       throw new MongoParseError('Cannot have undefined values in key value pairs');
     }
     try {
@@ -219,7 +219,7 @@ export function parseOptions(
   mongoClient: MongoClient | MongoClientOptions | undefined = undefined,
   options: MongoClientOptions = {}
 ): MongoOptions {
-  if (typeof mongoClient !== 'undefined' && !(mongoClient instanceof MongoClient)) {
+  if (mongoClient != null && !(mongoClient instanceof MongoClient)) {
     options = mongoClient;
     mongoClient = undefined;
   }
@@ -284,7 +284,7 @@ export function parseOptions(
   }
 
   const objectOptions = new CaseInsensitiveMap(
-    Object.entries(options).filter(([, v]) => (v ?? null) !== null)
+    Object.entries(options).filter(([, v]) => v != null)
   );
 
   const allOptions = new CaseInsensitiveMap();
@@ -418,7 +418,7 @@ function setOption(
       mongoOptions[name] = getUint(name, values[0]);
       break;
     case 'string':
-      if (values[0] === undefined) {
+      if (values[0] == null) {
         break;
       }
       mongoOptions[name] = String(values[0]);
@@ -1051,6 +1051,6 @@ export const OPTIONS = {
 
 export const DEFAULT_OPTIONS = new CaseInsensitiveMap(
   Object.entries(OPTIONS)
-    .filter(([, descriptor]) => typeof descriptor.default !== 'undefined')
+    .filter(([, descriptor]) => descriptor.default != null)
     .map(([k, d]) => [k, d.default])
 );
diff --git a/src/cursor/abstract_cursor.ts b/src/cursor/abstract_cursor.ts
@@ -150,7 +150,7 @@ export abstract class AbstractCursor<
       this[kOptions].batchSize = options.batchSize;
     }
 
-    if (typeof options.comment !== 'undefined') {
+    if (options.comment != null) {
       this[kOptions].comment = options.comment;
     }
 
@@ -225,9 +225,7 @@ export abstract class AbstractCursor<
     return {
       next: () =>
         this.next().then(value =>
-          value !== null && value !== undefined
-            ? { value, done: false }
-            : { value: undefined, done: true }
+          value != null ? { value, done: false } : { value: undefined, done: true }
         )
     };
   }
@@ -817,7 +815,7 @@ function makeCursorStream<TSchema extends Document>(cursor: AbstractCursor<TSche
   function readNext() {
     needToClose = false;
     next(cursor, true, (err, result) => {
-      needToClose = err ? !cursor.closed : result !== null;
+      needToClose = err ? !cursor.closed : result != null;
 
       if (err) {
         // NOTE: This is questionable, but we have a test backing the behavior. It seems the
@@ -839,7 +837,7 @@ function makeCursorStream<TSchema extends Document>(cursor: AbstractCursor<TSche
         return readable.destroy(err);
       }
 
-      if (result === null) {
+      if (result == null) {
         readable.push(null);
       } else if (readable.destroyed) {
         cursor.close();
diff --git a/src/cursor/aggregation_cursor.ts b/src/cursor/aggregation_cursor.ts
@@ -86,7 +86,7 @@ export class AggregationCursor<TSchema = Document> extends AbstractCursor<TSchem
     callback?: Callback<Document>
   ): Promise<Document> | void {
     if (typeof verbosity === 'function') (callback = verbosity), (verbosity = true);
-    if (verbosity === undefined) verbosity = true;
+    if (verbosity == null) verbosity = true;
 
     return executeOperation(
       this.topology,
diff --git a/src/cursor/find_cursor.ts b/src/cursor/find_cursor.ts
@@ -52,7 +52,7 @@ export class FindCursor<TSchema = Document> extends AbstractCursor<TSchema> {
     this[kFilter] = filter || {};
     this[kBuiltOptions] = options;
 
-    if (typeof options.sort !== 'undefined') {
+    if (options.sort != null) {
       this[kBuiltOptions].sort = formatSort(options.sort);
     }
   }
@@ -155,7 +155,7 @@ export class FindCursor<TSchema = Document> extends AbstractCursor<TSchema> {
     callback?: Callback<Document>
   ): Promise<Document> | void {
     if (typeof verbosity === 'function') (callback = verbosity), (verbosity = true);
-    if (verbosity === undefined) verbosity = true;
+    if (verbosity == null) verbosity = true;
 
     return executeOperation(
       this.topology,
diff --git a/src/error.ts b/src/error.ts
@@ -898,7 +898,7 @@ export function isResumableError(error?: MongoError, wireVersion?: number): bool
     return true;
   }
 
-  if (typeof wireVersion !== 'undefined' && wireVersion >= 9) {
+  if (wireVersion != null && wireVersion >= 9) {
     // DRIVERS-1308: For 4.4 drivers running against 4.4 servers, drivers will add a special case to treat the CursorNotFound error code as resumable
     if (error && error instanceof MongoError && error.code === 43) {
       return true;
diff --git a/src/explain.ts b/src/explain.ts
@@ -40,7 +40,7 @@ export class Explain {
   }
 
   static fromOptions(options?: ExplainOptions): Explain | undefined {
-    if (options?.explain === undefined) return;
+    if (options?.explain == null) return;
 
     const explain = options.explain;
     if (typeof explain === 'boolean' || typeof explain === 'string') {
diff --git a/src/operations/command.ts b/src/operations/command.ts
@@ -97,14 +97,14 @@ export abstract class CommandOperation<T> extends AbstractOperation<T> {
 
     if (this.hasAspect(Aspect.EXPLAINABLE)) {
       this.explain = Explain.fromOptions(options);
-    } else if (options?.explain !== undefined) {
+    } else if (options?.explain != null) {
       throw new MongoInvalidArgumentError(`Option "explain" is not supported on this command`);
     }
   }
 
   get canRetryWrite(): boolean {
     if (this.hasAspect(Aspect.EXPLAINABLE)) {
-      return this.explain === undefined;
+      return this.explain == null;
     }
     return true;
   }
diff --git a/src/operations/count.ts b/src/operations/count.ts
@@ -47,7 +47,7 @@ export class CountOperation extends CommandOperation<number> {
       cmd.skip = options.skip;
     }
 
-    if (typeof options.hint !== 'undefined') {
+    if (options.hint != null) {
       cmd.hint = options.hint;
     }
 
diff --git a/src/operations/delete.ts b/src/operations/delete.ts
@@ -64,7 +64,7 @@ export class DeleteOperation extends CommandOperation<Document> {
       return false;
     }
 
-    return this.statements.every(op => (typeof op.limit !== 'undefined' ? op.limit > 0 : true));
+    return this.statements.every(op => (op.limit != null ? op.limit > 0 : true));
   }
 
   execute(server: Server, session: ClientSession, callback: Callback): void {
@@ -80,7 +80,7 @@ export class DeleteOperation extends CommandOperation<Document> {
       command.let = options.let;
     }
 
-    if (options.explain !== undefined && maxWireVersion(server) < 3) {
+    if (options.explain != null && maxWireVersion(server) < 3) {
       return callback
         ? callback(
             new MongoCompatibilityError(`Server ${server.name} does not support explain on delete`)
diff --git a/src/operations/find.ts b/src/operations/find.ts
@@ -106,7 +106,7 @@ export class FindOperation extends CommandOperation<Document> {
 
     const serverWireVersion = maxWireVersion(server);
     const options = this.options;
-    if (typeof options.allowDiskUse !== 'undefined' && serverWireVersion < 4) {
+    if (options.allowDiskUse != null && serverWireVersion < 4) {
       callback(
         new MongoCompatibilityError('Option "allowDiskUse" is not supported on MongoDB < 3.2')
       );
@@ -338,7 +338,7 @@ function makeLegacyFindCommand(
     findCommand.$maxTimeMS = options.maxTimeMS;
   }
 
-  if (typeof options.explain !== 'undefined') {
+  if (options.explain != null) {
     findCommand.$explain = true;
   }
 
diff --git a/src/operations/insert.ts b/src/operations/insert.ts
@@ -37,7 +37,7 @@ export class InsertOperation extends CommandOperation<Document> {
       command.bypassDocumentValidation = options.bypassDocumentValidation;
     }
 
-    if (typeof options.comment !== 'undefined') {
+    if (options.comment != null) {
       command.comment = options.comment;
     }
 
diff --git a/src/operations/update.ts b/src/operations/update.ts
@@ -159,7 +159,7 @@ export class UpdateOneOperation extends UpdateOperation {
   ): void {
     super.execute(server, session, (err, res) => {
       if (err || !res) return callback(err);
-      if (typeof this.explain !== 'undefined') return callback(undefined, res);
+      if (this.explain != null) return callback(undefined, res);
       if (res.code) return callback(new MongoServerError(res));
       if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0]));
 
@@ -196,7 +196,7 @@ export class UpdateManyOperation extends UpdateOperation {
   ): void {
     super.execute(server, session, (err, res) => {
       if (err || !res) return callback(err);
-      if (typeof this.explain !== 'undefined') return callback(undefined, res);
+      if (this.explain != null) return callback(undefined, res);
       if (res.code) return callback(new MongoServerError(res));
       if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0]));
 
@@ -250,7 +250,7 @@ export class ReplaceOneOperation extends UpdateOperation {
   ): void {
     super.execute(server, session, (err, res) => {
       if (err || !res) return callback(err);
-      if (typeof this.explain !== 'undefined') return callback(undefined, res);
+      if (this.explain != null) return callback(undefined, res);
       if (res.code) return callback(new MongoServerError(res));
       if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0]));
 
diff --git a/src/read_preference.ts b/src/read_preference.ts
@@ -86,7 +86,7 @@ export class ReadPreference {
     if (!ReadPreference.isValid(mode)) {
       throw new MongoInvalidArgumentError(`Invalid read preference mode ${JSON.stringify(mode)}`);
     }
-    if (options === undefined && typeof tags === 'object' && !Array.isArray(tags)) {
+    if (options == null && typeof tags === 'object' && !Array.isArray(tags)) {
       options = tags;
       tags = undefined;
     } else if (tags && !Array.isArray(tags)) {
diff --git a/src/sdam/topology_description.ts b/src/sdam/topology_description.ts
@@ -113,7 +113,7 @@ export class TopologyDescription {
           break;
         }
 
-        if (this.logicalSessionTimeoutMinutes === undefined) {
+        if (this.logicalSessionTimeoutMinutes == null) {
           // First server with a non null logicalSessionsTimeout
           this.logicalSessionTimeoutMinutes = server.logicalSessionTimeoutMinutes;
           continue;
diff --git a/src/sessions.ts b/src/sessions.ts
@@ -854,7 +854,7 @@ export function applySession(
       Object.assign(command.readConcern, { afterClusterTime: session.operationTime });
     } else if (session[kSnapshotEnabled]) {
       command.readConcern = command.readConcern || { level: ReadConcernLevel.snapshot };
-      if (session[kSnapshotTime] !== undefined) {
+      if (session[kSnapshotTime] != null) {
         Object.assign(command.readConcern, { atClusterTime: session[kSnapshotTime] });
       }
     }
@@ -897,7 +897,7 @@ export function updateSessionFromResponse(session: ClientSession, document: Docu
     session.transaction._recoveryToken = document.recoveryToken;
   }
 
-  if (session?.[kSnapshotEnabled] && session[kSnapshotTime] === undefined) {
+  if (session?.[kSnapshotEnabled] && session[kSnapshotTime] == null) {
     // find and aggregate commands return atClusterTime on the cursor
     // distinct includes it in the response body
     const atClusterTime = document.cursor?.atClusterTime || document.atClusterTime;
diff --git a/src/sort.ts b/src/sort.ts
diff --git a/src/utils.ts b/src/utils.ts

Original file line number	Diff line number	Diff line change
`@@ -311,7 +311,7 @@ const hiLengthMap = {`
`311`	`311`	`function HI(data: string, salt: Buffer, iterations: number, cryptoMethod: CryptoMethod) {`
`312`	`312`	`// omit the work if already generated`
`313`	`313`	`const key = [data, salt.toString('base64'), iterations].join('_');`
`314`		`- if (_hiCache[key] !== undefined) {`
	`314`	`+ if (_hiCache[key] != null) {`
`315`	`315`	`return _hiCache[key];`
`316`	`316`	`}`
`317`	`317`
Original file line number	Diff line number	Diff line change
`@@ -212,15 +212,15 @@ function extractCommand(command: WriteProtocolMessageType): Document {`
`212`	`212`	`// up-convert legacy find command`
`213`	`213`	`result = { find: collectionName(command) };`
`214`	`214`	`Object.keys(LEGACY_FIND_QUERY_MAP).forEach(key => {`
`215`		`- if (typeof command.query[key] !== 'undefined') {`
	`215`	`+ if (command.query[key] != null) {`
`216`	`216`	`result[LEGACY_FIND_QUERY_MAP[key]] = deepCopy(command.query[key]);`
`217`	`217`	`}`
`218`	`218`	`});`
`219`	`219`	`}`
`220`	`220`
`221`	`221`	`Object.keys(LEGACY_FIND_OPTIONS_MAP).forEach(key => {`
`222`	`222`	`const legacyKey = key as keyof typeof LEGACY_FIND_OPTIONS_MAP;`
`223`		`- if (typeof command[legacyKey] !== 'undefined') {`
	`223`	`+ if (command[legacyKey] != null) {`
`224`	`224`	`result[LEGACY_FIND_OPTIONS_MAP[legacyKey]] = deepCopy(command[legacyKey]);`
`225`	`225`	`}`
`226`	`226`	`});`
`@@ -232,7 +232,7 @@ function extractCommand(command: WriteProtocolMessageType): Document {`
`232`	`232`	`}`
`233`	`233`	`});`
`234`	`234`
`235`		`- if (typeof command.pre32Limit !== 'undefined') {`
	`235`	`+ if (command.pre32Limit != null) {`
`236`	`236`	`result.limit = command.pre32Limit;`
`237`	`237`	`}`
`238`	`238`
`@@ -286,7 +286,7 @@ function extractReply(command: WriteProtocolMessageType, reply?: Document) {`
`286`	`286`	`}`
`287`	`287`
`288`	`288`	`// is this a legacy find command?`
`289`		`- if (command.query && typeof command.query.$query !== 'undefined') {`
	`289`	`+ if (command.query && command.query.$query != null) {`
`290`	`290`	`return {`
`291`	`291`	`ok: 1,`
`292`	`292`	`cursor: {`
Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@ export class StreamDescription {`
`51`	`51`	`receiveResponse(response: Document): void {`
`52`	`52`	`this.type = parseServerType(response);`
`53`	`53`	`RESPONSE_FIELDS.forEach(field => {`
`54`		`- if (typeof response[field] !== 'undefined') {`
	`54`	`+ if (typeof response[field] != null) {`
`55`	`55`	`this[field] = response[field];`
`56`	`56`	`}`
`57`	`57`