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 refactoring that annotates an implicit or explicit any with a best guess type inferred from its use sites #13243

Open
aluanhaddad opened this issue Jan 1, 2017 · 17 comments
Labels
Committed The team has roadmapped this issue Domain: Quick Fixes Editor-provided fixes, often called code actions. Suggestion An idea for TypeScript
Milestone

Comments

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Jan 1, 2017

I would very much appreciate having a codefix (refactoring) that, given an explicit any annotation, offers to change it to an {} annotation.

For example, given

function log(x: any) {
  console.log(x);
}

a code fix would be offered to change the declared type of x to {} resulting in

function log(x: {}) {
  console.log(x);
}

However, given

function log(x: any) {
  console.log(x.name);
}

a code fix would be offered to change the declared type of x to { name: any } resulting in

function log(x: {name: any}) {
  console.log(x);
}
@aluanhaddad aluanhaddad changed the title add Codefix that demotes explicit any annotation to {} Add a refactoring that demotes an explicit any annotation to an {} annotation Jan 1, 2017
@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented Jan 2, 2017

but any is more than {} technically speaking, if we take it to absurd level it's no different than asking to replace any with HTMLDivElement

why {}?

(might have been {} | null | undefined | void or {} | null | undefined | void | boolean | string | number | Date | Regexp | [] for that matter)

@basarat
Copy link
Contributor

basarat commented Jan 2, 2017

Perhaps he wants any => all the members that are currently accessed?

function log(x: any) {
  console.log(x.bar.baz);
}

=>

function log(x: {bar: {baz: any}}) {
  console.log(x.bar.baz);
}

