From 1a9850404d2cc02efd2ba6290e4f1ce7ba8d33e6 Mon Sep 17 00:00:00 2001 From: mwu Date: Thu, 27 Aug 2020 19:57:21 +0200 Subject: [PATCH 01/34] [wip] A new kind of an empty node placeholder that represents known function arguments. --- src/rust/ide/lib/ast/impl/src/prefix.rs | 60 ++++++++++++++++++ src/rust/ide/lib/span-tree/src/action.rs | 39 +++++++++++- src/rust/ide/lib/span-tree/src/builder.rs | 2 + src/rust/ide/lib/span-tree/src/generate.rs | 63 ++++++++++++++++++- src/rust/ide/lib/span-tree/src/lib.rs | 33 +++++++++- src/rust/ide/lib/span-tree/src/node.rs | 10 ++- src/rust/ide/src/controller/graph.rs | 6 ++ src/rust/ide/src/controller/graph/executed.rs | 11 ++++ 8 files changed, 217 insertions(+), 7 deletions(-) diff --git a/src/rust/ide/lib/ast/impl/src/prefix.rs b/src/rust/ide/lib/ast/impl/src/prefix.rs index 0b79fe210e..ee897dda65 100644 --- a/src/rust/ide/lib/ast/impl/src/prefix.rs +++ b/src/rust/ide/lib/ast/impl/src/prefix.rs @@ -30,12 +30,21 @@ pub struct Argument { pub prefix_id : Option, } +impl Argument { + fn new_blank(offset:usize, prefix_id:Option) -> Self { + let sast = Shifted::new(offset,Ast::blank()); + Self {sast,prefix_id} + } +} + impl HasTokens for Argument { fn feed_to(&self, consumer: &mut impl TokenConsumer) { self.sast.feed_to(consumer) } } + + // ==================== // === Prefix Chain === // ==================== @@ -156,6 +165,19 @@ impl Chain { } self.func } + + /// Insert argument at given position in the prefix chain. If index is out of bounds, + /// additional blank `_` arguments will be placed. + pub fn insert_arg(&mut self, index:usize, argument:Argument) { + if let Some(blanks_to_add) = index.checked_sub(self.args.len()) { + let make_blank = || { + let prefix_id = argument.prefix_id.map(|_| Id::new_v4()); + Argument::new_blank(argument.sast.off,prefix_id) + }; + self.args.extend(std::iter::repeat_with(make_blank).take(blanks_to_add)); + } + self.args.insert(index,argument); + } } impl HasTokens for Chain { @@ -209,5 +231,43 @@ mod tests { assert_eq!(chain.into_ast().repr(), "a b c"); } + #[test] + fn inserting_arg() { + let a = Ast::var("a"); + let b = Ast::var("b"); + let c = Ast::var("c"); + let chain = Chain::new(a,vec![b,c]); + assert_eq!(chain.repr(), "a b c"); + + let arg = |text:&str| Argument { + prefix_id : None, + sast : Shifted::new(1,Ast::var(text)), + }; + + { + let mut chain = chain.clone(); + chain.insert_arg(0, arg("arg")); + assert_eq!(chain.repr(), "a arg b c"); + } + + { + let mut chain = chain.clone(); + chain.insert_arg(2, arg("arg")); + assert_eq!(chain.repr(), "a b c arg"); + } + + { + let mut chain = chain.clone(); + chain.insert_arg(3, arg("arg")); + assert_eq!(chain.repr(), "a b c _ arg"); + } + + { + let mut chain = chain.clone(); + chain.insert_arg(4, arg("arg")); + assert_eq!(chain.repr(), "a b c _ _ arg"); + } + } + // TODO[ao] add tests for modifying chain. } diff --git a/src/rust/ide/lib/span-tree/src/action.rs b/src/rust/ide/lib/span-tree/src/action.rs index b80e3d8742..87e156c0ac 100644 --- a/src/rust/ide/lib/span-tree/src/action.rs +++ b/src/rust/ide/lib/span-tree/src/action.rs @@ -126,6 +126,8 @@ impl<'a> Implementation for node::Ref<'a> { BeforeTarget | AfterTarget => infix.target = Some(item), Append if has_arg => infix.push_operand(item), Append => *last_arg = Some(item), + // TODO? below should never happen, as operator arity is always fixed to 2? + FixedArgument(i) => infix.insert_operand(*i,item), }; infix.into_ast() } else { @@ -135,9 +137,10 @@ impl<'a> Implementation for node::Ref<'a> { prefix_id : None, }; match ins_type { - BeforeTarget => prefix.args.insert(0,item), - AfterTarget => prefix.args.insert(1,item), - Append => prefix.args.push(item), + BeforeTarget => prefix.insert_arg(0,item), + AfterTarget => prefix.insert_arg(1,item), + Append => prefix.args.push(item), + FixedArgument(i) => prefix.insert_arg(*i,item), } prefix.into_ast() }; @@ -203,6 +206,10 @@ mod test { use data::text::Index; use data::text::Span; use std::ops::Range; + use crate::builder::TreeBuilder; + use crate::node::Kind::Operation; + use crate::node::Kind::Target; + use crate::node::InsertType::FixedArgument; #[wasm_bindgen_test] fn actions_in_span_tree() { @@ -345,4 +352,30 @@ mod test { let parser = Parser::new_or_panic(); for case in cases { case.run(&parser); } } + + #[test] + fn setting_positional_arguments() { + // Consider Span Tree for `foo bar` where `foo` is a method known to take 3 parameters. + // We can try setting each of 3 arguments to `baz`. + let is_removable = false; + let tree = TreeBuilder::new(7) + .add_leaf(0,3,Operation ,PrefixCrumb::Func) + .add_leaf(4,7,Target{is_removable},PrefixCrumb::Arg) + .add_empty_child(7,FixedArgument(1)) + .add_empty_child(7,FixedArgument(2)) + .build(); + + let ast = Ast::prefix(Ast::var("foo"), Ast::var("bar")); + assert_eq!(ast.repr(),"foo bar"); + let baz = Ast::var("baz"); + + let after = tree.root_ref().child(1).unwrap().set(&ast,baz.clone_ref()).unwrap(); + assert_eq!(after.repr(),"foo baz"); + + let after = tree.root_ref().child(2).unwrap().set(&ast,baz.clone_ref()).unwrap(); + assert_eq!(after.repr(),"foo bar baz"); + + let after = tree.root_ref().child(3).unwrap().set(&ast,baz.clone_ref()).unwrap(); + assert_eq!(after.repr(),"foo bar _ baz"); + } } diff --git a/src/rust/ide/lib/span-tree/src/builder.rs b/src/rust/ide/lib/span-tree/src/builder.rs index d854888237..5ebf4c0220 100644 --- a/src/rust/ide/lib/span-tree/src/builder.rs +++ b/src/rust/ide/lib/span-tree/src/builder.rs @@ -25,6 +25,7 @@ pub trait Builder : Sized { size : Size::new(len), children : vec![], expression_id : None, + argument_info : None, }; let child = node::Child { node, offset : Size::new(offset), @@ -82,6 +83,7 @@ impl TreeBuilder { size : Size::new(len), children : vec![], expression_id : None, + argument_info : None, } } } diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 42995edf8e..922322cb8e 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -118,7 +118,8 @@ impl SpanTreeGenerator for Ast { let size = Size::new(self.len()); let children = default(); let expression_id = self.id; - Ok(Node {kind,size,children,expression_id}) + let argument_info = default(); + Ok(Node {kind,size,children,expression_id,argument_info}) }, } } @@ -179,6 +180,7 @@ impl SpanTreeGenerator for ast::opr::Chain { size : gen.current_offset, children : gen.children, expression_id : elem.infix_id, + argument_info : None, }, elem.offset)) })?; Ok(node) @@ -214,6 +216,7 @@ impl SpanTreeGenerator for ast::prefix::Chain { size : gen.current_offset, children : gen.children, expression_id : arg.prefix_id, + argument_info : None, }) }) } @@ -245,6 +248,7 @@ impl SpanTreeGenerator for ast::known::Match { size : gen.current_offset, children : gen.children, expression_id : self.id(), + argument_info : None, }) } } @@ -280,6 +284,7 @@ impl SpanTreeGenerator for ast::known::Ambiguous { size : gen.current_offset, children : gen.children, expression_id : self.id(), + argument_info : None, }) } } @@ -322,7 +327,8 @@ mod test { use parser::Parser; use wasm_bindgen_test::wasm_bindgen_test; use wasm_bindgen_test::wasm_bindgen_test_configure; - use ast::IdMap; + use ast::{IdMap, Id}; + use crate::{InvocationResolver, InvocationInfo, ParameterInfo}; wasm_bindgen_test_configure!(run_in_browser); @@ -600,4 +606,57 @@ mod test { assert_eq!(expected,tree); } + + #[test] + fn generating_span_tree_for_unfinished_call() { + let parser = Parser::new_or_panic(); + let ast = parser.parse_line("foo here").unwrap(); + + #[derive(Clone,Debug,Default)] + struct MockContext { + map : HashMap, + } + impl MockContext { + fn new_single(id:Id, info:InvocationInfo) -> Self { + let mut ret = Self::default(); + ret.map.insert(id,info); + ret + } + } + impl InvocationResolver for MockContext { + fn invocation_info(&self, id:Id) -> Option { + self.map.get(&id).cloned() + } + } + + let invocation_info = InvocationInfo { + parameters : vec![ + ParameterInfo{name : Some("this".to_owned()), typename : Some("Any".to_owned())}, + ParameterInfo{name : Some("arg1".to_owned()), typename : Some("Number".to_owned())}, + ParameterInfo{name : Some("arg1".to_owned()), typename : None}, + ] + }; + + let ctx = MockContext::new_single(ast.id.unwrap(),invocation_info); + println!("{:?}",ast); + + let tree = SpanTree::new(&ast); + //let mut tree = ast.generate_tree().unwrap(); + println!("{:#?}",tree); + // clear_expression_ids(&mut tree.root); + // + // let is_removable = false; + // let expected = TreeBuilder::new(13) + // .add_leaf(0,3,Operation,PrefixCrumb::Func) + // .add_empty_child(4,BeforeTarget) + // .add_leaf(4,9,Target{is_removable},PrefixCrumb::Arg) + // .add_empty_child(13,Append) + // .build(); + // + // assert_eq!(expected,tree); + + // TODO + // czy jezeli mam atom `foo` i potem zrobie z niego prefix app przez ustawienie inputu + // to nie zepsuje metadanych + } } diff --git a/src/rust/ide/lib/span-tree/src/lib.rs b/src/rust/ide/lib/span-tree/src/lib.rs index 750009e9fe..b5d652d43b 100644 --- a/src/rust/ide/lib/span-tree/src/lib.rs +++ b/src/rust/ide/lib/span-tree/src/lib.rs @@ -46,6 +46,36 @@ pub mod prelude { use traits::*; use prelude::*; +// ========================== +// === InvocationResolver === +// ========================== + +/// Information available about some function parameter. +#[derive(Clone,Debug,Eq,PartialEq)] +#[allow(missing_docs)] +pub struct ParameterInfo { + pub name : Option, + pub typename : Option, + // TODO? [mwu] + // If needed more information could be added here, like param being suspended, defaulted, etc. + +} + +/// Information about a method call that span tree is concerned about. +#[derive(Clone,Debug,Eq,PartialEq)] +pub struct InvocationInfo { + /// Information about arguments taken by a called method. + parameters : Vec, +} + +/// Entity that is able to provide information whether a given expression is a known method +/// invocation. If so, additional information is provided. +pub trait InvocationResolver { + /// Checks if the given expression is known to be a call to a known method. If so, returns the + /// available information. + fn invocation_info(&self, id:ast::Id) -> Option; +} + // ================ @@ -91,7 +121,8 @@ impl Default for SpanTree { let kind = node::Kind::Root; let size = default(); let children = default(); - let root = Node {kind,size,children,expression_id}; + let argument_info = default(); + let root = Node {kind,size,children,expression_id,argument_info}; Self {root} } } diff --git a/src/rust/ide/lib/span-tree/src/node.rs b/src/rust/ide/lib/span-tree/src/node.rs index 68297b10a5..52b41217f9 100644 --- a/src/rust/ide/lib/span-tree/src/node.rs +++ b/src/rust/ide/lib/span-tree/src/node.rs @@ -53,7 +53,13 @@ impl Kind { /// module. #[allow(missing_docs)] #[derive(Copy,Clone,Debug,Eq,PartialEq)] -pub enum InsertType {BeforeTarget,AfterTarget,Append} +pub enum InsertType { + BeforeTarget, + AfterTarget, + Append, + /// Ast should be inserted as an argument at given index (counting the `this` argument). + FixedArgument(usize), +} // === Errors === @@ -95,6 +101,7 @@ pub struct Node { pub size : Size, pub children : Vec, pub expression_id : Option, + pub argument_info : Option, } impl Node { @@ -105,6 +112,7 @@ impl Node { size : Size::new(0), children : Vec::new(), expression_id : None, + argument_info : None, } } diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index e548cb4d2c..fd442a425e 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -497,6 +497,12 @@ impl Handle { Ok(Connections::new(&graph)) } + /// Returns information about all the connections between graph's nodes. + pub fn connections_smarter(&self) -> FallibleResult { + let graph = self.graph_info()?; + Ok(Connections::new(&graph)) + } + /// Suggests a name for a variable that shall store the node value. /// /// Analyzes the expression, e.g. result for "a+b" shall be named "sum". diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index f4fd5e55dd..488f6d0aca 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -13,6 +13,8 @@ use crate::model::execution_context::VisualizationUpdateData; use enso_protocol::language_server::MethodPointer; +pub use crate::controller::graph::Connections; + // ============== @@ -217,6 +219,15 @@ impl Handle { pub fn graph(&self) -> controller::Graph { self.graph.borrow().clone_ref() } + + /// Returns information about all the connections between graph's nodes. + /// + /// In contrast with the `controller::Graph::connections` this uses information received from + /// the LS to enrich the generated span trees with function signatures (arity and argument + /// names). + pub fn connections(&self) -> FallibleResult { + self.graph.borrow().connections_smarter() + } } From 1303f7e6380b30a2bab912b4e69353eb692650e6 Mon Sep 17 00:00:00 2001 From: mwu Date: Thu, 27 Aug 2020 23:47:14 +0200 Subject: [PATCH 02/34] This looks like trouble. --- src/rust/ide/lib/span-tree/src/action.rs | 11 ++-- src/rust/ide/lib/span-tree/src/generate.rs | 24 ++++---- src/rust/ide/lib/span-tree/src/lib.rs | 35 ++++++++++- src/rust/ide/src/controller/graph.rs | 58 +++++++++++-------- src/rust/ide/src/controller/graph/executed.rs | 22 ++++++- src/rust/ide/src/view/node_editor.rs | 4 +- 6 files changed, 108 insertions(+), 46 deletions(-) diff --git a/src/rust/ide/lib/span-tree/src/action.rs b/src/rust/ide/lib/span-tree/src/action.rs index 87e156c0ac..13f2c5ff6c 100644 --- a/src/rust/ide/lib/span-tree/src/action.rs +++ b/src/rust/ide/lib/span-tree/src/action.rs @@ -3,15 +3,17 @@ //! The actions are in WIP state - they will be implemented along connection operations. use crate::prelude::*; -use crate::node; +use crate::{node, InvocationResolver, InvocationInfo}; use crate::node::Kind; -use ast::Ast; +use ast::{Ast, Id}; use ast::Shifted; use ast::crumbs::*; use ast::opr::ArgWithOffset; + + /// ============== /// === Errors === /// ============== @@ -210,6 +212,7 @@ mod test { use crate::node::Kind::Operation; use crate::node::Kind::Target; use crate::node::InsertType::FixedArgument; + use crate::EmptyContext; #[wasm_bindgen_test] fn actions_in_span_tree() { @@ -224,7 +227,7 @@ mod test { impl Case { fn run(&self, parser:&Parser) { let ast = parser.parse_line(self.expr).unwrap(); - let tree = ast.generate_tree().unwrap(); + let tree = ast.generate_tree(&EmptyContext).unwrap(); let span_begin = Index::new(self.span.start); let span_end = Index::new(self.span.end); let span = Span::from_indices(span_begin,span_end); @@ -306,7 +309,7 @@ mod test { impl Case { fn run(&self, parser:&Parser) { let ast = parser.parse_line(self.expr).unwrap(); - let tree = ast.generate_tree().unwrap(); + let tree = ast.generate_tree(&EmptyContext).unwrap(); let span_begin = Index::new(self.span.start); let span_end = Index::new(self.span.end); let span = Span::from_indices(span_begin,span_end); diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 922322cb8e..5ac6c34a85 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -3,7 +3,7 @@ pub mod macros; use crate::prelude::*; -use crate::node; +use crate::{node, InvocationResolver}; use crate::node::InsertType; use crate::Node; use crate::SpanTree; @@ -30,7 +30,7 @@ pub trait SpanTreeGenerator { fn generate_node(&self, kind:node::Kind) -> FallibleResult; /// Generate tree for this AST treated as root for the whole expression. - fn generate_tree(&self) -> FallibleResult { + fn generate_tree(&self, context:&impl InvocationResolver) -> FallibleResult { Ok(SpanTree { root : self.generate_node(node::Kind::Root)? }) @@ -328,7 +328,7 @@ mod test { use wasm_bindgen_test::wasm_bindgen_test; use wasm_bindgen_test::wasm_bindgen_test_configure; use ast::{IdMap, Id}; - use crate::{InvocationResolver, InvocationInfo, ParameterInfo}; + use crate::{InvocationResolver, InvocationInfo, ParameterInfo, EmptyContext}; wasm_bindgen_test_configure!(run_in_browser); @@ -354,7 +354,7 @@ mod test { id_map.generate(14..15); id_map.generate(4..11); let ast = parser.parse_line_with_id_map("2 + foo bar - 3",id_map.clone()).unwrap(); - let mut tree = ast.generate_tree().unwrap(); + let mut tree = ast.generate_tree(&EmptyContext).unwrap(); // Check the expression ids we defined: for id_map_entry in id_map.vec { @@ -395,7 +395,7 @@ mod test { fn generate_span_tree_with_chains() { let parser = Parser::new_or_panic(); let ast = parser.parse_line("2 + 3 + foo bar baz 13 + 5").unwrap(); - let mut tree = ast.generate_tree().unwrap(); + let mut tree = ast.generate_tree(&EmptyContext).unwrap(); clear_expression_ids(&mut tree.root); let is_removable = true; @@ -438,7 +438,7 @@ mod test { fn generating_span_tree_from_right_assoc_operator() { let parser = Parser::new_or_panic(); let ast = parser.parse_line("1,2,3").unwrap(); - let mut tree = ast.generate_tree().unwrap(); + let mut tree = ast.generate_tree(&EmptyContext).unwrap(); clear_expression_ids(&mut tree.root); let is_removable = true; @@ -465,7 +465,7 @@ mod test { // The star makes `SectionSides` ast being one of the parameters of + chain. First + makes // SectionRight, and last + makes SectionLeft. let ast = parser.parse_line("+ * + + 2 +").unwrap(); - let mut tree = ast.generate_tree().unwrap(); + let mut tree = ast.generate_tree(&EmptyContext).unwrap(); clear_expression_ids(&mut tree.root); let is_removable = true; @@ -500,7 +500,7 @@ mod test { fn generating_span_tree_from_right_assoc_section() { let parser = Parser::new_or_panic(); let ast = parser.parse_line(",2,").unwrap(); - let mut tree = ast.generate_tree().unwrap(); + let mut tree = ast.generate_tree(&EmptyContext).unwrap(); clear_expression_ids(&mut tree.root); let is_removable = true; @@ -527,7 +527,7 @@ mod test { id_map.generate(0..29); let expression = "if foo then (a + b) x else ()"; let ast = parser.parse_line_with_id_map(expression,id_map.clone()).unwrap(); - let mut tree = ast.generate_tree().unwrap(); + let mut tree = ast.generate_tree(&EmptyContext).unwrap(); // Check if expression id is set let (_,expected_id) = id_map.vec.first().unwrap(); @@ -572,7 +572,7 @@ mod test { let mut id_map = IdMap::default(); id_map.generate(0..2); let ast = parser.parse_line_with_id_map("(4",id_map.clone()).unwrap(); - let mut tree = ast.generate_tree().unwrap(); + let mut tree = ast.generate_tree(&EmptyContext).unwrap(); // Check the expression id: let (_,expected_id) = id_map.vec.first().unwrap(); @@ -593,7 +593,7 @@ mod test { fn generating_span_tree_for_lambda() { let parser = Parser::new_or_panic(); let ast = parser.parse_line("foo a-> b + c").unwrap(); - let mut tree = ast.generate_tree().unwrap(); + let mut tree = ast.generate_tree(&EmptyContext).unwrap(); clear_expression_ids(&mut tree.root); let is_removable = false; @@ -640,7 +640,7 @@ mod test { let ctx = MockContext::new_single(ast.id.unwrap(),invocation_info); println!("{:?}",ast); - let tree = SpanTree::new(&ast); + let tree = SpanTree::new(&ast,&ctx); //let mut tree = ast.generate_tree().unwrap(); println!("{:#?}",tree); // clear_expression_ids(&mut tree.root); diff --git a/src/rust/ide/lib/span-tree/src/lib.rs b/src/rust/ide/lib/span-tree/src/lib.rs index b5d652d43b..165814f565 100644 --- a/src/rust/ide/lib/span-tree/src/lib.rs +++ b/src/rust/ide/lib/span-tree/src/lib.rs @@ -45,6 +45,7 @@ pub mod prelude { use traits::*; use prelude::*; +use ast::Id; // ========================== // === InvocationResolver === @@ -76,6 +77,36 @@ pub trait InvocationResolver { fn invocation_info(&self, id:ast::Id) -> Option; } +pub struct Merged { + first : First, + second : Second +} + +impl Merged { + pub fn new(first:First, second:Second) -> Self { + Self { + first,second + } + } +} + +impl InvocationResolver for Merged +where First : InvocationResolver, + Second : InvocationResolver { + fn invocation_info(&self, id:Id) -> Option { + self.first.invocation_info(id).or_else(|| self.second.invocation_info(id)) + } +} + + + +pub struct EmptyContext; +impl InvocationResolver for EmptyContext { + fn invocation_info(&self, id:ast::Id) -> Option { + None + } +} + // ================ @@ -94,8 +125,8 @@ pub struct SpanTree { impl SpanTree { /// Create span tree from something that could generate it (usually AST). - pub fn new(generator:&impl SpanTreeGenerator) -> FallibleResult { - generator.generate_tree() + pub fn new(generator:&impl SpanTreeGenerator, context:&impl InvocationResolver) -> FallibleResult { + generator.generate_tree(context) } /// Get the `NodeRef` of root node. diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index fd442a425e..6f987c2d0d 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -20,7 +20,7 @@ use enso_protocol::language_server; use parser::Parser; use span_tree::action::Actions; use span_tree::action::Action; -use span_tree::SpanTree; +use span_tree::{SpanTree, InvocationResolver, EmptyContext}; pub use crate::double_representation::graph::LocationHint; pub use crate::double_representation::graph::Id; @@ -176,10 +176,10 @@ pub struct NodeTrees { impl NodeTrees { #[allow(missing_docs)] - pub fn new(node:&NodeInfo) -> Option { - let inputs = SpanTree::new(node.expression()).ok()?; + pub fn new(node:&NodeInfo, context:&impl InvocationResolver) -> Option { + let inputs = SpanTree::new(node.expression(),context).ok()?; let outputs = if let Some(pat) = node.pattern() { - Some(SpanTree::new(pat).ok()?) + Some(SpanTree::new(pat,context).ok()?) } else { None }; @@ -220,9 +220,9 @@ pub struct Connections { impl Connections { /// Describes a connection for given double representation graph. - pub fn new(graph:&GraphInfo) -> Connections { + pub fn new(graph:&GraphInfo, context:&impl InvocationResolver) -> Connections { let trees = graph.nodes().iter().flat_map(|node| { - Some((node.id(), NodeTrees::new(node)?)) + Some((node.id(), NodeTrees::new(node,context)?)) }).collect(); let mut ret = Connections {trees, connections:default()}; @@ -321,11 +321,13 @@ pub struct EndpointInfo { impl EndpointInfo { /// Construct information about endpoint. Ast must be the node's expression or pattern. - pub fn new(endpoint:&Endpoint, ast:&Ast) -> FallibleResult { + pub fn new + (endpoint:&Endpoint, ast:&Ast, context:&impl InvocationResolver) + -> FallibleResult { Ok(EndpointInfo { endpoint : endpoint.clone(), ast : ast.clone(), - span_tree : SpanTree::new(ast)?, + span_tree : SpanTree::new(ast,context)?, }) } @@ -494,13 +496,16 @@ impl Handle { /// Returns information about all the connections between graph's nodes. pub fn connections(&self) -> FallibleResult { let graph = self.graph_info()?; - Ok(Connections::new(&graph)) + let context = &span_tree::EmptyContext; // TODO create a context that provides information from metadata + Ok(Connections::new(&graph,context)) } /// Returns information about all the connections between graph's nodes. - pub fn connections_smarter(&self) -> FallibleResult { + pub fn connections_smarter(&self, context:&impl InvocationResolver) -> FallibleResult { let graph = self.graph_info()?; - Ok(Connections::new(&graph)) + // TODO perhaps this should merge given context with the metadata information + // or perhaps this should just do exactly what it is told + Ok(Connections::new(&graph,context)) } /// Suggests a name for a variable that shall store the node value. @@ -560,17 +565,17 @@ impl Handle { } /// Obtains information for connection's destination endpoint. - pub fn destination_info(&self, connection:&Connection) -> FallibleResult { + pub fn destination_info(&self, connection:&Connection, context:&impl InvocationResolver) -> FallibleResult { let destination_node = self.node_info(connection.destination.node)?; let target_node_ast = destination_node.expression(); - EndpointInfo::new(&connection.destination,target_node_ast) + EndpointInfo::new(&connection.destination,target_node_ast,context) } /// Obtains information about connection's source endpoint. - pub fn source_info(&self, connection:&Connection) -> FallibleResult { + pub fn source_info(&self, connection:&Connection, context:&impl InvocationResolver) -> FallibleResult { let source_node = self.node_info(connection.source.node)?; if let Some(pat) = source_node.pattern() { - EndpointInfo::new(&connection.source,pat) + EndpointInfo::new(&connection.source,pat,context) } else { // For subports we would not have any idea what pattern to introduce. So we fail. Err(NoPatternOnNode {node : connection.source.node}.into()) @@ -606,16 +611,20 @@ impl Handle { Ok(()) } + pub fn span_tree_context(&self) -> impl InvocationResolver { + EmptyContext + } + /// Create connection in graph. - pub fn connect(&self, connection:&Connection) -> FallibleResult<()> { + pub fn connect(&self, connection:&Connection, context:&impl InvocationResolver) -> FallibleResult<()> { if connection.source.port.is_empty() { // If we create connection from node's expression root, we are able to introduce missing // pattern with a new variable. self.introduce_pattern_if_missing(connection.source.node)?; } - let source_info = self.source_info(connection)?; - let destination_info = self.destination_info(connection)?; + let source_info = self.source_info(connection,context)?; + let destination_info = self.destination_info(connection,context)?; let source_identifier = source_info.target_ast()?.clone(); let updated_target_node_expr = destination_info.set(source_identifier)?; self.set_expression_ast(connection.destination.node,updated_target_node_expr)?; @@ -628,8 +637,8 @@ impl Handle { } /// Remove the connections from the graph. - pub fn disconnect(&self, connection:&Connection) -> FallibleResult<()> { - let info = self.destination_info(connection)?; + pub fn disconnect(&self, connection:&Connection, context:&impl InvocationResolver) -> FallibleResult<()> { + let info = self.destination_info(connection,context)?; let updated_expression = if connection.destination.var_crumbs.is_empty() { let port = info.port()?; @@ -815,6 +824,7 @@ pub mod tests { use parser::Parser; use utils::test::ExpectTuple; use wasm_bindgen_test::wasm_bindgen_test; + use span_tree::EmptyContext; /// All the data needed to set up and run the graph controller in mock environment. #[derive(Clone,Debug)] @@ -1244,7 +1254,7 @@ main = let source = Endpoint::new(node0.info.id(),src_port.to_vec()); let destination = Endpoint::new(node1.info.id(),dst_port.to_vec()); let connection = Connection{source,destination}; - graph.connect(&connection).unwrap(); + graph.connect(&connection,&EmptyContext).unwrap(); let new_main = graph.graph_definition_info().unwrap().ast.repr(); assert_eq!(new_main,expected,"Case {:?}",this); }) @@ -1288,7 +1298,7 @@ main = var_crumbs: vec![] } }; - graph.connect(&connection_to_add).unwrap(); + graph.connect(&connection_to_add,&EmptyContext).unwrap(); let new_main = graph.graph_definition_info().unwrap().ast.repr(); assert_eq!(new_main,EXPECTED); }) @@ -1325,7 +1335,7 @@ main = var_crumbs: vec![] } }; - graph.connect(&connection_to_add).unwrap(); + graph.connect(&connection_to_add,&EmptyContext).unwrap(); let new_main = graph.graph_definition_info().unwrap().ast.repr(); assert_eq!(new_main,EXPECTED); }) @@ -1374,7 +1384,7 @@ main = test.run(|graph| async move { let connections = graph.connections().unwrap(); let connection = connections.connections.first().unwrap(); - graph.disconnect(connection).unwrap(); + graph.disconnect(connection,&EmptyContext).unwrap(); let new_main = graph.graph_definition_info().unwrap().ast.repr(); assert_eq!(new_main,expected,"Case {:?}",this); }) diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index 488f6d0aca..3e317b3282 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -13,8 +13,9 @@ use crate::model::execution_context::VisualizationUpdateData; use enso_protocol::language_server::MethodPointer; +pub use crate::controller::graph::Connection; pub use crate::controller::graph::Connections; - +use span_tree::EmptyContext; // ============== @@ -220,13 +221,30 @@ impl Handle { self.graph.borrow().clone_ref() } + pub fn span_tree_context(&self) -> impl span_tree::InvocationResolver { + // TODO build context that provides information from registry (and perhaps the graph itself) + let registry_context = EmptyContext; + let graph_context = self.graph.borrow().span_tree_context(); + span_tree::Merged::new(registry_context,graph_context) + } + /// Returns information about all the connections between graph's nodes. /// /// In contrast with the `controller::Graph::connections` this uses information received from /// the LS to enrich the generated span trees with function signatures (arity and argument /// names). pub fn connections(&self) -> FallibleResult { - self.graph.borrow().connections_smarter() + self.graph.borrow().connections_smarter(&self.span_tree_context()) + } + + /// Create connection in graph. + pub fn connect(&self, connection:&Connection) -> FallibleResult<()> { + self.graph.borrow().connect(connection,&self.span_tree_context()) + } + + /// Remove the connections from the graph. + pub fn disconnect(&self, connection:&Connection) -> FallibleResult<()> { + self.graph.borrow().disconnect(connection,&self.span_tree_context()) } } diff --git a/src/rust/ide/src/view/node_editor.rs b/src/rust/ide/src/view/node_editor.rs index ce00a455b6..434191cefe 100644 --- a/src/rust/ide/src/view/node_editor.rs +++ b/src/rust/ide/src/view/node_editor.rs @@ -672,14 +672,14 @@ impl GraphEditorIntegratedWithControllerModel { internal_warning!(self.logger,"Created connection {edge_id} overwrite some old \ mappings in GraphEditorIntegration.") } - self.controller.graph().connect(&con)?; + self.controller.connect(&con)?; Ok(()) } fn connection_removed_in_ui(&self, edge_id:&graph_editor::EdgeId) -> FallibleResult<()> { let connection = self.get_controller_connection(*edge_id)?; self.connection_views.borrow_mut().remove_by_left(&connection); - self.controller.graph().disconnect(&connection)?; + self.controller.disconnect(&connection)?; Ok(()) } From c0f51ecaaa6f04a0306a4fd4b5f2eefd4ac37728 Mon Sep 17 00:00:00 2001 From: mwu Date: Fri, 28 Aug 2020 16:58:28 +0200 Subject: [PATCH 03/34] mostly just moving stuff around --- src/rust/ide/lib/span-tree/src/action.rs | 20 +++--- src/rust/ide/lib/span-tree/src/generate.rs | 27 ++++---- .../ide/lib/span-tree/src/generate/context.rs | 67 +++++++++++++++++++ src/rust/ide/lib/span-tree/src/lib.rs | 42 +----------- src/rust/ide/src/controller/graph.rs | 35 +++++----- src/rust/ide/src/controller/graph/executed.rs | 9 +-- 6 files changed, 117 insertions(+), 83 deletions(-) create mode 100644 src/rust/ide/lib/span-tree/src/generate/context.rs diff --git a/src/rust/ide/lib/span-tree/src/action.rs b/src/rust/ide/lib/span-tree/src/action.rs index 13f2c5ff6c..a9555e7f29 100644 --- a/src/rust/ide/lib/span-tree/src/action.rs +++ b/src/rust/ide/lib/span-tree/src/action.rs @@ -3,10 +3,10 @@ //! The actions are in WIP state - they will be implemented along connection operations. use crate::prelude::*; -use crate::{node, InvocationResolver, InvocationInfo}; +use crate::node; use crate::node::Kind; -use ast::{Ast, Id}; +use ast::Ast; use ast::Shifted; use ast::crumbs::*; use ast::opr::ArgWithOffset; @@ -202,17 +202,19 @@ mod test { use Action::*; + use crate::builder::TreeBuilder; + use crate::generate::context; + use crate::node::Kind::Operation; + use crate::node::Kind::Target; + use crate::node::InsertType::FixedArgument; + use wasm_bindgen_test::wasm_bindgen_test; use parser::Parser; use ast::HasRepr; use data::text::Index; use data::text::Span; use std::ops::Range; - use crate::builder::TreeBuilder; - use crate::node::Kind::Operation; - use crate::node::Kind::Target; - use crate::node::InsertType::FixedArgument; - use crate::EmptyContext; + #[wasm_bindgen_test] fn actions_in_span_tree() { @@ -227,7 +229,7 @@ mod test { impl Case { fn run(&self, parser:&Parser) { let ast = parser.parse_line(self.expr).unwrap(); - let tree = ast.generate_tree(&EmptyContext).unwrap(); + let tree = ast.generate_tree(&context::Empty).unwrap(); let span_begin = Index::new(self.span.start); let span_end = Index::new(self.span.end); let span = Span::from_indices(span_begin,span_end); @@ -309,7 +311,7 @@ mod test { impl Case { fn run(&self, parser:&Parser) { let ast = parser.parse_line(self.expr).unwrap(); - let tree = ast.generate_tree(&EmptyContext).unwrap(); + let tree = ast.generate_tree(&context::Empty).unwrap(); let span_begin = Index::new(self.span.start); let span_end = Index::new(self.span.end); let span = Span::from_indices(span_begin,span_end); diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 5ac6c34a85..963e8bbfdc 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -1,9 +1,10 @@ //! A module containing code related to SpanTree generation. +pub mod context; pub mod macros; use crate::prelude::*; -use crate::{node, InvocationResolver}; +use crate::{node, InvocationInfo}; use crate::node::InsertType; use crate::Node; use crate::SpanTree; @@ -17,6 +18,8 @@ use ast::HasLength; use ast::opr::GeneralizedInfix; use data::text::Size; +pub use context::Context; + // ============= @@ -30,7 +33,7 @@ pub trait SpanTreeGenerator { fn generate_node(&self, kind:node::Kind) -> FallibleResult; /// Generate tree for this AST treated as root for the whole expression. - fn generate_tree(&self, context:&impl InvocationResolver) -> FallibleResult { + fn generate_tree(&self, context:&impl Context) -> FallibleResult { Ok(SpanTree { root : self.generate_node(node::Kind::Root)? }) @@ -328,7 +331,7 @@ mod test { use wasm_bindgen_test::wasm_bindgen_test; use wasm_bindgen_test::wasm_bindgen_test_configure; use ast::{IdMap, Id}; - use crate::{InvocationResolver, InvocationInfo, ParameterInfo, EmptyContext}; + use crate::{Context, InvocationInfo, ParameterInfo}; wasm_bindgen_test_configure!(run_in_browser); @@ -354,7 +357,7 @@ mod test { id_map.generate(14..15); id_map.generate(4..11); let ast = parser.parse_line_with_id_map("2 + foo bar - 3",id_map.clone()).unwrap(); - let mut tree = ast.generate_tree(&EmptyContext).unwrap(); + let mut tree = ast.generate_tree(&context::Empty).unwrap(); // Check the expression ids we defined: for id_map_entry in id_map.vec { @@ -395,7 +398,7 @@ mod test { fn generate_span_tree_with_chains() { let parser = Parser::new_or_panic(); let ast = parser.parse_line("2 + 3 + foo bar baz 13 + 5").unwrap(); - let mut tree = ast.generate_tree(&EmptyContext).unwrap(); + let mut tree = ast.generate_tree(&context::Empty).unwrap(); clear_expression_ids(&mut tree.root); let is_removable = true; @@ -438,7 +441,7 @@ mod test { fn generating_span_tree_from_right_assoc_operator() { let parser = Parser::new_or_panic(); let ast = parser.parse_line("1,2,3").unwrap(); - let mut tree = ast.generate_tree(&EmptyContext).unwrap(); + let mut tree = ast.generate_tree(&context::Empty).unwrap(); clear_expression_ids(&mut tree.root); let is_removable = true; @@ -465,7 +468,7 @@ mod test { // The star makes `SectionSides` ast being one of the parameters of + chain. First + makes // SectionRight, and last + makes SectionLeft. let ast = parser.parse_line("+ * + + 2 +").unwrap(); - let mut tree = ast.generate_tree(&EmptyContext).unwrap(); + let mut tree = ast.generate_tree(&context::Empty).unwrap(); clear_expression_ids(&mut tree.root); let is_removable = true; @@ -500,7 +503,7 @@ mod test { fn generating_span_tree_from_right_assoc_section() { let parser = Parser::new_or_panic(); let ast = parser.parse_line(",2,").unwrap(); - let mut tree = ast.generate_tree(&EmptyContext).unwrap(); + let mut tree = ast.generate_tree(&context::Empty).unwrap(); clear_expression_ids(&mut tree.root); let is_removable = true; @@ -527,7 +530,7 @@ mod test { id_map.generate(0..29); let expression = "if foo then (a + b) x else ()"; let ast = parser.parse_line_with_id_map(expression,id_map.clone()).unwrap(); - let mut tree = ast.generate_tree(&EmptyContext).unwrap(); + let mut tree = ast.generate_tree(&context::Empty).unwrap(); // Check if expression id is set let (_,expected_id) = id_map.vec.first().unwrap(); @@ -572,7 +575,7 @@ mod test { let mut id_map = IdMap::default(); id_map.generate(0..2); let ast = parser.parse_line_with_id_map("(4",id_map.clone()).unwrap(); - let mut tree = ast.generate_tree(&EmptyContext).unwrap(); + let mut tree = ast.generate_tree(&context::Empty).unwrap(); // Check the expression id: let (_,expected_id) = id_map.vec.first().unwrap(); @@ -593,7 +596,7 @@ mod test { fn generating_span_tree_for_lambda() { let parser = Parser::new_or_panic(); let ast = parser.parse_line("foo a-> b + c").unwrap(); - let mut tree = ast.generate_tree(&EmptyContext).unwrap(); + let mut tree = ast.generate_tree(&context::Empty).unwrap(); clear_expression_ids(&mut tree.root); let is_removable = false; @@ -623,7 +626,7 @@ mod test { ret } } - impl InvocationResolver for MockContext { + impl Context for MockContext { fn invocation_info(&self, id:Id) -> Option { self.map.get(&id).cloned() } diff --git a/src/rust/ide/lib/span-tree/src/generate/context.rs b/src/rust/ide/lib/span-tree/src/generate/context.rs new file mode 100644 index 0000000000..7db73d56b4 --- /dev/null +++ b/src/rust/ide/lib/span-tree/src/generate/context.rs @@ -0,0 +1,67 @@ +use crate::prelude::*; + +use crate::InvocationInfo; + + + +// =============== +// === Context === +// =============== + +/// Entity that is able to provide information whether a given expression is a known method +/// invocation. If so, additional information is provided. +pub trait Context { + /// Checks if the given expression is known to be a call to a known method. If so, returns the + /// available information. + fn invocation_info(&self, id:ast::Id) -> Option; + + fn merge(self, other:U) -> Merged + where Self:Sized, U:Context { + Merged::new(self,other) + } +} + +fn a(_:Box) {} // TODO remove + + + +// =============== +// === Context === +// =============== + +#[derive(Clone,Debug)] +pub struct Merged { + first : First, + second : Second +} + +impl Merged { + pub fn new(first:First, second:Second) -> Self { + Self { + first,second + } + } +} + +impl Context for Merged + where First : Context, + Second : Context { + fn invocation_info(&self, id:ast::Id) -> Option { + self.first.invocation_info(id).or_else(|| self.second.invocation_info(id)) + } +} + + +// ============= +// === Empty === +// ============= + +#[derive(Copy,Clone,Debug)] +pub struct Empty; + +impl Context for Empty { + fn invocation_info(&self, _id:ast::Id) -> Option { + None + } +} + diff --git a/src/rust/ide/lib/span-tree/src/lib.rs b/src/rust/ide/lib/span-tree/src/lib.rs index 165814f565..4aad584cb1 100644 --- a/src/rust/ide/lib/span-tree/src/lib.rs +++ b/src/rust/ide/lib/span-tree/src/lib.rs @@ -45,7 +45,7 @@ pub mod prelude { use traits::*; use prelude::*; -use ast::Id; +use crate::generate::Context; // ========================== // === InvocationResolver === @@ -69,44 +69,6 @@ pub struct InvocationInfo { parameters : Vec, } -/// Entity that is able to provide information whether a given expression is a known method -/// invocation. If so, additional information is provided. -pub trait InvocationResolver { - /// Checks if the given expression is known to be a call to a known method. If so, returns the - /// available information. - fn invocation_info(&self, id:ast::Id) -> Option; -} - -pub struct Merged { - first : First, - second : Second -} - -impl Merged { - pub fn new(first:First, second:Second) -> Self { - Self { - first,second - } - } -} - -impl InvocationResolver for Merged -where First : InvocationResolver, - Second : InvocationResolver { - fn invocation_info(&self, id:Id) -> Option { - self.first.invocation_info(id).or_else(|| self.second.invocation_info(id)) - } -} - - - -pub struct EmptyContext; -impl InvocationResolver for EmptyContext { - fn invocation_info(&self, id:ast::Id) -> Option { - None - } -} - // ================ @@ -125,7 +87,7 @@ pub struct SpanTree { impl SpanTree { /// Create span tree from something that could generate it (usually AST). - pub fn new(generator:&impl SpanTreeGenerator, context:&impl InvocationResolver) -> FallibleResult { + pub fn new(generator:&impl SpanTreeGenerator, context:&impl Context) -> FallibleResult { generator.generate_tree(context) } diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 6f987c2d0d..c030eba0c3 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -20,11 +20,11 @@ use enso_protocol::language_server; use parser::Parser; use span_tree::action::Actions; use span_tree::action::Action; -use span_tree::{SpanTree, InvocationResolver, EmptyContext}; +use span_tree::SpanTree; pub use crate::double_representation::graph::LocationHint; pub use crate::double_representation::graph::Id; - +use span_tree::generate::Context; // ============== @@ -176,7 +176,7 @@ pub struct NodeTrees { impl NodeTrees { #[allow(missing_docs)] - pub fn new(node:&NodeInfo, context:&impl InvocationResolver) -> Option { + pub fn new(node:&NodeInfo, context:&impl Context) -> Option { let inputs = SpanTree::new(node.expression(),context).ok()?; let outputs = if let Some(pat) = node.pattern() { Some(SpanTree::new(pat,context).ok()?) @@ -220,7 +220,7 @@ pub struct Connections { impl Connections { /// Describes a connection for given double representation graph. - pub fn new(graph:&GraphInfo, context:&impl InvocationResolver) -> Connections { + pub fn new(graph:&GraphInfo, context:&impl Context) -> Connections { let trees = graph.nodes().iter().flat_map(|node| { Some((node.id(), NodeTrees::new(node,context)?)) }).collect(); @@ -322,7 +322,7 @@ pub struct EndpointInfo { impl EndpointInfo { /// Construct information about endpoint. Ast must be the node's expression or pattern. pub fn new - (endpoint:&Endpoint, ast:&Ast, context:&impl InvocationResolver) + (endpoint:&Endpoint, ast:&Ast, context:&impl Context) -> FallibleResult { Ok(EndpointInfo { endpoint : endpoint.clone(), @@ -496,12 +496,12 @@ impl Handle { /// Returns information about all the connections between graph's nodes. pub fn connections(&self) -> FallibleResult { let graph = self.graph_info()?; - let context = &span_tree::EmptyContext; // TODO create a context that provides information from metadata + let context = &span_tree::generate::context::Empty; // TODO create a context that provides information from metadata Ok(Connections::new(&graph,context)) } /// Returns information about all the connections between graph's nodes. - pub fn connections_smarter(&self, context:&impl InvocationResolver) -> FallibleResult { + pub fn connections_smarter(&self, context:&impl Context) -> FallibleResult { let graph = self.graph_info()?; // TODO perhaps this should merge given context with the metadata information // or perhaps this should just do exactly what it is told @@ -565,14 +565,14 @@ impl Handle { } /// Obtains information for connection's destination endpoint. - pub fn destination_info(&self, connection:&Connection, context:&impl InvocationResolver) -> FallibleResult { + pub fn destination_info(&self, connection:&Connection, context:&impl Context) -> FallibleResult { let destination_node = self.node_info(connection.destination.node)?; let target_node_ast = destination_node.expression(); EndpointInfo::new(&connection.destination,target_node_ast,context) } /// Obtains information about connection's source endpoint. - pub fn source_info(&self, connection:&Connection, context:&impl InvocationResolver) -> FallibleResult { + pub fn source_info(&self, connection:&Connection, context:&impl Context) -> FallibleResult { let source_node = self.node_info(connection.source.node)?; if let Some(pat) = source_node.pattern() { EndpointInfo::new(&connection.source,pat,context) @@ -611,12 +611,12 @@ impl Handle { Ok(()) } - pub fn span_tree_context(&self) -> impl InvocationResolver { - EmptyContext + pub fn span_tree_context(&self) -> impl Context { + span_tree::generate::context::Empty } /// Create connection in graph. - pub fn connect(&self, connection:&Connection, context:&impl InvocationResolver) -> FallibleResult<()> { + pub fn connect(&self, connection:&Connection, context:&impl Context) -> FallibleResult<()> { if connection.source.port.is_empty() { // If we create connection from node's expression root, we are able to introduce missing // pattern with a new variable. @@ -637,7 +637,7 @@ impl Handle { } /// Remove the connections from the graph. - pub fn disconnect(&self, connection:&Connection, context:&impl InvocationResolver) -> FallibleResult<()> { + pub fn disconnect(&self, connection:&Connection, context:&impl Context) -> FallibleResult<()> { let info = self.destination_info(connection,context)?; let updated_expression = if connection.destination.var_crumbs.is_empty() { @@ -824,7 +824,6 @@ pub mod tests { use parser::Parser; use utils::test::ExpectTuple; use wasm_bindgen_test::wasm_bindgen_test; - use span_tree::EmptyContext; /// All the data needed to set up and run the graph controller in mock environment. #[derive(Clone,Debug)] @@ -1254,7 +1253,7 @@ main = let source = Endpoint::new(node0.info.id(),src_port.to_vec()); let destination = Endpoint::new(node1.info.id(),dst_port.to_vec()); let connection = Connection{source,destination}; - graph.connect(&connection,&EmptyContext).unwrap(); + graph.connect(&connection,&span_tree::generate::context::Empty).unwrap(); let new_main = graph.graph_definition_info().unwrap().ast.repr(); assert_eq!(new_main,expected,"Case {:?}",this); }) @@ -1298,7 +1297,7 @@ main = var_crumbs: vec![] } }; - graph.connect(&connection_to_add,&EmptyContext).unwrap(); + graph.connect(&connection_to_add,&span_tree::generate::context::Empty).unwrap(); let new_main = graph.graph_definition_info().unwrap().ast.repr(); assert_eq!(new_main,EXPECTED); }) @@ -1335,7 +1334,7 @@ main = var_crumbs: vec![] } }; - graph.connect(&connection_to_add,&EmptyContext).unwrap(); + graph.connect(&connection_to_add,&span_tree::generate::context::Empty).unwrap(); let new_main = graph.graph_definition_info().unwrap().ast.repr(); assert_eq!(new_main,EXPECTED); }) @@ -1384,7 +1383,7 @@ main = test.run(|graph| async move { let connections = graph.connections().unwrap(); let connection = connections.connections.first().unwrap(); - graph.disconnect(connection,&EmptyContext).unwrap(); + graph.disconnect(connection,&span_tree::generate::context::Empty).unwrap(); let new_main = graph.graph_definition_info().unwrap().ast.repr(); assert_eq!(new_main,expected,"Case {:?}",this); }) diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index 3e317b3282..2e15b66c30 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -12,10 +12,11 @@ use crate::model::execution_context::VisualizationId; use crate::model::execution_context::VisualizationUpdateData; use enso_protocol::language_server::MethodPointer; +use span_tree::generate::Context; pub use crate::controller::graph::Connection; pub use crate::controller::graph::Connections; -use span_tree::EmptyContext; + // ============== @@ -221,11 +222,11 @@ impl Handle { self.graph.borrow().clone_ref() } - pub fn span_tree_context(&self) -> impl span_tree::InvocationResolver { + pub fn span_tree_context(&self) -> impl span_tree::generate::Context { // TODO build context that provides information from registry (and perhaps the graph itself) - let registry_context = EmptyContext; + let registry_context = span_tree::generate::context::Empty; let graph_context = self.graph.borrow().span_tree_context(); - span_tree::Merged::new(registry_context,graph_context) + registry_context.merge(graph_context) } /// Returns information about all the connections between graph's nodes. From d68fef878ee7a8e30d42112fd39fbb3dabc77e66 Mon Sep 17 00:00:00 2001 From: mwu Date: Fri, 28 Aug 2020 18:58:03 +0200 Subject: [PATCH 04/34] propagating the context, not adding append placeholders for prefix invocations of known arity --- src/rust/ide/lib/ast/impl/src/prefix.rs | 8 ++ src/rust/ide/lib/span-tree/src/generate.rs | 77 +++++++++++-------- .../ide/lib/span-tree/src/generate/context.rs | 6 +- 3 files changed, 56 insertions(+), 35 deletions(-) diff --git a/src/rust/ide/lib/ast/impl/src/prefix.rs b/src/rust/ide/lib/ast/impl/src/prefix.rs index ee897dda65..01de96293f 100644 --- a/src/rust/ide/lib/ast/impl/src/prefix.rs +++ b/src/rust/ide/lib/ast/impl/src/prefix.rs @@ -166,6 +166,14 @@ impl Chain { self.func } + /// Get the ID of the Ast represented by this chain. + pub fn id(&self) -> Option { + match self.args.last() { + Some(last_arg) => last_arg.prefix_id, + None => self.func.id, + } + } + /// Insert argument at given position in the prefix chain. If index is out of bounds, /// additional blank `_` arguments will be placed. pub fn insert_arg(&mut self, index:usize, argument:Argument) { diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 963e8bbfdc..de9ec3312e 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -30,12 +30,12 @@ pub use context::Context; /// all AST-like structures. pub trait SpanTreeGenerator { /// Generate node with it's whole subtree. - fn generate_node(&self, kind:node::Kind) -> FallibleResult; + fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult; /// Generate tree for this AST treated as root for the whole expression. fn generate_tree(&self, context:&impl Context) -> FallibleResult { Ok(SpanTree { - root : self.generate_node(node::Kind::Root)? + root : self.generate_node(node::Kind::Root,context)? }) } } @@ -63,8 +63,9 @@ impl ChildGenerator { } fn generate_ast_node - (&mut self, child_ast:Located, kind:node::Kind) -> FallibleResult<&node::Child> { - let node = child_ast.item.generate_node(kind)?; + (&mut self, child_ast:Located, kind:node::Kind, context:&impl Context) + -> FallibleResult<&node::Child> { + let node = child_ast.item.generate_node(kind,context)?; Ok(self.add_node(child_ast.crumbs,node)) } @@ -104,19 +105,19 @@ impl ChildGenerator { // === AST === impl SpanTreeGenerator for Ast { - fn generate_node(&self, kind:node::Kind) -> FallibleResult { + fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { if let Some(infix) = GeneralizedInfix::try_new(self) { - infix.flatten().generate_node(kind) + infix.flatten().generate_node(kind,context) } else { match self.shape() { ast::Shape::Prefix(_) => - ast::prefix::Chain::from_ast(self).unwrap().generate_node(kind), + ast::prefix::Chain::from_ast(self).unwrap().generate_node(kind,context), // Lambdas should fall in _ case, because we don't want to create subports for // them ast::Shape::Match(_) if ast::macros::as_lambda_match(self).is_none() => - ast::known::Match::try_new(self.clone_ref()).unwrap().generate_node(kind), + ast::known::Match::try_new(self.clone_ref()).unwrap().generate_node(kind,context), ast::Shape::Ambiguous(_) => - ast::known::Ambiguous::try_new(self.clone_ref()).unwrap().generate_node(kind), + ast::known::Ambiguous::try_new(self.clone_ref()).unwrap().generate_node(kind,context), _ => { let size = Size::new(self.len()); let children = default(); @@ -133,13 +134,13 @@ impl SpanTreeGenerator for Ast { // === Operators (Sections and Infixes) === impl SpanTreeGenerator for ast::opr::Chain { - fn generate_node(&self, kind:node::Kind) -> FallibleResult { + fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { // Removing operands is possible only when chain has at least 3 of them // (target and two arguments). let is_removable = self.args.len() >= 2; let node_and_offset:FallibleResult<(Node,usize)> = match &self.target { Some(target) => { - let node = target.arg.generate_node(node::Kind::Target {is_removable})?; + let node = target.arg.generate_node(node::Kind::Target {is_removable},context)?; Ok((node,target.offset)) }, None => Ok((Node::new_empty(InsertType::BeforeTarget),0)), @@ -164,13 +165,13 @@ impl SpanTreeGenerator for ast::opr::Chain { gen.add_node(left_crumbs,node); if has_target { gen.generate_empty_node(InsertType::AfterTarget); } gen.spacing(off); - gen.generate_ast_node(opr_ast,node::Kind::Operation)?; + gen.generate_ast_node(opr_ast,node::Kind::Operation,context)?; if let Some(operand) = &elem.operand { let arg_crumbs = elem.crumb_to_operand(has_left); let arg_ast = Located::new(arg_crumbs,operand.arg.clone_ref()); gen.spacing(operand.offset); - gen.generate_ast_node(arg_ast,node::Kind::Argument {is_removable})?; + gen.generate_ast_node(arg_ast,node::Kind::Argument {is_removable},context)?; } gen.generate_empty_node(InsertType::Append); @@ -194,13 +195,18 @@ impl SpanTreeGenerator for ast::opr::Chain { // === Application === impl SpanTreeGenerator for ast::prefix::Chain { - fn generate_node(&self, kind:node::Kind) -> FallibleResult { + fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { + let invocation_info = self.id().and_then(|id| context.invocation_info(id)); + dbg!(&invocation_info); + //assert!(context.invocation_info(id).is_none()); + use ast::crumbs::PrefixCrumb::*; // Removing arguments is possible if there at least two of them let is_removable = self.args.len() >= 2; - let node = self.func.generate_node(node::Kind::Operation); + let node = self.func.generate_node(node::Kind::Operation,context); self.args.iter().enumerate().fold(node, |node,(i,arg)| { let node = node?; + let argument_info = invocation_info.and_then(|info| info.parameters.get(i).cloned()); let is_first = i == 0; let is_last = i + 1 == self.args.len(); let arg_kind = if is_first { node::Kind::Target {is_removable} } @@ -209,11 +215,16 @@ impl SpanTreeGenerator for ast::prefix::Chain { let mut gen = ChildGenerator::default(); gen.add_node(vec![Func.into()],node); gen.spacing(arg.sast.off); - if let node::Kind::Target {..} = arg_kind { - gen.generate_empty_node(InsertType::BeforeTarget); + if invocation_info.is_none() { + if let node::Kind::Target { .. } = arg_kind { + gen.generate_empty_node(InsertType::BeforeTarget); + } + } + let arg_ast = arg.sast.wrapped.clone_ref(); + gen.generate_ast_node(Located::new(Arg,arg_ast),arg_kind,context)?; + if invocation_info.is_none() { + gen.generate_empty_node(InsertType::Append); } - gen.generate_ast_node(Located::new(Arg,arg.sast.wrapped.clone_ref()), arg_kind)?; - gen.generate_empty_node(InsertType::Append); Ok(Node { kind : if is_last {kind} else {node::Kind::Chained}, size : gen.current_offset, @@ -229,7 +240,7 @@ impl SpanTreeGenerator for ast::prefix::Chain { // === Match === impl SpanTreeGenerator for ast::known::Match { - fn generate_node(&self, kind:node::Kind) -> FallibleResult { + fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { let is_removable = false; let children_kind = node::Kind::Argument {is_removable}; let mut gen = ChildGenerator::default(); @@ -237,15 +248,15 @@ impl SpanTreeGenerator for ast::known::Match { for macros::AstInPattern {ast,crumbs} in macros::all_ast_nodes_in_pattern(&pat) { let ast_crumb = ast::crumbs::MatchCrumb::Pfx {val:crumbs}; let located_ast = Located::new(ast_crumb,ast.wrapped); - gen.generate_ast_node(located_ast,children_kind)?; + gen.generate_ast_node(located_ast,children_kind,context)?; gen.spacing(ast.off); } } let first_segment_index = 0; - generate_children_from_segment(&mut gen,first_segment_index,&self.segs.head)?; + generate_children_from_segment(&mut gen,first_segment_index,&self.segs.head,context)?; for (index,segment) in self.segs.tail.iter().enumerate() { gen.spacing(segment.off); - generate_children_from_segment(&mut gen,index+1,&segment.wrapped)?; + generate_children_from_segment(&mut gen,index+1,&segment.wrapped,context)?; } Ok(Node {kind, size : gen.current_offset, @@ -257,7 +268,8 @@ impl SpanTreeGenerator for ast::known::Match { } fn generate_children_from_segment -(gen:&mut ChildGenerator, index:usize, segment:&MacroMatchSegment) -> FallibleResult<()> { +(gen:&mut ChildGenerator, index:usize, segment:&MacroMatchSegment, context:&impl Context) +-> FallibleResult<()> { let is_removable = false; let children_kind = node::Kind::Argument {is_removable}; gen.spacing(segment.head.len()); @@ -266,7 +278,7 @@ fn generate_children_from_segment let segment_crumb = ast::crumbs::SegmentMatchCrumb::Body {val:crumbs}; let ast_crumb = ast::crumbs::MatchCrumb::Segs{val:segment_crumb, index}; let located_ast = Located::new(ast_crumb,ast.wrapped); - gen.generate_ast_node(located_ast,children_kind)?; + gen.generate_ast_node(located_ast,children_kind,context)?; } Ok(()) } @@ -275,13 +287,13 @@ fn generate_children_from_segment // === Ambiguous == impl SpanTreeGenerator for ast::known::Ambiguous { - fn generate_node(&self, kind:node::Kind) -> FallibleResult { + fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { let mut gen = ChildGenerator::default(); let first_segment_index = 0; - generate_children_from_abiguous_segment(&mut gen,first_segment_index,&self.segs.head)?; + generate_children_from_abiguous_segment(&mut gen,first_segment_index,&self.segs.head,context)?; for (index,segment) in self.segs.tail.iter().enumerate() { gen.spacing(segment.off); - generate_children_from_abiguous_segment(&mut gen, index+1, &segment.wrapped)?; + generate_children_from_abiguous_segment(&mut gen, index+1, &segment.wrapped,context)?; } Ok(Node{kind, size : gen.current_offset, @@ -293,7 +305,8 @@ impl SpanTreeGenerator for ast::known::Ambiguous { } fn generate_children_from_abiguous_segment -(gen:&mut ChildGenerator, index:usize, segment:&MacroAmbiguousSegment) -> FallibleResult<()> { +(gen:&mut ChildGenerator, index:usize, segment:&MacroAmbiguousSegment, context:&impl Context) +-> FallibleResult<()> { let is_removable = false; let children_kind = node::Kind::Argument {is_removable}; gen.spacing(segment.head.len()); @@ -301,7 +314,7 @@ fn generate_children_from_abiguous_segment gen.spacing(sast.off); let field = ast::crumbs::AmbiguousSegmentCrumb::Body; let located_ast = Located::new(ast::crumbs::AmbiguousCrumb{index,field}, sast.clone_ref()); - gen.generate_ast_node(located_ast,children_kind)?; + gen.generate_ast_node(located_ast,children_kind,context)?; } Ok(()) } @@ -627,8 +640,8 @@ mod test { } } impl Context for MockContext { - fn invocation_info(&self, id:Id) -> Option { - self.map.get(&id).cloned() + fn invocation_info(&self, id:Id) -> Option<&InvocationInfo> { + self.map.get(&id) } } diff --git a/src/rust/ide/lib/span-tree/src/generate/context.rs b/src/rust/ide/lib/span-tree/src/generate/context.rs index 7db73d56b4..0e95578d8b 100644 --- a/src/rust/ide/lib/span-tree/src/generate/context.rs +++ b/src/rust/ide/lib/span-tree/src/generate/context.rs @@ -13,7 +13,7 @@ use crate::InvocationInfo; pub trait Context { /// Checks if the given expression is known to be a call to a known method. If so, returns the /// available information. - fn invocation_info(&self, id:ast::Id) -> Option; + fn invocation_info(&self, id:ast::Id) -> Option<&InvocationInfo>; fn merge(self, other:U) -> Merged where Self:Sized, U:Context { @@ -46,7 +46,7 @@ impl Merged { impl Context for Merged where First : Context, Second : Context { - fn invocation_info(&self, id:ast::Id) -> Option { + fn invocation_info(&self, id:ast::Id) -> Option<&InvocationInfo> { self.first.invocation_info(id).or_else(|| self.second.invocation_info(id)) } } @@ -60,7 +60,7 @@ impl Context for Merged pub struct Empty; impl Context for Empty { - fn invocation_info(&self, _id:ast::Id) -> Option { + fn invocation_info(&self, _id:ast::Id) -> Option<&InvocationInfo> { None } } From 96a12c18a2cc815722c21a955efc5523167610a3 Mon Sep 17 00:00:00 2001 From: mwu Date: Tue, 1 Sep 2020 03:44:09 +0200 Subject: [PATCH 05/34] [wip] --- src/rust/ide/lib/span-tree/src/generate.rs | 30 +++++-- .../ide/lib/span-tree/src/generate/context.rs | 6 +- src/rust/ide/lib/span-tree/src/lib.rs | 3 +- src/rust/ide/src/controller/graph.rs | 83 ++++++++++++++----- src/rust/ide/src/controller/graph/executed.rs | 22 ++++- src/rust/ide/src/controller/module.rs | 13 +-- 6 files changed, 117 insertions(+), 40 deletions(-) diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index de9ec3312e..74f336314f 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -4,7 +4,7 @@ pub mod macros; use crate::prelude::*; -use crate::{node, InvocationInfo}; +use crate::node; use crate::node::InsertType; use crate::Node; use crate::SpanTree; @@ -197,6 +197,8 @@ impl SpanTreeGenerator for ast::opr::Chain { impl SpanTreeGenerator for ast::prefix::Chain { fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { let invocation_info = self.id().and_then(|id| context.invocation_info(id)); + let invocation_info = invocation_info.as_ref(); + let fixed_arity = invocation_info.is_some(); dbg!(&invocation_info); //assert!(context.invocation_info(id).is_none()); @@ -206,7 +208,10 @@ impl SpanTreeGenerator for ast::prefix::Chain { let node = self.func.generate_node(node::Kind::Operation,context); self.args.iter().enumerate().fold(node, |node,(i,arg)| { let node = node?; - let argument_info = invocation_info.and_then(|info| info.parameters.get(i).cloned()); + // TODO we can get i-th argument but we need to also take into account that prefix + // target can be in a form of access chain that passes already `this` + // if so everything should be shifted by one + let argument_info = invocation_info.and_then(|info| info.parameters.get(i)); let is_first = i == 0; let is_last = i + 1 == self.args.len(); let arg_kind = if is_first { node::Kind::Target {is_removable} } @@ -215,14 +220,21 @@ impl SpanTreeGenerator for ast::prefix::Chain { let mut gen = ChildGenerator::default(); gen.add_node(vec![Func.into()],node); gen.spacing(arg.sast.off); - if invocation_info.is_none() { - if let node::Kind::Target { .. } = arg_kind { - gen.generate_empty_node(InsertType::BeforeTarget); - } + if fixed_arity && matches!(arg_kind,node::Kind::Target {..}) { + gen.generate_empty_node(InsertType::BeforeTarget); } let arg_ast = arg.sast.wrapped.clone_ref(); gen.generate_ast_node(Located::new(Arg,arg_ast),arg_kind,context)?; - if invocation_info.is_none() { + if let Some(info) = is_last.and_option(invocation_info) { + // After last arg push the ones that are not yet here. + // TODO likely should follow tree-like nested structure, rather than flat + // or endpoint ids won't be consistent + let remaining_args = info.parameters.iter().skip(i+1); + dbg!(remaining_args.collect_vec()); + // TODO finish + } + + if fixed_arity { gen.generate_empty_node(InsertType::Append); } Ok(Node { @@ -640,8 +652,8 @@ mod test { } } impl Context for MockContext { - fn invocation_info(&self, id:Id) -> Option<&InvocationInfo> { - self.map.get(&id) + fn invocation_info(&self, id:Id) -> Option { + self.map.get(&id).cloned() } } diff --git a/src/rust/ide/lib/span-tree/src/generate/context.rs b/src/rust/ide/lib/span-tree/src/generate/context.rs index 0e95578d8b..7db73d56b4 100644 --- a/src/rust/ide/lib/span-tree/src/generate/context.rs +++ b/src/rust/ide/lib/span-tree/src/generate/context.rs @@ -13,7 +13,7 @@ use crate::InvocationInfo; pub trait Context { /// Checks if the given expression is known to be a call to a known method. If so, returns the /// available information. - fn invocation_info(&self, id:ast::Id) -> Option<&InvocationInfo>; + fn invocation_info(&self, id:ast::Id) -> Option; fn merge(self, other:U) -> Merged where Self:Sized, U:Context { @@ -46,7 +46,7 @@ impl Merged { impl Context for Merged where First : Context, Second : Context { - fn invocation_info(&self, id:ast::Id) -> Option<&InvocationInfo> { + fn invocation_info(&self, id:ast::Id) -> Option { self.first.invocation_info(id).or_else(|| self.second.invocation_info(id)) } } @@ -60,7 +60,7 @@ impl Context for Merged pub struct Empty; impl Context for Empty { - fn invocation_info(&self, _id:ast::Id) -> Option<&InvocationInfo> { + fn invocation_info(&self, _id:ast::Id) -> Option { None } } diff --git a/src/rust/ide/lib/span-tree/src/lib.rs b/src/rust/ide/lib/span-tree/src/lib.rs index 4aad584cb1..c53b805c33 100644 --- a/src/rust/ide/lib/span-tree/src/lib.rs +++ b/src/rust/ide/lib/span-tree/src/lib.rs @@ -9,6 +9,7 @@ #![feature(associated_type_bounds)] #![feature(option_result_contains)] #![feature(trait_alias)] +#![feature(matches_macro)] #![warn(missing_docs)] #![warn(trivial_casts)] #![warn(trivial_numeric_casts)] @@ -66,7 +67,7 @@ pub struct ParameterInfo { #[derive(Clone,Debug,Eq,PartialEq)] pub struct InvocationInfo { /// Information about arguments taken by a called method. - parameters : Vec, + pub parameters : Vec, } diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index c030eba0c3..2560677b0c 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -20,7 +20,7 @@ use enso_protocol::language_server; use parser::Parser; use span_tree::action::Actions; use span_tree::action::Action; -use span_tree::SpanTree; +use span_tree::{SpanTree, InvocationInfo, ParameterInfo}; pub use crate::double_representation::graph::LocationHint; pub use crate::double_representation::graph::Id; @@ -256,6 +256,39 @@ impl Connections { } +/////////////////////////////////////////////////////////////////////// + +/// Span Tree generation context for a graph that does not know about execution. +pub struct GraphContext { + pub graph : Handle, +} + +impl span_tree::generate::Context for GraphContext { + fn invocation_info(&self, id:node::Id) -> Option { + let db = &self.graph.suggestion_db; + let metadata = self.graph.module.node_metadata(id).ok()?; + let db_entry = db.lookup_method(metadata.intended_method?)?; + Some(entry_to_invocation_info(&db_entry)) + } +} + +pub fn entry_param_to_span_tree_param_info(param_info:&model::suggestion_database::Argument) -> ParameterInfo { + ParameterInfo { + // TODO check if suggestion database actually always know the name and type + // or if it can contain just some empty strings or sth + name : Some(param_info.name.clone()), + typename : Some(param_info.repr_type.clone()), + } +} + +pub fn entry_to_invocation_info(entry:&model::suggestion_database::Entry) -> InvocationInfo { + let parameters = entry.arguments.iter().map(entry_param_to_span_tree_param_info).collect(); + InvocationInfo {parameters} +} + +/////////////////////////////////////////////////////////////////////// + + // ================= // === Utilities === @@ -406,28 +439,38 @@ impl EndpointInfo { #[allow(missing_docs)] pub struct Handle { /// Identifier of the graph accessed through this controller. - pub id : Rc, - pub module : model::Module, - parser : Parser, - logger : Logger, + pub id : Rc, + pub module : model::Module, + pub suggestion_db : Rc, + parser : Parser, + logger : Logger, } impl Handle { /// Creates a new controller. Does not check if id is valid. pub fn new_unchecked - (parent:impl AnyLogger, module:model::Module, parser:Parser, id:Id) -> Handle { + ( parent : impl AnyLogger + , module : model::Module + , suggestion_db : Rc + , parser : Parser + , id : Id + ) -> Handle { let id = Rc::new(id); let logger = Logger::sub(parent,format!("Graph Controller {}", id)); - Handle {module,parser,id,logger} + Handle {id,module,suggestion_db,parser,logger} } /// Create a new graph controller. Given ID should uniquely identify a definition in the /// module. Fails if ID cannot be resolved. pub fn new - (parent:impl AnyLogger, module:model::Module, parser:Parser, id:Id) - -> FallibleResult { - let ret = Self::new_unchecked(parent,module,parser,id); + ( parent : impl AnyLogger + , module : model::Module + , suggestion_db : Rc + , parser : Parser + , id : Id + ) -> FallibleResult { + let ret = Self::new_unchecked(parent,module,suggestion_db,parser,id); // Get and discard definition info, we are just making sure it can be obtained. let _ = ret.graph_definition_info()?; Ok(ret) @@ -445,7 +488,7 @@ impl Handle { let module = project.module(module_path).await?; let module_ast = module.ast(); let definition = double_representation::module::lookup_method(&module_ast,&method)?; - Self::new(parent,module,project.parser(),definition) + Self::new(parent,module,project.suggestion_db(),project.parser(),definition) } /// Retrieves double rep information about definition providing this graph. @@ -493,11 +536,17 @@ impl Handle { Ok(nodes) } + pub fn span_tree_context(&self) -> impl Context { + GraphContext { + graph : self.clone_ref() + } + } + /// Returns information about all the connections between graph's nodes. pub fn connections(&self) -> FallibleResult { - let graph = self.graph_info()?; - let context = &span_tree::generate::context::Empty; // TODO create a context that provides information from metadata - Ok(Connections::new(&graph,context)) + let graph = self.graph_info()?; + let context = self.span_tree_context(); + Ok(Connections::new(&graph,&context)) } /// Returns information about all the connections between graph's nodes. @@ -611,10 +660,6 @@ impl Handle { Ok(()) } - pub fn span_tree_context(&self) -> impl Context { - span_tree::generate::context::Empty - } - /// Create connection in graph. pub fn connect(&self, connection:&Connection, context:&impl Context) -> FallibleResult<()> { if connection.source.port.is_empty() { @@ -871,7 +916,7 @@ pub mod tests { let parser = Parser::new().unwrap(); let module = self.module_data().plain(&parser); let id = self.graph_id.clone(); - Handle::new(logger,module,parser,id).unwrap() + Handle::new(logger,module,todo!(),parser,id).unwrap() } pub fn method(&self) -> MethodPointer { diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index 2e15b66c30..d94890e610 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -223,9 +223,11 @@ impl Handle { } pub fn span_tree_context(&self) -> impl span_tree::generate::Context { - // TODO build context that provides information from registry (and perhaps the graph itself) - let registry_context = span_tree::generate::context::Empty; - let graph_context = self.graph.borrow().span_tree_context(); + let registry_context = GraphContext { + registry : self.execution_ctx.computed_value_info_registry().clone_ref(), + db : self.project.suggestion_db(), + }; + let graph_context = self.graph.borrow().span_tree_context(); registry_context.merge(graph_context) } @@ -250,6 +252,20 @@ impl Handle { } +/// Span Tree generation context for a graph that does not know about execution. +struct GraphContext { + registry : Rc, + db : Rc, +} + +impl span_tree::generate::Context for GraphContext { + fn invocation_info(&self, id:double_representation::node::Id) -> Option { + let info = self.registry.get(&id)?; + let entry = self.db.lookup(info.method_call?).ok()?; + Some(controller::graph::entry_to_invocation_info(&entry)) + } +} + // ============ // === Test === diff --git a/src/rust/ide/src/controller/module.rs b/src/rust/ide/src/controller/module.rs index 35e8aba4ee..83eb4daf88 100644 --- a/src/rust/ide/src/controller/module.rs +++ b/src/rust/ide/src/controller/module.rs @@ -95,16 +95,19 @@ impl Handle { /// Returns a graph controller for graph in this module's subtree identified by `id`. pub fn graph_controller - (&self, id:double_representation::graph::Id) -> FallibleResult { - controller::Graph::new(&self.logger, self.model.clone_ref(), self.parser.clone_ref(), id) + (&self, id:double_representation::graph::Id, suggestion_db:Rc) + -> FallibleResult { + controller::Graph::new(&self.logger, self.model.clone_ref(), suggestion_db, + self.parser.clone_ref(), id) } /// Returns a graph controller for graph in this module's subtree identified by `id` without /// checking if the graph exists. pub fn graph_controller_unchecked - (&self, id:double_representation::graph::Id) -> controller::Graph { - controller::Graph::new_unchecked(&self.logger, self.model.clone_ref(), - self.parser.clone_ref(), id) + (&self, id:double_representation::graph::Id, suggestion_db:Rc) + -> controller::Graph { + controller::Graph::new_unchecked(&self.logger, self.model.clone_ref(), suggestion_db, + self.parser.clone_ref(), id) } /// Get the module's qualified name. From eadd31720d318935d57a49012eb5406effaab1b6 Mon Sep 17 00:00:00 2001 From: mwu Date: Tue, 1 Sep 2020 13:48:11 +0200 Subject: [PATCH 06/34] [wip] --- src/rust/ide/src/controller/graph.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 2560677b0c..eb4a81eb2b 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -27,6 +27,7 @@ pub use crate::double_representation::graph::Id; use span_tree::generate::Context; + // ============== // === Errors === // ============== @@ -259,6 +260,8 @@ impl Connections { /////////////////////////////////////////////////////////////////////// /// Span Tree generation context for a graph that does not know about execution. +/// +/// It just applies the information from the metadata. pub struct GraphContext { pub graph : Handle, } @@ -267,6 +270,7 @@ impl span_tree::generate::Context for GraphContext { fn invocation_info(&self, id:node::Id) -> Option { let db = &self.graph.suggestion_db; let metadata = self.graph.module.node_metadata(id).ok()?; + // TODO who should validate if the function name matches? let db_entry = db.lookup_method(metadata.intended_method?)?; Some(entry_to_invocation_info(&db_entry)) } From 6524415f6879a9b3d6c0a6ef389408c2c65fc3a8 Mon Sep 17 00:00:00 2001 From: mwu Date: Wed, 2 Sep 2020 16:13:46 +0200 Subject: [PATCH 07/34] [wip] --- src/rust/ide/lib/span-tree/src/generate.rs | 100 +++++++------ .../ide/lib/span-tree/src/generate/context.rs | 10 ++ src/rust/ide/src/controller/graph.rs | 22 ++- src/rust/ide/src/controller/graph/executed.rs | 3 + src/rust/ide/src/controller/searcher.rs | 2 +- src/rust/ide/src/model/project.rs | 5 + src/rust/ide/src/model/suggestion_database.rs | 13 ++ src/rust/ide/src/test.rs | 131 ++++++++++++++++++ 8 files changed, 241 insertions(+), 45 deletions(-) diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 74f336314f..1353abbb26 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -64,17 +64,17 @@ impl ChildGenerator { fn generate_ast_node (&mut self, child_ast:Located, kind:node::Kind, context:&impl Context) - -> FallibleResult<&node::Child> { + -> FallibleResult<&mut node::Child> { let node = child_ast.item.generate_node(kind,context)?; Ok(self.add_node(child_ast.crumbs,node)) } - fn add_node(&mut self, ast_crumbs:ast::Crumbs, node:Node) -> &node::Child { + fn add_node(&mut self, ast_crumbs:ast::Crumbs, node:Node) -> &mut node::Child { let offset = self.current_offset; let child = node::Child {node,ast_crumbs,offset}; self.current_offset += child.node.size; self.children.push(child); - self.children.last().unwrap() + self.children.last_mut().unwrap() } fn generate_empty_node(&mut self, insert_type:InsertType) -> &node::Child { @@ -198,43 +198,42 @@ impl SpanTreeGenerator for ast::prefix::Chain { fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { let invocation_info = self.id().and_then(|id| context.invocation_info(id)); let invocation_info = invocation_info.as_ref(); - let fixed_arity = invocation_info.is_some(); + let known_args = invocation_info.is_some(); dbg!(&invocation_info); - //assert!(context.invocation_info(id).is_none()); + + // TODO test for case when there are more arguments supplied than the known arity of function? + let supplied_arg_count = self.args.len(); + let method_arity = invocation_info.map(|info| info.parameters.len()); + let arity = supplied_arg_count.max(method_arity.unwrap_or(0)); use ast::crumbs::PrefixCrumb::*; // Removing arguments is possible if there at least two of them let is_removable = self.args.len() >= 2; let node = self.func.generate_node(node::Kind::Operation,context); - self.args.iter().enumerate().fold(node, |node,(i,arg)| { + let ret = self.args.iter().enumerate().fold(node, |node,(i,arg)| { + println!("Will generate argument node for {}",arg.sast.wrapped); let node = node?; // TODO we can get i-th argument but we need to also take into account that prefix // target can be in a form of access chain that passes already `this` // if so everything should be shifted by one + // But on the other hand -- the first "prefix" argument would not be a target + // anymore then. let argument_info = invocation_info.and_then(|info| info.parameters.get(i)); let is_first = i == 0; - let is_last = i + 1 == self.args.len(); + let is_last = i + 1 == arity; let arg_kind = if is_first { node::Kind::Target {is_removable} } else { node::Kind::Argument {is_removable} }; let mut gen = ChildGenerator::default(); gen.add_node(vec![Func.into()],node); gen.spacing(arg.sast.off); - if fixed_arity && matches!(arg_kind,node::Kind::Target {..}) { + if !known_args && matches!(arg_kind,node::Kind::Target {..}) { gen.generate_empty_node(InsertType::BeforeTarget); } let arg_ast = arg.sast.wrapped.clone_ref(); - gen.generate_ast_node(Located::new(Arg,arg_ast),arg_kind,context)?; - if let Some(info) = is_last.and_option(invocation_info) { - // After last arg push the ones that are not yet here. - // TODO likely should follow tree-like nested structure, rather than flat - // or endpoint ids won't be consistent - let remaining_args = info.parameters.iter().skip(i+1); - dbg!(remaining_args.collect_vec()); - // TODO finish - } - - if fixed_arity { + let arg_child = gen.generate_ast_node(Located::new(Arg,arg_ast),arg_kind,context)?; + arg_child.node.argument_info = argument_info.cloned(); + if !known_args { gen.generate_empty_node(InsertType::Append); } Ok(Node { @@ -244,7 +243,28 @@ impl SpanTreeGenerator for ast::prefix::Chain { expression_id : arg.prefix_id, argument_info : None, }) - }) + })?; + + if let Some(info) = invocation_info { + let missing_args = info.parameters.iter().enumerate().skip(self.args.len()); + Ok(missing_args.fold(ret, |node,(i,param)| { + println!("Will generate missing argument node for {:?}",param); + let mut gen = ChildGenerator::default(); + gen.add_node(vec![],node); + let is_last = i + 1 == arity; + + gen.generate_empty_node(InsertType::FixedArgument(i)); + Node { + kind : if is_last {kind} else {node::Kind::Chained}, + size : gen.current_offset, + children : gen.children, + expression_id : None, + argument_info : Some(param.clone()), + } + })) + } else { + Ok(ret) + } } } @@ -360,6 +380,23 @@ mod test { wasm_bindgen_test_configure!(run_in_browser); + #[derive(Clone,Debug,Default)] + struct MockContext { + map : HashMap, + } + impl MockContext { + fn new_single(id:Id, info:InvocationInfo) -> Self { + let mut ret = Self::default(); + ret.map.insert(id,info); + ret + } + } + impl Context for MockContext { + fn invocation_info(&self, id:Id) -> Option { + self.map.get(&id).cloned() + } + } + /// A helper function which removes information about expression id from thw tree rooted at /// `node`. /// @@ -640,37 +677,22 @@ mod test { let parser = Parser::new_or_panic(); let ast = parser.parse_line("foo here").unwrap(); - #[derive(Clone,Debug,Default)] - struct MockContext { - map : HashMap, - } - impl MockContext { - fn new_single(id:Id, info:InvocationInfo) -> Self { - let mut ret = Self::default(); - ret.map.insert(id,info); - ret - } - } - impl Context for MockContext { - fn invocation_info(&self, id:Id) -> Option { - self.map.get(&id).cloned() - } - } - let invocation_info = InvocationInfo { parameters : vec![ ParameterInfo{name : Some("this".to_owned()), typename : Some("Any".to_owned())}, ParameterInfo{name : Some("arg1".to_owned()), typename : Some("Number".to_owned())}, - ParameterInfo{name : Some("arg1".to_owned()), typename : None}, + ParameterInfo{name : Some("arg2".to_owned()), typename : None}, ] }; let ctx = MockContext::new_single(ast.id.unwrap(),invocation_info); println!("{:?}",ast); - let tree = SpanTree::new(&ast,&ctx); + let tree = SpanTree::new(&ast,&ctx).unwrap(); //let mut tree = ast.generate_tree().unwrap(); println!("{:#?}",tree); + println!("Leaves: === \n{:#?}\n",tree.root_ref().leaf_iter().collect_vec()); + // clear_expression_ids(&mut tree.root); // // let is_removable = false; diff --git a/src/rust/ide/lib/span-tree/src/generate/context.rs b/src/rust/ide/lib/span-tree/src/generate/context.rs index 7db73d56b4..0c16cb1c87 100644 --- a/src/rust/ide/lib/span-tree/src/generate/context.rs +++ b/src/rust/ide/lib/span-tree/src/generate/context.rs @@ -1,3 +1,7 @@ +//! Span tree shape and contained data depends not only on the AST but also some context-dependent +//! information. This module defined trait [`Context`] that provides the information known to +//! Span Tree during its construction. + use crate::prelude::*; use crate::InvocationInfo; @@ -15,6 +19,8 @@ pub trait Context { /// available information. fn invocation_info(&self, id:ast::Id) -> Option; + /// Build a new context that merges this context and the one given in argument that will be used + /// as a fallback. fn merge(self, other:U) -> Merged where Self:Sized, U:Context { Merged::new(self,other) @@ -29,6 +35,7 @@ fn a(_:Box) {} // TODO remove // === Context === // =============== +/// Represents a context created from merging two other contexts. #[derive(Clone,Debug)] pub struct Merged { first : First, @@ -36,6 +43,7 @@ pub struct Merged { } impl Merged { + /// Creates a context merging the contexts from arguments. pub fn new(first:First, second:Second) -> Self { Self { first,second @@ -52,10 +60,12 @@ impl Context for Merged } + // ============= // === Empty === // ============= +/// An empty context that provides no information whatsoever. #[derive(Copy,Clone,Debug)] pub struct Empty; diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index eb4a81eb2b..5397ce30a7 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -262,6 +262,7 @@ impl Connections { /// Span Tree generation context for a graph that does not know about execution. /// /// It just applies the information from the metadata. +#[derive(Clone,Debug)] pub struct GraphContext { pub graph : Handle, } @@ -874,6 +875,8 @@ pub mod tests { use utils::test::ExpectTuple; use wasm_bindgen_test::wasm_bindgen_test; + use crate::model::suggestion_database; + /// All the data needed to set up and run the graph controller in mock environment. #[derive(Clone,Debug)] pub struct MockData { @@ -881,6 +884,7 @@ pub mod tests { pub graph_id : Id, pub project_name : String, pub code : String, + pub suggestions : HashMap, } impl MockData { @@ -892,6 +896,7 @@ pub mod tests { 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(), + suggestions : default(), } } @@ -916,16 +921,23 @@ pub mod tests { /// Create a graph controller from the current mock data. pub fn graph(&self) -> Handle { - let logger = Logger::new("Test"); - let parser = Parser::new().unwrap(); - let module = self.module_data().plain(&parser); - let id = self.graph_id.clone(); - Handle::new(logger,module,todo!(),parser,id).unwrap() + let logger = Logger::new("Test"); + let parser = Parser::new().unwrap(); + let module = self.module_data().plain(&parser); + let id = self.graph_id.clone(); + let db = self.suggestion_db(); + Handle::new(logger,module,db,parser,id).unwrap() } pub fn method(&self) -> MethodPointer { self.module_path.method_pointer(&self.project_name,self.graph_id.to_string()) } + + pub fn suggestion_db(&self) -> Rc { + use model::suggestion_database::SuggestionDatabase; + let entries = self.suggestions.iter(); + Rc::new(SuggestionDatabase::new_from_entries(Logger::new("Test"),entries)) + } } impl Default for MockData { diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index d94890e610..bff1098158 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -304,6 +304,9 @@ pub mod tests { model::project::test::expect_execution_ctx(&mut project,ctx); // Root ID is needed to generate module path used to get the module. model::project::test::expect_root_id(&mut project,crate::test::mock::data::ROOT_ID); + // Both graph controllers need suggestion DB to provide context to their span trees. + let suggestion_db = self.graph.suggestion_db(); + model::project::test::expect_suggestion_db(&mut project,suggestion_db); let project = Rc::new(project); Handle::new(Logger::default(),project.clone_ref(),method).boxed_local().expect_ok() diff --git a/src/rust/ide/src/controller/searcher.rs b/src/rust/ide/src/controller/searcher.rs index 950e2bc5cd..ef2ccaa146 100644 --- a/src/rust/ide/src/controller/searcher.rs +++ b/src/rust/ide/src/controller/searcher.rs @@ -835,7 +835,7 @@ mod test { fn new_custom(client_setup:F) -> Self where F : FnOnce(&mut MockData,&mut language_server::MockClient) { let test = TestWithLocalPoolExecutor::set_up(); - let mut data = MockData::default(); + let mut data:MockData = MockData::default(); let mut client = language_server::MockClient::default(); client.require_all_calls(); client_setup(&mut data,&mut client); diff --git a/src/rust/ide/src/model/project.rs b/src/rust/ide/src/model/project.rs index 5a14ad84ae..f068885606 100644 --- a/src/rust/ide/src/model/project.rs +++ b/src/rust/ide/src/model/project.rs @@ -109,4 +109,9 @@ pub mod test { pub fn expect_root_id(project:&mut MockAPI, root_id:Uuid) { project.expect_content_root_id().return_const(root_id); } + + /// Sets up module expectation on the mock project, returning a give module. + pub fn expect_suggestion_db(project:&mut MockAPI, suggestion_db:Rc) { + project.expect_suggestion_db().returning_st(move || suggestion_db.clone_ref()); + } } diff --git a/src/rust/ide/src/model/suggestion_database.rs b/src/rust/ide/src/model/suggestion_database.rs index c03319b5d9..5de9ad2ab4 100644 --- a/src/rust/ide/src/model/suggestion_database.rs +++ b/src/rust/ide/src/model/suggestion_database.rs @@ -248,6 +248,19 @@ pub struct SuggestionDatabase { } impl SuggestionDatabase { + pub fn new_empty(logger:impl AnyLogger) -> Self { + Self { + logger : Logger::sub(logger,"SuggestionDatabase"), + ..default() + } + } + + pub fn new_from_entries<'a>(logger:impl AnyLogger, entries:impl IntoIterator) -> Self { + let ret = Self::new_empty(logger); + ret.entries.borrow_mut().extend(entries.into_iter().map(|(id,entry)| (*id,Rc::new(entry.clone())))); + ret + } + /// Create a new database which will take its initial content from the Language Server. pub async fn create_synchronized (language_server:&language_server::Connection) -> FallibleResult { diff --git a/src/rust/ide/src/test.rs b/src/rust/ide/src/test.rs index f7e7c71e01..252a9c9810 100644 --- a/src/rust/ide/src/test.rs +++ b/src/rust/ide/src/test.rs @@ -22,6 +22,7 @@ pub mod mock { pub const DEFINITION_NAME : &str = "main"; pub const TYPE_NAME : &str = "Mock_Type"; pub const MAIN_FINISH : Position = Position {line:1, character:9}; + pub const CONTEXT_ID : Uuid = Uuid::from_u128(0xFE); pub fn module_path() -> crate::model::module::Path { crate::model::module::Path::from_name_segments(ROOT_ID, &[MODULE_NAME]).unwrap() @@ -38,6 +39,136 @@ pub mod mock { pub fn graph_id() -> crate::double_representation::graph::Id { crate::double_representation::graph::Id::new_plain_name(DEFINITION_NAME) } + + pub fn suggestion_db() -> crate::model::SuggestionDatabase { + crate::model::SuggestionDatabase::default() + } + } + + #[derive(Clone,Debug)] + struct Unified { + pub logger : Logger, + pub project_name : String, + pub module_path : model::module::Path, + pub graph_id : double_representation::graph::Id, + pub suggestions : HashMap, + pub context_id : model::execution_context::Id, + pub parser : parser::Parser, + code : String, + id_map : ast::IdMap, + metadata : crate::model::module::Metadata, + root_definition : double_representation::definition::DefinitionName, + } + + impl Unified { + pub fn new() -> Self { + use crate::test::mock::data::*; + Unified { + logger : Logger::default(), + project_name : PROJECT_NAME.to_owned(), + module_path : module_path(), + graph_id : graph_id(), + code : CODE.to_owned(), + suggestions : default(), + id_map : default(), + metadata : default(), + context_id : CONTEXT_ID, + root_definition : definition_name(), + parser : parser::Parser::new_or_panic(), + } + } + + pub fn module(&self) -> crate::model::Module { + let ast = self.parser.parse_module(self.code.clone(),self.id_map.clone()).unwrap(); + let module = crate::model::module::Plain::new(self.module_path.clone(),ast,self.metadata.clone()); + Rc::new(module) + } + + pub fn module_qualified_name(&self) -> double_representation::module::QualifiedName { + self.module_path.qualified_module_name(&self.project_name) + } + + pub fn definition_id(&self) -> double_representation::definition::Id { + double_representation::definition::Id::new_single_crumb(self.root_definition.clone()) + } + + pub fn method_pointer(&self) -> enso_protocol::language_server::MethodPointer { + enso_protocol::language_server::MethodPointer { + module : self.module_qualified_name().to_string(), + defined_on_type : self.module_path.module_name().to_string(), + name : self.root_definition.to_string(), + } + } + + /// Create a graph controller from the current mock data. + pub fn graph(&self, module:model::Module, db:Rc) + -> crate::controller::Graph { + let logger = Logger::new("Test"); + let id = self.graph_id.clone(); + let parser = self.parser.clone_ref(); + crate::controller::Graph::new(logger,module,db,parser,id).unwrap() + } + + pub fn execution_context(&self) -> model::ExecutionContext { + let logger = Logger::sub(&self.logger,"Mocked Execution Context"); + Rc::new(model::execution_context::Plain::new(logger,self.method_pointer())) + } + + pub fn project(&self, module:model::Module, execution_context:model::ExecutionContext) + -> model::Project { + let mut project = model::project::MockAPI::new(); + model::project::test::expect_parser(&mut project,&self.parser); + model::project::test::expect_module(&mut project,module); + model::project::test::expect_execution_ctx(&mut project,execution_context); + // Root ID is needed to generate module path used to get the module. + model::project::test::expect_root_id(&mut project,crate::test::mock::data::ROOT_ID); + Rc::new(project) + } + + pub fn bake(&self) -> Baked { + let logger = Logger::default(); // TODO + let module = self.module(); + let suggestion_db = Rc::new(model::SuggestionDatabase::new_from_entries(logger, + &self.suggestions)); + let graph = self.graph(module.clone_ref(), suggestion_db.clone_ref()); + let execution = self.execution_context(); + let method_ptr = self.method_pointer(); + let project = self.project(module.clone_ref(),execution.clone_ref()); + let executed_graph = controller::ExecutedGraph::new_internal(graph.clone_ref(), + project.clone_ref(),execution.clone_ref()); + Baked { + data : self.clone(), + module, + graph, + executed_graph, + execution, + suggestion_db, + project, + } + } + } + + #[derive(Clone,Debug)] + struct Baked { + data : Unified, + module : model::Module, + graph : controller::Graph, + execution : model::ExecutionContext, + executed_graph : controller::ExecutedGraph, + suggestion_db : Rc, + project : model::Project, + } + + impl Baked { + // pub fn module(&mut self) -> crate::model::Module { + // self.module.get_or_insert(self.data.module()).clone_ref() + // } + // + // /// Create a graph controller from the current mock data. + // pub fn graph(&mut self, module:model::Module) -> crate::controller::Graph { + // let module = self.module(); + // self.data.graph(module) + // } } pub fn indent(line:impl AsRef) -> String { From 102e81646f82a09c9beeb09f4ac02a0d1a87c853 Mon Sep 17 00:00:00 2001 From: mwu Date: Fri, 4 Sep 2020 14:02:08 +0200 Subject: [PATCH 08/34] tests, fixes, some cleanups --- src/rust/ide/lib/span-tree/src/action.rs | 10 +- src/rust/ide/lib/span-tree/src/builder.rs | 4 +- src/rust/ide/lib/span-tree/src/generate.rs | 237 ++++++++++++------ .../ide/lib/span-tree/src/generate/context.rs | 34 ++- src/rust/ide/lib/span-tree/src/lib.rs | 32 +-- src/rust/ide/lib/span-tree/src/node.rs | 23 +- src/rust/ide/src/controller/graph.rs | 144 +++++++++-- src/rust/ide/src/controller/graph/executed.rs | 29 ++- src/rust/ide/src/controller/searcher.rs | 2 +- src/rust/ide/src/double_representation.rs | 20 ++ .../src/double_representation/identifier.rs | 4 + src/rust/ide/src/model/suggestion_database.rs | 10 +- src/rust/ide/src/test.rs | 19 +- src/rust/ide/src/view/node_editor.rs | 2 +- 14 files changed, 409 insertions(+), 161 deletions(-) diff --git a/src/rust/ide/lib/span-tree/src/action.rs b/src/rust/ide/lib/span-tree/src/action.rs index a9555e7f29..2f56e84100 100644 --- a/src/rust/ide/lib/span-tree/src/action.rs +++ b/src/rust/ide/lib/span-tree/src/action.rs @@ -129,7 +129,7 @@ impl<'a> Implementation for node::Ref<'a> { Append if has_arg => infix.push_operand(item), Append => *last_arg = Some(item), // TODO? below should never happen, as operator arity is always fixed to 2? - FixedArgument(i) => infix.insert_operand(*i,item), + ExpectedArgument(i) => infix.insert_operand(*i, item), }; infix.into_ast() } else { @@ -142,7 +142,7 @@ impl<'a> Implementation for node::Ref<'a> { BeforeTarget => prefix.insert_arg(0,item), AfterTarget => prefix.insert_arg(1,item), Append => prefix.args.push(item), - FixedArgument(i) => prefix.insert_arg(*i,item), + ExpectedArgument(i) => prefix.insert_arg(*i, item), } prefix.into_ast() }; @@ -206,7 +206,7 @@ mod test { use crate::generate::context; use crate::node::Kind::Operation; use crate::node::Kind::Target; - use crate::node::InsertType::FixedArgument; + use crate::node::InsertType::ExpectedArgument; use wasm_bindgen_test::wasm_bindgen_test; use parser::Parser; @@ -366,8 +366,8 @@ mod test { let tree = TreeBuilder::new(7) .add_leaf(0,3,Operation ,PrefixCrumb::Func) .add_leaf(4,7,Target{is_removable},PrefixCrumb::Arg) - .add_empty_child(7,FixedArgument(1)) - .add_empty_child(7,FixedArgument(2)) + .add_empty_child(7, ExpectedArgument(1)) + .add_empty_child(7, ExpectedArgument(2)) .build(); let ast = Ast::prefix(Ast::var("foo"), Ast::var("bar")); diff --git a/src/rust/ide/lib/span-tree/src/builder.rs b/src/rust/ide/lib/span-tree/src/builder.rs index 5ebf4c0220..e16b34e3fe 100644 --- a/src/rust/ide/lib/span-tree/src/builder.rs +++ b/src/rust/ide/lib/span-tree/src/builder.rs @@ -25,7 +25,7 @@ pub trait Builder : Sized { size : Size::new(len), children : vec![], expression_id : None, - argument_info : None, + parameter_info: None, }; let child = node::Child { node, offset : Size::new(offset), @@ -83,7 +83,7 @@ impl TreeBuilder { size : Size::new(len), children : vec![], expression_id : None, - argument_info : None, + parameter_info: None, } } } diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 1353abbb26..0fcba5251c 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -4,10 +4,11 @@ pub mod macros; use crate::prelude::*; -use crate::node; -use crate::node::InsertType; use crate::Node; +use crate::ParameterInfo; use crate::SpanTree; +use crate::node; +use crate::node::InsertType; use ast::Ast; use ast::MacroMatchSegment; @@ -77,14 +78,14 @@ impl ChildGenerator { self.children.last_mut().unwrap() } - fn generate_empty_node(&mut self, insert_type:InsertType) -> &node::Child { + fn generate_empty_node(&mut self, insert_type:InsertType) -> &mut node::Child { let child = node::Child { - node : Node::new_empty(insert_type), - offset : self.current_offset, - ast_crumbs : vec![] + node : Node::new_empty(insert_type), + offset : self.current_offset, + ast_crumbs : vec![] }; self.children.push(child); - self.children.last().unwrap() + self.children.last_mut().unwrap() } fn reverse_children(&mut self) { @@ -120,10 +121,25 @@ impl SpanTreeGenerator for Ast { ast::known::Ambiguous::try_new(self.clone_ref()).unwrap().generate_node(kind,context), _ => { let size = Size::new(self.len()); - let children = default(); let expression_id = self.id; - let argument_info = default(); - Ok(Node {kind,size,children,expression_id,argument_info}) + let children = default(); + if let Some(info) = expression_id.and_then(|id| context.invocation_info(id)) { + let node = Node { + size, + children, + expression_id, + kind : node::Kind::Operation, + parameter_info : None, + }; + let arity = info.parameters.len(); + let params = info.parameters.iter().cloned().enumerate(); + Ok(params.fold(node,|node,(i,param)| { + generate_known_parameter(node,kind,i,arity,param) + })) + } else { + let parameter_info = default(); + Ok(Node {kind,size,children,expression_id,parameter_info}) + } }, } } @@ -180,11 +196,11 @@ impl SpanTreeGenerator for ast::opr::Chain { } Ok((Node { - kind : if is_last {kind} else {node::Kind::Chained}, - size : gen.current_offset, - children : gen.children, - expression_id : elem.infix_id, - argument_info : None, + kind : if is_last {kind} else {node::Kind::Chained}, + size : gen.current_offset, + children : gen.children, + expression_id : elem.infix_id, + parameter_info : None, }, elem.offset)) })?; Ok(node) @@ -232,35 +248,23 @@ impl SpanTreeGenerator for ast::prefix::Chain { } let arg_ast = arg.sast.wrapped.clone_ref(); let arg_child = gen.generate_ast_node(Located::new(Arg,arg_ast),arg_kind,context)?; - arg_child.node.argument_info = argument_info.cloned(); + arg_child.node.parameter_info = argument_info.cloned(); if !known_args { gen.generate_empty_node(InsertType::Append); } Ok(Node { - kind : if is_last {kind} else {node::Kind::Chained}, - size : gen.current_offset, - children : gen.children, - expression_id : arg.prefix_id, - argument_info : None, + kind : if is_last {kind} else {node::Kind::Chained}, + size : gen.current_offset, + children : gen.children, + expression_id : arg.prefix_id, + parameter_info : None, }) })?; if let Some(info) = invocation_info { - let missing_args = info.parameters.iter().enumerate().skip(self.args.len()); + let missing_args = info.parameters.iter().cloned().enumerate().skip(self.args.len()); Ok(missing_args.fold(ret, |node,(i,param)| { - println!("Will generate missing argument node for {:?}",param); - let mut gen = ChildGenerator::default(); - gen.add_node(vec![],node); - let is_last = i + 1 == arity; - - gen.generate_empty_node(InsertType::FixedArgument(i)); - Node { - kind : if is_last {kind} else {node::Kind::Chained}, - size : gen.current_offset, - children : gen.children, - expression_id : None, - argument_info : Some(param.clone()), - } + generate_known_parameter(node, kind, i, arity, param) })) } else { Ok(ret) @@ -268,6 +272,22 @@ impl SpanTreeGenerator for ast::prefix::Chain { } } +fn generate_known_parameter +(node:Node, kind:node::Kind, index:usize, arity:usize, parameter_info:ParameterInfo) -> Node { + println!("Will generate missing argument node for {:?}",parameter_info); + let is_last = index + 1 == arity; + let mut gen = ChildGenerator::default(); + gen.add_node(vec![],node); + let arg_node = gen.generate_empty_node(InsertType::ExpectedArgument(index)); + arg_node.node.parameter_info = Some(parameter_info); + Node { + kind : if is_last {kind} else {node::Kind::Chained}, + size : gen.current_offset, + children : gen.children, + expression_id : None, + parameter_info : None, + } +} // === Match === @@ -291,10 +311,10 @@ impl SpanTreeGenerator for ast::known::Match { generate_children_from_segment(&mut gen,index+1,&segment.wrapped,context)?; } Ok(Node {kind, - size : gen.current_offset, - children : gen.children, - expression_id : self.id(), - argument_info : None, + size : gen.current_offset, + children : gen.children, + expression_id : self.id(), + parameter_info : None, }) } } @@ -328,10 +348,10 @@ impl SpanTreeGenerator for ast::known::Ambiguous { generate_children_from_abiguous_segment(&mut gen, index+1, &segment.wrapped,context)?; } Ok(Node{kind, - size : gen.current_offset, - children : gen.children, - expression_id : self.id(), - argument_info : None, + size : gen.current_offset, + children : gen.children, + expression_id : self.id(), + parameter_info : None, }) } } @@ -360,7 +380,10 @@ fn generate_children_from_abiguous_segment mod test { use super::*; + use crate::Context; + use crate::ParameterInfo; use crate::builder::TreeBuilder; + use crate::generate::context::InvocationInfo; use crate::node::Kind::*; use crate::node::InsertType::*; @@ -375,8 +398,7 @@ mod test { use parser::Parser; use wasm_bindgen_test::wasm_bindgen_test; use wasm_bindgen_test::wasm_bindgen_test_configure; - use ast::{IdMap, Id}; - use crate::{Context, InvocationInfo, ParameterInfo}; + use ast::{IdMap, Id, Crumbs}; wasm_bindgen_test_configure!(run_in_browser); @@ -395,6 +417,10 @@ mod test { fn invocation_info(&self, id:Id) -> Option { self.map.get(&id).cloned() } + + fn named_invocation_info(&self, id:Id, _name:Option<&str>) -> Option { + self.invocation_info(id) + } } /// A helper function which removes information about expression id from thw tree rooted at @@ -409,6 +435,17 @@ mod test { } } + /// A helper function which removes parameter information from nodes. + /// + /// It is used in tests. Because constructing trees with set parameter infos is troublesome, + /// it is often more convenient to test them separately and then erase infos and test for shape. + fn clear_parameter_infos(node:&mut Node) { + node.parameter_info = None; + for child in &mut node.children { + clear_parameter_infos(&mut child.node); + } + } + #[wasm_bindgen_test] fn generating_span_tree() { let parser = Parser::new_or_panic(); @@ -674,39 +711,91 @@ mod test { #[test] fn generating_span_tree_for_unfinished_call() { - let parser = Parser::new_or_panic(); - let ast = parser.parse_line("foo here").unwrap(); + let parser = Parser::new_or_panic(); + let this_param = ParameterInfo{ + name : Some("this".to_owned()), + typename : Some("Any".to_owned()), + }; + let param1 = ParameterInfo{ + name : Some("arg1".to_owned()), + typename : Some("Number".to_owned()), + }; + let param2 = ParameterInfo{ + name : Some("arg2".to_owned()), + typename : None, + }; + + + // === Single function name === + + let ast = parser.parse_line("foo").unwrap(); + let invocation_info = InvocationInfo { + parameters : vec![this_param.clone()] + }; + let ctx = MockContext::new_single(ast.id.unwrap(),invocation_info); + let mut tree = SpanTree::new(&ast,&ctx).unwrap(); + match tree.root_ref().leaf_iter().collect_vec().as_slice() { + [_func,arg0] => assert_eq!(arg0.parameter_info.as_ref(),Some(&this_param)), + sth_else => panic!("There should be 2 leaves, found: {}",sth_else.len()), + } + let expected = TreeBuilder::new(3) + .add_leaf(0,3,Operation,Crumbs::default()) + .add_empty_child(3,ExpectedArgument(0)) + .build(); + clear_expression_ids(&mut tree.root); + clear_parameter_infos(&mut tree.root); + assert_eq!(tree,expected); + + // === Complete application chain === + + let ast = parser.parse_line("foo here").unwrap(); let invocation_info = InvocationInfo { - parameters : vec![ - ParameterInfo{name : Some("this".to_owned()), typename : Some("Any".to_owned())}, - ParameterInfo{name : Some("arg1".to_owned()), typename : Some("Number".to_owned())}, - ParameterInfo{name : Some("arg2".to_owned()), typename : None}, - ] + parameters : vec![this_param.clone()] }; + let ctx = MockContext::new_single(ast.id.unwrap(),invocation_info); + let mut tree = SpanTree::new(&ast,&ctx).unwrap(); + match tree.root_ref().leaf_iter().collect_vec().as_slice() { + [_func,arg0] => assert_eq!(arg0.parameter_info.as_ref(),Some(&this_param)), + sth_else => panic!("There should be 2 leaves, found: {}",sth_else.len()), + } + let expected = TreeBuilder::new(8) + .add_leaf(0,3,Operation,PrefixCrumb::Func) + .add_leaf(4,4,Target {is_removable:false},PrefixCrumb::Arg) + .build(); + clear_expression_ids(&mut tree.root); + clear_parameter_infos(&mut tree.root); + assert_eq!(tree,expected); + + // === Partial application chain === + + let ast = parser.parse_line("foo here").unwrap(); + let invocation_info = InvocationInfo { + parameters : vec![this_param.clone(), param1.clone(), param2.clone()] + }; let ctx = MockContext::new_single(ast.id.unwrap(),invocation_info); - println!("{:?}",ast); - - let tree = SpanTree::new(&ast,&ctx).unwrap(); - //let mut tree = ast.generate_tree().unwrap(); - println!("{:#?}",tree); - println!("Leaves: === \n{:#?}\n",tree.root_ref().leaf_iter().collect_vec()); - - // clear_expression_ids(&mut tree.root); - // - // let is_removable = false; - // let expected = TreeBuilder::new(13) - // .add_leaf(0,3,Operation,PrefixCrumb::Func) - // .add_empty_child(4,BeforeTarget) - // .add_leaf(4,9,Target{is_removable},PrefixCrumb::Arg) - // .add_empty_child(13,Append) - // .build(); - // - // assert_eq!(expected,tree); - - // TODO - // czy jezeli mam atom `foo` i potem zrobie z niego prefix app przez ustawienie inputu - // to nie zepsuje metadanych + let mut tree = SpanTree::new(&ast,&ctx).unwrap(); + match tree.root_ref().leaf_iter().collect_vec().as_slice() { + [_func,arg0,arg1,arg2] => { + assert_eq!(arg0.parameter_info.as_ref(),Some(&this_param)); + assert_eq!(arg1.parameter_info.as_ref(),Some(¶m1)); + assert_eq!(arg2.parameter_info.as_ref(),Some(¶m2)); + }, + sth_else => panic!("There should be 4 leaves, found: {}",sth_else.len()), + } + let expected = TreeBuilder::new(8) + .add_child(0,8,Chained ,Crumbs::default()) + .add_child(0,8,Chained ,Crumbs::default()) + .add_leaf(0,3,Operation,PrefixCrumb::Func) + .add_leaf(4,4,Target {is_removable:false},PrefixCrumb::Arg) + .done() + .add_empty_child(8,ExpectedArgument(1)) + .done() + .add_empty_child(8,ExpectedArgument(2)) + .build(); + clear_expression_ids(&mut tree.root); + clear_parameter_infos(&mut tree.root); + assert_eq!(tree,expected); } } diff --git a/src/rust/ide/lib/span-tree/src/generate/context.rs b/src/rust/ide/lib/span-tree/src/generate/context.rs index 0c16cb1c87..9f4863cac4 100644 --- a/src/rust/ide/lib/span-tree/src/generate/context.rs +++ b/src/rust/ide/lib/span-tree/src/generate/context.rs @@ -4,7 +4,16 @@ use crate::prelude::*; -use crate::InvocationInfo; +use crate::ParameterInfo; +use ast::Id; + + +/// Additional information available on nodes that are an invocation of a known methods. +#[derive(Clone,Debug,Eq,PartialEq)] +pub struct InvocationInfo { + /// Information about arguments taken by a called method. + pub parameters : Vec, +} @@ -17,7 +26,11 @@ use crate::InvocationInfo; pub trait Context { /// Checks if the given expression is known to be a call to a known method. If so, returns the /// available information. - fn invocation_info(&self, id:ast::Id) -> Option; + fn invocation_info(&self, id:Id) -> Option; + + /// Checks if the given expression is known to be a call to a known method. If so, returns the + /// available information. + fn named_invocation_info(&self, id:Id, name:Option<&str>) -> Option; /// Build a new context that merges this context and the one given in argument that will be used /// as a fallback. @@ -27,8 +40,6 @@ pub trait Context { } } -fn a(_:Box) {} // TODO remove - // =============== @@ -44,6 +55,8 @@ pub struct Merged { impl Merged { /// Creates a context merging the contexts from arguments. + /// + /// The first context is checked first, the second one is used as a fallback. pub fn new(first:First, second:Second) -> Self { Self { first,second @@ -54,9 +67,14 @@ impl Merged { impl Context for Merged where First : Context, Second : Context { - fn invocation_info(&self, id:ast::Id) -> Option { + fn invocation_info(&self, id:Id) -> Option { self.first.invocation_info(id).or_else(|| self.second.invocation_info(id)) } + + fn named_invocation_info(&self, id: Id, name:Option<&str>) -> Option { + self.first.named_invocation_info(id,name).or_else(|| + self.second.named_invocation_info(id,name)) + } } @@ -70,7 +88,11 @@ impl Context for Merged pub struct Empty; impl Context for Empty { - fn invocation_info(&self, _id:ast::Id) -> Option { + fn invocation_info(&self, _id:Id) -> Option { + None + } + + fn named_invocation_info(&self, _id:Id, _name:Option<&str>) -> Option { None } } diff --git a/src/rust/ide/lib/span-tree/src/lib.rs b/src/rust/ide/lib/span-tree/src/lib.rs index c53b805c33..311e432f6a 100644 --- a/src/rust/ide/lib/span-tree/src/lib.rs +++ b/src/rust/ide/lib/span-tree/src/lib.rs @@ -48,26 +48,18 @@ use traits::*; use prelude::*; use crate::generate::Context; -// ========================== -// === InvocationResolver === -// ========================== -/// Information available about some function parameter. + +// ===================== +// === ParameterInfo === +// ===================== + +/// Additional information available for nodes being function arguments or their placeholders. #[derive(Clone,Debug,Eq,PartialEq)] #[allow(missing_docs)] pub struct ParameterInfo { pub name : Option, pub typename : Option, - // TODO? [mwu] - // If needed more information could be added here, like param being suspended, defaulted, etc. - -} - -/// Information about a method call that span tree is concerned about. -#[derive(Clone,Debug,Eq,PartialEq)] -pub struct InvocationInfo { - /// Information about arguments taken by a called method. - pub parameters : Vec, } @@ -111,12 +103,12 @@ impl SpanTree { impl Default for SpanTree { fn default() -> Self { - let expression_id = None; - let kind = node::Kind::Root; - let size = default(); - let children = default(); - let argument_info = default(); - let root = Node {kind,size,children,expression_id,argument_info}; + let expression_id = None; + let kind = node::Kind::Root; + let size = default(); + let children = default(); + let parameter_info = default(); + let root = Node {kind,size,children,expression_id,parameter_info}; Self {root} } } diff --git a/src/rust/ide/lib/span-tree/src/node.rs b/src/rust/ide/lib/span-tree/src/node.rs index 52b41217f9..e1477ec985 100644 --- a/src/rust/ide/lib/span-tree/src/node.rs +++ b/src/rust/ide/lib/span-tree/src/node.rs @@ -9,6 +9,7 @@ use data::text::Index; use data::text::Size; + // ==================== // === Helper Types === // ==================== @@ -58,7 +59,7 @@ pub enum InsertType { AfterTarget, Append, /// Ast should be inserted as an argument at given index (counting the `this` argument). - FixedArgument(usize), + ExpectedArgument(usize), } @@ -97,22 +98,22 @@ pub fn parent_crumbs(crumbs:&[Crumb]) -> Option<&[Crumb]> { #[derive(Clone,Debug,Eq,PartialEq)] #[allow(missing_docs)] pub struct Node { - pub kind : Kind, - pub size : Size, - pub children : Vec, - pub expression_id : Option, - pub argument_info : Option, + pub kind : Kind, + pub size : Size, + pub children : Vec, + pub expression_id : Option, + pub parameter_info : Option, } impl Node { /// Create Empty node. pub fn new_empty(insert_type:InsertType) -> Self { Node { - kind : Kind::Empty(insert_type), - size : Size::new(0), - children : Vec::new(), - expression_id : None, - argument_info : None, + kind : Kind::Empty(insert_type), + size : Size::new(0), + children : Vec::new(), + expression_id : None, + parameter_info : None, } } diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 5397ce30a7..1db7a8caf1 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -8,7 +8,7 @@ use crate::prelude::*; use crate::double_representation::definition; use crate::double_representation::graph::GraphInfo; -use crate::double_representation::identifier::LocatedName; +use crate::double_representation::identifier::{LocatedName, NormalizedName}; use crate::double_representation::identifier::generate_name; use crate::double_representation::module; use crate::double_representation::node; @@ -20,12 +20,12 @@ use enso_protocol::language_server; use parser::Parser; use span_tree::action::Actions; use span_tree::action::Action; -use span_tree::{SpanTree, InvocationInfo, ParameterInfo}; +use span_tree::{SpanTree, ParameterInfo}; pub use crate::double_representation::graph::LocationHint; pub use crate::double_representation::graph::Id; -use span_tree::generate::Context; - +use span_tree::generate::Context as SpanTreeContext; +use span_tree::generate::context::InvocationInfo; // ============== @@ -177,7 +177,7 @@ pub struct NodeTrees { impl NodeTrees { #[allow(missing_docs)] - pub fn new(node:&NodeInfo, context:&impl Context) -> Option { + pub fn new(node:&NodeInfo, context:&impl SpanTreeContext) -> Option { let inputs = SpanTree::new(node.expression(),context).ok()?; let outputs = if let Some(pat) = node.pattern() { Some(SpanTree::new(pat,context).ok()?) @@ -221,7 +221,7 @@ pub struct Connections { impl Connections { /// Describes a connection for given double representation graph. - pub fn new(graph:&GraphInfo, context:&impl Context) -> Connections { + pub fn new(graph:&GraphInfo, context:&impl SpanTreeContext) -> Connections { let trees = graph.nodes().iter().flat_map(|node| { Some((node.id(), NodeTrees::new(node,context)?)) }).collect(); @@ -257,27 +257,50 @@ impl Connections { } -/////////////////////////////////////////////////////////////////////// + +// ========================= +// === Span Tree Context === +// ========================= /// Span Tree generation context for a graph that does not know about execution. /// /// It just applies the information from the metadata. #[derive(Clone,Debug)] +#[allow(missing_docs)] pub struct GraphContext { pub graph : Handle, } impl span_tree::generate::Context for GraphContext { fn invocation_info(&self, id:node::Id) -> Option { + self.named_invocation_info(id,None) + } + + fn named_invocation_info(&self, id:node::Id, name:Option<&str>) -> Option { let db = &self.graph.suggestion_db; let metadata = self.graph.module.node_metadata(id).ok()?; - // TODO who should validate if the function name matches? let db_entry = db.lookup_method(metadata.intended_method?)?; - Some(entry_to_invocation_info(&db_entry)) + // If the name is different than intended method than apparently it is not intended anymore + // and should be ignored. + let matching = if let Some(name) = name { + NormalizedName::new(name) == NormalizedName::new(&db_entry.name) + } else { + true + }; + matching.then_with(|| entry_to_invocation_info(&db_entry)) } } -pub fn entry_param_to_span_tree_param_info(param_info:&model::suggestion_database::Argument) -> ParameterInfo { + + +// =========================================== +// === Suggestion DB <=> Span Tree Interop === +// =========================================== +// TODO Reconsider where this should be placed. Unfortunately in neither of the respective crates. + +/// Generate parameter info description from the suggestion database information. +pub fn entry_param_to_span_tree_param_info +(param_info:&model::suggestion_database::Argument) -> ParameterInfo { ParameterInfo { // TODO check if suggestion database actually always know the name and type // or if it can contain just some empty strings or sth @@ -286,6 +309,7 @@ pub fn entry_param_to_span_tree_param_info(param_info:&model::suggestion_databas } } +/// Generate invocation info description from the suggestion database information. pub fn entry_to_invocation_info(entry:&model::suggestion_database::Entry) -> InvocationInfo { let parameters = entry.arguments.iter().map(entry_param_to_span_tree_param_info).collect(); InvocationInfo {parameters} @@ -360,7 +384,7 @@ pub struct EndpointInfo { impl EndpointInfo { /// Construct information about endpoint. Ast must be the node's expression or pattern. pub fn new - (endpoint:&Endpoint, ast:&Ast, context:&impl Context) + (endpoint:&Endpoint, ast:&Ast, context:&impl SpanTreeContext) -> FallibleResult { Ok(EndpointInfo { endpoint : endpoint.clone(), @@ -541,21 +565,27 @@ impl Handle { Ok(nodes) } - pub fn span_tree_context(&self) -> impl Context { + /// Context for building span tree. + /// + /// Provides information about intented method based on the metadata. + pub fn span_tree_context(&self) -> impl SpanTreeContext { GraphContext { graph : self.clone_ref() } } /// Returns information about all the connections between graph's nodes. - pub fn connections(&self) -> FallibleResult { + /// + /// Will use `self.span_tree_context()` as the context for span tree generation. + pub fn connections_legacy(&self) -> FallibleResult { let graph = self.graph_info()?; let context = self.span_tree_context(); Ok(Connections::new(&graph,&context)) } /// Returns information about all the connections between graph's nodes. - pub fn connections_smarter(&self, context:&impl Context) -> FallibleResult { + pub fn connections + (&self, context:&impl SpanTreeContext) -> FallibleResult { let graph = self.graph_info()?; // TODO perhaps this should merge given context with the metadata information // or perhaps this should just do exactly what it is told @@ -619,14 +649,16 @@ impl Handle { } /// Obtains information for connection's destination endpoint. - pub fn destination_info(&self, connection:&Connection, context:&impl Context) -> FallibleResult { + pub fn destination_info + (&self, connection:&Connection, context:&impl SpanTreeContext) -> FallibleResult { let destination_node = self.node_info(connection.destination.node)?; let target_node_ast = destination_node.expression(); EndpointInfo::new(&connection.destination,target_node_ast,context) } /// Obtains information about connection's source endpoint. - pub fn source_info(&self, connection:&Connection, context:&impl Context) -> FallibleResult { + pub fn source_info + (&self, connection:&Connection, context:&impl SpanTreeContext) -> FallibleResult { let source_node = self.node_info(connection.source.node)?; if let Some(pat) = source_node.pattern() { EndpointInfo::new(&connection.source,pat,context) @@ -666,7 +698,8 @@ impl Handle { } /// Create connection in graph. - pub fn connect(&self, connection:&Connection, context:&impl Context) -> FallibleResult<()> { + pub fn connect + (&self, connection:&Connection, context:&impl SpanTreeContext) -> FallibleResult<()> { if connection.source.port.is_empty() { // If we create connection from node's expression root, we are able to introduce missing // pattern with a new variable. @@ -687,7 +720,8 @@ impl Handle { } /// Remove the connections from the graph. - pub fn disconnect(&self, connection:&Connection, context:&impl Context) -> FallibleResult<()> { + pub fn disconnect + (&self, connection:&Connection, context:&impl SpanTreeContext) -> FallibleResult<()> { let info = self.destination_info(connection,context)?; let updated_expression = if connection.destination.var_crumbs.is_empty() { @@ -864,7 +898,7 @@ pub mod tests { use crate::double_representation::identifier::NormalizedName; use crate::executor::test_utils::TestWithLocalPoolExecutor; - use crate::model::module::Position; + use crate::model::module::{Position, MethodId}; use ast::crumbs; use ast::test_utils::expect_shape; @@ -876,6 +910,7 @@ pub mod tests { use wasm_bindgen_test::wasm_bindgen_test; use crate::model::suggestion_database; + use crate::double_representation::module::QualifiedName; /// All the data needed to set up and run the graph controller in mock environment. #[derive(Clone,Debug)] @@ -1036,6 +1071,69 @@ main = }) } + #[test] + fn span_tree_context_handling_metadata_and_name() { + let entry = suggestion_database::Entry { + name : "foo".to_owned(), + module : QualifiedName::from_segments("Std",&["Base"]).unwrap(), + self_type : Some("Base".to_owned()), + arguments : vec![suggestion_database::Argument { + name : "this".to_owned(), + repr_type : "Base".to_owned(), + is_suspended : false, + default_value : None, + }], + return_type : "Any".to_owned(), + kind : suggestion_database::EntryKind::Method, + scope : suggestion_database::Scope::Everywhere, + documentation : None + }; + + let mut test = Fixture::set_up(); + test.data.suggestions.insert(0,entry.clone()); + test.data.code = "main = bar".to_owned(); + test.run(|graph| async move { + let nodes = graph.nodes().unwrap(); + assert_eq!(nodes.len(),1); + let id = nodes[0].info.id(); + graph.module.set_node_metadata(id,NodeMetadata { + position : None, + intended_method : Some(MethodId { + module : entry.module.clone(), + name : entry.name.clone(), + defined_on_type : entry.self_type.clone().unwrap(), + }) + }); + + let context = graph.span_tree_context(); + let get_invocation_info = || { + let node = &graph.nodes().unwrap()[0]; + assert_eq!(node.info.id(),id); + let expression = node.info.expression().repr(); + context.named_invocation_info(id,Some(expression.as_str())) + }; + + // Now node is `bar` while the intended method is `foo`. + // No invocation should be reported, as the name is mismatched. + assert!(get_invocation_info().is_none()); + + // Now the name should be good and we should the information about node being a call. + graph.set_expression(id,"foo").unwrap(); + match get_invocation_info().unwrap().parameters.as_slice() { + [param] => { + let arg = &entry.arguments[0]; + assert_eq!(param.name.as_ref(),Some(&arg.name)); + assert_eq!(param.typename.as_ref(),Some(&arg.repr_type)); + } + _ => panic!("Expected a single parameter in invocation info!"), + } + + // Now we remove metadata, so the information is no more. + graph.module.remove_node_metadata(id).unwrap(); + assert!(get_invocation_info().is_none()); + }) + } + #[wasm_bindgen_test] fn graph_controller_used_names_in_inline_def() { let mut test = Fixture::set_up(); @@ -1243,7 +1341,7 @@ main = print z"; test.data.code = PROGRAM.into(); test.run(|graph| async move { - let connections = graph.connections().unwrap(); + let connections = graph.connections_legacy().unwrap(); let (node0,node1,node2,node3,node4) = graph.nodes().unwrap().expect_tuple(); assert_eq!(node0.info.expression().repr(), "get_pos"); @@ -1344,7 +1442,7 @@ main = sum = _ + b"; test.data.code = PROGRAM.into(); test.run(|graph| async move { - assert!(graph.connections().unwrap().connections.is_empty()); + assert!(graph.connections_legacy().unwrap().connections.is_empty()); let (node0,_node1,node2) = graph.nodes().unwrap().expect_tuple(); let connection_to_add = Connection { source : Endpoint { @@ -1381,7 +1479,7 @@ main = calculate1 = calculate2 calculate3 calculate5 = calculate5 calculate4"; test.run(|graph| async move { - assert!(graph.connections().unwrap().connections.is_empty()); + assert!(graph.connections_legacy().unwrap().connections.is_empty()); let (node0,node1,_) = graph.nodes().unwrap().expect_tuple(); let connection_to_add = Connection { source : Endpoint { @@ -1442,7 +1540,7 @@ main = let expected = format!("{}{}",MAIN_PREFIX,self.dest_node_expected); let this = self.clone(); test.run(|graph| async move { - let connections = graph.connections().unwrap(); + let connections = graph.connections_legacy().unwrap(); let connection = connections.connections.first().unwrap(); graph.disconnect(connection,&span_tree::generate::context::Empty).unwrap(); let new_main = graph.graph_definition_info().unwrap().ast.repr(); diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index bff1098158..2823a16ba6 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -12,7 +12,8 @@ use crate::model::execution_context::VisualizationId; use crate::model::execution_context::VisualizationUpdateData; use enso_protocol::language_server::MethodPointer; -use span_tree::generate::Context; +use span_tree::generate::context::Context; +use span_tree::generate::context::InvocationInfo; pub use crate::controller::graph::Connection; pub use crate::controller::graph::Connections; @@ -60,6 +61,7 @@ pub enum Notification { // ============== // === Handle === // ============== + /// Handle providing executed graph controller interface. #[derive(Clone,CloneRef,Debug)] pub struct Handle { @@ -222,7 +224,10 @@ impl Handle { self.graph.borrow().clone_ref() } - pub fn span_tree_context(&self) -> impl span_tree::generate::Context { + /// Context for span tree generation. + /// + /// It includes both information from the computed values registry and the node metadata. + pub fn span_tree_context(&self) -> impl Context { let registry_context = GraphContext { registry : self.execution_ctx.computed_value_info_registry().clone_ref(), db : self.project.suggestion_db(), @@ -237,7 +242,7 @@ impl Handle { /// the LS to enrich the generated span trees with function signatures (arity and argument /// names). pub fn connections(&self) -> FallibleResult { - self.graph.borrow().connections_smarter(&self.span_tree_context()) + self.graph.borrow().connections(&self.span_tree_context()) } /// Create connection in graph. @@ -252,21 +257,35 @@ impl Handle { } + +// ==================== +// === GraphContext === +// ==================== + /// Span Tree generation context for a graph that does not know about execution. +/// Provides information based on metadata entries. struct GraphContext { registry : Rc, db : Rc, } -impl span_tree::generate::Context for GraphContext { - fn invocation_info(&self, id:double_representation::node::Id) -> Option { +impl Context for GraphContext { + fn invocation_info + (&self, id:double_representation::node::Id) -> Option { let info = self.registry.get(&id)?; let entry = self.db.lookup(info.method_call?).ok()?; Some(controller::graph::entry_to_invocation_info(&entry)) } + + fn named_invocation_info(&self, id:ast::Id, _name:Option<&str>) -> Option { + // We have the knowledge from Language Server. If it says that this is a call, it is a call. + // Regardless of some petty name mismatches. + self.invocation_info(id) + } } + // ============ // === Test === // ============ diff --git a/src/rust/ide/src/controller/searcher.rs b/src/rust/ide/src/controller/searcher.rs index ef2ccaa146..88fb02cb88 100644 --- a/src/rust/ide/src/controller/searcher.rs +++ b/src/rust/ide/src/controller/searcher.rs @@ -1315,7 +1315,7 @@ mod test { } } - #[wasm_bindgen_test] + #[test] fn committing_node() { let Fixture{test:_test,mut searcher,entry4,..} = Fixture::new(); let module = searcher.graph.graph().module.clone_ref(); diff --git a/src/rust/ide/src/double_representation.rs b/src/rust/ide/src/double_representation.rs index 9b09e9edde..a691472610 100644 --- a/src/rust/ide/src/double_representation.rs +++ b/src/rust/ide/src/double_representation.rs @@ -14,6 +14,10 @@ pub mod text; #[cfg(test)] pub mod test_utils; +use crate::prelude::*; + +use crate::double_representation::identifier::Identifier; + // ============== @@ -27,3 +31,19 @@ pub mod test_utils; /// /// Link: https://github.com/luna/enso/blob/main/doc/syntax/encoding.md pub const INDENT : usize = 4; + +// struct InvocationTarget { +// func_name : Identifier, +// this_arg : Option, +// } + +pub fn target_method_name(ast:&Ast) -> Option { + if let Some(chain) = ast::prefix::Chain::from_ast(ast) { + target_method_name(&chain.func) + } else if let Some(chain) = ast::opr::as_access_chain(ast) { + let presumed_name = chain.args.last()?; + identifier::Identifier::new(presumed_name.operand.as_ref()?.arg.clone_ref()) + } else { + identifier::Identifier::new(ast.clone()) + } +} diff --git a/src/rust/ide/src/double_representation/identifier.rs b/src/rust/ide/src/double_representation/identifier.rs index 9d62eface0..04a6db223e 100644 --- a/src/rust/ide/src/double_representation/identifier.rs +++ b/src/rust/ide/src/double_representation/identifier.rs @@ -88,6 +88,10 @@ impl Identifier { Err(OperatorCantBeMadeIntoVar(name.to_owned())) } } + + pub fn normalized(&self) -> NormalizedName { + NormalizedName::new(self.name()) + } } diff --git a/src/rust/ide/src/model/suggestion_database.rs b/src/rust/ide/src/model/suggestion_database.rs index 5de9ad2ab4..893b576e3e 100644 --- a/src/rust/ide/src/model/suggestion_database.rs +++ b/src/rust/ide/src/model/suggestion_database.rs @@ -248,6 +248,7 @@ pub struct SuggestionDatabase { } impl SuggestionDatabase { + /// Create a database with no entries. pub fn new_empty(logger:impl AnyLogger) -> Self { Self { logger : Logger::sub(logger,"SuggestionDatabase"), @@ -255,9 +256,12 @@ impl SuggestionDatabase { } } - pub fn new_from_entries<'a>(logger:impl AnyLogger, entries:impl IntoIterator) -> Self { - let ret = Self::new_empty(logger); - ret.entries.borrow_mut().extend(entries.into_iter().map(|(id,entry)| (*id,Rc::new(entry.clone())))); + /// Create a database filled with entries provided by the given iterator. + pub fn new_from_entries<'a> + (logger:impl AnyLogger, entries:impl IntoIterator) -> Self { + let ret = Self::new_empty(logger); + let entries = entries.into_iter().map(|(id,entry)| (*id,Rc::new(entry.clone()))); + ret.entries.borrow_mut().extend(entries); ret } diff --git a/src/rust/ide/src/test.rs b/src/rust/ide/src/test.rs index 252a9c9810..81738abae8 100644 --- a/src/rust/ide/src/test.rs +++ b/src/rust/ide/src/test.rs @@ -46,7 +46,7 @@ pub mod mock { } #[derive(Clone,Debug)] - struct Unified { + pub struct Unified { pub logger : Logger, pub project_name : String, pub module_path : model::module::Path, @@ -132,7 +132,6 @@ pub mod mock { &self.suggestions)); let graph = self.graph(module.clone_ref(), suggestion_db.clone_ref()); let execution = self.execution_context(); - let method_ptr = self.method_pointer(); let project = self.project(module.clone_ref(),execution.clone_ref()); let executed_graph = controller::ExecutedGraph::new_internal(graph.clone_ref(), project.clone_ref(),execution.clone_ref()); @@ -149,14 +148,14 @@ pub mod mock { } #[derive(Clone,Debug)] - struct Baked { - data : Unified, - module : model::Module, - graph : controller::Graph, - execution : model::ExecutionContext, - executed_graph : controller::ExecutedGraph, - suggestion_db : Rc, - project : model::Project, + pub struct Baked { + pub data : Unified, + pub module : model::Module, + pub graph : controller::Graph, + pub execution : model::ExecutionContext, + pub executed_graph : controller::ExecutedGraph, + pub suggestion_db : Rc, + pub project : model::Project, } impl Baked { diff --git a/src/rust/ide/src/view/node_editor.rs b/src/rust/ide/src/view/node_editor.rs index 434191cefe..5f438fc30e 100644 --- a/src/rust/ide/src/view/node_editor.rs +++ b/src/rust/ide/src/view/node_editor.rs @@ -348,7 +348,7 @@ impl GraphEditorIntegratedWithControllerModel { pub fn refresh_graph_view(&self) -> FallibleResult<()> { info!(self.logger, "Refreshing the graph view."); use controller::graph::Connections; - let Connections{trees,connections} = self.controller.graph().connections()?; + let Connections{trees,connections} = self.controller.graph().connections_legacy()?; self.refresh_node_views(trees)?; self.refresh_connection_views(connections)?; Ok(()) From ee84e6d2f6a851da29d6ef9b2c371def07dc53f1 Mon Sep 17 00:00:00 2001 From: mwu Date: Fri, 4 Sep 2020 16:46:11 +0200 Subject: [PATCH 09/34] cleanups, refactorings, setting up more test --- src/rust/ide/lib/span-tree/src/generate.rs | 11 +- .../ide/lib/span-tree/src/generate/context.rs | 22 +--- src/rust/ide/src/controller/graph.rs | 106 ++++++------------ src/rust/ide/src/controller/graph/executed.rs | 71 ++++++------ src/rust/ide/src/lib.rs | 1 + src/rust/ide/src/test.rs | 62 +++++++++- 6 files changed, 133 insertions(+), 140 deletions(-) diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 0fcba5251c..2a93a2a065 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -123,7 +123,8 @@ impl SpanTreeGenerator for Ast { let size = Size::new(self.len()); let expression_id = self.id; let children = default(); - if let Some(info) = expression_id.and_then(|id| context.invocation_info(id)) { + // TODO + if let Some(info) = expression_id.and_then(|id| context.invocation_info(id,None)) { let node = Node { size, children, @@ -212,7 +213,7 @@ impl SpanTreeGenerator for ast::opr::Chain { impl SpanTreeGenerator for ast::prefix::Chain { fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { - let invocation_info = self.id().and_then(|id| context.invocation_info(id)); + let invocation_info = self.id().and_then(|id| context.invocation_info(id,None)); // TODO let invocation_info = invocation_info.as_ref(); let known_args = invocation_info.is_some(); dbg!(&invocation_info); @@ -414,13 +415,9 @@ mod test { } } impl Context for MockContext { - fn invocation_info(&self, id:Id) -> Option { + fn invocation_info(&self, id:Id, _name:Option<&str>) -> Option { self.map.get(&id).cloned() } - - fn named_invocation_info(&self, id:Id, _name:Option<&str>) -> Option { - self.invocation_info(id) - } } /// A helper function which removes information about expression id from thw tree rooted at diff --git a/src/rust/ide/lib/span-tree/src/generate/context.rs b/src/rust/ide/lib/span-tree/src/generate/context.rs index 9f4863cac4..19190411bb 100644 --- a/src/rust/ide/lib/span-tree/src/generate/context.rs +++ b/src/rust/ide/lib/span-tree/src/generate/context.rs @@ -26,11 +26,7 @@ pub struct InvocationInfo { pub trait Context { /// Checks if the given expression is known to be a call to a known method. If so, returns the /// available information. - fn invocation_info(&self, id:Id) -> Option; - - /// Checks if the given expression is known to be a call to a known method. If so, returns the - /// available information. - fn named_invocation_info(&self, id:Id, name:Option<&str>) -> Option; + fn invocation_info(&self, id:Id, name:Option<&str>) -> Option; /// Build a new context that merges this context and the one given in argument that will be used /// as a fallback. @@ -67,13 +63,9 @@ impl Merged { impl Context for Merged where First : Context, Second : Context { - fn invocation_info(&self, id:Id) -> Option { - self.first.invocation_info(id).or_else(|| self.second.invocation_info(id)) - } - - fn named_invocation_info(&self, id: Id, name:Option<&str>) -> Option { - self.first.named_invocation_info(id,name).or_else(|| - self.second.named_invocation_info(id,name)) + fn invocation_info(&self, id: Id, name:Option<&str>) -> Option { + self.first.invocation_info(id, name).or_else(|| + self.second.invocation_info(id, name)) } } @@ -88,11 +80,7 @@ impl Context for Merged pub struct Empty; impl Context for Empty { - fn invocation_info(&self, _id:Id) -> Option { - None - } - - fn named_invocation_info(&self, _id:Id, _name:Option<&str>) -> Option { + fn invocation_info(&self, _id:Id, _name:Option<&str>) -> Option { None } } diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 1db7a8caf1..15b008b7b2 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -8,7 +8,8 @@ use crate::prelude::*; use crate::double_representation::definition; use crate::double_representation::graph::GraphInfo; -use crate::double_representation::identifier::{LocatedName, NormalizedName}; +use crate::double_representation::identifier::LocatedName; +use crate::double_representation::identifier::NormalizedName; use crate::double_representation::identifier::generate_name; use crate::double_representation::module; use crate::double_representation::node; @@ -18,14 +19,15 @@ use crate::model::module::NodeMetadata; use ast::crumbs::InfixCrumb; use enso_protocol::language_server; use parser::Parser; +use span_tree::SpanTree; +use span_tree::ParameterInfo; use span_tree::action::Actions; use span_tree::action::Action; -use span_tree::{SpanTree, ParameterInfo}; +use span_tree::generate::Context as SpanTreeContext; +use span_tree::generate::context::InvocationInfo; pub use crate::double_representation::graph::LocationHint; pub use crate::double_representation::graph::Id; -use span_tree::generate::Context as SpanTreeContext; -use span_tree::generate::context::InvocationInfo; // ============== @@ -258,41 +260,6 @@ impl Connections { -// ========================= -// === Span Tree Context === -// ========================= - -/// Span Tree generation context for a graph that does not know about execution. -/// -/// It just applies the information from the metadata. -#[derive(Clone,Debug)] -#[allow(missing_docs)] -pub struct GraphContext { - pub graph : Handle, -} - -impl span_tree::generate::Context for GraphContext { - fn invocation_info(&self, id:node::Id) -> Option { - self.named_invocation_info(id,None) - } - - fn named_invocation_info(&self, id:node::Id, name:Option<&str>) -> Option { - let db = &self.graph.suggestion_db; - let metadata = self.graph.module.node_metadata(id).ok()?; - let db_entry = db.lookup_method(metadata.intended_method?)?; - // If the name is different than intended method than apparently it is not intended anymore - // and should be ignored. - let matching = if let Some(name) = name { - NormalizedName::new(name) == NormalizedName::new(&db_entry.name) - } else { - true - }; - matching.then_with(|| entry_to_invocation_info(&db_entry)) - } -} - - - // =========================================== // === Suggestion DB <=> Span Tree Interop === // =========================================== @@ -565,22 +532,12 @@ impl Handle { Ok(nodes) } - /// Context for building span tree. - /// - /// Provides information about intented method based on the metadata. - pub fn span_tree_context(&self) -> impl SpanTreeContext { - GraphContext { - graph : self.clone_ref() - } - } - /// Returns information about all the connections between graph's nodes. /// /// Will use `self.span_tree_context()` as the context for span tree generation. pub fn connections_legacy(&self) -> FallibleResult { let graph = self.graph_info()?; - let context = self.span_tree_context(); - Ok(Connections::new(&graph,&context)) + Ok(Connections::new(&graph,self)) } /// Returns information about all the connections between graph's nodes. @@ -887,6 +844,28 @@ impl Handle { } +// === Span Tree Context === + +/// Span Tree generation context for a graph that does not know about execution. +/// +/// It just applies the information from the metadata. +impl span_tree::generate::Context for Handle { + fn invocation_info(&self, id:node::Id, name:Option<&str>) -> Option { + let db = &self.suggestion_db; + let metadata = self.module.node_metadata(id).ok()?; + let db_entry = db.lookup_method(metadata.intended_method?)?; + // If the name is different than intended method than apparently it is not intended anymore + // and should be ignored. + let matching = if let Some(name) = name { + NormalizedName::new(name) == NormalizedName::new(&db_entry.name) + } else { + true + }; + matching.then_with(|| entry_to_invocation_info(&db_entry)) + } +} + + // ============ // === Test === @@ -910,7 +889,6 @@ pub mod tests { use wasm_bindgen_test::wasm_bindgen_test; use crate::model::suggestion_database; - use crate::double_representation::module::QualifiedName; /// All the data needed to set up and run the graph controller in mock environment. #[derive(Clone,Debug)] @@ -1073,22 +1051,7 @@ main = #[test] fn span_tree_context_handling_metadata_and_name() { - let entry = suggestion_database::Entry { - name : "foo".to_owned(), - module : QualifiedName::from_segments("Std",&["Base"]).unwrap(), - self_type : Some("Base".to_owned()), - arguments : vec![suggestion_database::Argument { - name : "this".to_owned(), - repr_type : "Base".to_owned(), - is_suspended : false, - default_value : None, - }], - return_type : "Any".to_owned(), - kind : suggestion_database::EntryKind::Method, - scope : suggestion_database::Scope::Everywhere, - documentation : None - }; - + let entry = crate::test::mock::data::suggestion_entry_foo(); let mut test = Fixture::set_up(); test.data.suggestions.insert(0,entry.clone()); test.data.code = "main = bar".to_owned(); @@ -1098,19 +1061,14 @@ main = let id = nodes[0].info.id(); graph.module.set_node_metadata(id,NodeMetadata { position : None, - intended_method : Some(MethodId { - module : entry.module.clone(), - name : entry.name.clone(), - defined_on_type : entry.self_type.clone().unwrap(), - }) + intended_method : entry.method_id(), }); - let context = graph.span_tree_context(); let get_invocation_info = || { let node = &graph.nodes().unwrap()[0]; assert_eq!(node.info.id(),id); let expression = node.info.expression().repr(); - context.named_invocation_info(id,Some(expression.as_str())) + graph.invocation_info(id, Some(expression.as_str())) }; // Now node is `bar` while the intended method is `foo`. diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index 2823a16ba6..c56386869f 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -224,63 +224,38 @@ impl Handle { self.graph.borrow().clone_ref() } - /// Context for span tree generation. - /// - /// It includes both information from the computed values registry and the node metadata. - pub fn span_tree_context(&self) -> impl Context { - let registry_context = GraphContext { - registry : self.execution_ctx.computed_value_info_registry().clone_ref(), - db : self.project.suggestion_db(), - }; - let graph_context = self.graph.borrow().span_tree_context(); - registry_context.merge(graph_context) - } - /// Returns information about all the connections between graph's nodes. /// /// In contrast with the `controller::Graph::connections` this uses information received from /// the LS to enrich the generated span trees with function signatures (arity and argument /// names). pub fn connections(&self) -> FallibleResult { - self.graph.borrow().connections(&self.span_tree_context()) + self.graph.borrow().connections(self) } /// Create connection in graph. pub fn connect(&self, connection:&Connection) -> FallibleResult<()> { - self.graph.borrow().connect(connection,&self.span_tree_context()) + self.graph.borrow().connect(connection,self) } /// Remove the connections from the graph. pub fn disconnect(&self, connection:&Connection) -> FallibleResult<()> { - self.graph.borrow().disconnect(connection,&self.span_tree_context()) + self.graph.borrow().disconnect(connection,self) } } - -// ==================== -// === GraphContext === -// ==================== +// === Span Tree Context === /// Span Tree generation context for a graph that does not know about execution. /// Provides information based on metadata entries. -struct GraphContext { - registry : Rc, - db : Rc, -} - -impl Context for GraphContext { - fn invocation_info - (&self, id:double_representation::node::Id) -> Option { - let info = self.registry.get(&id)?; - let entry = self.db.lookup(info.method_call?).ok()?; - Some(controller::graph::entry_to_invocation_info(&entry)) - } - - fn named_invocation_info(&self, id:ast::Id, _name:Option<&str>) -> Option { - // We have the knowledge from Language Server. If it says that this is a call, it is a call. - // Regardless of some petty name mismatches. - self.invocation_info(id) +impl Context for Handle { + fn invocation_info(&self, id:ast::Id, name:Option<&str>) -> Option { + let info = self.computed_value_info_registry().get(&id)?; + match self.project.suggestion_db().lookup(info.method_call?) { + Ok(entry) => Some(controller::graph::entry_to_invocation_info(&entry)), + Err(_) => self.graph.borrow().invocation_info(id,name) + } } } @@ -301,6 +276,7 @@ pub mod tests { use utils::test::traits::*; use wasm_bindgen_test::wasm_bindgen_test; use wasm_bindgen_test::wasm_bindgen_test_configure; + use crate::model::module::NodeMetadata; wasm_bindgen_test_configure!(run_in_browser); @@ -367,4 +343,27 @@ pub mod tests { assert_eq!(typename_in_registry,expected_typename); notifications.expect_pending(); } + + #[test] + fn span_tree_context() { + use crate::test::mock; + let mut data = mock::Unified::new(); + data.set_inline_code("foo"); + + let mut fixture = data.bake(); + let mock::Fixture{graph,executed_graph,module,..} = &fixture; + + let entry = mock::data::suggestion_entry_foo(); + let node = &graph.nodes().unwrap()[0]; + let id = node.info.id(); + + let get_invocation_info = || executed_graph.invocation_info(id,Some("foo")); + + assert!(get_invocation_info().is_none()); + module.set_node_metadata(id,NodeMetadata { + position : None, + intended_method : entry.method_id(), + }); + assert!(get_invocation_info().is_some()); + } } diff --git a/src/rust/ide/src/lib.rs b/src/rust/ide/src/lib.rs index 65c7f3ca2c..ea8a2b8875 100644 --- a/src/rust/ide/src/lib.rs +++ b/src/rust/ide/src/lib.rs @@ -14,6 +14,7 @@ #![feature(matches_macro)] #![feature(result_cloned)] #![feature(slice_patterns)] +#![feature(result_map_or_else)] #![recursion_limit="256"] #![warn(missing_docs)] #![warn(trivial_casts)] diff --git a/src/rust/ide/src/test.rs b/src/rust/ide/src/test.rs index 81738abae8..e9c4d49897 100644 --- a/src/rust/ide/src/test.rs +++ b/src/rust/ide/src/test.rs @@ -12,6 +12,9 @@ pub mod mock { /// consistent data. #[allow(missing_docs)] pub mod data { + use crate::double_representation::module; + use crate::model::suggestion_database; + use enso_protocol::language_server::Position; use uuid::Uuid; @@ -28,7 +31,7 @@ pub mod mock { crate::model::module::Path::from_name_segments(ROOT_ID, &[MODULE_NAME]).unwrap() } - pub fn module_qualified_name() -> crate::double_representation::module::QualifiedName { + pub fn module_qualified_name() -> module::QualifiedName { module_path().qualified_module_name(PROJECT_NAME) } @@ -41,7 +44,46 @@ pub mod mock { } pub fn suggestion_db() -> crate::model::SuggestionDatabase { - crate::model::SuggestionDatabase::default() + let entry1 = suggestion_entry_foo(); + let entry2 = suggestion_entry_bar(); + let entries = vec![(&1,&entry1),(&2,&entry2)]; + let logger = logger::enabled::Logger::default(); + crate::model::SuggestionDatabase::new_from_entries(logger,entries) + } + + pub fn base_this_parameter() -> suggestion_database::Argument { + suggestion_database::Argument { + name : "this".to_owned(), + repr_type : "Base".to_owned(), + is_suspended : false, + default_value : None, + } + } + + pub fn suggestion_entry_foo() -> suggestion_database::Entry { + suggestion_database::Entry { + name : "foo".to_owned(), + module : module::QualifiedName::from_segments("Std",&["Base"]).unwrap(), + self_type : Some("Base".to_owned()), + arguments : vec![base_this_parameter()], + return_type : "Any".to_owned(), + kind : suggestion_database::EntryKind::Method, + scope : suggestion_database::Scope::Everywhere, + documentation : None + } + } + + pub fn suggestion_entry_bar() -> suggestion_database::Entry { + suggestion_database::Entry { + name : "bar".to_owned(), + module : module::QualifiedName::from_segments("Std",&["Base"]).unwrap(), + self_type : Some("Base".to_owned()), + arguments : vec![base_this_parameter()], + return_type : "Any".to_owned(), + kind : suggestion_database::EntryKind::Method, + scope : suggestion_database::Scope::Everywhere, + documentation : None + } } } @@ -61,6 +103,14 @@ pub mod mock { } impl Unified { + pub fn set_inline_code(&mut self, code:impl AsRef) { + let method = self.method_pointer(); + //self.code = format!("{}.{} = {}",method.defined_on_type,method.name,code.as_ref()) + // FIXME [mwu] should be as above but definition lookup doesn't handle properly implicit + // module name + self.code = format!("{} = {}",method.name,code.as_ref()) + } + pub fn new() -> Self { use crate::test::mock::data::*; Unified { @@ -125,7 +175,7 @@ pub mod mock { Rc::new(project) } - pub fn bake(&self) -> Baked { + pub fn bake(&self) -> Fixture { let logger = Logger::default(); // TODO let module = self.module(); let suggestion_db = Rc::new(model::SuggestionDatabase::new_from_entries(logger, @@ -135,7 +185,7 @@ pub mod mock { let project = self.project(module.clone_ref(),execution.clone_ref()); let executed_graph = controller::ExecutedGraph::new_internal(graph.clone_ref(), project.clone_ref(),execution.clone_ref()); - Baked { + Fixture { data : self.clone(), module, graph, @@ -148,7 +198,7 @@ pub mod mock { } #[derive(Clone,Debug)] - pub struct Baked { + pub struct Fixture { pub data : Unified, pub module : model::Module, pub graph : controller::Graph, @@ -158,7 +208,7 @@ pub mod mock { pub project : model::Project, } - impl Baked { + impl Fixture { // pub fn module(&mut self) -> crate::model::Module { // self.module.get_or_insert(self.data.module()).clone_ref() // } From c1f089da9aca69a61c055cd77650f0f57581bc66 Mon Sep 17 00:00:00 2001 From: mwu Date: Fri, 4 Sep 2020 18:14:31 +0200 Subject: [PATCH 10/34] more tests and fixes --- .../src/language_server/types.rs | 10 +++ src/rust/ide/src/controller/graph.rs | 34 ++-------- src/rust/ide/src/controller/graph/executed.rs | 58 +++++++++++------ .../src/double_representation/identifier.rs | 1 + src/rust/ide/src/model/suggestion_database.rs | 23 +++++++ src/rust/ide/src/test.rs | 62 +++++++++++++------ 6 files changed, 121 insertions(+), 67 deletions(-) diff --git a/src/rust/ide/lib/enso-protocol/src/language_server/types.rs b/src/rust/ide/lib/enso-protocol/src/language_server/types.rs index 20a8c9b058..4270a227a9 100644 --- a/src/rust/ide/lib/enso-protocol/src/language_server/types.rs +++ b/src/rust/ide/lib/enso-protocol/src/language_server/types.rs @@ -656,4 +656,14 @@ pub mod test { method_pointer : None, } } + + /// Generate `ExpressionValueUpdate` with update for a single expression bringing only the + /// method pointer. + pub fn value_update_with_method_ptr(id:ExpressionId, method_pointer:SuggestionId) -> ExpressionValueUpdate { + ExpressionValueUpdate { + expression_id : id, + typename : None, + method_pointer : Some(method_pointer), + } + } } diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 15b008b7b2..c2e752f758 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -20,7 +20,6 @@ use ast::crumbs::InfixCrumb; use enso_protocol::language_server; use parser::Parser; use span_tree::SpanTree; -use span_tree::ParameterInfo; use span_tree::action::Actions; use span_tree::action::Action; use span_tree::generate::Context as SpanTreeContext; @@ -30,6 +29,7 @@ pub use crate::double_representation::graph::LocationHint; pub use crate::double_representation::graph::Id; + // ============== // === Errors === // ============== @@ -260,32 +260,6 @@ impl Connections { -// =========================================== -// === Suggestion DB <=> Span Tree Interop === -// =========================================== -// TODO Reconsider where this should be placed. Unfortunately in neither of the respective crates. - -/// Generate parameter info description from the suggestion database information. -pub fn entry_param_to_span_tree_param_info -(param_info:&model::suggestion_database::Argument) -> ParameterInfo { - ParameterInfo { - // TODO check if suggestion database actually always know the name and type - // or if it can contain just some empty strings or sth - name : Some(param_info.name.clone()), - typename : Some(param_info.repr_type.clone()), - } -} - -/// Generate invocation info description from the suggestion database information. -pub fn entry_to_invocation_info(entry:&model::suggestion_database::Entry) -> InvocationInfo { - let parameters = entry.arguments.iter().map(entry_param_to_span_tree_param_info).collect(); - InvocationInfo {parameters} -} - -/////////////////////////////////////////////////////////////////////// - - - // ================= // === Utilities === // ================= @@ -861,7 +835,7 @@ impl span_tree::generate::Context for Handle { } else { true }; - matching.then_with(|| entry_to_invocation_info(&db_entry)) + matching.then_with(|| db_entry.invocation_info()) } } @@ -877,7 +851,7 @@ pub mod tests { use crate::double_representation::identifier::NormalizedName; use crate::executor::test_utils::TestWithLocalPoolExecutor; - use crate::model::module::{Position, MethodId}; + use crate::model::module::Position; use ast::crumbs; use ast::test_utils::expect_shape; @@ -1049,7 +1023,7 @@ main = }) } - #[test] + #[wasm_bindgen_test] fn span_tree_context_handling_metadata_and_name() { let entry = crate::test::mock::data::suggestion_entry_foo(); let mut test = Fixture::set_up(); diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index c56386869f..fbdf86eb76 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -248,14 +248,16 @@ impl Handle { // === Span Tree Context === /// Span Tree generation context for a graph that does not know about execution. -/// Provides information based on metadata entries. +/// Provides information based on computed value registry, using metadata as a fallback. impl Context for Handle { fn invocation_info(&self, id:ast::Id, name:Option<&str>) -> Option { - let info = self.computed_value_info_registry().get(&id)?; - match self.project.suggestion_db().lookup(info.method_call?) { - Ok(entry) => Some(controller::graph::entry_to_invocation_info(&entry)), - Err(_) => self.graph.borrow().invocation_info(id,name) - } + let lookup_registry = || { + let info = self.computed_value_info_registry().get(&id)?; + let entry = self.project.suggestion_db().lookup(info.method_call?).ok()?; + Some(entry.invocation_info()) + }; + let fallback = || self.graph.borrow().invocation_info(id,name); + lookup_registry().or_else(fallback) } } @@ -272,7 +274,7 @@ pub mod tests { use crate::executor::test_utils::TestWithLocalPoolExecutor; use crate::model::execution_context::ExpressionId; - use enso_protocol::language_server::types::test::value_update_with_type; + use enso_protocol::language_server::types::test::{value_update_with_type, value_update_with_method_ptr}; use utils::test::traits::*; use wasm_bindgen_test::wasm_bindgen_test; use wasm_bindgen_test::wasm_bindgen_test_configure; @@ -344,26 +346,46 @@ pub mod tests { notifications.expect_pending(); } - #[test] + #[wasm_bindgen_test] fn span_tree_context() { use crate::test::mock; let mut data = mock::Unified::new(); data.set_inline_code("foo"); - let mut fixture = data.bake(); - let mock::Fixture{graph,executed_graph,module,..} = &fixture; - - let entry = mock::data::suggestion_entry_foo(); - let node = &graph.nodes().unwrap()[0]; - let id = node.info.id(); - + let mock::Fixture{graph,executed_graph,module,suggestion_db,..} = &data.fixture(); + let id = graph.nodes().unwrap()[0].info.id(); let get_invocation_info = || executed_graph.invocation_info(id,Some("foo")); - assert!(get_invocation_info().is_none()); + + // Check that if we set metadata, executed graph can see this info. + let entry1 = suggestion_db.lookup(1).unwrap(); module.set_node_metadata(id,NodeMetadata { position : None, - intended_method : entry.method_id(), + intended_method : entry1.method_id(), }); - assert!(get_invocation_info().is_some()); + let info = get_invocation_info().unwrap(); + match info.parameters.as_slice() { + [param] => { + let expected = &entry1.arguments[0]; + let expected = model::suggestion_database::to_span_tree_param(expected); + assert_eq!(param,&expected); + } + _ => panic!("Expected only single parameter!"), + }; + + // Now send update that expression actually was computed to be a call to the second + // suggestion entry and check that executed graph provides this info over the metadata one. + let entry2 = suggestion_db.lookup(2).unwrap(); + let update = value_update_with_method_ptr(id,2); + executed_graph.computed_value_info_registry().apply_updates(vec![update]); + let info = get_invocation_info().unwrap(); + match info.parameters.as_slice() { + [param] => { + let expected = &entry2.arguments[0]; + let expected = model::suggestion_database::to_span_tree_param(expected); + assert_eq!(param,&expected); + } + _ => panic!("Expected only single parameter!"), + }; } } diff --git a/src/rust/ide/src/double_representation/identifier.rs b/src/rust/ide/src/double_representation/identifier.rs index 04a6db223e..208bb87d46 100644 --- a/src/rust/ide/src/double_representation/identifier.rs +++ b/src/rust/ide/src/double_representation/identifier.rs @@ -89,6 +89,7 @@ impl Identifier { } } + /// Get a normalized version of this identifier. pub fn normalized(&self) -> NormalizedName { NormalizedName::new(self.name()) } diff --git a/src/rust/ide/src/model/suggestion_database.rs b/src/rust/ide/src/model/suggestion_database.rs index 893b576e3e..5af9784b4e 100644 --- a/src/rust/ide/src/model/suggestion_database.rs +++ b/src/rust/ide/src/model/suggestion_database.rs @@ -197,6 +197,11 @@ impl Entry { let output = parser.generate_html_doc_pure(doc); Ok(output?) } + + /// Generate information about invoking this entity for span tree context. + pub fn invocation_info(&self) -> span_tree::generate::context::InvocationInfo { + self.into() + } } impl TryFrom for Entry { @@ -228,6 +233,24 @@ impl TryFrom for language_server::MethodPointer { } } +impl From<&Entry> for span_tree::generate::context::InvocationInfo { + fn from(entry:&Entry) -> span_tree::generate::context::InvocationInfo { + let parameters = entry.arguments.iter().map(to_span_tree_param).collect(); + span_tree::generate::context::InvocationInfo{parameters} + } +} + +/// Converts the information about function parameter from suggestion database into the form used +/// by the span tree nodes. +pub fn to_span_tree_param +(param_info:&Argument) -> span_tree::ParameterInfo { + span_tree::ParameterInfo { + // TODO [mwu] Check if database actually do must always have both of these filled. + name : Some(param_info.name.clone()), + typename : Some(param_info.repr_type.clone()), + } +} + // ================ diff --git a/src/rust/ide/src/test.rs b/src/rust/ide/src/test.rs index e9c4d49897..aba343ca21 100644 --- a/src/rust/ide/src/test.rs +++ b/src/rust/ide/src/test.rs @@ -5,6 +5,10 @@ pub mod mock { use crate::prelude::*; + use crate::double_representation::module; + use crate::model::suggestion_database; + use crate::executor::test_utils::TestWithLocalPoolExecutor; + /// Data used to create mock IDE components. /// /// Contains a number of constants and functions building more complex structures from them. @@ -12,8 +16,7 @@ pub mod mock { /// consistent data. #[allow(missing_docs)] pub mod data { - use crate::double_representation::module; - use crate::model::suggestion_database; + use super::*; use enso_protocol::language_server::Position; use uuid::Uuid; @@ -51,7 +54,7 @@ pub mod mock { crate::model::SuggestionDatabase::new_from_entries(logger,entries) } - pub fn base_this_parameter() -> suggestion_database::Argument { + pub fn foo_method_parameter() -> suggestion_database::Argument { suggestion_database::Argument { name : "this".to_owned(), repr_type : "Base".to_owned(), @@ -60,12 +63,21 @@ pub mod mock { } } + pub fn bar_method_parameter() -> suggestion_database::Argument { + suggestion_database::Argument { + name : "this".to_owned(), + repr_type : "Other".to_owned(), + is_suspended : false, + default_value : None, + } + } + pub fn suggestion_entry_foo() -> suggestion_database::Entry { suggestion_database::Entry { name : "foo".to_owned(), module : module::QualifiedName::from_segments("Std",&["Base"]).unwrap(), self_type : Some("Base".to_owned()), - arguments : vec![base_this_parameter()], + arguments : vec![foo_method_parameter()], return_type : "Any".to_owned(), kind : suggestion_database::EntryKind::Method, scope : suggestion_database::Scope::Everywhere, @@ -76,9 +88,9 @@ pub mod mock { pub fn suggestion_entry_bar() -> suggestion_database::Entry { suggestion_database::Entry { name : "bar".to_owned(), - module : module::QualifiedName::from_segments("Std",&["Base"]).unwrap(), - self_type : Some("Base".to_owned()), - arguments : vec![base_this_parameter()], + module : module::QualifiedName::from_segments("Std",&["Other"]).unwrap(), + self_type : Some("Other".to_owned()), + arguments : vec![bar_method_parameter()], return_type : "Any".to_owned(), kind : suggestion_database::EntryKind::Method, scope : suggestion_database::Scope::Everywhere, @@ -93,7 +105,7 @@ pub mod mock { pub project_name : String, pub module_path : model::module::Path, pub graph_id : double_representation::graph::Id, - pub suggestions : HashMap, + pub suggestions : HashMap, pub context_id : model::execution_context::Id, pub parser : parser::Parser, code : String, @@ -113,13 +125,16 @@ pub mod mock { pub fn new() -> Self { use crate::test::mock::data::*; + let mut suggestions = HashMap::new(); + suggestions.insert(1,suggestion_entry_foo()); + suggestions.insert(2,suggestion_entry_bar()); Unified { + suggestions, logger : Logger::default(), project_name : PROJECT_NAME.to_owned(), module_path : module_path(), graph_id : graph_id(), code : CODE.to_owned(), - suggestions : default(), id_map : default(), metadata : default(), context_id : CONTEXT_ID, @@ -134,7 +149,7 @@ pub mod mock { Rc::new(module) } - pub fn module_qualified_name(&self) -> double_representation::module::QualifiedName { + pub fn module_qualified_name(&self) -> module::QualifiedName { self.module_path.qualified_module_name(&self.project_name) } @@ -164,29 +179,37 @@ pub mod mock { Rc::new(model::execution_context::Plain::new(logger,self.method_pointer())) } - pub fn project(&self, module:model::Module, execution_context:model::ExecutionContext) - -> model::Project { + pub fn project + ( &self, module:model::Module + , execution_context:model::ExecutionContext + , suggestion_database:Rc + ) -> model::Project { let mut project = model::project::MockAPI::new(); model::project::test::expect_parser(&mut project,&self.parser); model::project::test::expect_module(&mut project,module); model::project::test::expect_execution_ctx(&mut project,execution_context); // Root ID is needed to generate module path used to get the module. model::project::test::expect_root_id(&mut project,crate::test::mock::data::ROOT_ID); + model::project::test::expect_suggestion_db(&mut project,suggestion_database); Rc::new(project) } - pub fn bake(&self) -> Fixture { - let logger = Logger::default(); // TODO - let module = self.module(); + pub fn fixture(&self) -> Fixture { + let logger = Logger::default(); // TODO + let module = self.module(); let suggestion_db = Rc::new(model::SuggestionDatabase::new_from_entries(logger, &self.suggestions)); - let graph = self.graph(module.clone_ref(), suggestion_db.clone_ref()); + let graph = self.graph(module.clone_ref(), suggestion_db.clone_ref()); let execution = self.execution_context(); - let project = self.project(module.clone_ref(),execution.clone_ref()); + let project = self.project(module.clone_ref(),execution.clone_ref(), + suggestion_db.clone_ref()); let executed_graph = controller::ExecutedGraph::new_internal(graph.clone_ref(), project.clone_ref(),execution.clone_ref()); + let executor = TestWithLocalPoolExecutor::set_up(); + let data = self.clone(); Fixture { - data : self.clone(), + executor, + data, module, graph, executed_graph, @@ -197,8 +220,9 @@ pub mod mock { } } - #[derive(Clone,Debug)] + #[derive(Debug)] pub struct Fixture { + pub executor : TestWithLocalPoolExecutor, pub data : Unified, pub module : model::Module, pub graph : controller::Graph, From dd6584862beee67e9291c6c15714c4c3b445434d Mon Sep 17 00:00:00 2001 From: mwu Date: Fri, 4 Sep 2020 18:31:13 +0200 Subject: [PATCH 11/34] bump size, mark tests as wasm --- build/run.js | 2 +- src/rust/ide/src/controller/searcher.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build/run.js b/build/run.js index 42299b95da..9afd0bd02f 100755 --- a/build/run.js +++ b/build/run.js @@ -136,7 +136,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.83 + let limit = 3.84 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/src/rust/ide/src/controller/searcher.rs b/src/rust/ide/src/controller/searcher.rs index 88fb02cb88..ef2ccaa146 100644 --- a/src/rust/ide/src/controller/searcher.rs +++ b/src/rust/ide/src/controller/searcher.rs @@ -1315,7 +1315,7 @@ mod test { } } - #[test] + #[wasm_bindgen_test] fn committing_node() { let Fixture{test:_test,mut searcher,entry4,..} = Fixture::new(); let module = searcher.graph.graph().module.clone_ref(); From 787d6bad0d1fdd39e13802869a8f74a501dbf2e3 Mon Sep 17 00:00:00 2001 From: mwu Date: Sat, 5 Sep 2020 02:28:00 +0200 Subject: [PATCH 12/34] cleanups & fixes --- src/rust/ide/lib/span-tree/src/action.rs | 14 +++--- src/rust/ide/lib/span-tree/src/generate.rs | 38 ++++++---------- src/rust/ide/lib/span-tree/src/lib.rs | 1 + src/rust/ide/src/double_representation.rs | 24 +++++----- src/rust/ide/src/model/project.rs | 11 +++++ src/rust/ide/src/test.rs | 36 ++++++++++++--- src/rust/ide/src/tests.rs | 51 +++++++++++++++++++++- 7 files changed, 121 insertions(+), 54 deletions(-) diff --git a/src/rust/ide/lib/span-tree/src/action.rs b/src/rust/ide/lib/span-tree/src/action.rs index 2f56e84100..5500b3eb2d 100644 --- a/src/rust/ide/lib/span-tree/src/action.rs +++ b/src/rust/ide/lib/span-tree/src/action.rs @@ -13,7 +13,6 @@ use ast::opr::ArgWithOffset; - /// ============== /// === Errors === /// ============== @@ -129,7 +128,7 @@ impl<'a> Implementation for node::Ref<'a> { Append if has_arg => infix.push_operand(item), Append => *last_arg = Some(item), // TODO? below should never happen, as operator arity is always fixed to 2? - ExpectedArgument(i) => infix.insert_operand(*i, item), + ExpectedArgument(i) => infix.insert_operand(*i, item), }; infix.into_ast() } else { @@ -139,9 +138,9 @@ impl<'a> Implementation for node::Ref<'a> { prefix_id : None, }; match ins_type { - BeforeTarget => prefix.insert_arg(0,item), - AfterTarget => prefix.insert_arg(1,item), - Append => prefix.args.push(item), + BeforeTarget => prefix.insert_arg(0,item), + AfterTarget => prefix.insert_arg(1,item), + Append => prefix.args.push(item), ExpectedArgument(i) => prefix.insert_arg(*i, item), } prefix.into_ast() @@ -208,13 +207,12 @@ mod test { use crate::node::Kind::Target; use crate::node::InsertType::ExpectedArgument; - use wasm_bindgen_test::wasm_bindgen_test; - use parser::Parser; use ast::HasRepr; use data::text::Index; use data::text::Span; use std::ops::Range; - + use parser::Parser; + use wasm_bindgen_test::wasm_bindgen_test; #[wasm_bindgen_test] fn actions_in_span_tree() { diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 2a93a2a065..0abc39bd84 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -122,9 +122,10 @@ impl SpanTreeGenerator for Ast { _ => { let size = Size::new(self.len()); let expression_id = self.id; - let children = default(); - // TODO - if let Some(info) = expression_id.and_then(|id| context.invocation_info(id,None)) { + let children = default(); + // TODO [mwu] handle cases where Ast is like "here.foo" + let name = ast::identifier::name(self); + if let Some(info) = self.id.and_then(|id| context.invocation_info(id,name)) { let node = Node { size, children, @@ -213,7 +214,9 @@ impl SpanTreeGenerator for ast::opr::Chain { impl SpanTreeGenerator for ast::prefix::Chain { fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { - let invocation_info = self.id().and_then(|id| context.invocation_info(id,None)); // TODO + // TODO [mwu] handle cases where Ast is like "here.foo" + let name = ast::identifier::name(&self.func); + let invocation_info = self.id().and_then(|id| context.invocation_info(id,name)); let invocation_info = invocation_info.as_ref(); let known_args = invocation_info.is_some(); dbg!(&invocation_info); @@ -273,22 +276,6 @@ impl SpanTreeGenerator for ast::prefix::Chain { } } -fn generate_known_parameter -(node:Node, kind:node::Kind, index:usize, arity:usize, parameter_info:ParameterInfo) -> Node { - println!("Will generate missing argument node for {:?}",parameter_info); - let is_last = index + 1 == arity; - let mut gen = ChildGenerator::default(); - gen.add_node(vec![],node); - let arg_node = gen.generate_empty_node(InsertType::ExpectedArgument(index)); - arg_node.node.parameter_info = Some(parameter_info); - Node { - kind : if is_last {kind} else {node::Kind::Chained}, - size : gen.current_offset, - children : gen.children, - expression_id : None, - parameter_info : None, - } -} // === Match === @@ -343,10 +330,10 @@ impl SpanTreeGenerator for ast::known::Ambiguous { fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { let mut gen = ChildGenerator::default(); let first_segment_index = 0; - generate_children_from_abiguous_segment(&mut gen,first_segment_index,&self.segs.head,context)?; + generate_children_from_ambiguous(&mut gen,first_segment_index,&self.segs.head,context)?; for (index,segment) in self.segs.tail.iter().enumerate() { gen.spacing(segment.off); - generate_children_from_abiguous_segment(&mut gen, index+1, &segment.wrapped,context)?; + generate_children_from_ambiguous(&mut gen, index+1, &segment.wrapped, context)?; } Ok(Node{kind, size : gen.current_offset, @@ -357,7 +344,7 @@ impl SpanTreeGenerator for ast::known::Ambiguous { } } -fn generate_children_from_abiguous_segment +fn generate_children_from_ambiguous (gen:&mut ChildGenerator, index:usize, segment:&MacroAmbiguousSegment, context:&impl Context) -> FallibleResult<()> { let is_removable = false; @@ -373,6 +360,7 @@ fn generate_children_from_abiguous_segment } + // ============ // === Test === // ============ @@ -388,6 +376,9 @@ mod test { use crate::node::Kind::*; use crate::node::InsertType::*; + use ast::Crumbs; + use ast::Id; + use ast::IdMap; use ast::crumbs::AmbiguousCrumb; use ast::crumbs::AmbiguousSegmentCrumb; use ast::crumbs::InfixCrumb; @@ -399,7 +390,6 @@ mod test { use parser::Parser; use wasm_bindgen_test::wasm_bindgen_test; use wasm_bindgen_test::wasm_bindgen_test_configure; - use ast::{IdMap, Id, Crumbs}; wasm_bindgen_test_configure!(run_in_browser); diff --git a/src/rust/ide/lib/span-tree/src/lib.rs b/src/rust/ide/lib/span-tree/src/lib.rs index 311e432f6a..8f7333b2f1 100644 --- a/src/rust/ide/lib/span-tree/src/lib.rs +++ b/src/rust/ide/lib/span-tree/src/lib.rs @@ -46,6 +46,7 @@ pub mod prelude { use traits::*; use prelude::*; + use crate::generate::Context; diff --git a/src/rust/ide/src/double_representation.rs b/src/rust/ide/src/double_representation.rs index a691472610..7aceaddfa9 100644 --- a/src/rust/ide/src/double_representation.rs +++ b/src/rust/ide/src/double_representation.rs @@ -32,18 +32,14 @@ use crate::double_representation::identifier::Identifier; /// Link: https://github.com/luna/enso/blob/main/doc/syntax/encoding.md pub const INDENT : usize = 4; -// struct InvocationTarget { -// func_name : Identifier, -// this_arg : Option, -// } -pub fn target_method_name(ast:&Ast) -> Option { - if let Some(chain) = ast::prefix::Chain::from_ast(ast) { - target_method_name(&chain.func) - } else if let Some(chain) = ast::opr::as_access_chain(ast) { - let presumed_name = chain.args.last()?; - identifier::Identifier::new(presumed_name.operand.as_ref()?.arg.clone_ref()) - } else { - identifier::Identifier::new(ast.clone()) - } -} +// pub fn target_method_name(ast:&Ast) -> Option { +// if let Some(chain) = ast::prefix::Chain::from_ast(ast) { +// target_method_name(&chain.func) +// } else if let Some(chain) = ast::opr::as_access_chain(ast) { +// let presumed_name = chain.args.last()?; +// identifier::Identifier::new(presumed_name.operand.as_ref()?.arg.clone_ref()) +// } else { +// identifier::Identifier::new(ast.clone()) +// } +// } diff --git a/src/rust/ide/src/model/project.rs b/src/rust/ide/src/model/project.rs index f068885606..fddabd11b0 100644 --- a/src/rust/ide/src/model/project.rs +++ b/src/rust/ide/src/model/project.rs @@ -114,4 +114,15 @@ pub mod test { pub fn expect_suggestion_db(project:&mut MockAPI, suggestion_db:Rc) { project.expect_suggestion_db().returning_st(move || suggestion_db.clone_ref()); } + + /// Sets up module expectation on the mock project, returning a give module. + pub fn expect_json_rpc(project:&mut MockAPI, json_rpc:Rc) { + project.expect_json_rpc().returning_st(move || json_rpc.clone_ref()); + } + + /// Sets up module expectation on the mock project, returning a give module. + pub fn expect_name(project:&mut MockAPI, name:impl Into) { + let name = ImString::new(name); + project.expect_name().returning_st(move || name.clone_ref()); + } } diff --git a/src/rust/ide/src/test.rs b/src/rust/ide/src/test.rs index aba343ca21..e94defbbdd 100644 --- a/src/rust/ide/src/test.rs +++ b/src/rust/ide/src/test.rs @@ -9,6 +9,8 @@ pub mod mock { use crate::model::suggestion_database; use crate::executor::test_utils::TestWithLocalPoolExecutor; + use enso_protocol::language_server; + /// Data used to create mock IDE components. /// /// Contains a number of constants and functions building more complex structures from them. @@ -180,33 +182,51 @@ pub mod mock { } pub fn project - ( &self, module:model::Module - , execution_context:model::ExecutionContext - , suggestion_database:Rc + ( &self + , module : model::Module + , execution_context : model::ExecutionContext + , suggestion_database : Rc + , json_client : language_server::MockClient ) -> model::Project { let mut project = model::project::MockAPI::new(); + model::project::test::expect_name(&mut project,&self.project_name); model::project::test::expect_parser(&mut project,&self.parser); model::project::test::expect_module(&mut project,module); model::project::test::expect_execution_ctx(&mut project,execution_context); // Root ID is needed to generate module path used to get the module. model::project::test::expect_root_id(&mut project,crate::test::mock::data::ROOT_ID); model::project::test::expect_suggestion_db(&mut project,suggestion_database); + let json_rpc = language_server::Connection::new_mock_rc(json_client); + model::project::test::expect_json_rpc(&mut project,json_rpc); Rc::new(project) } pub fn fixture(&self) -> Fixture { + self.fixture_customize(|_,_| {}) + } + + pub fn fixture_customize + (&self, customize_json_rpc:impl FnOnce(&Self,&mut language_server::MockClient)) + -> Fixture { + let mut json_client = language_server::MockClient::default(); + customize_json_rpc(self,&mut json_client); + let logger = Logger::default(); // TODO let module = self.module(); - let suggestion_db = Rc::new(model::SuggestionDatabase::new_from_entries(logger, + let suggestion_db = Rc::new(model::SuggestionDatabase::new_from_entries(&logger, &self.suggestions)); let graph = self.graph(module.clone_ref(), suggestion_db.clone_ref()); let execution = self.execution_context(); let project = self.project(module.clone_ref(),execution.clone_ref(), - suggestion_db.clone_ref()); + suggestion_db.clone_ref(),json_client); let executed_graph = controller::ExecutedGraph::new_internal(graph.clone_ref(), project.clone_ref(),execution.clone_ref()); - let executor = TestWithLocalPoolExecutor::set_up(); - let data = self.clone(); + let executor = TestWithLocalPoolExecutor::set_up(); + let data = self.clone(); + let selected_nodes = Vec::new(); + let searcher_mode = controller::searcher::Mode::NewNode {position:None}; + let searcher = controller::Searcher::new_from_graph_controller(&logger,&project, + executed_graph.clone_ref(),searcher_mode,selected_nodes).unwrap(); Fixture { executor, data, @@ -216,6 +236,7 @@ pub mod mock { execution, suggestion_db, project, + searcher, } } } @@ -230,6 +251,7 @@ pub mod mock { pub executed_graph : controller::ExecutedGraph, pub suggestion_db : Rc, pub project : model::Project, + pub searcher : controller::Searcher, } impl Fixture { diff --git a/src/rust/ide/src/tests.rs b/src/rust/ide/src/tests.rs index 4c5ad3032c..942a06a638 100644 --- a/src/rust/ide/src/tests.rs +++ b/src/rust/ide/src/tests.rs @@ -1,14 +1,16 @@ use super::prelude::*; +use crate::controller::graph::NodeTrees; use crate::transport::test_utils::TestWithMockedTransport; use crate::ide::IdeInitializer; +use enso_protocol::language_server::response::Completion; use enso_protocol::project_manager; use json_rpc::expect_call; use json_rpc::test_util::transport::mock::MockTransport; +use serde_json::json; use wasm_bindgen_test::wasm_bindgen_test_configure; use wasm_bindgen_test::wasm_bindgen_test; -use serde_json::json; wasm_bindgen_test_configure!(run_in_browser); @@ -75,3 +77,50 @@ async fn get_most_recent_project_or_create_new() { let project = project.await; assert_eq!(expected_project, project.expect("Couldn't get project.")) } + +#[wasm_bindgen_test] +fn span_tree_args() { + use crate::test::mock::*; + + let data = Unified::new(); + let fixture = data.fixture_customize(|_,json_client| { + for _ in 0..2 { + json_client.expect.completion(|_, _, _, _, _| { + Ok(Completion { + results : vec![1], + current_version : default(), + }) + }); + } + }); + let Fixture{graph,executed_graph,searcher,suggestion_db,..} = &fixture; + let entry = suggestion_db.lookup(1).unwrap(); + searcher.pick_completion(entry.clone_ref()).unwrap(); + let id = searcher.commit_node().unwrap(); + + let get_node = || graph.node(id).unwrap(); + let get_inputs = || NodeTrees::new(&get_node().info,executed_graph).unwrap().inputs; + let get_param1 = || get_inputs().root_ref().leaf_iter().nth(1).and_then(|node| { + node.parameter_info.clone() + }); + + let expected_param = model::suggestion_database::to_span_tree_param(&entry.arguments[0]); + + // TODO [mwu] The searcher inserts "Base.foo". This should work as well but needs redesigned + // target detection rules in the span tree. + // + + + assert_eq!(entry.name,"foo"); + graph.set_expression(id,"foo").unwrap(); + assert_eq!(get_param1().as_ref(), Some(&expected_param)); + + graph.set_expression(id,"foo Base").unwrap(); + assert_eq!(get_param1().as_ref(), Some(&expected_param)); + + graph.set_expression(id,"bar").unwrap(); + assert_eq!(get_param1(), None); + + graph.set_expression(id,"bar Base").unwrap(); + assert_eq!(get_param1(), None); +} \ No newline at end of file From 359c1ce620cefcf4dd9a536bfbf1d089b732d4d6 Mon Sep 17 00:00:00 2001 From: mwu Date: Mon, 7 Sep 2020 01:55:15 +0200 Subject: [PATCH 13/34] restored missing method --- src/rust/ide/lib/span-tree/src/generate.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 0abc39bd84..3d3c1b1c16 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -360,6 +360,26 @@ fn generate_children_from_ambiguous } +// === Common Utility == + +fn generate_known_parameter +(node:Node, kind:node::Kind, index:usize, arity:usize, parameter_info:ParameterInfo) -> Node { + println!("Will generate missing argument node for {:?}",parameter_info); + let is_last = index + 1 == arity; + let mut gen = ChildGenerator::default(); + gen.add_node(ast::Crumbs::new(),node); + let arg_node = gen.generate_empty_node(InsertType::ExpectedArgument(index)); + arg_node.node.parameter_info = Some(parameter_info); + Node { + kind : if is_last {kind} else {node::Kind::Chained}, + size : gen.current_offset, + children : gen.children, + expression_id : None, + parameter_info : None, + } +} + + // ============ // === Test === From 376a97f22ddd3c17d93742ea45249cfd18ee6d2b Mon Sep 17 00:00:00 2001 From: mwu Date: Mon, 7 Sep 2020 21:27:44 +0200 Subject: [PATCH 14/34] bad ideas --- src/rust/ide/lib/ast/impl/src/opr.rs | 16 ++- src/rust/ide/lib/parser/src/lib.rs | 29 ++++ src/rust/ide/lib/span-tree/src/generate.rs | 153 ++++++++++++++++++--- src/rust/ide/lib/span-tree/src/node.rs | 5 +- src/rust/ide/src/test.rs | 11 +- src/rust/ide/src/tests.rs | 10 +- 6 files changed, 195 insertions(+), 29 deletions(-) diff --git a/src/rust/ide/lib/ast/impl/src/opr.rs b/src/rust/ide/lib/ast/impl/src/opr.rs index 75d9188975..de5f14ddb0 100644 --- a/src/rust/ide/lib/ast/impl/src/opr.rs +++ b/src/rust/ide/lib/ast/impl/src/opr.rs @@ -345,14 +345,14 @@ impl Chain { /// Iterates over &Located, beginning with target (this argument) and then subsequent /// arguments. - pub fn enumerate_operands<'a> - (&'a self) -> impl Iterator>> + 'a { + pub fn enumerate_operands0<'a> + (&'a self) -> impl Iterator>>> + 'a { let rev_args = self.args.iter().rev(); let target_crumbs = rev_args.map(ChainElement::crumb_to_previous).collect_vec(); let target = self.target.as_ref(); - let loc_target = target.map(|opr| Located::new(target_crumbs,opr)).into_iter(); + let loc_target = std::iter::once(target.map(|opr| Located::new(target_crumbs,opr))); let args = self.args.iter().enumerate(); - let loc_args = args.filter_map(move |(i,elem)| { + let loc_args = args.map(move |(i,elem)| { elem.operand.as_ref().map(|operand| { let latter_args = self.args.iter().skip(i+1); let to_infix = latter_args.rev().map(ChainElement::crumb_to_previous); @@ -364,6 +364,14 @@ impl Chain { loc_target.chain(loc_args) } + + /// Iterates over &Located, beginning with target (this argument) and then subsequent + /// arguments. + pub fn enumerate_operands<'a> + (&'a self) -> impl Iterator>> + 'a { + self.enumerate_operands0().flatten() + } + /// Iterates over all operator's AST in this chain, starting from target side. pub fn enumerate_operators<'a>(&'a self) -> impl Iterator> + 'a { self.args.iter().enumerate().map(move |(i,elem)| { diff --git a/src/rust/ide/lib/parser/src/lib.rs b/src/rust/ide/lib/parser/src/lib.rs index 52fa84987d..f52982a557 100644 --- a/src/rust/ide/lib/parser/src/lib.rs +++ b/src/rust/ide/lib/parser/src/lib.rs @@ -170,3 +170,32 @@ impl DocParser { self.borrow_mut().generate_html_doc_pure(code) } } + +#[cfg(test)] +mod tests { + use ast::HasRepr; + use super::*; + use wasm_bindgen_test::wasm_bindgen_test; + + wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser); + + fn roundtrip_program(program:&str) { + let mut parser = Parser::new_or_panic(); + let ast = parser.parse(program.to_string(), Default::default()).unwrap(); + assert_eq!(ast.repr(), program, "{:#?}", ast); + } + + #[test] + fn roundtrip() { + let programs = vec![ + "main = 10 + 10", + "main =\n a = 10\n b = 20\n a * b", + "main =\n foo a =\n a * 10\n foo 10\n print \"hello\"", + "main =\n foo\n \n bar", + "main =\n \n foo\n \n bar" + ]; + for program in programs { + roundtrip_program(program); + } + } +} diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 3d3c1b1c16..9c1ee7945a 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -102,6 +102,32 @@ impl ChildGenerator { /// === Trait Implementations === /// ============================= +struct ApplicationBase<'a> { + function_name : Option <&'a str>, + has_target : bool, +} + +impl<'a> ApplicationBase<'a> { + fn new(ast:&'a Ast) -> Self { + if let Some(chain) = ast::opr::as_access_chain(ast) { + let get_name = || -> Option<&'a str> { + let crumbs = chain.enumerate_operands0().last()??.crumbs; + let ast = ast.get_traversing(&crumbs).ok()?; + ast::identifier::name(ast) + }; + ApplicationBase { + function_name : get_name(), + has_target : true, + } + } else { + ApplicationBase { + function_name : ast::identifier::name(ast), + has_target : false, + } + } + } +} + // === AST === @@ -136,7 +162,7 @@ impl SpanTreeGenerator for Ast { let arity = info.parameters.len(); let params = info.parameters.iter().cloned().enumerate(); Ok(params.fold(node,|node,(i,param)| { - generate_known_parameter(node,kind,i,arity,param) + generate_expected_argument(node, kind, i, arity, param) })) } else { let parameter_info = default(); @@ -214,23 +240,113 @@ impl SpanTreeGenerator for ast::opr::Chain { impl SpanTreeGenerator for ast::prefix::Chain { fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { - // TODO [mwu] handle cases where Ast is like "here.foo" - let name = ast::identifier::name(&self.func); - let invocation_info = self.id().and_then(|id| context.invocation_info(id,name)); + // TODO test for case when there are more arguments supplied than the known arity of function? + + //panic!(); + let base = ApplicationBase::new(&self.func); + let invocation_info = self.id().and_then(|id| context.invocation_info(id,base.function_name)); let invocation_info = invocation_info.as_ref(); let known_args = invocation_info.is_some(); - dbg!(&invocation_info); + println!("AST {} has name {:?} and info {:?}",self.clone().into_ast(),base.function_name,invocation_info); - // TODO test for case when there are more arguments supplied than the known arity of function? - let supplied_arg_count = self.args.len(); - let method_arity = invocation_info.map(|info| info.parameters.len()); - let arity = supplied_arg_count.max(method_arity.unwrap_or(0)); + // Skip the leading parameter info (`this`-like entity) if provided through the prefix base. + let known_parameters = match invocation_info { + Some(info) if base.has_target && info.parameters.len() > 0 => &info.parameters[1..], + Some(info) if !base.has_target => &info.parameters, + _ => &[], + }; + + let arity = self.args.len().max(known_parameters.len()); + // let missing_param_count = known_parameters.len().saturating_sub(self.args.len()); + // + // // Pad both provided arguments and known parameters with `None` and zip into a single stream + // // of `arity` length with enumerated index. + // let provided_hlp = self.args .map(Some).chain(std::iter::repeat(None)); + // let known_hlp = known_parameters.map(Some).chain(std::iter::repeat(None)); + // let arguments = provided_hlp.zip(known_hlp).take(arity).enumerate(); + // + // use ast::crumbs::PrefixCrumb::*; + // // Removing arguments is possible if there at least two of them + // let is_removable = self.args.len() >= 2; + // let node = self.func.generate_node(node::Kind::Operation,context); + // let params = provided_parameters.iter().map(Some).chain(std::iter::repeat(None)); + // let args = self.args.iter().zip(params); + // let ret = args.fold(node, |node,(arg,parameter_info)| { + // println!("Will generate argument node for {}, param is {:?}",arg.sast.wrapped,parameter_info); + // let node = node?; + // // TODO we can get i-th argument but we need to also take into account that prefix + // // target can be in a form of access chain that passes already `this` + // // if so everything should be shifted by one + // // But on the other hand -- the first "prefix" argument would not be a target + // // anymore then. + // let is_first = i == 0; + // let is_last = i + 1 == arity; + // let arg_kind = if is_first { node::Kind::Target {is_removable} } + // else { node::Kind::Argument {is_removable} }; + // + // let mut gen = ChildGenerator::default(); + // gen.add_node(vec![Func.into()],node); + // gen.spacing(arg.sast.off); + // if !known_args && matches!(arg_kind,node::Kind::Target {..}) { + // gen.generate_empty_node(InsertType::BeforeTarget); + // } + // let arg_ast = arg.sast.wrapped.clone_ref(); + // let arg_child = gen.generate_ast_node(Located::new(Arg,arg_ast),arg_kind,context)?; + // arg_child.node.parameter_info = parameter_info.cloned(); + // if !known_args { + // gen.generate_empty_node(InsertType::Append); + // } + // Ok(Node { + // kind : if is_last {kind} else {node::Kind::Chained}, + // size : gen.current_offset, + // children : gen.children, + // expression_id : arg.prefix_id, + // parameter_info : None, + // }) + // })?; + // + // if let Some(info) = invocation_info { + // let provided_args = self.args.len() + if base.has_target {1} else {0}; + // let missing_args = info.parameters.iter().cloned().enumerate().skip(provided_args); + // Ok(missing_args.fold(ret, |node,(i,param)| { + // generate_expected_argument(node, kind, i, arity, param) + // })) + // } else { + // Ok(ret) + // } + + + + + + // // Skip the leading parameter info (`this`-like entity) if provided through the prefix base. + // let known_parameters = invocation_info.and_then(|info| { + // if base.has_target { + // (info.parameters.len()>=1).as_some(&info.parameters[1..]) + // } else { + // Some(info.parameters.as_slice()) + // } + // }).unwrap_or(&[]); + // + // let provided_count = self.args.len(); + // let (provided_parameters,missing_parameters) = if known_parameters.len() >= provided_count { + // known_parameters.split_at(provided_count) + // } else { + // (known_parameters,[].as_ref()) + // }; + + // let mut parameter_iter = invocation_info.map(|info| info.parameters.iter()).iter().flatten(); + // + // let arity = provided_count.max(known_parameters.len()); + // + // let mut index = 0; // This is argument index in the prefix chain. + // use ast::crumbs::PrefixCrumb::*; // Removing arguments is possible if there at least two of them let is_removable = self.args.len() >= 2; let node = self.func.generate_node(node::Kind::Operation,context); - let ret = self.args.iter().enumerate().fold(node, |node,(i,arg)| { + let ret = self.args.iter().enumerate().fold(node, |node,(i,arg)| { println!("Will generate argument node for {}",arg.sast.wrapped); let node = node?; // TODO we can get i-th argument but we need to also take into account that prefix @@ -238,7 +354,6 @@ impl SpanTreeGenerator for ast::prefix::Chain { // if so everything should be shifted by one // But on the other hand -- the first "prefix" argument would not be a target // anymore then. - let argument_info = invocation_info.and_then(|info| info.parameters.get(i)); let is_first = i == 0; let is_last = i + 1 == arity; let arg_kind = if is_first { node::Kind::Target {is_removable} } @@ -252,7 +367,7 @@ impl SpanTreeGenerator for ast::prefix::Chain { } let arg_ast = arg.sast.wrapped.clone_ref(); let arg_child = gen.generate_ast_node(Located::new(Arg,arg_ast),arg_kind,context)?; - arg_child.node.parameter_info = argument_info.cloned(); + arg_child.node.parameter_info = known_parameters.get(i).cloned(); if !known_args { gen.generate_empty_node(InsertType::Append); } @@ -265,14 +380,10 @@ impl SpanTreeGenerator for ast::prefix::Chain { }) })?; - if let Some(info) = invocation_info { - let missing_args = info.parameters.iter().cloned().enumerate().skip(self.args.len()); - Ok(missing_args.fold(ret, |node,(i,param)| { - generate_known_parameter(node, kind, i, arity, param) - })) - } else { - Ok(ret) - } + let missing_arguments = known_parameters.iter().enumerate().skip(self.args.len()); + Ok(missing_arguments.fold(ret,|node,(i,parameter)| { + generate_expected_argument(node,kind,i,arity,param) + })) } } @@ -362,7 +473,7 @@ fn generate_children_from_ambiguous // === Common Utility == -fn generate_known_parameter +fn generate_expected_argument (node:Node, kind:node::Kind, index:usize, arity:usize, parameter_info:ParameterInfo) -> Node { println!("Will generate missing argument node for {:?}",parameter_info); let is_last = index + 1 == arity; diff --git a/src/rust/ide/lib/span-tree/src/node.rs b/src/rust/ide/lib/span-tree/src/node.rs index e1477ec985..a41fb8706f 100644 --- a/src/rust/ide/lib/span-tree/src/node.rs +++ b/src/rust/ide/lib/span-tree/src/node.rs @@ -58,7 +58,10 @@ pub enum InsertType { BeforeTarget, AfterTarget, Append, - /// Ast should be inserted as an argument at given index (counting the `this` argument). + /// Ast should be inserted as an argument at given index into the chain. + /// Note that this is just argument index in the application, it may be not the same as the + /// index of the function parameter, as `this` argument might be passed using the `this.func` + /// notation. ExpectedArgument(usize), } diff --git a/src/rust/ide/src/test.rs b/src/rust/ide/src/test.rs index e94defbbdd..a6b80b3fe4 100644 --- a/src/rust/ide/src/test.rs +++ b/src/rust/ide/src/test.rs @@ -65,6 +65,15 @@ pub mod mock { } } + pub fn foo_method_parameter2() -> suggestion_database::Argument { + suggestion_database::Argument { + name : "param1".to_owned(), + repr_type : "Number".to_owned(), + is_suspended : false, + default_value : None, + } + } + pub fn bar_method_parameter() -> suggestion_database::Argument { suggestion_database::Argument { name : "this".to_owned(), @@ -79,7 +88,7 @@ pub mod mock { name : "foo".to_owned(), module : module::QualifiedName::from_segments("Std",&["Base"]).unwrap(), self_type : Some("Base".to_owned()), - arguments : vec![foo_method_parameter()], + arguments : vec![foo_method_parameter(),foo_method_parameter2()], return_type : "Any".to_owned(), kind : suggestion_database::EntryKind::Method, scope : suggestion_database::Scope::Everywhere, diff --git a/src/rust/ide/src/tests.rs b/src/rust/ide/src/tests.rs index 942a06a638..0e692868e7 100644 --- a/src/rust/ide/src/tests.rs +++ b/src/rust/ide/src/tests.rs @@ -78,7 +78,7 @@ async fn get_most_recent_project_or_create_new() { assert_eq!(expected_project, project.expect("Couldn't get project.")) } -#[wasm_bindgen_test] +#[test] fn span_tree_args() { use crate::test::mock::*; @@ -109,6 +109,12 @@ fn span_tree_args() { // TODO [mwu] The searcher inserts "Base.foo". This should work as well but needs redesigned // target detection rules in the span tree. // + graph.set_expression(id,"Base.foo 50").unwrap(); + println!("{:#?}",get_inputs().root_ref().leaf_iter().collect_vec()); + + + return; + panic!("{}",get_node().info.expression().repr()); assert_eq!(entry.name,"foo"); @@ -123,4 +129,4 @@ fn span_tree_args() { graph.set_expression(id,"bar Base").unwrap(); assert_eq!(get_param1(), None); -} \ No newline at end of file +} From 9f3a04d802271737706cee78c325da8c7cbaaa68 Mon Sep 17 00:00:00 2001 From: mwu Date: Mon, 7 Sep 2020 21:28:02 +0200 Subject: [PATCH 15/34] now hidden --- src/rust/ide/lib/span-tree/src/generate.rs | 86 +--------------------- 1 file changed, 1 insertion(+), 85 deletions(-) diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 9c1ee7945a..16c508524e 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -256,92 +256,8 @@ impl SpanTreeGenerator for ast::prefix::Chain { Some(info) if !base.has_target => &info.parameters, _ => &[], }; - let arity = self.args.len().max(known_parameters.len()); - // let missing_param_count = known_parameters.len().saturating_sub(self.args.len()); - // - // // Pad both provided arguments and known parameters with `None` and zip into a single stream - // // of `arity` length with enumerated index. - // let provided_hlp = self.args .map(Some).chain(std::iter::repeat(None)); - // let known_hlp = known_parameters.map(Some).chain(std::iter::repeat(None)); - // let arguments = provided_hlp.zip(known_hlp).take(arity).enumerate(); - // - // use ast::crumbs::PrefixCrumb::*; - // // Removing arguments is possible if there at least two of them - // let is_removable = self.args.len() >= 2; - // let node = self.func.generate_node(node::Kind::Operation,context); - // let params = provided_parameters.iter().map(Some).chain(std::iter::repeat(None)); - // let args = self.args.iter().zip(params); - // let ret = args.fold(node, |node,(arg,parameter_info)| { - // println!("Will generate argument node for {}, param is {:?}",arg.sast.wrapped,parameter_info); - // let node = node?; - // // TODO we can get i-th argument but we need to also take into account that prefix - // // target can be in a form of access chain that passes already `this` - // // if so everything should be shifted by one - // // But on the other hand -- the first "prefix" argument would not be a target - // // anymore then. - // let is_first = i == 0; - // let is_last = i + 1 == arity; - // let arg_kind = if is_first { node::Kind::Target {is_removable} } - // else { node::Kind::Argument {is_removable} }; - // - // let mut gen = ChildGenerator::default(); - // gen.add_node(vec![Func.into()],node); - // gen.spacing(arg.sast.off); - // if !known_args && matches!(arg_kind,node::Kind::Target {..}) { - // gen.generate_empty_node(InsertType::BeforeTarget); - // } - // let arg_ast = arg.sast.wrapped.clone_ref(); - // let arg_child = gen.generate_ast_node(Located::new(Arg,arg_ast),arg_kind,context)?; - // arg_child.node.parameter_info = parameter_info.cloned(); - // if !known_args { - // gen.generate_empty_node(InsertType::Append); - // } - // Ok(Node { - // kind : if is_last {kind} else {node::Kind::Chained}, - // size : gen.current_offset, - // children : gen.children, - // expression_id : arg.prefix_id, - // parameter_info : None, - // }) - // })?; - // - // if let Some(info) = invocation_info { - // let provided_args = self.args.len() + if base.has_target {1} else {0}; - // let missing_args = info.parameters.iter().cloned().enumerate().skip(provided_args); - // Ok(missing_args.fold(ret, |node,(i,param)| { - // generate_expected_argument(node, kind, i, arity, param) - // })) - // } else { - // Ok(ret) - // } - - - - - - // // Skip the leading parameter info (`this`-like entity) if provided through the prefix base. - // let known_parameters = invocation_info.and_then(|info| { - // if base.has_target { - // (info.parameters.len()>=1).as_some(&info.parameters[1..]) - // } else { - // Some(info.parameters.as_slice()) - // } - // }).unwrap_or(&[]); - // - // let provided_count = self.args.len(); - // let (provided_parameters,missing_parameters) = if known_parameters.len() >= provided_count { - // known_parameters.split_at(provided_count) - // } else { - // (known_parameters,[].as_ref()) - // }; - - // let mut parameter_iter = invocation_info.map(|info| info.parameters.iter()).iter().flatten(); - // - // let arity = provided_count.max(known_parameters.len()); - // - // let mut index = 0; // This is argument index in the prefix chain. - // + use ast::crumbs::PrefixCrumb::*; // Removing arguments is possible if there at least two of them let is_removable = self.args.len() >= 2; From c92fa613d864dcae4292162ba6f816eeb20f4adb Mon Sep 17 00:00:00 2001 From: mwu Date: Tue, 8 Sep 2020 00:36:01 +0200 Subject: [PATCH 16/34] [wip] --- src/rust/ide/lib/span-tree/src/generate.rs | 52 ++++++++++++++-------- src/rust/ide/src/tests.rs | 5 +++ 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 16c508524e..5889a14172 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -20,7 +20,8 @@ use ast::opr::GeneralizedInfix; use data::text::Size; pub use context::Context; - +use utils::vec::VecExt; +use crate::generate::context::InvocationInfo; // ============= @@ -126,6 +127,13 @@ impl<'a> ApplicationBase<'a> { } } } + + fn prefix_params(&self, mut invocation_info:InvocationInfo) -> Vec { + if self.has_target { + invocation_info.parameters.pop_front() + } + invocation_info.parameters + } } @@ -134,7 +142,21 @@ impl<'a> ApplicationBase<'a> { impl SpanTreeGenerator for Ast { fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { if let Some(infix) = GeneralizedInfix::try_new(self) { - infix.flatten().generate_node(kind,context) + use ast::opr::is_access_opr; + let chain = infix.flatten(); + let app_base = is_access_opr(infix.opr.ast()).as_some_from(|| { + ApplicationBase::new(self) + }); + let invocation = || -> Option { + context.invocation_info(self.id?,app_base?.function_name) + }(); + if invocation.contains_if(|info| info.parameters.len() > 1) { + let invocation = invocation.unwrap(); // we just checked that it contains sth + let node = chain.generate_node(node::Kind::Operation,context); + todo!() + } else { + chain.generate_node(kind,context) + } } else { match self.shape() { ast::Shape::Prefix(_) => @@ -245,19 +267,16 @@ impl SpanTreeGenerator for ast::prefix::Chain { //panic!(); let base = ApplicationBase::new(&self.func); let invocation_info = self.id().and_then(|id| context.invocation_info(id,base.function_name)); - let invocation_info = invocation_info.as_ref(); let known_args = invocation_info.is_some(); println!("AST {} has name {:?} and info {:?}",self.clone().into_ast(),base.function_name,invocation_info); - - + let mut known_parameters = invocation_info.map(|info| info.parameters).unwrap_or_default(); // Skip the leading parameter info (`this`-like entity) if provided through the prefix base. - let known_parameters = match invocation_info { - Some(info) if base.has_target && info.parameters.len() > 0 => &info.parameters[1..], - Some(info) if !base.has_target => &info.parameters, - _ => &[], - }; + if base.has_target { + known_parameters.pop_front(); + } + let arity = self.args.len().max(known_parameters.len()); - + use ast::crumbs::PrefixCrumb::*; // Removing arguments is possible if there at least two of them let is_removable = self.args.len() >= 2; @@ -265,11 +284,6 @@ impl SpanTreeGenerator for ast::prefix::Chain { let ret = self.args.iter().enumerate().fold(node, |node,(i,arg)| { println!("Will generate argument node for {}",arg.sast.wrapped); let node = node?; - // TODO we can get i-th argument but we need to also take into account that prefix - // target can be in a form of access chain that passes already `this` - // if so everything should be shifted by one - // But on the other hand -- the first "prefix" argument would not be a target - // anymore then. let is_first = i == 0; let is_last = i + 1 == arity; let arg_kind = if is_first { node::Kind::Target {is_removable} } @@ -296,9 +310,9 @@ impl SpanTreeGenerator for ast::prefix::Chain { }) })?; - let missing_arguments = known_parameters.iter().enumerate().skip(self.args.len()); + let missing_arguments = known_parameters.into_iter().enumerate().skip(self.args.len()); Ok(missing_arguments.fold(ret,|node,(i,parameter)| { - generate_expected_argument(node,kind,i,arity,param) + generate_expected_argument(node,kind,i,arity,parameter) })) } } @@ -406,6 +420,8 @@ fn generate_expected_argument } } +//fn fill_with_expected_args(base:Node, kind:node::Kind, explicit_args:usize, arity:usi) + // ============ diff --git a/src/rust/ide/src/tests.rs b/src/rust/ide/src/tests.rs index 0e692868e7..2d0be457a1 100644 --- a/src/rust/ide/src/tests.rs +++ b/src/rust/ide/src/tests.rs @@ -109,6 +109,11 @@ fn span_tree_args() { // TODO [mwu] The searcher inserts "Base.foo". This should work as well but needs redesigned // target detection rules in the span tree. // + + + println!("{:#?}",get_inputs().root_ref().leaf_iter().collect_vec()); + return; + graph.set_expression(id,"Base.foo 50").unwrap(); println!("{:#?}",get_inputs().root_ref().leaf_iter().collect_vec()); From cd7ca7627351bb92d9b7b6ff78c7b512bd5bd92b Mon Sep 17 00:00:00 2001 From: mwu Date: Wed, 9 Sep 2020 03:50:40 +0200 Subject: [PATCH 17/34] support for method notation and other fixes / improvements --- src/rust/ide/lib/parser/src/lib.rs | 29 ----- src/rust/ide/lib/parser/tests/parsing.rs | 21 ++++ src/rust/ide/lib/span-tree/src/generate.rs | 114 ++++++++++-------- .../ide/lib/span-tree/src/generate/context.rs | 12 +- src/rust/ide/lib/span-tree/src/lib.rs | 1 + src/rust/ide/src/controller/graph.rs | 6 +- src/rust/ide/src/controller/graph/executed.rs | 8 +- src/rust/ide/src/double_representation.rs | 16 --- src/rust/ide/src/model/suggestion_database.rs | 8 +- src/rust/ide/src/tests.rs | 75 +++++++++--- 10 files changed, 158 insertions(+), 132 deletions(-) diff --git a/src/rust/ide/lib/parser/src/lib.rs b/src/rust/ide/lib/parser/src/lib.rs index f52982a557..52fa84987d 100644 --- a/src/rust/ide/lib/parser/src/lib.rs +++ b/src/rust/ide/lib/parser/src/lib.rs @@ -170,32 +170,3 @@ impl DocParser { self.borrow_mut().generate_html_doc_pure(code) } } - -#[cfg(test)] -mod tests { - use ast::HasRepr; - use super::*; - use wasm_bindgen_test::wasm_bindgen_test; - - wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser); - - fn roundtrip_program(program:&str) { - let mut parser = Parser::new_or_panic(); - let ast = parser.parse(program.to_string(), Default::default()).unwrap(); - assert_eq!(ast.repr(), program, "{:#?}", ast); - } - - #[test] - fn roundtrip() { - let programs = vec![ - "main = 10 + 10", - "main =\n a = 10\n b = 20\n a * b", - "main =\n foo a =\n a * 10\n foo 10\n print \"hello\"", - "main =\n foo\n \n bar", - "main =\n \n foo\n \n bar" - ]; - for program in programs { - roundtrip_program(program); - } - } -} diff --git a/src/rust/ide/lib/parser/tests/parsing.rs b/src/rust/ide/lib/parser/tests/parsing.rs index 4ad386bf4f..fed584118e 100644 --- a/src/rust/ide/lib/parser/tests/parsing.rs +++ b/src/rust/ide/lib/parser/tests/parsing.rs @@ -32,6 +32,12 @@ fn assert_opr>(ast:&Ast, name:StringLike) { assert_eq!(*actual,expected); } +fn roundtrip_program(program:&str) { + let parser = parser::Parser::new_or_panic(); + let ast = parser.parse(program.to_string(), Default::default()).unwrap(); + assert_eq!(ast.repr(), program, "{:#?}", ast); +} + // =============== @@ -436,3 +442,18 @@ impl Fixture { fn parser_tests() { Fixture::new().run() } + +/// Test case for https://github.com/enso-org/ide/issues/296 +#[wasm_bindgen_test] +fn block_roundtrip() { + let programs = vec![ + "main = 10 + 10", + "main =\n a = 10\n b = 20\n a * b", + "main =\n foo a =\n a * 10\n foo 10\n print \"hello\"", + "main =\n foo\n \n bar", + "main =\n \n foo\n \n bar" + ]; + for program in programs { + roundtrip_program(program); + } +} diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 5889a14172..22930c321d 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -7,6 +7,7 @@ use crate::prelude::*; use crate::Node; use crate::ParameterInfo; use crate::SpanTree; +use crate::generate::context::CalledMethodInfo; use crate::node; use crate::node::InsertType; @@ -20,8 +21,7 @@ use ast::opr::GeneralizedInfix; use data::text::Size; pub use context::Context; -use utils::vec::VecExt; -use crate::generate::context::InvocationInfo; + // ============= @@ -103,6 +103,7 @@ impl ChildGenerator { /// === Trait Implementations === /// ============================= +#[derive(Clone,Debug)] struct ApplicationBase<'a> { function_name : Option <&'a str>, has_target : bool, @@ -128,11 +129,13 @@ impl<'a> ApplicationBase<'a> { } } - fn prefix_params(&self, mut invocation_info:InvocationInfo) -> Vec { + fn prefix_params + (&self, invocation_info:Option) -> impl ExactSizeIterator { + let mut ret = invocation_info.map(|info| info.parameters).unwrap_or_default().into_iter(); if self.has_target { - invocation_info.parameters.pop_front() + ret.next(); } - invocation_info.parameters + ret } } @@ -141,22 +144,21 @@ impl<'a> ApplicationBase<'a> { impl SpanTreeGenerator for Ast { fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { + // Code like `this.func` or `a+b+c`. if let Some(infix) = GeneralizedInfix::try_new(self) { - use ast::opr::is_access_opr; - let chain = infix.flatten(); - let app_base = is_access_opr(infix.opr.ast()).as_some_from(|| { - ApplicationBase::new(self) - }); - let invocation = || -> Option { - context.invocation_info(self.id?,app_base?.function_name) + let chain = infix.flatten(); + let app_base = ApplicationBase::new(self); + let invocation = || -> Option { + context.call_info(self.id?, app_base.function_name) }(); - if invocation.contains_if(|info| info.parameters.len() > 1) { - let invocation = invocation.unwrap(); // we just checked that it contains sth - let node = chain.generate_node(node::Kind::Operation,context); - todo!() - } else { - chain.generate_node(kind,context) - } + + // All prefix params are missing arguments, since there is no prefix application. + let missing_args = app_base.prefix_params(invocation); + let arity = missing_args.len(); + let base_node_kind = if arity==0 {kind} else {node::Kind::Operation}; + let node = chain.generate_node(base_node_kind,context)?; + let provided_prefix_arg_count = 0; + Ok(generate_expected_arguments(node,kind,provided_prefix_arg_count,missing_args)) } else { match self.shape() { ast::Shape::Prefix(_) => @@ -173,7 +175,7 @@ impl SpanTreeGenerator for Ast { let children = default(); // TODO [mwu] handle cases where Ast is like "here.foo" let name = ast::identifier::name(self); - if let Some(info) = self.id.and_then(|id| context.invocation_info(id,name)) { + if let Some(info) = self.id.and_then(|id| context.call_info(id, name)) { let node = Node { size, children, @@ -181,11 +183,12 @@ impl SpanTreeGenerator for Ast { kind : node::Kind::Operation, parameter_info : None, }; - let arity = info.parameters.len(); - let params = info.parameters.iter().cloned().enumerate(); - Ok(params.fold(node,|node,(i,param)| { - generate_expected_argument(node, kind, i, arity, param) - })) + // Note that in this place it is impossible that Ast is in form of + // `this.method` -- it is covered by the former if arm. As such, we don't + // need to use `ApplicationBase` here as we do elsewhere. + let provided_prefix_arg_count = 0; + let params = info.parameters.into_iter(); + Ok(generate_expected_arguments(node,kind,provided_prefix_arg_count,params)) } else { let parameter_info = default(); Ok(Node {kind,size,children,expression_id,parameter_info}) @@ -266,16 +269,13 @@ impl SpanTreeGenerator for ast::prefix::Chain { //panic!(); let base = ApplicationBase::new(&self.func); - let invocation_info = self.id().and_then(|id| context.invocation_info(id,base.function_name)); + let invocation_info = self.id().and_then(|id| context.call_info(id, base.function_name)); let known_args = invocation_info.is_some(); println!("AST {} has name {:?} and info {:?}",self.clone().into_ast(),base.function_name,invocation_info); - let mut known_parameters = invocation_info.map(|info| info.parameters).unwrap_or_default(); - // Skip the leading parameter info (`this`-like entity) if provided through the prefix base. - if base.has_target { - known_parameters.pop_front(); - } - - let arity = self.args.len().max(known_parameters.len()); + let mut known_params = base.prefix_params(invocation_info); + dbg!(&base); + let arity = self.args.len().max(known_params.len()); + dbg!(arity); use ast::crumbs::PrefixCrumb::*; // Removing arguments is possible if there at least two of them @@ -286,7 +286,7 @@ impl SpanTreeGenerator for ast::prefix::Chain { let node = node?; let is_first = i == 0; let is_last = i + 1 == arity; - let arg_kind = if is_first { node::Kind::Target {is_removable} } + let arg_kind = if is_first && !base.has_target { node::Kind::Target {is_removable} } else { node::Kind::Argument {is_removable} }; let mut gen = ChildGenerator::default(); @@ -297,7 +297,7 @@ impl SpanTreeGenerator for ast::prefix::Chain { } let arg_ast = arg.sast.wrapped.clone_ref(); let arg_child = gen.generate_ast_node(Located::new(Arg,arg_ast),arg_kind,context)?; - arg_child.node.parameter_info = known_parameters.get(i).cloned(); + arg_child.node.parameter_info = known_params.next(); if !known_args { gen.generate_empty_node(InsertType::Append); } @@ -310,10 +310,7 @@ impl SpanTreeGenerator for ast::prefix::Chain { }) })?; - let missing_arguments = known_parameters.into_iter().enumerate().skip(self.args.len()); - Ok(missing_arguments.fold(ret,|node,(i,parameter)| { - generate_expected_argument(node,kind,i,arity,parameter) - })) + Ok(generate_expected_arguments(ret,kind,self.args.len(),known_params)) } } @@ -403,13 +400,17 @@ fn generate_children_from_ambiguous // === Common Utility == +/// Build a prefix application-like span tree structure where the prefix argument has not been +/// provided but instead its information is known from method's ParameterInfo. +/// +/// `index` is the argument's position in the prefix chain which may be different from parameter +/// index in the method's parameter list. fn generate_expected_argument -(node:Node, kind:node::Kind, index:usize, arity:usize, parameter_info:ParameterInfo) -> Node { +(node:Node, kind:node::Kind, index:usize, is_last:bool, parameter_info:ParameterInfo) -> Node { println!("Will generate missing argument node for {:?}",parameter_info); - let is_last = index + 1 == arity; let mut gen = ChildGenerator::default(); gen.add_node(ast::Crumbs::new(),node); - let arg_node = gen.generate_empty_node(InsertType::ExpectedArgument(index)); + let arg_node = gen.generate_empty_node(InsertType::ExpectedArgument(index)); arg_node.node.parameter_info = Some(parameter_info); Node { kind : if is_last {kind} else {node::Kind::Chained}, @@ -420,6 +421,19 @@ fn generate_expected_argument } } +fn generate_expected_arguments +(node:Node +, kind : node::Kind +, supplied_prefix_arg_count : usize +, expected_args : impl ExactSizeIterator +) -> Node { + let arity = supplied_prefix_arg_count + expected_args.len(); + (supplied_prefix_arg_count..).zip(expected_args).fold(node, |node, (index,parameter)| { + let is_last = index + 1 == arity; + generate_expected_argument(node,kind,index,is_last,parameter) + }) +} + //fn fill_with_expected_args(base:Node, kind:node::Kind, explicit_args:usize, arity:usi) @@ -435,7 +449,7 @@ mod test { use crate::Context; use crate::ParameterInfo; use crate::builder::TreeBuilder; - use crate::generate::context::InvocationInfo; + use crate::generate::context::CalledMethodInfo; use crate::node::Kind::*; use crate::node::InsertType::*; @@ -458,17 +472,17 @@ mod test { #[derive(Clone,Debug,Default)] struct MockContext { - map : HashMap, + map : HashMap, } impl MockContext { - fn new_single(id:Id, info:InvocationInfo) -> Self { + fn new_single(id:Id, info: CalledMethodInfo) -> Self { let mut ret = Self::default(); ret.map.insert(id,info); ret } } impl Context for MockContext { - fn invocation_info(&self, id:Id, _name:Option<&str>) -> Option { + fn call_info(&self, id:Id, _name:Option<&str>) -> Option { self.map.get(&id).cloned() } } @@ -759,7 +773,7 @@ mod test { assert_eq!(expected,tree); } - #[test] + #[wasm_bindgen_test] fn generating_span_tree_for_unfinished_call() { let parser = Parser::new_or_panic(); let this_param = ParameterInfo{ @@ -779,7 +793,7 @@ mod test { // === Single function name === let ast = parser.parse_line("foo").unwrap(); - let invocation_info = InvocationInfo { + let invocation_info = CalledMethodInfo { parameters : vec![this_param.clone()] }; let ctx = MockContext::new_single(ast.id.unwrap(),invocation_info); @@ -800,7 +814,7 @@ mod test { // === Complete application chain === let ast = parser.parse_line("foo here").unwrap(); - let invocation_info = InvocationInfo { + let invocation_info = CalledMethodInfo { parameters : vec![this_param.clone()] }; let ctx = MockContext::new_single(ast.id.unwrap(),invocation_info); @@ -821,7 +835,7 @@ mod test { // === Partial application chain === let ast = parser.parse_line("foo here").unwrap(); - let invocation_info = InvocationInfo { + let invocation_info = CalledMethodInfo { parameters : vec![this_param.clone(), param1.clone(), param2.clone()] }; let ctx = MockContext::new_single(ast.id.unwrap(),invocation_info); diff --git a/src/rust/ide/lib/span-tree/src/generate/context.rs b/src/rust/ide/lib/span-tree/src/generate/context.rs index 19190411bb..98a4013332 100644 --- a/src/rust/ide/lib/span-tree/src/generate/context.rs +++ b/src/rust/ide/lib/span-tree/src/generate/context.rs @@ -10,7 +10,7 @@ use ast::Id; /// Additional information available on nodes that are an invocation of a known methods. #[derive(Clone,Debug,Eq,PartialEq)] -pub struct InvocationInfo { +pub struct CalledMethodInfo { /// Information about arguments taken by a called method. pub parameters : Vec, } @@ -26,7 +26,7 @@ pub struct InvocationInfo { pub trait Context { /// Checks if the given expression is known to be a call to a known method. If so, returns the /// available information. - fn invocation_info(&self, id:Id, name:Option<&str>) -> Option; + fn call_info(&self, id:Id, name:Option<&str>) -> Option; /// Build a new context that merges this context and the one given in argument that will be used /// as a fallback. @@ -63,9 +63,9 @@ impl Merged { impl Context for Merged where First : Context, Second : Context { - fn invocation_info(&self, id: Id, name:Option<&str>) -> Option { - self.first.invocation_info(id, name).or_else(|| - self.second.invocation_info(id, name)) + fn call_info(&self, id: Id, name:Option<&str>) -> Option { + self.first.call_info(id,name).or_else(|| + self.second.call_info(id,name)) } } @@ -80,7 +80,7 @@ impl Context for Merged pub struct Empty; impl Context for Empty { - fn invocation_info(&self, _id:Id, _name:Option<&str>) -> Option { + fn call_info(&self, _id:Id, _name:Option<&str>) -> Option { None } } diff --git a/src/rust/ide/lib/span-tree/src/lib.rs b/src/rust/ide/lib/span-tree/src/lib.rs index 8f7333b2f1..e83f68e1d6 100644 --- a/src/rust/ide/lib/span-tree/src/lib.rs +++ b/src/rust/ide/lib/span-tree/src/lib.rs @@ -10,6 +10,7 @@ #![feature(option_result_contains)] #![feature(trait_alias)] #![feature(matches_macro)] +#![feature(exact_size_is_empty)] #![warn(missing_docs)] #![warn(trivial_casts)] #![warn(trivial_numeric_casts)] diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index c2e752f758..56b31b4f00 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -23,7 +23,7 @@ use span_tree::SpanTree; use span_tree::action::Actions; use span_tree::action::Action; use span_tree::generate::Context as SpanTreeContext; -use span_tree::generate::context::InvocationInfo; +use span_tree::generate::context::CalledMethodInfo; pub use crate::double_representation::graph::LocationHint; pub use crate::double_representation::graph::Id; @@ -824,7 +824,7 @@ impl Handle { /// /// It just applies the information from the metadata. impl span_tree::generate::Context for Handle { - fn invocation_info(&self, id:node::Id, name:Option<&str>) -> Option { + fn call_info(&self, id:node::Id, name:Option<&str>) -> Option { let db = &self.suggestion_db; let metadata = self.module.node_metadata(id).ok()?; let db_entry = db.lookup_method(metadata.intended_method?)?; @@ -1042,7 +1042,7 @@ main = let node = &graph.nodes().unwrap()[0]; assert_eq!(node.info.id(),id); let expression = node.info.expression().repr(); - graph.invocation_info(id, Some(expression.as_str())) + graph.call_info(id, Some(expression.as_str())) }; // Now node is `bar` while the intended method is `foo`. diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index fbdf86eb76..c77d722622 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -13,7 +13,7 @@ use crate::model::execution_context::VisualizationUpdateData; use enso_protocol::language_server::MethodPointer; use span_tree::generate::context::Context; -use span_tree::generate::context::InvocationInfo; +use span_tree::generate::context::CalledMethodInfo; pub use crate::controller::graph::Connection; pub use crate::controller::graph::Connections; @@ -250,13 +250,13 @@ impl Handle { /// Span Tree generation context for a graph that does not know about execution. /// Provides information based on computed value registry, using metadata as a fallback. impl Context for Handle { - fn invocation_info(&self, id:ast::Id, name:Option<&str>) -> Option { + fn call_info(&self, id:ast::Id, name:Option<&str>) -> Option { let lookup_registry = || { let info = self.computed_value_info_registry().get(&id)?; let entry = self.project.suggestion_db().lookup(info.method_call?).ok()?; Some(entry.invocation_info()) }; - let fallback = || self.graph.borrow().invocation_info(id,name); + let fallback = || self.graph.borrow().call_info(id, name); lookup_registry().or_else(fallback) } } @@ -354,7 +354,7 @@ pub mod tests { let mock::Fixture{graph,executed_graph,module,suggestion_db,..} = &data.fixture(); let id = graph.nodes().unwrap()[0].info.id(); - let get_invocation_info = || executed_graph.invocation_info(id,Some("foo")); + let get_invocation_info = || executed_graph.call_info(id, Some("foo")); assert!(get_invocation_info().is_none()); // Check that if we set metadata, executed graph can see this info. diff --git a/src/rust/ide/src/double_representation.rs b/src/rust/ide/src/double_representation.rs index 7aceaddfa9..9b09e9edde 100644 --- a/src/rust/ide/src/double_representation.rs +++ b/src/rust/ide/src/double_representation.rs @@ -14,10 +14,6 @@ pub mod text; #[cfg(test)] pub mod test_utils; -use crate::prelude::*; - -use crate::double_representation::identifier::Identifier; - // ============== @@ -31,15 +27,3 @@ use crate::double_representation::identifier::Identifier; /// /// Link: https://github.com/luna/enso/blob/main/doc/syntax/encoding.md pub const INDENT : usize = 4; - - -// pub fn target_method_name(ast:&Ast) -> Option { -// if let Some(chain) = ast::prefix::Chain::from_ast(ast) { -// target_method_name(&chain.func) -// } else if let Some(chain) = ast::opr::as_access_chain(ast) { -// let presumed_name = chain.args.last()?; -// identifier::Identifier::new(presumed_name.operand.as_ref()?.arg.clone_ref()) -// } else { -// identifier::Identifier::new(ast.clone()) -// } -// } diff --git a/src/rust/ide/src/model/suggestion_database.rs b/src/rust/ide/src/model/suggestion_database.rs index 5af9784b4e..a66c018729 100644 --- a/src/rust/ide/src/model/suggestion_database.rs +++ b/src/rust/ide/src/model/suggestion_database.rs @@ -199,7 +199,7 @@ impl Entry { } /// Generate information about invoking this entity for span tree context. - pub fn invocation_info(&self) -> span_tree::generate::context::InvocationInfo { + pub fn invocation_info(&self) -> span_tree::generate::context::CalledMethodInfo { self.into() } } @@ -233,10 +233,10 @@ impl TryFrom for language_server::MethodPointer { } } -impl From<&Entry> for span_tree::generate::context::InvocationInfo { - fn from(entry:&Entry) -> span_tree::generate::context::InvocationInfo { +impl From<&Entry> for span_tree::generate::context::CalledMethodInfo { + fn from(entry:&Entry) -> span_tree::generate::context::CalledMethodInfo { let parameters = entry.arguments.iter().map(to_span_tree_param).collect(); - span_tree::generate::context::InvocationInfo{parameters} + span_tree::generate::context::CalledMethodInfo {parameters} } } diff --git a/src/rust/ide/src/tests.rs b/src/rust/ide/src/tests.rs index 2d0be457a1..d1c86dcda2 100644 --- a/src/rust/ide/src/tests.rs +++ b/src/rust/ide/src/tests.rs @@ -11,6 +11,7 @@ use json_rpc::test_util::transport::mock::MockTransport; use serde_json::json; use wasm_bindgen_test::wasm_bindgen_test_configure; use wasm_bindgen_test::wasm_bindgen_test; +use span_tree::node::{InsertType, Kind}; wasm_bindgen_test_configure!(run_in_browser); @@ -78,9 +79,10 @@ async fn get_most_recent_project_or_create_new() { assert_eq!(expected_project, project.expect("Couldn't get project.")) } -#[test] +#[wasm_bindgen_test] fn span_tree_args() { use crate::test::mock::*; + use span_tree::Node; let data = Unified::new(); let fixture = data.fixture_customize(|_,json_client| { @@ -100,38 +102,71 @@ fn span_tree_args() { let get_node = || graph.node(id).unwrap(); let get_inputs = || NodeTrees::new(&get_node().info,executed_graph).unwrap().inputs; - let get_param1 = || get_inputs().root_ref().leaf_iter().nth(1).and_then(|node| { + let get_param = |n| get_inputs().root_ref().leaf_iter().nth(n).and_then(|node| { node.parameter_info.clone() }); + let expected_this_param = model::suggestion_database::to_span_tree_param(&entry.arguments[0]); + let expected_arg1_param = model::suggestion_database::to_span_tree_param(&entry.arguments[1]); + + + // === Method notation, without prefix application === + assert_eq!(get_node().info.expression().repr(), "Base.foo"); + match get_inputs().root.children.as_slice() { + // The tree here should have two nodes under root - one with given Ast and second for + // an additional prefix application argument. + [_,second] => { + let Node{children,kind,parameter_info,..} = &second.node; + assert!(children.is_empty()); + assert_eq!(kind,&Kind::Empty(InsertType::ExpectedArgument(0))); + assert_eq!(parameter_info,&Some(expected_arg1_param.clone())); + } + _ => panic!("Expected only two children in the span tree's root"), + }; - let expected_param = model::suggestion_database::to_span_tree_param(&entry.arguments[0]); - - // TODO [mwu] The searcher inserts "Base.foo". This should work as well but needs redesigned - // target detection rules in the span tree. - // - - - println!("{:#?}",get_inputs().root_ref().leaf_iter().collect_vec()); - return; + // === Method notation, with prefix application === graph.set_expression(id,"Base.foo 50").unwrap(); - println!("{:#?}",get_inputs().root_ref().leaf_iter().collect_vec()); - - - return; - panic!("{}",get_node().info.expression().repr()); + match get_inputs().root.children.as_slice() { + // The tree here should have two nodes under root - one with given Ast and second for + // an additional prefix application argument. + [_,second] => { + let Node{children,kind,parameter_info,..} = &second.node; + assert!(children.is_empty()); + assert_eq!(kind,&Kind::Argument{is_removable:false}); + assert_eq!(parameter_info,&Some(expected_arg1_param.clone())); + } + _ => panic!("Expected only two children in the span tree's root"), + }; + // === Function notation, without prefix application === assert_eq!(entry.name,"foo"); graph.set_expression(id,"foo").unwrap(); - assert_eq!(get_param1().as_ref(), Some(&expected_param)); + assert_eq!(get_param(1).as_ref(),Some(&expected_this_param)); + assert_eq!(get_param(2).as_ref(),Some(&expected_arg1_param)); + assert_eq!(get_param(3).as_ref(),None); + + // === Function notation, with prefix application === graph.set_expression(id,"foo Base").unwrap(); - assert_eq!(get_param1().as_ref(), Some(&expected_param)); + assert_eq!(get_param(1).as_ref(),Some(&expected_this_param)); + assert_eq!(get_param(2).as_ref(),Some(&expected_arg1_param)); + assert_eq!(get_param(3).as_ref(),None); + + // === Changed function name, should not have known parameters === graph.set_expression(id,"bar").unwrap(); - assert_eq!(get_param1(), None); + assert_eq!(get_param(1),None); + assert_eq!(get_param(2),None); + assert_eq!(get_param(3),None); graph.set_expression(id,"bar Base").unwrap(); - assert_eq!(get_param1(), None); + assert_eq!(get_param(1),None); + assert_eq!(get_param(2),None); + assert_eq!(get_param(3),None); + + graph.set_expression(id,"Base.bar").unwrap(); + assert_eq!(get_param(1),None); + assert_eq!(get_param(2),None); + assert_eq!(get_param(3),None); } From 9e7bc7214cf9cc2f26312b6f9303df89dffd0725 Mon Sep 17 00:00:00 2001 From: mwu Date: Wed, 9 Sep 2020 04:03:24 +0200 Subject: [PATCH 18/34] this and that --- src/rust/ide/lib/ast/impl/src/opr.rs | 14 +++++++------- src/rust/ide/lib/ast/impl/src/prefix.rs | 3 ++- src/rust/ide/lib/span-tree/src/generate.rs | 2 +- src/rust/ide/src/controller/graph.rs | 12 ++++++------ src/rust/ide/src/controller/searcher.rs | 2 +- .../src/double_representation/alias_analysis.rs | 2 +- .../ide/src/double_representation/definition.rs | 2 +- src/rust/ide/src/view/node_editor.rs | 2 +- 8 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/rust/ide/lib/ast/impl/src/opr.rs b/src/rust/ide/lib/ast/impl/src/opr.rs index de5f14ddb0..9dbc203309 100644 --- a/src/rust/ide/lib/ast/impl/src/opr.rs +++ b/src/rust/ide/lib/ast/impl/src/opr.rs @@ -343,9 +343,9 @@ impl Chain { (infix.name() == operator).as_some_from(|| infix.flatten()) } - /// Iterates over &Located, beginning with target (this argument) and then subsequent + /// Iterates over operands beginning with target (this argument) and then subsequent /// arguments. - pub fn enumerate_operands0<'a> + pub fn enumerate_operands<'a> (&'a self) -> impl Iterator>>> + 'a { let rev_args = self.args.iter().rev(); let target_crumbs = rev_args.map(ChainElement::crumb_to_previous).collect_vec(); @@ -365,11 +365,11 @@ impl Chain { } - /// Iterates over &Located, beginning with target (this argument) and then subsequent + /// Iterates over non-empty operands beginning with target (this argument) and then subsequent /// arguments. - pub fn enumerate_operands<'a> + pub fn enumerate_non_empty_operands<'a> (&'a self) -> impl Iterator>> + 'a { - self.enumerate_operands0().flatten() + self.enumerate_operands().flatten() } /// Iterates over all operator's AST in this chain, starting from target side. @@ -530,8 +530,8 @@ mod tests { } fn test_enumerating(chain:&Chain, root_ast:&Ast, expected_asts:&[&Ast]) { - assert_eq!(chain.enumerate_operands().count(), expected_asts.len()); - for (elem,expected) in chain.enumerate_operands().zip(expected_asts) { + assert_eq!(chain.enumerate_non_empty_operands().count(), expected_asts.len()); + for (elem,expected) in chain.enumerate_non_empty_operands().zip(expected_asts) { assert_eq!(elem.item.arg,**expected); let ast = root_ast.get_traversing(&elem.crumbs).unwrap(); assert_eq!(ast,*expected); diff --git a/src/rust/ide/lib/ast/impl/src/prefix.rs b/src/rust/ide/lib/ast/impl/src/prefix.rs index 01de96293f..7fc4266832 100644 --- a/src/rust/ide/lib/ast/impl/src/prefix.rs +++ b/src/rust/ide/lib/ast/impl/src/prefix.rs @@ -31,7 +31,8 @@ pub struct Argument { } impl Argument { - fn new_blank(offset:usize, prefix_id:Option) -> Self { + /// Make an argument consisting of a single blank placeholder: `_`. + pub fn new_blank(offset:usize, prefix_id:Option) -> Self { let sast = Shifted::new(offset,Ast::blank()); Self {sast,prefix_id} } diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 22930c321d..d61f99d937 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -113,7 +113,7 @@ impl<'a> ApplicationBase<'a> { fn new(ast:&'a Ast) -> Self { if let Some(chain) = ast::opr::as_access_chain(ast) { let get_name = || -> Option<&'a str> { - let crumbs = chain.enumerate_operands0().last()??.crumbs; + let crumbs = chain.enumerate_operands().last()??.crumbs; let ast = ast.get_traversing(&crumbs).ok()?; ast::identifier::name(ast) }; diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 56b31b4f00..9efc847d4e 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -508,8 +508,8 @@ impl Handle { /// Returns information about all the connections between graph's nodes. /// - /// Will use `self.span_tree_context()` as the context for span tree generation. - pub fn connections_legacy(&self) -> FallibleResult { + /// Will use `self` as the context for span tree generation. + pub fn connections_unevaluated_ctx(&self) -> FallibleResult { let graph = self.graph_info()?; Ok(Connections::new(&graph,self)) } @@ -1273,7 +1273,7 @@ main = print z"; test.data.code = PROGRAM.into(); test.run(|graph| async move { - let connections = graph.connections_legacy().unwrap(); + let connections = graph.connections_unevaluated_ctx().unwrap(); let (node0,node1,node2,node3,node4) = graph.nodes().unwrap().expect_tuple(); assert_eq!(node0.info.expression().repr(), "get_pos"); @@ -1374,7 +1374,7 @@ main = sum = _ + b"; test.data.code = PROGRAM.into(); test.run(|graph| async move { - assert!(graph.connections_legacy().unwrap().connections.is_empty()); + assert!(graph.connections_unevaluated_ctx().unwrap().connections.is_empty()); let (node0,_node1,node2) = graph.nodes().unwrap().expect_tuple(); let connection_to_add = Connection { source : Endpoint { @@ -1411,7 +1411,7 @@ main = calculate1 = calculate2 calculate3 calculate5 = calculate5 calculate4"; test.run(|graph| async move { - assert!(graph.connections_legacy().unwrap().connections.is_empty()); + assert!(graph.connections_unevaluated_ctx().unwrap().connections.is_empty()); let (node0,node1,_) = graph.nodes().unwrap().expect_tuple(); let connection_to_add = Connection { source : Endpoint { @@ -1472,7 +1472,7 @@ main = let expected = format!("{}{}",MAIN_PREFIX,self.dest_node_expected); let this = self.clone(); test.run(|graph| async move { - let connections = graph.connections_legacy().unwrap(); + let connections = graph.connections_unevaluated_ctx().unwrap(); let connection = connections.connections.first().unwrap(); graph.disconnect(connection,&span_tree::generate::context::Empty).unwrap(); let new_main = graph.graph_definition_info().unwrap().ast.repr(); diff --git a/src/rust/ide/src/controller/searcher.rs b/src/rust/ide/src/controller/searcher.rs index ef2ccaa146..950e2bc5cd 100644 --- a/src/rust/ide/src/controller/searcher.rs +++ b/src/rust/ide/src/controller/searcher.rs @@ -835,7 +835,7 @@ mod test { fn new_custom(client_setup:F) -> Self where F : FnOnce(&mut MockData,&mut language_server::MockClient) { let test = TestWithLocalPoolExecutor::set_up(); - let mut data:MockData = MockData::default(); + let mut data = MockData::default(); let mut client = language_server::MockClient::default(); client.require_all_calls(); client_setup(&mut data,&mut client); diff --git a/src/rust/ide/src/double_representation/alias_analysis.rs b/src/rust/ide/src/double_representation/alias_analysis.rs index 7912d6c4b4..2acceacbcb 100644 --- a/src/rust/ide/src/double_representation/alias_analysis.rs +++ b/src/rust/ide/src/double_representation/alias_analysis.rs @@ -246,7 +246,7 @@ impl AliasAnalyzer { // (the possibility of definition has been already excluded) if let Some(infix_chain) = ast::opr::Chain::try_new(ast) { // Infix always acts as pattern-match in left-side. - for operand in infix_chain.enumerate_operands() { + for operand in infix_chain.enumerate_non_empty_operands() { self.process_located_ast(&operand.map(|operand| &operand.arg)) } for operator in infix_chain.enumerate_operators() { diff --git a/src/rust/ide/src/double_representation/definition.rs b/src/rust/ide/src/double_representation/definition.rs index f811f4e2e1..74169a3f20 100644 --- a/src/rust/ide/src/double_representation/definition.rs +++ b/src/rust/ide/src/double_representation/definition.rs @@ -143,7 +143,7 @@ impl DefinitionName { } let mut pieces = Vec::new(); - for piece in accessor_chain.enumerate_operands() { + for piece in accessor_chain.enumerate_non_empty_operands() { let name = ast::identifier::name(&piece.item.arg)?.to_owned(); pieces.push(piece.map(|_| name)); } diff --git a/src/rust/ide/src/view/node_editor.rs b/src/rust/ide/src/view/node_editor.rs index 5f438fc30e..90d3ede4fc 100644 --- a/src/rust/ide/src/view/node_editor.rs +++ b/src/rust/ide/src/view/node_editor.rs @@ -348,7 +348,7 @@ impl GraphEditorIntegratedWithControllerModel { pub fn refresh_graph_view(&self) -> FallibleResult<()> { info!(self.logger, "Refreshing the graph view."); use controller::graph::Connections; - let Connections{trees,connections} = self.controller.graph().connections_legacy()?; + let Connections{trees,connections} = self.controller.graph().connections_unevaluated_ctx()?; self.refresh_node_views(trees)?; self.refresh_connection_views(connections)?; Ok(()) From 6bbd07cd9ab0e7ec6a6deb3f7a30320f2e969a68 Mon Sep 17 00:00:00 2001 From: mwu Date: Wed, 9 Sep 2020 18:52:47 +0200 Subject: [PATCH 19/34] cr feedback --- src/rust/ide/lib/span-tree/src/generate.rs | 9 +++- .../ide/lib/span-tree/src/generate/context.rs | 8 +++- src/rust/ide/src/controller/graph.rs | 11 +---- src/rust/ide/src/controller/graph/executed.rs | 30 ++++-------- src/rust/ide/src/test.rs | 47 +++++++++++-------- 5 files changed, 52 insertions(+), 53 deletions(-) diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index d61f99d937..3850022f9e 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -103,10 +103,16 @@ impl ChildGenerator { /// === Trait Implementations === /// ============================= +/// Helper structure constructed from Ast that consists base of prefix application. +/// +/// It recognizes whether the base uses a method-style notation (`this.method` instead of +/// `method this`) and what is the invoked function name. #[derive(Clone,Debug)] struct ApplicationBase<'a> { + /// The name of invoked function. function_name : Option <&'a str>, - has_target : bool, + /// True when Ast uses method notation to pass this as an invocation target. + has_target : bool, } impl<'a> ApplicationBase<'a> { @@ -173,7 +179,6 @@ impl SpanTreeGenerator for Ast { let size = Size::new(self.len()); let expression_id = self.id; let children = default(); - // TODO [mwu] handle cases where Ast is like "here.foo" let name = ast::identifier::name(self); if let Some(info) = self.id.and_then(|id| context.call_info(id, name)) { let node = Node { diff --git a/src/rust/ide/lib/span-tree/src/generate/context.rs b/src/rust/ide/lib/span-tree/src/generate/context.rs index 98a4013332..81c2f40fb8 100644 --- a/src/rust/ide/lib/span-tree/src/generate/context.rs +++ b/src/rust/ide/lib/span-tree/src/generate/context.rs @@ -24,8 +24,14 @@ pub struct CalledMethodInfo { /// Entity that is able to provide information whether a given expression is a known method /// invocation. If so, additional information is provided. pub trait Context { - /// Checks if the given expression is known to be a call to a known method. If so, returns the + /// Check if the given expression is known to be a call to a known method. If so, return the /// available information. + /// + /// The `name` parameter can be used to pass a known target method identifier (if the caller + /// knows what name is supplied at the invocation site). + /// + /// Trait implementors may used it to filter-out results, however they are not required to do + /// so. Caller should not assume that the called method has the same name as given identifier. fn call_info(&self, id:Id, name:Option<&str>) -> Option; /// Build a new context that merges this context and the one given in argument that will be used diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 9efc847d4e..ada0a74fea 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -1050,15 +1050,8 @@ main = assert!(get_invocation_info().is_none()); // Now the name should be good and we should the information about node being a call. - graph.set_expression(id,"foo").unwrap(); - match get_invocation_info().unwrap().parameters.as_slice() { - [param] => { - let arg = &entry.arguments[0]; - assert_eq!(param.name.as_ref(),Some(&arg.name)); - assert_eq!(param.typename.as_ref(),Some(&arg.repr_type)); - } - _ => panic!("Expected a single parameter in invocation info!"), - } + graph.set_expression(id,&entry.name).unwrap(); + crate::test::assert_call_info(get_invocation_info().unwrap(),&entry); // Now we remove metadata, so the information is no more. graph.module.remove_node_metadata(id).unwrap(); diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index c77d722622..b065ce5606 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -349,43 +349,31 @@ pub mod tests { #[wasm_bindgen_test] fn span_tree_context() { use crate::test::mock; + use crate::test::assert_call_info; + let mut data = mock::Unified::new(); - data.set_inline_code("foo"); + let entry1 = data.suggestions.get(&1).unwrap().clone(); + let entry2 = data.suggestions.get(&2).unwrap().clone(); + data.set_inline_code(&entry1.name); - let mock::Fixture{graph,executed_graph,module,suggestion_db,..} = &data.fixture(); + let mock::Fixture{graph,executed_graph,module,..} = &data.fixture(); let id = graph.nodes().unwrap()[0].info.id(); - let get_invocation_info = || executed_graph.call_info(id, Some("foo")); + let get_invocation_info = || executed_graph.call_info(id,Some(&entry1.name)); assert!(get_invocation_info().is_none()); // Check that if we set metadata, executed graph can see this info. - let entry1 = suggestion_db.lookup(1).unwrap(); module.set_node_metadata(id,NodeMetadata { position : None, intended_method : entry1.method_id(), }); let info = get_invocation_info().unwrap(); - match info.parameters.as_slice() { - [param] => { - let expected = &entry1.arguments[0]; - let expected = model::suggestion_database::to_span_tree_param(expected); - assert_eq!(param,&expected); - } - _ => panic!("Expected only single parameter!"), - }; + assert_call_info(info,&entry1); // Now send update that expression actually was computed to be a call to the second // suggestion entry and check that executed graph provides this info over the metadata one. - let entry2 = suggestion_db.lookup(2).unwrap(); let update = value_update_with_method_ptr(id,2); executed_graph.computed_value_info_registry().apply_updates(vec![update]); let info = get_invocation_info().unwrap(); - match info.parameters.as_slice() { - [param] => { - let expected = &entry2.arguments[0]; - let expected = model::suggestion_database::to_span_tree_param(expected); - assert_eq!(param,&expected); - } - _ => panic!("Expected only single parameter!"), - }; + assert_call_info(info,&entry2); } } diff --git a/src/rust/ide/src/test.rs b/src/rust/ide/src/test.rs index a6b80b3fe4..1cff04571e 100644 --- a/src/rust/ide/src/test.rs +++ b/src/rust/ide/src/test.rs @@ -1,15 +1,17 @@ +#![cfg(test)] //! Module for support code for writing tests. -/// Utilities for mocking IDE components. -#[cfg(test)] -pub mod mock { - use crate::prelude::*; +use crate::prelude::*; + +use crate::double_representation::module; +use crate::model::suggestion_database; +use crate::executor::test_utils::TestWithLocalPoolExecutor; - use crate::double_representation::module; - use crate::model::suggestion_database; - use crate::executor::test_utils::TestWithLocalPoolExecutor; +use enso_protocol::language_server; - use enso_protocol::language_server; +/// Utilities for mocking IDE components. +pub mod mock { + use super::*; /// Data used to create mock IDE components. /// @@ -218,6 +220,12 @@ pub mod mock { (&self, customize_json_rpc:impl FnOnce(&Self,&mut language_server::MockClient)) -> Fixture { let mut json_client = language_server::MockClient::default(); + // Creating a searcher controller always triggers a query for completion. + let completion = language_server::response::Completion { + results : default(), + current_version : default(), + }; + json_client.expect.completion(|_,_,_,_,_| Ok(completion)); customize_json_rpc(self,&mut json_client); let logger = Logger::default(); // TODO @@ -263,18 +271,6 @@ pub mod mock { pub searcher : controller::Searcher, } - impl Fixture { - // pub fn module(&mut self) -> crate::model::Module { - // self.module.get_or_insert(self.data.module()).clone_ref() - // } - // - // /// Create a graph controller from the current mock data. - // pub fn graph(&mut self, module:model::Module) -> crate::controller::Graph { - // let module = self.module(); - // self.data.graph(module) - // } - } - pub fn indent(line:impl AsRef) -> String { iformat!(" {line.as_ref()}") } @@ -289,3 +285,14 @@ pub mod mock { iformat!("{name} =\n{body}") } } + +pub fn assert_call_info +( info:span_tree::generate::context::CalledMethodInfo + , entry:&model::suggestion_database::Entry +) { + assert_eq!(info.parameters.len(),entry.arguments.len()); + for (encountered,expected) in info.parameters.iter().zip(entry.arguments.iter()) { + let expected_info = model::suggestion_database::to_span_tree_param(expected); + assert_eq!(encountered,&expected_info); + } +} From bcded9779428abba292157f8852ccb172b392b9d Mon Sep 17 00:00:00 2001 From: mwu Date: Thu, 10 Sep 2020 01:24:19 +0200 Subject: [PATCH 20/34] notifications --- src/rust/ide/src/controller/graph.rs | 24 ++++++++++-- src/rust/ide/src/controller/graph/executed.rs | 37 ++++++++++++------ src/rust/ide/src/model/module/plain.rs | 3 +- src/rust/ide/src/model/suggestion_database.rs | 39 +++++++++++++++---- src/rust/ide/src/notification.rs | 6 +++ src/rust/ide/src/test.rs | 9 ++++- 6 files changed, 94 insertions(+), 24 deletions(-) diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index ada0a74fea..6f1cca7397 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -806,14 +806,19 @@ impl Handle { /// Subscribe to updates about changes in this graph. pub fn subscribe(&self) -> impl Stream { - let module_sub = self.module.subscribe(); - module_sub.map(|notification| { + let module_sub = self.module.subscribe().map(|notification| { match notification { model::module::Notification::Invalidate | model::module::Notification::CodeChanged{..} | model::module::Notification::MetadataChanged => Notification::Invalidate, } - }) + }); + let db_sub = self.suggestion_db.subscribe().map(|notification| { + match notification { + model::suggestion_database::Notification::Updated => Notification::Invalidate + } + }); + futures::stream::select(module_sub,db_sub) } } @@ -978,6 +983,19 @@ pub mod tests { }); } + #[wasm_bindgen_test] + fn suggestion_db_updates_invalidate_graph() { + Fixture::set_up().run(|graph| async move { + let mut sub = graph.subscribe(); + let update = language_server::types::SuggestionDatabaseUpdatesEvent { + updates : vec![], + current_version : default(), + }; + graph.suggestion_db.apply_update_event(update); + assert_eq!(Some(Notification::Invalidate), sub.next().await); + }); + } + #[wasm_bindgen_test] fn graph_controller_inline_definition() { let mut test = Fixture::set_up(); diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index b065ce5606..e4ece42be1 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -147,7 +147,20 @@ impl Handle { let value_stream = registry.subscribe().map(Notification::ComputedValueInfo).boxed_local(); let graph_stream = self.graph().subscribe().map(Notification::Graph).boxed_local(); let self_stream = self.notifier.subscribe().boxed_local(); - futures::stream::select_all(vec![value_stream,graph_stream,self_stream]) + + // Note: [Argument Names-related invalidations] + let db_stream = self.project.suggestion_db().subscribe().map(|notification| { + match notification { + model::suggestion_database::Notification::Updated => + Notification::Graph(controller::graph::Notification::Invalidate), + } + }).boxed_local(); + let update_stream = registry.subscribe().map(|_| { + Notification::Graph(controller::graph::Notification::Invalidate) + }).boxed_local(); + + let streams = vec![value_stream,graph_stream,self_stream,db_stream,update_stream]; + futures::stream::select_all(streams) } /// Get a type of the given expression as soon as it is available. @@ -311,16 +324,13 @@ pub mod tests { } // Test that checks that value computed notification is properly relayed by the executed graph. - #[wasm_bindgen_test] + #[test] fn dispatching_value_computed_notification() { + use crate::test::mock::Fixture; + use crate::test::mock::Unified; // Setup the controller. - let mut fixture = TestWithLocalPoolExecutor::set_up(); - let graph_data = controller::graph::tests::MockData::new(); - let graph = graph_data.graph(); - let execution_data = model::execution_context::plain::test::MockData::new(); - let execution = Rc::new(execution_data.create()); - let project = Rc::new(model::project::MockAPI::new()); - let executed_graph = Handle::new_internal(graph,project,execution.clone_ref()); + let mut fixture = crate::test::mock::Unified::new().fixture(); + let Fixture{executed_graph,execution,executor,..} = &mut fixture; // Generate notification. let updated_id = ExpressionId::new_v4(); @@ -334,8 +344,8 @@ pub mod tests { assert!(registry.get(&updated_id).is_none()); // Sending notification. - execution.computed_value_info_registry.apply_updates(vec![update]); - fixture.run_until_stalled(); + execution.computed_value_info_registry().apply_updates(vec![update]); + executor.run_until_stalled(); // Observing that notification was relayed. let observed_notification = notifications.expect_next(); @@ -343,6 +353,11 @@ pub mod tests { let expected_typename = Some(ImString::new(typename)); assert_eq!(observed_notification,Notification::ComputedValueInfo(vec![updated_id])); assert_eq!(typename_in_registry,expected_typename); + + // Having a computed value also requires graph invalidation (span tree context change). + let observed_notification = notifications.expect_next(); + let expected = Notification::Graph(controller::graph::Notification::Invalidate); + assert_eq!(observed_notification,expected); notifications.expect_pending(); } diff --git a/src/rust/ide/src/model/module/plain.rs b/src/rust/ide/src/model/module/plain.rs index 8ab2ac1736..f1b6602a5e 100644 --- a/src/rust/ide/src/model/module/plain.rs +++ b/src/rust/ide/src/model/module/plain.rs @@ -35,8 +35,7 @@ impl Module { } fn notify(&self, notification:Notification) { - let notify = self.notifications.publish(notification); - executor::global::spawn(notify); + self.notifications.notify(notification) } } diff --git a/src/rust/ide/src/model/suggestion_database.rs b/src/rust/ide/src/model/suggestion_database.rs index a66c018729..a97232d180 100644 --- a/src/rust/ide/src/model/suggestion_database.rs +++ b/src/rust/ide/src/model/suggestion_database.rs @@ -4,9 +4,12 @@ use crate::prelude::*; use crate::double_representation::module::QualifiedName; use crate::model::module::MethodId; +use crate::notification; use data::text::TextLocation; use enso_protocol::language_server; +use enso_protocol::language_server::SuggestionId; +use flo_stream::Subscriber; use language_server::types::SuggestionsDatabaseVersion; use language_server::types::SuggestionDatabaseUpdatesEvent; use parser::DocParser; @@ -14,7 +17,6 @@ use parser::DocParser; pub use language_server::types::SuggestionEntryArgument as Argument; pub use language_server::types::SuggestionId as EntryId; pub use language_server::types::SuggestionsDatabaseUpdate as Update; -use enso_protocol::language_server::SuggestionId; @@ -251,6 +253,11 @@ pub fn to_span_tree_param } } +#[derive(Clone,Copy,Debug,PartialEq)] +pub enum Notification { + Updated +} + // ================ @@ -265,9 +272,10 @@ pub fn to_span_tree_param /// argument names and types. #[derive(Clone,Debug,Default)] pub struct SuggestionDatabase { - logger : Logger, - entries : RefCell>>, - version : Cell, + logger : Logger, + entries : RefCell>>, + version : Cell, + notifications : notification::Publisher, } impl SuggestionDatabase { @@ -309,11 +317,17 @@ impl SuggestionDatabase { } Self { logger, - entries : RefCell::new(entries), - version : Cell::new(response.current_version), + entries : RefCell::new(entries), + version : Cell::new(response.current_version), + notifications : default() } } + /// Subscribe for notifications about changes in the database. + pub fn subscribe(&self) -> Subscriber { + self.notifications.subscribe() + } + /// Get suggestion entry by id. pub fn lookup(&self, id:EntryId) -> Result,NoSuchEntry> { self.entries.borrow().get(&id).cloned().ok_or(NoSuchEntry(id)) @@ -345,6 +359,7 @@ impl SuggestionDatabase { }; } self.version.set(event.current_version); + self.notifications.notify(Notification::Updated); } @@ -414,8 +429,10 @@ mod test { use super::*; use enso_protocol::language_server::SuggestionsDatabaseEntry; + use utils::test::stream::StreamTestExt; use wasm_bindgen_test::wasm_bindgen_test_configure; use wasm_bindgen_test::wasm_bindgen_test; + use crate::executor::test_utils::TestWithLocalPoolExecutor; @@ -524,6 +541,7 @@ mod test { #[test] fn applying_update() { + let mut fixture = TestWithLocalPoolExecutor::set_up(); let entry1 = language_server::types::SuggestionEntry::Atom { name : "Entry1".to_string(), module : "TestProject.TestModule".to_string(), @@ -552,7 +570,9 @@ mod test { entries : vec![db_entry1,db_entry2], current_version : 1, }; - let db = SuggestionDatabase::from_ls_response(initial_response); + let db = SuggestionDatabase::from_ls_response(initial_response); + let mut notifications = db.subscribe().boxed_local(); + notifications.expect_pending(); // Remove let remove_update = Update::Remove {id:2}; @@ -561,6 +581,8 @@ mod test { current_version : 2 }; db.apply_update_event(update); + fixture.run_until_stalled(); + assert_eq!(notifications.expect_next(),Notification::Updated); assert_eq!(db.lookup(2), Err(NoSuchEntry(2))); assert_eq!(db.version.get(), 2); @@ -571,6 +593,9 @@ mod test { current_version : 3, }; db.apply_update_event(update); + fixture.run_until_stalled(); + assert_eq!(notifications.expect_next(),Notification::Updated); + notifications.expect_pending(); assert_eq!(db.lookup(2).unwrap().name, "NewEntry2"); assert_eq!(db.version.get(), 3); } diff --git a/src/rust/ide/src/notification.rs b/src/rust/ide/src/notification.rs index 1d17961046..674df99ab2 100644 --- a/src/rust/ide/src/notification.rs +++ b/src/rust/ide/src/notification.rs @@ -61,4 +61,10 @@ where Message : 'static + Send, pub fn subscribe(&self) -> Subscriber { self.0.borrow_mut().subscribe() } + + /// Use global executor to publish a message. + pub fn notify(&self, message:Message) { + let notify = self.publish(message); + executor::global::spawn(notify); + } } diff --git a/src/rust/ide/src/test.rs b/src/rust/ide/src/test.rs index 1cff04571e..f83fd782ab 100644 --- a/src/rust/ide/src/test.rs +++ b/src/rust/ide/src/test.rs @@ -271,6 +271,13 @@ pub mod mock { pub searcher : controller::Searcher, } + impl Fixture { + /// Runs all tasks in the pool and returns if no more progress can be made on any task. + pub fn run_until_stalled(&mut self) { + self.executor.run_until_stalled(); + } + } + pub fn indent(line:impl AsRef) -> String { iformat!(" {line.as_ref()}") } @@ -288,7 +295,7 @@ pub mod mock { pub fn assert_call_info ( info:span_tree::generate::context::CalledMethodInfo - , entry:&model::suggestion_database::Entry +, entry:&model::suggestion_database::Entry ) { assert_eq!(info.parameters.len(),entry.arguments.len()); for (encountered,expected) in info.parameters.iter().zip(entry.arguments.iter()) { From a00121108721a7634225ba8909b23bf77b23219a Mon Sep 17 00:00:00 2001 From: mwu Date: Thu, 10 Sep 2020 02:05:41 +0200 Subject: [PATCH 21/34] wasm --- src/rust/ide/src/controller/graph/executed.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index e4ece42be1..10088e9b16 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -324,7 +324,7 @@ pub mod tests { } // Test that checks that value computed notification is properly relayed by the executed graph. - #[test] + #[wasm_bindgen_test] fn dispatching_value_computed_notification() { use crate::test::mock::Fixture; use crate::test::mock::Unified; From ecd670b655aff6406a9a88a3c8ece926b58254ef Mon Sep 17 00:00:00 2001 From: mwu Date: Thu, 10 Sep 2020 03:06:56 +0200 Subject: [PATCH 22/34] cleanups --- src/rust/ide/lib/span-tree/src/generate.rs | 19 ++++----------- src/rust/ide/src/controller/graph/executed.rs | 4 ++-- src/rust/ide/src/controller/searcher.rs | 9 ++++++-- src/rust/ide/src/test.rs | 6 +---- src/rust/ide/src/tests.rs | 23 ++++++++++--------- 5 files changed, 27 insertions(+), 34 deletions(-) diff --git a/src/rust/ide/lib/span-tree/src/generate.rs b/src/rust/ide/lib/span-tree/src/generate.rs index 3850022f9e..d1b45ef2c5 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -270,27 +270,20 @@ impl SpanTreeGenerator for ast::opr::Chain { impl SpanTreeGenerator for ast::prefix::Chain { fn generate_node(&self, kind:node::Kind, context:&impl Context) -> FallibleResult { - // TODO test for case when there are more arguments supplied than the known arity of function? - - //panic!(); - let base = ApplicationBase::new(&self.func); - let invocation_info = self.id().and_then(|id| context.call_info(id, base.function_name)); - let known_args = invocation_info.is_some(); - println!("AST {} has name {:?} and info {:?}",self.clone().into_ast(),base.function_name,invocation_info); + let base = ApplicationBase::new(&self.func); + let invocation_info = self.id().and_then(|id| context.call_info(id, base.function_name)); + let known_args = invocation_info.is_some(); let mut known_params = base.prefix_params(invocation_info); - dbg!(&base); - let arity = self.args.len().max(known_params.len()); - dbg!(arity); + let prefix_arity = self.args.len().max(known_params.len()); use ast::crumbs::PrefixCrumb::*; // Removing arguments is possible if there at least two of them let is_removable = self.args.len() >= 2; let node = self.func.generate_node(node::Kind::Operation,context); let ret = self.args.iter().enumerate().fold(node, |node,(i,arg)| { - println!("Will generate argument node for {}",arg.sast.wrapped); let node = node?; let is_first = i == 0; - let is_last = i + 1 == arity; + let is_last = i + 1 == prefix_arity; let arg_kind = if is_first && !base.has_target { node::Kind::Target {is_removable} } else { node::Kind::Argument {is_removable} }; @@ -439,8 +432,6 @@ fn generate_expected_arguments }) } -//fn fill_with_expected_args(base:Node, kind:node::Kind, explicit_args:usize, arity:usi) - // ============ diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index 10088e9b16..cd50934c5e 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -284,10 +284,10 @@ impl Context for Handle { pub mod tests { use super::*; - use crate::executor::test_utils::TestWithLocalPoolExecutor; use crate::model::execution_context::ExpressionId; - use enso_protocol::language_server::types::test::{value_update_with_type, value_update_with_method_ptr}; + use enso_protocol::language_server::types::test::value_update_with_type; + use enso_protocol::language_server::types::test::value_update_with_method_ptr; use utils::test::traits::*; use wasm_bindgen_test::wasm_bindgen_test; use wasm_bindgen_test::wasm_bindgen_test_configure; diff --git a/src/rust/ide/src/controller/searcher.rs b/src/rust/ide/src/controller/searcher.rs index 950e2bc5cd..5c601e4b73 100644 --- a/src/rust/ide/src/controller/searcher.rs +++ b/src/rust/ide/src/controller/searcher.rs @@ -761,7 +761,7 @@ impl SimpleFunctionCall { // ============= #[cfg(test)] -mod test { +pub mod test { use super::*; use crate::executor::test_utils::TestWithLocalPoolExecutor; @@ -774,13 +774,18 @@ mod test { use utils::test::traits::*; use enso_protocol::language_server::SuggestionId; - fn completion_response(results:&[SuggestionId]) -> language_server::response::Completion { + pub fn completion_response(results:&[SuggestionId]) -> language_server::response::Completion { language_server::response::Completion { results : results.to_vec(), current_version : default(), } } + pub fn expect_completion(client:&mut language_server::MockClient, results:&[SuggestionId]) { + let response = completion_response(results); + client.expect.completion(|_,_,_,_,_| Ok(response)) + } + #[derive(Debug,Derivative)] #[derivative(Default)] struct MockData { diff --git a/src/rust/ide/src/test.rs b/src/rust/ide/src/test.rs index f83fd782ab..66effd3777 100644 --- a/src/rust/ide/src/test.rs +++ b/src/rust/ide/src/test.rs @@ -221,11 +221,7 @@ pub mod mock { -> Fixture { let mut json_client = language_server::MockClient::default(); // Creating a searcher controller always triggers a query for completion. - let completion = language_server::response::Completion { - results : default(), - current_version : default(), - }; - json_client.expect.completion(|_,_,_,_,_| Ok(completion)); + controller::searcher::test::expect_completion(&mut json_client, &[]); customize_json_rpc(self,&mut json_client); let logger = Logger::default(); // TODO diff --git a/src/rust/ide/src/tests.rs b/src/rust/ide/src/tests.rs index d1c86dcda2..40ec70524b 100644 --- a/src/rust/ide/src/tests.rs +++ b/src/rust/ide/src/tests.rs @@ -9,9 +9,10 @@ use enso_protocol::project_manager; use json_rpc::expect_call; use json_rpc::test_util::transport::mock::MockTransport; use serde_json::json; +use span_tree::node::InsertType; +use span_tree::node::Kind; use wasm_bindgen_test::wasm_bindgen_test_configure; use wasm_bindgen_test::wasm_bindgen_test; -use span_tree::node::{InsertType, Kind}; wasm_bindgen_test_configure!(run_in_browser); @@ -79,21 +80,15 @@ async fn get_most_recent_project_or_create_new() { assert_eq!(expected_project, project.expect("Couldn't get project.")) } -#[wasm_bindgen_test] +#[test] fn span_tree_args() { use crate::test::mock::*; use span_tree::Node; let data = Unified::new(); let fixture = data.fixture_customize(|_,json_client| { - for _ in 0..2 { - json_client.expect.completion(|_, _, _, _, _| { - Ok(Completion { - results : vec![1], - current_version : default(), - }) - }); - } + // Additional completion request happens after picking completion. + controller::searcher::test::expect_completion(json_client,&[1]); }); let Fixture{graph,executed_graph,searcher,suggestion_db,..} = &fixture; let entry = suggestion_db.lookup(1).unwrap(); @@ -169,4 +164,10 @@ fn span_tree_args() { assert_eq!(get_param(1),None); assert_eq!(get_param(2),None); assert_eq!(get_param(3),None); -} + + // === Oversaturated call === + graph.set_expression(id,"foo Base 10 20 30").unwrap(); + assert_eq!(get_param(1).as_ref(),Some(&expected_this_param)); + assert_eq!(get_param(2).as_ref(),Some(&expected_arg1_param)); + assert_eq!(get_param(3).as_ref(),None); +} \ No newline at end of file From 18f3676239da77ec9e73640bd8bb516700a7a398 Mon Sep 17 00:00:00 2001 From: mwu Date: Thu, 10 Sep 2020 03:44:37 +0200 Subject: [PATCH 23/34] minor --- src/rust/ide/src/controller/graph/executed.rs | 3 +-- src/rust/ide/src/model/suggestion_database.rs | 8 ++++++++ src/rust/ide/src/test.rs | 9 +++++---- src/rust/ide/src/tests.rs | 3 +-- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index cd50934c5e..e5a8ad1209 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -327,9 +327,8 @@ pub mod tests { #[wasm_bindgen_test] fn dispatching_value_computed_notification() { use crate::test::mock::Fixture; - use crate::test::mock::Unified; // Setup the controller. - let mut fixture = crate::test::mock::Unified::new().fixture(); + let mut fixture = crate::test::mock::Unified::new().fixture(); let Fixture{executed_graph,execution,executor,..} = &mut fixture; // Generate notification. diff --git a/src/rust/ide/src/model/suggestion_database.rs b/src/rust/ide/src/model/suggestion_database.rs index a97232d180..58a83e87c5 100644 --- a/src/rust/ide/src/model/suggestion_database.rs +++ b/src/rust/ide/src/model/suggestion_database.rs @@ -253,8 +253,16 @@ pub fn to_span_tree_param } } + + +// ==================== +// === Notification === +// ==================== + +/// Notification about change in a suggestion database, #[derive(Clone,Copy,Debug,PartialEq)] pub enum Notification { + /// The database has been updated. Updated } diff --git a/src/rust/ide/src/test.rs b/src/rust/ide/src/test.rs index 66effd3777..21efaf6cc3 100644 --- a/src/rust/ide/src/test.rs +++ b/src/rust/ide/src/test.rs @@ -179,9 +179,9 @@ pub mod mock { } /// Create a graph controller from the current mock data. - pub fn graph(&self, module:model::Module, db:Rc) + pub fn graph + (&self, logger:impl AnyLogger, module:model::Module, db:Rc) -> crate::controller::Graph { - let logger = Logger::new("Test"); let id = self.graph_id.clone(); let parser = self.parser.clone_ref(); crate::controller::Graph::new(logger,module,db,parser,id).unwrap() @@ -224,11 +224,11 @@ pub mod mock { controller::searcher::test::expect_completion(&mut json_client, &[]); customize_json_rpc(self,&mut json_client); - let logger = Logger::default(); // TODO + let logger = Logger::new("UnifiedMock"); let module = self.module(); let suggestion_db = Rc::new(model::SuggestionDatabase::new_from_entries(&logger, &self.suggestions)); - let graph = self.graph(module.clone_ref(), suggestion_db.clone_ref()); + let graph = self.graph(&logger,module.clone_ref(),suggestion_db.clone_ref()); let execution = self.execution_context(); let project = self.project(module.clone_ref(),execution.clone_ref(), suggestion_db.clone_ref(),json_client); @@ -289,6 +289,7 @@ pub mod mock { } } +/// Check that given `CalledMethodInfo` is consistent with suggestion database `Entry`. pub fn assert_call_info ( info:span_tree::generate::context::CalledMethodInfo , entry:&model::suggestion_database::Entry diff --git a/src/rust/ide/src/tests.rs b/src/rust/ide/src/tests.rs index 40ec70524b..2d6c9313ed 100644 --- a/src/rust/ide/src/tests.rs +++ b/src/rust/ide/src/tests.rs @@ -4,7 +4,6 @@ use crate::controller::graph::NodeTrees; use crate::transport::test_utils::TestWithMockedTransport; use crate::ide::IdeInitializer; -use enso_protocol::language_server::response::Completion; use enso_protocol::project_manager; use json_rpc::expect_call; use json_rpc::test_util::transport::mock::MockTransport; @@ -170,4 +169,4 @@ fn span_tree_args() { assert_eq!(get_param(1).as_ref(),Some(&expected_this_param)); assert_eq!(get_param(2).as_ref(),Some(&expected_arg1_param)); assert_eq!(get_param(3).as_ref(),None); -} \ No newline at end of file +} From 47b3204b849e73e840640d0143536b8a7a2d8147 Mon Sep 17 00:00:00 2001 From: mwu Date: Thu, 10 Sep 2020 03:50:44 +0200 Subject: [PATCH 24/34] fix / cleanups --- src/rust/ide/src/controller/graph.rs | 29 +++++++++++++++------------- src/rust/ide/src/view/node_editor.rs | 2 +- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 6f1cca7397..8ac08b88f1 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -508,18 +508,14 @@ impl Handle { /// Returns information about all the connections between graph's nodes. /// - /// Will use `self` as the context for span tree generation. - pub fn connections_unevaluated_ctx(&self) -> FallibleResult { - let graph = self.graph_info()?; - Ok(Connections::new(&graph,self)) - } - - /// Returns information about all the connections between graph's nodes. + /// The context is used to create all span trees and possible affects the tree structure (so + /// port ids depend on context). + /// + /// To obtain connection using only the locally available data, one may invoke this method + /// passing `self` (i.e. the call target) as the context. pub fn connections (&self, context:&impl SpanTreeContext) -> FallibleResult { let graph = self.graph_info()?; - // TODO perhaps this should merge given context with the metadata information - // or perhaps this should just do exactly what it is told Ok(Connections::new(&graph,context)) } @@ -869,6 +865,13 @@ pub mod tests { use crate::model::suggestion_database; + /// Returns information about all the connections between graph's nodes. + /// + /// Will use `self` as the context for span tree generation. + pub fn connections(graph:Handle) -> FallibleResult { + graph.connections(&graph) + } + /// All the data needed to set up and run the graph controller in mock environment. #[derive(Clone,Debug)] pub struct MockData { @@ -1284,7 +1287,7 @@ main = print z"; test.data.code = PROGRAM.into(); test.run(|graph| async move { - let connections = graph.connections_unevaluated_ctx().unwrap(); + let connections = connections(graph).unwrap(); let (node0,node1,node2,node3,node4) = graph.nodes().unwrap().expect_tuple(); assert_eq!(node0.info.expression().repr(), "get_pos"); @@ -1385,7 +1388,7 @@ main = sum = _ + b"; test.data.code = PROGRAM.into(); test.run(|graph| async move { - assert!(graph.connections_unevaluated_ctx().unwrap().connections.is_empty()); + assert!(connections(graph).unwrap().connections.is_empty()); let (node0,_node1,node2) = graph.nodes().unwrap().expect_tuple(); let connection_to_add = Connection { source : Endpoint { @@ -1422,7 +1425,7 @@ main = calculate1 = calculate2 calculate3 calculate5 = calculate5 calculate4"; test.run(|graph| async move { - assert!(graph.connections_unevaluated_ctx().unwrap().connections.is_empty()); + assert!(connections(graph).unwrap().connections.is_empty()); let (node0,node1,_) = graph.nodes().unwrap().expect_tuple(); let connection_to_add = Connection { source : Endpoint { @@ -1483,7 +1486,7 @@ main = let expected = format!("{}{}",MAIN_PREFIX,self.dest_node_expected); let this = self.clone(); test.run(|graph| async move { - let connections = graph.connections_unevaluated_ctx().unwrap(); + let connections = connections(graph).unwrap(); let connection = connections.connections.first().unwrap(); graph.disconnect(connection,&span_tree::generate::context::Empty).unwrap(); let new_main = graph.graph_definition_info().unwrap().ast.repr(); diff --git a/src/rust/ide/src/view/node_editor.rs b/src/rust/ide/src/view/node_editor.rs index 90d3ede4fc..be66cc4ea9 100644 --- a/src/rust/ide/src/view/node_editor.rs +++ b/src/rust/ide/src/view/node_editor.rs @@ -348,7 +348,7 @@ impl GraphEditorIntegratedWithControllerModel { pub fn refresh_graph_view(&self) -> FallibleResult<()> { info!(self.logger, "Refreshing the graph view."); use controller::graph::Connections; - let Connections{trees,connections} = self.controller.graph().connections_unevaluated_ctx()?; + let Connections{trees,connections} = self.controller.connections()?; self.refresh_node_views(trees)?; self.refresh_connection_views(connections)?; Ok(()) From 365690e92ee8a7a3c9ee02601a47fbc547fa30f0 Mon Sep 17 00:00:00 2001 From: mwu Date: Thu, 10 Sep 2020 03:53:30 +0200 Subject: [PATCH 25/34] follow up fix --- src/rust/ide/src/controller/graph.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 8ac08b88f1..5df49714d7 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -868,8 +868,8 @@ pub mod tests { /// Returns information about all the connections between graph's nodes. /// /// Will use `self` as the context for span tree generation. - pub fn connections(graph:Handle) -> FallibleResult { - graph.connections(&graph) + pub fn connections(graph:&Handle) -> FallibleResult { + graph.connections(graph) } /// All the data needed to set up and run the graph controller in mock environment. @@ -1287,7 +1287,7 @@ main = print z"; test.data.code = PROGRAM.into(); test.run(|graph| async move { - let connections = connections(graph).unwrap(); + let connections = connections(&graph).unwrap(); let (node0,node1,node2,node3,node4) = graph.nodes().unwrap().expect_tuple(); assert_eq!(node0.info.expression().repr(), "get_pos"); @@ -1388,7 +1388,7 @@ main = sum = _ + b"; test.data.code = PROGRAM.into(); test.run(|graph| async move { - assert!(connections(graph).unwrap().connections.is_empty()); + assert!(connections(&graph).unwrap().connections.is_empty()); let (node0,_node1,node2) = graph.nodes().unwrap().expect_tuple(); let connection_to_add = Connection { source : Endpoint { @@ -1425,7 +1425,7 @@ main = calculate1 = calculate2 calculate3 calculate5 = calculate5 calculate4"; test.run(|graph| async move { - assert!(connections(graph).unwrap().connections.is_empty()); + assert!(connections(&graph).unwrap().connections.is_empty()); let (node0,node1,_) = graph.nodes().unwrap().expect_tuple(); let connection_to_add = Connection { source : Endpoint { @@ -1486,7 +1486,7 @@ main = let expected = format!("{}{}",MAIN_PREFIX,self.dest_node_expected); let this = self.clone(); test.run(|graph| async move { - let connections = connections(graph).unwrap(); + let connections = connections(&graph).unwrap(); let connection = connections.connections.first().unwrap(); graph.disconnect(connection,&span_tree::generate::context::Empty).unwrap(); let new_main = graph.graph_definition_info().unwrap().ast.repr(); From 9a8e5be5cbfb85e45ca1e8c706bb5d0378f6ee4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wawrzyniec=20Urba=C5=84czyk?= Date: Thu, 10 Sep 2020 14:02:36 +0200 Subject: [PATCH 26/34] Update src/rust/ide/src/controller/module.rs Co-authored-by: Adam Obuchowicz --- src/rust/ide/src/controller/module.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rust/ide/src/controller/module.rs b/src/rust/ide/src/controller/module.rs index 83eb4daf88..15ba55b8a5 100644 --- a/src/rust/ide/src/controller/module.rs +++ b/src/rust/ide/src/controller/module.rs @@ -97,8 +97,8 @@ impl Handle { pub fn graph_controller (&self, id:double_representation::graph::Id, suggestion_db:Rc) -> FallibleResult { - controller::Graph::new(&self.logger, self.model.clone_ref(), suggestion_db, - self.parser.clone_ref(), id) + controller::Graph::new(&self.logger,self.model.clone_ref(),suggestion_db, + self.parser.clone_ref(),id) } /// Returns a graph controller for graph in this module's subtree identified by `id` without From 9c496355e42d5f50e35b6a9fc2ef3fcfd49a86f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wawrzyniec=20Urba=C5=84czyk?= Date: Thu, 10 Sep 2020 14:02:50 +0200 Subject: [PATCH 27/34] Update src/rust/ide/src/model/project.rs Co-authored-by: Adam Obuchowicz --- src/rust/ide/src/model/project.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rust/ide/src/model/project.rs b/src/rust/ide/src/model/project.rs index fddabd11b0..8e21040593 100644 --- a/src/rust/ide/src/model/project.rs +++ b/src/rust/ide/src/model/project.rs @@ -110,7 +110,7 @@ pub mod test { project.expect_content_root_id().return_const(root_id); } - /// Sets up module expectation on the mock project, returning a give module. + /// Sets up module expectation on the mock project, returning a given module. pub fn expect_suggestion_db(project:&mut MockAPI, suggestion_db:Rc) { project.expect_suggestion_db().returning_st(move || suggestion_db.clone_ref()); } From f12d8617bef918740b8c87334928d2bba58fb2bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wawrzyniec=20Urba=C5=84czyk?= Date: Thu, 10 Sep 2020 14:02:57 +0200 Subject: [PATCH 28/34] Update src/rust/ide/src/controller/module.rs Co-authored-by: Adam Obuchowicz --- src/rust/ide/src/controller/module.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rust/ide/src/controller/module.rs b/src/rust/ide/src/controller/module.rs index 15ba55b8a5..4abbe2d92c 100644 --- a/src/rust/ide/src/controller/module.rs +++ b/src/rust/ide/src/controller/module.rs @@ -106,8 +106,8 @@ impl Handle { pub fn graph_controller_unchecked (&self, id:double_representation::graph::Id, suggestion_db:Rc) -> controller::Graph { - controller::Graph::new_unchecked(&self.logger, self.model.clone_ref(), suggestion_db, - self.parser.clone_ref(), id) + controller::Graph::new_unchecked(&self.logger,self.model.clone_ref(),suggestion_db, + self.parser.clone_ref(),id) } /// Get the module's qualified name. From b9d2a03e994ed91df4ea8e20e17a893886a54809 Mon Sep 17 00:00:00 2001 From: mwu Date: Thu, 10 Sep 2020 16:25:23 +0200 Subject: [PATCH 29/34] cr feedback --- .../ide/lib/span-tree/src/generate/context.rs | 41 ------------------- src/rust/ide/src/controller/graph.rs | 8 ++-- src/rust/ide/src/model/module/plain.rs | 16 +++----- 3 files changed, 10 insertions(+), 55 deletions(-) diff --git a/src/rust/ide/lib/span-tree/src/generate/context.rs b/src/rust/ide/lib/span-tree/src/generate/context.rs index 81c2f40fb8..afbf07042a 100644 --- a/src/rust/ide/lib/span-tree/src/generate/context.rs +++ b/src/rust/ide/lib/span-tree/src/generate/context.rs @@ -33,46 +33,6 @@ pub trait Context { /// Trait implementors may used it to filter-out results, however they are not required to do /// so. Caller should not assume that the called method has the same name as given identifier. fn call_info(&self, id:Id, name:Option<&str>) -> Option; - - /// Build a new context that merges this context and the one given in argument that will be used - /// as a fallback. - fn merge(self, other:U) -> Merged - where Self:Sized, U:Context { - Merged::new(self,other) - } -} - - - -// =============== -// === Context === -// =============== - -/// Represents a context created from merging two other contexts. -#[derive(Clone,Debug)] -pub struct Merged { - first : First, - second : Second -} - -impl Merged { - /// Creates a context merging the contexts from arguments. - /// - /// The first context is checked first, the second one is used as a fallback. - pub fn new(first:First, second:Second) -> Self { - Self { - first,second - } - } -} - -impl Context for Merged - where First : Context, - Second : Context { - fn call_info(&self, id: Id, name:Option<&str>) -> Option { - self.first.call_info(id,name).or_else(|| - self.second.call_info(id,name)) - } } @@ -90,4 +50,3 @@ impl Context for Empty { None } } - diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 5df49714d7..c86337b4b8 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -224,12 +224,12 @@ pub struct Connections { impl Connections { /// Describes a connection for given double representation graph. pub fn new(graph:&GraphInfo, context:&impl SpanTreeContext) -> Connections { - let trees = graph.nodes().iter().flat_map(|node| { + let trees = graph.nodes().iter().filter_map(|node| { Some((node.id(), NodeTrees::new(node,context)?)) }).collect(); - let mut ret = Connections {trees, connections:default()}; - let connections = graph.connections().into_iter().flat_map(|c| + let mut ret = Connections {trees, connections:default()}; + let connections = graph.connections().into_iter().filter_map(|c| ret.convert_connection(&c) ).collect(); ret.connections = connections; @@ -239,7 +239,7 @@ impl Connections { /// Converts Endpoint from double representation to the span tree crumbs. pub fn convert_endpoint (&self, endpoint:&double_representation::connection::Endpoint) -> Option { - let tree = self.trees.get(&endpoint.node)?; + let tree = self.trees.get(&endpoint.node)?; let span_tree_node = tree.get_span_tree_node(&endpoint.crumbs)?; Some(Endpoint{ node : endpoint.node, diff --git a/src/rust/ide/src/model/module/plain.rs b/src/rust/ide/src/model/module/plain.rs index f1b6602a5e..d5fe068dd5 100644 --- a/src/rust/ide/src/model/module/plain.rs +++ b/src/rust/ide/src/model/module/plain.rs @@ -33,10 +33,6 @@ impl Module { notifications : default(), } } - - fn notify(&self, notification:Notification) { - self.notifications.notify(notification) - } } impl model::module::API for Module { @@ -69,12 +65,12 @@ impl model::module::API for Module { fn update_whole(&self, content:Content) { *self.content.borrow_mut() = content; - self.notify(Notification::Invalidate); + self.notifications.notify(Notification::Invalidate); } fn update_ast(&self, ast:ast::known::Module) { self.content.borrow_mut().ast = ast; - self.notify(Notification::Invalidate); + self.notifications.notify(Notification::Invalidate); } fn apply_code_change @@ -84,19 +80,19 @@ impl model::module::API for Module { change.apply(&mut code); let new_ast = parser.parse(code,new_id_map)?.try_into()?; self.content.borrow_mut().ast = new_ast; - self.notify(Notification::CodeChanged {change,replaced_location}); + self.notifications.notify(Notification::CodeChanged {change,replaced_location}); Ok(()) } fn set_node_metadata(&self, id:ast::Id, data:NodeMetadata) { self.content.borrow_mut().metadata.ide.node.insert(id, data); - self.notify(Notification::MetadataChanged); + self.notifications.notify(Notification::MetadataChanged); } fn remove_node_metadata(&self, id:ast::Id) -> FallibleResult { let lookup = self.content.borrow_mut().metadata.ide.node.remove(&id); let data = lookup.ok_or_else(|| NodeMetadataNotFound(id))?; - self.notify(Notification::MetadataChanged); + self.notifications.notify(Notification::MetadataChanged); Ok(data) } @@ -105,7 +101,7 @@ impl model::module::API for Module { let mut data = lookup.unwrap_or_default(); fun(&mut data); self.content.borrow_mut().metadata.ide.node.insert(id, data); - self.notify(Notification::MetadataChanged); + self.notifications.notify(Notification::MetadataChanged); } } From 1ab6d3340c7fa42daf1165549aefe432faf64be2 Mon Sep 17 00:00:00 2001 From: mwu Date: Thu, 10 Sep 2020 17:50:12 +0200 Subject: [PATCH 30/34] fixed tests --- src/rust/ide/src/test.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/rust/ide/src/test.rs b/src/rust/ide/src/test.rs index 21efaf6cc3..1bfecff69f 100644 --- a/src/rust/ide/src/test.rs +++ b/src/rust/ide/src/test.rs @@ -117,7 +117,6 @@ pub mod mock { pub logger : Logger, pub project_name : String, pub module_path : model::module::Path, - pub graph_id : double_representation::graph::Id, pub suggestions : HashMap, pub context_id : model::execution_context::Id, pub parser : parser::Parser, @@ -130,9 +129,6 @@ pub mod mock { impl Unified { pub fn set_inline_code(&mut self, code:impl AsRef) { let method = self.method_pointer(); - //self.code = format!("{}.{} = {}",method.defined_on_type,method.name,code.as_ref()) - // FIXME [mwu] should be as above but definition lookup doesn't handle properly implicit - // module name self.code = format!("{} = {}",method.name,code.as_ref()) } @@ -146,7 +142,6 @@ pub mod mock { logger : Logger::default(), project_name : PROJECT_NAME.to_owned(), module_path : module_path(), - graph_id : graph_id(), code : CODE.to_owned(), id_map : default(), metadata : default(), @@ -181,10 +176,12 @@ pub mod mock { /// Create a graph controller from the current mock data. pub fn graph (&self, logger:impl AnyLogger, module:model::Module, db:Rc) - -> crate::controller::Graph { - let id = self.graph_id.clone(); - let parser = self.parser.clone_ref(); - crate::controller::Graph::new(logger,module,db,parser,id).unwrap() + -> crate::controller::Graph { + let parser = self.parser.clone_ref(); + let method = self.method_pointer(); + let module_ast = module.ast(); + let definition = module::lookup_method(&module_ast,&method).unwrap(); + crate::controller::Graph::new(logger,module,db,parser,definition).unwrap() } pub fn execution_context(&self) -> model::ExecutionContext { From c3208b332546e40802d84fdabced968020f174b1 Mon Sep 17 00:00:00 2001 From: mwu Date: Thu, 10 Sep 2020 20:24:11 +0200 Subject: [PATCH 31/34] note, wasm test --- src/rust/ide/src/controller/graph/executed.rs | 11 +++++++++++ src/rust/ide/src/tests.rs | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index e5a8ad1209..d2ad1004e3 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -163,6 +163,17 @@ impl Handle { futures::stream::select_all(streams) } + // Note [Argument Names-related invalidations] + // =========================================== + // Currently the shape of span tree depends on how method calls are recognized and resolved. + // This involves lookups in the metadata, computed values registry and suggestion database. + // As such, any of these will lead to emission of invalidation signal, as span tree shape + // affects the connection identifiers. + // + // This is inefficient and should be addressed in the future. + // See: https://github.com/enso-org/ide/issues/787 + + /// Get a type of the given expression as soon as it is available. pub fn expression_type(&self, id:ast::Id) -> StaticBoxFuture> { let registry = self.execution_ctx.computed_value_info_registry(); diff --git a/src/rust/ide/src/tests.rs b/src/rust/ide/src/tests.rs index 2d6c9313ed..3e4ce95ee4 100644 --- a/src/rust/ide/src/tests.rs +++ b/src/rust/ide/src/tests.rs @@ -79,7 +79,7 @@ async fn get_most_recent_project_or_create_new() { assert_eq!(expected_project, project.expect("Couldn't get project.")) } -#[test] +#[wasm_bindgen_test] fn span_tree_args() { use crate::test::mock::*; use span_tree::Node; From 8f8e251217cefda8473614094b4eea48a8bfe12f Mon Sep 17 00:00:00 2001 From: mwu Date: Fri, 11 Sep 2020 03:43:24 +0200 Subject: [PATCH 32/34] Update span trees even if expression wasn't changed. --- src/rust/ide/src/view/node_editor.rs | 42 ++++++++++++---------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/rust/ide/src/view/node_editor.rs b/src/rust/ide/src/view/node_editor.rs index be66cc4ea9..de92087800 100644 --- a/src/rust/ide/src/view/node_editor.rs +++ b/src/rust/ide/src/view/node_editor.rs @@ -439,32 +439,26 @@ impl GraphEditorIntegratedWithControllerModel { self.view.graph().frp.inputs.set_node_position.emit_event(&(id, position.vector)); } let expression = node.info.expression().repr(); - let expression_changed = with(self.expression_views.borrow(), |expression_views| { - let expression_view = expression_views.get(&id); - // The node expression will newer contain spaces at the both ends, however user could - // decide to put some in node's edited expression; thus we should not eagerly remove - // those spaces. - let trimmed_view = expression_view.map(|e| e.trim()); - !trimmed_view.contains(&expression) - }); - if expression_changed { - let code_and_trees = graph_editor::component::node::port::Expression { - code : expression.clone(), - input_span_tree : trees.inputs, - output_span_tree : trees.outputs.unwrap_or_else(default) - }; - self.view.graph().frp.inputs.set_node_expression.emit_event(&(id, code_and_trees)); - self.expression_views.borrow_mut().insert(id, expression); - - // Set initially available type information on ports (identifiable expression's - // sub-parts). - for expression_part in node.info.expression().iter_recursive() { - if let Some(id) = expression_part.id { - self.refresh_computed_info(id); - } + + // TODO [MWU] + // Currently we cannot limit updates, as each invalidation can affect span tree generation + // context and as such may require updating span trees. So no matter whether expression + // changed or not, we shall emit the updates. + // This should be addressed as part of https://github.com/enso-org/ide/issues/787 + let code_and_trees = graph_editor::component::node::port::Expression { + code : expression.clone(), + input_span_tree : trees.inputs, + output_span_tree : trees.outputs.unwrap_or_else(default) + }; + self.view.graph().frp.inputs.set_node_expression.emit_event(&(id, code_and_trees)); + self.expression_views.borrow_mut().insert(id, expression); + + // Set initially available type information on ports (identifiable expression's sub-parts). + for expression_part in node.info.expression().iter_recursive() { + if let Some(id) = expression_part.id { + self.refresh_computed_info(id); } } - } /// Like `refresh_computed_info` but for multiple expressions. From bfee177d44cf4ff33c73886858cfa9efe3df046b Mon Sep 17 00:00:00 2001 From: mwu Date: Fri, 11 Sep 2020 15:24:11 +0200 Subject: [PATCH 33/34] fixing the shaky test --- src/rust/ide/lib/utils/src/test/stream.rs | 24 ++++++++++++++ src/rust/ide/src/controller/graph/executed.rs | 31 +++++++++++++------ 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/src/rust/ide/lib/utils/src/test/stream.rs b/src/rust/ide/lib/utils/src/test/stream.rs index e922677a6c..0611665d42 100644 --- a/src/rust/ide/lib/utils/src/test/stream.rs +++ b/src/rust/ide/lib/utils/src/test/stream.rs @@ -56,6 +56,30 @@ pub trait StreamTestExt { panic!("Stream has terminated, while it should be waiting for the next value."), } } + + /// Asserts that the stream has two values ready. First needs to match against any of the + /// given predicates, the second one against the other predicate. + /// + /// The order of these two values is irrelevant. + fn expect_both(&mut self, one:impl Fn(&S::Item) -> bool, other:impl Fn(&S::Item) -> bool) { + self.expect_many(vec![Box::new(one),Box::new(other)]) + } + + /// Expect many items being ready but in arbitrary order. + /// + /// Takes a list of predicates. Items are matched against them, after predicate succeeds match + /// it is removed from the list. + fn expect_many<'a>(&mut self, mut expected:Vec bool + 'a>>) { + while !expected.is_empty() { + let item = self.expect_next(); + match expected.iter().find_position(|expected_predicate| expected_predicate(&item)) { + Some((index,_)) => { let _ = expected.remove(index); } + _ => + panic!("Stream yielded item that did not match to any of the given predicates. \ + Item: {:?}"), + } + } + } } impl StreamTestExt for Pin

