Add interfaces as a proof-of-concept experimental feature #10181
Conversation
| { | ||
| r.property_declarations.insert(prop_name.clone(), prop_decl.clone()); | ||
| } | ||
| } |
There was a problem hiding this comment.
This likely needs more checks, e.g. that the properties do not already existed on base components.
| let message = format!( | ||
| "Cannot override binding for property '{}' from interface '{}'", | ||
| prop_name, uses_statement.interface_name | ||
| ); |
There was a problem hiding this comment.
I struggled to come up with a test case for this error - the previous checks caught it first. Suggestions would be appreciated.
There was a problem hiding this comment.
I think this would be something like
property <XXX> foo: something;
foo: something_else;Or, in your case:
... use ... (that has a property foo with a binding)
foo: something_else;Unclear if this should be an error actually.
ogoffart
left a comment
ogoffart
left a comment
There was a problem hiding this comment.
Looks very good, thank you.
I think that apart from the failling CI, this could actually be merged.
I just wrote some comments that could make the patch even better.
| let message = format!( | ||
| "Cannot override binding for property '{}' from interface '{}'", | ||
| prop_name, uses_statement.interface_name | ||
| ); |
There was a problem hiding this comment.
I think this would be something like
property <XXX> foo: something;
foo: something_else;Or, in your case:
... use ... (that has a property foo with a binding)
foo: something_else;Unclear if this should be an error actually.
| } | ||
| } | ||
|
|
||
| /// Check that the given element implements the given interface. Emits a diagnostic if the interface is not implemented. |
There was a problem hiding this comment.
So this is like implicit interface where the components itself didn't explicitly sayd implements <...>
I'm wondering if interface implementation shouldn't be explicit.
There was a problem hiding this comment.
Yes. My initial approach was to store the list of interfaces that were implemented as a member of the element (i.e. interfaces should be explicit). That wasn't working because the list was getting lost/not available where I was expecting it, so I switched to this approach for the proof-of-concept. I'd be happy to look at explicit interfaces as a follow-up.
| /// component F uses { I from A } implements J { } | ||
| /// component F uses { I from A } inherits B { } | ||
| /// component F uses { I from A, J from B } { } | ||
| /// component F uses { I from A, J from B } implements J { } | ||
| /// component F uses { I from A, J from B } inherits C { } |
There was a problem hiding this comment.
I'm not sure we should support having both Implement ans inherits or having several interface.
There was a problem hiding this comment.
I don't see why you wouldn't want to support implements and inherits for the same component. For example,
interface ButtonInterface {
callback clicked;
}
export component MyButton implements ButtonInterface inherits Rectangle {
}means that the both the rectangle and interface APIs are available to the user of MyButton. Alternatively we have to verbosely duplicate all the API we want to expose from MyButton, which kind of defeats the purpose of the interface.
I also don't see why you wouldn't want to be able to expose more than one interface. In principle std-widgets might want to expose at least two: the particular std-widget interface, and an accessibility interface.
|
(I believe the test faillure is caused by the merge of #9703 and just need to rerun the test with the SLINT_SYNTAX_TEST_UPDATE env variable set) |
Add a new ElementType variant that will allow us to parse interface declarations using the existing element parsing code. The match arms implemented are my best guess based on the context of the enclosing function and how a global is handled.
A document may contain zero or more interfaces. Interfaces do not support inheritance.
Verify that an interface cannot have: - sub elements; - repeated elements; - property animations; - states; - transitions; - an init callback declaration; - an init callback implementation.
Warn if private properties are declared.
It is treated as an alias for `inherits`, for now.
The compiler produces an error when the `implements` keyword is used before a global, component, or builtin type. The parser doesn't know about this kind of relationship - the "implements" and "inherits" keywords are just consumed. For now we just go back and check what the expected relationship type was whilst constructing an Element.
Attempt to verify that the interpreter can see and use properties declared in an interface that a component implements. This is an initial proof-of-concept, we will need to come back and add checks and tests for mixing 'inherits' with 'implements'.
Group interface related tests in a subdirectory of their own.
Allow a new component to specify that it exposes one or more interfaces via a given child element. This commit introduces the syntax, we will add the implementation in a future commit.
For each uses statement, iterate through the interface properties and create two-way bindings to the property on the specified base component. The error cases will be populated in follow-up commit. For now, demonstrate that the initial concept works.
Detect and emit errors where: - the type specified in a uses statement is not an interface; - the type specified in a uses statement is unknown; - the type specified in a uses statement cannot be used in this context.
…does not exist Add a test case to verify the expected error message.
Add a test to verify that interfaces from imported modules can be used.
Add a test case to verify that: - we can import an interface as another name and re-use it; - we can import a component that implements a renamed interface and use it.
For each property in an interface that a child element is expected to implement, verify that the child has a property with the same name, type and visibility. This is easier than attempting to store and match interface names which may change if the user renames an interface when importing.
Emit a compile error if the user attempts to declare a property that would override a property from an interface.
…an inherited property Look up the interface property on the base type before allowing a component to implement the interface.
…xisting binding I couldn't figure out a good way to test this - everything I came up with got caught by the prior check that the property does not already exist.
Guard these keywords behind the experimental features flag. Slint must be compiled with the `SLINT_ENABLE_EXPERIMENTAL_FEATURES` environment variable set to enable these keywords.
…ifier Previously we were not creating a DeclaredIdentifier if a syntax error was encountered. This would cause a panic when `UsesIdentifier::DeclaredIdentifier()` was called for a node with a syntax error. As a result, we can convert `TryFrom<&syntax_nodes::UsesIdentifier> for UsesStatement` to `From<&syntax_nodes::UsesIdentifier> for UsesStatement`.
…ier` to `UsesSpecifier -> UsesIdentifier` We don't need to intermediate list object because we only expect a UsesSpecifier to contain UsesIdentifiers.
Previously the space between the DeclaredIdentifier and the `from`
keyword was being removed. E.g.:
```slint
component C uses { Foo from bar } { }
```
would become:
```slint
component C uses { Foofrom bar } { }
```
This would cause a syntax error. This no longer happens.
Previously this was a warning to match the global behaviour. Given that interfaces are a new feature, make it an error to declare them using the deprecated ':='.
It does not make sense to declare an interface with properties that are not accessible to the user of the interface.
The syntax for declaring an expected diagnostic has changed to include the span (slint-ui#9703). Update the interface tests that were added using the old syntax.
0x6e
force-pushed
the
feature/interfaces-v2
branch
from
ce79e69 to
56d9063
Compare
|
Thank you! |
…nterface errors The `implements` keyword should not be supported if experimental features are not enabled.
Only store the `syntax_nodes::UsesIdentifier` and replace the old members with functions returning the appropriate child node of the `UsesIdentifier`.
We cannot instantiate an interface element, so we should not be able to reach one by recursing into the children of a component.
2 participants
Proof of concept for #1870.
Introduce a new keyword
interfacewhich allows a user to declare an interface. This follows the same rules as a global. An interface cannot have:Introduce a new keyword
implementswhich allows a user to declare that a component implements an interface. This initial implementation simply copies the interface property declarations into the component.Introduce a new keyword
useswhich allows a user to declare that a parent component implements an interface through a child component. This initial implementation creates two way property bindings between the parent component and the child for each property in the interface.This PR only deals with simple property declarations. Future PRs will be needed to: