diff --git a/gui/src/rust/ide/src/controller/searcher.rs b/gui/src/rust/ide/src/controller/searcher.rs index d27819ae458..af22fc09b63 100644 --- a/gui/src/rust/ide/src/controller/searcher.rs +++ b/gui/src/rust/ide/src/controller/searcher.rs @@ -209,6 +209,29 @@ impl ParsedInput { CompletedFragmentId::Argument {..} => TypingArgument, } } + + /// Convert the current input to Prefix Chain representation. + pub fn as_prefix_chain + (&self, parser:&Parser) -> Option> { + let parsed_pattern = parser.parse_line(&self.pattern).ok(); + let pattern_sast = parsed_pattern.map(|p| ast::Shifted::new(self.pattern_offset,p)); + // If there is an expression part of input, we add current pattern as the last argument. + if let Some(chain) = &self.expression { + let mut chain = chain.clone(); + if let Some(sast) = pattern_sast { + let prefix_id = None; + let argument = ast::prefix::Argument {sast,prefix_id}; + chain.wrapped.args.push(argument); + } + Some(chain) + // If there isn't any expression part, the pattern is the whole input. + } else if let Some(sast) = pattern_sast { + let chain = ast::prefix::Chain::from_ast_non_strict(&sast.wrapped); + Some(ast::Shifted::new(self.pattern_offset,chain)) + } else { + None + } + } } impl HasRepr for ParsedInput { @@ -312,8 +335,8 @@ pub struct FragmentAddedByPickingSuggestion { impl FragmentAddedByPickingSuggestion { /// Check if the picked fragment is still unmodified by user. fn is_still_unmodified - (&self, input:&ParsedInput, this_var:Option<&str>, current_module:&QualifiedName) -> bool { - let expected_code = &self.picked_suggestion.code_to_insert(this_var,Some(current_module)); + (&self, input:&ParsedInput, current_module:&QualifiedName) -> bool { + let expected_code = &self.picked_suggestion.code_to_insert(Some(current_module)); input.completed_fragment(self.id).contains(expected_code) } } @@ -354,10 +377,10 @@ impl Data { id : CompletedFragmentId::Function, picked_suggestion : entry }; - // When editing node, we don't have any special handling for "this" node. - // This might be revisited in the future. - let this_var = None; - fragment.is_still_unmodified(&input,this_var,¤t_module).and_option(Some(fragment)) + // This is meant to work with single function calls (without "this" argument). + // In other case we should know what the function is from the engine, as the function + // should be resolved. + fragment.is_still_unmodified(&input,¤t_module).and_option(Some(fragment)) }); let mut fragments_added_by_picking = Vec::::new(); initial_fragment.for_each(|f| fragments_added_by_picking.push(f)); @@ -475,18 +498,18 @@ impl Searcher { self.this_arg.deref().as_ref().map(|this| this.var.as_ref()) } - /// The variable name that is used for `this` argument due to the selected node. - fn this_var_for(&self, id:CompletedFragmentId) -> Option<&str> { - (id == CompletedFragmentId::Function).and_option(self.this_var()) - } - /// Code that will be inserted by expanding given suggestion at given location. /// - /// Code depends on the location, as the first fragment can introduce `this` variable access. - fn code_to_insert(&self, suggestion:&suggestion::Completion, id:CompletedFragmentId) -> String { - let var = self.this_var_for(id); + /// Code depends on the location, as the first fragment can introduce `this` variable access, + /// and then we don't want to put any module name. + fn code_to_insert + (&self, suggestion:&suggestion::Completion, loc:CompletedFragmentId) -> String { let current_module = self.module_qualified_name(); - suggestion.code_to_insert(var,Some(¤t_module)) + if loc == CompletedFragmentId::Function && self.this_arg.is_some() { + suggestion.code_to_insert_skip_module() + } else { + suggestion.code_to_insert(Some(¤t_module)) + } } /// Pick a completion suggestion. @@ -546,12 +569,11 @@ impl Searcher { /// /// False if it was modified after picking or if it wasn't picked at all. pub fn is_function_fragment_unmodified(&self) -> bool { - let this_var = self.this_var(); let current_module = self.module_qualified_name(); with(self.data.borrow(), |data| { data.fragments_added_by_picking.first().contains_if(|frag| { let is_function = frag.id == CompletedFragmentId::Function; - is_function && frag.is_still_unmodified(&data.input,this_var,¤t_module) + is_function && frag.is_still_unmodified(&data.input,¤t_module) }) }) } @@ -562,19 +584,23 @@ impl Searcher { /// expression, otherwise a new node is added. This will also add all imports required by /// picked suggestions. pub fn commit_node(&self) -> FallibleResult { - let data_borrowed = self.data.borrow(); - let expression = data_borrowed.input.repr(); + let input_chain = self.data.borrow().input.as_prefix_chain(&self.parser); + + let expression = match (self.this_var(),input_chain) { + (Some(this_var),Some(input)) => + apply_this_argument(this_var,&input.wrapped.into_ast()).repr(), + (None,Some(input)) => input.wrapped.into_ast().repr(), + (_,None) => "".to_owned(), + }; let intended_method = self.intended_method(); let id = match *self.mode { Mode::NewNode {position} => { - let mut new_node = NewNodeInfo::new_pushed_back(expression); - new_node.metadata = Some(NodeMetadata {position,intended_method}); - let graph = self.graph.graph(); - if self.is_function_fragment_unmodified() { - if let Some(this) = self.this_arg.deref().as_ref() { - this.introduce_pattern(graph.clone_ref())?; - } + let mut new_node = NewNodeInfo::new_pushed_back(expression); + new_node.metadata = Some(NodeMetadata {position,intended_method}); + let graph = self.graph.graph(); + if let Some(this) = self.this_arg.deref().as_ref() { + this.introduce_pattern(graph.clone_ref())?; } graph.add_node(new_node)? }, @@ -595,10 +621,9 @@ impl Searcher { let data = data.deref_mut(); let input = &data.input; let current_module = self.module_qualified_name(); - data.fragments_added_by_picking.drain_filter(|frag| { - let this = self.this_var_for(frag.id); - !frag.is_still_unmodified(input,this,¤t_module) - }); + data.fragments_added_by_picking.drain_filter( + |frag| !frag.is_still_unmodified(input,¤t_module) + ); } fn add_required_imports(&self) -> FallibleResult { @@ -661,6 +686,7 @@ impl Searcher { self.possible_function_calls() }; suggestions.into_iter().filter_map(|suggestion| { + let arg_index = arg_index + if self.this_arg.is_some() { 1 } else { 0 }; let arg_index = arg_index + if self.has_this_argument() { 1 } else { 0 }; let parameter = suggestion.arguments.get(arg_index)?; Some(parameter.repr_type.clone()) @@ -825,6 +851,36 @@ impl SimpleFunctionCall { } } +fn apply_this_argument(this_var:&str, ast:&Ast) -> Ast { + if let Ok(opr) = ast::known::Opr::try_from(ast) { + let shape = ast::SectionLeft { + arg: Ast::var(this_var), + off: 1, + opr: opr.into() + }; + Ast::new(shape,None) + } else if let Some(mut infix_chain) = ast::opr::Chain::try_new(ast) { + if let Some(ref mut target) = &mut infix_chain.target { + target.arg = apply_this_argument(this_var,&target.arg); + } else { + infix_chain.target = Some(ast::opr::ArgWithOffset {arg:Ast::var(this_var), offset:1}); + } + infix_chain.into_ast() + } else if let Some(mut prefix_chain) = ast::prefix::Chain::from_ast(ast) { + prefix_chain.func = apply_this_argument(this_var, &prefix_chain.func); + prefix_chain.into_ast() + } else { + let shape = ast::Infix { + larg : Ast::var(this_var), + loff : 0, + opr : Ast::opr(ast::opr::predefined::ACCESS), + roff : 0, + rarg : ast.clone_ref(), + }; + Ast::new(shape,None) + } +} + // ============= @@ -1039,8 +1095,7 @@ pub mod test { /// not immediately presented; /// 2) instead the searcher model obtains the type information for the selected node and uses it /// to query Language Server for the suggestion list; - /// 3) the first (and only the first) picked completion gets the selected node variable's - /// access prepended. + /// 3) The query for argument type takes the this-argument presence into consideration. #[wasm_bindgen_test] fn loading_list_w_self() { let mock_type = crate::test::mock::data::TYPE_NAME; @@ -1052,27 +1107,29 @@ pub mod test { node_line:&'static str, /// If the searcher should enter "connect to this" mode at all and wait for type info. sets_this:bool, - /// Expected input after accepting "entry1" twice. `{}` will expand to the entry's - /// function name. - expected_input:&'static str, }; let cases = [ - Case {node_line:"2+2", sets_this:true, expected_input:"sum1.{} {} "}, - Case {node_line:"the_sum = 2 + 2", sets_this:true, expected_input:"the_sum.{} {} "}, - Case {node_line:"[x,y] = 2 + 2", sets_this:false, expected_input:"{} {} "}, + Case {node_line:"2+2", sets_this:true }, + Case {node_line:"the_sum = 2 + 2", sets_this:true }, + Case {node_line:"[x,y] = 2 + 2", sets_this:false}, ]; for case in &cases { - let Fixture { mut test, searcher, entry1, .. } = Fixture::new_custom(|data,client| { + let Fixture { mut test, searcher, entry1, entry9,.. } = Fixture::new_custom(|data,client| { data.change_main_body(case.node_line); data.selected_node = true; // We expect following calls: // 1) for the function - with the "this" filled (if the test case says so); // 2) for subsequent completions - without "this" data.expect_completion(client,case.sets_this.as_some(mock_type),None,&[1,5,9]); - data.expect_completion(client,None,None,&[1,5,9]); - data.expect_completion(client,None,None,&[1,5,9]); + // If we are about to add the self type, then we expect the second argument first, + // and then none. Otherwise we expect all arguments starting from the first. + let expected_types = if case.sets_this {[Some("Number"),None ]} + else {[Some("Text") ,Some("Number")]}; + + data.expect_completion(client,None,expected_types[0],&[1,5,9]); + data.expect_completion(client,None,expected_types[1],&[1,5,9]); }); searcher.reload_list(); @@ -1092,9 +1149,9 @@ pub mod test { test.run_until_stalled(); assert!(!searcher.suggestions().is_loading()); + searcher.pick_completion(entry9.clone_ref()).unwrap(); searcher.pick_completion(entry1.clone_ref()).unwrap(); - searcher.pick_completion(entry1.clone_ref()).unwrap(); - let expected_input = case.expected_input.replace("{}",&entry1.name); + let expected_input = format!("{} {} ",entry9.name,entry1.name); assert_eq!(searcher.data.borrow().input.repr(),expected_input); } } @@ -1332,6 +1389,41 @@ pub mod test { assert!(Rc::ptr_eq(&arg.picked_suggestion,&entry2)); } + #[wasm_bindgen_test] + fn applying_this_var() { + #[derive(Copy,Clone,Debug)] + struct Case { + before : &'static str, + after : &'static str, + } + + impl Case { + fn new(before:&'static str, after:&'static str) -> Self { + Case{before,after} + } + + fn run(&self) { + let parser = Parser::new_or_panic(); + let ast = parser.parse_line(self.before).unwrap(); + let new_ast = apply_this_argument("foo",&ast); + assert_eq!(new_ast.repr(),self.after,"Case {:?} failed: {:?}",self,ast); + } + } + + let cases = + [ Case::new("bar", "foo.bar") + , Case::new("bar.baz", "foo.bar.baz") + , Case::new("bar baz", "foo.bar baz") + , Case::new("+ 2", "foo + 2") + , Case::new("+ 2 + 3", "foo + 2 + 3") + , Case::new("+ 2 - 3", "foo + 2 - 3") + , Case::new("+ bar baz", "foo + bar baz") + , Case::new("map x-> x.characters.length", "foo.map x-> x.characters.length") + ]; + + for case in &cases { case.run(); } + } + #[wasm_bindgen_test] fn adding_node_introducing_this_var() { struct Case { @@ -1351,29 +1443,21 @@ pub mod test { } } - let cases = vec![ - // No need to introduce variable name, as the input was manually written, not picked. - Case::new("2 + 2",&["2 + 2","testFunction1"], |f| { - f.searcher.set_input("testFunction1 ".to_owned()).unwrap(); - }), - // Need to introduce variable name, as the completion was picked. + // Completion was picked. Case::new("2 + 2",&["sum1 = 2 + 2","sum1.testFunction1"], |f| { f.searcher.pick_completion(f.entry1.clone()).unwrap(); }), - // No need to introduce variable name, as the input was modified after picking. - Case::new("2 + 2",&["2 + 2","var.testFunction1"], |f| { + // The input was manually written (not picked). + Case::new("2 + 2",&["sum1 = 2 + 2","sum1.testFunction1"], |f| { + f.searcher.set_input("testFunction1 ".to_owned()).unwrap(); + }), + // Completion was picked and edited. + Case::new("2 + 2",&["sum1 = 2 + 2","sum1.var.testFunction1"], |f| { f.searcher.pick_completion(f.entry1.clone()).unwrap(); let new_parsed_input = ParsedInput::new("var.testFunction1",&f.searcher.parser); f.searcher.data.borrow_mut().input = new_parsed_input.unwrap(); }), - // Need to introduce variable name, as the completion was picked and edit was no-op. - Case::new("2 + 2",&["sum1 = 2 + 2","sum1.testFunction1"], |f| { - f.searcher.pick_completion(f.entry1.clone()).unwrap(); - // TODO [mwu] is this ok that the trailing space is actually required for this? - let new_parsed_input = ParsedInput::new("sum1.testFunction1 ",&f.searcher.parser); - f.searcher.data.borrow_mut().input = dbg!(new_parsed_input.unwrap()); - }), // Variable name already present, need to use it. And not break it. Case::new("my_var = 2 + 2",&["my_var = 2 + 2","my_var.testFunction1"], |f| { f.searcher.pick_completion(f.entry1.clone()).unwrap(); diff --git a/gui/src/rust/ide/src/controller/searcher/suggestion.rs b/gui/src/rust/ide/src/controller/searcher/suggestion.rs index 74e3f52988e..e906740dc96 100644 --- a/gui/src/rust/ide/src/controller/searcher/suggestion.rs +++ b/gui/src/rust/ide/src/controller/searcher/suggestion.rs @@ -23,7 +23,11 @@ impl Suggestion { /// The suggestion caption (suggested function name, or action name, etc.). pub fn caption(&self) -> String { match self { - Self::Completion(completion) => completion.code_to_insert(None,None) + Self::Completion(completion) => if let Some(self_type) = completion.self_type.as_ref() { + format!("{}.{}",self_type,completion.name) + } else { + completion.name.clone() + } } } } diff --git a/gui/src/rust/ide/src/model/suggestion_database.rs b/gui/src/rust/ide/src/model/suggestion_database.rs index bf62c202d9c..5285e552520 100644 --- a/gui/src/rust/ide/src/model/suggestion_database.rs +++ b/gui/src/rust/ide/src/model/suggestion_database.rs @@ -116,20 +116,23 @@ impl Entry { } /// Returns the code which should be inserted to Searcher input when suggestion is picked. - pub fn code_to_insert - (&self, this_var:Option<&str>, current_module:Option<&QualifiedName>) -> String { + pub fn code_to_insert(&self, current_module:Option<&QualifiedName>) -> String { let module_name = self.module.name(); if self.has_self_type(&module_name) { let module_var = if current_module.contains(&&self.module) {keywords::HERE} else {module_name}; - format!("{}.{}",this_var.unwrap_or(module_var),self.name) - } else if let Some(this_var) = this_var { - format!("{}.{}",this_var,self.name) + format!("{}.{}",module_var,self.name) } else { self.name.clone() } } + /// Returns the code which should be inserted to Searcher input when suggestion is picked, + /// omitting module name. + pub fn code_to_insert_skip_module(&self) -> String { + self.name.clone() + } + /// Return the Method Id of suggested method. /// /// Returns none, if this is not suggestion for a method. @@ -582,41 +585,20 @@ mod test { ..method_entry.clone() }; - let this_var = None; let current_module = None; - assert_eq!(atom_entry.code_to_insert(this_var,current_module) , "Atom"); - assert_eq!(method_entry.code_to_insert(this_var,current_module) , "method"); - assert_eq!(module_method_entry.code_to_insert(this_var,current_module), "Main.moduleMethod"); + assert_eq!(atom_entry.code_to_insert(current_module) , "Atom"); + assert_eq!(method_entry.code_to_insert(current_module) , "method"); + assert_eq!(module_method_entry.code_to_insert(current_module), "Main.moduleMethod"); - let this_var = Some("var"); - let current_module = None; - assert_eq!(atom_entry.code_to_insert(this_var,current_module) , "var.Atom"); - assert_eq!(method_entry.code_to_insert(this_var,current_module) , "var.method"); - assert_eq!(module_method_entry.code_to_insert(this_var,current_module), "var.moduleMethod"); - - let this_var = None; let current_module = Some(&module); - assert_eq!(atom_entry.code_to_insert(this_var,current_module) , "Atom"); - assert_eq!(method_entry.code_to_insert(this_var,current_module) , "method"); - assert_eq!(module_method_entry.code_to_insert(this_var,current_module), "here.moduleMethod"); + assert_eq!(atom_entry.code_to_insert(current_module) , "Atom"); + assert_eq!(method_entry.code_to_insert(current_module) , "method"); + assert_eq!(module_method_entry.code_to_insert(current_module), "here.moduleMethod"); - let this_var = Some("var"); - let current_module = Some(&module); - assert_eq!(atom_entry.code_to_insert(this_var,current_module) , "var.Atom"); - assert_eq!(method_entry.code_to_insert(this_var,current_module) , "var.method"); - assert_eq!(module_method_entry.code_to_insert(this_var,current_module), "var.moduleMethod"); - - let this_var = None; let current_module = Some(&another_module); - assert_eq!(atom_entry.code_to_insert(this_var,current_module) , "Atom"); - assert_eq!(method_entry.code_to_insert(this_var,current_module) , "method"); - assert_eq!(module_method_entry.code_to_insert(this_var,current_module), "Main.moduleMethod"); - - let this_var = Some("var"); - let current_module = Some(&another_module); - assert_eq!(atom_entry.code_to_insert(this_var,current_module) , "var.Atom"); - assert_eq!(method_entry.code_to_insert(this_var,current_module) , "var.method"); - assert_eq!(module_method_entry.code_to_insert(this_var,current_module), "var.moduleMethod"); + assert_eq!(atom_entry.code_to_insert(current_module) , "Atom"); + assert_eq!(method_entry.code_to_insert(current_module) , "method"); + assert_eq!(module_method_entry.code_to_insert(current_module), "Main.moduleMethod"); } #[test] diff --git a/gui/src/rust/ide/src/view/integration.rs b/gui/src/rust/ide/src/view/integration.rs index 14a3db3343c..747361945da 100644 --- a/gui/src/rust/ide/src/view/integration.rs +++ b/gui/src/rust/ide/src/view/integration.rs @@ -1133,7 +1133,7 @@ impl DataProviderForView { EntryKind::Local => "Local variable", EntryKind::Method => "Method", }; - format!("{} `{}`\n\nNo documentation available", title,suggestion.code_to_insert(None,None)) + format!("{} `{}`\n\nNo documentation available", title,suggestion.code_to_insert(None)) } }