Enable @typescript-eslint/strict-boolean-expressions rule (#3872)

Motivation: Fix edge cases like in #3869 Also, I noticed a similar issue in #3867, so I fixed it for the entire codebase. P.S. I also resulted in a small but measurable speedup, probably because I replaced a bunch of ObjMap with ES6 Map.
graphql · Apr 1, 2023 · 8c749e9 · 8c749e9
1 parent 3cf08e6
commit 8c749e9
Show file tree

Hide file tree

Showing 37 changed files with 261 additions and 203 deletions.
diff --git a/.eslintrc.cjs b/.eslintrc.cjs
@@ -605,7 +605,10 @@ module.exports = {
         '@typescript-eslint/restrict-plus-operands': 'off', // TODO: temporarily disabled
         '@typescript-eslint/restrict-template-expressions': 'off', // TODO: temporarily disabled
         '@typescript-eslint/sort-type-union-intersection-members': 'off', // TODO: consider
-        '@typescript-eslint/strict-boolean-expressions': 'off', // TODO: consider
+        '@typescript-eslint/strict-boolean-expressions': [
+          'error',
+          { allowNullableBoolean: true }, // TODO: consider removing
+        ],
         '@typescript-eslint/switch-exhaustiveness-check': 'error',
         '@typescript-eslint/triple-slash-reference': 'error',
         '@typescript-eslint/typedef': 'off',

diff --git a/resources/gen-changelog.ts b/resources/gen-changelog.ts
@@ -33,7 +33,7 @@ const labelsConfig: { [label: string]: { section: string; fold?: boolean } } = {
 };
 const { GH_TOKEN } = process.env;
 
-if (!GH_TOKEN) {
+if (GH_TOKEN == null) {
   console.error('Must provide GH_TOKEN as environment variable!');
   process.exit(1);
 }
@@ -88,7 +88,7 @@ async function genChangeLog(): Promise<string> {
     }
 
     const label = labels[0];
-    if (!labelsConfig[label]) {
+    if (labelsConfig[label] != null) {
       throw new Error(`Unknown label: ${label}. See ${pr.url}`);
     }
     byLabel[label] ??= [];
@@ -99,7 +99,7 @@ async function genChangeLog(): Promise<string> {
   let changelog = `## ${tag ?? 'Unreleased'} (${date})\n`;
   for (const [label, config] of Object.entries(labelsConfig)) {
     const prs = byLabel[label];
-    if (prs) {
+    if (prs != null) {
       const shouldFold = config.fold && prs.length > 1;
 
       changelog += `\n#### ${config.section}\n`;
@@ -149,7 +149,7 @@ async function graphqlRequest(query: string) {
   }
 
   const json = await response.json();
-  if (json.errors) {
+  if (json.errors != null) {
     throw new Error('Errors: ' + JSON.stringify(json.errors, null, 2));
   }
   return json.data;

diff --git a/src/error/GraphQLError.ts b/src/error/GraphQLError.ts
@@ -141,13 +141,13 @@ export class GraphQLError extends Error {
     // Include (non-enumerable) stack trace.
     /* c8 ignore start */
     // FIXME: https://github.com/graphql/graphql-js/issues/2317
-    if (originalError?.stack) {
+    if (originalError?.stack != null) {
       Object.defineProperty(this, 'stack', {
         value: originalError.stack,
         writable: true,
         configurable: true,
       });
-    } else if (Error.captureStackTrace) {
+    } else if (Error.captureStackTrace != null) {
       Error.captureStackTrace(this, GraphQLError);
     } else {
       Object.defineProperty(this, 'stack', {

diff --git a/src/execution/__tests__/abstract-test.ts b/src/execution/__tests__/abstract-test.ts
@@ -19,6 +19,10 @@ import { buildSchema } from '../../utilities/buildASTSchema.js';
 
 import { execute, executeSync } from '../execute.js';
 
+interface Context {
+  async: boolean;
+}
+
 async function executeQuery(args: {
   schema: GraphQLSchema;
   query: string;
@@ -30,13 +34,13 @@ async function executeQuery(args: {
     schema,
     document,
     rootValue,
-    contextValue: { async: false },
+    contextValue: { async: false } satisfies Context,
   });
   const asyncResult = await execute({
     schema,
     document,
     rootValue,
-    contextValue: { async: true },
+    contextValue: { async: true } satisfies Context,
   });
 
   expectJSON(result).toDeepEqual(asyncResult);
@@ -72,7 +76,7 @@ describe('Execute: Handles execution of abstract types', () => {
       },
     });
 
-    const DogType = new GraphQLObjectType({
+    const DogType = new GraphQLObjectType<Dog, { async: boolean }>({
       name: 'Dog',
       interfaces: [PetType],
       isTypeOf(obj, context) {
@@ -85,7 +89,7 @@ describe('Execute: Handles execution of abstract types', () => {
       },
     });
 
-    const CatType = new GraphQLObjectType({
+    const CatType = new GraphQLObjectType<Cat, { async: boolean }>({
       name: 'Cat',
       interfaces: [PetType],
       isTypeOf(obj, context) {
@@ -151,7 +155,7 @@ describe('Execute: Handles execution of abstract types', () => {
       },
     });
 
-    const DogType = new GraphQLObjectType({
+    const DogType = new GraphQLObjectType<Dog, Context>({
       name: 'Dog',
       interfaces: [PetType],
       isTypeOf(_source, context) {
@@ -167,7 +171,7 @@ describe('Execute: Handles execution of abstract types', () => {
       },
     });
 
-    const CatType = new GraphQLObjectType({
+    const CatType = new GraphQLObjectType<Cat, Context>({
       name: 'Cat',
       interfaces: [PetType],
       isTypeOf: undefined,
@@ -233,7 +237,7 @@ describe('Execute: Handles execution of abstract types', () => {
       },
     });
 
-    const DogType = new GraphQLObjectType({
+    const DogType = new GraphQLObjectType<Dog, Context>({
       name: 'Dog',
       interfaces: [PetType],
       isTypeOf(_source, context) {
@@ -280,7 +284,7 @@ describe('Execute: Handles execution of abstract types', () => {
   });
 
   it('isTypeOf used to resolve runtime type for Union', async () => {
-    const DogType = new GraphQLObjectType({
+    const DogType = new GraphQLObjectType<Dog, Context>({
       name: 'Dog',
       isTypeOf(obj, context) {
         const isDog = obj instanceof Dog;
@@ -292,7 +296,7 @@ describe('Execute: Handles execution of abstract types', () => {
       },
     });
 
-    const CatType = new GraphQLObjectType({
+    const CatType = new GraphQLObjectType<Cat, Context>({
       name: 'Cat',
       isTypeOf(obj, context) {
         const isCat = obj instanceof Cat;
@@ -357,7 +361,7 @@ describe('Execute: Handles execution of abstract types', () => {
       name: 'Pet',
       resolveType(_source, context) {
         const error = new Error('We are testing this error');
-        if (context.async) {
+        if (context.async === true) {
           return Promise.reject(error);
         }
         throw error;
@@ -367,7 +371,7 @@ describe('Execute: Handles execution of abstract types', () => {
       },
     });
 
-    const DogType = new GraphQLObjectType({
+    const DogType = new GraphQLObjectType<Dog, Context>({
       name: 'Dog',
       interfaces: [PetType],
       fields: {
@@ -376,7 +380,7 @@ describe('Execute: Handles execution of abstract types', () => {
       },
     });
 
-    const CatType = new GraphQLObjectType({
+    const CatType = new GraphQLObjectType<Cat, Context>({
       name: 'Cat',
       interfaces: [PetType],
       fields: {

diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts
@@ -1143,11 +1143,11 @@ describe('Execute: Handles basic execution tasks', () => {
       }
     }
 
-    const SpecialType = new GraphQLObjectType({
+    const SpecialType = new GraphQLObjectType<Special, { async: boolean }>({
       name: 'SpecialType',
       isTypeOf(obj, context) {
         const result = obj instanceof Special;
-        return context?.async ? Promise.resolve(result) : result;
+        return context.async ? Promise.resolve(result) : result;
       },
       fields: { value: { type: GraphQLString } },
     });
@@ -1166,7 +1166,12 @@ describe('Execute: Handles basic execution tasks', () => {
       specials: [new Special('foo'), new NotSpecial('bar')],
     };
 
-    const result = executeSync({ schema, document, rootValue });
+    const result = executeSync({
+      schema,
+      document,
+      rootValue,
+      contextValue: { async: false },
+    });
     expectJSON(result).toDeepEqual({
       data: {
         specials: [{ value: 'foo' }, null],
@@ -1181,12 +1186,11 @@ describe('Execute: Handles basic execution tasks', () => {
       ],
     });
 
-    const contextValue = { async: true };
     const asyncResult = await execute({
       schema,
       document,
       rootValue,
-      contextValue,
+      contextValue: { async: true },
     });
     expect(asyncResult).to.deep.equal(result);
   });

diff --git a/src/execution/__tests__/stream-test.ts b/src/execution/__tests__/stream-test.ts
@@ -1839,7 +1839,7 @@ describe('Execute: stream directive', () => {
       [Symbol.asyncIterator]: () => ({
         next: () => {
           const friend = friends[index++];
-          if (!friend) {
+          if (friend == null) {
             return Promise.resolve({ done: true, value: undefined });
           }
           return Promise.resolve({ done: false, value: friend });
@@ -1898,7 +1898,7 @@ describe('Execute: stream directive', () => {
       [Symbol.asyncIterator]: () => ({
         next: () => {
           const friend = friends[index++];
-          if (!friend) {
+          if (friend == null) {
             return Promise.resolve({ done: true, value: undefined });
           }
           return Promise.resolve({ done: false, value: friend });
@@ -1954,7 +1954,7 @@ describe('Execute: stream directive', () => {
       [Symbol.asyncIterator]: () => ({
         next: () => {
           const friend = friends[index++];
-          if (!friend) {
+          if (friend == null) {
             return Promise.resolve({ done: true, value: undefined });
           }
           return Promise.resolve({ done: false, value: friend });

diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts
@@ -191,7 +191,7 @@ function collectFieldsImpl(
 
         const fragment = fragments[fragName];
         if (
-          !fragment ||
+          fragment == null ||
           !doesFragmentConditionMatch(schema, fragment, runtimeType)
         ) {
           continue;

diff --git a/src/execution/execute.ts b/src/execution/execute.ts
@@ -2082,7 +2082,7 @@ function getCompletedIncrementalResults(
     }
 
     incrementalResult.path = asyncPayloadRecord.path;
-    if (asyncPayloadRecord.label) {
+    if (asyncPayloadRecord.label != null) {
       incrementalResult.label = asyncPayloadRecord.label;
     }
     if (asyncPayloadRecord.errors.length > 0) {

diff --git a/src/execution/values.ts b/src/execution/values.ts
@@ -1,5 +1,4 @@
 import { inspect } from '../jsutils/inspect.js';
-import { keyMap } from '../jsutils/keyMap.js';
 import type { Maybe } from '../jsutils/Maybe.js';
 import type { ObjMap } from '../jsutils/ObjMap.js';
 import { printPathArray } from '../jsutils/printPathArray.js';
@@ -159,14 +158,14 @@ export function getArgumentValues(
   // FIXME: https://github.com/graphql/graphql-js/issues/2203
   /* c8 ignore next */
   const argumentNodes = node.arguments ?? [];
-  const argNodeMap = keyMap(argumentNodes, (arg) => arg.name.value);
+  const argNodeMap = new Map(argumentNodes.map((arg) => [arg.name.value, arg]));
 
   for (const argDef of def.args) {
     const name = argDef.name;
     const argType = argDef.type;
-    const argumentNode = argNodeMap[name];
+    const argumentNode = argNodeMap.get(name);
 
-    if (!argumentNode) {
+    if (argumentNode == null) {
       if (argDef.defaultValue !== undefined) {
         coercedValues[name] = argDef.defaultValue;
       } else if (isNonNullType(argType)) {

diff --git a/src/jsutils/didYouMean.ts b/src/jsutils/didYouMean.ts
@@ -23,7 +23,7 @@ export function didYouMean(
   }
 
   let message = ' Did you mean ';
-  if (subMessage) {
+  if (subMessage != null) {
     message += subMessage + ' ';
   }
 

diff --git a/src/language/printer.ts b/src/language/printer.ts
@@ -143,7 +143,8 @@ const printDocASTReducer: ASTReducer<string> = {
   FloatValue: { leave: ({ value }) => value },
   StringValue: {
     leave: ({ value, block: isBlockString }) =>
-      isBlockString ? printBlockString(value) : printString(value),
+      // @ts-expect-error FIXME: it's a problem with ASTReducer, will be fixed in separate PR
+      isBlockString === true ? printBlockString(value) : printString(value),
   },
   BooleanValue: { leave: ({ value }) => (value ? 'true' : 'false') },
   NullValue: { leave: () => 'null' },

diff --git a/src/language/visitor.ts b/src/language/visitor.ts
@@ -234,7 +234,7 @@ export function visit(
       edits = stack.edits;
       inArray = stack.inArray;
       stack = stack.prev;
-    } else if (parent) {
+    } else if (parent != null) {
       key = inArray ? index : keys[index];
       node = parent[key];
       if (node === null || node === undefined) {
@@ -287,7 +287,7 @@ export function visit(
       keys = inArray ? node : (visitorKeys as any)[node.kind] ?? [];
       index = -1;
       edits = [];
-      if (parent) {
+      if (parent != null) {
         ancestors.push(parent);
       }
       parent = node;

diff --git a/src/type/__tests__/enumType-test.ts b/src/type/__tests__/enumType-test.ts
@@ -71,12 +71,12 @@ const QueryType = new GraphQLObjectType({
         provideBadValue: { type: GraphQLBoolean },
       },
       resolve(_source, { fromEnum, provideGoodValue, provideBadValue }) {
-        if (provideGoodValue) {
+        if (provideGoodValue === true) {
           // Note: this is one of the references of the internal values which
           // ComplexEnum allows.
           return Complex2;
         }
-        if (provideBadValue) {
+        if (provideBadValue === true) {
           // Note: similar shape, but not the same *reference*
           // as Complex2 above. Enum internal values require === equality.
           return { someRandomValue: 123 };

diff --git a/src/type/introspection.ts b/src/type/introspection.ts
@@ -112,7 +112,7 @@ export const __Directive: GraphQLObjectType = new GraphQLObjectType({
           },
         },
         resolve(field, { includeDeprecated }) {
-          return includeDeprecated
+          return includeDeprecated === true
             ? field.args
             : field.args.filter((arg) => arg.deprecationReason == null);
         },
@@ -266,7 +266,7 @@ export const __Type: GraphQLObjectType = new GraphQLObjectType({
         resolve(type, { includeDeprecated }) {
           if (isObjectType(type) || isInterfaceType(type)) {
             const fields = Object.values(type.getFields());
-            return includeDeprecated
+            return includeDeprecated === true
               ? fields
               : fields.filter((field) => field.deprecationReason == null);
           }
@@ -296,7 +296,7 @@ export const __Type: GraphQLObjectType = new GraphQLObjectType({
         resolve(type, { includeDeprecated }) {
           if (isEnumType(type)) {
             const values = type.getValues();
-            return includeDeprecated
+            return includeDeprecated === true
               ? values
               : values.filter((field) => field.deprecationReason == null);
           }
@@ -313,7 +313,7 @@ export const __Type: GraphQLObjectType = new GraphQLObjectType({
         resolve(type, { includeDeprecated }) {
           if (isInputObjectType(type)) {
             const values = Object.values(type.getFields());
-            return includeDeprecated
+            return includeDeprecated === true
               ? values
               : values.filter((field) => field.deprecationReason == null);
           }
@@ -351,7 +351,7 @@ export const __Field: GraphQLObjectType = new GraphQLObjectType({
           },
         },
         resolve(field, { includeDeprecated }) {
-          return includeDeprecated
+          return includeDeprecated === true
             ? field.args
             : field.args.filter((arg) => arg.deprecationReason == null);
         },

diff --git a/src/type/validate.ts b/src/type/validate.ts
@@ -373,7 +373,7 @@ function validateTypeImplementsInterface(
     const typeField = typeFieldMap[fieldName];
 
     // Assert interface field exists on type.
-    if (!typeField) {
+    if (typeField == null) {
       context.reportError(
         `Interface field ${iface.name}.${fieldName} expected but ${type.name} does not provide it.`,
         [ifaceField.astNode, type.astNode, ...type.extensionASTNodes],