Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
fix(eslint-plugin): [non-nullable-type-assertion-style] fix false positive when asserting to a generic type that might be nullish#4509
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
Uh oh!
There was an error while loading.Please reload this page.
Changes fromall commits
32dfcf4ce4706e754d48d8befbe2da49d03File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -17,7 +17,7 @@ export default util.createRule({ | ||
| fixable: 'code', | ||
| messages: { | ||
| preferNonNullAssertion: | ||
| 'Use a ! assertion to moresuccinctly remove null and undefined from the type.', | ||
JoshuaKGoldberg marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| }, | ||
| schema: [], | ||
| type: 'suggestion', | ||
| @@ -43,22 +43,42 @@ export default util.createRule({ | ||
| return tsutils.unionTypeParts(type); | ||
| }; | ||
| const couldBeNullish = (type: ts.Type): boolean => { | ||
| if (type.flags & ts.TypeFlags.TypeParameter) { | ||
| const constraint = type.getConstraint(); | ||
| return constraint == null || couldBeNullish(constraint); | ||
| } else if (tsutils.isUnionType(type)) { | ||
djcsdy marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| for (const part of type.types) { | ||
| if (couldBeNullish(part)) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } else { | ||
| return ( | ||
| (type.flags & (ts.TypeFlags.Null | ts.TypeFlags.Undefined)) !== 0 | ||
| ); | ||
| } | ||
| }; | ||
| const sameTypeWithoutNullish = ( | ||
| assertedTypes: ts.Type[], | ||
| originalTypes: ts.Type[], | ||
| ): boolean => { | ||
| const nonNullishOriginalTypes = originalTypes.filter( | ||
| type => | ||
| (type.flags & (ts.TypeFlags.Null | ts.TypeFlags.Undefined)) === 0, | ||
| ); | ||
| if (nonNullishOriginalTypes.length === originalTypes.length) { | ||
| return false; | ||
| } | ||
| for (const assertedType of assertedTypes) { | ||
| if ( | ||
| couldBeNullish(assertedType) || | ||
| !nonNullishOriginalTypes.includes(assertedType) | ||
| ) { | ||
| return false; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "extends":"./tsconfig.json", | ||
| "compilerOptions": { | ||
| "noUncheckedIndexedAccess":true | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Really nice find here. I shudder to think what other subtle edge cases have popped up because of that 😄 ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. fwiw this issue can arise even without Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Oh? I was playing around with this for a good bit to try to find one but couldn't find anything. Is there a code snippet you have in mind? ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I couldn't find one either to be honest, but I didn't try all that hard. Maybe it really is dependent on the flag. But even if it isn't I think this fix should suffice. I'll do some experimentation. | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -7,7 +7,7 @@ const ruleTester = new RuleTester({ | ||
| parserOptions: { | ||
| sourceType: 'module', | ||
| tsconfigRootDir: rootDir, | ||
| project: './tsconfig.noUncheckedIndexedAccess.json', | ||
| }, | ||
| parser: '@typescript-eslint/parser', | ||
| }); | ||
| @@ -61,6 +61,35 @@ const x = 1 as 1; | ||
| declare function foo<T = any>(): T; | ||
| const bar = foo() as number; | ||
| `, | ||
| ` | ||
| function first<T>(array: ArrayLike<T>): T | null { | ||
| return array.length > 0 ? (array[0] as T) : null; | ||
| } | ||
| `, | ||
| ` | ||
| function first<T extends string | null>(array: ArrayLike<T>): T | null { | ||
djcsdy marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| return array.length > 0 ? (array[0] as T) : null; | ||
| } | ||
| `, | ||
| ` | ||
| function first<T extends string | undefined>(array: ArrayLike<T>): T | null { | ||
| return array.length > 0 ? (array[0] as T) : null; | ||
| } | ||
| `, | ||
| ` | ||
| function first<T extends string | null | undefined>( | ||
| array: ArrayLike<T>, | ||
| ): T | null { | ||
| return array.length > 0 ? (array[0] as T) : null; | ||
| } | ||
| `, | ||
| ` | ||
| type A = 'a' | 'A'; | ||
| type B = 'b' | 'B'; | ||
| function first<T extends A | B | null>(array: ArrayLike<T>): T | null { | ||
| return array.length > 0 ? (array[0] as T) : null; | ||
| } | ||
| `, | ||
| ], | ||
| invalid: [ | ||
| @@ -199,5 +228,26 @@ declare const x: T; | ||
| const y = x!; | ||
| `, | ||
| }, | ||
| { | ||
| code: ` | ||
| function first<T extends string | number>(array: ArrayLike<T>): T | null { | ||
| return array.length > 0 ? (array[0] as T) : null; | ||
| } | ||
| `, | ||
| errors: [ | ||
| { | ||
| column: 30, | ||
| line: 3, | ||
| messageId: 'preferNonNullAssertion', | ||
| }, | ||
| ], | ||
| // Output is not expected to match required formatting due to excess parentheses | ||
| // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting | ||
| output: ` | ||
| function first<T extends string | number>(array: ArrayLike<T>): T | null { | ||
| return array.length > 0 ? (array[0]!) : null; | ||
| } | ||
| `, | ||
| }, | ||
| ], | ||
| }); | ||