Add fixers for no-any and object-literal-shortand (properties only) #2165
Conversation
adidahiya
changed the title
| @@ -42,7 +43,10 @@ export class Rule extends Lint.Rules.AbstractRule { | |||
|
|
|||
| class NoAnyWalker extends Lint.RuleWalker { | |||
| public visitAnyKeyword(node: ts.Node) { | |||
| this.addFailureAtNode(node, Rule.FAILURE_STRING); | |||
| const fix = this.createFix( | |||
| this.createReplacement(node.getStart(), node.getWidth(), "{}"), | |||
There was a problem hiding this comment.
What is the benefit of replacing any with {}? I don't think this rule can be autofixed since the point is to use a specific type.
There was a problem hiding this comment.
It cannot really be auto-fixed, but I see a lot of people using any when they mean {}, i.e. a type they don't care about and don't want to access properties on that is a super type of everything. I agree, it's not all that great, but maybe better than no fix?
There was a problem hiding this comment.
If you have the no-any rule turned on (none of our projects do @nchen63), this seems like a reasonable fix to me. Auto-fixes can always be reverted, so I'm open to trying this out and waiting for feedback.
There was a problem hiding this comment.
any (both a supertype and a subtype of every type) is semantically different from {} (a supertype of every type). Consider the following example:
declare const a1: any;
const a2: number = a1; // `any` can be cast to `number`
declare const b1: {};
const b2: number = b1; // error: `{}` cannot be cast to `numer`
It is okay to advice using {} in linter warning message. But autofix should try to preserve the original semantic as much as possible without changing the semantic of the source code. At the minimum least, can tslint provide option to selectively disable autofix for this specific linter rule?
There was a problem hiding this comment.
@pyrocat101 indeed, this linter autofix changes the (type) semantics of the code. I think that's the point of the linter warning though. You enable it when you don't want people to use any by default. As there is no semantically compatible option to any that we could automatically introduce, finding the appropriate replacement for any usually requires a human brain, so users most of the time will have to fix it themselves.
There is a large subset of cases though where users use any to mean whatever, i.e. a type with no requirements. Here, {} is an appropriate replacement.
I'd suggest either disabling the warning in the first place if you're cool with using any in your code base, or using the other auto-fix option that IDEs will show, i.e. adding the comment to disable the warning.
|
@mprobst thanks! |
PR checklist
What changes did you make?
Add fixers for no-any and objectLiteralShorthand (for keys).
For object literals with functions, it's sadly pretty tricky to find the opening
(token, so for now those will have to live without a fix.