Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat(eslint-plugin): [no-unnecessary-condition] flag unnecessary type predicates based on assignability#11735

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
azat-io wants to merge3 commits intotypescript-eslint:main
base:main
Choose a base branch
Loading
fromazat-io:feat/no-unnecessary-condition-array
Open
Show file tree
Hide file tree
Changes fromall 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
View file
Open in desktop

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

If I replace this entire diff with

-          if (typeOfArgument === typeGuardAssertedArgument.type) {+          if (+            checker.isTypeAssignableTo(+              typeOfArgument,+              typeGuardAssertedArgument.type,+            )+          ) {

(which is what was intended in#11716 (comment)), the only test case that doesn't pass (apart from spurious reasons) is

functionprocessArray(arr:readonlystring[]){if(Array.isArray(arr)){returnarr.length;}}

So, I'm still thinking that changing to the assignability API makes sense, at least as a first step. In other words, let's take on the assignability API change before worrying about the edge case ofreadonly arrays withArray.isArray() (which, FWIW, I think is basically a corollary ofmicrosoft/TypeScript#17002). What do you think?

Also cc@ronami

ronami reacted with thumbs up emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Done! Changed to usechecker.isTypeAssignableTo() for Array.isArray

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@azat-io almost! I really mean the entire diff, though, not just the Array.isArray case. The thought is to change all unnecessary type predicate detection to be based on assignability.

Note that this change would require accompanying report message changes and additional testing

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Done! Changed to useisTypeAssignableTo() for all type predicates. RemovedisArrayIsArrayCall() function and updated the tests accordingly.

Original file line numberDiff line numberDiff line change
Expand Up@@ -599,7 +599,12 @@ export default createRule<Options, MessageId>({
services,
typeGuardAssertedArgument.argument,
);
if (typeOfArgument === typeGuardAssertedArgument.type) {
if (
checker.isTypeAssignableTo(
typeOfArgument,
typeGuardAssertedArgument.type,
)
) {
context.report({
node: typeGuardAssertedArgument.argument,
messageId: 'typeGuardAlreadyIsType',
Expand Down
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1123,21 +1123,53 @@ isString(a);
options: [{ checkTypePredicates: false }],
},
{
// Technically, this has type 'falafel' and not string.
code: `
declare function assertString(x: unknown): asserts x is string;
assertString('falafel');
declare const items: number[] | null;
if (Array.isArray(items)) {
console.log('items is an array');
}
`,
options: [{ checkTypePredicates: true }],
},
{
// Technically, this has type 'falafel' and not string.
code: `
declare function isString(x: unknown): x is string;
isString('falafel');
declare const items: number[] | string;
if (Array.isArray(items)) {
console.log('items is an array');
}
`,
options: [{ checkTypePredicates: true }],
},
{
code: `
declare const items: unknown;
if (Array.isArray(items)) {
console.log('items is an array');
}
`,
options: [{ checkTypePredicates: true }],
},
{
code: `
function process(value: string | number[]) {
if (Array.isArray(value)) {
return value.length;
}
return value.length;
}
`,
options: [{ checkTypePredicates: true }],
},
{
code: `
declare const items: number[];
if (Array.isArray(items)) {
console.log('items is an array');
}
`,
options: [{ checkTypePredicates: false }],
},

`
type A = { [name in Lowercase<string>]?: A };
declare const a: A;
Expand DownExpand Up@@ -3518,7 +3550,77 @@ isString('fa' + 'lafel');
],
options: [{ checkTypePredicates: true }],
},

{
code: `
declare const items: number[];
if (Array.isArray(items)) {
console.log('items is an array');
}
`,
errors: [
{
line: 3,
messageId: 'typeGuardAlreadyIsType',
},
],
options: [{ checkTypePredicates: true }],
},
{
code: `
declare const items: string[];
Array.isArray(items);
`,
errors: [
{
line: 3,
messageId: 'typeGuardAlreadyIsType',
},
],
options: [{ checkTypePredicates: true }],
},
{
code: `
declare const matrix: number[][];
if (Array.isArray(matrix)) {
console.log('matrix is an array');
}
`,
errors: [
{
line: 3,
messageId: 'typeGuardAlreadyIsType',
},
],
options: [{ checkTypePredicates: true }],
},
{
code: `
declare function isString(x: unknown): x is string;
isString('falafel');
`,
errors: [
{
column: 10,
line: 3,
messageId: 'typeGuardAlreadyIsType',
},
],
options: [{ checkTypePredicates: true }],
},
{
code: `
declare function assertString(x: unknown): asserts x is string;
assertString('falafel');
`,
errors: [
{
column: 14,
line: 3,
messageId: 'typeGuardAlreadyIsType',
},
],
options: [{ checkTypePredicates: true }],
},
// "branded" types
unnecessaryConditionTest('"" & {}', 'alwaysFalsy'),
unnecessaryConditionTest('"" & { __brand: string }', 'alwaysFalsy'),
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp