Skip to content

Check ill body of contextual element early#4233

Closed
Myriad-Dreamin wants to merge 1 commit intotypst:mainfrom
Myriad-Dreamin:check-ill-contextual
Closed

Check ill body of contextual element early#4233
Myriad-Dreamin wants to merge 1 commit intotypst:mainfrom
Myriad-Dreamin:check-ill-contextual

Conversation

@Myriad-Dreamin
Copy link
Copy Markdown
Contributor

When editing the code from #{context text.font} to #{context}, the typst_ide's analyze_expr panics at src/eval/call.rs:286:68.

thread 'analysis::track_values::tests::test_broken_expr' panicked at src/eval/call.rs:286:68:
called `Option::unwrap()` on a `None` value

let (name, params, body) = match closure.node.cast::<ast::Closure>() {
Some(node) => (node.name(), node.params(), node.body()),
None => (None, ast::Params::default(), closure.node.cast().unwrap()),
};

I printed the contextual element, and found that it is caused by the body of contextual expression Eof is not an expression.

context closure: Closure { node: Eof: "", defaults: [], captured: Scope {}, num_pos_params: 0 }

It is strange that typst-cli and webapp don't panic with this editing, but I indeed eliminate the panic by checking the legality of the body when constructing the contextual element.

laurmaedje added a commit that referenced this pull request May 29, 2024
The current impl led to `Expr::default().to_untyped().cast::<Expr>()` yielding `None`, which is undesirable.

Supersedes #4233. Because this crash was triggered by tooltip code, this also adds a bit of test scaffolding for tooltips and a few tooltip tests.
@laurmaedje
Copy link
Copy Markdown
Member

laurmaedje commented May 29, 2024

Thanks for spotting this! This somewhat weird situation arises because we still evaluate malformed files for IDE stuff... I tried to fix this more generally in #4288, so that we don't run into such a problem again.

@laurmaedje laurmaedje closed this May 29, 2024
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.

2 participants