8000 fix(eslint-plugin): add getConstraintInfo to handle generic constrain… · OlivierZal/typescript-eslint@fdfe5ea · GitHub
[go: up one dir, main page]
Skip to content

Commit fdfe5ea

Browse files
kirkwaiblingertypescript-eslint[bot]JoshuaKGoldberg
authored and
OlivierZal
committed
fix(eslint-plugin): add getConstraintInfo to handle generic constraints better (typescript-eslint#10496)
* add getConstraintTypeInfoAtLocation * revisit typescript-eslint#10314 * handle typescript-eslint#10443 * tweak tests * Apply suggestions from code review Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com> * code review * deal with deprecated API lint failures * space * remove vfs, derp * typo * simplify * more simplify * jsdoc * more better * better --------- Co-authored-by: typescript-eslint[bot] <53356952+typescript-eslint[bot]@users.noreply.github.com> Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
1 parent ac9f6c1 commit fdfe5ea

10 files changed

+454
-80
lines changed

‎packages/eslint-plugin/src/rules/await-thenable.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,17 @@ export default createRule<[], MessageId>({
5151

5252
return {
5353
AwaitExpression(node): void {
54-
const type = services.getTypeAtLocation(node.argument);
55-
56-
const originalNode = services.esTreeNodeToTSNodeMap.get(node);
57-
const certainty = needsToBeAwaited(checker, originalNode, type);
54+
const awaitArgumentEsNode = node.argument;
55+
const awaitArgumentType =
56+
services.getTypeAtLocation(awaitArgumentEsNode);
57+
const awaitArgumentTsNode =
58+
services.esTreeNodeToTSNodeMap.get(awaitArgumentEsNode);
59+
60+
const certainty = needsToBeAwaited(
61+
checker,
62+
awaitArgumentTsNode,
63+
awaitArgumentType,
64+
);
5865

5966
if (certainty === Awaitable.Never) {
6067
context.report({

‎packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as ts from 'typescript';
66

77
import {
88
createRule,
9-
getConstrainedTypeAtLocation,
9+
getConstraintInfo,
1010
getParserServices,
1111
isStrongPrecedenceNode,
1212
} from '../util';
@@ -85,6 +85,7 @@ export default createRule<Options, MessageIds>({
8585
],
8686
create(context, [options]) {
8787
const services = getParserServices(context);
88+
const checker = services.program.getTypeChecker();
8889

< 67E6 /td>
8990
function getBooleanComparison(
9091
node: TSESTree.BinaryExpression,
@@ -94,19 +95,23 @@ export default createRule<Options, MessageIds>({
9495
return undefined;
9596
}
9697

97-
const expressionType = getConstrainedTypeAtLocation(
98-
services,
99-
comparison.expression,
98+
const { constraintType, isTypeParameter } = getConstraintInfo(
99+
checker,
100+
services.getTypeAtLocation(comparison.expression),
100101
);
101102

102-
if (isBooleanType(expressionType)) {
103+
if (isTypeParameter && constraintType == null) {
104+
return undefined;
105+
} A3E2
106+
107+
if (isBooleanType(constraintType)) {
103108
return {
104109
...comparison,
105110
expressionIsNullableBoolean: false,
106111
};
107112
}
108113

109-
if (isNullableBoolean(expressionType)) {
114+
if (isNullableBoolean(constraintType)) {
110115
return {
111116
...comparison,
112117
expressionIsNullableBoolean: true,

‎packages/eslint-plugin/src/rules/no-unnecessary-condition.ts

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as ts from 'typescript';
77
import {
88
createRule,
99
getConstrainedTypeAtLocation,
10+
getConstraintInfo,
1011
getParserServices,
1112
getTypeName,
1213
getTypeOfPropertyOfName,
@@ -653,16 +654,7 @@ export default createRule<Options, MessageId>({
653654
.getCallSignaturesOfType(
654655
getConstrainedTypeAtLocation(services, callback),
655656
)
656-
.map(sig => sig.getReturnType())
657-
.map(t => {
658-
// TODO: use `getConstraintTypeInfoAtLocation` once it's merged
659-
// https://github.com/typescript-eslint/typescript-eslint/pull/10496
660-
if (tsutils.isTypeParameter(t)) {
661-
return checker.getBaseConstraintOfType(t);
662-
}
663-
664-
return t;
665-
});
657+
.map(sig => sig.getReturnType());
666658

667659
if (returnTypes.length === 0) {
668660
// Not a callable function, e.g. `any`
@@ -673,16 +665,21 @@ export default createRule<Options, MessageId>({
673665
let hasTruthyReturnTypes = false;
674666

675667
for (const type of returnTypes) {
668+
const { constraintType } = getConstraintInfo(checker, type);
676669
// Predicate is always necessary if it involves `any` or `unknown`
677-
if (!type || isTypeAnyType(type) || isTypeUnknownType(type)) {
670+
if (
671+
!constraintType ||
672+
isTypeAnyType(constraintType) ||
673+
isTypeUnknownType(constraintType)
674+
) {
678675
return;
679676
}
680677

681-
if (isPossiblyFalsy(type)) {
678+
if (isPossiblyFalsy(constraintType)) {
682679
hasFalsyReturnTypes = true;
683680
}
684681

685-
if (isPossiblyTruthy(type)) {
682+
if (isPossiblyTruthy(constraintType)) {
686683
hasTruthyReturnTypes = true;
687684
}
688685

‎packages/eslint-plugin/src/util/getConstraintInfo.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import type * as ts from 'typescript';
2+
3+
import * as tsutils from 'ts-api-utils';
4+
5+
export interface ConstraintTypeInfoUnconstrained {
6+
constraintType: undefined;
7+
isTypeParameter: true;
8+
}
9+
10+
export interface ConstraintTypeInfoConstrained {
11+
constraintType: ts.Type;
12+
isTypeParameter: true;
13+
}
14+
15+
export interface ConstraintTypeInfoNonGeneric {
16+
constraintType: ts.Type;
17+
isTypeParameter: false;
18+
}
19+
20+
export type ConstraintTypeInfo =
21+
| ConstraintTypeInfoConstrained
22+
| ConstraintTypeInfoNonGeneric
23+
| ConstraintTypeInfoUnconstrained;
24+
25+
/**
26+
* Returns whether the type is a generic and what its constraint is.
27+
*
28+
* If the type is not a generic, `isTypeParameter` will be `false`, and
29+
* `constraintType` will be the same as the input type.
30+
*
31+
* If the type is a generic, and it is constrained, `isTypeParameter` will be
32+
* `true`, and `constraintType` will be the constraint type.
33+
*
34+
* If the type is a generic, but it is not constrained, `constraintType` will be
35+
* `undefined` (rather than an `unknown` type), due to https://github.com/microsoft/TypeScript/issues/60475
36+
*
37+
* Successor to {@link getConstrainedTypeAtLocation} due to https://github.com/typescript-eslint/typescript-eslint/issues/10438
38+
*
39+
* This is considered internal since it is unstable for now and may have breaking changes at any time.
40+
* Use at your own risk.
41+
*
42+
* @internal
43+
*
44+
*/
45+
export function getConstraintInfo(
46+
checker: ts.TypeChecker,
47+
type: ts.Type,
48+
): ConstraintTypeInfo {
49+
if (tsutils.isTypeParameter(type)) {
50+
const constraintType = checker.getBaseConstraintOfType(type);
51+
return {
52+
constraintType,
53+
isTypeParameter: true,
54+
};
55+
}
56+
return {
57+
constraintType: type,
58+
isTypeParameter: false,
59+
};
60+
}

‎packages/eslint-plugin/src/util/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@ export * from './objectIterators';
2323
export * from './needsToBeAwaited';
2424
export * from './scopeUtils';
2525
export * from './types';
26+
export * from './getConstraintInfo';
2627

2728
// this is done for convenience - saves migrating all of the old rules
2829
export * from '@typescript-eslint/type-utils';
30+
2931
const {
3032
applyDefault,
3133
deepMerge,

‎packages/eslint-plugin/src/util/needsToBeAwaited.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import {
66
} from '@typescript-eslint/type-utils';
77
import * as tsutils from 'ts-api-utils';
88

9+
import { getConstraintInfo } from './getConstraintInfo';
10+
911
export enum Awaitable {
1012
Always,
1113
Never,
@@ -17,24 +19,20 @@ export function needsToBeAwaited(
1719
node: ts.Node,
1820
type: ts.Type,
1921
): Awaitable {
20-
// can't use `getConstrainedTypeAtLocation` directly since it's bugged for
21-
// unconstrained generics.
22-
const constrainedType = !tsutils.isTypeParameter(type)
23-
? type
24-
: checker.getBaseConstraintOfType(type);
22+
const { constraintType, isTypeParameter } = getConstraintInfo(checker, type);
2523

2624
// unconstrained generic types should be treated as unknown
27-
if (constrainedType == null) {
25+
if (isTypeParameter && constraintType == null) {
2826
return Awaitable.May;
2927
}
3028

3129
// `any` and `unknown` types may need to be awaited
32-
if (isTypeAnyType(constrainedType) || isTypeUnknownType(constrainedType)) {
30+
if (isTypeAnyType(constraintType) || isTypeUnknownType(constraintType)) {
3331
return Awaitable.May;
3432
}
3533

3634
// 'thenable' values should always be be awaited
37-
if (tsutils.isThenableType(checker, node, constrainedType)) {
35+
if (tsutils.isThenableType(checker, node, constraintType)) {
3836
return Awaitable.Always;
3937
}
4038

‎packages/eslint-plugin/tests/rules/no-unnecessary-boolean-literal-compare.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,20 @@ ruleTester.run('no-unnecessary-boolean-literal-compare', rule, {
109109
},
110110
"'false' === true;",
111111
"'true' === false;",
112+
`
113+
const unconstrained: <T>(someCondition: T) => void = someCondition => {
114+
if (someCondition === true) {
115+
}
116+
};
117+
`,
118+
`
119+
const extendsUnknown: <T extends unknown>(
120+
someCondition: T,
121+
) => void = someCondition => {
122+
if (someCondition === true) {
123+
}
124+
};
125+
`,
112126
],
113127

114128
invalid: [

0 commit comments

Comments
 (0)
0