Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Apr 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
8 changes: 4 additions & 4 deletions resources/gen-changelog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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] ??= [];
Expand All @@ -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`;
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/error/GraphQLError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
28 changes: 16 additions & 12 deletions src/execution/__tests__/abstract-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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: {
Expand All @@ -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: {
Expand Down
14 changes: 9 additions & 5 deletions src/execution/__tests__/executor-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } },
});
Expand All @@ -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],
Expand All @@ -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);
});
Expand Down
6 changes: 3 additions & 3 deletions src/execution/__tests__/stream-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down Expand Up @@ -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 });
Expand Down Expand Up @@ -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 });
Expand Down
2 changes: 1 addition & 1 deletion src/execution/collectFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ function collectFieldsImpl(

const fragment = fragments[fragName];
if (
!fragment ||
fragment == null ||
!doesFragmentConditionMatch(schema, fragment, runtimeType)
) {
continue;
Expand Down
2 changes: 1 addition & 1 deletion src/execution/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 3 additions & 4 deletions src/execution/values.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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)) {
Expand Down
2 changes: 1 addition & 1 deletion src/jsutils/didYouMean.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function didYouMean(
}

let message = ' Did you mean ';
if (subMessage) {
if (subMessage != null) {
message += subMessage + ' ';
}

Expand Down
3 changes: 2 additions & 1 deletion src/language/printer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
Expand Down
4 changes: 2 additions & 2 deletions src/language/visitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/type/__tests__/enumType-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
10 changes: 5 additions & 5 deletions src/type/introspection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
},
Expand Down
2 changes: 1 addition & 1 deletion src/type/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
Loading