diff --git a/internal/compiler/expression_tree.rs b/internal/compiler/expression_tree.rs index 7c6c03cfc..e33fac2e6 100644 --- a/internal/compiler/expression_tree.rs +++ b/internal/compiler/expression_tree.rs @@ -1130,7 +1130,26 @@ impl Expression { nr.mark_as_set(); let lookup = nr.element().borrow().lookup_property(nr.name()); if lookup.is_valid_for_assignment() { - true + if !nr + .element() + .borrow() + .property_analysis + .borrow() + .get(nr.name()) + .map_or(false, |d| d.is_linked_to_read_only) + { + true + } else if is_legacy_component { + diag.push_warning("Modifying a property that is linked to a read-only property is deprecated".into(), node); + true + } else { + diag.push_error( + "Cannot modify a property that is linked to a read-only property" + .into(), + node, + ); + false + } } else if is_legacy_component && lookup.property_visibility == PropertyVisibility::Output { diff --git a/internal/compiler/langtype.rs b/internal/compiler/langtype.rs index 5f362af3f..fa3168c07 100644 --- a/internal/compiler/langtype.rs +++ b/internal/compiler/langtype.rs @@ -346,7 +346,7 @@ pub struct BuiltinPropertyInfo { impl BuiltinPropertyInfo { pub fn new(ty: Type) -> Self { - Self { ty, default_value: None, property_visibility: PropertyVisibility::Private } + Self { ty, default_value: None, property_visibility: PropertyVisibility::InOut } } pub fn is_native_output(&self) -> bool { diff --git a/internal/compiler/load_builtins.rs b/internal/compiler/load_builtins.rs index 5eb68cfc2..9318e099a 100644 --- a/internal/compiler/load_builtins.rs +++ b/internal/compiler/load_builtins.rs @@ -86,6 +86,8 @@ pub fn load_builtins(register: &mut TypeRegister) { register, )); + info.property_visibility = PropertyVisibility::Private; + for token in p.children_with_tokens() { if token.kind() != SyntaxKind::Identifier { continue; diff --git a/internal/compiler/object_tree.rs b/internal/compiler/object_tree.rs index 1a354fe0b..6a56d8886 100644 --- a/internal/compiler/object_tree.rs +++ b/internal/compiler/object_tree.rs @@ -614,6 +614,9 @@ pub struct PropertyAnalysis { /// true if this property is read from another component pub is_read_externally: bool, + + /// True if the property is linked to another property that is read only. That property becomes read-only + pub is_linked_to_read_only: bool, } impl PropertyAnalysis { @@ -1233,7 +1236,7 @@ impl Element { && lookup_result.property_visibility == PropertyVisibility::Output { diag.push_warning( - format!("Assigning to out property '{unresolved_name}' is deprecated"), + format!("Assigning to output property '{unresolved_name}' is deprecated"), &name_token, ); } else { diff --git a/internal/compiler/passes/resolving.rs b/internal/compiler/passes/resolving.rs index d67b8bda4..99f097dc3 100644 --- a/internal/compiler/passes/resolving.rs +++ b/internal/compiler/passes/resolving.rs @@ -30,7 +30,6 @@ fn resolve_expression( scope: &ComponentScope, type_register: &TypeRegister, type_loader: &crate::typeloader::TypeLoader, - two_ways: &mut Vec<(String, NamedReference)>, diag: &mut BuildDiagnostics, ) { if let Expression::Uncompiled(node) = expr { @@ -59,14 +58,7 @@ fn resolve_expression( Expression::from_binding_expression_node(node.clone(), &mut lookup_ctx) } SyntaxKind::TwoWayBinding => { - if lookup_ctx.property_type == Type::Invalid { - // An attempt to resolve this already failed when trying to resolve the property type - assert!(diag.has_error()); - return; - } - if let Some(nr) = resolve_two_way_binding(node.clone().into(), &mut lookup_ctx) { - two_ways.push((property_name.unwrap().into(), nr)); - } + assert!(diag.has_error(), "Two way binding should have been resolved already (property: {property_name:?})"); Expression::Invalid } _ => { @@ -83,6 +75,8 @@ pub fn resolve_expressions( type_loader: &crate::typeloader::TypeLoader, diag: &mut BuildDiagnostics, ) { + resolve_two_way_bindings(doc, &doc.local_registry, diag); + for component in doc.inner_components.iter() { let scope = ComponentScope(vec![]); @@ -90,7 +84,6 @@ pub fn resolve_expressions( let mut new_scope = scope.clone(); let mut is_repeated = elem.borrow().repeated.is_some(); new_scope.0.push(elem.clone()); - let mut two_ways = vec![]; visit_element_expressions(elem, |expr, property_name, property_type| { if is_repeated { // The first expression is always the model and it needs to be resolved with the parent scope @@ -102,7 +95,6 @@ pub fn resolve_expressions( &scope, &doc.local_registry, type_loader, - &mut two_ways, diag, ); is_repeated = false; @@ -114,14 +106,10 @@ pub fn resolve_expressions( &new_scope, &doc.local_registry, type_loader, - &mut two_ways, diag, ) } }); - for (prop, nr) in two_ways { - elem.borrow().bindings.get(&prop).unwrap().borrow_mut().two_way_bindings.push(nr); - } new_scope }) } @@ -1139,6 +1127,123 @@ fn maybe_lookup_object( base } +/// Go through all the two way binding and resolve them first +fn resolve_two_way_bindings( + doc: &Document, + type_register: &TypeRegister, + diag: &mut BuildDiagnostics, +) { + for component in doc.inner_components.iter() { + let scope = ComponentScope(vec![]); + + recurse_elem(&component.root_element, &scope, &mut |elem, scope| { + let mut new_scope = scope.clone(); + new_scope.0.push(elem.clone()); + for (prop_name, binding) in &elem.borrow().bindings { + let mut binding = binding.borrow_mut(); + if let Expression::Uncompiled(node) = binding.expression.clone() { + if let Some(n) = syntax_nodes::TwoWayBinding::new(node.clone()) { + let lhs_lookup = elem.borrow().lookup_property(prop_name); + if lhs_lookup.property_type == Type::Invalid { + // An attempt to resolve this already failed when trying to resolve the property type + assert!(diag.has_error()); + continue; + } + let mut lookup_ctx = LookupCtx { + property_name: Some(prop_name.as_str()), + property_type: lhs_lookup.property_type.clone(), + component_scope: &new_scope.0, + diag, + arguments: vec![], + type_register, + type_loader: None, + current_token: Some(node.clone().into()), + }; + + binding.expression = Expression::Invalid; + + if let Some(nr) = resolve_two_way_binding(n, &mut lookup_ctx) { + binding.two_way_bindings.push(nr.clone()); + + // Check the compatibility. + let rhs_lookup = nr.element().borrow().lookup_property(nr.name()); + + if !rhs_lookup.is_valid_for_assignment() { + match ( + lhs_lookup.property_visibility, + rhs_lookup.property_visibility, + ) { + (PropertyVisibility::Input, PropertyVisibility::Input) + if !lhs_lookup.is_local_to_component => + { + assert!(rhs_lookup.is_local_to_component); + marked_linked_read_only(elem, prop_name); + } + ( + PropertyVisibility::Output | PropertyVisibility::Private, + PropertyVisibility::Output | PropertyVisibility::Input, + ) => { + assert!(lhs_lookup.is_local_to_component); + marked_linked_read_only(elem, prop_name); + } + (PropertyVisibility::Input, PropertyVisibility::Output) + if !lhs_lookup.is_local_to_component => + { + assert!(!rhs_lookup.is_local_to_component); + marked_linked_read_only(elem, prop_name); + } + _ => { + if lookup_ctx.is_legacy_component() { + diag.push_warning( + format!( + "Link to a {} property is deprecated", + rhs_lookup.property_visibility + ), + &node, + ); + } else { + diag.push_error( + format!( + "Cannot link to a {} property", + rhs_lookup.property_visibility + ), + &node, + ) + } + } + } + } else if !lhs_lookup.is_valid_for_assignment() { + if rhs_lookup.is_local_to_component + && rhs_lookup.property_visibility == PropertyVisibility::InOut + { + if lookup_ctx.is_legacy_component() { + debug_assert!(!diag.is_empty()); // warning should already be reported + } else { + diag.push_error("Cannot link input property".into(), &node); + } + } else { + // This is allowed, but then the rhs must also become read only. + marked_linked_read_only(&nr.element(), nr.name()); + } + } + } + } + } + } + new_scope + }) + } + + fn marked_linked_read_only(elem: &ElementRc, prop_name: &str) { + elem.borrow() + .property_analysis + .borrow_mut() + .entry(prop_name.to_string()) + .or_default() + .is_linked_to_read_only = true; + } +} + pub fn resolve_two_way_binding( node: syntax_nodes::TwoWayBinding, ctx: &mut LookupCtx, diff --git a/internal/compiler/tests/syntax/new_syntax/input_output2.slint b/internal/compiler/tests/syntax/new_syntax/input_output2.slint index 376348da5..ee9e7ca3e 100644 --- a/internal/compiler/tests/syntax/new_syntax/input_output2.slint +++ b/internal/compiler/tests/syntax/new_syntax/input_output2.slint @@ -45,7 +45,7 @@ OldCompo := Rectangle { // ^warning{Self assignment on an output property is deprecated} } has-hover: true; -// ^warning{Assigning to out property 'has-hover' is deprecated} +// ^warning{Assigning to output property 'has-hover' is deprecated} } } diff --git a/internal/compiler/tests/syntax/new_syntax/two_way_input_output.slint b/internal/compiler/tests/syntax/new_syntax/two_way_input_output.slint new file mode 100644 index 000000000..f9bbd42eb --- /dev/null +++ b/internal/compiler/tests/syntax/new_syntax/two_way_input_output.slint @@ -0,0 +1,325 @@ +// Copyright © SixtyFPS GmbH +// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-commercial + +component Button { + in property enabled : true; + out property pressed; + in-out property checked; + callback clicked(); +} + +component C1 { + out property out; + in property in; + in-out property inout; + property priv; + + // pressed is "output" in Button + out <=> b.pressed; // ok (but then there should be no other assignment?) + in <=> b.pressed; // Error +// ^error{Cannot link to a output property} + inout <=> b.pressed; +// ^error{Cannot link to a output property} + priv <=> b.pressed; // makes assignment forbidden + + + + b:= Button { + clicked => { + in = !in; +// ^error{Assignment on a input property} + out = !out; +// ^error{Cannot modify a property that is linked to a read-only property} + inout = !inout; + priv = !priv; +// ^error{Cannot modify a property that is linked to a read-only property} + + self.enabled = !self.enabled; + self.checked = !self.checked; + self.pressed = !self.pressed; +// ^error{Assignment on a output property} + + } + } + +} + + + component C2 { + out property out; + in property in; + in-out property inout; + property priv; + + // enabled is "input" in Button + out <=> b.enabled; + in <=> b.enabled; + inout <=> b.enabled; + priv <=> b.enabled; + + b:= Button { + clicked => { + in = !in; +// ^error{Assignment on a input property} + out = !out; + inout = !inout; + priv = !priv; + + + self.enabled = !self.enabled; +// ^error{Cannot modify a property that is linked to a read-only property} + self.checked = !self.checked; + self.pressed = !self.pressed; +// ^error{Assignment on a output property} + + } + } +} + +component C3 { + out property out; + in property in; + in-out property inout; + property priv; + + // checked is "input/output" in Button + out <=> b.checked; + in <=> b.checked; + inout <=> b.checked; + priv <=> b.checked; + + b:= Button { + clicked => { + in = !in; +// ^error{Assignment on a input property} + out = !out; + inout = !inout; + priv = !priv; + + self.enabled = !self.enabled; + self.checked = !self.checked; +// ^error{Cannot modify a property that is linked to a read-only property} + self.pressed = !self.pressed; +// ^error{Assignment on a output property} + } + } +} + + +component C5 { + out property out; + in property in; + in-out property inout; + property priv; + + Button { enabled <=> out; } + Button { + enabled <=> in; + clicked => { self.enabled = !self.enabled; } +// ^error{Cannot modify a property that is linked to a read-only property} + } + Button { enabled <=> inout; } + Button { enabled <=> priv; } +} + +component C6 { + out property out; + in property in; + in-out property inout; + property priv; + + Button { + checked <=> out; + clicked => { out = !out; self.checked = !self.checked; } + } + Button { + checked <=> in; +// ^error{Cannot link to a input property} + clicked => { self.checked = !self.checked; } + } + Button { + checked <=> inout; + clicked => { inout = !inout; self.checked = !self.checked; } + } + Button { + checked <=> priv; + clicked => { priv = !priv; self.checked = !self.checked; } + } +} + +component C7 { + b1 := Button { + clicked => { + self.enabled = !self.enabled; + self.checked = !self.checked; + self.pressed = !self.pressed; +// ^error{Assignment on a output property} + } + } + Button { + enabled <=> b1.pressed; + clicked => { self.enabled = !self.enabled; } +// ^error{Cannot modify a property that is linked to a read-only property} + } + b2 := Button { + clicked => { + self.enabled = !self.enabled; + self.checked = !self.checked; + self.pressed = !self.pressed; +// ^error{Assignment on a output property} + } + } + Button { checked <=> b2.pressed; } +// ^error{Cannot link to a output property} + + b3 := Button { + clicked => { + self.enabled = !self.enabled; + self.checked = !self.checked; + self.pressed = !self.pressed; +// ^error{Assignment on a output property} + } + } + Button { + checked <=> b3.checked; + enabled <=> b3.checked; + } + Button { + checked <=> b3.enabled; + enabled <=> b3.enabled; + } +} + +component C8 { + out property out1; + out property out2; + out property out3; + out property out4; + + out property out <=> out1; + in property in <=> out2; + in-out property inout <=> out3; + property priv <=> out4; + + Button { + clicked => { + out1 = !out1; + out2 = !out2; +// ^error{Cannot modify a property that is linked to a read-only property} + out3 = !out3; + out4 = !out4; + } + } +} + +component C9 { + in property in1; + in property in2; + in property in3; + in property in4; + + out property out <=> in1; + in property in <=> in2; +// ^error{Cannot link to a input property} + in-out property inout <=> in3; +// ^error{Cannot link to a input property} + property priv <=> in4; + + Button { + clicked => { + out = !out; +// ^error{Cannot modify a property that is linked to a read-only property} + in = !in; +// ^error{Assignment on a input property} + inout = !inout; + priv = !priv; +// ^error{Cannot modify a property that is linked to a read-only property} + } + } +} + + +component C10 { + in-out property inout1; + in-out property inout2; + in-out property inout3; + in-out property inout4; + + out property out <=> inout1; + in property in <=> inout2; +// ^error{Cannot link input property} + in-out property inout <=> inout3; + property priv <=> inout4; + + Button { + clicked => { + inout1 = !inout1; + inout2 = !inout2; + inout3 = !inout3; + inout4 = !inout4; + out = !out; + in = !in; +// ^error{Assignment on a input property} + inout = !inout; + priv = !priv; + } + } +} + +component C11 { + property priv1; + property priv2; + property priv3; + property priv4; + + out property out <=> priv1; + in property in <=> priv2; + in-out property inout <=> priv3; + property priv <=> priv4; + + Button { + clicked => { + priv1 = !priv1; + priv2 = !priv2; +// ^error{Cannot modify a property that is linked to a read-only property} + priv3 = !priv3; + priv4 = !priv4; + out = !out; + in = !in; +// ^error{Assignment on a input property} + inout = !inout; + priv = !priv; + } + } +} + +Legacy1 := Rectangle { + b1:= Button {} + in property in1 <=> b1.pressed; +// ^warning{Link to a output property is deprecated} + out property out1 <=> b1.pressed; + in-out property inout1 <=> b1.pressed; +// ^warning{Link to a output property is deprecated} + + + property p1; + Button { + pressed <=> p1; +// ^warning{Assigning to output property 'pressed' is deprecated} + clicked => { + p1 = !p1; + out1 = !out1; +// ^warning{Modifying a property that is linked to a read-only property is deprecated} + } + } + Button { + enabled <=> self.pressed; + clicked => { + self.enabled = !self.enabled; +// ^warning{Modifying a property that is linked to a read-only property is deprecated} + } + } + + +} + diff --git a/internal/compiler/widgets/common/common.slint b/internal/compiler/widgets/common/common.slint index f6d889f7a..54a62d959 100644 --- a/internal/compiler/widgets/common/common.slint +++ b/internal/compiler/widgets/common/common.slint @@ -11,7 +11,7 @@ export LineEditInner := Rectangle { property text <=> input.text; property placeholder-color <=> placeholder.color; property enabled <=> input.enabled; - property has-focus <=> input.has-focus; + property has-focus: input.has-focus; property input-type <=> input.input-type; property horizontal-alignment <=> input.horizontal-alignment; property read-only <=> input.read-only; @@ -47,7 +47,7 @@ export LineEditInner := Rectangle { export TextEdit := ScrollView { property font-size <=> input.font-size; property text <=> input.text; - has-focus <=> input.has-focus; + has-focus: input.has-focus; enabled <=> input.enabled; property wrap <=> input.wrap; property horizontal-alignment <=> input.horizontal-alignment; diff --git a/internal/compiler/widgets/fluent-base/std-widgets-impl.slint b/internal/compiler/widgets/fluent-base/std-widgets-impl.slint index e46c45144..9bbdaf03c 100644 --- a/internal/compiler/widgets/fluent-base/std-widgets-impl.slint +++ b/internal/compiler/widgets/fluent-base/std-widgets-impl.slint @@ -83,7 +83,7 @@ export global StyleMetrics := { export Button := Rectangle { callback clicked; property text <=> text.text; - property has-focus <=> fs.has-focus; + property has-focus: fs.has-focus; property pressed: self.enabled && touch.pressed; property enabled <=> touch.enabled; property checkable; diff --git a/internal/compiler/widgets/fluent-base/std-widgets.slint b/internal/compiler/widgets/fluent-base/std-widgets.slint index 73e81def1..08c80a524 100644 --- a/internal/compiler/widgets/fluent-base/std-widgets.slint +++ b/internal/compiler/widgets/fluent-base/std-widgets.slint @@ -218,7 +218,7 @@ export Slider := Rectangle { property maximum: 100; property minimum: 0; property value; - property has-focus <=> fs.has-focus; + property has-focus: fs.has-focus; property enabled <=> touch.enabled; callback changed(float);