Skip to content

Commit 8c749e9

Browse files
IvanGoncharovIvanGoncharov
authored
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.
1 parent 3cf08e6 commit 8c749e9

37 files changed

+261
-203
lines changed

‎.eslintrc.cjs

+4-1
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,10 @@ module.exports = {
605605
'@typescript-eslint/restrict-plus-operands': 'off', // TODO: temporarily disabled
606606
'@typescript-eslint/restrict-template-expressions': 'off', // TODO: temporarily disabled
607607
'@typescript-eslint/sort-type-union-intersection-members': 'off', // TODO: consider
608-
'@typescript-eslint/strict-boolean-expressions': 'off', // TODO: consider
608+
'@typescript-eslint/strict-boolean-expressions': [
609+
'error',
610+
{ allowNullableBoolean: true }, // TODO: consider removing
611+
],
609612
'@typescript-eslint/switch-exhaustiveness-check': 'error',
610613
'@typescript-eslint/triple-slash-reference': 'error',
611614
'@typescript-eslint/typedef': 'off',

‎resources/gen-changelog.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const labelsConfig: { [label: string]: { section: string; fold?: boolean } } = {
3333
};
3434
const { GH_TOKEN } = process.env;
3535

36-
if (!GH_TOKEN) {
36+
if (GH_TOKEN == null) {
3737
console.error('Must provide GH_TOKEN as environment variable!');
3838
process.exit(1);
3939
}
@@ -88,7 +88,7 @@ async function genChangeLog(): Promise<string> {
8888
}
8989

9090
const label = labels[0];
91-
if (!labelsConfig[label]) {
91+
if (labelsConfig[label] != null) {
9292
throw new Error(`Unknown label: ${label}. See ${pr.url}`);
9393
}
9494
byLabel[label] ??= [];
@@ -99,7 +99,7 @@ async function genChangeLog(): Promise<string> {
9999
let changelog = `## ${tag ?? 'Unreleased'} (${date})\n`;
100100
for (const [label, config] of Object.entries(labelsConfig)) {
101101
const prs = byLabel[label];
102-
if (prs) {
102+
if (prs != null) {
103103
const shouldFold = config.fold && prs.length > 1;
104104

105105
changelog += `\n#### ${config.section}\n`;
@@ -149,7 +149,7 @@ async function graphqlRequest(query: string) {
149149
}
150150

151151
const json = await response.json();
152-
if (json.errors) {
152+
if (json.errors != null) {
153153
throw new Error('Errors: ' + JSON.stringify(json.errors, null, 2));
154154
}
155155
return json.data;

‎src/error/GraphQLError.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,13 @@ export class GraphQLError extends Error {
141141
// Include (non-enumerable) stack trace.
142142
/* c8 ignore start */
143143
// FIXME: https://github.com/graphql/graphql-js/issues/2317
144-
if (originalError?.stack) {
144+
if (originalError?.stack != null) {
145145
Object.defineProperty(this, 'stack', {
146146
value: originalError.stack,
147147
writable: true,
148148
configurable: true,
149149
});
150-
} else if (Error.captureStackTrace) {
150+
} else if (Error.captureStackTrace != null) {
151151
Error.captureStackTrace(this, GraphQLError);
152152
} else {
153153
Object.defineProperty(this, 'stack', {

‎src/execution/__tests__/abstract-test.ts

+16-12
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ import { buildSchema } from '../../utilities/buildASTSchema.js';
1919

2020
import { execute, executeSync } from '../execute.js';
2121

22+
interface Context {
23+
async: boolean;
24+
}
25+
2226
async function executeQuery(args: {
2327
schema: GraphQLSchema;
2428
query: string;
@@ -30,13 +34,13 @@ async function executeQuery(args: {
3034
schema,
3135
document,
3236
rootValue,
33-
contextValue: { async: false },
37+
contextValue: { async: false } satisfies Context,
3438
});
3539
const asyncResult = await execute({
3640
schema,
3741
document,
3842
rootValue,
39-
contextValue: { async: true },
43+
contextValue: { async: true } satisfies Context,
4044
});
4145

4246
expectJSON(result).toDeepEqual(asyncResult);
@@ -72,7 +76,7 @@ describe('Execute: Handles execution of abstract types', () => {
7276
},
7377
});
7478

75-
const DogType = new GraphQLObjectType({
79+
const DogType = new GraphQLObjectType<Dog, { async: boolean }>({
7680
name: 'Dog',
7781
interfaces: [PetType],
7882
isTypeOf(obj, context) {
@@ -85,7 +89,7 @@ describe('Execute: Handles execution of abstract types', () => {
8589
},
8690
});
8791

88-
const CatType = new GraphQLObjectType({
92+
const CatType = new GraphQLObjectType<Cat, { async: boolean }>({
8993
name: 'Cat',
9094
interfaces: [PetType],
9195
isTypeOf(obj, context) {
@@ -151,7 +155,7 @@ describe('Execute: Handles execution of abstract types', () => {
151155
},
152156
});
153157

154-
const DogType = new GraphQLObjectType({
158+
const DogType = new GraphQLObjectType<Dog, Context>({
155159
name: 'Dog',
156160
interfaces: [PetType],
157161
isTypeOf(_source, context) {
@@ -167,7 +171,7 @@ describe('Execute: Handles execution of abstract types', () => {
167171
},
168172
});
169173

170-
const CatType = new GraphQLObjectType({
174+
const CatType = new GraphQLObjectType<Cat, Context>({
171175
name: 'Cat',
172176
interfaces: [PetType],
173177
isTypeOf: undefined,
@@ -233,7 +237,7 @@ describe('Execute: Handles execution of abstract types', () => {
233237
},
234238
});
235239

236-
const DogType = new GraphQLObjectType({
240+
const DogType = new GraphQLObjectType<Dog, Context>({
237241
name: 'Dog',
238242
interfaces: [PetType],
239243
isTypeOf(_source, context) {
@@ -280,7 +284,7 @@ describe('Execute: Handles execution of abstract types', () => {
280284
});
281285

282286
it('isTypeOf used to resolve runtime type for Union', async () => {
283-
const DogType = new GraphQLObjectType({
287+
const DogType = new GraphQLObjectType<Dog, Context>({
284288
name: 'Dog',
285289
isTypeOf(obj, context) {
286290
const isDog = obj instanceof Dog;
@@ -292,7 +296,7 @@ describe('Execute: Handles execution of abstract types', () => {
292296
},
293297
});
294298

295-
const CatType = new GraphQLObjectType({
299+
const CatType = new GraphQLObjectType<Cat, Context>({
296300
name: 'Cat',
297301
isTypeOf(obj, context) {
298302
const isCat = obj instanceof Cat;
@@ -357,7 +361,7 @@ describe('Execute: Handles execution of abstract types', () => {
357361
name: 'Pet',
358362
resolveType(_source, context) {
359363
const error = new Error('We are testing this error');
360-
if (context.async) {
364+
if (context.async === true) {
361365
return Promise.reject(error);
362366
}
363367
throw error;
@@ -367,7 +371,7 @@ describe('Execute: Handles execution of abstract types', () => {
367371
},
368372
});
369373

370-
const DogType = new GraphQLObjectType({
374+
const DogType = new GraphQLObjectType<Dog, Context>({
371375
name: 'Dog',
372376
interfaces: [PetType],
373377
fields: {
@@ -376,7 +380,7 @@ describe('Execute: Handles execution of abstract types', () => {
376380
},
377381
});
378382

379-
const CatType = new GraphQLObjectType({
383+
const CatType = new GraphQLObjectType<Cat, Context>({
380384
name: 'Cat',
381385
interfaces: [PetType],
382386
fields: {

‎src/execution/__tests__/executor-test.ts

+9-5
Original file line numberDiff line numberDiff line change
@@ -1143,11 +1143,11 @@ describe('Execute: Handles basic execution tasks', () => {
11431143
}
11441144
}
11451145

1146-
const SpecialType = new GraphQLObjectType({
1146+
const SpecialType = new GraphQLObjectType<Special, { async: boolean }>({
11471147
name: 'SpecialType',
11481148
isTypeOf(obj, context) {
11491149
const result = obj instanceof Special;
1150-
return context?.async ? Promise.resolve(result) : result;
1150+
return context.async ? Promise.resolve(result) : result;
11511151
},
11521152
fields: { value: { type: GraphQLString } },
11531153
});
@@ -1166,7 +1166,12 @@ describe('Execute: Handles basic execution tasks', () => {
11661166
specials: [new Special('foo'), new NotSpecial('bar')],
11671167
};
11681168

1169-
const result = executeSync({ schema, document, rootValue });
1169+
const result = executeSync({
1170+
schema,
1171+
document,
1172+
rootValue,
1173+
contextValue: { async: false },
1174+
});
11701175
expectJSON(result).toDeepEqual({
11711176
data: {
11721177
specials: [{ value: 'foo' }, null],
@@ -1181,12 +1186,11 @@ describe('Execute: Handles basic execution tasks', () => {
11811186
],
11821187
});
11831188

1184-
const contextValue = { async: true };
11851189
const asyncResult = await execute({
11861190
schema,
11871191
document,
11881192
rootValue,
1189-
contextValue,
1193+
contextValue: { async: true },
11901194
});
11911195
expect(asyncResult).to.deep.equal(result);
11921196
});

‎src/execution/__tests__/stream-test.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -1839,7 +1839,7 @@ describe('Execute: stream directive', () => {
18391839
[Symbol.asyncIterator]: () => ({
18401840
next: () => {
18411841
const friend = friends[index++];
1842-
if (!friend) {
1842+
if (friend == null) {
18431843
return Promise.resolve({ done: true, value: undefined });
18441844
}
18451845
return Promise.resolve({ done: false, value: friend });
@@ -1898,7 +1898,7 @@ describe('Execute: stream directive', () => {
18981898
[Symbol.asyncIterator]: () => ({
18991899
next: () => {
19001900
const friend = friends[index++];
1901-
if (!friend) {
1901+
if (friend == null) {
19021902
return Promise.resolve({ done: true, value: undefined });
19031903
}
19041904
return Promise.resolve({ done: false, value: friend });
@@ -1954,7 +1954,7 @@ describe('Execute: stream directive', () => {
19541954
[Symbol.asyncIterator]: () => ({
19551955
next: () => {
19561956
const friend = friends[index++];
1957-
if (!friend) {
1957+
if (friend == null) {
19581958
return Promise.resolve({ done: true, value: undefined });
19591959
}
19601960
return Promise.resolve({ done: false, value: friend });

‎src/execution/collectFields.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ function collectFieldsImpl(
191191

192192
const fragment = fragments[fragName];
193193
if (
194-
!fragment ||
194+
fragment == null ||
195195
!doesFragmentConditionMatch(schema, fragment, runtimeType)
196196
) {
197197
continue;

‎src/execution/execute.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -2082,7 +2082,7 @@ function getCompletedIncrementalResults(
20822082
}
20832083

20842084
incrementalResult.path = asyncPayloadRecord.path;
2085-
if (asyncPayloadRecord.label) {
2085+
if (asyncPayloadRecord.label != null) {
20862086
incrementalResult.label = asyncPayloadRecord.label;
20872087
}
20882088
if (asyncPayloadRecord.errors.length > 0) {

‎src/execution/values.ts

+3-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { inspect } from '../jsutils/inspect.js';
2-
import { keyMap } from '../jsutils/keyMap.js';
32
import type { Maybe } from '../jsutils/Maybe.js';
43
import type { ObjMap } from '../jsutils/ObjMap.js';
54
import { printPathArray } from '../jsutils/printPathArray.js';
@@ -159,14 +158,14 @@ export function getArgumentValues(
159158
// FIXME: https://github.com/graphql/graphql-js/issues/2203
160159
/* c8 ignore next */
161160
const argumentNodes = node.arguments ?? [];
162-
const argNodeMap = keyMap(argumentNodes, (arg) => arg.name.value);
161+
const argNodeMap = new Map(argumentNodes.map((arg) => [arg.name.value, arg]));
163162

164163
for (const argDef of def.args) {
165164
const name = argDef.name;
166165
const argType = argDef.type;
167-
const argumentNode = argNodeMap[name];
166+
const argumentNode = argNodeMap.get(name);
168167

169-
if (!argumentNode) {
168+
if (argumentNode == null) {
170169
if (argDef.defaultValue !== undefined) {
171170
coercedValues[name] = argDef.defaultValue;
172171
} else if (isNonNullType(argType)) {

‎src/jsutils/didYouMean.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export function didYouMean(
2323
}
2424

2525
let message = ' Did you mean ';
26-
if (subMessage) {
26+
if (subMessage != null) {
2727
message += subMessage + ' ';
2828
}
2929

‎src/language/printer.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ const printDocASTReducer: ASTReducer<string> = {
143143
FloatValue: { leave: ({ value }) => value },
144144
StringValue: {
145145
leave: ({ value, block: isBlockString }) =>
146-
isBlockString ? printBlockString(value) : printString(value),
146+
// @ts-expect-error FIXME: it's a problem with ASTReducer, will be fixed in separate PR
147+
isBlockString === true ? printBlockString(value) : printString(value),
147148
},
148149
BooleanValue: { leave: ({ value }) => (value ? 'true' : 'false') },
149150
NullValue: { leave: () => 'null' },

‎src/language/visitor.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ export function visit(
234234
edits = stack.edits;
235235
inArray = stack.inArray;
236236
stack = stack.prev;
237-
} else if (parent) {
237+
} else if (parent != null) {
238238
key = inArray ? index : keys[index];
239239
node = parent[key];
240240
if (node === null || node === undefined) {
@@ -287,7 +287,7 @@ export function visit(
287287
keys = inArray ? node : (visitorKeys as any)[node.kind] ?? [];
288288
index = -1;
289289
edits = [];
290-
if (parent) {
290+
if (parent != null) {
291291
ancestors.push(parent);
292292
}
293293
parent = node;

‎src/type/__tests__/enumType-test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,12 @@ const QueryType = new GraphQLObjectType({
7171
provideBadValue: { type: GraphQLBoolean },
7272
},
7373
resolve(_source, { fromEnum, provideGoodValue, provideBadValue }) {
74-
if (provideGoodValue) {
74+
if (provideGoodValue === true) {
7575
// Note: this is one of the references of the internal values which
7676
// ComplexEnum allows.
7777
return Complex2;
7878
}
79-
if (provideBadValue) {
79+
if (provideBadValue === true) {
8080
// Note: similar shape, but not the same *reference*
8181
// as Complex2 above. Enum internal values require === equality.
8282
return { someRandomValue: 123 };

‎src/type/introspection.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ export const __Directive: GraphQLObjectType = new GraphQLObjectType({
112112
},
113113
},
114114
resolve(field, { includeDeprecated }) {
115-
return includeDeprecated
115+
return includeDeprecated === true
116116
? field.args
117117
: field.args.filter((arg) => arg.deprecationReason == null);
118118
},
@@ -266,7 +266,7 @@ export const __Type: GraphQLObjectType = new GraphQLObjectType({
266266
resolve(type, { includeDeprecated }) {
267267
if (isObjectType(type) || isInterfaceType(type)) {
268268
const fields = Object.values(type.getFields());
269-
return includeDeprecated
269+
return includeDeprecated === true
270270
? fields
271271
: fields.filter((field) => field.deprecationReason == null);
272272
}
@@ -296,7 +296,7 @@ export const __Type: GraphQLObjectType = new GraphQLObjectType({
296296
resolve(type, { includeDeprecated }) {
297297
if (isEnumType(type)) {
298298
const values = type.getValues();
299-
return includeDeprecated
299+
return includeDeprecated === true
300300
? values
301301
: values.filter((field) => field.deprecationReason == null);
302302
}
@@ -313,7 +313,7 @@ export const __Type: GraphQLObjectType = new GraphQLObjectType({
313313
resolve(type, { includeDeprecated }) {
314314
if (isInputObjectType(type)) {
315315
const values = Object.values(type.getFields());
316-
return includeDeprecated
316+
return includeDeprecated === true
317317
? values
318318
: values.filter((field) => field.deprecationReason == null);
319319
}
@@ -351,7 +351,7 @@ export const __Field: GraphQLObjectType = new GraphQLObjectType({
351351
},
352352
},
353353
resolve(field, { includeDeprecated }) {
354-
return includeDeprecated
354+
return includeDeprecated === true
355355
? field.args
356356
: field.args.filter((arg) => arg.deprecationReason == null);
357357
},

‎src/type/validate.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ function validateTypeImplementsInterface(
373373
const typeField = typeFieldMap[fieldName];
374374

375375
// Assert interface field exists on type.
376-
if (!typeField) {
376+
if (typeField == null) {
377377
context.reportError(
378378
`Interface field ${iface.name}.${fieldName} expected but ${type.name} does not provide it.`,
379379
[ifaceField.astNode, type.astNode, ...type.extensionASTNodes],

0 commit comments

Comments
 (0)