diff --git a/src/rust/ide/src/controller/graph/executed.rs b/src/rust/ide/src/controller/graph/executed.rs index d2ad1004e3..a6ff1d38e5 100644 --- a/src/rust/ide/src/controller/graph/executed.rs +++ b/src/rust/ide/src/controller/graph/executed.rs @@ -303,6 +303,7 @@ pub mod tests { use wasm_bindgen_test::wasm_bindgen_test; use wasm_bindgen_test::wasm_bindgen_test_configure; use crate::model::module::NodeMetadata; + use logger::enabled::Logger; wasm_bindgen_test_configure!(run_in_browser); @@ -358,16 +359,26 @@ pub mod tests { executor.run_until_stalled(); // Observing that notification was relayed. - let observed_notification = notifications.expect_next(); - let typename_in_registry = registry.get(&updated_id).unwrap().typename.clone(); - let expected_typename = Some(ImString::new(typename)); - assert_eq!(observed_notification,Notification::ComputedValueInfo(vec![updated_id])); - assert_eq!(typename_in_registry,expected_typename); - - // Having a computed value also requires graph invalidation (span tree context change). - let observed_notification = notifications.expect_next(); - let expected = Notification::Graph(controller::graph::Notification::Invalidate); - assert_eq!(observed_notification,expected); + // Both computed values update and graph invalidation are expected, in any order. + notifications.expect_both( + |notification| match notification { + Notification::ComputedValueInfo(updated_ids) => { + assert_eq!(updated_ids,&vec![updated_id]); + let typename_in_registry = registry.get(&updated_id).unwrap().typename.clone(); + let expected_typename = Some(ImString::new(typename)); + assert_eq!(typename_in_registry,expected_typename); + true + } + _ => false, + }, + |notification| match notification { + Notification::Graph(graph_notification) => { + assert_eq!(graph_notification,&controller::graph::Notification::Invalidate); + true + } + _ => false, + }); + notifications.expect_pending(); } From 5d42397f1bc884800c28a774f98c1d28e62dd118 Mon Sep 17 00:00:00 2001 From: mwu Date: Fri, 11 Sep 2020 15:51:21 +0200 Subject: [PATCH 34/34] clippy --- src/rust/ide/lib/utils/src/test/stream.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/rust/ide/lib/utils/src/test/stream.rs b/src/rust/ide/lib/utils/src/test/stream.rs index 0611665d42..debc768580 100644 --- a/src/rust/ide/lib/utils/src/test/stream.rs +++ b/src/rust/ide/lib/utils/src/test/stream.rs @@ -61,7 +61,8 @@ pub trait StreamTestExt { /// given predicates, the second one against the other predicate. /// /// The order of these two values is irrelevant. - fn expect_both(&mut self, one:impl Fn(&S::Item) -> bool, other:impl Fn(&S::Item) -> bool) { + fn expect_both(&mut self, one:impl Fn(&S::Item) -> bool, other:impl Fn(&S::Item) -> bool) + where S::Item:Debug { self.expect_many(vec![Box::new(one),Box::new(other)]) } @@ -69,14 +70,15 @@ pub trait StreamTestExt { /// /// Takes a list of predicates. Items are matched against them, after predicate succeeds match /// it is removed from the list. - fn expect_many<'a>(&mut self, mut expected:Vec bool + 'a>>) { + fn expect_many<'a>(&mut self, mut expected:Vec bool + 'a>>) + where S::Item:Debug { while !expected.is_empty() { let item = self.expect_next(); match expected.iter().find_position(|expected_predicate| expected_predicate(&item)) { Some((index,_)) => { let _ = expected.remove(index); } _ => panic!("Stream yielded item that did not match to any of the given predicates. \ - Item: {:?}"), + Item: {:?}",item), } } }