Bad suggestions from the no-any rule

[ChrisMBarr]

The changes in wording from #2164 are, in my opinion, a very bad idea. It gives a bad suggestion which basically encourages the use of {} over any - both of these are not ideal if the goal is to use strong typing in a project. The current text reads:

Type declaration of 'any' loses type-safety. Consider replacing it with a more precise type, the empty type ('{}'), or suppress this occurrence.

It states that any loses type-safety, but to be fair, so does {}.

I have developers on my team now using {} as a type, which in no way describes the object properly, but now there are no lint violations thrown for this new bad typing.

Here's a screenshot of a recent commit where the developer just did what the rule said to do, but not 100% understanding it.

(note their commit message on the left side)

the type ng.IPromise<any> got converted to {}, but the correct solution here was to use the type ng.IPromise<void>

Can this text be reverted, or possibly changed in some way to not entourage weak typing?

[adidahiya]

I'm inclined to agree, I think we can just remove the second & third parts of that suggestion and say "Consider replacing it with a more precise type." (rule suppression is a global concept and doesn't need to be noted in any specific rule).

Any objections @mprobst or @alexeagle?

[mprobst]

I'm okay with that Adi.

[adidahiya]

Hm, what does this mean for the auto-fixer, though? If we no longer suggest {} in the failure message, we probably shouldn't automatically replace any with {} either. It would technically be a slight regression to remove the fixer, but that feels like a safe path forward.

[alexeagle]

The empty type is a lot more type-safe than any since you can't arbitrarily call it as a function or reference its properties. We want to encourage using a specific type where possible, but {} has its place as a convenient and safer "unknown"type.
It looks to me in the original post that the bug was ng.IPromise<any> was auto-fixed to {} but should have been fixed to ng.IPromise<{}>?

[adidahiya]

I think the original post referred to a manual lint fix that misinterpreted the rule failure location & suggestion, not an auto-fix using tslint --fix.

[alexeagle]

I see. In any case I agree that auto-fixing with {} is not great, developers expect auto-fixes to be applicable without much thought, but this one is usually the wrong thing to do. Removing the auto-fix SGTM.

[evmar]

Within Google, based on user feedback, we've amended this error message to link to our (internal) docs that have a few paragraphs describing the various ways to fix this. Unfortunately it's hard to fit all the information a user needs in a one-liner. Our docs open with three broad categories of fixes and then goes into where each one applies:

"In circumstances where you want to use any, consider one of:

• Provide a more specific type
• Use the empty object type
• Suppress the lint warning"

[ChrisMBarr]

In my experience, developers use the any type as a quick way to get something to "just work" even though proper typedefs do exist that they could use. Suggesting and/or auto-fixing it with {} is bad in every way for this use case in particular because neither of these typedefs give strong typing, which is the entire goal of this rule.

[mprobst]

Chris: `{}` gives strong checks, they are just super restrictive (don't allow properties to be accessed). So that at least can be a prod for developers not to avoid `any` and then fix it up to use the correct type. At least that's what we're seeing in usage over here.

…

[ChrisMBarr]

@mprobst that's not 100% true in every case though. IMO the point of a fix is to replace the bad code portion with the 100% correct alternative, usually for style-related code. replace var with let, or replace " with ', add spaces or semicolons where they are expected. Stuff like that.

I'd argue that this rule is different because there's no alternative to any that's 100% correct all the time (well, at least not one that can be auto-fixed).

I think perhaps this rule should have an option to suggest/fix any with {} to give developers the best of both worlds with what they need for each app. In the apps I work on, I never want to suggest that a developer should use {} as an alternative to any, but clearly other developers do. We all have a different context for the apps we work on, so I think an option here might be helpful.

[alexeagle]

This is a great example where we've conflated a linter and a formatter. A formatter has 100% correct fixes, so we just run it blindly without asking it for individual diagnostic messages for each wrong spot. Anything that has a 100% correct fix (like fixing up all quotes to double/single) can therefore move into the formatter. If it's in the linter, I believe it may have different fixes depending on the developer: - suppress this instance because I want it this way - fix it in the conservative way: maintain the current semantics even if the pattern looks wrong - try to restore the original developer intent, but possibly breaking something I think developers should *always* inspect the suggestion in these cases. We could also offer multiple suggestions (this is allowed in the API)

…

On Tue, Apr 11, 2017 at 9:58 AM Chris Barr ***@***.***> wrote: @mprobst <https://github.com/mprobst> that's not 100% true in every case though. IMO the point of a fix is to replace the bad code portion with the 100% correct alternative, usually for style-related code. replace var with let, or replace " with ', add spaces or semicolons where they are expected. Stuff like that. I'd argue that this rule is different because there's no alternative to any that's 100% correct all the time (well, at least not one that can be auto-fixed). I think perhaps this rule should have an option to suggest/fix any with {} to give developers the best of both worlds with what they need for each app. In the apps I work on, I *never* want to suggest that a developer should use {} as an alternative to any, but clearly other developers do. We all have a different context for the apps we work on, so I think an option here might be helpful. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2491 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC5Iw9aU7l9nqJnizjYLJQWUp2kK80-ks5ru7FBgaJpZM4My1kk> .

[alexeagle]

@ChrisMBarr do you remember why you closed this? Still seems like we should remove the fixer for no-any

[ChrisMBarr]

@alexeagle Oh yes, sorry. I was under the impression that the auto-fixer was already removed and this issue was forgotten about. Sorry. Re-opened now.