Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.x] Several minor LSP issues #53293

Closed
Rubonnek opened this issue Oct 1, 2021 · 5 comments
Closed

[3.x] Several minor LSP issues #53293

Rubonnek opened this issue Oct 1, 2021 · 5 comments

Comments

@Rubonnek
Copy link
Member

Rubonnek commented Oct 1, 2021

Godot version

3.x branch at 8d0b2ed

System information

Arch Linux

Issue description

This ticket is following up on #53238 (comment). On that issue, #53238, it was determined that one of the underlying causes that made the engine to crash was a buggy LSP client.

For a while now I've been stumbling upon strange errors that seem to be caused by a buggy LSP client, in this case Vim-ALE, and I'm bringing this up in case there are relevant changes that should be made to Godot.

Most of my tests are on 3.x. I am unsure if the issues below affect the master branch.

The errors described below are also not shown in the editor, but in an external terminal emulator that is attached to Godot.

Steps to reproduce

All the cases below assume the user has opened the engine using godot -e at the path where project.godot is at.

Case 1

Hovering over the extends in a GDScript file generates the following LSP event:

{"method": "textDocument/hover", "jsonrpc": "2.0", "id": 2, "params": {"textDocument": {"uri": "file:///home/wilson/game_development/godot/godot-template/src/addons/tiled_importer/custom_property_manager_for_tilesets_template.gd"}, "position": {"character": 0, "line": 1}}}

which generates the following Error:

ERROR: Method failed.
   at: _determine_inheritance (modules/gdscript/gdscript_parser.cpp:5423)

Case 2

Hovering over an empty line generates the following LSP event:

{"method": "textDocument/hover", "jsonrpc": "2.0", "id": 2, "params": {"textDocument": {"uri": "file:///home/wilson/game_development/godot/godot-template/src/addons/tiled_importer/custom_property_manager_for_tilesets_template.gd"}, "position": {"character": 0, "line": 1}}}

Which generates the following error:

ERROR: Index p_position.character = 0 is out of bounds (line.size() = 0).
   at: get_identifier_under_position (modules/gdscript/language_server/gdscript_extend_parser.cpp:490)
ERROR: Index p_position.character = 0 is out of bounds (line.size() = 0).
   at: get_identifier_under_position (modules/gdscript/language_server/gdscript_extend_parser.cpp:490)

Case 3

Opening a GDScript file outside of the Godot project generates a ton of events, but the following seems to be the important one:

{"method": "textDocument/didOpen", "jsonrpc": "2.0", "params": {"textDocument": {"uri": "file:///tmp/outside_of_any_project/Node2D.gd", "version": 1, "languageId": "gdscript3", "text": "extends Node2D\n"}}}

And right after you'll see the following error:

ERROR: Condition "err" is true. Returned: err
   at: load_source_code (modules/gdscript/gdscript.cpp:775)
ERROR: Cannot load source code from file 'file:/tmp/outside_of_any_project/Node2D.gd'.
   at: load (modules/gdscript/gdscript.cpp:2177)
ERROR: Failed loading resource: file:/tmp/outside_of_any_project/Node2D.gd. Make sure resources have been imported by opening the project in the editor at least once.

Minimal reproduction project

A GDScript template will do:

extends Node2D

func _ready() -> void:
	pass
@Rubonnek
Copy link
Member Author

Rubonnek commented Oct 1, 2021

CC @Razoric480

@Rubonnek Rubonnek changed the title [3.x] Several minor LSP issues due to a _possibly_ buggy client (Vim ALE) [3.x] Several minor LSP issues Oct 1, 2021
@Rubonnek
Copy link
Member Author

Rubonnek commented Oct 1, 2021

I've tested the steps above with VSCode in Linux and it doesn't seem like these minor issues are caused by a buggy LSP client this time.

@Razoric480
Copy link
Contributor

Razoric480 commented Oct 1, 2021

I'm investigating these now. Just putting my notes down here as a way to show progress and just to have someplace to put them.

Number 1 appears to be caused by the LSP inserting 0xFFFF to represent the location of the cursor. Usually, this gets caught by the tokenizer and just skipped over. In a function call this seems to help differentiate the identifier from the parenthesis. (func _ready0xFFFF())

When the statement is after extends (extends0xFFFF Node2D), the parser breaks when parsing the extends line, skipping over the following identifier entirely, which leading to a failure of the parse extends, which sets _determine_inheritance() in error. Trying to narrow down why that's happening.

EDIT: Fixed by adding a check for TK_CURSOR in tokenizer for parsing the extends.

@Razoric480
Copy link
Contributor

Two PRs have been submitted to fix the issue - 2 and 3 exist on 4.0 and so should be cherry-picked in #53308, and 1 only exists in 3.x and is fixed by #53309.

@akien-mga akien-mga added this to the 3.4 milestone Oct 1, 2021
@akien-mga
Copy link
Member

Fixed by #53308 and #53309.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants