fstirlitz / luaparse

A Lua parser written in JavaScript
https://fstirlitz.github.io/luaparse/
MIT License
459 stars 91 forks source link

Add originating function info to onLocalDeclaration #49

Open pablomayobre opened 7 years ago

pablomayobre commented 7 years ago

This commit is related to the pull request #38 which added onLocalDeclaration, both target the specific use case of dapetcu21/atom-autocomplete-lua.

This pull request adds information about the function where the local value is being used, this allows Autocomplete-Lua to check whether the value's type is the expected for said function.

This is of course only necessary for functions so other elements don't pass the optional data object.

This commit was actually made by @dapetcu21 at dapetcu21/luaparse@5a1fd2868adecb79f6bc57ee1f1880a3ea83ad8b I have only rebased it

dapetcu21 commented 7 years ago

I was meaning to do this PR myself when I got time, but I didn't do it yet because I wasn't fully happy with the semantics. For example, I'm not completely happy about the fact that the data object is not passed for non-functions. Any opinions?

(Also, there should be tests/docs for this)

pablomayobre commented 7 years ago

I understand what you mean, I just wanted to start discussion on it!

the data object gives the context of where the variable will be declared, which is useful when considering functions since you are getting the arguments.

The other cases where this function is being called:

So I don't see any meaning in passing the data argument in these other cases.

Maybe we could use what kind of identifier we are talking about like ForVariable, LocalVariable, Label and FunctionArgument. Is there any other information which could be useful to have in this callback?

EDIT: Another thing that could be useful is the location (line, column) of the identifier for that declaration, which is currently not found anywhere

fstirlitz commented 7 years ago

Given that identifiers are always added to the current scope, how about passing metadata identifying the scope to the onCreateScope callback instead? Whoever provides the callbacks can pluck out the information they want and maintain their own state. onLocalDeclaration can then receive only the kind of declaration encountered (local statement, for loop, formal parameter) and its location in the source code.

Labels are already distinguished from variables, by the way; their names are passed with the surrounding :: marks. Admittedly not the most efficient design, but...

dapetcu21 commented 7 years ago

@fstirlitz That could work. If onCreateScope() identifies the new scope as a function scope and then onLocalDeclaration() subsequently calls back saying "this is a function argument", then that's enough information.