From 9c52ff9db62da9bb33afd600df57c4b78f756adf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wawrzyniec=20Urba=C5=84czyk?= Date: Thu, 23 Jul 2020 03:12:16 +0200 Subject: [PATCH] Searcher Controller has access to Executed Graph (https://github.com/enso-org/ide/pull/687) Co-authored-by: Adam Obuchowicz Original commit: https://github.com/enso-org/ide/commit/5ced6ee5bcbd778aba987799dce17f58bf2f23f8 --- gui/build/run.js | 2 +- gui/src/rust/ide/src/controller/graph.rs | 21 +++-- .../rust/ide/src/controller/graph/executed.rs | 26 +++++- gui/src/rust/ide/src/controller/searcher.rs | 83 ++++++++++++------- gui/src/rust/ide/src/executor/test_utils.rs | 4 +- gui/src/rust/ide/src/lib.rs | 5 +- .../ide/src/model/execution_context/plain.rs | 7 +- gui/src/rust/ide/src/model/module.rs | 4 +- gui/src/rust/ide/src/model/project.rs | 12 ++- gui/src/rust/ide/src/test.rs | 30 +++++++ gui/src/rust/ide/src/view/node_editor.rs | 5 -- gui/src/rust/ide/src/view/node_searcher.rs | 9 +- gui/src/rust/lib/data/src/text.rs | 7 ++ 13 files changed, 153 insertions(+), 62 deletions(-) create mode 100644 gui/src/rust/ide/src/test.rs diff --git a/gui/build/run.js b/gui/build/run.js index f05526fac2a..a1e0d072e52 100755 --- a/gui/build/run.js +++ b/gui/build/run.js @@ -127,7 +127,7 @@ commands.build.rust = async function(argv) { console.log('Checking the resulting WASM size.') let stats = fss.statSync(paths.dist.wasm.mainOptGz) - let limit = 3.58 + let limit = 3.59 let size = Math.round(100 * stats.size / 1024 / 1024) / 100 if (size > limit) { throw(`Output file size exceeds the limit (${size}MB > ${limit}MB).`) diff --git a/gui/src/rust/ide/src/controller/graph.rs b/gui/src/rust/ide/src/controller/graph.rs index af116bfa1d2..c6105f1bf8c 100644 --- a/gui/src/rust/ide/src/controller/graph.rs +++ b/gui/src/rust/ide/src/controller/graph.rs @@ -401,10 +401,11 @@ impl EndpointInfo { /// Handle providing graph controller interface. #[derive(Clone,CloneRef,Debug)] pub struct Handle { + /// Identifier of the graph accessed through this controller. + pub id : Rc, /// Model of the module which this graph belongs to. pub module : model::Module, parser : Parser, - id : Rc, logger : Logger, } @@ -764,6 +765,7 @@ pub mod tests { use ast::test_utils::expect_shape; use data::text::Index; use data::text::TextChange; + use enso_protocol::language_server::MethodPointer; use parser::Parser; use utils::test::ExpectTuple; use wasm_bindgen_test::wasm_bindgen_test; @@ -782,19 +784,20 @@ pub mod tests { /// node. pub fn new() -> Self { MockData { - module_path : model::module::Path::from_mock_module_name("Main"), - graph_id : Id::new_plain_name("main"), - project_name : "MockProject".to_string(), - code : "main = 2 + 2".to_string(), + module_path : crate::test::mock::data::module_path(), + graph_id : crate::test::mock::data::graph_id(), + project_name : crate::test::mock::data::PROJECT_NAME.to_owned(), + code : crate::test::mock::data::CODE.to_owned(), } } /// Creates a mock data with the main function being an inline definition. /// /// The single node's expression is taken as the argument. - pub fn new_inline(main_body:impl Into) -> Self { + pub fn new_inline(main_body:impl AsRef) -> Self { + let definition_name = crate::test::mock::data::DEFINITION_NAME; MockData { - code : format!("main = {}", main_body.into()), + code : format!("{} = {}",definition_name,main_body.as_ref()), ..Self::new() } } @@ -815,6 +818,10 @@ pub mod tests { let id = self.graph_id.clone(); Handle::new(logger,module,parser,id).unwrap() } + + pub fn method(&self) -> MethodPointer { + self.module_path.method_pointer(self.graph_id.to_string()) + } } impl Default for MockData { diff --git a/gui/src/rust/ide/src/controller/graph/executed.rs b/gui/src/rust/ide/src/controller/graph/executed.rs index b78ebb36a18..69978452199 100644 --- a/gui/src/rust/ide/src/controller/graph/executed.rs +++ b/gui/src/rust/ide/src/controller/graph/executed.rs @@ -199,7 +199,7 @@ impl Handle { // ============ #[cfg(test)] -mod tests { +pub mod tests { use super::*; use crate::executor::test_utils::TestWithLocalPoolExecutor; @@ -211,6 +211,30 @@ mod tests { wasm_bindgen_test_configure!(run_in_browser); + #[derive(Debug,Default)] + pub struct MockData { + pub graph : controller::graph::tests::MockData, + pub module : model::module::test::MockData, + pub ctx : model::execution_context::plain::test::MockData, + } + + impl MockData { + pub fn controller(&self) -> Handle { + let parser = parser::Parser::new_or_panic(); + let module = self.module.plain(&parser); + let method = self.graph.method(); + let mut project = model::project::MockAPI::new(); + let ctx = Rc::new(self.ctx.create()); + model::project::test::expect_parser(&mut project,&parser); + model::project::test::expect_module(&mut project,module); + model::project::test::expect_execution_ctx(&mut project,ctx); + + let project = Rc::new(project); + Handle::new(Logger::default(),project.clone_ref(),method).boxed_local().expect_ok() + } + + } + // Test that checks that value computed notification is properly relayed by the executed graph. #[wasm_bindgen_test] fn dispatching_value_computed_notification() { diff --git a/gui/src/rust/ide/src/controller/searcher.rs b/gui/src/rust/ide/src/controller/searcher.rs index 499b8dc5f53..847c2318c79 100644 --- a/gui/src/rust/ide/src/controller/searcher.rs +++ b/gui/src/rust/ide/src/controller/searcher.rs @@ -225,8 +225,7 @@ pub struct Searcher { logger : Logger, data : Rc>, notifier : notification::Publisher, - module : model::module::Path, - position : Immutable, + graph : controller::ExecutedGraph, database : Rc, language_server : Rc, parser : Parser, @@ -234,16 +233,22 @@ pub struct Searcher { impl Searcher { /// Create new Searcher Controller. - pub fn new + pub async fn new ( parent : impl AnyLogger , project : &model::Project - , module : model::module::Path - , position : TextLocation - ) -> Self { + , method : language_server::MethodPointer + ) -> FallibleResult { + let graph = controller::ExecutedGraph::new(&parent,project.clone_ref(),method).await?; + Ok(Self::new_from_graph_controller(parent,project,graph)) + } + + /// Create new Searcher Controller, when you have Executed Graph Controller handy. + pub fn new_from_graph_controller + (parent:impl AnyLogger, project:&model::Project, graph:controller::ExecutedGraph) + -> Self { + let logger = Logger::sub(parent,"Searcher Controller"); let this = Self { - module, - position : Immutable(position), - logger : Logger::sub(parent,"Searcher Controller"), + logger,graph, data : default(), notifier : default(), database : project.suggestion_db(), @@ -332,7 +337,9 @@ impl Searcher { fn reload_list(&self) { let next_completion = self.data.borrow().input.next_completion_id(); let new_suggestions = if next_completion == CompletedFragmentId::Function { - self.get_suggestion_list_from_engine(None,None); + if let Err(err) = self.get_suggestion_list_from_engine(None,None) { + error!(self.logger,"Cannot request engine for suggestions: {err}"); + } Suggestions::Loading } else { // TODO[ao] Requesting for argument. @@ -342,16 +349,21 @@ impl Searcher { } fn get_suggestion_list_from_engine - (&self, return_type:Option, tags:Option>) { - let ls = &self.language_server; - let module = self.module.file_path(); - let self_type = None; - let position = self.position.deref().into(); - let request = ls.completion(module,&position,&self_type,&return_type,&tags); - let data = self.data.clone_ref(); - let database = self.database.clone_ref(); - let logger = self.logger.clone_ref(); - let notifier = self.notifier.clone_ref(); + (&self, return_type:Option, tags:Option>) + -> FallibleResult<()> { + let ls = &self.language_server; + let graph = self.graph.graph(); + let graph_id = &*graph.id; + let module_ast = graph.module.ast(); + let file = graph.module.path().file_path(); + let self_type = None; + let def_span = double_representation::module::definition_span(&module_ast,&graph_id)?; + let position = TextLocation::convert_span(module_ast.repr(),&def_span).end.into(); + let request = ls.completion(&file,&position,&self_type,&return_type,&tags); + let data = self.data.clone_ref(); + let database = self.database.clone_ref(); + let logger = self.logger.clone_ref(); + let notifier = self.notifier.clone_ref(); executor::global::spawn(async move { info!(logger,"Requesting new suggestion list."); let response = request.await; @@ -374,10 +386,12 @@ impl Searcher { data.borrow_mut().suggestions = new_suggestions; notifier.publish(Notification::NewSuggestionList).await; }); + Ok(()) } } + // ============= // === Tests === // ============= @@ -387,8 +401,8 @@ mod test { use super::*; use crate::executor::test_utils::TestWithLocalPoolExecutor; - use crate::model::module::Path; + use controller::graph::executed::tests::MockData as ExecutedGraphMockData; use json_rpc::expect_call; use utils::test::traits::*; @@ -400,16 +414,16 @@ mod test { } impl Fixture { - fn new(client_setup:impl FnOnce(&mut language_server::MockClient)) -> Self { - let mut client = language_server::MockClient::default(); - let module_path = Path::from_mock_module_name("Test"); - client_setup(&mut client); + fn new_custom(client_setup:F) -> Self + where F : FnOnce(&mut ExecutedGraphMockData,&mut language_server::MockClient) { + let mut graph_data = controller::graph::executed::tests::MockData::default(); + let mut client = language_server::MockClient::default(); + client_setup(&mut graph_data,&mut client); let searcher = Searcher { logger : default(), data : default(), notifier : default(), - module : module_path, - position : Immutable(TextLocation::at_document_begin()), + graph : graph_data.controller(), database : default(), language_server : language_server::Connection::new_mock_rc(client), parser : Parser::new_or_panic(), @@ -438,20 +452,27 @@ mod test { let entry9 = searcher.database.get(9).unwrap(); Fixture{searcher,entry1,entry2,entry9} } + + fn new() -> Self { + Self::new_custom(|_,_| {}) + } } #[wasm_bindgen_test] fn loading_list() { let mut test = TestWithLocalPoolExecutor::set_up(); - let Fixture{searcher,entry1,entry9,..} = Fixture::new(|client| { + let Fixture{searcher,entry1,entry9,..} = Fixture::new_custom(|data,client| { let completion_response = language_server::response::Completion { results: vec![1,5,9], current_version: default(), }; + + let file_end = data.module.code.chars().count(); + let expected_location = TextLocation {line:0, column:file_end}; expect_call!(client.completion( - module = Path::from_mock_module_name("Test").file_path().clone(), - position = TextLocation::at_document_begin().into(), + module = data.module.path.file_path().clone(), + position = expected_location.into(), self_type = None, return_type = None, tag = None @@ -540,7 +561,7 @@ mod test { #[wasm_bindgen_test] fn picked_completions_list_maintaining() { - let Fixture{searcher,entry1,entry2,..} = Fixture::new(|_|{}); + let Fixture{searcher,entry1,entry2,..} = Fixture::new(); let frags_borrow = || Ref::map(searcher.data.borrow(),|d| &d.fragments_added_by_picking); // Picking first suggestion. diff --git a/gui/src/rust/ide/src/executor/test_utils.rs b/gui/src/rust/ide/src/executor/test_utils.rs index 80b2d0c2db5..b2992bffffa 100644 --- a/gui/src/rust/ide/src/executor/test_utils.rs +++ b/gui/src/rust/ide/src/executor/test_utils.rs @@ -90,6 +90,8 @@ impl TestWithLocalPoolExecutor { impl Drop for TestWithLocalPoolExecutor { fn drop(&mut self) { // We should be able to finish test. - self.expect_finished(); + if !std::thread::panicking() { + self.expect_finished(); + } } } diff --git a/gui/src/rust/ide/src/lib.rs b/gui/src/rust/ide/src/lib.rs index 05e0edc060a..1cf725c8e34 100644 --- a/gui/src/rust/ide/src/lib.rs +++ b/gui/src/rust/ide/src/lib.rs @@ -21,15 +21,16 @@ #![warn(missing_debug_implementations)] pub mod config; +pub mod constants; pub mod controller; pub mod double_representation; pub mod executor; +pub mod ide; pub mod model; pub mod notification; +pub mod test; pub mod transport; pub mod view; -pub mod constants; -pub mod ide; pub use crate::ide::IdeInitializer; diff --git a/gui/src/rust/ide/src/model/execution_context/plain.rs b/gui/src/rust/ide/src/model/execution_context/plain.rs index 360a0345272..2ec7d241dab 100644 --- a/gui/src/rust/ide/src/model/execution_context/plain.rs +++ b/gui/src/rust/ide/src/model/execution_context/plain.rs @@ -184,7 +184,6 @@ pub mod test { use super::*; use crate::double_representation::definition::DefinitionName; - use crate::constants::DEFAULT_PROJECT_NAME; #[derive(Clone,Derivative)] #[derivative(Debug)] @@ -205,9 +204,9 @@ pub mod test { pub fn new() -> MockData { MockData { context_id : model::execution_context::Id::new_v4(), - module_path : model::module::Path::from_mock_module_name("Test"), - root_definition : DefinitionName::new_plain("main"), - project_name : DEFAULT_PROJECT_NAME.to_string(), + module_path : crate::test::mock::data::module_path(), + root_definition : crate::test::mock::data::definition_name(), + project_name : crate::test::mock::data::PROJECT_NAME.to_owned(), } } diff --git a/gui/src/rust/ide/src/model/module.rs b/gui/src/rust/ide/src/model/module.rs index 8990fe3c60c..1f096e588f4 100644 --- a/gui/src/rust/ide/src/model/module.rs +++ b/gui/src/rust/ide/src/model/module.rs @@ -378,8 +378,8 @@ pub mod test { impl Default for MockData { fn default() -> Self { Self { - path : Path::from_mock_module_name("Main"), - code : "main = 2 + 2".into(), + path : crate::test::mock::data::module_path(), + code : crate::test::mock::data::CODE.to_owned(), id_map : default(), metadata : default(), } diff --git a/gui/src/rust/ide/src/model/project.rs b/gui/src/rust/ide/src/model/project.rs index f7b000f8a5b..bf1d76d262d 100644 --- a/gui/src/rust/ide/src/model/project.rs +++ b/gui/src/rust/ide/src/model/project.rs @@ -80,6 +80,8 @@ pub type Synchronized = synchronized::Project; pub mod test { use super::*; + use futures::future::ready; + /// Sets up parser expectation on the mock project. pub fn expect_parser(project:&mut MockAPI, parser:&Parser) { let parser = parser.clone_ref(); @@ -91,6 +93,14 @@ pub mod test { let module_path = module.path().clone_ref(); project.expect_module() .withf_st (move |path| path == &module_path) - .returning_st(move |_path| futures::future::ready(Ok(module.clone_ref())).boxed_local()); + .returning_st(move |_path| ready(Ok(module.clone_ref())).boxed_local()); + } + + /// Sets up module expectation on the mock project, returning a give module. + pub fn expect_execution_ctx(project:&mut MockAPI, ctx:model::ExecutionContext) { + let ctx2 = ctx.clone_ref(); + project.expect_create_execution_context() + .withf_st (move |root_definition| root_definition == &ctx.current_method()) + .returning_st(move |_root_definition| ready(Ok(ctx2.clone_ref())).boxed_local()); } } diff --git a/gui/src/rust/ide/src/test.rs b/gui/src/rust/ide/src/test.rs new file mode 100644 index 00000000000..cd96cb64e84 --- /dev/null +++ b/gui/src/rust/ide/src/test.rs @@ -0,0 +1,30 @@ +//! Module for support code for writing tests. + +//! Utilities for mocking IDE components. +#[cfg(test)] +pub mod mock { + //! Data used to create mock IDE components. + //! + //! Contains a number of constants and functions building more complex structures from them. + //! The purpose is to allow different parts of tests that mock different models using + //! consistent data. + #[allow(missing_docs)] + pub mod data { + pub const PROJECT_NAME : &str = "MockProject"; + pub const MODULE_NAME : &str = "Mock_Module"; + pub const CODE : &str = "main = 2 + 2"; + pub const DEFINITION_NAME : &str = "main"; + + pub fn module_path() -> crate::model::module::Path { + crate::model::module::Path::from_mock_module_name(MODULE_NAME) + } + + pub fn definition_name() -> crate::double_representation::definition::DefinitionName { + crate::double_representation::definition::DefinitionName::new_plain(DEFINITION_NAME) + } + + pub fn graph_id() -> crate::double_representation::graph::Id { + crate::double_representation::graph::Id::new_plain_name(DEFINITION_NAME) + } + } +} diff --git a/gui/src/rust/ide/src/view/node_editor.rs b/gui/src/rust/ide/src/view/node_editor.rs index a05fb3de897..5fe269643aa 100644 --- a/gui/src/rust/ide/src/view/node_editor.rs +++ b/gui/src/rust/ide/src/view/node_editor.rs @@ -822,11 +822,6 @@ impl NodeEditor { info!(self.logger, "Initialized."); Ok(self) } - - /// The path to the module, which graph is currently displayed. - pub fn displayed_module(&self) -> model::module::Path { - self.graph.model.controller.graph().module.path().clone_ref() - } } impl display::Object for NodeEditor { diff --git a/gui/src/rust/ide/src/view/node_searcher.rs b/gui/src/rust/ide/src/view/node_searcher.rs index 64e1ae7851a..ae5af9afddf 100644 --- a/gui/src/rust/ide/src/view/node_searcher.rs +++ b/gui/src/rust/ide/src/view/node_searcher.rs @@ -9,7 +9,6 @@ use crate::model::module::NodeMetadata; use crate::model::module::Position; use crate::view::node_editor::NodeEditor; -use data::text::TextLocation; use ensogl::data::color; use ensogl::display; use ensogl::display::Scene; @@ -89,12 +88,8 @@ impl NodeSearcher { self.display_object.add_child(&self.text_field.display_object()); self.text_field.clear_content(); self.text_field.set_focus(); - let module = self.node_editor.displayed_module(); - //TODO[ao]: Now we use some predefined location, until this task will be done: - // https://github.com/enso-org/ide/issues/653 . This code should be replaced with - // the proper Searcher view integration anyway. - let position = TextLocation { line:2, column:4 }; - let controller = controller::Searcher::new(&self.logger,&self.project,module,position); + let graph = self.node_editor.graph.controller().clone_ref(); + let controller = controller::Searcher::new_from_graph_controller(&self.logger,&self.project,graph); let logger = self.logger.clone_ref(); let weak = Rc::downgrade(&self.controller); executor::global::spawn(controller.subscribe().for_each(move |notification| { diff --git a/gui/src/rust/lib/data/src/text.rs b/gui/src/rust/lib/data/src/text.rs index bff5ffc7413..c62ab53755b 100644 --- a/gui/src/rust/lib/data/src/text.rs +++ b/gui/src/rust/lib/data/src/text.rs @@ -396,6 +396,13 @@ impl TextLocation { Self::from_index(content,range.start)..Self::from_index(content,range.end) } + /// Converts a span into a range of TextLocation. It iterates over all characters before range's + /// end. + pub fn convert_span(content:impl Str, span:&Span) -> Range { + let range = span.index..span.end(); + Self::convert_range(content,&range) + } + /// Converts a range in bytes into a range of TextLocation. It iterates over all characters /// before range's end. pub fn convert_byte_range(content:impl Str, range:&Range) -> Range {