godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.45k stars 21.27k forks source link

Language Server: Symbol resolution does not take context into account #83796

Open ryanabx opened 1 year ago

ryanabx commented 1 year ago

Godot version

v4.2.beta2.official [f8818f85e]

System information

Godot v4.2.beta (8c25a98fd) - Windows 10.0.22621 - Vulkan (Mobile) - integrated AMD Radeon(TM) Graphics (Advanced Micro Devices, Inc.; 31.0.22011.4008) - AMD Ryzen 9 6900HS with Radeon Graphics (16 Threads)

Issue description

When the language server resolves a symbol, it does not take into account what object the symbol is a part of, or even whether the symbol is inside a string or not. An example of this issue can be found in the images below:

These are the symbols that the lsp found that match the "symbol" I clicked, which is the variable declaration var pressed: float = 2. The only two symbols that should be highlighted are the reference to pressed inside double_tap_input, and the variable declaration for pressed. Instead, event.pressed is highlighted, as well as an erroneous symbol altogether (pressed inside a string) image

When using "Find All References" on this symbol, this is what is retrieved from the lsp image

This affects Find All References, and Rename functionalities. The problem likely stems from GDScriptWorkspace::resolve_symbol or one of the functions called within it.

Steps to reproduce

Using Visual Studio Code with the godot-tools extension, we can see this problem easily:

  1. Click over an identifier that may have a duplicate name as an identifier in another class, or even a word in a string.
  2. The highlighted variables are a result of the lsp finding all symbols that match the clicked symbol.
  3. You can also use "Find All References" to see this information in the sidebar

Another use of "Find All References" on x in this MRP: image

Minimal reproduction project

MRP_LSPBUG.zip

atirut-w commented 1 year ago

These are the symbols that the lsp found that match the "symbol" I clicked

  • Click over an identifier that may have a duplicate name as an identifier in another class, or even a word in a string.
  • The highlighted variables are a result of the lsp finding all symbols that match the clicked symbol.

That's not how it works. Putting a cursor on a word in VSCode highlights all instances of that word. It does this even without the LSP being connected to. See:

image

  1. You can also use "Find All References" to see this information in the sidebar

This is indeed caused by the LSP.

ryanabx commented 1 year ago

These are the symbols that the lsp found that match the "symbol" I clicked

  • Click over an identifier that may have a duplicate name as an identifier in another class, or even a word in a string.

  • The highlighted variables are a result of the lsp finding all symbols that match the clicked symbol.

That's not how it works. Putting a cursor on a word in VSCode highlights all instances of that word. It does this even without the LSP being connected to. See:

image

  1. You can also use "Find All References" to see this information in the sidebar

This is indeed caused by the LSP.

Whenever a word is clicked in a document that is managed by a language server, it calls to the language server to resolve the symbol. I don't have my computer open right now but I double checked this before posting with one of Godot's source code files in C++, comparing a word within a string with a variable of the same word. It didn't highlight the variable when I clicked the word in the string, because that word shouldn't be a symbol.

I also was trying to debug the issue before posting it by using print_line statements in the symbol resolution code for the lsp. A print statement was fired every time I'd click a new word.

atirut-w commented 1 year ago

Try testing it with Godot closed, that should give us a conclusive answer.

ryanabx commented 1 year ago

Try testing it with Godot closed, that should give us a conclusive answer.

See this issue for the C++ extension https://github.com/microsoft/vscode/issues/100530

The user notes separate behavior from when the language server is enabled and not. Read his notes on the expected behavior. This is how the language server is supposed to work.

HolonProduction commented 2 months ago

If someone is able to reproduce this, please try turning off the Godot LSP smart resolve feature in the Godot Editor settings and see whether it changes something.