genius257 / vscode-autoit

AutoIt language extension for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=genius257.autoit
MIT License
4 stars 3 forks source link

Go To Definition functionality #24

Closed Danp2 closed 1 year ago

Danp2 commented 1 year ago

Running some basic tests, I've encountered one small issue with the GTD functionality. Here's a code sample --

#include "..\webdriver\wd_core.au3"
#include "..\webdriver\wd_capabilities.au3"

Func Main()
    $_WD_Debug = $_WD_DEBUG_Full

    _WD_CapabilitiesDefine($_WD_KEYS__STANDARD_PRIMITIVE, 'webSocketUrl')
    _WD_CapabilitiesDefine($_WD_KEYS__STANDARD_PRIMITIVE, 'moz:debuggerAddress')
EndFunc

With the above, everything is good except that I am unable to find a reference for $_WD_Debug, which is a global variable declared in wd_core.au3. Taking a deeper dive, it seems that most of the global variables from include files are not being recognized.

genius257 commented 1 year ago

So this one is a bug, but it is tricky.

When you want to find the declaration for $_WD_Debug, the line itself within the function main IS a variable declaration. It is however not the FIRST... If you used goto declaration on $_WD_Debug used in a expression in main, but after your declaration, would you expect the declaration in main or the very first declaration to be found, and which of the two options is most helpful in general development context?

There is also the question of where global defined variables within functions or conditionals fall into this declaration hierarchy.

I would argue my solution makes sense in a development environment, but when testing behavior in PHP (A language where no keyword is needed to declare a variable) with the DEVSENSE.phptools-vscode extension, it jumps to the first declaration occurrence outside of an function, so this seems to be the solution to implement? :)

Sven-Seyfert commented 1 year ago

Hi @genius257,

Sven (SOLVE-SMART) here. In my opinion

it jumps to the first declaration occurrence outside of an function, so this seems to be the solution to implement? :)

this would be the way to go and how I would expect it. Like you already mentioned, other extensions do exactly this. Let's see what Dan is thinking of 🤝 .

Best regards Sven

Danp2 commented 1 year ago

I believe the default should be to locate the first declaration occurrence outside of a function. Maybe you could offer a way to toggle between these two "styles" of GTD.

Danp2 commented 1 year ago

What should happen in a scenario like the following?

Global $x

$x = 'test1'
myFunc1('test2')
MyFunc2()

Func MyFunc1($x)
    ConsoleWrite($x & @CRLF)
EndFunc

Func MyFunc2()
    Local $x

    $x = 'test3'
EndFunc

Both functions create a local instance of $x, but GTD always refers to the initial Global.

Sven-Seyfert commented 1 year ago

I personally would expect, the search for the Global occurrence and if there is non then search for a Local one. But nevermind, it's on @genius257 to decide 🤭 .

genius257 commented 1 year ago

So I'm thinking using the actual AutoIt3 runtime behavior for varaibles in scopes (functions) and how most if not all other language intellisense does it for global.

The following list behavior will stop and return if match is found.

Within a function:

  1. check function parameters for a match
  2. check lines before the current origin of the declaration request, within the function
  3. default to the Global lookup behavior

Outside of a function:

  1. Check lines before the current origin of the declaration request. 1.1. Check for declaration statement 1.2. Check for include statement and include recursively on the included file, if resolved to an existing file when checking.
  2. check for declaration statements with global scope specified within functions.

In step 1 of Outside of a function case, function declarations will be added to an array, making it correctly sorted for step 2

If no scope is defined for variable declarations within functions, Local scope is assumed. Always assuming Local scope, is specific for declaration lookup, and other providers, like future check all references would update this scope to Global if a declaration of said variable with Global was found anywhere in the code.

Let me know if it makes sense :)


I just realized that i currently aren't processing Assign, Eval, Execute and IsDeclared.

I plan on making issues for each. They will be low priority for now, just a heads up...

genius257 commented 1 year ago

Should now be fixed with version 1.2.4 :smile:

Danp2 commented 1 year ago

@genius257 Unfortunately, the latest update appears to have broken the GTD functionality for functions. ☹️

genius257 commented 1 year ago

I'm sorry to hear :disappointed: I will look into it and fix it! :smile: Hopefully without breaking anything else :sweat_smile: Jest tests are not implemented yet, so my competence as tester before release is currently the only safeguard, besides TypeScript type checking :stuck_out_tongue_winking_eye:

Danp2 commented 1 year ago

Fixed by 254eb03b1603b8d88d3d7253aed8fc23d94c4d52