slint-ui / slint

Slint is a declarative GUI toolkit to build native user interfaces for Rust, C++, or JavaScript apps.
https://slint.dev
Other
17.72k stars 615 forks source link

Panic when setting `opacity` on a component in live editor #5696

Closed mitchgrout closed 3 months ago

mitchgrout commented 4 months ago

Start with the following component:

export component ExportedComponent {
  opacity: 1;
}

In the live editor, select the opacity field, use the arrow keys to move to the beginning of the field, and then insert a space. This triggers the following panic:

thread 'main' panicked at D:\cargo\registry\src\index.crates.io-6f17d22bba15001f\rowan-0.15.15\src\green\builder.rs:113:9:
assertion `left == right` failed
  left: 2
 right: 1

Triggered on VSCode Slint v1.7.1, but seemingly not triggerable on SlintPad

mitchgrout commented 4 months ago

The assertion in question is from the rowan crate, in GreenNodeBuilder::finish:

assert_eq!(self.children.len(), 1);

Unfortunately I do not have any symbols available in my stack trace, so I cannot provide any extra information there.

ogoffart commented 4 months ago

Thanks for the bug report.

I can reproduce the problem.

Backtrace:

3: core::panicking::assert_failed
   4: rowan::green::builder::GreenNodeBuilder::finish
   5: slint_lsp::common::properties::set_binding
   6: slint_lsp::preview::evaluate_binding
   7: slint_lsp::preview::test_binding
   8: i_slint_core::callbacks::Callback<Arg,Ret>::set_handler::{{closure}}
   9: core::ops::function::FnOnce::call_once
  10: core::ops::function::FnOnce::call_once
  11: i_slint_core::callbacks::Callback<Arg,Ret>::call
  12: i_slint_core::items::TextInputVTable::key_event
  13: i_slint_core::window::WindowInner::process_key_input

I think one issue is that parse_expression_as_bindingexpression function inlined in set_binding, calls p.start_node(SyntaxKind::BindingExpression), but that will first skip all the whitespace, so we'd have first the whitespace before the BindingExpression.

The fix would be to use the API on p.builder directly for the root node. (It is also suspicious that the code doesn't validate that there are no trailing token) I noticed another bug in that code that we don't validate errors if there are still token