Flow: Readonly errors (#1116)

This uses $ReadOnlyArray and `+` properties to ensure Errors are read-only, removing some use of `*`.

Author: leebyron

src/error/GraphQLError.js

@@ -11,6 +11,11 @@ import { getLocation } from '../language/location';
 import type { ASTNode } from '../language/ast';
 import type { Source } from '../language/source';
 
+export type GraphQLErrorLocation = {|
+  +line: number,
+  +column: number,
+|};
+
 /**
  * A GraphQLError describes an Error found during the parse, validate, or
  * execute phases of performing a GraphQL operation. In addition to a message
@@ -20,10 +25,10 @@ import type { Source } from '../language/source';
 declare class GraphQLError extends Error {
   constructor(
     message: string,
-    nodes?: ?Array<*>,
+    nodes?: ?$ReadOnlyArray<ASTNode>,
     source?: ?Source,
-    positions?: ?Array<number>,
-    path?: ?Array<string | number>,
+    positions?: ?$ReadOnlyArray<number>,
+    path?: ?$ReadOnlyArray<string | number>,
     originalError?: ?Error,
     extensions?: ?{ [key: string]: mixed },
   ): void;
@@ -32,6 +37,8 @@ declare class GraphQLError extends Error {
    * A message describing the Error for debugging purposes.
    *
    * Enumerable, and appears in the result of JSON.stringify().
+   *
+   * Note: should be treated as readonly, despite invariant usage.
    */
   message: string;
 
@@ -45,49 +52,49 @@ declare class GraphQLError extends Error {
    *
    * Enumerable, and appears in the result of JSON.stringify().
    */
-  locations: Array<{ line: number, column: number }> | void;
+  +locations: $ReadOnlyArray<GraphQLErrorLocation> | void;
 
   /**
    * An array describing the JSON-path into the execution response which
    * corresponds to this error. Only included for errors during execution.
    *
    * Enumerable, and appears in the result of JSON.stringify().
    */
-  path: Array<string | number> | void;
+  +path: $ReadOnlyArray<string | number> | void;
 
   /**
    * An array of GraphQL AST Nodes corresponding to this error.
    */
-  nodes: Array<ASTNode> | void;
+  +nodes: $ReadOnlyArray<ASTNode> | void;
 
   /**
    * The source GraphQL document corresponding to this error.
    */
-  source: Source | void;
+  +source: Source | void;
 
   /**
    * An array of character offsets within the source GraphQL document
    * which correspond to this error.
    */
-  positions: Array<number> | void;
+  +positions: $ReadOnlyArray<number> | void;
 
   /**
    * The original error thrown from a field resolver during execution.
    */
-  originalError: ?Error;
+  +originalError: ?Error;
 
   /**
    * The original error thrown from a field resolver during execution.
    */
-  extensions: ?{ [key: string]: mixed };
+  +extensions: ?{ [key: string]: mixed };
 }
 
 export function GraphQLError( // eslint-disable-line no-redeclare
   message: string,
-  nodes?: ?Array<*>,
+  nodes?: ?$ReadOnlyArray<ASTNode>,
   source?: ?Source,
-  positions?: ?Array<number>,
-  path?: ?Array<string | number>,
+  positions?: ?$ReadOnlyArray<number>,
+  path?: ?$ReadOnlyArray<string | number>,
   originalError?: ?Error,
   extensions?: ?{ [key: string]: mixed },
 ) {
@@ -100,9 +107,12 @@ export function GraphQLError( // eslint-disable-line no-redeclare
 
   let _positions = positions;
   if (!_positions && nodes) {
-    _positions = nodes
-      .filter(node => Boolean(node.loc))
-      .map(node => node.loc.start);
+    _positions = nodes.reduce((list, node) => {
+      if (node.loc) {
+        list.push(node.loc.start);
+      }
+      return list;
+    }, []);
   }
   if (_positions && _positions.length === 0) {
     _positions = undefined;

src/error/formatError.js

@@ -8,7 +8,7 @@
  */
 
 import invariant from '../jsutils/invariant';
-import type { GraphQLError } from './GraphQLError';
+import type { GraphQLError, GraphQLErrorLocation } from './GraphQLError';
 
 /**
  * Given a GraphQLError, format it according to the rules described by the
@@ -25,12 +25,9 @@ export function formatError(error: GraphQLError): GraphQLFormattedError {
 }
 
 export type GraphQLFormattedError = {
-  message: string,
-  locations: ?Array<GraphQLErrorLocation>,
-  path: ?Array<string | number>,
-};
-
-export type GraphQLErrorLocation = {
-  line: number,
-  column: number,
+  +message: string,
+  +locations: $ReadOnlyArray<GraphQLErrorLocation> | void,
+  +path: $ReadOnlyArray<string | number> | void,
+  // Extensions
+  +[key: string]: mixed,
 };

src/error/index.js

@@ -12,7 +12,5 @@ export { syntaxError } from './syntaxError';
 export { locatedError } from './locatedError';
 export { formatError } from './formatError';
 
-export type {
-  GraphQLFormattedError,
-  GraphQLErrorLocation,
-} from './formatError';
+export type { GraphQLErrorLocation } from './GraphQLError';
+export type { GraphQLFormattedError } from './formatError';

src/error/locatedError.js

@@ -8,6 +8,7 @@
  */
 
 import { GraphQLError } from './GraphQLError';
+import type { ASTNode } from '../language/ast';
 
 /**
  * Given an arbitrary Error, presumably thrown while attempting to execute a
@@ -16,8 +17,8 @@ import { GraphQLError } from './GraphQLError';
  */
 export function locatedError(
   originalError: ?Error,
-  nodes: Array<*>,
-  path: Array<string | number>,
+  nodes: $ReadOnlyArray<ASTNode>,
+  path: $ReadOnlyArray<string | number>,
 ): GraphQLError {
   // Note: this uses a brand-check to support GraphQL errors originating from
   // other contexts.

src/execution/execute.js

@@ -101,7 +101,7 @@ export type ExecutionContext = {
  *   - `data` is the result of a successful execution of the query.
  */
 export type ExecutionResult = {
-  errors?: Array<GraphQLError>,
+  errors?: $ReadOnlyArray<GraphQLError>,
   data?: ObjMap<mixed>,
 };
 
@@ -235,7 +235,7 @@ function buildResponse(
  */
 export function responsePathAsArray(
   path: ResponsePath,
-): Array<string | number> {
+): $ReadOnlyArray<string | number> {
   const flattened = [];
   let curr = path;
   while (curr) {
@@ -663,7 +663,7 @@ function resolveField(
   exeContext: ExecutionContext,
   parentType: GraphQLObjectType,
   source: mixed,
-  fieldNodes: Array<FieldNode>,
+  fieldNodes: $ReadOnlyArray<FieldNode>,
   path: ResponsePath,
 ): mixed {
   const fieldNode = fieldNodes[0];
@@ -708,7 +708,7 @@ function resolveField(
 export function buildResolveInfo(
   exeContext: ExecutionContext,
   fieldDef: GraphQLField<*, *>,
-  fieldNodes: Array<FieldNode>,
+  fieldNodes: $ReadOnlyArray<FieldNode>,
   parentType: GraphQLObjectType,
   path: ResponsePath,
 ): GraphQLResolveInfo {
@@ -733,7 +733,7 @@ export function buildResolveInfo(
 export function resolveFieldValueOrError<TSource>(
   exeContext: ExecutionContext,
   fieldDef: GraphQLField<TSource, *>,
-  fieldNodes: Array<FieldNode>,
+  fieldNodes: $ReadOnlyArray<FieldNode>,
   resolveFn: GraphQLFieldResolver<TSource, *>,
   source: TSource,
   info: GraphQLResolveInfo,
@@ -766,7 +766,7 @@ export function resolveFieldValueOrError<TSource>(
 function completeValueCatchingError(
   exeContext: ExecutionContext,
   returnType: GraphQLType,
-  fieldNodes: Array<FieldNode>,
+  fieldNodes: $ReadOnlyArray<FieldNode>,
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,
@@ -820,7 +820,7 @@ function completeValueCatchingError(
 function completeValueWithLocatedError(
   exeContext: ExecutionContext,
   returnType: GraphQLType,
-  fieldNodes: Array<FieldNode>,
+  fieldNodes: $ReadOnlyArray<FieldNode>,
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,
@@ -872,7 +872,7 @@ function completeValueWithLocatedError(
 function completeValue(
   exeContext: ExecutionContext,
   returnType: GraphQLType,
-  fieldNodes: Array<FieldNode>,
+  fieldNodes: $ReadOnlyArray<FieldNode>,
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,
@@ -972,7 +972,7 @@ function completeValue(
 function completeListValue(
   exeContext: ExecutionContext,
   returnType: GraphQLList<*>,
-  fieldNodes: Array<FieldNode>,
+  fieldNodes: $ReadOnlyArray<FieldNode>,
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,
@@ -1034,7 +1034,7 @@ function completeLeafValue(returnType: GraphQLLeafType, result: mixed): mixed {
 function completeAbstractValue(
   exeContext: ExecutionContext,
   returnType: GraphQLAbstractType,
-  fieldNodes: Array<FieldNode>,
+  fieldNodes: $ReadOnlyArray<FieldNode>,
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,
@@ -1085,7 +1085,7 @@ function ensureValidRuntimeType(
   runtimeTypeOrName: ?GraphQLObjectType | string,
   exeContext: ExecutionContext,
   returnType: GraphQLAbstractType,
-  fieldNodes: Array<FieldNode>,
+  fieldNodes: $ReadOnlyArray<FieldNode>,
   info: GraphQLResolveInfo,
   result: mixed,
 ): GraphQLObjectType {
@@ -1123,7 +1123,7 @@ function ensureValidRuntimeType(
 function completeObjectValue(
   exeContext: ExecutionContext,
   returnType: GraphQLObjectType,
-  fieldNodes: Array<FieldNode>,
+  fieldNodes: $ReadOnlyArray<FieldNode>,
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,
@@ -1169,7 +1169,7 @@ function completeObjectValue(
 function invalidReturnTypeError(
   returnType: GraphQLObjectType,
   result: mixed,
-  fieldNodes: Array<FieldNode>,
+  fieldNodes: $ReadOnlyArray<FieldNode>,
 ): GraphQLError {
   return new GraphQLError(
     `Expected value of type "${returnType.name}" but got: ${String(result)}.`,
@@ -1180,7 +1180,7 @@ function invalidReturnTypeError(
 function collectAndExecuteSubfields(
   exeContext: ExecutionContext,
   returnType: GraphQLObjectType,
-  fieldNodes: Array<FieldNode>,
+  fieldNodes: $ReadOnlyArray<FieldNode>,
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,

src/type/definition.js

@@ -619,20 +619,23 @@ export type GraphQLFieldResolver<TSource, TContext> = (
   info: GraphQLResolveInfo,
 ) => mixed;
 
-export type GraphQLResolveInfo = {
-  fieldName: string,
-  fieldNodes: Array<FieldNode>,
-  returnType: GraphQLOutputType,
-  parentType: GraphQLObjectType,
-  path: ResponsePath,
-  schema: GraphQLSchema,
-  fragments: ObjMap<FragmentDefinitionNode>,
-  rootValue: mixed,
-  operation: OperationDefinitionNode,
-  variableValues: { [variable: string]: mixed },
-};
-
-export type ResponsePath = { prev: ResponsePath | void, key: string | number };
+export type GraphQLResolveInfo = {|
+  +fieldName: string,
+  +fieldNodes: $ReadOnlyArray<FieldNode>,
+  +returnType: GraphQLOutputType,
+  +parentType: GraphQLObjectType,
+  +path: ResponsePath,
+  +schema: GraphQLSchema,
+  +fragments: ObjMap<FragmentDefinitionNode>,
+  +rootValue: mixed,
+  +operation: OperationDefinitionNode,
+  +variableValues: { [variable: string]: mixed },
+|};
+
+export type ResponsePath = {|
+  +prev: ResponsePath | void,
+  +key: string | number,
+|};
 
 export type GraphQLFieldConfig<TSource, TContext> = {
   type: GraphQLOutputType,

src/validation/rules/ScalarLeafs.js

@@ -43,17 +43,18 @@ export function ScalarLeafs(context: ValidationContext): any {
   return {
     Field(node: FieldNode) {
       const type = context.getType();
+      const selectionSet = node.selectionSet;
       if (type) {
         if (isLeafType(getNamedType(type))) {
-          if (node.selectionSet) {
+          if (selectionSet) {
             context.reportError(
               new GraphQLError(
                 noSubselectionAllowedMessage(node.name.value, type),
-                [node.selectionSet],
+                [selectionSet],
               ),
             );
           }
-        } else if (!node.selectionSet) {
+        } else if (!selectionSet) {
           context.reportError(
             new GraphQLError(
               requiredSubselectionMessage(node.name.value, type),