API review of the slint interpreter Compiler api

Closes #5466
slint-ui · Jul 5, 2024 · 5dfa8d5 · 5dfa8d5
1 parent 35a6e7b
commit 5dfa8d5
Show file tree

Hide file tree

Showing 40 changed files with 169 additions and 137 deletions.
diff --git a/api/node/src/interpreter/component_compiler.rs b/api/node/src/interpreter/component_compiler.rs
@@ -106,7 +106,7 @@ impl JsComponentCompiler {
     pub fn build_from_path(&mut self, path: String) -> HashMap<String, JsComponentDefinition> {
         let r = spin_on::spin_on(self.internal.build_from_path(PathBuf::from(path)));
         self.diagnostics = r.diagnostics().collect();
-        r.component_names().filter_map(|n| Some((n.to_owned(), r.component(n)?.into()))).collect()
+        r.components().map(|c| (c.name().to_owned(), c.into())).collect()
     }
 
     /// Compile some .slint code into a ComponentDefinition
@@ -118,6 +118,6 @@ impl JsComponentCompiler {
     ) -> HashMap<String, JsComponentDefinition> {
         let r = spin_on::spin_on(self.internal.build_from_source(source_code, PathBuf::from(path)));
         self.diagnostics = r.diagnostics().collect();
-        r.component_names().filter_map(|n| Some((n.to_owned(), r.component(n)?.into()))).collect()
+        r.components().map(|c| (c.name().to_owned(), c.into())).collect()
     }
 }
diff --git a/api/rs/build/lib.rs b/api/rs/build/lib.rs
@@ -362,7 +362,7 @@ pub fn compile_with_config(
     let mut diag = BuildDiagnostics::default();
     let syntax_node = i_slint_compiler::parser::parse_file(&path, &mut diag);
 
-    if diag.has_error() {
+    if diag.has_errors() {
         let vec = diag.to_string_vec();
         diag.print();
         return Err(CompileError::CompileError(vec));
@@ -377,7 +377,7 @@ pub fn compile_with_config(
     let (doc, diag, loader) =
         spin_on::spin_on(i_slint_compiler::compile_syntax_node(syntax_node, diag, compiler_config));
 
-    if diag.has_error() {
+    if diag.has_errors() {
         let vec = diag.to_string_vec();
         diag.print();
         return Err(CompileError::CompileError(vec));

diff --git a/api/rs/macros/lib.rs b/api/rs/macros/lib.rs
@@ -388,7 +388,7 @@ pub fn slint(stream: TokenStream) -> TokenStream {
     };
     let mut diag = BuildDiagnostics::default();
     let syntax_node = parser::parse_tokens(tokens.clone(), source_file, &mut diag);
-    if diag.has_error() {
+    if diag.has_errors() {
         return diag.report_macro_diagnostic(&tokens);
     }
 
@@ -397,7 +397,7 @@ pub fn slint(stream: TokenStream) -> TokenStream {
     let (root_component, diag, loader) =
         spin_on::spin_on(compile_syntax_node(syntax_node, diag, compiler_config));
     //println!("{:#?}", tree);
-    if diag.has_error() {
+    if diag.has_errors() {
         return diag.report_macro_diagnostic(&tokens);
     }
 

diff --git a/internal/compiler/diagnostics.rs b/internal/compiler/diagnostics.rs
@@ -367,7 +367,7 @@ impl BuildDiagnostics {
     }
 
     /// Return true if there is at least one compilation error for this file
-    pub fn has_error(&self) -> bool {
+    pub fn has_errors(&self) -> bool {
         self.inner.iter().any(|diag| diag.level == DiagnosticLevel::Error)
     }
 
@@ -467,7 +467,7 @@ impl BuildDiagnostics {
         span_map: &[crate::parser::Token],
     ) -> proc_macro::TokenStream {
         let mut result = proc_macro::TokenStream::default();
-        let mut needs_error = self.has_error();
+        let mut needs_error = self.has_errors();
         self.call_diagnostics(
             &mut (),
             Some(&mut |diag| {
@@ -543,7 +543,7 @@ impl BuildDiagnostics {
     #[cfg(feature = "display-diagnostics")]
     #[must_use]
     pub fn check_and_exit_on_error(self) -> Self {
-        if self.has_error() {
+        if self.has_errors() {
             self.print();
             std::process::exit(-1);
         }
@@ -552,7 +552,7 @@ impl BuildDiagnostics {
 
     #[cfg(feature = "display-diagnostics")]
     pub fn print_warnings_and_exit_on_error(self) {
-        let has_error = self.has_error();
+        let has_error = self.has_errors();
         self.print();
         if has_error {
             std::process::exit(-1);

diff --git a/internal/compiler/lib.rs b/internal/compiler/lib.rs
@@ -54,20 +54,24 @@ pub enum EmbedResourcesKind {
     EmbedTextures,
 }
 
-#[derive(Clone, Debug, Eq, PartialEq)]
-pub enum ComponentsToGenerate {
-    /// All exported Windows.
+#[derive(Clone, Debug, Eq, PartialEq, Default)]
+#[non_exhaustive]
+pub enum ComponentSelection {
+    /// All components that inherit from Window.
     ///
-    /// When set, there will be a warning if an exported component is not a window.
-    AllExportedWindows,
+    /// Note: Components marked for export but lacking Window inheritance are not selected (this will produce a warning),
+    /// For compatibility reason, the last exported component is still selected even if it doesn't inherit Window,
+    /// and if no component is exported, the last component is selected
+    #[default]
+    ExportedWindows,
     /// The Last component (legacy for the viewer / interpreter)
 
     /// Only the last exported component is generated, regardless if this is a Window or not,
     /// (and it will be transformed in a Window)
-    LastComponent,
+    LastExported,
 
     /// The component with the given name is generated
-    ComponentWithName(String),
+    Named(String),
 }
 
 /// CompilationConfiguration allows configuring different aspects of the compiler.
@@ -120,7 +124,7 @@ pub struct CompilerConfiguration {
     /// Generate debug information for elements (ids, type names)
     pub debug_info: bool,
 
-    pub components_to_generate: ComponentsToGenerate,
+    pub components_to_generate: ComponentSelection,
 }
 
 impl CompilerConfiguration {
@@ -195,7 +199,7 @@ impl CompilerConfiguration {
             translation_domain: None,
             cpp_namespace,
             debug_info,
-            components_to_generate: ComponentsToGenerate::AllExportedWindows,
+            components_to_generate: ComponentSelection::ExportedWindows,
         }
     }
 }
@@ -244,7 +248,7 @@ pub async fn compile_syntax_node(
         &type_registry,
     );
 
-    if !diagnostics.has_error() {
+    if !diagnostics.has_errors() {
         passes::run_passes(&mut doc, &mut loader, false, &mut diagnostics).await;
     } else {
         // Don't run all the passes in case of errors because because some invariants are not met.

diff --git a/internal/compiler/load_builtins.rs b/internal/compiler/load_builtins.rs
@@ -243,7 +243,7 @@ fn compiled(
         &mut crate::lookup::LookupCtx::empty_context(type_register, &mut diag),
     )
     .maybe_convert_to(ty, &node, &mut diag);
-    if diag.has_error() {
+    if diag.has_errors() {
         let vec = diag.to_string_vec();
         #[cfg(feature = "display-diagnostics")]
         diag.print();

diff --git a/internal/compiler/object_tree.rs b/internal/compiler/object_tree.rs
@@ -31,7 +31,7 @@ macro_rules! unwrap_or_continue {
         match $e {
             Some(x) => x,
             None => {
-                debug_assert!($diag.has_error()); // error should have been reported at parsing time
+                debug_assert!($diag.has_errors()); // error should have been reported at parsing time
                 continue;
             }
         }
@@ -91,7 +91,7 @@ impl Document {
             if let Type::Struct { name, .. } = &mut ty {
                 *name = parser::identifier_text(&n.DeclaredIdentifier());
             } else {
-                assert!(diag.has_error());
+                assert!(diag.has_errors());
                 return;
             }
             local_registry.insert_type(ty.clone());
@@ -102,7 +102,7 @@ impl Document {
                             local_registry: &mut TypeRegister,
                             inner_types: &mut Vec<Type>| {
             let Some(name) = parser::identifier_text(&n.DeclaredIdentifier()) else {
-                assert!(diag.has_error());
+                assert!(diag.has_errors());
                 return;
             };
             let mut existing_names = HashSet::new();
@@ -929,7 +929,7 @@ impl Element {
             ElementType::Global
         } else if parent_type != ElementType::Error {
             // This should normally never happen because the parser does not allow for this
-            assert!(diag.has_error());
+            assert!(diag.has_errors());
             return ElementRc::default();
         } else {
             tr.empty_type()
@@ -1195,7 +1195,7 @@ impl Element {
                 .insert(name.clone(), BindingExpression::new_uncompiled(func.clone().into()).into())
                 .is_some()
             {
-                assert!(diag.has_error());
+                assert!(diag.has_errors());
             }
 
             let mut visibility = PropertyVisibility::Private;
@@ -1868,7 +1868,7 @@ pub fn type_from_node(
     } else if let Some(array_node) = node.ArrayType() {
         Type::Array(Box::new(type_from_node(array_node.Type(), diag, tr)))
     } else {
-        assert!(diag.has_error());
+        assert!(diag.has_errors());
         Type::Invalid
     }
 }
@@ -2490,7 +2490,7 @@ impl Exports {
                 let name_ident: SyntaxNode = component.DeclaredIdentifier().into();
                 let name =
                     parser::identifier_text(&component.DeclaredIdentifier()).unwrap_or_else(|| {
-                        debug_assert!(diag.has_error());
+                        debug_assert!(diag.has_errors());
                         String::new()
                     });
 
@@ -2512,7 +2512,7 @@ impl Exports {
                 })
                 .filter_map(|name_ident| {
                     let name = parser::identifier_text(&name_ident).unwrap_or_else(|| {
-                        debug_assert!(diag.has_error());
+                        debug_assert!(diag.has_errors());
                         String::new()
                     });
 

diff --git a/internal/compiler/parser-test-macro/lib.rs b/internal/compiler/parser-test-macro/lib.rs
@@ -68,7 +68,7 @@ fn generate_test(fn_name: &str, doc: &str, extra_args: usize) -> String {
             let mut diag = Default::default();
             let mut p = DefaultParser::new("{line}", &mut diag);
             {fn_name}(&mut p{follow_args});
-            let has_error = p.diags.has_error();
+            let has_error = p.diags.has_errors();
             //#[cfg(feature = "display-diagnostics")]
             //p.diags.print();
             assert!(!has_error);

diff --git a/internal/compiler/parser.rs b/internal/compiler/parser.rs
@@ -980,7 +980,7 @@ pub fn parse_expression_as_bindingexpression(
         };
         let node = rowan::SyntaxNode::new_root(p.builder.finish());
 
-        if !build_diagnostics.has_error() && token.kind() != SyntaxKind::Eof {
+        if !build_diagnostics.has_errors() && token.kind() != SyntaxKind::Eof {
             build_diagnostics.push_error_with_span(
                 format!("Expected end of string, found \"{}\"", &token.kind()),
                 crate::diagnostics::SourceLocation {

diff --git a/internal/compiler/passes.rs b/internal/compiler/passes.rs
@@ -189,7 +189,7 @@ pub async fn run_passes(
     remove_return::remove_return(doc);
 
     doc.visit_all_used_components(|component| {
-        if !diag.has_error() {
+        if !diag.has_errors() {
             // binding loop causes panics in const_propagation
             const_propagation::const_propagation(component);
         }

diff --git a/internal/compiler/passes/check_expressions.rs b/internal/compiler/passes/check_expressions.rs
@@ -20,7 +20,7 @@ fn check_expression(component: &Rc<Component>, e: &Expression, diag: &mut BuildD
     match e {
         Expression::MemberFunction { .. } => {
             // Must already have been be reported.
-            debug_assert!(diag.has_error());
+            debug_assert!(diag.has_errors());
         }
         Expression::BuiltinMacroReference(_, node) => {
             diag.push_error("Builtin function must be called".into(), node);

diff --git a/internal/compiler/passes/check_public_api.rs b/internal/compiler/passes/check_public_api.rs
@@ -8,7 +8,7 @@ use std::rc::Rc;
 use crate::diagnostics::{BuildDiagnostics, DiagnosticLevel};
 use crate::langtype::ElementType;
 use crate::object_tree::{Component, Document, ExportedName, PropertyVisibility};
-use crate::{CompilerConfiguration, ComponentsToGenerate};
+use crate::{CompilerConfiguration, ComponentSelection};
 use itertools::Either;
 
 pub fn check_public_api(
@@ -18,9 +18,7 @@ pub fn check_public_api(
 ) {
     let last = doc.last_exported_component();
 
-    if last.is_none()
-        && !matches!(&config.components_to_generate, ComponentsToGenerate::ComponentWithName(_))
-    {
+    if last.is_none() && !matches!(&config.components_to_generate, ComponentSelection::Named(_)) {
         let last_imported = doc
             .node
             .as_ref()
@@ -36,7 +34,7 @@ pub fn check_public_api(
     }
 
     match &config.components_to_generate {
-        ComponentsToGenerate::AllExportedWindows => doc.exports.retain(|export| {
+        ComponentSelection::ExportedWindows => doc.exports.retain(|export| {
             // Warn about exported non-window (and remove them from the export unless it's the last for compatibility)
             if let Either::Left(c) = &export.1 {
                 if !c.is_global() && !super::ensure_window::inherits_window(c) {
@@ -52,15 +50,15 @@ pub fn check_public_api(
             true
         }),
         // Only keep the last component if there is one
-        ComponentsToGenerate::LastComponent => doc.exports.retain(|export| {
+        ComponentSelection::LastExported => doc.exports.retain(|export| {
             if let Either::Left(c) = &export.1 {
                 c.is_global() || last.as_ref().map_or(true, |last| Rc::ptr_eq(last, c))
             } else {
                 true
             }
         }),
         // Only keep the component with the given name
-        ComponentsToGenerate::ComponentWithName(name) => {
+        ComponentSelection::Named(name) => {
             doc.exports.retain(|export| {
                 if let Either::Left(c) = &export.1 {
                     c.is_global() || &c.id == name

diff --git a/internal/compiler/passes/const_propagation.rs b/internal/compiler/passes/const_propagation.rs
@@ -222,7 +222,7 @@ export component Foo {
     );
     let (doc, diag, _) =
         spin_on::spin_on(crate::compile_syntax_node(doc_node, test_diags, compiler_config));
-    assert!(!diag.has_error());
+    assert!(!diag.has_errors());
 
     let out_binding = doc
         .inner_components

diff --git a/internal/compiler/passes/default_geometry.rs b/internal/compiler/passes/default_geometry.rs
@@ -485,7 +485,7 @@ fn test_no_property_for_100pc() {
     );
     let (doc, diag, _) =
         spin_on::spin_on(crate::compile_syntax_node(doc_node, test_diags, compiler_config));
-    assert!(!diag.has_error(), "{:?}", diag.to_string_vec());
+    assert!(!diag.has_errors(), "{:?}", diag.to_string_vec());
 
     let root_elem = doc.inner_components.last().unwrap().root_element.borrow();
 

diff --git a/internal/compiler/passes/focus_handling.rs b/internal/compiler/passes/focus_handling.rs
@@ -99,7 +99,7 @@ impl<'a> LocalFocusForwards<'a> {
             let Expression::ElementReference(focus_target) = &forward_focus_binding.expression
             else {
                 // resolve expressions pass has produced type errors
-                debug_assert!(diag.has_error());
+                debug_assert!(diag.has_errors());
                 return;
             };
 
@@ -178,7 +178,7 @@ impl<'a> LocalFocusForwards<'a> {
                     }
                     if arguments.len() != 1 {
                         assert!(
-                            self.diag.has_error(),
+                            self.diag.has_errors(),
                             "Invalid argument generated for {} call",
                             focus_function.name()
                         );

diff --git a/internal/compiler/passes/infer_aliases_types.rs b/internal/compiler/passes/infer_aliases_types.rs
@@ -71,22 +71,22 @@ fn resolve_alias(
                 Type::InferredCallback | Type::InferredProperty
             ));
             // It is still unresolved because there is an error in that component
-            assert!(diag.has_error());
+            assert!(diag.has_errors());
             return;
         }
     };
     drop(borrow_mut);
 
     let borrow = elem.borrow();
     let Some(binding) = borrow.bindings.get(prop) else {
-        assert!(diag.has_error());
+        assert!(diag.has_errors());
         return;
     };
     let nr = match &binding.borrow().expression {
         Expression::Uncompiled(node) => {
             let Some(node) = syntax_nodes::TwoWayBinding::new(node.clone()) else {
                 assert!(
-                    diag.has_error(),
+                    diag.has_errors(),
                     "The parser only avoid missing types for two way bindings"
                 );
                 return;
@@ -135,7 +135,7 @@ fn resolve_alias(
     } else if old_type == Type::InferredCallback {
         if !matches!(ty, Type::Callback { .. }) {
             if nr.is_some() && ty == Type::Invalid {
-                debug_assert!(diag.has_error());
+                debug_assert!(diag.has_errors());
             } else {
                 diag.push_error(
                     format!("Binding to callback '{}' must bind to another callback", prop),

diff --git a/internal/compiler/passes/lower_layout.rs b/internal/compiler/passes/lower_layout.rs
@@ -97,7 +97,7 @@ fn lower_element_layout(
             // We shouldn't lower layout if we have a Row in there. Unless the Row is the root of a repeated item,
             // in which case another error has been reported
             assert!(
-                diag.has_error()
+                diag.has_errors()
                     && Rc::ptr_eq(&component.root_element, elem)
                     && component
                         .parent_element