Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a rule to disallow mixed types #361

Closed
kangax opened this issue Oct 9, 2018 · 12 comments
Closed

Add a rule to disallow mixed types #361

kangax opened this issue Oct 9, 2018 · 12 comments

Comments

@kangax
Copy link
Contributor

kangax commented Oct 9, 2018

There are cases in our app where developers use mixed instead of a more exact value. While there are definitely valid scenarios for mixed — e.g. on a utility/library level — they're almost never needed on an application level.

It would be nice to have the rule to disallow them.

This could be another value in a no-weak-types or a separate rule.

@gajus
Copy link
Owner

gajus commented Oct 13, 2018

🎉 This issue has been resolved in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Hypnosphi
Copy link

Hypnosphi commented Oct 17, 2018

mixed is actually stricter than any other type, as it allows no assumptions about the value. I don't get how can it be considered "weak".

The usecase for app is any function that accepts a callback and doesn't use its return value:

function callCb(cb: () => mixed) {
  cb()
}

https://flow.org/en/docs/types/functions/#toc-function-type

@gajus
Copy link
Owner

gajus commented Oct 17, 2018

https://flow.org/en/docs/types/mixed/

Can be any primitive data type (number, string).

I don't understand the example that you've shared.

Shouldn't it be void instead?

function callCb(cb: () => void) {
  cb()
}

@Hypnosphi
Copy link

Can be any primitive data type

It doesn't make the type weak. You can't mistakenly type number as string when using mixed (as contrary to any). Coercing mixed to any other type always throws an error.

Shouldn't it be void instead?

then it will error on a callback that returns something (a promise, for example)

The example is from docs: https://flow.org/en/docs/types/functions/#toc-function-type

@gajus
Copy link
Owner

gajus commented Oct 19, 2018

It doesn't make the type weak. You can't mistakenly type number as string when using mixed (as contrary to any). Coercing mixed to any other type always throws an error.

OK. I can see that. It doesn't make much sense to refer to it as a weak type indeed.

Does it make sense to have this logic as a separate rule?

@gajus gajus reopened this Oct 19, 2018
@Hypnosphi
Copy link

Does it make sense to have this logic as a separate rule?

Sure, people may want to only accept callbacks with a specific return value in their app.

@leoselig
Copy link
Contributor

leoselig commented Nov 1, 2018

I would sincerely propose to revert this change. As @Hypnosphi pointed out correctly, mixed is not in any way weak. It is the proper way to demand code dealing with the value to deal with all possible values.
This is the absolute opposite to:

  • Function (call it with what you want)
  • Object (it's an object, but you.can.access.anything.you.want)
  • any (essentially: skip type-checking anything you do with it until casting it to a concrete type again)

@gajus
Copy link
Owner

gajus commented Nov 1, 2018

I would sincerely propose to revert this change.

Agreed. Done.

@kangax
Copy link
Contributor Author

kangax commented Nov 1, 2018

@leoselig @gajus I find this to be a matter of semantics. In Flow lingo, Function, Object, and any are called "unsafe". There's no such thing as "weak", as far as I know, so it really is up to the rule to determine what to consider "weak" and what not to. Alternatively, the rule could be called "no-unsafe-types" and then we wouldn't need to argue about any of this :)

The bottom line is that mixed is a more permissive set of values than any of the number, string, boolean, etc. As a result, people tend to use mixed where a more specific value would lead to a better safety. This is where a lint rule comes in. And this is why I think it's useful.

I can make a follow-up PR to have this as a separate rule. What would be a good name? Perhaps something generic where we disallow specified values (I can't imagine why someone would want to disable other types but who knows)?

@leoselig
Copy link
Contributor

leoselig commented Nov 1, 2018

@kangax
That sounds like a neat solution. The use cases just seem to different to me since any,Object, Function make you code not type check in parts (hence less safe) while mixed might be "lazy" generalization that should be avoided to reduce the necessity for checks.

For the same reasons that you mention I would actually vote for no-mixed. :D

@gajus
Copy link
Owner

gajus commented Nov 1, 2018

I can make a follow-up PR to have this as a separate rule. What would be a good name? Perhaps something generic where we disallow specified values (I can't imagine why someone would want to disable other types but who knows)?

Please go ahead.

Sorry for the back and forth.

@gajus
Copy link
Owner

gajus commented May 18, 2020

Closing as stale.

@gajus gajus closed this as completed May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants