Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

Use inferred types for function parameters in typescript #3

Closed
automatensalat opened this issue Mar 31, 2020 · 12 comments · Fixed by #15
Closed

Use inferred types for function parameters in typescript #3

automatensalat opened this issue Mar 31, 2020 · 12 comments · Fixed by #15
Assignees
Labels
💪 Improvement Created code doesn't handle a specific case

Comments

@automatensalat
Copy link
Contributor

Hey @nicoespeon, thanks for the recommendation, this extension is what I was looking for.

Describe what's missing

When using Typescript and invoking the "extract function" feature I'd like the generated code to use the declared or inferred Typescript types for function parameters and return types if possible.

How would it work

The extension would use some internal system which provides the Typescript types and use those when generating declaring the function declaration.

Expected behavior

E.g. if I have this code

  const a = 3;
  const b = "hello";
  const c: MyType[] = [];
  const result: boolean = doSomething(a, b, c);

I'd like the generated code to be

 function doSomething(a: number, b: string, c: MyType[]): boolean {
   
 }

Additional information

  • Version of the extension used: v1.1.0
@automatensalat automatensalat added the 💪 Improvement Created code doesn't handle a specific case label Mar 31, 2020
@nicoespeon
Copy link
Owner

nicoespeon commented Apr 2, 2020

Agreed, that would be great! I also think @fabien0102 would love to work on that with me 😉

I didn't know how to get the types of Identifiers from the extension code. This might be helpful: https://dev.to/videlalvaro/inside-visual-studio-code-classes-and-interfaces-59hn (it is not)

We might want to dig into VS Code LSP: https://code.visualstudio.com/api/language-extensions/language-server-extension-guide

@nicoespeon
Copy link
Owner

@nicoespeon
Copy link
Owner

Good news, we've got something 😏

image

@nicoespeon
Copy link
Owner

Here we go, I've introduced a TypeChecker that can be used like this:

const typeChecker = new TypeChecker("const hello = [1];");

typeChecker.getTypeAt(new Position(0, 7));
// => number[]

Here it is: https://github.com/nicoespeon/hocus-pocus/blob/master/src/create-function-with-types/type-checker.ts

My plan is to introduce a new operation: CreateFunctionWithType
It will likely extend CreateFunction and provide the inferred types in the function parameters.

Then I'll expose CreateFunctionWithType on TS language and restrict CreateFunction to JS language.

This is necessary to avoid overloading the regular CreateFunction with types should only be created for TS language (that would mess up the signature and add undesirable conditionals).

@automatensalat
Copy link
Contributor Author

Very nice!

@LeandroDG
Copy link

Thank you thank you thank you 👍 This is so useful :-)

@nicoespeon
Copy link
Owner

@automatensalat we've something working!

Check it out:

2020-05-21 Screenshot-CDbyw5rq

I'll release it in a new version today, so you can start using it and let me know if anything is off.

@automatensalat
Copy link
Contributor Author

Fantastic, I'll check it out! Thank you very much.

@automatensalat
Copy link
Contributor Author

I'm having some issues:

If I try to run it on the example I get this error:

Something went wrong: Error: Cannot find module 'typescript'
Require stack:

  • /home/tobias/.vscode/extensions/nicoespeon.hocus-pocus-1.4.0/out/extension.js
  • /usr/share/code/resources/app/out/vs/loader.js
  • /usr/share/code/resources/app/out/bootstrap-amd.js
  • /usr/share/code/resources/app/out/bootstrap-fork.js

If I run it on this:

function testCreateFuncWithTypes() {
	const a = 3;
	doSomething(a);
}

it creates

function doSomething(a: 3) {
  // Implement
}

Instead of the type number. When trying it with an object the extension generates any for the argument type.
What could be the problem here? Does the extension expect Typescript to be installed globally? We have it as a project-local dependency.

@nicoespeon
Copy link
Owner

@automatensalat the 3 instead of number is actually normal. You have a const that is assigned to 3, so it can't be re-assigned and TS will infer this is always the constant 3. If you use let instead it will correctly infer the type.

Now, that's the TS theory. I can definitely see that in real-life, we set const a = 3 as a test value, but that the created function should be more laxist in the type it expects. I think it's fair to assume 90% of the case will be the literal (string, number, boolean…).

I suggest we improve the type-checker to do that on top of TS response.

As for the object, I'm not sure. It might be the same issue. 🤔

Can you create another issue with a few real-life examples to handle? That would give us a baseline of tests to make pass 💪

@automatensalat
Copy link
Contributor Author

I can get behind the narrowed type argument, however I think the general types would be more useful in practice.
I created a couple of issues that I found just now. If I find the time I might try to fix some of those myself in the next days.
Thanks again for implementing this though!

@nicoespeon
Copy link
Owner

Thanks for the issues, that's perfect.

And yeah, I think we can improve the Type Checker to make usage the most convenient. General types will probably be more useful.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
💪 Improvement Created code doesn't handle a specific case
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants