Add simple symbol renaming

[KoBeWi]

Closes godotengine/godot-proposals#899
Probably supersedes #102380

This PR adds Rename Symbol functionality, which renames symbols. Unlike the other PR, I've taken a very simple approach: lookup the symbol to be renamed, do an exact Find in Files search, filter out results based on the lookup. Seems to perform well enough.

Differentiating class identifier from a string/comment:

dnHxeq4Gz5.mp4

Differentiating same-named local variables from different methods:

iNkwIAEcmH.mp4

I have tested it with my MetSys addon and it performs really well. The symbols can be renamed across files, and it even differentiates name aliases.

However I discovered a bug in the process (#117948), which makes some references incorrectly unmarked. You can still review the results manually and check them. (there shouldn't be false-positives, i.e. references checked, but belonging to other symbol)

[enetheru]

does it also handle doc comments? if so that would be pretty cool.

[AdriaandeJongh]

This is the number one feature request of the in-engine script editor, so I look forward to give it a spin!

This is perhaps a stupid question, but I’m asking it anyway; what are the limitations of this simpler method? Or in other words, what cases can this simpler method never address that the more complex implementation by @adamscott’s PR can?

[dalexeev]

what are the limitations of this simpler method?

Potentially, the alternative could detect symbol usages in more complex contexts. Like object["property"], object.get("property"), object.call("method"), etc. But I think this approach is quite good and significantly simpler (see Pareto principle).

Essentially, this is assisted renaming, which is entirely justified for dynamic and gradually typed languages ​​like GDScript. Our main goal is to eliminate false-positive detection of symbol usages (checkboxes should be automatically selected only for safe occurrences). False-negative detection is less of a problem; the user can manually check occurrences that the editor cannot confidently classify (due to dynamic context or implicit access).

[HolonProduction]

First of all: This is approach is totally valid, it's what the language server has been doing for years.

IMO this should not live in editor code. We need to stop treating lookup_code as a "one solution fits all" method in the public API. There is behaviour which makes sense for hover hints (e.g. resolving identifiers in comments), but for renaming you generally don't want that. (I'm not saying that the implementation needs to be changed to introduce this feature, I'm just talking about how to structure the API.)

A few takeaways from the language server that might be relevant to this PR:

• lookup_code is not exactly fast. An optimization like LSP: Optimize find_usages_in_file #115851 should be in place to prevent calling it if possible. (Another reason why this should not be implemented in general editor code, because this optimization requires knowledge about the languages syntax)
• The LSP applies additional resolve logic to fix lookup_code on declarations. This should mostly be not necessary anymore with Implement CompletionType::COMPLETION_DECLARATION and lambda tooltips #102937 however enum items are still not resolved by lookup_code iirc
• lookup_code isn't exactly good in terms of correctness and a lot of this will probably never be fixed since some people consider it useful for hover hints. -> at least in the public API we should split this from the API for hover hints.

[dalexeev]

Another limitation is that lookup_code() and the find/rename function only work on saved files. It won't find unsaved changes.

[AdriaandeJongh]

Is it also acceptable that this simpler solution can be implemented as a stopgap in case users do want more than assisted renaming?

Another limitation is that lookup_code() and the find/rename function only work on saved files. It won't find unsaved changes.

this is a pretty big caveat… hmm…

[Drogobo]

I am writing to say that this is absolutely something we need. I go to VSCode to solve my godot problem, which is not what I like to do while working with godot.

[nubels]

I think refactoring is one of Godot's biggest weaknesses, so it's awesome to see progress in this area. Thank you!

How about extending the functionality of this feature so that it also renames the variables in resource and scene files? This would prevent data loss when renaming exported variables. My current workaround is to use an external text editor, which is not ideal.

[themipper]

Every step towards any refactoring functionality within Godot is the right step.

I would still love to see the other PR #102380
land though as it probably covers more edge cases.

[KoBeWi]

How about extending the functionality of this feature so that it also renames the variables in resource and scene files?

The simple approach won't work for that. It'd need to be implemented separately and is much more complex.

[AThousandShips]

Suggested change

	rename_input->connect("text_submitted", callable_mp(this, &ScriptTextEditor::_confirm_rename));
	rename_input->connect(SceneStringName(text_submitted), callable_mp(this, &ScriptTextEditor::_confirm_rename));