noImplicitAny as suggestion #27693
noImplicitAny as suggestion #27693
Conversation
Note that not all noImplicitAny errors turn into suggestions. In particular, 1. reportErrorsFromWidening does not, because it doesn't log an error that infer-from-usage fixes, and fixing it would require otherwise-unnecessary code. 2. auto types do not have implicit any suggestions, because that would require running control flow in noImplicitAny mode.
| @@ -13270,7 +13264,7 @@ namespace ts { | |||
| return errorReported; | |||
| } | |||
|
|
|||
| function reportImplicitAnyError(declaration: Declaration, type: Type) { | |||
| function reportImplicitAnyError(noImplicitAny: boolean, declaration: Declaration, type: Type) { | |||
There was a problem hiding this comment.
May be rename this to reportImplicitAnyErrorOrSuggestion instead?
There was a problem hiding this comment.
It's not necessary to take noImplicitAny as a parameter since that's already in scope. In every instance of calling this the argument is noImplicitAny; in one case the argument is true but it's within if (noImplicitAny).
There was a problem hiding this comment.
I chose to shorten it to reportImplicitAny
There was a problem hiding this comment.
@andy-ms good catch. I'll get rid of the parameter.
|
@andy-ms I discussed this with @DanielRosenwasser and we decided that, for now, the easiest and strongest signal of intent is |
In JS, you only get implicit any errors/suggestions with checkJS or ts-check turned on.
| const typeAsString = typeToString(getWidenedType(type)); | ||
| if (isInJSFile(declaration) && !getSourceFileOfNode(declaration).pragmas.has("checkJSDirective") && !compilerOptions.checkJs) { |
There was a problem hiding this comment.
Do we even get here without checkJS? Also, this must be duplicate code of something?
There was a problem hiding this comment.
- Yes, allowJS reports errors as usual, but doesn't actually display them.
- Yes,
isCheckJsEnabledForFile.
There was a problem hiding this comment.
So, why go out of our way to not report this error, if no errors will be shown anyway?
There was a problem hiding this comment.
Suggestions will still be shown, since they're stored in a separate array, and errorOrSuggestion at the bottom of the function will create a suggestion instead of an error in an allowJs file.
|
Performance seems fine. I tried on webpack, which is a big project that has tons of implicit anys intermixed with existing jsdoc. I'm pretty sure the codefix calculations are correctly deferred to when the lightbulb is clicked. |
With noImplicitAny suggestions, people will see suggestions for the infer-from-usage codefix even with noImplicitAny turned off. This applies to JS users, who are otherwise unlikely to see that codefix.
Notes:
I want to evaluate performance before merging this.
I want to make sure the suggestions are mostly reliable and sensical before merging as well. I'm going to look over our user tests.
Not all noImplicitAny errors turn into suggestions. In
particular: