Flow: enable 'sketchy-null-bool' lint rule (#2371)

.flowconfig

@@ -10,7 +10,7 @@
 [libs]
 
 [lints]
-sketchy-null-bool=off
+sketchy-null-bool=error
 sketchy-null-string=error
 sketchy-null-number=error
 sketchy-null-mixed=error
@@ -35,6 +35,7 @@ uninitialized-instance-property=error
 include_warnings=true
 module.use_strict=true
 babel_loose_array_spread=true
+esproposal.optional_chaining=enable
 suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(<VERSION>\\)?)\\)
 suppress_comment=\\(.\\|\n\\)*\\$DisableFlowOnNegativeTest
 

src/error/GraphQLError.js

@@ -99,8 +99,7 @@ export class GraphQLError extends Error {
     // Compute locations in the source for the given nodes/positions.
     let _source = source;
     if (!_source && _nodes) {
-      const node = _nodes[0];
-      _source = node && node.loc && node.loc.source;
+      _source = _nodes[0].loc?.source;
     }
 
     let _positions = positions;
@@ -188,7 +187,7 @@ export class GraphQLError extends Error {
     });
 
     // Include (non-enumerable) stack trace.
-    if (originalError && originalError.stack) {
+    if (originalError?.stack) {
       Object.defineProperty(this, 'stack', {
         value: originalError.stack,
         writable: true,

src/error/__tests__/GraphQLError-test.js

@@ -19,7 +19,7 @@ const source = new Source(dedent`
 `);
 const ast = parse(source);
 const operationNode = ast.definitions[0];
-invariant(operationNode && operationNode.kind === Kind.OPERATION_DEFINITION);
+invariant(operationNode.kind === Kind.OPERATION_DEFINITION);
 const fieldNode = operationNode.selectionSet.selections[0];
 invariant(fieldNode);
 
@@ -161,7 +161,7 @@ describe('printError', () => {
       ),
     );
     const opA = docA.definitions[0];
-    invariant(opA && opA.kind === Kind.OBJECT_TYPE_DEFINITION && opA.fields);
+    invariant(opA.kind === Kind.OBJECT_TYPE_DEFINITION && opA.fields);
     const fieldA = opA.fields[0];
 
     const docB = parse(
@@ -175,7 +175,7 @@ describe('printError', () => {
       ),
     );
     const opB = docB.definitions[0];
-    invariant(opB && opB.kind === Kind.OBJECT_TYPE_DEFINITION && opB.fields);
+    invariant(opB.kind === Kind.OBJECT_TYPE_DEFINITION && opB.fields);
     const fieldB = opB.fields[0];
 
     const error = new GraphQLError('Example error with two nodes', [

src/error/locatedError.js

@@ -16,15 +16,15 @@ export function locatedError(
 ): GraphQLError {
   // Note: this uses a brand-check to support GraphQL errors originating from
   // other contexts.
-  if (originalError && Array.isArray(originalError.path)) {
+  if (Array.isArray(originalError.path)) {
     return (originalError: any);
   }
 
   return new GraphQLError(
-    originalError && originalError.message,
-    (originalError && (originalError: any).nodes) || nodes,
-    originalError && (originalError: any).source,
-    originalError && (originalError: any).positions,
+    originalError.message,
+    (originalError: any).nodes || nodes,
+    (originalError: any).source,
+    (originalError: any).positions,
     path,
     originalError,
   );

src/execution/__tests__/executor-test.js

@@ -292,7 +292,7 @@ describe('Execute: Handles basic execution tasks', () => {
     );
 
     const operation = document.definitions[0];
-    invariant(operation && operation.kind === Kind.OPERATION_DEFINITION);
+    invariant(operation.kind === Kind.OPERATION_DEFINITION);
 
     expect(resolvedInfo).to.include({
       fieldName: 'test',
@@ -1025,7 +1025,7 @@ describe('Execute: Handles basic execution tasks', () => {
       name: 'SpecialType',
       isTypeOf(obj, context) {
         const result = obj instanceof Special;
-        return context && context.async ? Promise.resolve(result) : result;
+        return context?.async ? Promise.resolve(result) : result;
       },
       fields: { value: { type: GraphQLString } },
     });

src/execution/execute.js

@@ -296,7 +296,7 @@ export function buildExecutionContext(
             ];
           }
           operation = definition;
-        } else if (definition.name && definition.name.value === operationName) {
+        } else if (definition.name?.value === operationName) {
           operation = definition;
         }
         break;
@@ -554,7 +554,7 @@ function shouldIncludeNode(
     node,
     exeContext.variableValues,
   );
-  if (skip && skip.if === true) {
+  if (skip?.if === true) {
     return false;
   }
 
@@ -563,7 +563,7 @@ function shouldIncludeNode(
     node,
     exeContext.variableValues,
   );
-  if (include && include.if === false) {
+  if (include?.if === false) {
     return false;
   }
   return true;

src/execution/values.js

@@ -50,7 +50,7 @@ export function getVariableValues(
   inputs: { +[variable: string]: mixed, ... },
   options?: {| maxErrors?: number |},
 ): CoercedVariableValues {
-  const maxErrors = options && options.maxErrors;
+  const maxErrors = options?.maxErrors;
   const errors = [];
   try {
     const coerced = coerceVariableValues(schema, varDefNodes, inputs, error => {

src/jsutils/isPromise.js

@@ -9,5 +9,5 @@ declare function isPromise(value: mixed): boolean %checks(value instanceof
 
 // eslint-disable-next-line no-redeclare
 export default function isPromise(value) {
-  return value != null && typeof value.then === 'function';
+  return typeof value?.then === 'function';
 }

src/language/__tests__/visitor-test.js

@@ -500,7 +500,7 @@ describe('Visitor', () => {
           'enter',
           node.kind,
           key,
-          parent && parent.kind != null ? parent.kind : undefined,
+          parent?.kind != null ? parent.kind : undefined,
         ]);
 
         checkVisitorFnArgs(ast, arguments);
@@ -512,7 +512,7 @@ describe('Visitor', () => {
           'leave',
           node.kind,
           key,
-          parent && parent.kind != null ? parent.kind : undefined,
+          parent?.kind != null ? parent.kind : undefined,
         ]);
 
         expect(argsStack.pop()).to.deep.equal([...arguments]);

src/language/parser.js

@@ -164,7 +164,7 @@ export function parseType(
 }
 
 class Parser {
-  _options: ParseOptions;
+  _options: ?ParseOptions;
   _lexer: Lexer;
 
   constructor(source: string | Source, options?: ParseOptions) {
@@ -175,12 +175,7 @@ class Parser {
     );
 
     this._lexer = new Lexer(sourceObj);
-    this._options = options || {
-      noLocation: false,
-      allowLegacySDLEmptyFields: false,
-      allowLegacySDLImplementsInterfaces: false,
-      experimentalFragmentVariables: false,
-    };
+    this._options = options;
   }
 
   /**
@@ -483,7 +478,7 @@ class Parser {
     // Experimental support for defining variables within fragments changes
     // the grammar of FragmentDefinition:
     //   - fragment FragmentName VariableDefinitions? on TypeCondition Directives? SelectionSet
-    if (this._options.experimentalFragmentVariables) {
+    if (this._options?.experimentalFragmentVariables === true) {
       return {
         kind: Kind.FRAGMENT_DEFINITION,
         name: this.parseFragmentName(),
@@ -869,7 +864,7 @@ class Parser {
       } while (
         this.expectOptionalToken(TokenKind.AMP) ||
         // Legacy support for the SDL?
-        (this._options.allowLegacySDLImplementsInterfaces &&
+        (this._options?.allowLegacySDLImplementsInterfaces === true &&
           this.peek(TokenKind.NAME))
       );
     }
@@ -882,7 +877,7 @@ class Parser {
   parseFieldsDefinition(): Array<FieldDefinitionNode> {
     // Legacy support for the SDL?
     if (
-      this._options.allowLegacySDLEmptyFields &&
+      this._options?.allowLegacySDLEmptyFields === true &&
       this.peek(TokenKind.BRACE_L) &&
       this._lexer.lookahead().kind === TokenKind.BRACE_R
     ) {
@@ -1403,7 +1398,7 @@ class Parser {
    * the source that created a given parsed object.
    */
   loc(startToken: Token): Location | void {
-    if (!this._options.noLocation) {
+    if (this._options?.noLocation !== true) {
       return new Location(
         startToken,
         this._lexer.lastToken,

src/subscription/__tests__/mapAsyncIterator-test.js

@@ -273,7 +273,7 @@ describe('mapAsyncIterator', () => {
     } catch (e) {
       caughtError = e;
     }
-    expect(caughtError && caughtError.message).to.equal('Goodbye');
+    expect(caughtError?.message).to.equal('Goodbye');
   });
 
   it('maps over thrown errors if second callback provided', async () => {
@@ -295,7 +295,7 @@ describe('mapAsyncIterator', () => {
 
     const result = await doubles.next();
     expect(result.value).to.be.instanceof(Error);
-    expect(result.value && result.value.message).to.equal('Goodbye');
+    expect(result.value?.message).to.equal('Goodbye');
     expect(result.done).to.equal(false);
 
     expect(await doubles.next()).to.deep.equal({

src/subscription/__tests__/subscribe-test.js

@@ -142,7 +142,7 @@ async function expectPromiseToThrow(promise, message) {
     /* istanbul ignore next */
     expect.fail('promise should have thrown but did not');
   } catch (error) {
-    expect(error && error.message).to.equal(message);
+    expect(error?.message).to.equal(message);
   }
 }
 

src/type/directives.js

@@ -65,7 +65,7 @@ export class GraphQLDirective {
     this.name = config.name;
     this.description = config.description;
     this.locations = config.locations;
-    this.isRepeatable = config.isRepeatable != null && config.isRepeatable;
+    this.isRepeatable = config.isRepeatable || false;
     this.extensions = config.extensions && toObjMap(config.extensions);
     this.astNode = config.astNode;
 

src/type/schema.js

@@ -138,7 +138,7 @@ export class GraphQLSchema {
   constructor(config: $ReadOnly<GraphQLSchemaConfig>): void {
     // If this schema was built from a source known to be valid, then it may be
     // marked with assumeValid to avoid an additional type system validation.
-    if (config && config.assumeValid) {
+    if (config.assumeValid === true) {
       this.__validationErrors = [];
     } else {
       this.__validationErrors = undefined;

src/type/validate.js

@@ -156,7 +156,7 @@ function validateDirectives(context: SchemaValidationContext): void {
     if (!isDirective(directive)) {
       context.reportError(
         `Expected directive but got: ${inspect(directive)}.`,
-        directive && directive.astNode,
+        directive?.astNode,
       );
       continue;
     }
@@ -204,7 +204,7 @@ function validateTypes(context: SchemaValidationContext): void {
     if (!isNamedType(type)) {
       context.reportError(
         `Expected GraphQL named type but got: ${inspect(type)}.`,
-        type && type.astNode,
+        type?.astNode,
       );
       continue;
     }
@@ -265,7 +265,7 @@ function validateFields(
       context.reportError(
         `The type of ${type.name}.${field.name} must be Output Type ` +
           `but got: ${inspect(field.type)}.`,
-        field.astNode && field.astNode.type,
+        field.astNode?.type,
       );
     }
 
@@ -281,7 +281,7 @@ function validateFields(
         context.reportError(
           `The type of ${type.name}.${field.name}(${argName}:) must be Input ` +
             `Type but got: ${inspect(arg.type)}.`,
-          arg.astNode && arg.astNode.type,
+          arg.astNode?.type,
         );
       }
     }
@@ -354,10 +354,7 @@ function validateTypeImplementsInterface(
         `Interface field ${iface.name}.${fieldName} expects type ` +
           `${inspect(ifaceField.type)} but ${type.name}.${fieldName} ` +
           `is type ${inspect(typeField.type)}.`,
-        [
-          ifaceField.astNode && ifaceField.astNode.type,
-          typeField.astNode && typeField.astNode.type,
-        ],
+        [ifaceField.astNode?.type, typeField.astNode?.type],
       );
     }
 
@@ -384,10 +381,7 @@ function validateTypeImplementsInterface(
             `expects type ${inspect(ifaceArg.type)} but ` +
             `${type.name}.${fieldName}(${argName}:) is type ` +
             `${inspect(typeArg.type)}.`,
-          [
-            ifaceArg.astNode && ifaceArg.astNode.type,
-            typeArg.astNode && typeArg.astNode.type,
-          ],
+          [ifaceArg.astNode?.type, typeArg.astNode?.type],
         );
       }
 
@@ -512,7 +506,7 @@ function validateInputFields(
       context.reportError(
         `The type of ${inputObj.name}.${field.name} must be Input Type ` +
           `but got: ${inspect(field.type)}.`,
-        field.astNode && field.astNode.type,
+        field.astNode?.type,
       );
     }
   }

src/utilities/__tests__/buildASTSchema-test.js

@@ -51,7 +51,7 @@ function cycleSDL(sdl, options = {}) {
 }
 
 function printASTNode(obj) {
-  invariant(obj != null && obj.astNode != null);
+  invariant(obj?.astNode != null);
   return print(obj.astNode);
 }
 

src/utilities/__tests__/extendSchema-test.js

@@ -37,7 +37,7 @@ import { extendSchema } from '../extendSchema';
 import { buildSchema } from '../buildASTSchema';
 
 function printExtensionNodes(obj) {
-  invariant(obj && obj.extensionASTNodes);
+  invariant(obj?.extensionASTNodes != null);
   return print({
     kind: Kind.DOCUMENT,
     definitions: obj.extensionASTNodes,
@@ -56,7 +56,7 @@ function printSchemaChanges(schema, extendedSchema) {
 }
 
 function printASTNode(obj) {
-  invariant(obj != null && obj.astNode != null);
+  invariant(obj?.astNode != null);
   return print(obj.astNode);
 }
 

src/utilities/astFromValue.js

@@ -47,7 +47,7 @@ import {
 export function astFromValue(value: mixed, type: GraphQLInputType): ?ValueNode {
   if (isNonNullType(type)) {
     const astValue = astFromValue(value, type.ofType);
-    if (astValue && astValue.kind === Kind.NULL) {
+    if (astValue?.kind === Kind.NULL) {
       return null;
     }
     return astValue;