Skip to content

Commit

Permalink
fix(NODE-3058): accept null or undefined anywhere we permit nullish v…
Browse files Browse the repository at this point in the history
…alues (#2921)
  • Loading branch information
nbbeeken authored and ljhaywar committed Nov 9, 2021
1 parent 6206ff4 commit 1d6abc9
Show file tree
Hide file tree
Showing 23 changed files with 60 additions and 47 deletions.
21 changes: 18 additions & 3 deletions .eslintrc.json
Expand Up @@ -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",
Expand All @@ -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"
}
},
{
Expand Down
2 changes: 1 addition & 1 deletion src/cmap/auth/scram.ts
Expand Up @@ -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];
}

Expand Down
8 changes: 4 additions & 4 deletions src/cmap/command_monitoring_events.ts
Expand Up @@ -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]);
}
});
Expand All @@ -232,7 +232,7 @@ function extractCommand(command: WriteProtocolMessageType): Document {
}
});

if (typeof command.pre32Limit !== 'undefined') {
if (command.pre32Limit != null) {
result.limit = command.pre32Limit;
}

Expand Down Expand Up @@ -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: {
Expand Down
6 changes: 3 additions & 3 deletions src/cmap/connection.ts
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/cmap/stream_description.ts
Expand Up @@ -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];
}

Expand Down
4 changes: 2 additions & 2 deletions src/collection.ts
Expand Up @@ -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'
);
Expand Down Expand Up @@ -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 = {});
Expand Down
10 changes: 5 additions & 5 deletions src/connection_string.ts
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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])
);
10 changes: 4 additions & 6 deletions src/cursor/abstract_cursor.ts
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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 }
)
};
}
Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/cursor/aggregation_cursor.ts
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/cursor/find_cursor.ts
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/error.ts
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/explain.ts
Expand Up @@ -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') {
Expand Down
4 changes: 2 additions & 2 deletions src/operations/command.ts
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/operations/count.ts
Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/operations/delete.ts
Expand Up @@ -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 {
Expand All @@ -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`)
Expand Down
4 changes: 2 additions & 2 deletions src/operations/find.ts
Expand Up @@ -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')
);
Expand Down Expand Up @@ -338,7 +338,7 @@ function makeLegacyFindCommand(
findCommand.$maxTimeMS = options.maxTimeMS;
}

if (typeof options.explain !== 'undefined') {
if (options.explain != null) {
findCommand.$explain = true;
}

Expand Down
2 changes: 1 addition & 1 deletion src/operations/insert.ts
Expand Up @@ -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;
}

Expand Down
6 changes: 3 additions & 3 deletions src/operations/update.ts
Expand Up @@ -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]));

Expand Down Expand Up @@ -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]));

Expand Down Expand Up @@ -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]));

Expand Down
2 changes: 1 addition & 1 deletion src/read_preference.ts
Expand Up @@ -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)) {
Expand Down
2 changes: 1 addition & 1 deletion src/sdam/topology_description.ts
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/sessions.ts
Expand Up @@ -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] });
}
}
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 1d6abc9

Please sign in to comment.