New syntax: Make it an error to set the value of a builtin output property

With the old syntax, this becomes a warning
This commit is contained in:
Olivier Goffart 2022-10-31 16:16:11 +01:00 committed by Olivier Goffart
parent 8dbe745fc4
commit 2ceafc6392
10 changed files with 98 additions and 42 deletions

View file

@ -1118,21 +1118,43 @@ impl Expression {
/// Try to mark this expression to a lvalue that can be assigned to.
///
/// Return true if the expression is a "lvalue" that can be used as the left hand side of a `=` or `+=` or similar
pub fn try_set_rw(&mut self) -> Result<(), String> {
pub fn try_set_rw(
&mut self,
is_legacy_component: bool,
diag: &mut BuildDiagnostics,
what: &'static str,
node: &dyn Spanned,
) -> bool {
match self {
Expression::PropertyReference(nr) => {
nr.mark_as_set();
let lookup = nr.element().borrow().lookup_property(nr.name());
if lookup.is_valid_for_assignment() {
Ok(())
true
} else if is_legacy_component
&& lookup.property_visibility == PropertyVisibility::Output
{
diag.push_warning(format!("{what} on an output property is deprecated"), node);
true
} else {
Err(format!("on a {} property", lookup.property_visibility))
diag.push_error(
format!("{what} on a {} property", lookup.property_visibility),
node,
);
false
}
}
Expression::StructFieldAccess { base, .. } => base.try_set_rw(),
Expression::RepeaterModelReference { .. } => Ok(()),
Expression::ArrayIndex { array, .. } => array.try_set_rw(),
_ => Err("needs to be done on a property".into()),
Expression::StructFieldAccess { base, .. } => {
base.try_set_rw(is_legacy_component, diag, what, node)
}
Expression::RepeaterModelReference { .. } => true,
Expression::ArrayIndex { array, .. } => {
array.try_set_rw(is_legacy_component, diag, what, node)
}
_ => {
diag.push_error(format!("{what} needs to be done on a property"), node);
false
}
}
}
}

View file

@ -408,7 +408,7 @@ impl ElementType {
Some(p) => PropertyLookupResult {
resolved_name,
property_type: p.ty.clone(),
property_visibility: PropertyVisibility::InOut,
property_visibility: p.property_visibility,
is_local_to_component: false,
},
}

View file

@ -67,6 +67,12 @@ impl<'a> LookupCtx<'a> {
&self.property_type
}
}
pub fn is_legacy_component(&self) -> bool {
self.component_scope
.first()
.map_or(false, |e| e.borrow().enclosing_component.upgrade().unwrap().is_legacy_syntax)
}
}
pub enum LookupResult {
@ -270,10 +276,7 @@ impl InScopeLookup {
mut visit_legacy_scope: impl FnMut(&ElementRc) -> Option<R>,
mut visit_scope: impl FnMut(&ElementRc) -> Option<R>,
) -> Option<R> {
let is_legacy = ctx
.component_scope
.first()
.map_or(false, |e| e.borrow().enclosing_component.upgrade().unwrap().is_legacy_syntax);
let is_legacy = ctx.is_legacy_component();
for (idx, elem) in ctx.component_scope.iter().rev().enumerate() {
if let Some(repeated) = &elem.borrow().repeated {
if !repeated.index_id.is_empty() {

View file

@ -249,6 +249,7 @@ impl Component {
tr: &TypeRegister,
) -> Rc<Self> {
let mut child_insertion_point = None;
let is_legacy_syntax = node.child_token(SyntaxKind::ColonEqual).is_some();
let c = Component {
id: parser::identifier_text(&node.DeclaredIdentifier()).unwrap_or_default(),
root_element: Element::from_node(
@ -260,11 +261,12 @@ impl Component {
ElementType::Error
},
&mut child_insertion_point,
is_legacy_syntax,
diag,
tr,
),
child_insertion_point: RefCell::new(child_insertion_point),
is_legacy_syntax: node.child_token(SyntaxKind::ColonEqual).is_some(),
is_legacy_syntax,
..Default::default()
};
let c = Rc::new(c);
@ -665,6 +667,7 @@ impl Element {
id: String,
parent_type: ElementType,
component_child_insertion_point: &mut Option<ChildrenInsertionPoint>,
is_in_legacy_component: bool,
diag: &mut BuildDiagnostics,
tr: &TypeRegister,
) -> ElementRc {
@ -754,12 +757,7 @@ impl Element {
}
}
let visibility = visibility.unwrap_or_else(|| {
if node
.parent()
.and_then(|n| syntax_nodes::Component::new(n))
.and_then(|c| c.child_token(SyntaxKind::ColonEqual))
.is_some()
{
if is_in_legacy_component {
PropertyVisibility::InOut
} else {
PropertyVisibility::Private
@ -807,11 +805,13 @@ impl Element {
node.Binding().filter_map(|b| {
Some((b.child_token(SyntaxKind::Identifier)?, b.BindingExpression().into()))
}),
is_in_legacy_component,
diag,
);
r.parse_bindings(
node.TwoWayBinding()
.filter_map(|b| Some((b.child_token(SyntaxKind::Identifier)?, b.into()))),
is_in_legacy_component,
diag,
);
@ -963,6 +963,7 @@ impl Element {
se.into(),
parent_type,
component_child_insertion_point,
is_in_legacy_component,
diag,
tr,
));
@ -972,6 +973,7 @@ impl Element {
se.into(),
&r,
&mut sub_child_insertion_point,
is_in_legacy_component,
diag,
tr,
);
@ -988,6 +990,7 @@ impl Element {
se.into(),
r.borrow().base_type.clone(),
&mut sub_child_insertion_point,
is_in_legacy_component,
diag,
tr,
);
@ -1068,6 +1071,7 @@ impl Element {
node: syntax_nodes::SubElement,
parent_type: ElementType,
component_child_insertion_point: &mut Option<ChildrenInsertionPoint>,
is_in_legacy_component: bool,
diag: &mut BuildDiagnostics,
tr: &TypeRegister,
) -> ElementRc {
@ -1083,6 +1087,7 @@ impl Element {
id,
parent_type,
component_child_insertion_point,
is_in_legacy_component,
diag,
tr,
)
@ -1092,6 +1097,7 @@ impl Element {
node: syntax_nodes::RepeatedElement,
parent: &ElementRc,
component_child_insertion_point: &mut Option<ChildrenInsertionPoint>,
is_in_legacy_component: bool,
diag: &mut BuildDiagnostics,
tr: &TypeRegister,
) -> ElementRc {
@ -1123,6 +1129,7 @@ impl Element {
node.SubElement(),
parent.borrow().base_type.clone(),
component_child_insertion_point,
is_in_legacy_component,
diag,
tr,
);
@ -1134,6 +1141,7 @@ impl Element {
node: syntax_nodes::ConditionalElement,
parent_type: ElementType,
component_child_insertion_point: &mut Option<ChildrenInsertionPoint>,
is_in_legacy_component: bool,
diag: &mut BuildDiagnostics,
tr: &TypeRegister,
) -> ElementRc {
@ -1148,6 +1156,7 @@ impl Element {
node.SubElement(),
parent_type,
component_child_insertion_point,
is_in_legacy_component,
diag,
tr,
);
@ -1182,6 +1191,7 @@ impl Element {
fn parse_bindings(
&mut self,
bindings: impl Iterator<Item = (crate::parser::SyntaxToken, SyntaxNode)>,
is_in_legacy_component: bool,
diag: &mut BuildDiagnostics,
) {
for (name_token, b) in bindings {
@ -1213,13 +1223,22 @@ impl Element {
&& (lookup_result.property_visibility == PropertyVisibility::Private
|| lookup_result.property_visibility == PropertyVisibility::Output)
{
diag.push_error(
format!(
"Cannot assign to {} property '{}'",
lookup_result.property_visibility, unresolved_name
),
&name_token,
);
if is_in_legacy_component
&& lookup_result.property_visibility == PropertyVisibility::Output
{
diag.push_warning(
format!("Assigning to output property '{unresolved_name}' is deprecated"),
&name_token,
);
} else {
diag.push_error(
format!(
"Cannot assign to {} property '{}'",
lookup_result.property_visibility, unresolved_name
),
&name_token,
);
}
}
if lookup_result.resolved_name != unresolved_name {
@ -1426,6 +1445,7 @@ fn animation_element_from_node(
anim.Binding().filter_map(|b| {
Some((b.child_token(SyntaxKind::Identifier)?, b.BindingExpression().into()))
}),
false,
diag,
);

View file

@ -714,12 +714,12 @@ impl Expression {
.or_else(|| node.child_token(SyntaxKind::Equal).and(Some('=')))
.unwrap_or('_');
if lhs.ty() != Type::Invalid {
if let Err(msg) = lhs.try_set_rw() {
ctx.diag.push_error(
format!("{} {}", if op == '=' { "Assignment" } else { "Self assignment" }, msg),
&node,
);
}
lhs.try_set_rw(
ctx.is_legacy_component(),
ctx.diag,
if op == '=' { "Assignment" } else { "Self assignment" },
&node,
);
}
let ty = lhs.ty();
let expected_ty = match op {

View file

@ -16,6 +16,10 @@ component Compo inherits Rectangle {
input1 = 75;
inout1 = 75;
}
pressed: true;
// ^error{Cannot assign to output property 'pressed'}
}
states [

View file

@ -16,6 +16,9 @@ component Compo inherits Rectangle {
input1 = 75;
// ^error{Assignment on a input property}
inout1 = 75;
self.pressed-x = 42px;
// ^error{Assignment on a output property}
}
}
@ -36,7 +39,13 @@ OldCompo := Rectangle {
input1 = 75;
// ^error{Assignment on a input property}
inout1 = 75;
pressed-x = 45px;
// ^warning{Assignment on an output property is deprecated}
pressed-y -= 45px;
// ^warning{Self assignment on an output property is deprecated}
}
has-hover: true;
// ^warning{Assigning to output property 'has-hover' is deprecated}
}
}

View file

@ -12,7 +12,7 @@ export CheckBox := Rectangle {
callback toggled;
property <string> text <=> text.text;
property <bool> checked;
property <bool> has-focus;
property <bool> has-focus: fs.has-focus;
property<bool> enabled: true;
min-height: 20px;
horizontal-stretch: 0;
@ -82,7 +82,6 @@ export CheckBox := Rectangle {
fs := FocusScope {
width: 0px; // Do not react on clicks
enabled <=> root.enabled;
has_focus <=> root.has-focus;
key-pressed(event) => {
if (event.text == " " || event.text == "\n") {

View file

@ -10,7 +10,7 @@ export CheckBox := Rectangle {
property <string> text <=> label.text;
property <bool> checked;
property <bool> has-focus;
property <bool> has-focus: fs.has-focus;
property<bool> enabled: true;
height: 18px;
@ -87,7 +87,6 @@ export CheckBox := Rectangle {
fs := FocusScope {
width: 0px; // Do not react on clicks
enabled <=> root.enabled;
has_focus <=> root.has-focus;
key-pressed(event) => {
if (event.text == " " || event.text == "\n") {

View file

@ -52,7 +52,7 @@ export Slider := NativeSlider {
accessible-value-maximum: maximum;
accessible-value-step: (maximum - minimum) / 100;
property <bool> has-focus <=> fs.has-focus;
property <bool> has-focus: fs.has-focus;
fs := FocusScope {
width: 0px;
@ -90,7 +90,7 @@ export LineEdit := NativeLineEdit {
property horizontal-alignment <=> inner.horizontal-alignment;
property read-only <=> inner.read-only;
enabled: true;
has-focus <=> inner.has-focus;
has-focus: inner.has-focus;
forward-focus: inner;
callback accepted <=> inner.accepted;
callback edited <=> inner.edited;
@ -120,9 +120,9 @@ export StandardListView := ListView {
item: item;
index: i;
is-selected: current-item == i;
TouchArea {
has-hover: ta.has-hover;
ta := TouchArea {
clicked => { current-item = i; }
has-hover <=> parent.has-hover;
}
}
FocusScope {
@ -163,8 +163,8 @@ export ComboBox := NativeComboBox {
for value[i] in root.model: NativeStandardListViewItem {
item: { text: value };
is-selected: current-index == i;
TouchArea {
has-hover <=> parent.has-hover;
has-hover: ta.has-hover;
ta := TouchArea {
clicked => {
if (root.enabled) {
current-index = i;