Add support for deprecated symbols to language server and script editor #100019

Meorge · 2024-12-04T18:24:02Z

Related to godotengine/godot-proposals#11079 !

This PR adds the ability for code completions from the language server to be marked as deprecated, according to the class documentation, so that they can be displayed as such in a code editor. It also implements this display for the built-in script editor:

In Godot's built-in script editor:

In VS Code:

Note

While the functionality is mostly working, the code itself is still in a messy WIP state. I'm definitely planning on improving its structure (and squashing commits, etc) before opening the full pull request. That being said, if you see patterns or things in the code that could be improved, please let me know!

Tested cases

Methods

A user-defined method marked with `@deprecated`

## This method is deprecated.
## @deprecated
func do_something():
    print("Hello world")

func _ready():
	# do_something will show as deprecated
	do_som...

A class built-in to Godot and marked as deprecated in the documentation

func _ready():
	var player = AnimationPlayer.new()
	# get_method_call_mode will show as deprecated
	player.get_method_c...

Classes

Main class of a file marked with `@deprecated`

class_name MyOldClass
extends Node

## This class is deprecated.
## @deprecated

# main.gd
extends Node

func _ready():
	# MyOldClass will show as deprecated
    var i = MyOl...

Inner class marked with `@deprecated`

## This inner class is deprecated.
## @deprecated
class InnerDeprecatedClass:
	pass

func _ready():
	# InnerDeprecatedClass will show as deprecated
	var i = InnerDep...

A class built-in to Godot and marked as deprecated in the documentation

func _ready():
	# AnimatedTexture will show as deprecated
	var i = AnimatedTex...

A class built-in to Godot and marked as deprecated in the documentation, used as a type

func _ready():
	# AnimatedTexture will show as deprecated
	var i: AnimatedTex...

An autoload with a global variable name

# deprecated_autoload.gd
extends Node

## This autoload is registered with the name DeprecatedAutoload,
## and is marked as deprecated.
## @deprecated

func do_thing():
   print("a thing has been done")

# main.gd
func _ready():
    # DeprecatedAutoload will show as deprecated
    DeprecatedAu...

[!WARNING]
I've found this doesn't always work. It seems to involve something with the .godot folder preventing the doc data from being fully parsed, so I think it's outside the scope of this PR to try to fix.

Enums and constants

A user-defined constant, marked with `@deprecated`

## This is a deprecated constant.
## @deprecated
const DEPRECATED_CONST = "hello"

