Skip to content

Commit 19bc432

Browse files
IvanGoncharovyaacovCR
IvanGoncharov
authored and
yaacovCR
committed
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 aeddced commit 19bc432

35 files changed

Lines changed: 255 additions & 203 deletions

‎.eslintrc.cjs‎

Lines changed: 4 additions & 1 deletion
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‎

Lines changed: 4 additions & 4 deletions
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‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,13 @@ export class GraphQLError extends Error {
154154
// Include (non-enumerable) stack trace.
155155
/* c8 ignore start */
156156
// FIXME: https://github.com/graphql/graphql-js/issues/2317
157-
if (originalError?.stack) {
157+
if (originalError?.stack != null) {
158158
Object.defineProperty(this, 'stack', {
159159
value: originalError.stack,
160160
writable: true,
161161
configurable: true,
162162
});
163-
} else if (Error.captureStackTrace) {
163+
} else if (Error.captureStackTrace != null) {
164164
Error.captureStackTrace(this, GraphQLError);
165165
} else {
166166
Object.defineProperty(this, 'stack', {

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

Lines changed: 16 additions & 12 deletions
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‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,11 +1106,11 @@ describe('Execute: Handles basic execution tasks', () => {
11061106
}
11071107
}
11081108

1109-
const SpecialType = new GraphQLObjectType({
1109+
const SpecialType = new GraphQLObjectType<Special, { async: boolean }>({
11101110
name: 'SpecialType',
11111111
isTypeOf(obj, context) {
11121112
const result = obj instanceof Special;
1113-
return context?.async ? Promise.resolve(result) : result;
1113+
return context.async ? Promise.resolve(result) : result;
11141114
},
11151115
fields: { value: { type: GraphQLString } },
11161116
});
@@ -1129,7 +1129,12 @@ describe('Execute: Handles basic execution tasks', () => {
11291129
specials: [new Special('foo'), new NotSpecial('bar')],
11301130
};
11311131

1132-
const result = executeSync({ schema, document, rootValue });
1132+
const result = executeSync({
1133+
schema,
1134+
document,
1135+
rootValue,
1136+
contextValue: { async: false },
1137+
});
11331138
expectJSON(result).toDeepEqual({
11341139
data: {
11351140
specials: [{ value: 'foo' }, null],
@@ -1144,12 +1149,11 @@ describe('Execute: Handles basic execution tasks', () => {
11441149
],
11451150
});
11461151

1147-
const contextValue = { async: true };
11481152
const asyncResult = await execute({
11491153
schema,
11501154
document,
11511155
rootValue,
1152-
contextValue,
1156+
contextValue: { async: true },
11531157
});
11541158
expect(asyncResult).to.deep.equal(result);
11551159
});

‎src/execution/collectFields.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ function collectFieldsImpl(
134134
visitedFragmentNames.add(fragName);
135135
const fragment = fragments[fragName];
136136
if (
137-
!fragment ||
137+
fragment == null ||
138138
!doesFragmentConditionMatch(schema, fragment, runtimeType)
139139
) {
140140
continue;

‎src/execution/values.ts‎

Lines changed: 3 additions & 4 deletions
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‎

Lines changed: 1 addition & 1 deletion
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/jsutils/instanceOf.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { inspect } from './inspect.js';
22

33
/* c8 ignore next 3 */
44
const isProduction =
5-
globalThis.process &&
5+
globalThis.process != null &&
66
// eslint-disable-next-line no-undef
77
process.env.NODE_ENV === 'production';
88

‎src/language/printer.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,8 @@ const printDocASTReducer: ASTReducer<string> = {
150150
FloatValue: { leave: ({ value }) => value },
151151
StringValue: {
152152
leave: ({ value, block: isBlockString }) =>
153-
isBlockString ? printBlockString(value) : printString(value),
153+
// @ts-expect-error FIXME: it's a problem with ASTReducer, will be fixed in separate PR
154+
isBlockString === true ? printBlockString(value) : printString(value),
154155
},
155156
BooleanValue: { leave: ({ value }) => (value ? 'true' : 'false') },
156157
NullValue: { leave: () => 'null' },

0 commit comments

Comments
 (0)