migtools / mig-ui

Openshift Migration UI
Apache License 2.0
10 stars 31 forks source link

Add no-implicit-any rule to eslint config #819

Open ibolton336 opened 4 years ago

ibolton336 commented 4 years ago

Add a rule enforcing the use of typescript in most cases other than those that are impractical to know what the type will be.

mturley commented 4 years ago

I just realized I was wrong about no-implicit-any being an eslint rule, it's a TypeScript compiler option (so it has no auto-fix). Also, I was originally thinking it would be good to enable it and then replace all implicit any with explicit any type annotations, so they stick out more until we can go and replace them... but after looking into it more, I'm not sure that's a good idea after all.

We should of course try to avoid implicit any when we can instead define an explicit (non-any) type. But it seems we shouldn't replace them with explicit any because that will disable the compiler's implicit type inference, which can sometimes save us even if we don't have a type annotation. This conversation is what convinced me to change my mind here: https://www.reddit.com/r/typescript/comments/cwmzu0/is_there_tool_to_refactor_all_implicit_anys_to/eyd53s6?utm_source=share&utm_medium=web2x

We should eventually try to have explicit types everywhere the compiler is assuming any, but now I'm thinking we shouldn't enable noImplicitAny until we do. If anything, at that point we should add both noImplicitAny to tsconfig and no-explicit-any to eslint, and only when we have full type annotations for every value. That's a lot of work, and I now realize throwing explicit any everywhere is a step backwards, not forwards.

Note that I do think it's fine to have implicit types if they don't resolve to any (e.g. if a function specifies that it returns a string, we don't really need to have the variable it is assigned to annotated as a string), the compiler will already know enough.