Skip to content

Commit

Permalink
TS: Fix strict issues in src/validation/
Browse files Browse the repository at this point in the history
* Add `"strict": true` to tsconfig.json
* Fix all issues that arise within `src/validation`
  • Loading branch information
leebyron committed May 26, 2021
1 parent c589c3d commit c19f90c
Show file tree
Hide file tree
Showing 10 changed files with 87 additions and 56 deletions.
6 changes: 6 additions & 0 deletions src/type/definition.ts
Expand Up @@ -457,6 +457,9 @@ export function getNullableType(type: undefined | null): void;
export function getNullableType<T extends GraphQLNullableType>(
type: T | GraphQLNonNull<T>,
): T;
export function getNullableType(
type: Maybe<GraphQLType>,
): GraphQLNullableType | undefined;
export function getNullableType(
type: Maybe<GraphQLType>,
): GraphQLNullableType | undefined {
Expand Down Expand Up @@ -504,6 +507,9 @@ export function getNamedType(type: undefined | null): void;
export function getNamedType(type: GraphQLInputType): GraphQLNamedInputType;
export function getNamedType(type: GraphQLOutputType): GraphQLNamedOutputType;
export function getNamedType(type: GraphQLType): GraphQLNamedType;
export function getNamedType(
type: Maybe<GraphQLType>,
): GraphQLNamedType | undefined;
export function getNamedType(
type: Maybe<GraphQLType>,
): GraphQLNamedType | undefined {
Expand Down
22 changes: 13 additions & 9 deletions src/validation/ValidationContext.ts
Expand Up @@ -44,7 +44,7 @@ interface VariableUsage {
export class ASTValidationContext {
private _ast: DocumentNode;
private _onError: (error: GraphQLError) => void;
private _fragments: Maybe<ObjMap<FragmentDefinitionNode>>;
private _fragments: ObjMap<FragmentDefinitionNode> | undefined;
private _fragmentSpreads: Map<
SelectionSetNode,
ReadonlyArray<FragmentSpreadNode>
Expand Down Expand Up @@ -72,15 +72,19 @@ export class ASTValidationContext {
}

getFragment(name: string): Maybe<FragmentDefinitionNode> {
if (!this._fragments) {
const fragments = (this._fragments = Object.create(null));
let fragments: ObjMap<FragmentDefinitionNode>;
if (this._fragments) {
fragments = this._fragments;
} else {
fragments = Object.create(null);
for (const defNode of this.getDocument().definitions) {
if (defNode.kind === Kind.FRAGMENT_DEFINITION) {
fragments[defNode.name.value] = defNode;
}
}
this._fragments = fragments;
}
return this._fragments[name];
return fragments[name];
}

getFragmentSpreads(
Expand All @@ -90,8 +94,8 @@ export class ASTValidationContext {
if (!spreads) {
spreads = [];
const setsToVisit: Array<SelectionSetNode> = [node];
while (setsToVisit.length !== 0) {
const set = setsToVisit.pop();
let set: SelectionSetNode | undefined;
while ((set = setsToVisit.pop())) {
for (const selection of set.selections) {
if (selection.kind === Kind.FRAGMENT_SPREAD) {
// @ts-expect-error FIXME: TS Conversion
Expand All @@ -114,8 +118,8 @@ export class ASTValidationContext {
fragments = [];
const collectedNames = Object.create(null);
const nodesToVisit: Array<SelectionSetNode> = [operation.selectionSet];
while (nodesToVisit.length !== 0) {
const node = nodesToVisit.pop();
let node: SelectionSetNode | undefined;
while ((node = nodesToVisit.pop())) {
for (const spread of this.getFragmentSpreads(node)) {
const fragName = spread.name.value;
if (collectedNames[fragName] !== true) {
Expand Down Expand Up @@ -189,7 +193,7 @@ export class ValidationContext extends ASTValidationContext {
getVariableUsages(node: NodeWithSelectionSet): ReadonlyArray<VariableUsage> {
let usages = this._variableUsages.get(node);
if (!usages) {
const newUsages = [];
const newUsages: Array<VariableUsage> = [];
const typeInfo = new TypeInfo(this._schema);
visit(
node,
Expand Down
4 changes: 3 additions & 1 deletion src/validation/__tests__/validation-test.ts
Expand Up @@ -3,6 +3,7 @@ import { describe, it } from 'mocha';

import { GraphQLError } from '../../error/GraphQLError';

import type { DirectiveNode } from '../../language/ast';
import { parse } from '../../language/parser';

import { TypeInfo } from '../../utilities/TypeInfo';
Expand All @@ -15,6 +16,7 @@ import { testSchema } from './harness';

describe('Validate: Supports full validation', () => {
it('rejects invalid documents', () => {
// TODO ts-expect-error (expects a DocumentNode as a second parameter)
expect(() => validate(testSchema, null)).to.throw('Must provide document.');
});

Expand Down Expand Up @@ -96,7 +98,7 @@ describe('Validate: Supports full validation', () => {

function customRule(context: ValidationContext) {
return {
Directive(node) {
Directive(node: DirectiveNode) {
const directiveDef = context.getDirective();
const error = new GraphQLError(
'Reporting directive: ' + String(directiveDef),
Expand Down
13 changes: 9 additions & 4 deletions src/validation/rules/NoFragmentCyclesRule.ts
@@ -1,7 +1,12 @@
import type { ObjMap } from '../../jsutils/ObjMap';

import { GraphQLError } from '../../error/GraphQLError';

import type {
FragmentDefinitionNode,
FragmentSpreadNode,
} from '../../language/ast';
import type { ASTVisitor } from '../../language/visitor';
import type { FragmentDefinitionNode } from '../../language/ast';

import type { ASTValidationContext } from '../ValidationContext';

Expand All @@ -10,13 +15,13 @@ export function NoFragmentCyclesRule(
): ASTVisitor {
// Tracks already visited fragments to maintain O(N) and to ensure that cycles
// are not redundantly reported.
const visitedFrags = Object.create(null);
const visitedFrags: ObjMap<boolean> = Object.create(null);

// Array of AST nodes used to produce meaningful errors
const spreadPath = [];
const spreadPath: Array<FragmentSpreadNode> = [];

// Position in the spread path
const spreadPathIndexByName = Object.create(null);
const spreadPathIndexByName: ObjMap<number | undefined> = Object.create(null);

return {
OperationDefinition: () => false,
Expand Down
8 changes: 6 additions & 2 deletions src/validation/rules/NoUnusedFragmentsRule.ts
@@ -1,5 +1,9 @@
import { GraphQLError } from '../../error/GraphQLError';

import type {
FragmentDefinitionNode,
OperationDefinitionNode,
} from '../../language/ast';
import type { ASTVisitor } from '../../language/visitor';

import type { ASTValidationContext } from '../ValidationContext';
Expand All @@ -13,8 +17,8 @@ import type { ASTValidationContext } from '../ValidationContext';
export function NoUnusedFragmentsRule(
context: ASTValidationContext,
): ASTVisitor {
const operationDefs = [];
const fragmentDefs = [];
const operationDefs: Array<OperationDefinitionNode> = [];
const fragmentDefs: Array<FragmentDefinitionNode> = [];

return {
OperationDefinition(node) {
Expand Down
3 changes: 2 additions & 1 deletion src/validation/rules/NoUnusedVariablesRule.ts
@@ -1,5 +1,6 @@
import { GraphQLError } from '../../error/GraphQLError';

import type { VariableDefinitionNode } from '../../language/ast';
import type { ASTVisitor } from '../../language/visitor';

import type { ValidationContext } from '../ValidationContext';
Expand All @@ -11,7 +12,7 @@ import type { ValidationContext } from '../ValidationContext';
* are used, either directly or within a spread fragment.
*/
export function NoUnusedVariablesRule(context: ValidationContext): ASTVisitor {
let variableDefs = [];
let variableDefs: Array<VariableDefinitionNode> = [];

return {
OperationDefinition: {
Expand Down
62 changes: 32 additions & 30 deletions src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
Expand Up @@ -17,7 +17,6 @@ import { print } from '../../language/printer';
import type {
GraphQLNamedType,
GraphQLOutputType,
GraphQLCompositeType,
GraphQLField,
} from '../../type/definition';
import {
Expand Down Expand Up @@ -97,12 +96,14 @@ type ConflictReason = [string, ConflictReasonMessage];
type ConflictReasonMessage = string | Array<ConflictReason>;
// Tuple defining a field node in a context.
type NodeAndDef = [
GraphQLCompositeType,
Maybe<GraphQLNamedType>,
FieldNode,
Maybe<GraphQLField<unknown, unknown>>,
];
// Map of array of those.
type NodeAndDefCollection = ObjMap<Array<NodeAndDef>>;
type FragmentNames = Array<string>;
type FieldsAndFragmentNames = readonly [NodeAndDefCollection, FragmentNames];

/**
* Algorithm:
Expand Down Expand Up @@ -164,12 +165,12 @@ type NodeAndDefCollection = ObjMap<Array<NodeAndDef>>;
// GraphQL Document.
function findConflictsWithinSelectionSet(
context: ValidationContext,
cachedFieldsAndFragmentNames,
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
comparedFragmentPairs: PairSet,
parentType: Maybe<GraphQLNamedType>,
selectionSet: SelectionSetNode,
): Array<Conflict> {
const conflicts = [];
const conflicts: Array<Conflict> = [];

const [fieldMap, fragmentNames] = getFieldsAndFragmentNames(
context,
Expand Down Expand Up @@ -226,7 +227,7 @@ function findConflictsWithinSelectionSet(
function collectConflictsBetweenFieldsAndFragment(
context: ValidationContext,
conflicts: Array<Conflict>,
cachedFieldsAndFragmentNames,
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
comparedFragmentPairs: PairSet,
areMutuallyExclusive: boolean,
fieldMap: NodeAndDefCollection,
Expand Down Expand Up @@ -280,7 +281,7 @@ function collectConflictsBetweenFieldsAndFragment(
function collectConflictsBetweenFragments(
context: ValidationContext,
conflicts: Array<Conflict>,
cachedFieldsAndFragmentNames,
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
comparedFragmentPairs: PairSet,
areMutuallyExclusive: boolean,
fragmentName1: string,
Expand Down Expand Up @@ -366,15 +367,15 @@ function collectConflictsBetweenFragments(
// between the sub-fields of two overlapping fields.
function findConflictsBetweenSubSelectionSets(
context: ValidationContext,
cachedFieldsAndFragmentNames,
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
comparedFragmentPairs: PairSet,
areMutuallyExclusive: boolean,
parentType1: Maybe<GraphQLNamedType>,
selectionSet1: SelectionSetNode,
parentType2: Maybe<GraphQLNamedType>,
selectionSet2: SelectionSetNode,
): Array<Conflict> {
const conflicts = [];
const conflicts: Array<Conflict> = [];

const [fieldMap1, fragmentNames1] = getFieldsAndFragmentNames(
context,
Expand Down Expand Up @@ -455,7 +456,7 @@ function findConflictsBetweenSubSelectionSets(
function collectConflictsWithin(
context: ValidationContext,
conflicts: Array<Conflict>,
cachedFieldsAndFragmentNames,
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
comparedFragmentPairs: PairSet,
fieldMap: NodeAndDefCollection,
): void {
Expand Down Expand Up @@ -496,7 +497,7 @@ function collectConflictsWithin(
function collectConflictsBetween(
context: ValidationContext,
conflicts: Array<Conflict>,
cachedFieldsAndFragmentNames,
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
comparedFragmentPairs: PairSet,
parentFieldsAreMutuallyExclusive: boolean,
fieldMap1: NodeAndDefCollection,
Expand Down Expand Up @@ -534,7 +535,7 @@ function collectConflictsBetween(
// comparing their sub-fields.
function findConflict(
context: ValidationContext,
cachedFieldsAndFragmentNames,
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
comparedFragmentPairs: PairSet,
parentFieldsAreMutuallyExclusive: boolean,
responseName: string,
Expand Down Expand Up @@ -677,32 +678,33 @@ function doTypesConflict(
// referenced via fragment spreads.
function getFieldsAndFragmentNames(
context: ValidationContext,
cachedFieldsAndFragmentNames,
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
parentType: Maybe<GraphQLNamedType>,
selectionSet: SelectionSetNode,
): [NodeAndDefCollection, Array<string>] {
let cached = cachedFieldsAndFragmentNames.get(selectionSet);
if (!cached) {
const nodeAndDefs = Object.create(null);
const fragmentNames = Object.create(null);
_collectFieldsAndFragmentNames(
context,
parentType,
selectionSet,
nodeAndDefs,
fragmentNames,
);
cached = [nodeAndDefs, Object.keys(fragmentNames)];
cachedFieldsAndFragmentNames.set(selectionSet, cached);
): FieldsAndFragmentNames {
const cached = cachedFieldsAndFragmentNames.get(selectionSet);
if (cached) {
return cached;
}
return cached;
const nodeAndDefs: NodeAndDefCollection = Object.create(null);
const fragmentNames: ObjMap<boolean> = Object.create(null);
_collectFieldsAndFragmentNames(
context,
parentType,
selectionSet,
nodeAndDefs,
fragmentNames,
);
const result = [nodeAndDefs, Object.keys(fragmentNames)] as const;
cachedFieldsAndFragmentNames.set(selectionSet, result);
return result;
}

// Given a reference to a fragment, return the represented collection of fields
// as well as a list of nested fragment names referenced via fragment spreads.
function getReferencedFieldsAndFragmentNames(
context: ValidationContext,
cachedFieldsAndFragmentNames,
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
fragment: FragmentDefinitionNode,
) {
// Short-circuit building a type from the node if possible.
Expand All @@ -724,8 +726,8 @@ function _collectFieldsAndFragmentNames(
context: ValidationContext,
parentType: Maybe<GraphQLNamedType>,
selectionSet: SelectionSetNode,
nodeAndDefs,
fragmentNames,
nodeAndDefs: NodeAndDefCollection,
fragmentNames: ObjMap<boolean>,
): void {
for (const selection of selectionSet.selections) {
switch (selection.kind) {
Expand Down
9 changes: 5 additions & 4 deletions src/validation/rules/PossibleTypeExtensionsRule.ts
@@ -1,3 +1,4 @@
import type { ObjMap } from '../../jsutils/ObjMap';
import { inspect } from '../../jsutils/inspect';
import { invariant } from '../../jsutils/invariant';
import { didYouMean } from '../../jsutils/didYouMean';
Expand All @@ -7,7 +8,7 @@ import { GraphQLError } from '../../error/GraphQLError';

import type { KindEnum } from '../../language/kinds';
import type { ASTVisitor } from '../../language/visitor';
import type { TypeExtensionNode } from '../../language/ast';
import type { DefinitionNode, TypeExtensionNode } from '../../language/ast';
import { Kind } from '../../language/kinds';
import { isTypeDefinitionNode } from '../../language/predicates';

Expand All @@ -32,7 +33,7 @@ export function PossibleTypeExtensionsRule(
context: SDLValidationContext,
): ASTVisitor {
const schema = context.getSchema();
const definedTypes = Object.create(null);
const definedTypes: ObjMap<DefinitionNode> = Object.create(null);

for (const def of context.getDocument().definitions) {
if (isTypeDefinitionNode(def)) {
Expand All @@ -54,7 +55,7 @@ export function PossibleTypeExtensionsRule(
const defNode = definedTypes[typeName];
const existingType = schema?.getType(typeName);

let expectedKind;
let expectedKind: KindEnum | undefined;
if (defNode) {
expectedKind = defKindToExtKind[defNode.kind];
} else if (existingType) {
Expand Down Expand Up @@ -89,7 +90,7 @@ export function PossibleTypeExtensionsRule(
}
}

const defKindToExtKind = {
const defKindToExtKind: ObjMap<KindEnum> = {
[Kind.SCALAR_TYPE_DEFINITION]: Kind.SCALAR_TYPE_EXTENSION,
[Kind.OBJECT_TYPE_DEFINITION]: Kind.OBJECT_TYPE_EXTENSION,
[Kind.INTERFACE_TYPE_DEFINITION]: Kind.INTERFACE_TYPE_EXTENSION,
Expand Down

0 comments on commit c19f90c

Please sign in to comment.