kos-language-server icon indicating copy to clipboard operation
kos-language-server copied to clipboard

Type hinting

Open krisavi opened this issue 6 years ago • 8 comments

  • What kind of change does this PR introduce? I made quite few changes like type hinting // #type: vesselTarget over the weekend. PR adds typehinting with syntax // #type: typestring and // #returns: typestring It also takes the parameters for functions to generate function signature. There is some residue code from testing out: Noticed that I had to do 2 passes over the whole code, because kos allows to have functions defined after calling them, but the code currently would give warning about signature not found. The code is not the prettiest and not the cleanest, but could be useful.

  • What is the current behavior? (You can also link to an open issue here) No possibility of code hinting

  • What is the new behavior (if this is a feature change)? Possibility of code hinting

  • Problems/Limitation: Currently the limitation is that 1 parameter per line, if there are more, then it still takes first typestring and applies it to first parameter in the line. There are some issues still. on function you can have // #type: typestring and // #returns: typestring. Both work the same. Needs way to differentiate between 2 of them. Type-hinting at the moment only lets you to enter only one type. Not possible to make it something like exist() command has (string or path) Probably should change the syntax to work more like // #parametername: typestring for parameters.

  • Other information: Some changes in suffixes and such that came out as problems when testing code. Probably another PR.

krisavi avatar Nov 18 '19 13:11 krisavi

I guess the automated test kind of got stuck.

krisavi avatar Nov 18 '19 14:11 krisavi

Seems I caused some memory leak issue in tests which I have to fix in PR.

krisavi avatar Nov 18 '19 14:11 krisavi

I was thinking of doing couple of tests yes, It needs some improvements, I found out some more issues with it. As a first step adding tests is way to go yes.

One more thing I remembered now that is missing is dynamic list generation, Parser needs improvements. I think I have defined 2-3 lists, Type hints are currently defined in one file and it is unknown what are the types they can use, Maybe autocomplete could be used to give some hints on what kind of type definitions there are.

Will look into writing some tests and improving code.

krisavi avatar Nov 19 '19 09:11 krisavi

It seems the parser config used in tests is not defining types. Have to figure out why it is ignoring all the hints.

krisavi avatar Nov 19 '19 12:11 krisavi

To your first comment, I think an autocomplete would definitely be a good thing to have. I think it would probably make sense to add it in a separate PR.

For type hinting depending on which spec file your in you may need to add typeInitializer() before the tests. This is located in "/src/typeChecker/initialize". Essentially this has to be run because some of the types have circular references to each other. This is especially true with the more primitive types like structure, boolean etc.

jonnyboyC avatar Nov 19 '19 15:11 jonnyboyC

image

Did some magic with automatic detection of return.

krisavi avatar Nov 19 '19 20:11 krisavi

Did struggle with testing. Not sure if i can make it work. During the test, it has not done the visitParameter it seems, so it has the type declared as structure and not picking up the updated one. That is the reason why it crashes for me. Function on the other hand had parameter types defined already, which is odd. So it needs some more figuring out how I can force the token types to be defined by the time the test takes place. It seems if I run it 2 times before test checks, everything is in order. Wanted to check the order it gets parameters-variables and such declared. Hoping I will find a way to make sure it is in order that will also not make it lag with bigger scripts.

For longer script I put my whole launch, orbit, rendezvous and deorbit and recovery with last part as suicide burn, while doing science collection and transmission as one big script. Basically took all the libraries and stitched it as one file to put language server to the test.

krisavi avatar Nov 20 '19 17:11 krisavi

Just as a point of reference I typically gauge speed against kerboscripts/parse_valid/ap.ks. it's about 2kloc. I believe it's the script for that sort of famous kOS quad copter.

Feel free to push code to your PR if you need another set of eyes on it. I can take a stab at writing some tests if you need some help. Definitely very interested in getting this merged!

jonnyboyC avatar Nov 21 '19 00:11 jonnyboyC