func _ready():
	# DEPRECATED_CONST will show as deprecated
	print(DEPRECATED_CON...

A user-defined named enum, marked with `@deprecated`

## This is a deprecated enum.
## @deprecated
enum DeprecatedEnum { ONE, TWO, THREE }

func _ready():
	# DeprecatedEnum will show as deprecated
	print(Depre...

Individual values in a user-defined anonymous enum, marked with `@deprecated`

enum {
    ANONYMOUS_ONE,
    ## This one is deprecated
    ## @deprecated
    ANONYMOUS_TWO
}

func _ready():
	# ANONYMOUS_TWO will show as deprecated
	print(ANONYMOUS_T...

Individual values in a user-defined named enum, marked with `@deprecated`

enum NamedEnum {
	ONE,
	## This is a deprecated constant in an enum.
	## @deprecated
	TWO
}

func _ready():
	# NamedEnum.TWO will show as deprecated
	print(NamedEnum.TW...

Individual values in an enum from a class built-in to Godot and marked as deprecated in the documentation

func _ready():
	# ANIMATION_PROCESS_PHYSICS will show as deprecated
	var i = AnimationPlayer.ANIMATION_PROCESS_PH...

	# Will also show as deprecated
	i = AnimationPlayer.AnimationProcessCallback.ANIMATION_PROCESS_PH...

Constants in `@GlobalScope`

func _ready():
	# PROPERTY_HINT_NODE_PATH_TO_EDITED_NODE will show as deprecated
	print(PROPERTY_HINT_NODE_PATH_T...

Properties

A user-defined property marked with `@deprecated`

## This is a deprecated property.
## @deprecated
var deprecated_property: float = 1.0

## This is a deprecated property with a getter and setter.
## @deprecated
var deprecated_property_2: float = 2.0:
    get:
        return sqrt(5)
    set(value):
        deprecated_property_2 = value

func _ready():
	# deprecated_property and deprecated_property_2 will show as deprecated
	print(deprecated_prop...

A class built-in to Godot and marked as deprecated in the documentation

func _ready():
	# auto_translate will show as deprecated
    var i = Control.new().auto_tr...

Signals

A user-defined signal marked with `@deprecated`

## This is a deprecated signal.
## @deprecated
signal deprecated_signal

func _ready():
	# deprecated_signal will show as deprecated
	depreca...

A class built-in to Godot and marked as deprecated in the documentation

func _ready():
	# setup_local_to_scene_requested will show as deprecated
	Resource.new().setup_local_t...

To-Do

Language server

Fix behavior with autoloads (determine how to retrieve their class's DocData)
General clean-up of code structure - it's messy right now! (Maybe add some static methods for getting is-deprecated hashmaps?)
- I've tried to standardize the variable names used so I think it's better now, but would still appreciate suggestions if you have them!

Godot Script editor

Ensure the strikethrough scales correctly as the editor scale is changed

Meorge · 2024-12-07T04:29:01Z

It seems like just about everything is working, except for global variable names for autoloads. As stated above:

Warning

Autoload global variable names will not display as deprecated.

# deprecated_autoload.gd
extends Node

## This autoload is registered with the name DeprecatedAutoload,
## and is marked as deprecated.
## @deprecated

func do_thing():
   print("a thing has been done")

# main.gd
func _ready():
    # DeprecatedAutoload will *not* show as deprecated
    DeprecatedAu...

I've spent a fair amount of time this evening trying to figure out how to get to the DocData::ClassDoc for an autoload, and based on the existing code it looks like I'd need some kind of function that maps script paths to ClassNodes (or DocData::ClassDocs).

What are others' thoughts about not displaying global variable names for autoloads as deprecated even if they're marked as deprecated in their GDScript file's documentation comments?

HolonProduction · 2024-12-07T11:09:42Z

From what I see autoload doc data is also registered in EditorHelp::get_doc_data().class_list under the name of the autoload, but the update behavior seems very flaky so I'm not sure whether we can rely on it.

Besides that the only way I know of would be getting the path of the autoload, loading the script and getting the info from the script.

Would need some testing regarding performance impact though, since in the worst case autocompletion gets called with every keystroke so we need to make sure that the resource loader caches the script and doc.

In general before merging we need to test this PR for performance. We already have the problem, that autocompletion can trigger parsing and analyzing of a lot of scripts with each keystroke. This PR shouldn't add more scripts to that list.

Meorge · 2024-12-07T20:11:54Z

From what I see autoload doc data is also registered in EditorHelp::get_doc_data().class_list under the name of the autoload, but the update behavior seems very flaky so I'm not sure whether we can rely on it.

I'm finding this to be shaky as well. I have a test project that I'm using, and can do the following:

I delete the project's .godot folder, then open the project. If I go straight to "Search Help" and start typing the name of the global variable the autoload is registered under, it appears in the list.
I make a change to a script file in the project and save it, then close the project.
I re-open the project and go to "Search Help" again, and start typing the name of the global variable the autoload is registered under. It doesn't appear anymore.

Indeed, using the following snippet:

for (const KeyValue<String, DocData::ClassDoc> &E2 : EditorHelp::get_doc_data()->class_list) {
    print_line(vformat("Class %s found", E2.key));
}

I do get the following output:

...
Class PackedColorArray found
Class PackedVector4Array found
Class @GlobalScope found
Class @GDScript found
Class DeprecatedAutoload found
Class DeprecatedClass found
Class MyScene.InnerDeprecatedClass found
Class MyScene found

(where DeprecatedAutoload is the name of the autoload's global variable, not its class_name).

And indeed, if I delete .godot then open the project, I can get the DeprecatedAutoload to display as deprecated:

So it seems to me like something being cached in .godot is causing the doc generation to halt prematurely, such that autoloads aren't being added. Last night as I was working on this, I noticed my other custom classes also weren't printing out. I'll look into getting an issue filed for this, if one doesn't already exist!

Regarding performance, I agree that's definitely a big concern, and one I've been conscious of as I've been working through putting this feature together. Across the file, there are a few ways I've gotten deprecated data from the docs:

When I have the Node for a symbol, I can grab the data from it directly via something like member.m_class->doc_data.is_deprecated. As far as I can tell, this would have very little overhead.

For checking if a class is deprecated, I have to get EditorHelp::get_doc_data()->class_list and access it by key. This seems like it might have a bit more overhead.

So far, I'm most nervous about when I want to check whether signals/properties/methods on some class are deprecated. My process so far has been to:

Get the ClassDoc for the given class.
Create an empty HashMap<StringName, bool> to store names and whether or not they are deprecated.
Iterate over the relevant vector within the ClassDoc, and for each item, add an entry to the HashMap where the key is the item's name and the value is whether or not the item is deprecated.
When creating the vector of CodeCompletionOptions, check if each one exists in the HashMap, and if so, set its deprecated value to the corresponding value in the HashMap.

In practice, that looks like this:

// Up at the top of the method, so it can be reused:
const HashMap<String, DocData::ClassDoc> class_doc_map = EditorHelp::get_doc_data()->class_list;

// ...

// Here we're going to create the list of constants to provide as autocomplete suggestions.
HashMap<StringName, Variant> constants;
scr->get_constants(&constants);

// Create a HashMap mapping constants' names to their deprecated status.
HashMap<StringName, bool> const_is_deprecated_map;
for (const DocData::ConstantDoc &constant_doc : class_doc_map.get(base_type.class_type->get_global_name()).constants) {
    const_is_deprecated_map.insert(constant_doc.name, constant_doc.is_deprecated);
}

for (const KeyValue<StringName, Variant> &E : constants) {
    int location = p_recursion_depth + _get_constant_location(scr, E.key);
    ScriptLanguage::CodeCompletionOption option(E.key.operator String(), ScriptLanguage::CODE_COMPLETION_KIND_CONSTANT, location);

    // Add the constant's deprecated status if we have it.
    if (const_is_deprecated_map.has(E.key.operator String())) {
        option.deprecated = const_is_deprecated_map.get(E.key.operator String());
    }
    r_result.insert(option.display, option);
}

Without changing how the doc data is structured, I'm not sure how this could be made much more efficient. These hashmaps could perhaps be computed earlier on, alongside the rest of the doc data, and then cached? Although then that introduces multiple sources of truth for deprecated statuses, which isn't ideal.

HolonProduction · 2024-12-12T10:13:48Z

For checking if a class is deprecated, I have to get EditorHelp::get_doc_data()->class_list and access it by key. This seems like it might have a bit more overhead.

I wouldn't worry about hashmap lookups and such, that should be fine. Just triggering parsing and analyzing for a script can get very expensive.

modules/gdscript/gdscript_editor.cpp

HolonProduction · 2024-12-12T10:47:08Z

modules/gdscript/gdscript_editor.cpp

 							r_result.insert(option.display, option);
 						}
 					} break;
 					case GDScriptParser::ClassNode::Member::CONSTANT: {
 						if (member.constant->get_datatype().is_meta_type && p_context.current_class->outer != nullptr) {
+							// TODO: Describe this case and its examples.


Suggested change

// TODO: Describe this case and its examples.

// Preloaded type within this script.

// const MyType = preload("res://my_type.gd)

// var m: MyT... # <-- will suggest MyType

I'm having trouble verifying this. When I try assembling this in my project, I don't see the completion option show up at all - so that may be a bug with the script parser overall 😅

Yeah forgot, it's broken on master. #94996 would fix it but I can't get any reviews it seems :(

~~Funnily enough the PR fixes this in a different code path. Might need to investigate this one again.~~ My bad I fixed a similar case in another place but this one as well. Just from looking at the code it should work in inner classes only at the moment.

modules/gdscript/gdscript_editor.cpp

modules/gdscript/language_server/godot_lsp.h

Meorge

Some of the behaviors I tried to verify with tests but wasn't able to. Specifically, it seemed like _find_enumeration_candidates was never being triggered. They may well be bugs we should file separate issues for.

modules/gdscript/gdscript_editor.cpp

Meorge · 2024-12-12T16:59:56Z

modules/gdscript/gdscript_editor.cpp

 							r_result.insert(option.display, option);
 						}
 					} break;
 					case GDScriptParser::ClassNode::Member::CONSTANT: {
 						if (member.constant->get_datatype().is_meta_type && p_context.current_class->outer != nullptr) {
+							// TODO: Describe this case and its examples.


I'm having trouble verifying this. When I try assembling this in my project, I don't see the completion option show up at all - so that may be a bug with the script parser overall 😅

modules/gdscript/gdscript_editor.cpp

Meorge · 2024-12-20T04:27:42Z

There was an issue where I was attempting to access editor theming, and causing some of the CI to fail, but I think I found a fix for that. If all the checks don't pass, I'll try to continue working through them on another branch so that they put less strain on the main repo's CI resources 😅

dalexeev · 2024-12-20T17:16:35Z

Just a note that after #91060 local variables and constants can also be deprecated/experimental. Not sure how useful this is in practice and whether LSP supports deprecating local identifiers, but it would be nice to add it.

Meorge · 2024-12-20T19:06:41Z

For me, practically speaking, it doesn't make sense for a local variable to be deprecated - if so then I feel like it's time for a refactor of your code 😅 But, if documentation comments are parsed for local identifiers and that includes deprecated status, I also agree that for consistency's sake it should be added.

And added it is! I may not push it to the branch that this PR is tracking immediately, as I'd like to get the CI issues resolved first.

dalexeev · 2024-12-21T13:22:08Z

modules/gdscript/gdscript_editor.cpp

@@ -1209,6 +1294,16 @@ static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base
 						if (!base_type.is_meta_type) {
 							List<PropertyInfo> members;
 							scr->get_script_property_list(&members);
+
+							HashSet<StringName> deprecated_properties;
+							if (base_type.class_type && class_doc_map.has(base_type.class_type->get_global_name())) {


I think you shouldn't use get_global_name() to determine the doc class name. Because for unnamed classes, inner classes and autoload singletons the name may differ from the global name.

After #91060 there is a new method Script::get_doc_class_name(), but there is no counterpart for GDScriptParser::ClassNode. I reused GDScriptDocGen::doctype_from_gdtype() (which uses GDScriptDocGen::_get_class_name()).

Thanks! I'll look more into this once I have the compatibility method stuff working and CI tests passing.

@GDscript

CodeCompletionOptions need to be given the correct deprecated status still Variables marked as deprecated are recognized Attempt to get deprecated info about GDScript functions Consult DocData for more things being marked deprecated @GDscript methods and other things work now! Support for methods on built-in classes Add support for enums, and clean up some of the code to not use iterators when not necessary Added more deprecated checks Attempt to understand named enum property documentation Display deprecated functions with strikethrough in built-in script editor Named enum members now working! Recognize deprecated built-in classes when used as types Add deprecated checks for some spots, while trying to find autoload spot Added better TODO note Autoloads do technically kinda work, but only if you clear .godot

…ld debugging code [no ci]

…ive enum type

…attempting to use it

…r `add_code_completion_option`

Meorge · 2024-12-26T18:35:34Z

I'm currently working on getting the CI checks to pass. By adding the deprecated argument to CodeEdit::add_code_completion_option, I broke compatibility for that method and am now trying to get a compatibility method for it to work correctly. To avoid using Godot's CI resources but still run the checks, I'm continuing it on the branch at https://github.com/Meorge/godot/tree/language-server-tomfoolery-dev. It seems to be failing with the "Hash changed" error.

In code_edit.compat.inc:

void CodeEdit::_add_code_completion_option_compat_100019(CodeCompletionKind p_type, const String &p_display_text, const String &p_insert_text, const Color &p_text_color, const Ref<Resource> &p_icon, const Variant &p_value, int p_location) {
	add_code_completion_option(p_type, p_display_text, p_insert_text, p_text_color, p_icon, p_value, p_location, false);
}

void CodeEdit::_bind_compatibility_methods() {
	// ...
	ClassDB::bind_compatibility_method(D_METHOD("add_code_completion_option", "type", "display_text", "insert_text", "text_color", "icon", "value", "location"), &CodeEdit::_add_code_completion_option_compat_100019, DEFVAL(Color(1, 1, 1)), DEFVAL(Ref<Resource>()), DEFVAL(Variant::NIL), DEFVAL(LOCATION_OTHER), DEFVAL(false));
}

In code_edit.h:

#ifndef DISABLE_DEPRECATED
	// ...
	void _add_code_completion_option_compat_100019(CodeCompletionKind p_type, const String &p_display_text, const String &p_insert_text, const Color &p_text_color = Color(1, 1, 1), const Ref<Resource> &p_icon = Ref<Resource>(), const Variant &p_value = Variant::NIL, int p_location = LOCATION_OTHER);
	static void _bind_compatibility_methods();
#endif

And, in 4.3-stable.expected:

GH-100019
--------
Validate extension JSON: Error: Field 'classes/CodeEdit/methods/add_code_completion_option/arguments': size changed value in new API, from 7 to 8.

New argument `deprecated` added to `add_code_completion_option`. Compatibility method registered.

However I'm still getting the error:

Compatibility to 4.3-stable is broken in the following ways:
Validate extension JSON: Error: Hash changed for 'classes/CodeEdit/methods/add_code_completion_option', from EB1A746E to 611C3D20. This means that the function has changed and no compatibility function was provided.

I feel like I've followed all of the steps at https://docs.godotengine.org/en/stable/contributing/development/handling_compatibility_breakages.html, so I'm feeling a bit out of ideas at the moment... 😅

Continue to try to get compatibility method to work Avoid making changes to original compatibility method Maybe it will work this time?? Please Add another API compatibility check thing

AThousandShips added enhancement topic:editor labels Dec 4, 2024

AThousandShips added this to the 4.x milestone Dec 4, 2024

dalexeev added the topic:gdscript label Dec 4, 2024

Meorge force-pushed the language-server-tomfoolery branch from fcace42 to 120e7b4 Compare December 11, 2024 23:27

HolonProduction reviewed Dec 12, 2024

View reviewed changes

Meorge commented Dec 12, 2024

View reviewed changes

AThousandShips reviewed Dec 14, 2024

View reviewed changes

Meorge force-pushed the language-server-tomfoolery branch from b368813 to 6d9a9fe Compare December 20, 2024 03:00

Meorge marked this pull request as ready for review December 20, 2024 03:01

Meorge requested review from a team as code owners December 20, 2024 03:01

Meorge force-pushed the language-server-tomfoolery branch from 6d9a9fe to 8310299 Compare December 20, 2024 04:26

Meorge requested a review from a team as a code owner December 20, 2024 20:12

dalexeev reviewed Dec 21, 2024

View reviewed changes

Meorge added 6 commits December 25, 2024 21:11

Add documentation for most code completion options, and remove some o…

95d238c

…ld debugging code [no ci]

Improve comments

59e93aa

Improve documentation, remove some debugging statements

59c5b17

Revert use of tags in language server

6837404

Provide example in comments for native enum value in context with nat…

ae36ea9

…ive enum type

Meorge added 9 commits December 25, 2024 21:11

Replace HashMaps with HashSets for better performance

e1de0ef

Make naming of class_doc_map consistent

362a983

Scale strikethrough to match editor scale

21efd26

Add support for local identifiers

b7276c1

Check for validity of EditorHelp::get_doc_data return value before …

8fbff3c

…attempting to use it

Update CodeEdit and its tests

03e28b9

Update glue

04f95b9

Fix glue more (compiles on my machine)

7946365

Update documentation for CodeEdit to include deprecated argument fo…

fcf9577

…r `add_code_completion_option`

Add compatibility method to pass CI tests

d9e37ed

Continue to try to get compatibility method to work Avoid making changes to original compatibility method Maybe it will work this time?? Please Add another API compatibility check thing

Meorge force-pushed the language-server-tomfoolery branch from f0f34f4 to d9e37ed Compare December 28, 2024 01:33

Meorge requested review from a team as code owners December 28, 2024 01:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for deprecated symbols to language server and script editor #100019

Add support for deprecated symbols to language server and script editor #100019

Meorge commented Dec 4, 2024 •

edited

Loading

Meorge commented Dec 7, 2024

HolonProduction commented Dec 7, 2024

Meorge commented Dec 7, 2024

HolonProduction commented Dec 12, 2024

HolonProduction Dec 12, 2024

Meorge Dec 12, 2024

HolonProduction Dec 13, 2024 •

edited

Loading

Meorge left a comment

Meorge Dec 12, 2024

Meorge commented Dec 20, 2024

dalexeev commented Dec 20, 2024

Meorge commented Dec 20, 2024

dalexeev Dec 21, 2024

Meorge Dec 26, 2024

Meorge commented Dec 26, 2024

-							// TODO: Describe this case and its examples.
+							// Preloaded type within this script.
+							// const MyType = preload("res://my_type.gd)
+							// var m: MyT... # <-- will suggest MyType

Add support for deprecated symbols to language server and script editor #100019

Are you sure you want to change the base?

Add support for deprecated symbols to language server and script editor #100019

Conversation

Meorge commented Dec 4, 2024 • edited Loading

Tested cases

Methods

Classes

Enums and constants

Properties

Signals

To-Do

Language server

Godot Script editor

Meorge commented Dec 7, 2024

HolonProduction commented Dec 7, 2024

Meorge commented Dec 7, 2024

HolonProduction commented Dec 12, 2024

HolonProduction Dec 12, 2024

Choose a reason for hiding this comment

Meorge Dec 12, 2024

Choose a reason for hiding this comment

HolonProduction Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

Meorge left a comment

Choose a reason for hiding this comment

Meorge Dec 12, 2024

Choose a reason for hiding this comment

Meorge commented Dec 20, 2024

dalexeev commented Dec 20, 2024

Meorge commented Dec 20, 2024

dalexeev Dec 21, 2024

Choose a reason for hiding this comment

Meorge Dec 26, 2024

Choose a reason for hiding this comment

Meorge commented Dec 26, 2024

Meorge commented Dec 4, 2024 •

edited

Loading

HolonProduction Dec 13, 2024 •

edited

Loading