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

GDScript: Do phrase level recovery when parsing faulty dictionaries #102218

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HolonProduction
Copy link
Member

@HolonProduction HolonProduction commented Jan 30, 2025

Fixes #78602
Fixes #37061 (this is on the 3.x milestone, but I don't think we'll able to back port any fix, so we might as well close it)
Fixes partially #96157

Partial alternative to #79424

Descritpion from there:

There are cases in the parser were successfully parsed nodes don't end up in the final parse tree because they are part of a faulty structure. Because those nodes do not end up in the parse tree they are never examined by the analyzer. This is usually fine because it is assumed that scripts don't contain errors. However when providing code completion a structure might be invalid because the user is currently typing it in. In this case we still want to provide the best autocompletion for the valid subparts. And indeed these subparts end up in the CompletionContext. But since the analyzer never resolved their types the autocompletion can be worse at some points.

This PR tries to resolve this by letting the parser save such completed fragments with the nearest valid Node if parsing fails somewhere. The analyzer does now also analyze all fragments for a node and can therefore add type information e.g. for identifiers.

In #79424 the parser would add such fully parsed parts to a new list "unfinished fragments" and the analyzer was adjusted to analyze this list as well. This PR takes a different approach and patches the tree in the parser in a way that is transparent to the analyzer. This is a better approach since it ensures that the analyzer encounters those fragments in the correct context.

The patched tree might be faulty on the analyzer level, but to the analyzer it looks like an syntactically correct file. Those analyzer errors resulting from faulty patching (e.g. patching a incompatible value into a typed dictionary) can be ignored, since analyzer errors are not shown as long as there are parser errors (which is always the case when we are patching the tree).

If we can reach any consensus on this approach I'll add recovery for the other cases from #79424 as well (ternaries and match statements). But I'd like to hear some thoughts on this first, since I don't think there is any precedence for this kind of recovery in the parser.

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