That said the original suggestion is easily done in most editors with expand selection + { + delete shortcuts e.g. alm:

tmp

I do this quite commonly myself as well to cleanup existing JS code to discover how the argument is actually used 🌹

@zpdDG4gta8XKpMCd
Copy link

@basarat what is the competitive advantage of atom compared to vscode?

@basarat
Copy link
Contributor

basarat commented Jan 2, 2017

@Aleksey-Bykov alm (not atom). Most devs have their own setup to help them work better / easily / faster / work the way they do. I just shared mine as alm. Features are covered here https://basarat.gitbooks.io/alm/content/. Trying it is easy npm install alm -g so give it a go ... or not .. be happy ❤️ 🌹

@basarat
Copy link
Contributor

basarat commented Jan 2, 2017

I forgot I wrote the reasons : https://basarat.gitbooks.io/alm/content/contributing/why.html . Been a while since someone has asked :)

@xLama
Copy link

xLama commented Jan 2, 2017

I think it would be a good idea for tslint

@aluanhaddad
Copy link
Contributor Author

aluanhaddad commented Jan 2, 2017

I apologize for not providing more context and not explaining my primary use case.

As we know, --noImplicitAny generates errors when a declaration implicitly has type any. To resolve these errors, it is necessary to add a type annotation.

Implicit anys arise in different scenarios.

In some scenarios, typically function declarations, we know what the type is but the compiler has not inferred it. In such cases it is only natural to annotate it with that the type we know it should be, e.g. HTMLElement | undefined | null.

However, in other scenarios we do not yet know, do not yet care, or have not yet decided what the type should be. In such cases it is tempting to explicitly annotate with any. As surrounding code solidifies we might discover or decide on a more specific type, but because we previously used any, we are not warned by the compiler.
Here is an example:

function log(x: any) {
  console.log(value); // seems fine.
}

Now assume we decide that we want to log the name property of x.

function log(x: any) {
  console.log(x.name);
}

In the above code, our function has changed its requirements but will continue to accept values that do not have a name property, so the interface of the function is wrong. Furthermore we will not get an error for accessing the name property of a value that very well may not have one, so the implementation of function is also incorrect.

Due to the viral nature of any, this problem compounds over time such that a piece of code may be littered with explicit anys, but implicit assumptions on what the annotated values represent. These persist even when the use of the values so annotated naturally implies a structure and a set of requirements.

If {} we used instead we would satisfy the typechecker initially and we would still receive errors indicating that we should evolve the type of the value as soon as we begin to make more specific demands on it.

Having a refactoring would encourage this.
This request is about code style.

Perhaps he wants any => all the members that are currently accessed?

@basarat indeed that is the spirit of this suggestion. That would be a great feature.

but any is more than {} technically speaking

@Aleksey-Bykov indeed. I want less not more. any is a hammer. This is about incrementally updating the type of a value.

@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented Jan 2, 2017

aside from you usecase which makes sense, {} is generally speaking not assignable from null and undefined, which are quite common valid options, how would it be like outside of your case?

furthermore it's rather trivial to decontaminate your code from any either by linting (ts.SyntaxKind.AnyKeyword) or full text search

// before
function log(value: any) {
}

log(null); // <-- valid case
// after
function log(value: {}) { // <-- any replaced to {} with help of the code action
}

log(null); // <-- no longer compiles, although still theoretically valid

@aluanhaddad
Copy link
Contributor Author

aluanhaddad commented Jan 2, 2017

@Aleksey-Bykov

{} is generally speaking not assignable from null and undefined, which are quite common valid options, how would it be like outside of your case?

That is a great point. Indeed you have clarified that what I actually want is not {} but rather {} | null | undefined, which certainly changes things (at least in terms of verbosity)

furthermore it's rather trivial to decontaminate your code from any either by linting (ts.SyntaxKind.AnyKeyword) or full text search

@xLama Suggested linting as well. Perhaps that is where this idea belongs.

@zpdDG4gta8XKpMCd
Copy link

i gave it a second thought, i might be helpful if such code action looked for all places that call this function and tried to deduce the replacement type out of all possible arguments out there

// before
function log(value: any) {
}
log(1);
log('a');
// after code action
log(value: number | string) {
}
log(1);
log('a');

@aluanhaddad aluanhaddad changed the title Add a refactoring that demotes an explicit any annotation to an {} annotation Add a refactoring that annotates an implicit any with best guest type inferred from on its use sites Jan 14, 2017
@aluanhaddad aluanhaddad changed the title Add a refactoring that annotates an implicit any with best guest type inferred from on its use sites Add a refactoring that annotates an implicit any with a best guest type inferred from its use sites Jan 14, 2017
@aluanhaddad
Copy link
Contributor Author

I have renamed this issue to reflect the far more useful refactoring proposed by @Aleksey-Bykov in his remarks above.

The proposed refactoring would look at how a value implicitly typed as any is used, determine a candidate type from these use sites, and annotate this value explicitly with that type.

@aluanhaddad aluanhaddad changed the title Add a refactoring that annotates an implicit any with a best guest type inferred from its use sites Add a refactoring that annotates an implicit or explicit any with a best guest type inferred from its use sites Jan 15, 2017
@mhegazy mhegazy added Domain: Quick Fixes Editor-provided fixes, often called code actions. Suggestion An idea for TypeScript labels Jan 16, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Apr 21, 2017

Addressed by #14786

@amcasey amcasey changed the title Add a refactoring that annotates an implicit or explicit any with a best guest type inferred from its use sites Add a refactoring that annotates an implicit or explicit any with a best guess type inferred from its use sites Aug 21, 2017
@amcasey
Copy link
Member

amcasey commented Aug 21, 2017

Can this issue be closed? It sounds like it's been addressed.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 21, 2017

nope. i need to update #14786, get it reviewed and merged.

@aluanhaddad
Copy link
Contributor Author

Still super excited about this feature. Thanks @mhegazy

@ntwilson
Copy link

I just read #14786, and am also incredibly excited to start using this feature! Out of curiosity, any reason not to use the same feature for implicitly inferring function parameters rather than assuming any unannotated inputs are type any? In other words, in addition to the Quick Fix, could the compiler use the inferred type instead of any even without the annotation? I feel as though that kind of static analysis could help TypeScript catch a lot of bugs on old javascript code bases that don't have any annotations.

@ntwilson
Copy link

Sorry, just noticed #15114 and all its friends that discuss that very issue. I'm quite happy to see this Quick Fix solution after reading through many of the problems with function parameter inference!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committed The team has roadmapped this issue Domain: Quick Fixes Editor-provided fixes, often called code actions. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

9 participants