coderaiser / putout

🐊 Pluggable and configurable JavaScript Linter, code transformer and formatter, drop-in ESLint superpower replacement 💪 with built-in support for js, jsx, typescript, flow, markdown, yaml and json. Write declarative codemods in a simplest possible way 😏
https://putout.cloudcmd.io/
MIT License
698 stars 40 forks source link

[types/convert-typeof-to-is-type]: autofixes to itself, if the function is the only function inside the file. #154

Closed ElPrudi closed 1 year ago

ElPrudi commented 1 year ago

I like the idea of this rule, but there is an edge case it doesn't look for. For clarity, I moved this function to it's own file:

export default function isString(value: unknown): value is string {
    return typeof value === 'string'
}

The funny thing is, eslint-plugin-putout see this as an error:

  2:12  error  Use function to check type instead of 'typeof' (types/convert-typeof-to-is-type)  putout/putout

And even funnier is, that it replaces the function body with itself:

export default function isString(value: unknown): value is string {
    return isString(value)
}
coderaiser commented 1 year ago

Thanks! Just fixed 🎉 , is it works for you?

ElPrudi commented 1 year ago

Does not work if you name your function differently. Instead of isFn I use isFunction and for isBool I use isBoolean. You need to find a way to specify this independently of the function name.

coderaiser commented 1 year ago

I don't think we should move far away from current list functions, stick to it, and think good before apply big changes.

isBool is fine since there is __bool, so this things are consistent, and isFn is short enough to understand what it is. There is also __args shorthand.

The main idea is: that's OK to use short names for functions used often. For example Rust simplifies syntax constructions, so this is the same code:

fn foo() -> i32 {
    return 3;
}
fn foo() -> i32 {
    3
}

It can look inconvenient on a first glance, but on the long run it is very comfortable.

ElPrudi commented 1 year ago

From that perspective, I understand. I would add a small hint that other function names are not recognized. I had assumed that it would be possible with PutoutScript to configure any aliases, for example to fix isFn with the alias isFunction. You have to decide. But from my point of view it would be completely fine.

coderaiser commented 1 year ago

That's a good idea, some kind of Options can be used for this purpose (similar to exclude), but a couple plugins should be changed:

So if you add options to types/convert-typeof-to-is-type I'll merge.