From 568fa31b49dc2935e57a22d773df73c2b351faa3 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Wed, 22 Jul 2020 14:37:57 +0200 Subject: [PATCH] Suggestions fixes (https://github.com/enso-org/ide/pull/685) Fixes all discrepancies between IDE and Engine since Engine delivered implementation of suggestions. Original commit: https://github.com/enso-org/ide/commit/feca0b72d7cb1af54877b6b826b114d722e853e3 --- .../lib/enso-protocol/src/language_server.rs | 2 +- .../src/language_server/types.rs | 55 ++++++----- gui/src/rust/ide/src/constants.rs | 9 ++ gui/src/rust/ide/src/controller/searcher.rs | 12 +-- .../ide/src/double_representation/graph.rs | 4 +- .../ide/src/model/project/synchronized.rs | 6 +- .../rust/ide/src/model/suggestion_database.rs | 97 +++++++++++-------- 7 files changed, 110 insertions(+), 75 deletions(-) diff --git a/gui/src/rust/ide/lib/enso-protocol/src/language_server.rs b/gui/src/rust/ide/lib/enso-protocol/src/language_server.rs index 50a2d45fb3a..8d94430d1b3 100644 --- a/gui/src/rust/ide/lib/enso-protocol/src/language_server.rs +++ b/gui/src/rust/ide/lib/enso-protocol/src/language_server.rs @@ -151,7 +151,7 @@ trait API { #[MethodInput=CompletionInput,rpc_name="search/completion"] fn completion ( &self - , module : String + , file : Path , position : Position , self_type : Option , return_type : Option diff --git a/gui/src/rust/ide/lib/enso-protocol/src/language_server/types.rs b/gui/src/rust/ide/lib/enso-protocol/src/language_server/types.rs index 72cb53bc5f6..ea60467e92b 100644 --- a/gui/src/rust/ide/lib/enso-protocol/src/language_server/types.rs +++ b/gui/src/rust/ide/lib/enso-protocol/src/language_server/types.rs @@ -123,8 +123,8 @@ pub enum Notification { ExecutionFailed(ExecutionFailed), /// Sent from server to the client to inform abouth the change in the suggestions database. - #[serde(rename = "search/suggestionsDatabaseUpdate")] - SuggestionDatabaseUpdate(SuggestionDatabaseUpdateEvent), + #[serde(rename = "search/suggestionsDatabaseUpdates")] + SuggestionDatabaseUpdates(SuggestionDatabaseUpdatesEvent), } /// Sent from the server to the client to inform about new information for certain expressions @@ -511,9 +511,8 @@ pub type SuggestionsDatabaseVersion = usize; pub struct SuggestionEntryArgument { /// The argument name. pub name: String, - /// The arguement type. String 'Any' is used to specify generic types. - #[serde(rename = "type")] // To avoid collision with the `type` keyword. - pub arg_type: String, + /// The argument type. String 'Any' is used to specify generic types. + pub repr_type: String, /// Indicates whether the argument is lazy. pub is_suspended: bool, /// Optional default value. @@ -525,8 +524,8 @@ pub struct SuggestionEntryArgument { #[serde(rename_all="camelCase")] #[allow(missing_docs)] pub struct SuggestionEntryScope { - pub start : usize, - pub end : usize, + pub start : Position, + pub end : Position, } /// A type of suggestion entry. @@ -537,17 +536,20 @@ pub enum SuggestionEntryType {Atom,Method,Function,Local} /// A Suggestion Entry. #[derive(Hash, Debug, Clone, PartialEq, Eq,Serialize,Deserialize)] -#[serde(rename_all="camelCase")] #[allow(missing_docs)] +#[serde(tag="type")] +#[serde(rename_all="camelCase")] pub enum SuggestionEntry { - SuggestionEntryAtom { + #[serde(rename_all="camelCase")] + Atom { name : String, module : String, arguments : Vec, return_type : String, documentation : Option, }, - SuggestionEntryMethod { + #[serde(rename_all="camelCase")] + Method { name : String, module : String, arguments : Vec, @@ -555,14 +557,16 @@ pub enum SuggestionEntry { return_type : String, documentation : Option, }, - SuggestionEntryFunction { + #[serde(rename_all="camelCase")] + Function { name : String, module : String, arguments : Vec, return_type : String, scope : SuggestionEntryScope, }, - SuggestionEntryLocal { + #[serde(rename_all="camelCase")] + Local { name : String, module : String, return_type : String, @@ -574,10 +578,10 @@ impl SuggestionEntry { /// Get name of the suggested entity. pub fn name(&self) -> &String { match self { - Self::SuggestionEntryAtom {name,..} => name, - Self::SuggestionEntryFunction {name,..} => name, - Self::SuggestionEntryLocal {name,..} => name, - Self::SuggestionEntryMethod {name,..} => name, + Self::Atom {name,..} => name, + Self::Function {name,..} => name, + Self::Local {name,..} => name, + Self::Method {name,..} => name, } } } @@ -597,24 +601,31 @@ pub struct SuggestionsDatabaseEntry { pub enum SuggestionsDatabaseUpdateKind {Add,Update,Delete} /// The update of the suggestions database. -#[derive(Hash, Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all="camelCase")] +#[derive(Hash,Debug,Clone,PartialEq,Eq,Serialize,Deserialize)] #[allow(missing_docs)] +#[serde(tag="type")] pub enum SuggestionsDatabaseUpdate { + #[serde(rename_all="camelCase")] Add { - id : SuggestionEntryId, - entry : SuggestionEntry, + id : SuggestionEntryId, + suggestion : SuggestionEntry, }, + #[serde(rename_all="camelCase")] Remove { - id : SuggestionEntryId + id : SuggestionEntryId, }, + #[serde(rename_all="camelCase")] + Modify { + id : SuggestionEntryId, + return_type : String, + } } /// Notification about change in the suggestions database. #[derive(Hash,Debug,Clone,PartialEq,Eq,Serialize,Deserialize)] #[serde(rename_all="camelCase")] #[allow(missing_docs)] -pub struct SuggestionDatabaseUpdateEvent { +pub struct SuggestionDatabaseUpdatesEvent { pub updates : Vec, pub current_version : SuggestionsDatabaseVersion, } diff --git a/gui/src/rust/ide/src/constants.rs b/gui/src/rust/ide/src/constants.rs index 0b6d55d0b8e..adf7412da63 100644 --- a/gui/src/rust/ide/src/constants.rs +++ b/gui/src/rust/ide/src/constants.rs @@ -20,3 +20,12 @@ pub const DEFAULT_PROJECT_NAME:&str = "Unnamed"; /// Visualization folder where IDE can look for user-defined visualizations per project. pub const VISUALIZATION_DIRECTORY:&str = "visualization"; + +/// A module with language-specific constants. +pub mod keywords { + /// A keyword indicating current module. + pub const HERE:&str = "here"; + + /// The "void" atom returned by function meant to not return any argument. + pub const NOTHING:&str = "Nothing"; +} diff --git a/gui/src/rust/ide/src/controller/searcher.rs b/gui/src/rust/ide/src/controller/searcher.rs index 179f1116741..499b8dc5f53 100644 --- a/gui/src/rust/ide/src/controller/searcher.rs +++ b/gui/src/rust/ide/src/controller/searcher.rs @@ -121,7 +121,7 @@ pub struct ParsedInput { } impl ParsedInput { - /// Contructor from the plain input. + /// Constructor from the plain input. fn new(mut input:String, parser:&Parser) -> FallibleResult { let leading_spaces = input.chars().take_while(|c| *c == ' ').count(); // To properly guess what is "still typed argument" we simulate type of one letter by user. @@ -225,7 +225,7 @@ pub struct Searcher { logger : Logger, data : Rc>, notifier : notification::Publisher, - module : Rc, + module : model::module::Path, position : Immutable, database : Rc, language_server : Rc, @@ -241,11 +241,11 @@ impl Searcher { , position : TextLocation ) -> Self { let this = Self { + module, position : Immutable(position), logger : Logger::sub(parent,"Searcher Controller"), data : default(), notifier : default(), - module : Rc::new(project.qualified_module_name(&module)), database : project.suggestion_db(), language_server : project.json_rpc(), parser : project.parser(), @@ -344,7 +344,7 @@ impl Searcher { fn get_suggestion_list_from_engine (&self, return_type:Option, tags:Option>) { let ls = &self.language_server; - let module = self.module.as_ref(); + 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); @@ -408,7 +408,7 @@ mod test { logger : default(), data : default(), notifier : default(), - module : Rc::new(module_path.qualified_module_name("Test")), + module : module_path, position : Immutable(TextLocation::at_document_begin()), database : default(), language_server : language_server::Connection::new_mock_rc(client), @@ -450,7 +450,7 @@ mod test { current_version: default(), }; expect_call!(client.completion( - module = "Test.Test".to_string(), + module = Path::from_mock_module_name("Test").file_path().clone(), position = TextLocation::at_document_begin().into(), self_type = None, return_type = None, diff --git a/gui/src/rust/ide/src/double_representation/graph.rs b/gui/src/rust/ide/src/double_representation/graph.rs index 4a93b51f6c0..46bf11031b7 100644 --- a/gui/src/rust/ide/src/double_representation/graph.rs +++ b/gui/src/rust/ide/src/double_representation/graph.rs @@ -107,7 +107,7 @@ impl GraphInfo { /// After removing last node, we want to insert a placeholder value for definition value. /// This defines its AST. Currently it is just `Nothing`. pub fn empty_graph_body() -> Ast { - Ast::cons("Nothing").with_new_id() + Ast::cons(constants::keywords::NOTHING).with_new_id() } /// Removes the node from graph. @@ -430,7 +430,7 @@ main = println!("zz"); let (node,) = graph.nodes().expect_tuple(); - assert_eq!(node.expression().repr(), "Nothing"); + assert_eq!(node.expression().repr(), constants::keywords::NOTHING); graph.expect_code("main = Nothing"); } diff --git a/gui/src/rust/ide/src/model/project/synchronized.rs b/gui/src/rust/ide/src/model/project/synchronized.rs index 1e70152f7db..6ae8a9f0eb3 100644 --- a/gui/src/rust/ide/src/model/project/synchronized.rs +++ b/gui/src/rust/ide/src/model/project/synchronized.rs @@ -252,7 +252,11 @@ impl Project { execution context being already dropped."); } } - Event::Notification(Notification::SuggestionDatabaseUpdate(update)) => { + Event::Notification(Notification::ExecutionFailed(update)) => { + error!(logger,"Execution failed in context {update.context_id}. Error: \ + {update.message}."); + } + Event::Notification(Notification::SuggestionDatabaseUpdates(update)) => { if let Some(suggestion_db) = weak_suggestion_db.upgrade() { suggestion_db.apply_update_event(update); } diff --git a/gui/src/rust/ide/src/model/suggestion_database.rs b/gui/src/rust/ide/src/model/suggestion_database.rs index 4ee12ffe14a..ba42a59db91 100644 --- a/gui/src/rust/ide/src/model/suggestion_database.rs +++ b/gui/src/rust/ide/src/model/suggestion_database.rs @@ -6,7 +6,7 @@ use crate::double_representation::module::QualifiedName; use enso_protocol::language_server; use language_server::types::SuggestionsDatabaseVersion; -use language_server::types::SuggestionDatabaseUpdateEvent; +use language_server::types::SuggestionDatabaseUpdatesEvent; pub use language_server::types::SuggestionEntryArgument as Argument; pub use language_server::types::SuggestionEntryId as EntryId; @@ -49,37 +49,33 @@ impl Entry { pub fn from_ls_entry(entry:language_server::types::SuggestionEntry) -> FallibleResult { use language_server::types::SuggestionEntry::*; let this = match entry { - SuggestionEntryAtom {name,module,arguments,return_type,documentation} => - Self { - name,arguments,return_type,documentation, - module : module.try_into()?, - self_type : None, - kind : EntryKind::Atom, - }, - SuggestionEntryMethod {name,module,arguments,self_type,return_type,documentation} => - Self { - name,arguments,return_type,documentation, - module : module.try_into()?, - self_type : Some(self_type), - kind : EntryKind::Method, - }, - SuggestionEntryFunction {name,module,arguments,return_type,..} => - Self { - name,arguments,return_type, - module : module.try_into()?, - self_type : None, - documentation : default(), - kind : EntryKind::Function, - }, - SuggestionEntryLocal {name,module,return_type,..} => - Self { - name,return_type, - arguments : default(), - module : module.try_into()?, - self_type : None, - documentation : default(), - kind : EntryKind::Local, - }, + Atom {name,module,arguments,return_type,documentation} => Self { + name,arguments,return_type,documentation, + module : module.try_into()?, + self_type : None, + kind : EntryKind::Atom, + }, + Method {name,module,arguments,self_type,return_type,documentation} => Self { + name,arguments,return_type,documentation, + module : module.try_into()?, + self_type : Some(self_type), + kind : EntryKind::Method, + }, + Function {name,module,arguments,return_type,..} => Self { + name,arguments,return_type, + module : module.try_into()?, + self_type : None, + documentation : default(), + kind : EntryKind::Function, + }, + Local {name,module,return_type,..} => Self { + name,return_type, + arguments : default(), + module : module.try_into()?, + self_type : None, + documentation : default(), + kind : EntryKind::Local, + }, }; Ok(this) } @@ -88,7 +84,9 @@ impl Entry { pub fn code_to_insert(&self) -> String { let module = self.module.name(); if self.self_type.as_ref().contains(&module) { - iformat!("{module}.{self.name}") + format!("{}.{}",module,self.name) + } else if self.self_type.as_ref().contains(&constants::keywords::HERE) { + format!("{}.{}",constants::keywords::HERE,self.name) } else { self.name.clone() } @@ -158,15 +156,28 @@ impl SuggestionDatabase { } /// Apply the update event to the database. - pub fn apply_update_event(&self, event:SuggestionDatabaseUpdateEvent) { + pub fn apply_update_event(&self, event:SuggestionDatabaseUpdatesEvent) { for update in event.updates { let mut entries = self.entries.borrow_mut(); match update { - Update::Add {id,entry} => match entry.try_into() { + Update::Add {id,suggestion} => match suggestion.try_into() { Ok(entry) => { entries.insert(id,Rc::new(entry)); }, Err(err) => { error!(self.logger, "Discarding update for {id}: {err}") }, }, - Update::Remove {id} => { entries.remove(&id); }, + Update::Remove {id} => { + let removed = entries.remove(&id); + if removed.is_none() { + error!(self.logger, "Received Remove event for nonexistent id: {id}"); + } + }, + Update::Modify {id,return_type} => { + if let Some(old_entry) = entries.get(&id) { + let new_entry = Entry {return_type,..old_entry.deref().clone()}; + entries.insert(id,Rc::new(new_entry)); + } else { + error!(self.logger, "Received Modify event for nonexistent id: {id}"); + } + } }; } self.version.set(event.current_version); @@ -241,7 +252,7 @@ mod test { assert_eq!(db.version.get() , 123); // Non-empty db - let entry = language_server::types::SuggestionEntry::SuggestionEntryAtom { + let entry = language_server::types::SuggestionEntry::Atom { name : "TextAtom".to_string(), module : "TestProject.TestModule".to_string(), arguments : vec![], @@ -261,21 +272,21 @@ mod test { #[test] fn applying_update() { - let entry1 = language_server::types::SuggestionEntry::SuggestionEntryAtom { + let entry1 = language_server::types::SuggestionEntry::Atom { name : "Entry1".to_string(), module : "TestProject.TestModule".to_string(), arguments : vec![], return_type : "TestAtom".to_string(), documentation : None }; - let entry2 = language_server::types::SuggestionEntry::SuggestionEntryAtom { + let entry2 = language_server::types::SuggestionEntry::Atom { name : "Entry2".to_string(), module : "TestProject.TestModule".to_string(), arguments : vec![], return_type : "TestAtom".to_string(), documentation : None }; - let new_entry2 = language_server::types::SuggestionEntry::SuggestionEntryAtom { + let new_entry2 = language_server::types::SuggestionEntry::Atom { name : "NewEntry2".to_string(), module : "TestProject.TestModule".to_string(), arguments : vec![], @@ -293,7 +304,7 @@ mod test { // Remove let remove_update = Update::Remove {id:2}; - let update = SuggestionDatabaseUpdateEvent { + let update = SuggestionDatabaseUpdatesEvent { updates : vec![remove_update], current_version : 2 }; @@ -302,8 +313,8 @@ mod test { assert_eq!(db.version.get(), 2 ); // Add - let add_update = Update::Add {id:2, entry:new_entry2}; - let update = SuggestionDatabaseUpdateEvent { + let add_update = Update::Add {id:2, suggestion:new_entry2}; + let update = SuggestionDatabaseUpdatesEvent { updates : vec![add_update], current_version : 3, };