Skip to content

fix(eslint-plugin): [no-unsafe-type-assertion] fix for unsafe assertion to a constrained type parameter #10461

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Dec 21, 2024
Merged

fix(eslint-plugin): [no-unsafe-type-assertion] fix for unsafe assertion to a constrained type parameter #10461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7529400
Add failing test case for unsafe type assertion with constrained gene…
controversial Dec 5, 2024
9117da0
Use actual type instead of “constrained type” for no-unsafe-type-asse…
controversial Dec 5, 2024
ece33d8
tweak test cases
controversial Dec 5, 2024
354dc32
Custom error message for “assignable to the constraint” case
controversial Dec 5, 2024
05bdf78
rename “assignable to the constraint” error message
controversial Dec 5, 2024
3578532
spelling
controversial Dec 5, 2024
a1cf417
tweak message
controversial Dec 5, 2024
d7a7600
Add new test cases from @kirkwaiblinger
controversial Dec 6, 2024
60cba85
Move isAssignableToConstraint check into isAssertionSafe
controversial Dec 6, 2024
e560ef3
Properly handle unconstrained type parameters, including new error me…
controversial Dec 6, 2024
c39c4b7
distinguish between “parameter extends other parameter” and “paramete…
controversial Dec 6, 2024
3c60099
Merge branch 'main' into unsafe-constrained-type-assertion
controversial Dec 6, 2024
7719c45
Merge branch 'main' into unsafe-constrained-type-assertion
controversial Dec 16, 2024
504df89
clean up extra pieces in unit tests
controversial Dec 17, 2024
239f3df
test lines
kirkwaiblinger Dec 17, 2024
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
71 changes: 52 additions & 19 deletions packages/eslint-plugin/src/rules/no-unsafe-type-assertion.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import * as ts from 'typescript';

import {
createRule,
getConstrainedTypeAtLocation,
getParserServices,
isTypeAnyType,
isTypeUnknownType,
Expand All @@ -25,8 +24,12 @@ export default createRule({
'Unsafe assertion from {{type}} detected: consider using type guards or a safer assertion.',
unsafeToAnyTypeAssertion:
'Unsafe assertion to {{type}} detected: consider using a more specific type to ensure safety.',
unsafeToUnconstrainedTypeAssertion:
"Unsafe type assertion: '{{type}}' could be instantiated with an arbitrary type which could be unrelated to the original type.",
unsafeTypeAssertion:
"Unsafe type assertion: type '{{type}}' is more narrow than the original type.",
unsafeTypeAssertionAssignableToConstraint:
"Unsafe type assertion: the original type is assignable to the constraint of type '{{type}}', but '{{type}}' could be instantiated with a different subtype of its constraint.",
},
schema: [],
},
Expand All @@ -49,14 +52,8 @@ export default createRule({
function checkExpression(
node: TSESTree.TSAsExpression | TSESTree.TSTypeAssertion,
): void {
const expressionType = getConstrainedTypeAtLocation(
services,
node.expression,
);
const assertedType = getConstrainedTypeAtLocation(
services,
node.typeAnnotation,
);
const expressionType = services.getTypeAtLocation(node.expression);
const assertedType = services.getTypeAtLocation(node.typeAnnotation);

if (expressionType === assertedType) {
return;
Expand Down Expand Up @@ -115,24 +112,60 @@ export default createRule({

// Use the widened type in case of an object literal so `isTypeAssignableTo()`
// won't fail on excess property check.
const nodeWidenedType = isObjectLiteralType(expressionType)
const expressionWidenedType = isObjectLiteralType(expressionType)
? checker.getWidenedType(expressionType)
: expressionType;

const isAssertionSafe = checker.isTypeAssignableTo(
nodeWidenedType,
expressionWidenedType,
assertedType,
);
if (isAssertionSafe) {
return;
}

if (!isAssertionSafe) {
context.report({
node,
messageId: 'unsafeTypeAssertion',
data: {
type: checker.typeToString(assertedType),
},
});
// Produce a more specific error message when targeting a type parameter
if (tsutils.isTypeParameter(assertedType)) {
const assertedTypeConstraint =
checker.getBaseConstraintOfType(assertedType);
if (!assertedTypeConstraint) {
// asserting to an unconstrained type parameter is unsafe
context.report({
node,
messageId: 'unsafeToUnconstrainedTypeAssertion',
data: {
type: checker.typeToString(assertedType),
},
});
return;
}

// special case message if the original type is assignable to the
// constraint of the target type parameter
const isAssignableToConstraint = checker.isTypeAssignableTo(
expressionWidenedType,
assertedTypeConstraint,
);
if (isAssignableToConstraint) {
context.report({
node,
messageId: 'unsafeTypeAssertionAssignableToConstraint',
data: {
type: checker.typeToString(assertedType),
},
});
return;
}
}

// General error message
context.report({
node,
messageId: 'unsafeTypeAssertion',
data: {
type: checker.typeToString(assertedType),
},
});
}

return {
Expand Down
Loading
Loading