sindresorhus / eslint-plugin-unicorn

More than 100 powerful ESLint rules
MIT License
4.28k stars 367 forks source link

`prefer-native-coercion-functions` should ignore type guards #1857

Open JacobLey opened 2 years ago

JacobLey commented 2 years ago

prefer-native-coercion-functions is used to simplify methods like filter that are simply trying to find truthy values. However the native type castings do not provide any typescript assertions, so the resulting type of the filter is unchanged. When manually providing these assertions, prefer-native-coercion-functions should not suggest a native coercion over the manual callback.

prefer-native-coercion-functions

const mixedData: (string | null)[] = ['abc', '', null, 'xyz'];

// Eslint is happy, but `null` is not removed from type
const untypedMixedData: (string | null)[] = mixedData.filter(Boolean);

// Eslint is unhappy, but `null` is removed from type
const typedMixedData: string[] = mixedData.filter((d): d is string => Boolean(d));

Ideally that second example is ignored by this eslint rule because the custom assertions provide context that the native coercion function does not.


Alternatively the resulting array can just be manually typed casted, but I would argue that typescript's default typings via assertions is preferred.

const castMixedData: string[] = mixedData.filter(Boolean) as string[];

Similarly I could provide some method like isTruthy

const isTruthy = (x: unknown): x is string | number | symbol | object | unknown[] => Boolean(x);
const isTruthyData = mixedData.filter(isTruthy);

but that will interfere with no-array-callback-reference.

fregante commented 1 year ago

I agree that this should work:

const mixedData: (string | null)[] = ['abc', '', null, 'xyz'];

// Eslint is unhappy, but `null` is removed from type
const typedMixedData: string[] = mixedData.filter((d): d is string => Boolean(d));

Similarly I could provide some method like isTruthy

Note that this example however is wrong, isTruthy(true) asserts that true is string | number | symbol | object | unknown[], which it isn't

carmanchris31 commented 1 year ago

related: there's a request in TypeScript to use boolean as a type guard: https://github.com/microsoft/TypeScript/issues/16069

leppaott commented 8 months ago

Would be nice to have this ignore ones with type guards if possible indeed because the rule seems useful. There seems to be a new PR to fix this on TS but it's been years and years so could be good to fix this for older TS versions.

lachieh commented 6 months ago

For those curious, the underlying TypeScript feature to assert types from boolean coercion functions has been implemented and will be released in TypeScript 5.5: https://github.com/microsoft/TypeScript/pull/57465#issuecomment-2016987675

fregante commented 2 months ago

Closing as mixedData.filter(Boolean) works in:

leppaott commented 2 months ago

@fregante what do you mean https://github.com/microsoft/TypeScript/pull/57465#issuecomment-2184170318 .filter(Boolean) doesn't work it has to be .filter((uuid) => uuid !== undefined) or .filter((uuid) => uuid !== null) to have narrowing working. I'm hoping they get it working later because that's a pretty common use-case indeed. We have TS 5.5.4.

fregante commented 2 months ago

Indeed, I was under the impression that it worked.

Demo: https://www.typescriptlang.org/play/?ts=5.7.0-dev.20240904#code/MYewdgzgLgBFAWBLMBzCMC8MDaBXMAJgKYBmyRBANDCQIYA2ER1UATrszAAzUCMAujFrpQkKAG4AUKOhx2CAJ69McJKggA6MvShFWACn1QAlJgB8MAISWTUmPYcwA9E5gA9APzTwstrkUATCoIyGhaiDp6+gBCICD0RLRgxnaO9i7uHkA

lachieh commented 2 months ago

Yeah, that's my bad. This is the issue to track: microsoft/TypeScript#16655