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

Both outline and codelenses support can now handle error trees #491

Merged

Conversation

PieterOlivier
Copy link

Both locateCodeLenses (in RascalLanguageServices.java) and outlineRascalModule (in Outline.rsc) would crash if they encountered trees with error nodes.

This PR fixes these problems.

In case of the outline, elements with errors are removed from the outline.

In case of the code lenses, top-level elements with errors are ignored for code lenses.

@PieterOlivier PieterOlivier merged commit 91d0e6b into error-recovery/rascal Oct 25, 2024
4 of 12 checks passed
children += [symbol(clean("<v.name>"), variable(), v@\loc, detail="variable <t> <v>") | v <- vars, !hasErrors(v)];
}

case decl: (Declaration) `<Tags _> <Visibility _> anno <Type t> <Type ot>@<Name name>;`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once the match succeeds we now this node is not in error, so we don't need to check hasErrors for that.. but...


case decl: (Declaration) `<Tags _> <Visibility _> anno <Type t> <Type ot>@<Name name>;`:
if (!hasErrors(decl)) {
children += [symbol(clean("<name>"), field(), t@\loc, detail="anno <t> <ot>")];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here you see only bound variables are yielded to string, and that is supported by error trees as well, so no problem..


case decl: (Declaration) `<Tags _> <Visibility _> alias <UserType u> = <Type al>;`:
if (!hasErrors(decl)) {
children += [symbol(clean("<u.name>"), struct(), u@\loc, detail="<u> = <al>")];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but here we use field projecten u.name and u might not even have a name if UserType u is an error tree. This is going to happen all over the place for error trees, so I'd like a more reusable/generic solution then having to guard all the field projections. It will become a cross-cutting and scattered concern everywhere otherwise, while error trees are a builtin language concept that features like . projection {c,sh,w}ould handle gracefully.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we may assume that if a programmer uses u.name then in that place u.name normally does not fail, and so it should also not fail for error children.

  • if u is an error tree, and the error "dot" is after the name field, then field projection can still work as it was supposed to.
  • otherwise if the "dot" is before the name field or on it, then it may be possible to return an empty stub error tree for name instead of failing.

If we have such a semantics for field projection, there would be a lot less pressure on user code for dealing with all kinds of exceptions. In general for productions with n children, there are I guess something like n!/(2 * (n-2)!) possible combinations of having and not having error children, so that can become untenable.

Another option is to rewrite all field projections to pattern matches, which would fail in the absence of a field. But in this example that would lead to a lot of superfluous case distintions:

syntax UserType
	= name: QualifiedName name 
	| parametric: QualifiedName name >> "[" "[" {Type ","}+ parameters "]" ;

Here we really do not care about the difference between the two rules but rather their commonality which is fine by the .name field projection. So I guess pattern matching is not a solution here.

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

Successfully merging this pull request may close these issues.

3 participants