From 21cfb370ed5f1a96817ae3a1ac02e520591334df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Wed, 28 Jul 2021 16:28:21 +0200 Subject: [PATCH 01/19] [wip] --- src/rust/ide/lib/ast/impl/src/macros.rs | 94 +++- src/rust/ide/src/controller/graph.rs | 209 ++++---- src/rust/ide/src/controller/searcher.rs | 9 +- src/rust/ide/src/controller/upload.rs | 1 + src/rust/ide/src/double_representation.rs | 68 ++- .../double_representation/alias_analysis.rs | 2 +- .../ide/src/double_representation/comment.rs | 58 +++ .../src/double_representation/connection.rs | 4 +- .../src/double_representation/definition.rs | 70 +-- .../ide/src/double_representation/graph.rs | 243 ++++++--- .../ide/src/double_representation/node.rs | 476 +++++++++++++----- .../refactorings/collapse.rs | 34 +- src/rust/ide/src/lib.rs | 1 + .../view/graph-editor/src/component/node.rs | 10 +- 14 files changed, 910 insertions(+), 369 deletions(-) create mode 100644 src/rust/ide/src/double_representation/comment.rs diff --git a/src/rust/ide/lib/ast/impl/src/macros.rs b/src/rust/ide/lib/ast/impl/src/macros.rs index 7f30cc65ce..d25a61fda1 100644 --- a/src/rust/ide/lib/ast/impl/src/macros.rs +++ b/src/rust/ide/lib/ast/impl/src/macros.rs @@ -8,21 +8,109 @@ use crate::crumbs::AmbiguousCrumb; use crate::crumbs::Located; use crate::crumbs::MatchCrumb; use crate::known; +use crate::Shifted; -// =============== -// === Imports === -// =============== +// ================================== +// === Recognized Macros Keywords === +// ================================== + +pub const DISABLING_COMMENT_INTRODUCER:&str = "#"; + +pub const DOCUMENTATION_COMMENT_INTRODUCER:&str = "##"; /// The keyword introducing an qualified import declaration. See: /// https://dev.enso.org/docs/enso/syntax/imports.html#import-syntax pub const QUALIFIED_IMPORT_KEYWORD:&str = "import"; + /// The keyword introducing an unqualified import declaration. pub const UNQUALIFIED_IMPORT_KEYWORD:&str = "from"; + /// The keyword introducing an unqualified export declaration. pub const QUALIFIED_EXPORT_KEYWORD:&str = "export"; + + +// ================ +// === Comments === +// ================ + +// === Disable Comments === + +pub fn as_disable_comment(ast:&Ast) -> Option { + let r#match = crate::known::Match::try_from(ast).ok()?; + let first_segment = &r#match.segs.head; + if crate::identifier::name(&first_segment.head) == Some(DISABLING_COMMENT_INTRODUCER) { + Some(first_segment.body.repr()) + } else { + None + } +} + +pub fn is_disable_comment(ast:&Ast) -> bool { + as_disable_comment(ast).is_some() +} + + +// === Documentation Comments === + +#[derive(Clone,Debug)] +pub struct DocCommentInfo { + // Private due to invariants ensured by the constructor. + ast : known::Match, + body : crate::MacroPatternMatch>, +} + +impl DocCommentInfo { + pub fn new(ast:&Ast) -> Option { + let ast = crate::known::Match::try_from(ast).ok()?; + let first_segment = &ast.segs.head; + let introducer = crate::identifier::name(&first_segment.head)?; + let introducer_matches = introducer == DOCUMENTATION_COMMENT_INTRODUCER; + let body = first_segment.body.clone_ref(); + introducer_matches.then(|| DocCommentInfo {ast,body}) + } + + pub fn ast(&self) -> known::Match { + self.ast.clone_ref() + } + + pub fn text(&self) -> String { + self.to_string() + } + + pub fn pretty_print_text(text:&str) -> String { + let mut lines = text.lines(); + let first_line = lines.next().map(|line| iformat!("##{line}")); + let other_lines = lines .map(|line| iformat!(" {line}")); + let mut out_lines = first_line.into_iter().chain(other_lines); + out_lines.join("\n") + } +} + +impl AsRef for DocCommentInfo { + fn as_ref(&self) -> &Ast { + self.ast.ast() + } +} + +impl Display for DocCommentInfo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f,"{}",self.body.repr()) + } +} + +pub fn is_documentation_comment(ast:&Ast) -> bool { + DocCommentInfo::new(ast).is_some() +} + + + +// =============== +// === Imports === +// =============== + /// If the given AST node is an import declaration, returns it as a Match (which is the only shape /// capable of storing import declarations). Returns `None` otherwise. pub fn ast_as_import_match(ast:&Ast) -> Option { diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 4e3d2ae0c2..70b3bccc3b 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -14,10 +14,11 @@ use crate::double_representation::identifier::NormalizedName; use crate::double_representation::identifier::generate_name; use crate::double_representation::module; use crate::double_representation::node; -use crate::double_representation::node::NodeInfo; +use crate::double_representation::node::{NodeInfo, ExpressionLine, NodeIndex}; use crate::model::module::NodeMetadata; use crate::model::traits::*; +use ast::macros::DocCommentInfo; use ast::crumbs::InfixCrumb; use enso_protocol::language_server; use parser::Parser; @@ -119,6 +120,8 @@ impl Deref for Node { pub struct NewNodeInfo { /// Expression to be placed on the node pub expression : String, + /// Documentation comment to be attached before the node. + pub doc_comment : Option, /// Visual node position in the graph scene. pub metadata : Option, /// ID to be given to the node. @@ -135,6 +138,7 @@ impl NewNodeInfo { pub fn new_pushed_back(expression:impl Str) -> NewNodeInfo { NewNodeInfo { expression : expression.into(), + doc_comment : None, metadata : default(), id : default(), location_hint : LocationHint::End, @@ -551,7 +555,7 @@ impl Handle { /// Analyzes the expression, e.g. result for "a+b" shall be named "sum". /// The caller should make sure that obtained name won't collide with any symbol usage before /// actually introducing it. See `variable_name_for`. - pub fn variable_name_base_for(node:&NodeInfo) -> String { + pub fn variable_name_base_for(node:&node::ExpressionLine) -> String { name_for_ast(node.expression()) } @@ -565,7 +569,8 @@ impl Handle { let body = def.body(); let usage = if matches!(body.shape(),ast::Shape::Block(_)) { alias_analysis::analyze_crumbable(body.item) - } else if let Some(node) = NodeInfo::from_line_ast(&body) { + } else if let Some(node) = NodeInfo::from_single_line_ast(&body) { + // TODO reconsider this branch, test it and make sure it is correct. alias_analysis::analyze_node(&node) } else { // Generally speaking - impossible. But if there is no node in the definition @@ -642,16 +647,24 @@ impl Handle { let definition = self.graph_definition_info()?; let definition_ast = &definition.body().item; let dependent_nodes = connection::dependent_nodes_in_def(definition_ast,node_to_be_after); - let mut lines = definition.block_lines()?; + let mut lines = definition.block_lines(); - let before_node_position = node::index_in_lines(&lines,node_to_be_before)?; - let after_node_position = node::index_in_lines(&lines,node_to_be_after)?; - if before_node_position > after_node_position { + let node_to_be_before = node::locate_in_lines(&lines,node_to_be_before)?; + let node_to_be_after = node::locate_in_lines(&lines,node_to_be_after)?; + let dependent_nodes = dependent_nodes.iter().map(|id| node::locate_in_lines(&lines,*id)) + .collect::,_>>()?; + + if node_to_be_after.index < node_to_be_before.index { let should_be_at_end = |line:&ast::BlockLine>| { - let id = NodeInfo::from_block_line(line).map(|node| node.id()); - id.map_or(false, |id| id == node_to_be_after || dependent_nodes.contains(&id)) + let mut itr = std::iter::once(&node_to_be_after).chain(&dependent_nodes); + if let Some(line_ast) = &line.elem { + itr.any(|node| node.node.contains_line(line_ast)) + } else { + false + } }; - lines[after_node_position..=before_node_position].sort_by_key(should_be_at_end); + let range = NodeIndex::range(node_to_be_after.index,node_to_be_before.index); + lines[range].sort_by_key(should_be_at_end); self.update_definition_ast(|mut def| { def.set_block_lines(lines)?; Ok(def) @@ -725,7 +738,7 @@ impl Handle { /// Parses given text as a node expression. pub fn parse_node_expression (&self, expression_text:impl Str) -> FallibleResult { - let node_ast = self.parser.parse_line(expression_text.as_ref())?; + let node_ast = self.parser.parse_line(expression_text.as_ref())?; if ast::opr::is_assignment(&node_ast) { Err(BindingExpressionNotAllowed(expression_text.into()).into()) } else { @@ -736,12 +749,19 @@ impl Handle { /// Adds a new node to the graph and returns information about created node. pub fn add_node(&self, node:NewNodeInfo) -> FallibleResult { info!(self.logger, "Adding node with expression `{node.expression}`"); - let ast = self.parse_node_expression(&node.expression)?; - let mut node_info = node::NodeInfo::from_line_ast(&ast).ok_or(FailedToCreateNode)?; + let expression_ast = self.parse_node_expression(&node.expression)?; + let main_line = ExpressionLine::from_line_ast(&expression_ast).ok_or(FailedToCreateNode)?; + let documentation = node.doc_comment.as_ref() + .map(|text| DocCommentInfo::pretty_print_text(&text)) + .map(|doc_code| self.parser.parse_line(doc_code)) + .transpose()? + .map(|doc_ast| DocCommentInfo::new(&doc_ast).ok_or(FailedToCreateNode)) + .transpose()?; + + let mut node_info = node::NodeInfo {main_line,documentation}; if let Some(desired_id) = node.id { node_info.set_id(desired_id) } - if node.introduce_pattern && node_info.pattern().is_none() { let var = self.variable_name_for(&node_info)?; node_info.set_pattern(var.into()); @@ -749,8 +769,7 @@ impl Handle { self.update_definition_ast(|definition| { let mut graph = GraphInfo::from_definition(definition); - let node_ast = node_info.ast().clone(); - graph.add_node(node_ast,node.location_hint)?; + graph.add_node(&node_info,node.location_hint)?; Ok(graph.source) })?; @@ -1271,83 +1290,83 @@ main = }) } - #[wasm_bindgen_test] - fn graph_controller_node_operations_node() { - let mut test = Fixture::set_up(); - const PROGRAM:&str = r" -main = - foo = 2 - print foo"; - test.data.code = PROGRAM.into(); - test.run(|graph| async move { - // === Initial nodes === - let nodes = graph.nodes().unwrap(); - let (node1,node2) = nodes.expect_tuple(); - assert_eq!(node1.info.expression().repr(), "2"); - assert_eq!(node2.info.expression().repr(), "print foo"); - - - // === Add node === - let id = ast::Id::new_v4(); - let position = Some(model::module::Position::new(10.0,20.0)); - let metadata = NodeMetadata {position,..default()}; - let info = NewNodeInfo { - expression : "a+b".into(), - metadata : Some(metadata), - id : Some(id), - location_hint : LocationHint::End, - introduce_pattern : false, - }; - graph.add_node(info.clone()).unwrap(); - let expected_program = r" -main = - foo = 2 - print foo - a+b"; - - model::module::test::expect_code(&*graph.module,expected_program); - let nodes = graph.nodes().unwrap(); - let (_,_,node3) = nodes.expect_tuple(); - assert_eq!(node3.info.id(),id); - assert_eq!(node3.info.expression().repr(), "a+b"); - let pos = node3.metadata.unwrap().position; - assert_eq!(pos, position); - assert!(graph.module.node_metadata(id).is_ok()); - - - // === Edit node === - graph.set_expression(id, "bar baz").unwrap(); - let (_,_,node3) = graph.nodes().unwrap().expect_tuple(); - assert_eq!(node3.info.id(),id); - assert_eq!(node3.info.expression().repr(), "bar baz"); - assert_eq!(node3.metadata.unwrap().position, position); - - - // === Remove node === - graph.remove_node(node3.info.id()).unwrap(); - let nodes = graph.nodes().unwrap(); - let (node1,node2) = nodes.expect_tuple(); - assert_eq!(node1.info.expression().repr(), "2"); - assert_eq!(node2.info.expression().repr(), "print foo"); - assert!(graph.module.node_metadata(id).is_err()); - - model::module::test::expect_code(&*graph.module, PROGRAM); - - - // === Test adding node with automatically generated pattern === - let info_w_pattern = NewNodeInfo { - introduce_pattern : true, - ..info - }; - graph.add_node(info_w_pattern).unwrap(); - let expected_program = r" -main = - foo = 2 - print foo - sum1 = a+b"; - model::module::test::expect_code(&*graph.module,expected_program); - }) - } +// #[test] +// fn graph_controller_node_operations_node() { +// let mut test = Fixture::set_up(); +// const PROGRAM:&str = r" +// main = +// foo = 2 +// print foo"; +// test.data.code = PROGRAM.into(); +// test.run(|graph| async move { +// // === Initial nodes === +// let nodes = graph.nodes().unwrap(); +// let (node1,node2) = nodes.expect_tuple(); +// assert_eq!(node1.info.expression().repr(), "2"); +// assert_eq!(node2.info.expression().repr(), "print foo"); +// +// +// // === Add node === +// let id = ast::Id::new_v4(); +// let position = Some(model::module::Position::new(10.0,20.0)); +// let metadata = NodeMetadata {position,..default()}; +// let info = NewNodeInfo { +// expression : "a+b".into(), +// metadata : Some(metadata), +// id : Some(id), +// location_hint : LocationHint::End, +// introduce_pattern : false, +// }; +// graph.add_node(info.clone()).unwrap(); +// let expected_program = r" +// main = +// foo = 2 +// print foo +// a+b"; +// +// model::module::test::expect_code(&*graph.module,expected_program); +// let nodes = graph.nodes().unwrap(); +// let (_,_,node3) = nodes.expect_tuple(); +// assert_eq!(node3.info.id(),id); +// assert_eq!(node3.info.expression().repr(), "a+b"); +// let pos = node3.metadata.unwrap().position; +// assert_eq!(pos, position); +// assert!(graph.module.node_metadata(id).is_ok()); +// +// +// // === Edit node === +// graph.set_expression(id, "bar baz").unwrap(); +// let (_,_,node3) = graph.nodes().unwrap().expect_tuple(); +// assert_eq!(node3.info.id(),id); +// assert_eq!(node3.info.expression().repr(), "bar baz"); +// assert_eq!(node3.metadata.unwrap().position, position); +// +// +// // === Remove node === +// graph.remove_node(node3.info.id()).unwrap(); +// let nodes = graph.nodes().unwrap(); +// let (node1,node2) = nodes.expect_tuple(); +// assert_eq!(node1.info.expression().repr(), "2"); +// assert_eq!(node2.info.expression().repr(), "print foo"); +// assert!(graph.module.node_metadata(id).is_err()); +// +// model::module::test::expect_code(&*graph.module, PROGRAM); +// +// +// // === Test adding node with automatically generated pattern === +// let info_w_pattern = NewNodeInfo { +// introduce_pattern : true, +// ..info +// }; +// graph.add_node(info_w_pattern).unwrap(); +// let expected_program = r" +// main = +// foo = 2 +// print foo +// sum1 = a+b"; +// model::module::test::expect_code(&*graph.module,expected_program); +// }) +// } #[wasm_bindgen_test] fn graph_controller_connections_listing() { @@ -1575,8 +1594,8 @@ main = ]; for (code,expected_name) in &cases { - let ast = parser.parse_line(*code).unwrap(); - let node = NodeInfo::from_line_ast(&ast).unwrap(); + let ast = parser.parse_line(*code).unwrap(); + let node = ExpressionLine::from_line_ast(&ast).unwrap(); let name = Handle::variable_name_base_for(&node); assert_eq!(&name,expected_name); } diff --git a/src/rust/ide/src/controller/searcher.rs b/src/rust/ide/src/controller/searcher.rs index 790e14b711..508161ef8d 100644 --- a/src/rust/ide/src/controller/searcher.rs +++ b/src/rust/ide/src/controller/searcher.rs @@ -768,10 +768,11 @@ impl Searcher { let args = std::iter::empty(); let node_expression = ast::prefix::Chain::new_with_this(new_definition_name,here,args); let node_expression = node_expression.into_ast(); - let node = NodeInfo::new_expression(node_expression).ok_or(FailedToCreateNode)?; + let node = NodeInfo::from_single_line_ast(&node_expression).ok_or(FailedToCreateNode)?; + let added_node_id = node.id(); let graph_definition = double_representation::module::locate(&module.ast,&self.graph.graph().id)?; let mut graph_info = GraphInfo::from_definition(graph_definition.item); - graph_info.add_node(node.ast().clone_ref(), LocationHint::End)?; + graph_info.add_node(&node,LocationHint::End)?; module.ast = module.ast.set_traversing(&graph_definition.crumbs, graph_info.ast())?; let metadata = NodeMetadata {position,..default()}; @@ -782,9 +783,9 @@ impl Searcher { module.add_module_import(&here,self.ide.parser(),&import); } graph.module.update_ast(module.ast)?; - graph.module.set_node_metadata(node.id(),metadata)?; + graph.module.set_node_metadata(added_node_id,metadata)?; - Ok(node.id()) + Ok(added_node_id) } fn invalidate_fragments_added_by_picking(&self) { diff --git a/src/rust/ide/src/controller/upload.rs b/src/rust/ide/src/controller/upload.rs index 6d0e940e36..4efe47af02 100644 --- a/src/rust/ide/src/controller/upload.rs +++ b/src/rust/ide/src/controller/upload.rs @@ -197,6 +197,7 @@ impl NodeFromDroppedFileHandler { fn new_node_info(file:&FileToUpload, position:Position) -> NewNodeInfo { NewNodeInfo { expression : Self::uploading_node_expression(&file.name), + doc_comment : None, metadata : Some(Self::metadata_of_new_node(file,position)), id : None, location_hint : LocationHint::End, diff --git a/src/rust/ide/src/double_representation.rs b/src/rust/ide/src/double_representation.rs index fb4b0d6f52..48f1d0af46 100644 --- a/src/rust/ide/src/double_representation.rs +++ b/src/rust/ide/src/double_representation.rs @@ -1,7 +1,14 @@ //! A module with all functions used to synchronize different representations of our language //! module. +use crate::prelude::*; + +use ast::{Ast, opr, prefix, known}; +use crate::double_representation::definition::{ScopeKind, DefinitionName, DefinitionInfo}; +use ast::crumbs::{Located, InfixCrumb}; + pub mod alias_analysis; +pub mod comment; pub mod connection; pub mod definition; pub mod graph; @@ -16,7 +23,6 @@ pub mod tp; pub mod test_utils; - // ============== // === Consts === // ============== @@ -28,3 +34,63 @@ pub mod test_utils; /// /// Link: https://github.com/enso-org/enso/blob/main/doc/syntax/encoding.md pub const INDENT : usize = 4; + +pub enum LineKind { + Definition { + ast : known::Infix, + name : Located, + args : Vec>, + }, + ExpressionAssignment { + ast : known::Infix, + }, + ExpressionPlain { + ast : Ast, + }, +} + +pub fn discern_line +(ast:&Ast, kind:ScopeKind) -> Option { + use LineKind::*; + let infix = match opr::to_assignment(ast) { + Some(infix) => infix, + None => { + return if ast::macros::is_documentation_comment(ast) { + None + } else { + Some(ExpressionPlain {ast:ast.clone_ref()}) + } + } + }; + // There two cases - function name is either a Var or operator. + // If this is a Var, we have Var, optionally under a Prefix chain with args. + // If this is an operator, we have SectionRight with (if any prefix in arguments). + let lhs = Located::new(InfixCrumb::LeftOperand,prefix::Chain::from_ast_non_strict(&infix.larg)); + let name = lhs.entered(|chain| { + let name_ast = chain.located_func(); + name_ast.map(DefinitionName::from_ast) + }).into_opt()?; + let args = lhs.enumerate_args().map(|located_ast| { + // We already in the left side of assignment, so we need to prepend this crumb. + let left = std::iter::once(ast::crumbs::Crumb::from(InfixCrumb::LeftOperand)); + let crumbs = left.chain(located_ast.crumbs); + let ast = located_ast.item.clone(); + Located::new(crumbs,ast) + }).collect_vec(); + + // Note [Scope Differences] + if kind == ScopeKind::NonRoot { + // 1. Not an extension method but an old setter syntax. Currently not supported in the + // language, treated as node with invalid pattern. + let is_setter = !name.extended_target.is_empty(); + // 2. No explicit args -- this is a proper node, not a definition. + let is_node = args.is_empty(); + if is_setter || is_node { + return Some(ExpressionAssignment{ast:infix}) + } + }; + + Some(LineKind::Definition { + args,name,ast:infix.clone_ref() + }) +} diff --git a/src/rust/ide/src/double_representation/alias_analysis.rs b/src/rust/ide/src/double_representation/alias_analysis.rs index d35cfa27ba..d9de476907 100644 --- a/src/rust/ide/src/double_representation/alias_analysis.rs +++ b/src/rust/ide/src/double_representation/alias_analysis.rs @@ -375,7 +375,7 @@ mod tests { DEBUG!("\n===========================================================================\n"); DEBUG!("Case: " case.code); let ast = parser.parse_line(&case.code).unwrap(); - let node = NodeInfo::from_line_ast(&ast).unwrap(); + let node = NodeInfo::from_single_line_ast(&ast).unwrap(); let result = analyze_node(&node); DEBUG!("Analysis results: {result:?}"); validate_identifiers("introduced",&node, case.expected_introduced, &result.introduced); diff --git a/src/rust/ide/src/double_representation/comment.rs b/src/rust/ide/src/double_representation/comment.rs new file mode 100644 index 0000000000..3d6cf67939 --- /dev/null +++ b/src/rust/ide/src/double_representation/comment.rs @@ -0,0 +1,58 @@ +use crate::prelude::*; + +use ast::{known, MacroPatternMatch, Shifted}; +use ast::prelude::fmt::Formatter; + + +#[cfg(test)] +mod tests { + use super::*; + use ast::Shape; + use crate::double_representation::definition::DefinitionProvider; + use ast::macros::DocCommentInfo; + + + #[test] + fn parse_comment() { + let parser = parser::Parser::new_or_panic(); + let code = r#" +main = + ## Adding number is important. + Like really, really important. + sum = 2+2 + sum"#; + let ast = parser.parse_module(code,default()).unwrap(); + let main_id = double_representation::definition::Id::new_plain_name("main"); + let module_info = double_representation::module::Info {ast:ast.clone_ref()}; + + let main = double_representation::module::get_definition(&ast,&main_id).unwrap(); + let lines = main.block_lines(); + + for line in lines { + if let Some(doc) = line.elem.as_ref().and_then(ast::macros::DocCommentInfo::new) { + DEBUG!("{doc}"); + } + } + + +// assert_eq!(as_disable_comment(&ast), Some("Żar".into())); + + + + // match ast.shape() { + // Shape::Match(r#match) => { + // let first_segment = &r#match.segs.head; + // assert_eq!(ast::identifier::name(&first_segment.head), Some("#")); + // use ast::HasTokens; + // ERROR!("'{first_segment.body.repr()}'"); + // + // // first_segment.body.feed_to(&mut |ast:ast::Token| { + // // ERROR!("{ast.repr()}"); + // // }); + // //ERROR!("{first_segment.body}"); + // } + // _ => todo!(), + // } + } + +} diff --git a/src/rust/ide/src/double_representation/connection.rs b/src/rust/ide/src/double_representation/connection.rs index 00288c72f2..1bdeb7c91f 100644 --- a/src/rust/ide/src/double_representation/connection.rs +++ b/src/rust/ide/src/double_representation/connection.rs @@ -6,7 +6,7 @@ use crate::double_representation::alias_analysis::analyze_crumbable; use crate::double_representation::definition::DefinitionInfo; use crate::double_representation::definition::ScopeKind; use crate::double_representation::identifier::NormalizedName; -use crate::double_representation::node::Id; +use crate::double_representation::node::{Id, ExpressionLine}; use crate::double_representation::node::NodeInfo; use ast::crumbs::Crumb; @@ -42,7 +42,7 @@ impl Endpoint { let line_ast = block.get(&line_crumb).ok()?; let definition = DefinitionInfo::from_line_ast(&line_ast,ScopeKind::NonRoot,block.indent); let is_non_def = definition.is_none(); - let node = is_non_def.and_option_from(|| NodeInfo::from_line_ast(&line_ast))?.id(); + let node = is_non_def.and_option_from(|| ExpressionLine::from_line_ast(&line_ast))?.id(); Some(Endpoint { node, crumbs }) } } diff --git a/src/rust/ide/src/double_representation/definition.rs b/src/rust/ide/src/double_representation/definition.rs index dc52e388b7..c5d7453bdd 100644 --- a/src/rust/ide/src/double_representation/definition.rs +++ b/src/rust/ide/src/double_representation/definition.rs @@ -10,6 +10,8 @@ use ast::known; use ast::prefix; use ast::opr; use parser::Parser; +use sha3::digest::generic_array::functional::FunctionalSequence; +use crate::double_representation::{LineKind, discern_line}; // ===================== @@ -238,16 +240,25 @@ impl DefinitionInfo { Located::new(InfixCrumb::RightOperand,&self.ast.rarg) } + // pub fn body_block(&self) -> Result,Located> { + // let body = self.body(); + // if let Ok(block) = known::Block::try_from(*body) { + // Ok(body.map(|_| block)) + // } else { + // Err(body.map(CloneRef::clone_ref)) + // } + // } + /// Gets the definition block lines. If `body` is a `Block`, it returns its `BlockLine`s, /// concatenating `empty_lines`, `first_line` and `lines`, in this exact order. If `body` is /// `Infix`, it returns a single `BlockLine`. - pub fn block_lines(&self) -> FallibleResult>>> { + pub fn block_lines(&self) -> Vec>> { if let Ok(block) = known::Block::try_from(*self.body()) { - Ok(block.all_lines()) + block.all_lines() } else { let elem = Some((*self.body()).clone()); let off = 0; - Ok(vec![ast::BlockLine{elem,off}]) + vec![ast::BlockLine{elem,off}] } } @@ -305,43 +316,23 @@ impl DefinitionInfo { Self::from_line_ast(ast,ScopeKind::Root,indent) } + pub fn is_definition(ast:&Ast, kind:ScopeKind) -> bool { + // FIXME ugly as hell + // the logic for def/node recognition should be extracted beyond these two + let whatever = 0; // This does not affect the recognition itself. + Self::from_line_ast(ast,kind,whatever).is_some() + } + /// Tries to interpret `Line`'s `Ast` as a function definition. /// /// Assumes that the AST represents the contents of line (and not e.g. right-hand side of /// some binding or other kind of subtree). pub fn from_line_ast (ast:&Ast, kind:ScopeKind, context_indent:usize) -> Option { - let infix = opr::to_assignment(ast)?; - // There two cases - function name is either a Var or operator. - // If this is a Var, we have Var, optionally under a Prefix chain with args. - // If this is an operator, we have SectionRight with (if any prefix in arguments). - let lhs = Located::new(InfixCrumb::LeftOperand,prefix::Chain::from_ast_non_strict(&infix.larg)); - let name = lhs.entered(|chain| { - let name_ast = chain.located_func(); - name_ast.map(DefinitionName::from_ast) - }).into_opt()?; - let args = lhs.enumerate_args().map(|located_ast| { - // We already in the left side of assignment, so we need to prepend this crumb. - let left = std::iter::once(ast::crumbs::Crumb::from(InfixCrumb::LeftOperand)); - let crumbs = left.chain(located_ast.crumbs); - let ast = located_ast.item.clone(); - Located::new(crumbs,ast) - }).collect_vec(); - let ret = DefinitionInfo {ast:infix,name,args,context_indent}; - - // Note [Scope Differences] - if kind == ScopeKind::NonRoot { - // 1. Not an extension method but setter. - let is_setter = !ret.name.extended_target.is_empty(); - // 2. No explicit args -- this is a node, not a definition. - let is_node = ret.args.is_empty(); - if is_setter || is_node { - None - } else { - Some(ret) - } + if let Some(LineKind::Definition{args,ast,name}) = discern_line(ast,kind) { + Some(DefinitionInfo { ast, name, args, context_indent }) } else { - Some(ret) + None } } } @@ -403,7 +394,10 @@ impl<'a> DefinitionIterator<'a> { /// Looks up direct child definition by given name. pub fn find_by_name(mut self, name:&DefinitionName) -> Result { let err = || CannotFindChild(name.clone()); - self.find(|child_def| &*child_def.item.name == name).ok_or_else(err) + self.find(|child_def| { + INFO!(&*child_def.item.name); + &*child_def.item.name == name + }).ok_or_else(err) } } @@ -553,6 +547,14 @@ impl ToAdd { + +pub fn enumerate_non_empty_lines<'a> +( lines : impl IntoIterator>> + 'a +) -> impl Iterator + 'a { + lines.into_iter().enumerate().filter_map(|(index,line)| line.elem.as_ref().map(|ast| (index,ast))) +} + + // ============= // === Tests === // ============= diff --git a/src/rust/ide/src/double_representation/graph.rs b/src/rust/ide/src/double_representation/graph.rs index e89c201c1a..b48f372832 100644 --- a/src/rust/ide/src/double_representation/graph.rs +++ b/src/rust/ide/src/double_representation/graph.rs @@ -5,7 +5,7 @@ use crate::prelude::*; use crate::double_representation::definition; use crate::double_representation::definition::DefinitionInfo; use crate::double_representation::node; -use crate::double_representation::node::NodeInfo; +use crate::double_representation::node::{NodeInfo, LocatedNode}; use ast::Ast; use ast::BlockLine; @@ -14,7 +14,6 @@ use utils::fail::FallibleResult; use crate::double_representation::connection::Connection; - /// Graph uses the same `Id` as the definition which introduces the graph. pub type Id = double_representation::definition::Id; @@ -61,8 +60,13 @@ impl GraphInfo { let body = ast.rarg.clone(); if let Ok(body_block) = known::Block::try_new(body.clone()) { block_nodes(&body_block) + } else if let Some(main_line) = node::ExpressionLine::from_line_ast(ast.as_ref()) { + // There's no way to attach a documentation comment to an inline node. + let documentation = None; + vec![ NodeInfo {documentation,main_line} ] } else { - expression_node(body) + // TODO warning? + vec![] } } @@ -83,20 +87,45 @@ impl GraphInfo { } /// Adds a new node to this graph. - pub fn add_node + pub fn add_line (&mut self, line_ast:Ast, location_hint:LocationHint) -> FallibleResult { - let mut lines = self.source.block_lines()?; + let mut lines = self.source.block_lines(); let last_non_empty = || lines.iter().rposition(|line| line.elem.is_some()); let index = match location_hint { LocationHint::Start => 0, LocationHint::End => last_non_empty().map_or(lines.len(),|ix| ix + 1), - LocationHint::After(id) => node::index_in_lines(&lines, id)? + 1, - LocationHint::Before(id) => node::index_in_lines(&lines, id)? + LocationHint::After(id) => node::index_in_lines(&lines, id)?.last() + 1, + LocationHint::Before(id) => node::index_in_lines(&lines, id)?.first() }; let elem = Some(line_ast); let off = 0; lines.insert(index,BlockLine{elem,off}); self.source.set_block_lines(lines) + // FIXME unify with below? + } + + /// Adds a new node to this graph. + pub fn add_node + (&mut self, node:&NodeInfo, location_hint:LocationHint) -> FallibleResult { + let mut lines = self.source.block_lines(); + let last_non_empty = || lines.iter().rposition(|line| line.elem.is_some()); + let index = match location_hint { + LocationHint::Start => 0, + LocationHint::End => last_non_empty().map_or(lines.len(),|ix| ix + 1), + LocationHint::After(id) => node::index_in_lines(&lines, id)?.last() + 1, + LocationHint::Before(id) => node::index_in_lines(&lines, id)?.first(), + }; + let elem = Some(node.ast().clone_ref()); + let off = 0; + lines.insert(index,BlockLine{elem,off}); + if let Some(documentation) = &node.documentation { + let line = BlockLine { + elem : Some(documentation.ast().into()), + off, + }; + lines.insert(index,line); + } + self.source.set_block_lines(lines) } /// Locates a node with the given id. @@ -124,27 +153,58 @@ impl GraphInfo { /// Sets a new state for the node. The id of the described node must denote already existing /// node. pub fn update_node(&mut self, id:ast::Id, f:impl FnOnce(NodeInfo) -> Option) -> FallibleResult { - let mut lines = self.source.block_lines()?; - let node_entry = lines.iter().enumerate().find_map(|(index,line)| { - let node = NodeInfo::from_block_line(line); - let filtered = node.filter(|node| node.id() == id); - filtered.map(|node| (index,node)) - }); - if let Some((index,node_info)) = node_entry { - if let Some(updated_node) = f(node_info) { - lines[index].elem = Some(updated_node.ast().clone_ref()); - } else { - lines.remove(index); + let mut lines = self.source.block_lines(); + let LocatedNode{index,node} = node::locate_in_lines(&lines,id)?; + + if let Some(updated_node) = f(node) { + lines[index.main_line].elem = Some(updated_node.main_line.ast().clone_ref()); + match (index.documentation_line, updated_node.documentation) { + (Some(old_comment_index),None) => { + lines.remove(old_comment_index); + } + (Some(old_comment_index),Some(new_comment)) => + lines[old_comment_index].elem = Some(new_comment.as_ref().clone_ref()), + (None,Some(new_comment)) => + lines.insert(index.main_line, BlockLine { + elem : Some(new_comment.as_ref().clone_ref()), + off : 0, + }), + (None,None) => {}, } - if lines.is_empty() { - self.source.set_body_ast(Self::empty_graph_body()); - Ok(()) - } else { - self.source.set_block_lines(lines) + } else { + lines.remove(index.main_line); + if let Some(doc_index) = index.documentation_line { + lines.remove(doc_index); } + } + if lines.is_empty() { + self.source.set_body_ast(Self::empty_graph_body()); + Ok(()) } else { - Err(node::IdNotFound {id}.into()) + self.source.set_block_lines(lines) } + + + // let node_entry = lines.iter().enumerate().find_map(|(index,line)| { + // let node = NodeInfo::from_block_line(line); + // let filtered = node.filter(|node| node.id() == id); + // filtered.map(|node| (index,node)) + // }); + // if let Some((index,node_info)) = node_entry { + // if let Some(updated_node) = f(node_info) { + // lines[index].elem = Some(updated_node.ast().clone_ref()); + // } else { + // lines.remove(index); + // } + // if lines.is_empty() { + // self.source.set_body_ast(Self::empty_graph_body()); + // Ok(()) + // } else { + // self.source.set_block_lines(lines) + // } + // } else { + // Err(node::IdNotFound {id}.into()) + // } } /// Sets expression of the given node. @@ -160,6 +220,18 @@ impl GraphInfo { let code = self.source.ast.repr(); assert_eq!(code,expected_code.as_ref()); } + + // pub fn get_comments(&self) -> BTreeMap{ + // self.source.block_lines().into_iter() + // .filter_map(|opt_line| opt_line.elem) + // .tuple_windows() + // .filter_map(|(first_line, second_line)| { + // let comment_info = DocCommentInfo::new(&first_line)?; + // let node_info = NodeInfo::from_line_ast(&second_line)?; + // Some((node_info.id(),comment_info.to_string())) + // }) + // .collect() + // } } @@ -169,21 +241,12 @@ impl GraphInfo { // ===================== /// Collects information about nodes in given code `Block`. -pub fn block_nodes(ast:&known::Block) -> Vec { - ast.iter().flat_map(|line_ast| { - let kind = definition::ScopeKind::NonRoot; - let indent = ast.indent; - // If this can be a definition, then don't treat it as a node. - match definition::DefinitionInfo::from_line_ast(line_ast,kind,indent) { - None => NodeInfo::from_line_ast(line_ast), - Some(_) => None - } - }).collect() -} - -/// Collects information about nodes in given trivial definition body. -pub fn expression_node(ast:Ast) -> Vec { - NodeInfo::new_expression(ast).into_iter().collect() +pub fn block_nodes<'a>(ast:&'a known::Block) -> Vec { + // Warning: this uses faux indices, as the empty lines are skipped first. + // It is fine here, since we throw away the result values depending on index. + let lines_iter = ast.iter().enumerate(); + let nodes_iter = node::NodeIterator {lines_iter:lines_iter}; + nodes_iter.map(|n| n.node).collect() } @@ -204,6 +267,7 @@ mod tests { use ast::test_utils::expect_single_line; use utils::test::ExpectTuple; use wasm_bindgen_test::wasm_bindgen_test; + use ast::macros::DocCommentInfo; wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser); @@ -245,11 +309,26 @@ mod tests { } } - fn create_node_ast(parser:&parser::Parser, expression:&str) -> (Ast,ast::Id) { + fn new_expression_node(parser:&parser::Parser, expression:&str) -> NodeInfo { let node_ast = parser.parse(expression.to_string(), default()).unwrap(); let line_ast = expect_single_line(&node_ast).clone(); - let id = line_ast.id.expect("line_ast should have an ID"); - (line_ast,id) + NodeInfo::from_single_line_ast(&line_ast).unwrap() + } + + fn assert_at(nodes:&[NodeInfo], index:usize, expected:&NodeInfo) { + assert_same(&nodes[index],expected) + } + + fn assert_all(nodes:&[NodeInfo], expected:&[NodeInfo]) { + for (left,right) in nodes.iter().zip(expected) { + assert_same(left,right) + } + } + + fn assert_same(left:&NodeInfo, right:&NodeInfo) { + assert_eq!(left.id(), right.id()); + assert_eq!(left.documentation.as_ref().map(DocCommentInfo::to_string), right.documentation.as_ref().map(DocCommentInfo::to_string)); + assert_eq!(left.main_line.repr(), right.main_line.repr()); } #[wasm_bindgen_test] @@ -263,19 +342,19 @@ mod tests { let expr0 = "a + 2"; let expr1 = "b + 3"; - let (line_ast0,id0) = create_node_ast(&parser, expr0); - let (line_ast1,id1) = create_node_ast(&parser, expr1); + let node_to_add0 = new_expression_node(&parser, expr0); + let node_to_add1 = new_expression_node(&parser, expr1); - graph.add_node(line_ast0, LocationHint::Start).unwrap(); + graph.add_node(&node_to_add0,LocationHint::Start).unwrap(); assert_eq!(graph.nodes().len(), 2); - graph.add_node(line_ast1, LocationHint::Before(graph.nodes()[0].id())).unwrap(); + graph.add_node(&node_to_add1,LocationHint::Before(graph.nodes()[0].id())).unwrap(); let nodes = graph.nodes(); assert_eq!(nodes.len(), 3); assert_eq!(nodes[0].expression().repr(), expr1); - assert_eq!(nodes[0].id(), id1); + assert_eq!(nodes[0].id(), node_to_add0.id()); assert_eq!(nodes[1].expression().repr(), expr0); - assert_eq!(nodes[1].id(), id0); + assert_eq!(nodes[1].id(), node_to_add1.id()); assert_eq!(nodes[2].expression().repr(), "print \"hello\""); } @@ -289,29 +368,31 @@ mod tests { let mut parser = parser::Parser::new_or_panic(); let mut graph = main_graph(&mut parser, program); - let (line_ast0,id0) = create_node_ast(&mut parser, "4 + 4"); - let (line_ast1,id1) = create_node_ast(&mut parser, "a + b"); - let (line_ast2,id2) = create_node_ast(&mut parser, "x * x"); - let (line_ast3,id3) = create_node_ast(&mut parser, "x / x"); - let (line_ast4,id4) = create_node_ast(&mut parser, "2 - 2"); + let node_to_add0 = new_expression_node(&mut parser, "4 + 4"); + let node_to_add1 = new_expression_node(&mut parser, "a + b"); + let node_to_add2 = new_expression_node(&mut parser, "x * x"); + let node_to_add3 = new_expression_node(&mut parser, "x / x"); + let node_to_add4 = new_expression_node(&mut parser, "2 - 2"); - graph.add_node(line_ast0, LocationHint::Start).unwrap(); - graph.add_node(line_ast1, LocationHint::Before(graph.nodes()[0].id())).unwrap(); - graph.add_node(line_ast2, LocationHint::After(graph.nodes()[1].id())).unwrap(); - graph.add_node(line_ast3, LocationHint::End).unwrap(); + graph.add_node(&node_to_add0, LocationHint::Start).unwrap(); + graph.add_node(&node_to_add1, LocationHint::Before(graph.nodes()[0].id())).unwrap(); + graph.add_node(&node_to_add2, LocationHint::After(graph.nodes()[1].id())).unwrap(); + graph.add_node(&node_to_add3, LocationHint::End).unwrap(); + // Node 4 will be added later. let nodes = graph.nodes(); assert_eq!(nodes.len(), 6); assert_eq!(nodes[0].expression().repr(), "a + b"); - assert_eq!(nodes[0].id(), id1); + assert_eq!(nodes[0].id(), node_to_add1.id()); + // Sic: `node_to_add1` was added at index `0`. assert_eq!(nodes[1].expression().repr(), "4 + 4"); - assert_eq!(nodes[1].id(), id0); + assert_eq!(nodes[1].id(), node_to_add0.id()); assert_eq!(nodes[2].expression().repr(), "x * x"); - assert_eq!(nodes[2].id(), id2); + assert_eq!(nodes[2].id(), node_to_add2.id()); assert_eq!(nodes[3].expression().repr(), "node"); assert_eq!(nodes[4].expression().repr(), "print \"hello\""); assert_eq!(nodes[5].expression().repr(), "x / x"); - assert_eq!(nodes[5].id(), id3); + assert_eq!(nodes[5].id(), node_to_add3.id()); let expected_code = r#"main = a + b @@ -325,10 +406,10 @@ mod tests { let mut graph = find_graph(&mut parser, program, "main.foo"); assert_eq!(graph.nodes().len(), 1); - graph.add_node(line_ast4, LocationHint::Start).unwrap(); + graph.add_node(&node_to_add4, LocationHint::Start).unwrap(); assert_eq!(graph.nodes().len(), 2); assert_eq!(graph.nodes()[0].expression().repr(), "2 - 2"); - assert_eq!(graph.nodes()[0].id(), id4); + assert_eq!(graph.nodes()[0].id(), node_to_add4.id()); assert_eq!(graph.nodes()[1].expression().repr(), "not_node"); } @@ -347,15 +428,15 @@ foo = 5"; let mut graph = main_graph(&mut parser, program); let id2 = graph.nodes()[0].id(); - let (line_ast0,_id0) = create_node_ast(&mut parser, "node0"); - let (line_ast1,_id1) = create_node_ast(&mut parser, "node1"); - let (line_ast3,_id3) = create_node_ast(&mut parser, "node3"); - let (line_ast4,_id4) = create_node_ast(&mut parser, "node4"); + let node_to_add0 = new_expression_node(&mut parser, "node0"); + let node_to_add1 = new_expression_node(&mut parser, "node1"); + let node_to_add3 = new_expression_node(&mut parser, "node3"); + let node_to_add4 = new_expression_node(&mut parser, "node4"); - graph.add_node(line_ast0, LocationHint::Start).unwrap(); - graph.add_node(line_ast1, LocationHint::Before(id2)).unwrap(); - graph.add_node(line_ast3, LocationHint::After(id2)).unwrap(); - graph.add_node(line_ast4, LocationHint::End).unwrap(); + graph.add_node(&node_to_add0, LocationHint::Start).unwrap(); + graph.add_node(&node_to_add1, LocationHint::Before(id2)).unwrap(); + graph.add_node(&node_to_add3, LocationHint::After(id2)).unwrap(); + graph.add_node(&node_to_add4, LocationHint::End).unwrap(); let expected_code = r"main = node0 @@ -369,24 +450,34 @@ foo = 5"; graph.expect_code(expected_code); } - #[wasm_bindgen_test] + #[test] fn multiple_node_graph() { let mut parser = parser::Parser::new_or_panic(); let program = r" main = - foo = node + ## Faux docstring + ## Docstring 0 + foo = node0 + ## Docstring 1 + # disabled node1 foo a = not_node - node + ## Docstring 2 + node2 + node3 "; // TODO [mwu] // Add case like `Int.+ a = not_node` once https://github.com/enso-org/enso/issues/565 is fixed let graph = main_graph(&mut parser, program); let nodes = graph.nodes(); - assert_eq!(nodes.len(), 2); + assert_eq!(nodes[0].documentation_text(), Some(" Docstring 0".into())); + + for node in nodes.iter() { - assert_eq!(node.expression().repr(), "node"); + DEBUG!(node.expression().repr()); + // assert_eq!(node.expression().repr(), "node"); } + assert_eq!(nodes.len(), 2); } #[wasm_bindgen_test] diff --git a/src/rust/ide/src/double_representation/node.rs b/src/rust/ide/src/double_representation/node.rs index 31dc1e574c..3940bbad8a 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -3,8 +3,13 @@ use crate::prelude::*; use ast::Ast; -use ast::crumbs::Crumbable; +use ast::crumbs::{Crumbable, Located}; use ast::known; +use std::cmp::Ordering; +use ast::macros::DocCommentInfo; +use crate::double_representation::{discern_line, LineKind}; +use crate::double_representation::definition::ScopeKind; + /// Node Id is the Ast Id attached to the node's expression. pub type Id = ast::Id; @@ -20,66 +25,263 @@ pub type Id = ast::Id; pub struct IdNotFound {pub id:Id} +#[derive(Clone,Copy,Debug,PartialEq)] +pub struct NodeIndex { + pub main_line : usize, + pub documentation_line : Option, +} + +impl PartialOrd for NodeIndex { + fn partial_cmp(&self, other: &Self) -> Option { + self.main_line.partial_cmp(&other.main_line) + } +} + +impl NodeIndex { + pub fn first(&self) -> usize { + self.documentation_line.unwrap_or(self.main_line) + } + + pub fn last(&self) -> usize { + self.main_line + } + + pub fn range(start:NodeIndex, last:NodeIndex) -> RangeInclusive { + start.first() ..= last.last() + } +} + + // =============== // === General === // =============== + +#[derive(Clone,Debug,Shrinkwrap)] +pub struct LocatedNode { + pub index : NodeIndex, + #[shrinkwrap(main_field)] + pub node : NodeInfo, +} + /// Tests if given line contents can be seen as node with a given id -pub fn is_node_by_id(line:&ast::BlockLine>, id:ast::Id) -> bool { - let node_info = NodeInfo::from_block_line(line); +pub fn contains_node_expression(line:&ast::BlockLine>, id:ast::Id) -> bool { + let node_info = ExpressionLine::from_block_line(line); node_info.contains_if(|node| node.id() == id) } /// Searches for `NodeInfo` with the associated `id` index in `lines`. Returns an error if /// the Id is not found. -pub fn index_in_lines(lines:&[ast::BlockLine>], id:ast::Id) -> FallibleResult { - let position = lines.iter().position(|line| is_node_by_id(&line,id)); - position.ok_or_else(|| IdNotFound{id}.into()) +pub fn index_in_lines(lines:&[ast::BlockLine>], id:ast::Id) -> FallibleResult { + locate_in_lines(lines,id).map(|LocatedNode{index,..}| index) +} + +/// Searches for `NodeInfo` with the associated `id` index in `lines`. Returns an error if +/// the Id is not found. +pub fn locate_in_lines(lines:&[ast::BlockLine>], id:ast::Id) -> FallibleResult { + Ok(locate_many_in_lines(lines,[id])?.remove(&id).unwrap()) } +/// Searches for `NodeInfo` with the associated `id` index in `lines` +/// . Returns an error if +/// the Id is not found. +pub fn locate_many_in_lines + (lines : &[ast::BlockLine>] + , looked_for: impl IntoIterator + ) -> FallibleResult> { + let lines = lines.into_iter(); + let lines_iter = double_representation::definition::enumerate_non_empty_lines(lines); + let mut looked_for : HashSet<_> = looked_for.into_iter().collect(); + + let mut ret = HashMap::new(); + let nodes = NodeIterator {lines_iter}; + for node in nodes { + if looked_for.remove(&node.id()) { + ret.insert(node.id(), node); + } + + if looked_for.is_empty() { + break + } + }; + + if let Some(id) = looked_for.into_iter().next() { + Err(IdNotFound{id}.into()) + } else { + Ok(ret) + } + +} // ================ // === NodeInfo === // ================ + + +pub struct NodeIterator<'a, T:Iterator + 'a> { + pub lines_iter : T +} + +impl<'a, T:Iterator + 'a> Iterator for NodeIterator<'a, T> { + type Item = LocatedNode; + + fn next(&mut self) -> Option { + let mut documentation = None; + while let Some((index,line)) = self.lines_iter.next() { + if let Some(documentation_info) = DocCommentInfo::new(line) { + documentation = Some((index,documentation_info)); + } /* else if ast::macros::is_disable_comment(line) { + // Do nothing. + } */ else if let Some(main_line) = ExpressionLine::from_line_ast(line) { + let (documentation_line,documentation) = match documentation { + Some((index,documentation)) => (Some(index),Some(documentation)), + None => (None,None) + }; + + let ret = LocatedNode { + node : NodeInfo {documentation,main_line}, + index : NodeIndex { + main_line : index, + documentation_line, + } + }; + return Some(ret); + } else { + // Non-node entity consumes any previous documentation. + documentation = None; + } + } + None + } +} + +#[derive(Clone,Debug,Shrinkwrap)] +#[shrinkwrap(mutable)] +pub struct NodeInfo { + /// Primary node AST that contains node's expression and optional pattern binding. + #[shrinkwrap(main_field)] + pub main_line: ExpressionLine, + /// If the node has doc comment attached, it will be represented here. + pub documentation: Option, +} + +impl NodeInfo { + // pub fn iter<'a, IntoLinesIter>(lines:IntoLinesIter) -> NodeIterator<'a, IntoLinesIter::IntoIter> + // where IntoLinesIter:IntoIterator { + // NodeIterator { lines_iter: lines.into_iter().enumerate() }.map(|n| n.node) + // } + + pub fn from_lines<'a>(lines:&mut impl Iterator) -> Option { + let mut documentation = None; + while let Some(line) = lines.next() { + if let Some(documentation_info) = DocCommentInfo::new(line) { + documentation = Some(documentation_info); + } else if ast::macros::is_disable_comment(line) { + // Do nothing. + } else if let Some(main_line) = ExpressionLine::from_line_ast(line) { + return Some(NodeInfo {documentation,main_line}); + } else { + // Non-node entity consumes any previous documentation. + documentation = None; + } + } + None + } + + pub fn contains_line(&self, line_ast:&Ast) -> bool { + let expression_id_matches = || ExpressionLine::from_line_ast(line_ast) + .as_ref() + .map(ExpressionLine::id) + .contains(&self.id()); + let doc_comment_id_matches = || match (self.doc_comment_id(), line_ast.id) { + (Some(node_doc_id),Some(line_ast_id)) => node_doc_id == line_ast_id, + _ => false, + }; + expression_id_matches() || doc_comment_id_matches() + } + + pub fn doc_comment_id(&self) -> Option { + self.documentation.as_ref().and_then(|comment| comment.ast().id()) + } + + pub fn from_single_line_ast(ast:&Ast) -> Option { + ExpressionLine::from_line_ast(ast).map(|ast| Self { main_line: ast, documentation:None}) + } + + pub fn nodes_from_lines<'a>(lines:impl IntoIterator) -> Vec { + let mut ret = Vec::new(); + + let mut lines = lines.into_iter(); + while let Some(line) = lines.next() { + // Node is either: + // * documentation comment line followed by the node expression, + // * node expression line. + if let Some(doc_comment) = DocCommentInfo::new(line) { + if let Some(node) = lines.next().and_then(ExpressionLine::from_line_ast) { + ret.push(NodeInfo { + main_line: node, + documentation: Some(doc_comment), + }) + } else { + // Dangling documentation comments never apply to the nodes, so we ignore them. + } + } else if let Some(node) = ExpressionLine::from_line_ast(line) { + ret.push(NodeInfo { + main_line: node, + documentation: None, + }) + } else { + WARNING!("Line '{line}' is neither a doc comment nor a node.") + } + } + ret + } + + pub fn documentation_text(&self) -> Option { + self.documentation.as_ref().map(|doc| doc.text()) + } +} + /// Description of the node that consists of all information locally available about node. /// Nodes are required to bear IDs. This enum should never contain an ast of node without id set. #[derive(Clone,Debug)] #[allow(missing_docs)] -pub enum NodeInfo { +pub enum ExpressionLine { /// Code with assignment, e.g. `foo = 2 + 2` Binding { infix: known::Infix }, /// Code without assignment (no variable binding), e.g. `2 + 2`. Expression { ast: Ast }, } -impl NodeInfo { +impl ExpressionLine { /// Tries to interpret the whole binding as a node. Right-hand side will become node's /// expression. - pub fn new_binding(infix:known::Infix) -> Option { + pub fn new_binding(infix:known::Infix) -> Option { infix.rarg.id?; - Some(NodeInfo::Binding {infix}) + Some(ExpressionLine::Binding {infix}) } /// Tries to interpret AST as node, treating whole AST as an expression. - pub fn new_expression(ast:Ast) -> Option { + pub fn new_expression(ast:Ast) -> Option { ast.id?; - Some(NodeInfo::Expression {ast}) + // TODO what if we are given an assignment. + Some(ExpressionLine::Expression {ast}) } /// Tries to interpret AST as node, treating whole AST as an expression. - pub fn from_line_ast(ast:&Ast) -> Option { - if let Some(infix) = ast::opr::to_assignment(ast) { - Self::new_binding(infix) - } else { - Self::new_expression(ast.clone()) + pub fn from_line_ast(ast:&Ast) -> Option { + match discern_line(ast, ScopeKind::NonRoot) { + Some(LineKind::ExpressionPlain{ast}) => Self::new_expression(ast), + Some(LineKind::ExpressionAssignment {ast}) => Self::new_binding(ast), + Some(LineKind::Definition {..}) | None => None, } } /// Tries to interpret AST as node, treating whole AST as an expression. - pub fn from_block_line(line:&ast::BlockLine>) -> Option { + pub fn from_block_line(line:&ast::BlockLine>) -> Option { Self::from_line_ast(line.elem.as_ref()?) } @@ -93,13 +295,13 @@ impl NodeInfo { /// Updates the node's AST so the node bears the given ID. pub fn set_id(&mut self, new_id:Id) { match self { - NodeInfo::Binding{ref mut infix} => { + ExpressionLine::Binding{ref mut infix} => { let new_rarg = infix.rarg.with_id(new_id); let set = infix.set(&ast::crumbs::InfixCrumb::RightOperand.into(),new_rarg); *infix = set.expect("Internal error: setting infix operand should always \ succeed."); } - NodeInfo::Expression{ref mut ast} => { + ExpressionLine::Expression{ref mut ast} => { *ast = ast.with_id(new_id); } }; @@ -108,16 +310,16 @@ impl NodeInfo { /// AST of the node's expression. pub fn expression(&self) -> &Ast { match self { - NodeInfo::Binding {infix} => &infix.rarg, - NodeInfo::Expression{ast} => &ast, + ExpressionLine::Binding {infix} => &infix.rarg, + ExpressionLine::Expression{ast} => &ast, } } /// AST of the node's pattern (assignment's left-hand side). pub fn pattern(&self) -> Option<&Ast> { match self { - NodeInfo::Binding {infix} => Some(&infix.larg), - NodeInfo::Expression{..} => None, + ExpressionLine::Binding {infix} => Some(&infix.larg), + ExpressionLine::Expression{..} => None, } } @@ -125,9 +327,9 @@ impl NodeInfo { pub fn set_expression(&mut self, expression:Ast) { let id = self.id(); match self { - NodeInfo::Binding{ref mut infix} => + ExpressionLine::Binding{ref mut infix} => infix.update_shape(|infix| infix.rarg = expression), - NodeInfo::Expression{ref mut ast} => *ast = expression, + ExpressionLine::Expression{ref mut ast} => *ast = expression, }; // Id might have been overwritten by the AST we have set. Now we restore it. self.set_id(id); @@ -136,8 +338,8 @@ impl NodeInfo { /// The whole AST of node. pub fn ast(&self) -> &Ast { match self { - NodeInfo::Binding {infix} => infix.into(), - NodeInfo::Expression{ast} => ast, + ExpressionLine::Binding {infix} => infix.into(), + ExpressionLine::Expression{ast} => ast, } } @@ -145,11 +347,11 @@ impl NodeInfo { /// assignment infix will be introduced. pub fn set_pattern(&mut self, pattern:Ast) { match self { - NodeInfo::Binding {infix} => { + ExpressionLine::Binding {infix} => { // Setting infix operand never fails. infix.update_shape(|infix| infix.larg = pattern) } - NodeInfo::Expression {ast} => { + ExpressionLine::Expression {ast} => { let infix = ast::Infix { larg : pattern, loff : 1, @@ -158,7 +360,7 @@ impl NodeInfo { rarg : ast.clone(), }; let infix = known::Infix::new(infix, None); - *self = NodeInfo::Binding {infix}; + *self = ExpressionLine::Binding {infix}; } } @@ -169,16 +371,16 @@ impl NodeInfo { /// If it is already an Expression node, no change is done. pub fn clear_pattern(&mut self) { match self { - NodeInfo::Binding {infix} => { - *self = NodeInfo::Expression {ast:infix.rarg.clone_ref()} + ExpressionLine::Binding {infix} => { + *self = ExpressionLine::Expression {ast:infix.rarg.clone_ref()} } - NodeInfo::Expression {..} => {} + ExpressionLine::Expression {..} => {} } } } -impl ast::HasTokens for NodeInfo { +impl ast::HasTokens for ExpressionLine { fn feed_to(&self, consumer:&mut impl ast::TokenConsumer) { self.ast().feed_to(consumer) } @@ -195,106 +397,106 @@ mod tests { use super::*; use ast::opr::predefined::ASSIGNMENT; - - fn expect_node(ast:Ast, expression_text:&str, id:Id) { - let node_info = NodeInfo::from_line_ast(&ast).expect("expected a node"); - assert_eq!(node_info.expression().repr(),expression_text); - assert_eq!(node_info.id(), id); - } - - #[test] - fn expression_node_test() { - // expression: `4` - let id = Id::new_v4(); - let ast = Ast::new(ast::Number { base:None, int: "4".into()}, Some(id)); - expect_node(ast,"4",id); - } - - #[test] - fn set_expression_binding() { - let ast = Ast::infix(Ast::var("foo"),"=",Ast::number(4).with_new_id()); - assert_eq!(ast.repr(), "foo = 4"); - - let mut node = NodeInfo::from_line_ast(&ast).expect("expected a node"); - let id = node.id(); - node.set_expression(Ast::var("bar")); - assert_eq!(node.expression().repr(), "bar"); - assert_eq!(node.ast().repr(), "foo = bar"); - assert_eq!(node.id(), id); - } - - #[test] - fn set_expression_plain() { - let ast = Ast::number(4).with_new_id(); - assert_eq!(ast.repr(), "4"); - - let mut node = NodeInfo::from_line_ast(&ast).expect("expected a node"); - let id = node.id(); - node.set_expression(Ast::var("bar")); - assert_eq!(node.expression().repr(), "bar"); - assert_eq!(node.ast().repr(), "bar"); - assert_eq!(node.id(), id); - } - - #[test] - fn binding_node_test() { - // expression: `foo = 4` - let id = Id::new_v4(); - let number = ast::Number { base:None, int: "4".into()}; - let larg = Ast::var("foo"); - let rarg = Ast::new(number, Some(id)); - let ast = Ast::infix(larg,ASSIGNMENT,rarg); - expect_node(ast,"4",id); - } - - #[test] - fn clearing_pattern_test() { - // expression: `foo = 4` - let id = Id::new_v4(); - let number = ast::Number { base:None, int: "4".into()}; - let larg = Ast::var("foo"); - let rarg = Ast::new(number, Some(id)); - let ast = Ast::infix(larg,ASSIGNMENT,rarg); - - let mut node = NodeInfo::from_line_ast(&ast).unwrap(); - assert_eq!(node.repr(),"foo = 4"); - assert_eq!(node.id(),id); - node.clear_pattern(); - assert_eq!(node.repr(),"4"); - assert_eq!(node.id(),id); - node.clear_pattern(); - assert_eq!(node.repr(),"4"); - assert_eq!(node.id(),id); - } - - #[test] - fn setting_pattern_on_expression_node_test() { - let id = uuid::Uuid::new_v4(); - let line_ast = Ast::number(2).with_id(id); - let mut node = NodeInfo::from_line_ast(&line_ast).unwrap(); - assert_eq!(node.repr(), "2"); - assert_eq!(node.id(),id); - - node.set_pattern(Ast::var("foo")); - - assert_eq!(node.repr(), "foo = 2"); - assert_eq!(node.id(),id); - } - - #[test] - fn setting_pattern_on_binding_node_test() { - let id = uuid::Uuid::new_v4(); - let larg = Ast::var("foo"); - let rarg = Ast::var("bar").with_id(id); - let line_ast = Ast::infix(larg,ASSIGNMENT,rarg); - let mut node = NodeInfo::from_line_ast(&line_ast).unwrap(); - - assert_eq!(node.repr(), "foo = bar"); - assert_eq!(node.id(),id); - - node.set_pattern(Ast::var("baz")); - - assert_eq!(node.repr(), "baz = bar"); - assert_eq!(node.id(),id); - } + // + // fn expect_node(ast:Ast, expression_text:&str, id:Id) { + // let node_info = NodeInfo::from_line_ast(&ast).expect("expected a node"); + // assert_eq!(node_info.expression().repr(),expression_text); + // assert_eq!(node_info.id(), id); + // } + // + // #[test] + // fn expression_node_test() { + // // expression: `4` + // let id = Id::new_v4(); + // let ast = Ast::new(ast::Number { base:None, int: "4".into()}, Some(id)); + // expect_node(ast,"4",id); + // } + // + // #[test] + // fn set_expression_binding() { + // let ast = Ast::infix(Ast::var("foo"),"=",Ast::number(4).with_new_id()); + // assert_eq!(ast.repr(), "foo = 4"); + // + // let mut node = NodeInfo::from_line_ast(&ast).expect("expected a node"); + // let id = node.id(); + // node.set_expression(Ast::var("bar")); + // assert_eq!(node.expression().repr(), "bar"); + // assert_eq!(node.ast().repr(), "foo = bar"); + // assert_eq!(node.id(), id); + // } + // + // #[test] + // fn set_expression_plain() { + // let ast = Ast::number(4).with_new_id(); + // assert_eq!(ast.repr(), "4"); + // + // let mut node = NodeInfo::from_line_ast(&ast).expect("expected a node"); + // let id = node.id(); + // node.set_expression(Ast::var("bar")); + // assert_eq!(node.expression().repr(), "bar"); + // assert_eq!(node.ast().repr(), "bar"); + // assert_eq!(node.id(), id); + // } + // + // #[test] + // fn binding_node_test() { + // // expression: `foo = 4` + // let id = Id::new_v4(); + // let number = ast::Number { base:None, int: "4".into()}; + // let larg = Ast::var("foo"); + // let rarg = Ast::new(number, Some(id)); + // let ast = Ast::infix(larg,ASSIGNMENT,rarg); + // expect_node(ast,"4",id); + // } + // + // #[test] + // fn clearing_pattern_test() { + // // expression: `foo = 4` + // let id = Id::new_v4(); + // let number = ast::Number { base:None, int: "4".into()}; + // let larg = Ast::var("foo"); + // let rarg = Ast::new(number, Some(id)); + // let ast = Ast::infix(larg,ASSIGNMENT,rarg); + // + // let mut node = NodeInfo::from_line_ast(&ast).unwrap(); + // assert_eq!(node.repr(),"foo = 4"); + // assert_eq!(node.id(),id); + // node.clear_pattern(); + // assert_eq!(node.repr(),"4"); + // assert_eq!(node.id(),id); + // node.clear_pattern(); + // assert_eq!(node.repr(),"4"); + // assert_eq!(node.id(),id); + // } + // + // #[test] + // fn setting_pattern_on_expression_node_test() { + // let id = uuid::Uuid::new_v4(); + // let line_ast = Ast::number(2).with_id(id); + // let mut node = NodeInfo::from_line_ast(&line_ast).unwrap(); + // assert_eq!(node.repr(), "2"); + // assert_eq!(node.id(),id); + // + // node.set_pattern(Ast::var("foo")); + // + // assert_eq!(node.repr(), "foo = 2"); + // assert_eq!(node.id(),id); + // } + // + // #[test] + // fn setting_pattern_on_binding_node_test() { + // let id = uuid::Uuid::new_v4(); + // let larg = Ast::var("foo"); + // let rarg = Ast::var("bar").with_id(id); + // let line_ast = Ast::infix(larg,ASSIGNMENT,rarg); + // let mut node = NodeInfo::from_line_ast(&line_ast).unwrap(); + // + // assert_eq!(node.repr(), "foo = bar"); + // assert_eq!(node.id(),id); + // + // node.set_pattern(Ast::var("baz")); + // + // assert_eq!(node.repr(), "baz = bar"); + // assert_eq!(node.id(),id); + // } } diff --git a/src/rust/ide/src/double_representation/refactorings/collapse.rs b/src/rust/ide/src/double_representation/refactorings/collapse.rs index afe481106d..3c57ab7575 100644 --- a/src/rust/ide/src/double_representation/refactorings/collapse.rs +++ b/src/rust/ide/src/double_representation/refactorings/collapse.rs @@ -11,7 +11,7 @@ use crate::double_representation::definition::DefinitionInfo; use crate::double_representation::definition; use crate::double_representation::identifier::Identifier; use crate::double_representation::node; -use crate::double_representation::node::NodeInfo; +use crate::double_representation::node::{NodeInfo, ExpressionLine}; use crate::double_representation::graph::GraphInfo; use ast::crumbs::Located; @@ -137,7 +137,7 @@ impl GraphHelper { -> FallibleResult { let mut updated_definition = self.info.source.clone(); let mut new_lines = Vec::new(); - for line in updated_definition.block_lines()? { + for line in updated_definition.block_lines() { match line_rewriter(&line)? { LineDisposition::Keep => new_lines.push(line), LineDisposition::Remove => {}, @@ -235,9 +235,9 @@ impl Extracted { Ok(Self {inputs,output,extracted_nodes,extracted_nodes_set}) } - /// Check if the given node belongs to the selection (i.e. is extracted into a new method). - pub fn is_selected(&self, id:node::Id) -> bool { - self.extracted_nodes_set.contains(&id) + /// Check if the given line belongs to the selection (i.e. is extracted into a new method). + pub fn belongs_to_selection(&self, line_ast:&Ast) -> bool { + self.extracted_nodes.iter().any(|extracted_node| extracted_node.contains_line(line_ast)) } /// Generate AST of a line that needs to be appended to the extracted nodes' Asts. @@ -266,7 +266,7 @@ impl Extracted { // === Collapser === // ================= -/// Collapser rewrites the refactoring definition line-by-line. This enum describes action to be +/// Collapser rewrites the refactored definition line-by-line. This enum describes action to be /// taken for a given line. #[allow(missing_docs)] #[derive(Clone,Debug)] @@ -334,17 +334,21 @@ impl Collapser { pub fn rewrite_line (&self, line:&BlockLine>, extracted_definition:&definition::ToAdd) -> FallibleResult { - let node_id = match line.elem.as_ref().and_then(NodeInfo::from_line_ast) { - Some(node_info) => node_info.id(), + let ast = match line.elem.as_ref() { // We leave lines without nodes (blank lines) intact. - _ => return Ok(LineDisposition::Keep), + None => return Ok(LineDisposition::Keep), + Some(ast) => ast, }; - if !self.extracted.is_selected(node_id) { + if !self.extracted.belongs_to_selection(ast) { Ok(LineDisposition::Keep) - } else if node_id == self.replaced_node { - let expression = self.call_to_extracted(&extracted_definition)?; - let no_node_err = failure::Error::from(CannotConstructCollapsedNode); - let mut new_node = NodeInfo::new_expression(expression.clone_ref()).ok_or(no_node_err)?; + } else if ExpressionLine::from_line_ast(ast).contains_if(|n| n.id() == self.replaced_node) { + let no_node_err = failure::Error::from(CannotConstructCollapsedNode); + let expression_ast = self.call_to_extracted(&extracted_definition)?; + let expression = ExpressionLine::from_line_ast(&expression_ast).ok_or(no_node_err)?; + let mut new_node = NodeInfo { + documentation : None, + main_line : expression, + }; new_node.set_id(self.collapsed_node); if let Some(Output{identifier,..}) = &self.extracted.output { new_node.set_pattern(identifier.with_new_id().into()) @@ -424,7 +428,7 @@ mod tests { // isn't passing just because it got selected nodes in some specific order. // The refactoring is expected to behave the same, no matter what the order of selected // nodes is. - let mut selected_nodes = nodes[extracted_lines].iter().map(NodeInfo::id).collect_vec(); + let mut selected_nodes = nodes[extracted_lines].iter().map(|node| node.id()).collect_vec(); run_internal(&selected_nodes); selected_nodes.reverse(); run_internal(&selected_nodes); diff --git a/src/rust/ide/src/lib.rs b/src/rust/ide/src/lib.rs index 2bcfdb36f5..f0b9f814e9 100644 --- a/src/rust/ide/src/lib.rs +++ b/src/rust/ide/src/lib.rs @@ -8,6 +8,7 @@ #![feature(cell_update)] #![feature(drain_filter)] #![feature(exact_size_is_empty)] +#![feature(impl_trait_in_bindings)] #![feature(iter_order_by)] #![feature(maybe_uninit_extra)] #![feature(option_result_contains)] diff --git a/src/rust/ide/view/graph-editor/src/component/node.rs b/src/rust/ide/view/graph-editor/src/component/node.rs index 8455463e20..0c62ad12b4 100644 --- a/src/rust/ide/view/graph-editor/src/component/node.rs +++ b/src/rust/ide/view/graph-editor/src/component/node.rs @@ -257,6 +257,7 @@ ensogl::define_endpoints! { set_visualization (Option), set_disabled (bool), set_input_connected (span_tree::Crumbs,Option,bool), + set_comment (String), set_expression (Expression), set_error (Option), /// Set the expression USAGE type. This is not the definition type, which can be set with @@ -278,6 +279,7 @@ ensogl::define_endpoints! { /// Press event. Emitted when user clicks on non-active part of the node, like its /// background. In edit mode, the whole node area is considered non-active. background_press (), + comment (String), expression (Text), skip (bool), freeze (bool), @@ -384,6 +386,7 @@ pub struct NodeModel { pub action_bar : action_bar::ActionBar, pub vcs_indicator : vcs::StatusIndicator, pub style : StyleWatchFrp, + pub comment : ensogl_text::Area, } impl NodeModel { @@ -457,10 +460,15 @@ impl NodeModel { let style = StyleWatchFrp::new(&app.display.scene().style_sheet); + let comment = ensogl_text::Area::new(app); + comment.set_position_y(-35.0); + display_object.add_child(&comment); + comment.set_content("Place for your comment. Lorem ipsum dolor sit amet"); + let app = app.clone_ref(); Self {app,display_object,logger,backdrop,background,drag_area,error_indicator ,profiling_label,input,output,visualization,error_visualization,action_bar - ,vcs_indicator,style}.init() + ,vcs_indicator,style,comment}.init() } pub fn get_crumbs_by_id(&self, id:ast::Id) -> Option { From 04f2c5b8f5446fd5d0729d051ff4659717c8fd60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Fri, 30 Jul 2021 12:33:20 +0200 Subject: [PATCH 02/19] [wip] --- src/rust/ide/lib/ast/impl/src/macros.rs | 33 +- src/rust/ide/src/controller/graph.rs | 13 +- src/rust/ide/src/double_representation.rs | 60 ++-- .../double_representation/alias_analysis.rs | 21 +- .../alias_analysis/test_utils.rs | 9 +- .../ide/src/double_representation/comment.rs | 43 +-- .../src/double_representation/connection.rs | 4 +- .../src/double_representation/definition.rs | 18 -- .../ide/src/double_representation/graph.rs | 69 ++-- .../ide/src/double_representation/node.rs | 304 +++++++++--------- src/rust/ide/src/lib.rs | 1 - .../view/graph-editor/src/component/node.rs | 22 +- src/rust/ide/view/graph-editor/src/lib.rs | 21 +- .../ide/view/src/debug_scenes/interface.rs | 3 + 14 files changed, 310 insertions(+), 311 deletions(-) diff --git a/src/rust/ide/lib/ast/impl/src/macros.rs b/src/rust/ide/lib/ast/impl/src/macros.rs index d25a61fda1..552399c44f 100644 --- a/src/rust/ide/lib/ast/impl/src/macros.rs +++ b/src/rust/ide/lib/ast/impl/src/macros.rs @@ -16,8 +16,10 @@ use crate::Shifted; // === Recognized Macros Keywords === // ================================== +/// The keyword introducing a disabled code line. pub const DISABLING_COMMENT_INTRODUCER:&str = "#"; +/// The keyword introducing a documentation block. pub const DOCUMENTATION_COMMENT_INTRODUCER:&str = "##"; /// The keyword introducing an qualified import declaration. See: @@ -38,6 +40,7 @@ pub const QUALIFIED_EXPORT_KEYWORD:&str = "export"; // === Disable Comments === +/// Try Interpreting the line as disabling comment. Return the text after `#`. pub fn as_disable_comment(ast:&Ast) -> Option { let r#match = crate::known::Match::try_from(ast).ok()?; let first_segment = &r#match.segs.head; @@ -48,6 +51,7 @@ pub fn as_disable_comment(ast:&Ast) -> Option { } } +/// Check if this AST is a disabling comment. pub fn is_disable_comment(ast:&Ast) -> bool { as_disable_comment(ast).is_some() } @@ -55,31 +59,47 @@ pub fn is_disable_comment(ast:&Ast) -> bool { // === Documentation Comments === +/// Ast known to be a documentation comment. #[derive(Clone,Debug)] pub struct DocCommentInfo { - // Private due to invariants ensured by the constructor. - ast : known::Match, - body : crate::MacroPatternMatch>, + ast : known::Match, + body : crate::MacroPatternMatch>, + /// The absolute indent of the block that contains the line with documentation comment. + pub block_indent : usize, } impl DocCommentInfo { + /// Try constructing from AST, return None if this is not a documentation comment. pub fn new(ast:&Ast) -> Option { + Self::new_indented(ast,0) + } + + pub fn new_indented(ast:&Ast, block_indent:usize) -> Option { let ast = crate::known::Match::try_from(ast).ok()?; let first_segment = &ast.segs.head; let introducer = crate::identifier::name(&first_segment.head)?; let introducer_matches = introducer == DOCUMENTATION_COMMENT_INTRODUCER; let body = first_segment.body.clone_ref(); - introducer_matches.then(|| DocCommentInfo {ast,body}) + introducer_matches.then(|| DocCommentInfo {ast,body,block_indent}) } + /// Get the documentation comment's AST. pub fn ast(&self) -> known::Match { self.ast.clone_ref() } + /// Get the documentation text. pub fn text(&self) -> String { - self.to_string() + // This gets us documentation text, however non-first lines have indent whitespace + // maintained. + let repr = self.body.repr(); + let indent = self.block_indent + DOCUMENTATION_COMMENT_INTRODUCER.len(); + let old = format!("\n{}", " ".repeat(indent)); + let new = "\n"; + repr.replace(&old,new) } + /// Get the documentation text. pub fn pretty_print_text(text:&str) -> String { let mut lines = text.lines(); let first_line = lines.next().map(|line| iformat!("##{line}")); @@ -97,10 +117,11 @@ impl AsRef for DocCommentInfo { impl Display for DocCommentInfo { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f,"{}",self.body.repr()) + write!(f,"{}",self.text()) } } +/// Check if given Ast stores a documentation comment. pub fn is_documentation_comment(ast:&Ast) -> bool { DocCommentInfo::new(ast).is_some() } diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 70b3bccc3b..fb12477cd2 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -555,7 +555,7 @@ impl Handle { /// Analyzes the expression, e.g. result for "a+b" shall be named "sum". /// The caller should make sure that obtained name won't collide with any symbol usage before /// actually introducing it. See `variable_name_for`. - pub fn variable_name_base_for(node:&node::ExpressionLine) -> String { + pub fn variable_name_base_for(node:&ExpressionLine) -> String { name_for_ast(node.expression()) } @@ -570,8 +570,7 @@ impl Handle { let usage = if matches!(body.shape(),ast::Shape::Block(_)) { alias_analysis::analyze_crumbable(body.item) } else if let Some(node) = NodeInfo::from_single_line_ast(&body) { - // TODO reconsider this branch, test it and make sure it is correct. - alias_analysis::analyze_node(&node) + alias_analysis::analyze_ast(node.ast()) } else { // Generally speaking - impossible. But if there is no node in the definition // body, then there is nothing that could use any symbols, so nothing is used. @@ -649,9 +648,9 @@ impl Handle { let dependent_nodes = connection::dependent_nodes_in_def(definition_ast,node_to_be_after); let mut lines = definition.block_lines(); - let node_to_be_before = node::locate_in_lines(&lines,node_to_be_before)?; - let node_to_be_after = node::locate_in_lines(&lines,node_to_be_after)?; - let dependent_nodes = dependent_nodes.iter().map(|id| node::locate_in_lines(&lines,*id)) + let node_to_be_before = node::locate(&lines, node_to_be_before)?; + let node_to_be_after = node::locate(&lines, node_to_be_after)?; + let dependent_nodes = dependent_nodes.iter().map(|id| node::locate(&lines, *id)) .collect::,_>>()?; if node_to_be_after.index < node_to_be_before.index { @@ -758,7 +757,7 @@ impl Handle { .map(|doc_ast| DocCommentInfo::new(&doc_ast).ok_or(FailedToCreateNode)) .transpose()?; - let mut node_info = node::NodeInfo {main_line,documentation}; + let mut node_info = NodeInfo {main_line,documentation}; if let Some(desired_id) = node.id { node_info.set_id(desired_id) } diff --git a/src/rust/ide/src/double_representation.rs b/src/rust/ide/src/double_representation.rs index 48f1d0af46..01c9e8ef08 100644 --- a/src/rust/ide/src/double_representation.rs +++ b/src/rust/ide/src/double_representation.rs @@ -4,7 +4,8 @@ use crate::prelude::*; use ast::{Ast, opr, prefix, known}; -use crate::double_representation::definition::{ScopeKind, DefinitionName, DefinitionInfo}; +use crate::double_representation::definition::DefinitionName; +use crate::double_representation::definition::ScopeKind; use ast::crumbs::{Located, InfixCrumb}; pub mod alias_analysis; @@ -52,29 +53,39 @@ pub enum LineKind { pub fn discern_line (ast:&Ast, kind:ScopeKind) -> Option { use LineKind::*; + + // First of all, if line is not an infix assignment, it can be only node or nothing. let infix = match opr::to_assignment(ast) { Some(infix) => infix, - None => { - return if ast::macros::is_documentation_comment(ast) { - None - } else { - Some(ExpressionPlain {ast:ast.clone_ref()}) - } - } + // Documentation comment lines are not nodes. Here they are ignored. + // They are discovered and processed as part of nodes that follow them. + // e.g. `## Comment`. + None if ast::macros::is_documentation_comment(ast) => return None, + // The simplest form of node, e.g. `Point 5 10` + None => return Some(ExpressionPlain {ast:ast.clone_ref()}), }; - // There two cases - function name is either a Var or operator. - // If this is a Var, we have Var, optionally under a Prefix chain with args. - // If this is an operator, we have SectionRight with (if any prefix in arguments). - let lhs = Located::new(InfixCrumb::LeftOperand,prefix::Chain::from_ast_non_strict(&infix.larg)); - let name = lhs.entered(|chain| { + + // Assignment can be either nodes or definitions. To discern, we check the left hand side. + // For definition it is a prefix chain, where first is the name, then arguments (if explicit). + // For node it is a pattern, either in a form of Var without args on Cons application. + let crumb = InfixCrumb::LeftOperand; + let lhs = Located::new(crumb,prefix::Chain::from_ast_non_strict(&infix.larg)); + let name = lhs.entered(|chain| { let name_ast = chain.located_func(); name_ast.map(DefinitionName::from_ast) - }).into_opt()?; - let args = lhs.enumerate_args().map(|located_ast| { + }).into_opt(); + + // If this is a pattern match, `name` will fail to construct and we'll treat line as a node. + // e.g. for `Point x y = get_point …` + let name = match name { + Some(name) => name, + None => return Some(ExpressionAssignment{ast:infix}) + }; + + let args = lhs.enumerate_args().map(|Located{crumbs,item}| { // We already in the left side of assignment, so we need to prepend this crumb. - let left = std::iter::once(ast::crumbs::Crumb::from(InfixCrumb::LeftOperand)); - let crumbs = left.chain(located_ast.crumbs); - let ast = located_ast.item.clone(); + let crumbs = lhs.crumbs.clone().into_iter().chain(crumbs); + let ast = item.clone(); Located::new(crumbs,ast) }).collect_vec(); @@ -82,15 +93,26 @@ pub fn discern_line if kind == ScopeKind::NonRoot { // 1. Not an extension method but an old setter syntax. Currently not supported in the // language, treated as node with invalid pattern. + // e.g. `point.x = 5` let is_setter = !name.extended_target.is_empty(); // 2. No explicit args -- this is a proper node, not a definition. + // e.g. `point = Point 5 10` let is_node = args.is_empty(); if is_setter || is_node { return Some(ExpressionAssignment{ast:infix}) } }; - Some(LineKind::Definition { + Some(Definition { args,name,ast:infix.clone_ref() }) } + +// Note [Scope Differences] +// ======================== +// When we are in definition scope (as opposed to global scope) certain patterns should not be +// considered to be function definitions. These are: +// 1. Expressions like "Int.x = …". In module, they'd be treated as extension methods. In +// definition scope they are treated as invalid constructs (setter syntax in the old design). +// 2. Expression like "foo = 5". In module, this is treated as method definition (with implicit +// this parameter). In definition, this is just a node (evaluated expression). diff --git a/src/rust/ide/src/double_representation/alias_analysis.rs b/src/rust/ide/src/double_representation/alias_analysis.rs index d9de476907..ec885f345b 100644 --- a/src/rust/ide/src/double_representation/alias_analysis.rs +++ b/src/rust/ide/src/double_representation/alias_analysis.rs @@ -7,7 +7,6 @@ use crate::double_representation::definition::DefinitionInfo; use crate::double_representation::definition::ScopeKind; use crate::double_representation::identifier::LocatedName; use crate::double_representation::identifier::NormalizedName; -use crate::double_representation::node::NodeInfo; use ast::crumbs::Crumb; use ast::crumbs::InfixCrumb; @@ -334,11 +333,12 @@ impl AliasAnalyzer { } } -/// Describes identifiers that nodes introduces into the graph and identifiers from graph's scope -/// that node uses. This logic serves as a base for connection discovery. -pub fn analyze_node(node:&NodeInfo) -> IdentifierUsage { +/// Describes identifiers that code represented by AST introduces into the graph and identifiers +/// from graph's scope that code uses. This logic serves as a base for connection discovery, +/// where ASTs are typically the node's ASTs. +pub fn analyze_ast(ast:&Ast) -> IdentifierUsage { let mut analyzer = AliasAnalyzer::new(); - analyzer.process_ast(node.ast()); + analyzer.process_ast(ast); analyzer.root_scope.symbols } @@ -365,8 +365,8 @@ mod tests { /// Checks if actual observed sequence of located identifiers matches the expected one. /// Expected identifiers are described as code spans in the node's text representation. fn validate_identifiers - (name:impl Str, node:&NodeInfo, expected:Vec>, actual:&Vec) { - let mut checker = IdentifierValidator::new(name,node,expected); + (name:impl Str, ast:&Ast, expected:Vec>, actual:&Vec) { + let mut checker = IdentifierValidator::new(name,ast,expected); checker.validate_identifiers(actual); } @@ -375,11 +375,10 @@ mod tests { DEBUG!("\n===========================================================================\n"); DEBUG!("Case: " case.code); let ast = parser.parse_line(&case.code).unwrap(); - let node = NodeInfo::from_single_line_ast(&ast).unwrap(); - let result = analyze_node(&node); + let result = analyze_ast(&ast); DEBUG!("Analysis results: {result:?}"); - validate_identifiers("introduced",&node, case.expected_introduced, &result.introduced); - validate_identifiers("used", &node, case.expected_used, &result.used); + validate_identifiers("introduced",&ast, case.expected_introduced, &result.introduced); + validate_identifiers("used", &ast, case.expected_used, &result.used); } /// Runs the test for the test case expressed using markdown notation. See `Case` for details. diff --git a/src/rust/ide/src/double_representation/alias_analysis/test_utils.rs b/src/rust/ide/src/double_representation/alias_analysis/test_utils.rs index cd1268f1f4..78ea4f3633 100644 --- a/src/rust/ide/src/double_representation/alias_analysis/test_utils.rs +++ b/src/rust/ide/src/double_representation/alias_analysis/test_utils.rs @@ -114,24 +114,23 @@ enum HasBeenValidated {No,Yes} /// Otherwise, it shall panic when dropped. #[derive(Clone,Debug)] pub struct IdentifierValidator<'a> { + ast : &'a Ast, name : String, - node : &'a NodeInfo, validations : HashMap, } impl<'a> IdentifierValidator<'a> { /// Creates a new checker, with identifier set obtained from given node's representation /// spans. - pub fn new(name:impl Str, node:&NodeInfo,spans:Vec>) -> IdentifierValidator { + pub fn new(name:impl Str, ast:&Ast,spans:Vec>) -> IdentifierValidator { let name = name.into(); - let ast = node.ast(); let repr = ast.repr(); let mut validations = HashMap::default(); for span in spans { let name = NormalizedName::new(&repr[span]); validations.insert(name, HasBeenValidated::No); } - IdentifierValidator {name,node,validations} + IdentifierValidator {ast,name,validations} } /// Marks given identifier as checked. @@ -148,7 +147,7 @@ impl<'a> IdentifierValidator<'a> { self.validate_identifier(&identifier.item); let crumbs = &identifier.crumbs; - let ast_result = self.node.ast().get_traversing(crumbs); + let ast_result = self.ast.get_traversing(crumbs); let ast = ast_result.expect("failed to retrieve ast from crumb"); let name_err = || iformat!("Failed to use AST {ast.repr()} as an identifier name"); let name = NormalizedName::try_from_ast(ast).expect(&name_err()); diff --git a/src/rust/ide/src/double_representation/comment.rs b/src/rust/ide/src/double_representation/comment.rs index 3d6cf67939..4838ff94cf 100644 --- a/src/rust/ide/src/double_representation/comment.rs +++ b/src/rust/ide/src/double_representation/comment.rs @@ -1,9 +1,5 @@ use crate::prelude::*; -use ast::{known, MacroPatternMatch, Shifted}; -use ast::prelude::fmt::Formatter; - - #[cfg(test)] mod tests { use super::*; @@ -12,47 +8,24 @@ mod tests { use ast::macros::DocCommentInfo; + #[test] - fn parse_comment() { + fn parse_multi_line_comment() { let parser = parser::Parser::new_or_panic(); let code = r#" main = - ## Adding number is important. - Like really, really important. - sum = 2+2 - sum"#; + ## First line + Second line + node"#; let ast = parser.parse_module(code,default()).unwrap(); let main_id = double_representation::definition::Id::new_plain_name("main"); let module_info = double_representation::module::Info {ast:ast.clone_ref()}; let main = double_representation::module::get_definition(&ast,&main_id).unwrap(); let lines = main.block_lines(); - - for line in lines { - if let Some(doc) = line.elem.as_ref().and_then(ast::macros::DocCommentInfo::new) { - DEBUG!("{doc}"); - } - } - - -// assert_eq!(as_disable_comment(&ast), Some("Żar".into())); - - - - // match ast.shape() { - // Shape::Match(r#match) => { - // let first_segment = &r#match.segs.head; - // assert_eq!(ast::identifier::name(&first_segment.head), Some("#")); - // use ast::HasTokens; - // ERROR!("'{first_segment.body.repr()}'"); - // - // // first_segment.body.feed_to(&mut |ast:ast::Token| { - // // ERROR!("{ast.repr()}"); - // // }); - // //ERROR!("{first_segment.body}"); - // } - // _ => todo!(), - // } + assert_eq!(lines.len(),2); + let doc = ast::macros::DocCommentInfo::new_indented(lines[0].elem.as_ref().unwrap(),4).unwrap(); + assert_eq!(doc.text(), " First line\n Second line"); } } diff --git a/src/rust/ide/src/double_representation/connection.rs b/src/rust/ide/src/double_representation/connection.rs index 1bdeb7c91f..36984f7cd4 100644 --- a/src/rust/ide/src/double_representation/connection.rs +++ b/src/rust/ide/src/double_representation/connection.rs @@ -6,8 +6,8 @@ use crate::double_representation::alias_analysis::analyze_crumbable; use crate::double_representation::definition::DefinitionInfo; use crate::double_representation::definition::ScopeKind; use crate::double_representation::identifier::NormalizedName; -use crate::double_representation::node::{Id, ExpressionLine}; -use crate::double_representation::node::NodeInfo; +use crate::double_representation::node::ExpressionLine; +use crate::double_representation::node::Id; use ast::crumbs::Crumb; use ast::crumbs::Crumbs; diff --git a/src/rust/ide/src/double_representation/definition.rs b/src/rust/ide/src/double_representation/definition.rs index c5d7453bdd..216c7959e3 100644 --- a/src/rust/ide/src/double_representation/definition.rs +++ b/src/rust/ide/src/double_representation/definition.rs @@ -7,10 +7,8 @@ use ast::crumbs::Crumbable; use ast::crumbs::InfixCrumb; use ast::crumbs::Located; use ast::known; -use ast::prefix; use ast::opr; use parser::Parser; -use sha3::digest::generic_array::functional::FunctionalSequence; use crate::double_representation::{LineKind, discern_line}; @@ -316,13 +314,6 @@ impl DefinitionInfo { Self::from_line_ast(ast,ScopeKind::Root,indent) } - pub fn is_definition(ast:&Ast, kind:ScopeKind) -> bool { - // FIXME ugly as hell - // the logic for def/node recognition should be extracted beyond these two - let whatever = 0; // This does not affect the recognition itself. - Self::from_line_ast(ast,kind,whatever).is_some() - } - /// Tries to interpret `Line`'s `Ast` as a function definition. /// /// Assumes that the AST represents the contents of line (and not e.g. right-hand side of @@ -337,15 +328,6 @@ impl DefinitionInfo { } } -// Note [Scope Differences] -// ======================== -// When we are in definition scope (as opposed to global scope) certain patterns should not be -// considered to be function definitions. These are: -// 1. Expressions like "Int.x = …". In module, they'd be treated as extension methods. In -// definition scope they are treated as accessor setters. -// 2. Expression like "foo = 5". In module, this is treated as method definition (with implicit -// this parameter). In definition, this is just a node (evaluated expression). - /// Definition stored under some known crumbs path. pub type ChildDefinition = Located; diff --git a/src/rust/ide/src/double_representation/graph.rs b/src/rust/ide/src/double_representation/graph.rs index b48f372832..068af38aad 100644 --- a/src/rust/ide/src/double_representation/graph.rs +++ b/src/rust/ide/src/double_representation/graph.rs @@ -2,10 +2,10 @@ use crate::prelude::*; -use crate::double_representation::definition; use crate::double_representation::definition::DefinitionInfo; use crate::double_representation::node; -use crate::double_representation::node::{NodeInfo, LocatedNode}; +use crate::double_representation::node::LocatedNode; +use crate::double_representation::node::NodeInfo; use ast::Ast; use ast::BlockLine; @@ -60,12 +60,14 @@ impl GraphInfo { let body = ast.rarg.clone(); if let Ok(body_block) = known::Block::try_new(body.clone()) { block_nodes(&body_block) - } else if let Some(main_line) = node::ExpressionLine::from_line_ast(ast.as_ref()) { + } else if let Some(main_line) = node::ExpressionLine::from_line_ast(&body) { // There's no way to attach a documentation comment to an inline node. let documentation = None; vec![ NodeInfo {documentation,main_line} ] } else { // TODO warning? + // should not be possible to have empty definition without any nodes but it is + // possible to represent such in AST vec![] } } @@ -94,8 +96,8 @@ impl GraphInfo { let index = match location_hint { LocationHint::Start => 0, LocationHint::End => last_non_empty().map_or(lines.len(),|ix| ix + 1), - LocationHint::After(id) => node::index_in_lines(&lines, id)?.last() + 1, - LocationHint::Before(id) => node::index_in_lines(&lines, id)?.first() + LocationHint::After(id) => node::locate(&lines, id)?.index.last() + 1, + LocationHint::Before(id) => node::locate(&lines, id)?.index.first() }; let elem = Some(line_ast); let off = 0; @@ -112,8 +114,8 @@ impl GraphInfo { let index = match location_hint { LocationHint::Start => 0, LocationHint::End => last_non_empty().map_or(lines.len(),|ix| ix + 1), - LocationHint::After(id) => node::index_in_lines(&lines, id)?.last() + 1, - LocationHint::Before(id) => node::index_in_lines(&lines, id)?.first(), + LocationHint::After(id) => node::locate(&lines, id)?.index.last() + 1, + LocationHint::Before(id) => node::locate(&lines, id)?.index.first(), }; let elem = Some(node.ast().clone_ref()); let off = 0; @@ -154,7 +156,7 @@ impl GraphInfo { /// node. pub fn update_node(&mut self, id:ast::Id, f:impl FnOnce(NodeInfo) -> Option) -> FallibleResult { let mut lines = self.source.block_lines(); - let LocatedNode{index,node} = node::locate_in_lines(&lines,id)?; + let LocatedNode{index,node} = node::locate(&lines, id)?; if let Some(updated_node) = f(node) { lines[index.main_line].elem = Some(updated_node.main_line.ast().clone_ref()); @@ -184,27 +186,7 @@ impl GraphInfo { self.source.set_block_lines(lines) } - - // let node_entry = lines.iter().enumerate().find_map(|(index,line)| { - // let node = NodeInfo::from_block_line(line); - // let filtered = node.filter(|node| node.id() == id); - // filtered.map(|node| (index,node)) - // }); - // if let Some((index,node_info)) = node_entry { - // if let Some(updated_node) = f(node_info) { - // lines[index].elem = Some(updated_node.ast().clone_ref()); - // } else { - // lines.remove(index); - // } - // if lines.is_empty() { - // self.source.set_body_ast(Self::empty_graph_body()); - // Ok(()) - // } else { - // self.source.set_block_lines(lines) - // } - // } else { - // Err(node::IdNotFound {id}.into()) - // } + // TODO tests for cases with comments involved } /// Sets expression of the given node. @@ -220,18 +202,6 @@ impl GraphInfo { let code = self.source.ast.repr(); assert_eq!(code,expected_code.as_ref()); } - - // pub fn get_comments(&self) -> BTreeMap{ - // self.source.block_lines().into_iter() - // .filter_map(|opt_line| opt_line.elem) - // .tuple_windows() - // .filter_map(|(first_line, second_line)| { - // let comment_info = DocCommentInfo::new(&first_line)?; - // let node_info = NodeInfo::from_line_ast(&second_line)?; - // Some((node_info.id(),comment_info.to_string())) - // }) - // .collect() - // } } @@ -245,7 +215,7 @@ pub fn block_nodes<'a>(ast:&'a known::Block) -> Vec { // Warning: this uses faux indices, as the empty lines are skipped first. // It is fine here, since we throw away the result values depending on index. let lines_iter = ast.iter().enumerate(); - let nodes_iter = node::NodeIterator {lines_iter:lines_iter}; + let nodes_iter = node::NodeIterator {lines_iter}; nodes_iter.map(|n| n.node).collect() } @@ -352,9 +322,9 @@ mod tests { let nodes = graph.nodes(); assert_eq!(nodes.len(), 3); assert_eq!(nodes[0].expression().repr(), expr1); - assert_eq!(nodes[0].id(), node_to_add0.id()); + assert_eq!(nodes[0].id(), node_to_add1.id()); assert_eq!(nodes[1].expression().repr(), expr0); - assert_eq!(nodes[1].id(), node_to_add1.id()); + assert_eq!(nodes[1].id(), node_to_add0.id()); assert_eq!(nodes[2].expression().repr(), "print \"hello\""); } @@ -450,7 +420,7 @@ foo = 5"; graph.expect_code(expected_code); } - #[test] + #[wasm_bindgen_test] fn multiple_node_graph() { let mut parser = parser::Parser::new_or_panic(); let program = r" @@ -471,13 +441,20 @@ main = let graph = main_graph(&mut parser, program); let nodes = graph.nodes(); assert_eq!(nodes[0].documentation_text(), Some(" Docstring 0".into())); + assert_eq!(nodes[0].ast().repr(), "foo = node0"); + assert_eq!(nodes[1].documentation_text(), Some(" Docstring 1".into())); + assert_eq!(nodes[1].ast().repr(), "# disabled node1"); + assert_eq!(nodes[2].documentation_text(), Some(" Docstring 2".into())); + assert_eq!(nodes[2].ast().repr(), "node2"); + assert_eq!(nodes[3].documentation_text(), None); + assert_eq!(nodes[3].ast().repr(), "node3"); for node in nodes.iter() { DEBUG!(node.expression().repr()); // assert_eq!(node.expression().repr(), "node"); } - assert_eq!(nodes.len(), 2); + assert_eq!(nodes.len(), 4); } #[wasm_bindgen_test] diff --git a/src/rust/ide/src/double_representation/node.rs b/src/rust/ide/src/double_representation/node.rs index 3940bbad8a..c114ef3388 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -3,7 +3,7 @@ use crate::prelude::*; use ast::Ast; -use ast::crumbs::{Crumbable, Located}; +use ast::crumbs::Crumbable; use ast::known; use std::cmp::Ordering; use ast::macros::DocCommentInfo; @@ -24,11 +24,13 @@ pub type Id = ast::Id; #[fail(display="Node with ID {} was not found.", id)] pub struct IdNotFound {pub id:Id} - +/// Indices of lines belonging to a node. #[derive(Clone,Copy,Debug,PartialEq)] pub struct NodeIndex { - pub main_line : usize, + /// Documentation comment line index, if present. pub documentation_line : Option, + /// Main line is a line that contains the node's expression. + pub main_line : usize, } impl PartialOrd for NodeIndex { @@ -38,14 +40,20 @@ impl PartialOrd for NodeIndex { } impl NodeIndex { + /// Index for the first line belonging to the node. pub fn first(&self) -> usize { self.documentation_line.unwrap_or(self.main_line) } + /// Index for the last line belonging to the node. pub fn last(&self) -> usize { self.main_line } + /// Inclusive range between first and last node's lines. + /// + /// Note that while a node can contain at most two lines, they may be interspersed by a + /// number of blank lines. pub fn range(start:NodeIndex, last:NodeIndex) -> RangeInclusive { start.first() ..= last.last() } @@ -57,42 +65,42 @@ impl NodeIndex { // === General === // =============== - +/// Information about the node coupled with its location within a block. #[derive(Clone,Debug,Shrinkwrap)] pub struct LocatedNode { + /// Line index in the block. Zero for inline definition nodes. pub index : NodeIndex, #[shrinkwrap(main_field)] + /// Information about the node. pub node : NodeInfo, } /// Tests if given line contents can be seen as node with a given id -pub fn contains_node_expression(line:&ast::BlockLine>, id:ast::Id) -> bool { +pub fn is_main_line_of(line:&ast::BlockLine>, id:Id) -> bool { let node_info = ExpressionLine::from_block_line(line); node_info.contains_if(|node| node.id() == id) } -/// Searches for `NodeInfo` with the associated `id` index in `lines`. Returns an error if -/// the Id is not found. -pub fn index_in_lines(lines:&[ast::BlockLine>], id:ast::Id) -> FallibleResult { - locate_in_lines(lines,id).map(|LocatedNode{index,..}| index) +/// Searches for `NodeInfo` with the associated `id` index in `lines`. +/// +/// Returns an error if the Id is not found. +pub fn locate<'a> +( lines : impl IntoIterator>> + 'a +, id : Id +) -> FallibleResult { + Ok(locate_many(lines, [id])?.remove(&id).unwrap()) } -/// Searches for `NodeInfo` with the associated `id` index in `lines`. Returns an error if -/// the Id is not found. -pub fn locate_in_lines(lines:&[ast::BlockLine>], id:ast::Id) -> FallibleResult { - Ok(locate_many_in_lines(lines,[id])?.remove(&id).unwrap()) -} - -/// Searches for `NodeInfo` with the associated `id` index in `lines` -/// . Returns an error if -/// the Id is not found. -pub fn locate_many_in_lines - (lines : &[ast::BlockLine>] - , looked_for: impl IntoIterator - ) -> FallibleResult> { - let lines = lines.into_iter(); +/// Obtain located node information for multiple nodes in a single pass. +/// +/// If any of the looked for nodes is not found, `Err` is returned. +/// Any `Ok(…)` return value is guaranteed to have length equal to `looked_for` argument. +pub fn locate_many<'a> +( lines : impl IntoIterator>> + 'a +, looked_for : impl IntoIterator +) -> FallibleResult> { let lines_iter = double_representation::definition::enumerate_non_empty_lines(lines); - let mut looked_for : HashSet<_> = looked_for.into_iter().collect(); + let mut looked_for = looked_for.into_iter().collect::>(); let mut ret = HashMap::new(); let nodes = NodeIterator {lines_iter}; @@ -115,13 +123,15 @@ pub fn locate_many_in_lines } + // ================ // === NodeInfo === // ================ - - +/// Iterator over indexed line ASTs that yields nodes. +#[derive(Clone,Debug)] pub struct NodeIterator<'a, T:Iterator + 'a> { + /// Input iterator that yields pairs (line index, line's Ast). pub lines_iter : T } @@ -130,19 +140,17 @@ impl<'a, T:Iterator + 'a> Iterator for NodeIterator<'a, T fn next(&mut self) -> Option { let mut documentation = None; - while let Some((index,line)) = self.lines_iter.next() { - if let Some(documentation_info) = DocCommentInfo::new(line) { + while let Some((index,ast)) = self.lines_iter.next() { + if let Some(documentation_info) = DocCommentInfo::new(ast) { documentation = Some((index,documentation_info)); - } /* else if ast::macros::is_disable_comment(line) { - // Do nothing. - } */ else if let Some(main_line) = ExpressionLine::from_line_ast(line) { + } else if let Some(main_line) = ExpressionLine::from_line_ast(ast) { let (documentation_line,documentation) = match documentation { Some((index,documentation)) => (Some(index),Some(documentation)), None => (None,None) }; let ret = LocatedNode { - node : NodeInfo {documentation,main_line}, + node : NodeInfo {documentation,main_line}, index : NodeIndex { main_line : index, documentation_line, @@ -158,6 +166,8 @@ impl<'a, T:Iterator + 'a> Iterator for NodeIterator<'a, T } } +/// Information about node, including both its main line (i.e. line with expression) and optionally +/// attached documentation comment. #[derive(Clone,Debug,Shrinkwrap)] #[shrinkwrap(mutable)] pub struct NodeInfo { @@ -169,29 +179,9 @@ pub struct NodeInfo { } impl NodeInfo { - // pub fn iter<'a, IntoLinesIter>(lines:IntoLinesIter) -> NodeIterator<'a, IntoLinesIter::IntoIter> - // where IntoLinesIter:IntoIterator { - // NodeIterator { lines_iter: lines.into_iter().enumerate() }.map(|n| n.node) - // } - - pub fn from_lines<'a>(lines:&mut impl Iterator) -> Option { - let mut documentation = None; - while let Some(line) = lines.next() { - if let Some(documentation_info) = DocCommentInfo::new(line) { - documentation = Some(documentation_info); - } else if ast::macros::is_disable_comment(line) { - // Do nothing. - } else if let Some(main_line) = ExpressionLine::from_line_ast(line) { - return Some(NodeInfo {documentation,main_line}); - } else { - // Non-node entity consumes any previous documentation. - documentation = None; - } - } - None - } - + /// Check if a given non-empty line's AST belongs to this node. pub fn contains_line(&self, line_ast:&Ast) -> bool { + // TODO refactor these two lambdas into methods let expression_id_matches = || ExpressionLine::from_line_ast(line_ast) .as_ref() .map(ExpressionLine::id) @@ -203,6 +193,7 @@ impl NodeInfo { expression_id_matches() || doc_comment_id_matches() } + /// TODO should not be needed as a method here pub fn doc_comment_id(&self) -> Option { self.documentation.as_ref().and_then(|comment| comment.ast().id()) } @@ -240,6 +231,7 @@ impl NodeInfo { ret } + /// Obtain documentation text. pub fn documentation_text(&self) -> Option { self.documentation.as_ref().map(|doc| doc.text()) } @@ -397,106 +389,106 @@ mod tests { use super::*; use ast::opr::predefined::ASSIGNMENT; - // - // fn expect_node(ast:Ast, expression_text:&str, id:Id) { - // let node_info = NodeInfo::from_line_ast(&ast).expect("expected a node"); - // assert_eq!(node_info.expression().repr(),expression_text); - // assert_eq!(node_info.id(), id); - // } - // - // #[test] - // fn expression_node_test() { - // // expression: `4` - // let id = Id::new_v4(); - // let ast = Ast::new(ast::Number { base:None, int: "4".into()}, Some(id)); - // expect_node(ast,"4",id); - // } - // - // #[test] - // fn set_expression_binding() { - // let ast = Ast::infix(Ast::var("foo"),"=",Ast::number(4).with_new_id()); - // assert_eq!(ast.repr(), "foo = 4"); - // - // let mut node = NodeInfo::from_line_ast(&ast).expect("expected a node"); - // let id = node.id(); - // node.set_expression(Ast::var("bar")); - // assert_eq!(node.expression().repr(), "bar"); - // assert_eq!(node.ast().repr(), "foo = bar"); - // assert_eq!(node.id(), id); - // } - // - // #[test] - // fn set_expression_plain() { - // let ast = Ast::number(4).with_new_id(); - // assert_eq!(ast.repr(), "4"); - // - // let mut node = NodeInfo::from_line_ast(&ast).expect("expected a node"); - // let id = node.id(); - // node.set_expression(Ast::var("bar")); - // assert_eq!(node.expression().repr(), "bar"); - // assert_eq!(node.ast().repr(), "bar"); - // assert_eq!(node.id(), id); - // } - // - // #[test] - // fn binding_node_test() { - // // expression: `foo = 4` - // let id = Id::new_v4(); - // let number = ast::Number { base:None, int: "4".into()}; - // let larg = Ast::var("foo"); - // let rarg = Ast::new(number, Some(id)); - // let ast = Ast::infix(larg,ASSIGNMENT,rarg); - // expect_node(ast,"4",id); - // } - // - // #[test] - // fn clearing_pattern_test() { - // // expression: `foo = 4` - // let id = Id::new_v4(); - // let number = ast::Number { base:None, int: "4".into()}; - // let larg = Ast::var("foo"); - // let rarg = Ast::new(number, Some(id)); - // let ast = Ast::infix(larg,ASSIGNMENT,rarg); - // - // let mut node = NodeInfo::from_line_ast(&ast).unwrap(); - // assert_eq!(node.repr(),"foo = 4"); - // assert_eq!(node.id(),id); - // node.clear_pattern(); - // assert_eq!(node.repr(),"4"); - // assert_eq!(node.id(),id); - // node.clear_pattern(); - // assert_eq!(node.repr(),"4"); - // assert_eq!(node.id(),id); - // } - // - // #[test] - // fn setting_pattern_on_expression_node_test() { - // let id = uuid::Uuid::new_v4(); - // let line_ast = Ast::number(2).with_id(id); - // let mut node = NodeInfo::from_line_ast(&line_ast).unwrap(); - // assert_eq!(node.repr(), "2"); - // assert_eq!(node.id(),id); - // - // node.set_pattern(Ast::var("foo")); - // - // assert_eq!(node.repr(), "foo = 2"); - // assert_eq!(node.id(),id); - // } - // - // #[test] - // fn setting_pattern_on_binding_node_test() { - // let id = uuid::Uuid::new_v4(); - // let larg = Ast::var("foo"); - // let rarg = Ast::var("bar").with_id(id); - // let line_ast = Ast::infix(larg,ASSIGNMENT,rarg); - // let mut node = NodeInfo::from_line_ast(&line_ast).unwrap(); - // - // assert_eq!(node.repr(), "foo = bar"); - // assert_eq!(node.id(),id); - // - // node.set_pattern(Ast::var("baz")); - // - // assert_eq!(node.repr(), "baz = bar"); - // assert_eq!(node.id(),id); - // } + + fn expect_node(ast:Ast, expression_text:&str, id:Id) { + let node_info = NodeInfo::from_single_line_ast(&ast).expect("expected a node"); + assert_eq!(node_info.expression().repr(),expression_text); + assert_eq!(node_info.id(), id); + } + + #[test] + fn expression_node_test() { + // expression: `4` + let id = Id::new_v4(); + let ast = Ast::new(ast::Number { base:None, int: "4".into()}, Some(id)); + expect_node(ast,"4",id); + } + + #[test] + fn binding_node_test() { + // expression: `foo = 4` + let id = Id::new_v4(); + let number = ast::Number { base:None, int: "4".into()}; + let larg = Ast::var("foo"); + let rarg = Ast::new(number, Some(id)); + let ast = Ast::infix(larg,ASSIGNMENT,rarg); + expect_node(ast,"4",id); + } + + #[test] + fn set_expression_binding() { + let ast = Ast::infix(Ast::var("foo"),"=",Ast::number(4).with_new_id()); + assert_eq!(ast.repr(), "foo = 4"); + + let mut node = NodeInfo::from_single_line_ast(&ast).expect("expected a node"); + let id = node.id(); + node.set_expression(Ast::var("bar")); + assert_eq!(node.expression().repr(), "bar"); + assert_eq!(node.ast().repr(), "foo = bar"); + assert_eq!(node.id(), id); + } + + #[test] + fn set_expression_plain() { + let ast = Ast::number(4).with_new_id(); + assert_eq!(ast.repr(), "4"); + + let mut node = NodeInfo::from_single_line_ast(&ast).expect("expected a node"); + let id = node.id(); + node.set_expression(Ast::var("bar")); + assert_eq!(node.expression().repr(), "bar"); + assert_eq!(node.ast().repr(), "bar"); + assert_eq!(node.id(), id); + } + + #[test] + fn clearing_pattern_test() { + // expression: `foo = 4` + let id = Id::new_v4(); + let number = ast::Number { base:None, int: "4".into()}; + let larg = Ast::var("foo"); + let rarg = Ast::new(number, Some(id)); + let ast = Ast::infix(larg,ASSIGNMENT,rarg); + + let mut node = NodeInfo::from_single_line_ast(&ast).unwrap(); + assert_eq!(node.repr(),"foo = 4"); + assert_eq!(node.id(),id); + node.clear_pattern(); + assert_eq!(node.repr(),"4"); + assert_eq!(node.id(),id); + node.clear_pattern(); + assert_eq!(node.repr(),"4"); + assert_eq!(node.id(),id); + } + + #[test] + fn setting_pattern_on_expression_node_test() { + let id = uuid::Uuid::new_v4(); + let line_ast = Ast::number(2).with_id(id); + let mut node = NodeInfo::from_single_line_ast(&line_ast).unwrap(); + assert_eq!(node.repr(), "2"); + assert_eq!(node.id(),id); + + node.set_pattern(Ast::var("foo")); + + assert_eq!(node.repr(), "foo = 2"); + assert_eq!(node.id(),id); + } + + #[test] + fn setting_pattern_on_binding_node_test() { + let id = uuid::Uuid::new_v4(); + let larg = Ast::var("foo"); + let rarg = Ast::var("bar").with_id(id); + let line_ast = Ast::infix(larg,ASSIGNMENT,rarg); + let mut node = NodeInfo::from_single_line_ast(&line_ast).unwrap(); + + assert_eq!(node.repr(), "foo = bar"); + assert_eq!(node.id(),id); + + node.set_pattern(Ast::var("baz")); + + assert_eq!(node.repr(), "baz = bar"); + assert_eq!(node.id(),id); + } } diff --git a/src/rust/ide/src/lib.rs b/src/rust/ide/src/lib.rs index f0b9f814e9..2bcfdb36f5 100644 --- a/src/rust/ide/src/lib.rs +++ b/src/rust/ide/src/lib.rs @@ -8,7 +8,6 @@ #![feature(cell_update)] #![feature(drain_filter)] #![feature(exact_size_is_empty)] -#![feature(impl_trait_in_bindings)] #![feature(iter_order_by)] #![feature(maybe_uninit_extra)] #![feature(option_result_contains)] diff --git a/src/rust/ide/view/graph-editor/src/component/node.rs b/src/rust/ide/view/graph-editor/src/component/node.rs index 0c62ad12b4..b1b6fedaa3 100644 --- a/src/rust/ide/view/graph-editor/src/component/node.rs +++ b/src/rust/ide/view/graph-editor/src/component/node.rs @@ -69,6 +69,14 @@ const UNRESOLVED_SYMBOL_TYPE : &str = "Builtins.Main.Unresolved_Symbol"; +// =============== +// === Comment === +// =============== + +pub type Comment = String; + + + // ============= // === Shape === // ============= @@ -257,8 +265,8 @@ ensogl::define_endpoints! { set_visualization (Option), set_disabled (bool), set_input_connected (span_tree::Crumbs,Option,bool), - set_comment (String), set_expression (Expression), + set_comment (Comment), set_error (Option), /// Set the expression USAGE type. This is not the definition type, which can be set with /// `set_expression` instead. In case the usage type is set to None, ports still may be @@ -279,8 +287,8 @@ ensogl::define_endpoints! { /// Press event. Emitted when user clicks on non-active part of the node, like its /// background. In edit mode, the whole node area is considered non-active. background_press (), - comment (String), expression (Text), + comment (Comment), skip (bool), freeze (bool), hover (bool), @@ -460,10 +468,10 @@ impl NodeModel { let style = StyleWatchFrp::new(&app.display.scene().style_sheet); + // TODO: Style the documentation comment properly. let comment = ensogl_text::Area::new(app); - comment.set_position_y(-35.0); + comment.set_position_y(HEIGHT * 1.5); display_object.add_child(&comment); - comment.set_content("Place for your comment. Lorem ipsum dolor sit amet"); let app = app.clone_ref(); Self {app,display_object,logger,backdrop,background,drag_area,error_indicator @@ -617,6 +625,12 @@ impl Node { model.output.set_expression_visibility <+ frp.set_output_expression_visibility; + // === Comment === + + model.comment.set_content <+ frp.set_comment; + out.source.expression <+ model.comment.content; + + // === Size === new_size <- model.input.frp.width.map(f!((w) model.set_width(*w))); diff --git a/src/rust/ide/view/graph-editor/src/lib.rs b/src/rust/ide/view/graph-editor/src/lib.rs index 4a9a571590..3f8cb907e9 100644 --- a/src/rust/ide/view/graph-editor/src/lib.rs +++ b/src/rust/ide/view/graph-editor/src/lib.rs @@ -483,6 +483,7 @@ ensogl::define_endpoints! { edit_node (NodeId), collapse_nodes ((Vec,NodeId)), set_node_expression ((NodeId,node::Expression)), + set_node_comment ((NodeId,node::Comment)), set_node_position ((NodeId,Vector2)), set_expression_usage_type ((NodeId,ast::Id,Option)), set_method_pointer ((ast::Id,Option)), @@ -555,6 +556,7 @@ ensogl::define_endpoints! { node_position_set ((NodeId,Vector2)), node_position_set_batched ((NodeId,Vector2)), node_expression_set ((NodeId,String)), + node_comment_set ((NodeId,String)), node_entered (NodeId), node_exited (), node_editing_started (NodeId), @@ -1183,6 +1185,10 @@ impl GraphEditorModelWithNetwork { hovered <- node.output.hover.map (move |t| Some(Switch::new(node_id,*t))); output.source.node_hovered <+ hovered; + eval node.comment ([model](comment) + model.frp.source.node_comment_set.emit((node_id,comment.clone())) + ); + node.set_output_expression_visibility <+ self.frp.nodes_labels_visible; eval node.frp.tooltip ((tooltip) tooltip_update.emit(tooltip)); @@ -1579,6 +1585,14 @@ impl GraphEditorModel { } } + fn set_node_comment(&self, node_id:impl Into, comment:impl Into) { + let node_id = node_id.into(); + let comment = comment.into(); + if let Some(node) = self.nodes.get_cloned_ref(&node_id) { + node.frp.set_comment.emit(comment); + } + } + fn is_connection(&self, edge_id:impl Into) -> bool { let edge_id = edge_id.into(); match self.edges.get_cloned_ref(&edge_id) { @@ -2590,6 +2604,12 @@ fn new_graph_editor(app:&Application) -> GraphEditor { } + // === Set Node Comment === + frp::extend! { network + + eval inputs.set_node_comment([model] ((id,comment)) model.set_node_comment(id,comment)); + } + // === Set Node Error === frp::extend! { network @@ -3046,7 +3066,6 @@ fn new_graph_editor(app:&Application) -> GraphEditor { port_to_refresh <= inputs.set_node_expression.map(f!(((id,_))model.node_in_edges(id))); eval port_to_refresh ((id) model.set_edge_target_connection_status(*id,true)); - // === Remove implementation === out.source.node_removed <+ inputs.remove_node; } diff --git a/src/rust/ide/view/src/debug_scenes/interface.rs b/src/rust/ide/view/src/debug_scenes/interface.rs index ae4d8847d0..2cf76e333f 100644 --- a/src/rust/ide/view/src/debug_scenes/interface.rs +++ b/src/rust/ide/view/src/debug_scenes/interface.rs @@ -118,6 +118,9 @@ fn init(app:&Application) { let expression_1 = expression_mock(); graph_editor.frp.set_node_expression.emit((node1_id,expression_1.clone())); + let comment_1 = String::from("Sample documentation comment."); + graph_editor.frp.set_node_comment.emit((node1_id,comment_1)); + let expression_2 = expression_mock3(); graph_editor.frp.set_node_expression.emit((node2_id,expression_2.clone())); From 135e8970ab2e6a7873fdbe81e8fe42969ec0f3c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Fri, 30 Jul 2021 15:22:23 +0200 Subject: [PATCH 03/19] [wip] --- src/rust/ide/src/controller/graph.rs | 14 +- src/rust/ide/src/controller/searcher.rs | 2 +- src/rust/ide/src/double_representation.rs | 134 +++++++++------- .../src/double_representation/connection.rs | 4 +- .../src/double_representation/definition.rs | 12 +- .../ide/src/double_representation/graph.rs | 6 +- .../ide/src/double_representation/node.rs | 147 ++++++++---------- .../refactorings/collapse.rs | 6 +- src/rust/ide/src/ide/integration/project.rs | 16 ++ 9 files changed, 179 insertions(+), 162 deletions(-) diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index fb12477cd2..77aae76688 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -14,7 +14,7 @@ use crate::double_representation::identifier::NormalizedName; use crate::double_representation::identifier::generate_name; use crate::double_representation::module; use crate::double_representation::node; -use crate::double_representation::node::{NodeInfo, ExpressionLine, NodeIndex}; +use crate::double_representation::node::{NodeInfo, MainLine, NodeIndex}; use crate::model::module::NodeMetadata; use crate::model::traits::*; @@ -555,7 +555,7 @@ impl Handle { /// Analyzes the expression, e.g. result for "a+b" shall be named "sum". /// The caller should make sure that obtained name won't collide with any symbol usage before /// actually introducing it. See `variable_name_for`. - pub fn variable_name_base_for(node:&ExpressionLine) -> String { + pub fn variable_name_base_for(node:&MainLine) -> String { name_for_ast(node.expression()) } @@ -569,7 +569,7 @@ impl Handle { let body = def.body(); let usage = if matches!(body.shape(),ast::Shape::Block(_)) { alias_analysis::analyze_crumbable(body.item) - } else if let Some(node) = NodeInfo::from_single_line_ast(&body) { + } else if let Some(node) = MainLine::from_ast(&body) { alias_analysis::analyze_ast(node.ast()) } else { // Generally speaking - impossible. But if there is no node in the definition @@ -648,8 +648,8 @@ impl Handle { let dependent_nodes = connection::dependent_nodes_in_def(definition_ast,node_to_be_after); let mut lines = definition.block_lines(); - let node_to_be_before = node::locate(&lines, node_to_be_before)?; - let node_to_be_after = node::locate(&lines, node_to_be_after)?; + let node_to_be_before = node::locate(&lines,node_to_be_before)?; + let node_to_be_after = node::locate(&lines,node_to_be_after)?; let dependent_nodes = dependent_nodes.iter().map(|id| node::locate(&lines, *id)) .collect::,_>>()?; @@ -749,7 +749,7 @@ impl Handle { pub fn add_node(&self, node:NewNodeInfo) -> FallibleResult { info!(self.logger, "Adding node with expression `{node.expression}`"); let expression_ast = self.parse_node_expression(&node.expression)?; - let main_line = ExpressionLine::from_line_ast(&expression_ast).ok_or(FailedToCreateNode)?; + let main_line = MainLine::from_ast(&expression_ast).ok_or(FailedToCreateNode)?; let documentation = node.doc_comment.as_ref() .map(|text| DocCommentInfo::pretty_print_text(&text)) .map(|doc_code| self.parser.parse_line(doc_code)) @@ -1594,7 +1594,7 @@ main = for (code,expected_name) in &cases { let ast = parser.parse_line(*code).unwrap(); - let node = ExpressionLine::from_line_ast(&ast).unwrap(); + let node = MainLine::from_ast(&ast).unwrap(); let name = Handle::variable_name_base_for(&node); assert_eq!(&name,expected_name); } diff --git a/src/rust/ide/src/controller/searcher.rs b/src/rust/ide/src/controller/searcher.rs index 508161ef8d..477fd4bb2e 100644 --- a/src/rust/ide/src/controller/searcher.rs +++ b/src/rust/ide/src/controller/searcher.rs @@ -768,7 +768,7 @@ impl Searcher { let args = std::iter::empty(); let node_expression = ast::prefix::Chain::new_with_this(new_definition_name,here,args); let node_expression = node_expression.into_ast(); - let node = NodeInfo::from_single_line_ast(&node_expression).ok_or(FailedToCreateNode)?; + let node = NodeInfo::from_main_line_ast(&node_expression).ok_or(FailedToCreateNode)?; let added_node_id = node.id(); let graph_definition = double_representation::module::locate(&module.ast,&self.graph.graph().id)?; let mut graph_info = GraphInfo::from_definition(graph_definition.item); diff --git a/src/rust/ide/src/double_representation.rs b/src/rust/ide/src/double_representation.rs index 01c9e8ef08..485878925b 100644 --- a/src/rust/ide/src/double_representation.rs +++ b/src/rust/ide/src/double_representation.rs @@ -7,6 +7,7 @@ use ast::{Ast, opr, prefix, known}; use crate::double_representation::definition::DefinitionName; use crate::double_representation::definition::ScopeKind; use ast::crumbs::{Located, InfixCrumb}; +use ast::macros::DocCommentInfo; pub mod alias_analysis; pub mod comment; @@ -36,76 +37,99 @@ pub mod test_utils; /// Link: https://github.com/enso-org/enso/blob/main/doc/syntax/encoding.md pub const INDENT : usize = 4; +/// What kind of node or definition a line should be treated as. +#[derive(Clone,Debug)] pub enum LineKind { + /// Definition is a binding, which defines a new entity with arguments. Definition { + /// Ast of the whole binding. ast : known::Infix, + /// Definition name. If this is an extension method, it might imply an implicit argument. name : Located, + /// Explicit arguments. Note that this might be empty when there are implicit arguments. args : Vec>, }, + /// Node in a binding form. ExpressionAssignment { + /// Ast of the whole binding. ast : known::Infix, }, + /// Node consisting of a plain expression, with no pattern binding. ExpressionPlain { + /// Ast of the whole expression. ast : Ast, }, + /// Documentation comment lines are not nodes. + /// Instead, they are discovered and processed as part of nodes that follow them. + DocumentationComment { + /// The comment representation. + comment : DocCommentInfo, + } } -pub fn discern_line -(ast:&Ast, kind:ScopeKind) -> Option { - use LineKind::*; - - // First of all, if line is not an infix assignment, it can be only node or nothing. - let infix = match opr::to_assignment(ast) { - Some(infix) => infix, - // Documentation comment lines are not nodes. Here they are ignored. - // They are discovered and processed as part of nodes that follow them. - // e.g. `## Comment`. - None if ast::macros::is_documentation_comment(ast) => return None, - // The simplest form of node, e.g. `Point 5 10` - None => return Some(ExpressionPlain {ast:ast.clone_ref()}), - }; - - // Assignment can be either nodes or definitions. To discern, we check the left hand side. - // For definition it is a prefix chain, where first is the name, then arguments (if explicit). - // For node it is a pattern, either in a form of Var without args on Cons application. - let crumb = InfixCrumb::LeftOperand; - let lhs = Located::new(crumb,prefix::Chain::from_ast_non_strict(&infix.larg)); - let name = lhs.entered(|chain| { - let name_ast = chain.located_func(); - name_ast.map(DefinitionName::from_ast) - }).into_opt(); - - // If this is a pattern match, `name` will fail to construct and we'll treat line as a node. - // e.g. for `Point x y = get_point …` - let name = match name { - Some(name) => name, - None => return Some(ExpressionAssignment{ast:infix}) - }; - - let args = lhs.enumerate_args().map(|Located{crumbs,item}| { - // We already in the left side of assignment, so we need to prepend this crumb. - let crumbs = lhs.crumbs.clone().into_iter().chain(crumbs); - let ast = item.clone(); - Located::new(crumbs,ast) - }).collect_vec(); - - // Note [Scope Differences] - if kind == ScopeKind::NonRoot { - // 1. Not an extension method but an old setter syntax. Currently not supported in the - // language, treated as node with invalid pattern. - // e.g. `point.x = 5` - let is_setter = !name.extended_target.is_empty(); - // 2. No explicit args -- this is a proper node, not a definition. - // e.g. `point = Point 5 10` - let is_node = args.is_empty(); - if is_setter || is_node { - return Some(ExpressionAssignment{ast:infix}) +impl LineKind { + + /// Tell how the given line (described by Ast) should be treated. + /// Returns `None` if line does not match any of the expected entities. + pub fn discern(ast:&Ast, kind:ScopeKind) -> Self { + use LineKind::*; + + // First of all, if line is not an infix assignment, it can be only node or nothing. + let infix = match opr::to_assignment(ast) { + Some(infix) => + infix, + None => + return if let Some(comment) = DocCommentInfo::new(ast) { + // e.g. `## My comment.` + DocumentationComment {comment} + } else { + // The simplest form of node, e.g. `Point 5 10` + ExpressionPlain {ast:ast.clone_ref()} + } + }; + + // Assignment can be either nodes or definitions. To discern, we check the left hand side. + // For definition it is a prefix chain, where first is the name, then arguments (if explicit). + // For node it is a pattern, either in a form of Var without args on Cons application. + let crumb = InfixCrumb::LeftOperand; + let lhs = Located::new(crumb,prefix::Chain::from_ast_non_strict(&infix.larg)); + let name = lhs.entered(|chain| { + let name_ast = chain.located_func(); + name_ast.map(DefinitionName::from_ast) + }).into_opt(); + + // If this is a pattern match, `name` will fail to construct and we'll treat line as a node. + // e.g. for `Point x y = get_point …` + let name = match name { + Some(name) => name, + None => return ExpressionAssignment{ast:infix} + }; + + let args = lhs.enumerate_args().map(|Located{crumbs,item}| { + // We already in the left side of assignment, so we need to prepend this crumb. + let crumbs = lhs.crumbs.clone().into_iter().chain(crumbs); + let ast = item.clone(); + Located::new(crumbs,ast) + }).collect_vec(); + + // Note [Scope Differences] + if kind == ScopeKind::NonRoot { + // 1. Not an extension method but an old setter syntax. Currently not supported in the + // language, treated as node with invalid pattern. + // e.g. `point.x = 5` + let is_setter = !name.extended_target.is_empty(); + // 2. No explicit args -- this is a proper node, not a definition. + // e.g. `point = Point 5 10` + let is_node = args.is_empty(); + if is_setter || is_node { + return ExpressionAssignment{ast:infix} + } + }; + + Definition { + args,name,ast:infix.clone_ref() } - }; - - Some(Definition { - args,name,ast:infix.clone_ref() - }) + } } // Note [Scope Differences] diff --git a/src/rust/ide/src/double_representation/connection.rs b/src/rust/ide/src/double_representation/connection.rs index 36984f7cd4..4cfbc83ba7 100644 --- a/src/rust/ide/src/double_representation/connection.rs +++ b/src/rust/ide/src/double_representation/connection.rs @@ -6,7 +6,7 @@ use crate::double_representation::alias_analysis::analyze_crumbable; use crate::double_representation::definition::DefinitionInfo; use crate::double_representation::definition::ScopeKind; use crate::double_representation::identifier::NormalizedName; -use crate::double_representation::node::ExpressionLine; +use crate::double_representation::node::MainLine; use crate::double_representation::node::Id; use ast::crumbs::Crumb; @@ -42,7 +42,7 @@ impl Endpoint { let line_ast = block.get(&line_crumb).ok()?; let definition = DefinitionInfo::from_line_ast(&line_ast,ScopeKind::NonRoot,block.indent); let is_non_def = definition.is_none(); - let node = is_non_def.and_option_from(|| ExpressionLine::from_line_ast(&line_ast))?.id(); + let node = is_non_def.and_option_from(|| MainLine::from_ast(&line_ast))?.id(); Some(Endpoint { node, crumbs }) } } diff --git a/src/rust/ide/src/double_representation/definition.rs b/src/rust/ide/src/double_representation/definition.rs index 216c7959e3..753e550268 100644 --- a/src/rust/ide/src/double_representation/definition.rs +++ b/src/rust/ide/src/double_representation/definition.rs @@ -9,7 +9,7 @@ use ast::crumbs::Located; use ast::known; use ast::opr; use parser::Parser; -use crate::double_representation::{LineKind, discern_line}; +use crate::double_representation::LineKind; // ===================== @@ -320,7 +320,7 @@ impl DefinitionInfo { /// some binding or other kind of subtree). pub fn from_line_ast (ast:&Ast, kind:ScopeKind, context_indent:usize) -> Option { - if let Some(LineKind::Definition{args,ast,name}) = discern_line(ast,kind) { + if let LineKind::Definition{args,ast,name} = LineKind::discern(ast,kind) { Some(DefinitionInfo { ast, name, args, context_indent }) } else { None @@ -529,14 +529,6 @@ impl ToAdd { - -pub fn enumerate_non_empty_lines<'a> -( lines : impl IntoIterator>> + 'a -) -> impl Iterator + 'a { - lines.into_iter().enumerate().filter_map(|(index,line)| line.elem.as_ref().map(|ast| (index,ast))) -} - - // ============= // === Tests === // ============= diff --git a/src/rust/ide/src/double_representation/graph.rs b/src/rust/ide/src/double_representation/graph.rs index 068af38aad..c0c001e2b2 100644 --- a/src/rust/ide/src/double_representation/graph.rs +++ b/src/rust/ide/src/double_representation/graph.rs @@ -60,7 +60,7 @@ impl GraphInfo { let body = ast.rarg.clone(); if let Ok(body_block) = known::Block::try_new(body.clone()) { block_nodes(&body_block) - } else if let Some(main_line) = node::ExpressionLine::from_line_ast(&body) { + } else if let Some(main_line) = node::MainLine::from_ast(&body) { // There's no way to attach a documentation comment to an inline node. let documentation = None; vec![ NodeInfo {documentation,main_line} ] @@ -211,7 +211,7 @@ impl GraphInfo { // ===================== /// Collects information about nodes in given code `Block`. -pub fn block_nodes<'a>(ast:&'a known::Block) -> Vec { +pub fn block_nodes(ast:&known::Block) -> Vec { // Warning: this uses faux indices, as the empty lines are skipped first. // It is fine here, since we throw away the result values depending on index. let lines_iter = ast.iter().enumerate(); @@ -282,7 +282,7 @@ mod tests { fn new_expression_node(parser:&parser::Parser, expression:&str) -> NodeInfo { let node_ast = parser.parse(expression.to_string(), default()).unwrap(); let line_ast = expect_single_line(&node_ast).clone(); - NodeInfo::from_single_line_ast(&line_ast).unwrap() + NodeInfo::from_main_line_ast(&line_ast).unwrap() } fn assert_at(nodes:&[NodeInfo], index:usize, expected:&NodeInfo) { diff --git a/src/rust/ide/src/double_representation/node.rs b/src/rust/ide/src/double_representation/node.rs index c114ef3388..54d99d6327 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -7,7 +7,7 @@ use ast::crumbs::Crumbable; use ast::known; use std::cmp::Ordering; use ast::macros::DocCommentInfo; -use crate::double_representation::{discern_line, LineKind}; +use crate::double_representation::LineKind; use crate::double_representation::definition::ScopeKind; /// Node Id is the Ast Id attached to the node's expression. @@ -77,7 +77,7 @@ pub struct LocatedNode { /// Tests if given line contents can be seen as node with a given id pub fn is_main_line_of(line:&ast::BlockLine>, id:Id) -> bool { - let node_info = ExpressionLine::from_block_line(line); + let node_info = MainLine::from_block_line(line); node_info.contains_if(|node| node.id() == id) } @@ -99,7 +99,11 @@ pub fn locate_many<'a> ( lines : impl IntoIterator>> + 'a , looked_for : impl IntoIterator ) -> FallibleResult> { - let lines_iter = double_representation::definition::enumerate_non_empty_lines(lines); + // Skip empty lines, while preserving indices. + let lines_iter = lines.into_iter() + .enumerate() + .filter_map(|(index,line)| line.elem.as_ref().map(|ast| (index,ast))); + let mut looked_for = looked_for.into_iter().collect::>(); let mut ret = HashMap::new(); @@ -143,7 +147,7 @@ impl<'a, T:Iterator + 'a> Iterator for NodeIterator<'a, T while let Some((index,ast)) = self.lines_iter.next() { if let Some(documentation_info) = DocCommentInfo::new(ast) { documentation = Some((index,documentation_info)); - } else if let Some(main_line) = ExpressionLine::from_line_ast(ast) { + } else if let Some(main_line) = MainLine::from_ast(ast) { let (documentation_line,documentation) = match documentation { Some((index,documentation)) => (Some(index),Some(documentation)), None => (None,None) @@ -171,20 +175,20 @@ impl<'a, T:Iterator + 'a> Iterator for NodeIterator<'a, T #[derive(Clone,Debug,Shrinkwrap)] #[shrinkwrap(mutable)] pub struct NodeInfo { + /// If the node has doc comment attached, it will be represented here. + pub documentation : Option, /// Primary node AST that contains node's expression and optional pattern binding. #[shrinkwrap(main_field)] - pub main_line: ExpressionLine, - /// If the node has doc comment attached, it will be represented here. - pub documentation: Option, + pub main_line : MainLine, } impl NodeInfo { /// Check if a given non-empty line's AST belongs to this node. pub fn contains_line(&self, line_ast:&Ast) -> bool { // TODO refactor these two lambdas into methods - let expression_id_matches = || ExpressionLine::from_line_ast(line_ast) + let expression_id_matches = || MainLine::from_ast(line_ast) .as_ref() - .map(ExpressionLine::id) + .map(MainLine::id) .contains(&self.id()); let doc_comment_id_matches = || match (self.doc_comment_id(), line_ast.id) { (Some(node_doc_id),Some(line_ast_id)) => node_doc_id == line_ast_id, @@ -198,37 +202,11 @@ impl NodeInfo { self.documentation.as_ref().and_then(|comment| comment.ast().id()) } - pub fn from_single_line_ast(ast:&Ast) -> Option { - ExpressionLine::from_line_ast(ast).map(|ast| Self { main_line: ast, documentation:None}) - } - - pub fn nodes_from_lines<'a>(lines:impl IntoIterator) -> Vec { - let mut ret = Vec::new(); - - let mut lines = lines.into_iter(); - while let Some(line) = lines.next() { - // Node is either: - // * documentation comment line followed by the node expression, - // * node expression line. - if let Some(doc_comment) = DocCommentInfo::new(line) { - if let Some(node) = lines.next().and_then(ExpressionLine::from_line_ast) { - ret.push(NodeInfo { - main_line: node, - documentation: Some(doc_comment), - }) - } else { - // Dangling documentation comments never apply to the nodes, so we ignore them. - } - } else if let Some(node) = ExpressionLine::from_line_ast(line) { - ret.push(NodeInfo { - main_line: node, - documentation: None, - }) - } else { - WARNING!("Line '{line}' is neither a doc comment nor a node.") - } - } - ret + /// Construct node information for a single line, without documentation. + pub fn from_main_line_ast(ast:&Ast) -> Option { + let main_line = MainLine::from_ast(ast)?; + let documentation = None; + Some(Self {main_line,documentation}) } /// Obtain documentation text. @@ -237,44 +215,51 @@ impl NodeInfo { } } -/// Description of the node that consists of all information locally available about node. -/// Nodes are required to bear IDs. This enum should never contain an ast of node without id set. +/// Representation of the main line of the node (as opposed to a documentation line). +/// +/// Each node must have exactly one main line. +/// Main line always contains an expression, either directly or under binding. The expression id +/// must be set and it serves as the whole node's expression. #[derive(Clone,Debug)] #[allow(missing_docs)] -pub enum ExpressionLine { +pub enum MainLine { /// Code with assignment, e.g. `foo = 2 + 2` Binding { infix: known::Infix }, /// Code without assignment (no variable binding), e.g. `2 + 2`. Expression { ast: Ast }, } -impl ExpressionLine { +impl MainLine { /// Tries to interpret the whole binding as a node. Right-hand side will become node's /// expression. - pub fn new_binding(infix:known::Infix) -> Option { + pub fn new_binding(infix:known::Infix) -> Option { infix.rarg.id?; - Some(ExpressionLine::Binding {infix}) + Some(MainLine::Binding {infix}) } /// Tries to interpret AST as node, treating whole AST as an expression. - pub fn new_expression(ast:Ast) -> Option { + pub fn new_expression(ast:Ast) -> Option { ast.id?; // TODO what if we are given an assignment. - Some(ExpressionLine::Expression {ast}) - } - - /// Tries to interpret AST as node, treating whole AST as an expression. - pub fn from_line_ast(ast:&Ast) -> Option { - match discern_line(ast, ScopeKind::NonRoot) { - Some(LineKind::ExpressionPlain{ast}) => Self::new_expression(ast), - Some(LineKind::ExpressionAssignment {ast}) => Self::new_binding(ast), - Some(LineKind::Definition {..}) | None => None, + Some(MainLine::Expression {ast}) + } + + /// Tries to interpret AST as node, treating whole AST as a node's primary line. + pub fn from_ast(ast:&Ast) -> Option { + // By definition, there are no nodes in the root scope. + // Being a node's line, we may assume that this is not a root scope. + let scope = ScopeKind::NonRoot; + match LineKind::discern(ast,scope) { + LineKind::ExpressionPlain {ast} => Self::new_expression(ast), + LineKind::ExpressionAssignment {ast} => Self::new_binding(ast), + LineKind::Definition {..} => None, + LineKind::DocumentationComment {..} => None, } } /// Tries to interpret AST as node, treating whole AST as an expression. - pub fn from_block_line(line:&ast::BlockLine>) -> Option { - Self::from_line_ast(line.elem.as_ref()?) + pub fn from_block_line(line:&ast::BlockLine>) -> Option { + Self::from_ast(line.elem.as_ref()?) } /// Node's unique ID. @@ -287,13 +272,13 @@ impl ExpressionLine { /// Updates the node's AST so the node bears the given ID. pub fn set_id(&mut self, new_id:Id) { match self { - ExpressionLine::Binding{ref mut infix} => { + MainLine::Binding{ref mut infix} => { let new_rarg = infix.rarg.with_id(new_id); let set = infix.set(&ast::crumbs::InfixCrumb::RightOperand.into(),new_rarg); *infix = set.expect("Internal error: setting infix operand should always \ succeed."); } - ExpressionLine::Expression{ref mut ast} => { + MainLine::Expression{ref mut ast} => { *ast = ast.with_id(new_id); } }; @@ -302,16 +287,16 @@ impl ExpressionLine { /// AST of the node's expression. pub fn expression(&self) -> &Ast { match self { - ExpressionLine::Binding {infix} => &infix.rarg, - ExpressionLine::Expression{ast} => &ast, + MainLine::Binding {infix} => &infix.rarg, + MainLine::Expression{ast} => &ast, } } /// AST of the node's pattern (assignment's left-hand side). pub fn pattern(&self) -> Option<&Ast> { match self { - ExpressionLine::Binding {infix} => Some(&infix.larg), - ExpressionLine::Expression{..} => None, + MainLine::Binding {infix} => Some(&infix.larg), + MainLine::Expression{..} => None, } } @@ -319,9 +304,9 @@ impl ExpressionLine { pub fn set_expression(&mut self, expression:Ast) { let id = self.id(); match self { - ExpressionLine::Binding{ref mut infix} => + MainLine::Binding{ref mut infix} => infix.update_shape(|infix| infix.rarg = expression), - ExpressionLine::Expression{ref mut ast} => *ast = expression, + MainLine::Expression{ref mut ast} => *ast = expression, }; // Id might have been overwritten by the AST we have set. Now we restore it. self.set_id(id); @@ -330,8 +315,8 @@ impl ExpressionLine { /// The whole AST of node. pub fn ast(&self) -> &Ast { match self { - ExpressionLine::Binding {infix} => infix.into(), - ExpressionLine::Expression{ast} => ast, + MainLine::Binding {infix} => infix.into(), + MainLine::Expression{ast} => ast, } } @@ -339,11 +324,11 @@ impl ExpressionLine { /// assignment infix will be introduced. pub fn set_pattern(&mut self, pattern:Ast) { match self { - ExpressionLine::Binding {infix} => { + MainLine::Binding {infix} => { // Setting infix operand never fails. infix.update_shape(|infix| infix.larg = pattern) } - ExpressionLine::Expression {ast} => { + MainLine::Expression {ast} => { let infix = ast::Infix { larg : pattern, loff : 1, @@ -352,7 +337,7 @@ impl ExpressionLine { rarg : ast.clone(), }; let infix = known::Infix::new(infix, None); - *self = ExpressionLine::Binding {infix}; + *self = MainLine::Binding {infix}; } } @@ -363,16 +348,16 @@ impl ExpressionLine { /// If it is already an Expression node, no change is done. pub fn clear_pattern(&mut self) { match self { - ExpressionLine::Binding {infix} => { - *self = ExpressionLine::Expression {ast:infix.rarg.clone_ref()} + MainLine::Binding {infix} => { + *self = MainLine::Expression {ast:infix.rarg.clone_ref()} } - ExpressionLine::Expression {..} => {} + MainLine::Expression {..} => {} } } } -impl ast::HasTokens for ExpressionLine { +impl ast::HasTokens for MainLine { fn feed_to(&self, consumer:&mut impl ast::TokenConsumer) { self.ast().feed_to(consumer) } @@ -391,7 +376,7 @@ mod tests { use ast::opr::predefined::ASSIGNMENT; fn expect_node(ast:Ast, expression_text:&str, id:Id) { - let node_info = NodeInfo::from_single_line_ast(&ast).expect("expected a node"); + let node_info = NodeInfo::from_main_line_ast(&ast).expect("expected a node"); assert_eq!(node_info.expression().repr(),expression_text); assert_eq!(node_info.id(), id); } @@ -420,7 +405,7 @@ mod tests { let ast = Ast::infix(Ast::var("foo"),"=",Ast::number(4).with_new_id()); assert_eq!(ast.repr(), "foo = 4"); - let mut node = NodeInfo::from_single_line_ast(&ast).expect("expected a node"); + let mut node = NodeInfo::from_main_line_ast(&ast).expect("expected a node"); let id = node.id(); node.set_expression(Ast::var("bar")); assert_eq!(node.expression().repr(), "bar"); @@ -433,7 +418,7 @@ mod tests { let ast = Ast::number(4).with_new_id(); assert_eq!(ast.repr(), "4"); - let mut node = NodeInfo::from_single_line_ast(&ast).expect("expected a node"); + let mut node = NodeInfo::from_main_line_ast(&ast).expect("expected a node"); let id = node.id(); node.set_expression(Ast::var("bar")); assert_eq!(node.expression().repr(), "bar"); @@ -450,7 +435,7 @@ mod tests { let rarg = Ast::new(number, Some(id)); let ast = Ast::infix(larg,ASSIGNMENT,rarg); - let mut node = NodeInfo::from_single_line_ast(&ast).unwrap(); + let mut node = NodeInfo::from_main_line_ast(&ast).unwrap(); assert_eq!(node.repr(),"foo = 4"); assert_eq!(node.id(),id); node.clear_pattern(); @@ -465,7 +450,7 @@ mod tests { fn setting_pattern_on_expression_node_test() { let id = uuid::Uuid::new_v4(); let line_ast = Ast::number(2).with_id(id); - let mut node = NodeInfo::from_single_line_ast(&line_ast).unwrap(); + let mut node = NodeInfo::from_main_line_ast(&line_ast).unwrap(); assert_eq!(node.repr(), "2"); assert_eq!(node.id(),id); @@ -481,7 +466,7 @@ mod tests { let larg = Ast::var("foo"); let rarg = Ast::var("bar").with_id(id); let line_ast = Ast::infix(larg,ASSIGNMENT,rarg); - let mut node = NodeInfo::from_single_line_ast(&line_ast).unwrap(); + let mut node = NodeInfo::from_main_line_ast(&line_ast).unwrap(); assert_eq!(node.repr(), "foo = bar"); assert_eq!(node.id(),id); diff --git a/src/rust/ide/src/double_representation/refactorings/collapse.rs b/src/rust/ide/src/double_representation/refactorings/collapse.rs index 3c57ab7575..374164b8cc 100644 --- a/src/rust/ide/src/double_representation/refactorings/collapse.rs +++ b/src/rust/ide/src/double_representation/refactorings/collapse.rs @@ -11,7 +11,7 @@ use crate::double_representation::definition::DefinitionInfo; use crate::double_representation::definition; use crate::double_representation::identifier::Identifier; use crate::double_representation::node; -use crate::double_representation::node::{NodeInfo, ExpressionLine}; +use crate::double_representation::node::{NodeInfo, MainLine}; use crate::double_representation::graph::GraphInfo; use ast::crumbs::Located; @@ -341,10 +341,10 @@ impl Collapser { }; if !self.extracted.belongs_to_selection(ast) { Ok(LineDisposition::Keep) - } else if ExpressionLine::from_line_ast(ast).contains_if(|n| n.id() == self.replaced_node) { + } else if MainLine::from_ast(ast).contains_if(|n| n.id() == self.replaced_node) { let no_node_err = failure::Error::from(CannotConstructCollapsedNode); let expression_ast = self.call_to_extracted(&extracted_definition)?; - let expression = ExpressionLine::from_line_ast(&expression_ast).ok_or(no_node_err)?; + let expression = MainLine::from_ast(&expression_ast).ok_or(no_node_err)?; let mut new_node = NodeInfo { documentation : None, main_line : expression, diff --git a/src/rust/ide/src/ide/integration/project.rs b/src/rust/ide/src/ide/integration/project.rs index e83e67df0e..f8d7ce4931 100644 --- a/src/rust/ide/src/ide/integration/project.rs +++ b/src/rust/ide/src/ide/integration/project.rs @@ -689,6 +689,7 @@ impl Model { self.refresh_node_selection(displayed,node_info); self.refresh_node_visualization(displayed,node_info); }; + self.refresh_node_comment(displayed,node_info); self.refresh_node_expression(displayed,node_info,node_trees); }, None => self.create_node_view(node_info,node_trees,*default_pos), @@ -746,6 +747,7 @@ impl Model { (&self, id:graph_editor::NodeId, node:&controller::graph::Node, trees:NodeTrees) { self.refresh_node_position(id,node); self.refresh_node_selection(id,node); + self.refresh_node_comment(id,node); self.refresh_node_expression(id,node,trees); self.refresh_node_visualization(id,node); } @@ -800,6 +802,20 @@ impl Model { } } } + /// Update the documentation comment on the node. + fn refresh_node_comment + (&self, id:graph_editor::NodeId, node:&controller::graph::Node) { + if let Some(node_view) = self.view.graph().model.nodes.get_cloned_ref(&id) { + let comment_as_per_controller = node.info.documentation_text().unwrap_or_default(); + let comment_as_per_view = node_view.comment.value(); + DEBUG!("Comment on node {node.info.ast()}: {comment_as_per_controller}"); + if comment_as_per_controller != comment_as_per_view { + node_view.set_comment(comment_as_per_controller); + } + } else { + DEBUG!("Cannot refresh on node that cannot be found!"); + } + } /// Update the expression of the node and all related properties e.g., types, ports). fn refresh_node_expression From 63fbcdb4bd63238fd3147fa4b6b9e28016761116 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Fri, 30 Jul 2021 15:28:34 +0200 Subject: [PATCH 04/19] [wip] --- src/rust/ide/src/controller/graph.rs | 2 +- src/rust/ide/src/double_representation/connection.rs | 2 +- src/rust/ide/src/double_representation/definition.rs | 9 --------- src/rust/ide/src/double_representation/graph.rs | 5 ++--- .../src/double_representation/refactorings/collapse.rs | 3 ++- 5 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 77aae76688..d3876fa1d0 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -18,8 +18,8 @@ use crate::double_representation::node::{NodeInfo, MainLine, NodeIndex}; use crate::model::module::NodeMetadata; use crate::model::traits::*; -use ast::macros::DocCommentInfo; use ast::crumbs::InfixCrumb; +use ast::macros::DocCommentInfo; use enso_protocol::language_server; use parser::Parser; use span_tree::SpanTree; diff --git a/src/rust/ide/src/double_representation/connection.rs b/src/rust/ide/src/double_representation/connection.rs index 4cfbc83ba7..f342a32c75 100644 --- a/src/rust/ide/src/double_representation/connection.rs +++ b/src/rust/ide/src/double_representation/connection.rs @@ -6,8 +6,8 @@ use crate::double_representation::alias_analysis::analyze_crumbable; use crate::double_representation::definition::DefinitionInfo; use crate::double_representation::definition::ScopeKind; use crate::double_representation::identifier::NormalizedName; -use crate::double_representation::node::MainLine; use crate::double_representation::node::Id; +use crate::double_representation::node::MainLine; use ast::crumbs::Crumb; use ast::crumbs::Crumbs; diff --git a/src/rust/ide/src/double_representation/definition.rs b/src/rust/ide/src/double_representation/definition.rs index 753e550268..3fb7de9e61 100644 --- a/src/rust/ide/src/double_representation/definition.rs +++ b/src/rust/ide/src/double_representation/definition.rs @@ -238,15 +238,6 @@ impl DefinitionInfo { Located::new(InfixCrumb::RightOperand,&self.ast.rarg) } - // pub fn body_block(&self) -> Result,Located> { - // let body = self.body(); - // if let Ok(block) = known::Block::try_from(*body) { - // Ok(body.map(|_| block)) - // } else { - // Err(body.map(CloneRef::clone_ref)) - // } - // } - /// Gets the definition block lines. If `body` is a `Block`, it returns its `BlockLine`s, /// concatenating `empty_lines`, `first_line` and `lines`, in this exact order. If `body` is /// `Infix`, it returns a single `BlockLine`. diff --git a/src/rust/ide/src/double_representation/graph.rs b/src/rust/ide/src/double_representation/graph.rs index c0c001e2b2..dcecf9957c 100644 --- a/src/rust/ide/src/double_representation/graph.rs +++ b/src/rust/ide/src/double_representation/graph.rs @@ -65,9 +65,8 @@ impl GraphInfo { let documentation = None; vec![ NodeInfo {documentation,main_line} ] } else { - // TODO warning? - // should not be possible to have empty definition without any nodes but it is - // possible to represent such in AST + // It should not be possible to have empty definition without any nodes but it is + // possible to represent such thing in AST. Anyway, it has no nodes. vec![] } } diff --git a/src/rust/ide/src/double_representation/refactorings/collapse.rs b/src/rust/ide/src/double_representation/refactorings/collapse.rs index 374164b8cc..93e537b3d1 100644 --- a/src/rust/ide/src/double_representation/refactorings/collapse.rs +++ b/src/rust/ide/src/double_representation/refactorings/collapse.rs @@ -11,7 +11,8 @@ use crate::double_representation::definition::DefinitionInfo; use crate::double_representation::definition; use crate::double_representation::identifier::Identifier; use crate::double_representation::node; -use crate::double_representation::node::{NodeInfo, MainLine}; +use crate::double_representation::node::NodeInfo; +use crate::double_representation::node::MainLine; use crate::double_representation::graph::GraphInfo; use ast::crumbs::Located; From 2cc87609c1cb32ccb722681ac91458a362cb3293 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Fri, 30 Jul 2021 15:57:55 +0200 Subject: [PATCH 05/19] [wip] --- src/rust/ide/src/double_representation/comment.rs | 2 ++ src/rust/ide/src/ide/integration/project.rs | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/rust/ide/src/double_representation/comment.rs b/src/rust/ide/src/double_representation/comment.rs index 4838ff94cf..7130f21d14 100644 --- a/src/rust/ide/src/double_representation/comment.rs +++ b/src/rust/ide/src/double_representation/comment.rs @@ -1,5 +1,7 @@ use crate::prelude::*; +// FIXME move to other parser-related tests + #[cfg(test)] mod tests { use super::*; diff --git a/src/rust/ide/src/ide/integration/project.rs b/src/rust/ide/src/ide/integration/project.rs index f8d7ce4931..5acda881c6 100644 --- a/src/rust/ide/src/ide/integration/project.rs +++ b/src/rust/ide/src/ide/integration/project.rs @@ -808,8 +808,9 @@ impl Model { if let Some(node_view) = self.view.graph().model.nodes.get_cloned_ref(&id) { let comment_as_per_controller = node.info.documentation_text().unwrap_or_default(); let comment_as_per_view = node_view.comment.value(); - DEBUG!("Comment on node {node.info.ast()}: {comment_as_per_controller}"); + DEBUG!("Comment on node {node.info.ast()}: `{comment_as_per_controller}`, while view `{comment_as_per_view}`"); if comment_as_per_controller != comment_as_per_view { + INFO!("Setting comment `{comment_as_per_controller}`"); node_view.set_comment(comment_as_per_controller); } } else { From 5518337b6e0a8b663cd7b3616075189d4a7fbf5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Fri, 30 Jul 2021 16:01:16 +0200 Subject: [PATCH 06/19] [wip] --- src/rust/ide/lib/ast/impl/src/macros.rs | 2 ++ src/rust/ide/src/controller/graph.rs | 6 ++++-- src/rust/ide/src/double_representation/node.rs | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/rust/ide/lib/ast/impl/src/macros.rs b/src/rust/ide/lib/ast/impl/src/macros.rs index 552399c44f..fa3548f928 100644 --- a/src/rust/ide/lib/ast/impl/src/macros.rs +++ b/src/rust/ide/lib/ast/impl/src/macros.rs @@ -74,6 +74,8 @@ impl DocCommentInfo { Self::new_indented(ast,0) } + /// Creates a documentation from Ast and information about indentation of the block it belongs + /// to. pub fn new_indented(ast:&Ast, block_indent:usize) -> Option { let ast = crate::known::Match::try_from(ast).ok()?; let first_segment = &ast.segs.head; diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index d3876fa1d0..ad4b13b99a 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -14,7 +14,9 @@ use crate::double_representation::identifier::NormalizedName; use crate::double_representation::identifier::generate_name; use crate::double_representation::module; use crate::double_representation::node; -use crate::double_representation::node::{NodeInfo, MainLine, NodeIndex}; +use crate::double_representation::node::MainLine; +use crate::double_representation::node::NodeIndex; +use crate::double_representation::node::NodeInfo; use crate::model::module::NodeMetadata; use crate::model::traits::*; @@ -757,7 +759,7 @@ impl Handle { .map(|doc_ast| DocCommentInfo::new(&doc_ast).ok_or(FailedToCreateNode)) .transpose()?; - let mut node_info = NodeInfo {main_line,documentation}; + let mut node_info = NodeInfo {documentation,main_line}; if let Some(desired_id) = node.id { node_info.set_id(desired_id) } diff --git a/src/rust/ide/src/double_representation/node.rs b/src/rust/ide/src/double_representation/node.rs index 54d99d6327..0b8a70155f 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -206,7 +206,7 @@ impl NodeInfo { pub fn from_main_line_ast(ast:&Ast) -> Option { let main_line = MainLine::from_ast(ast)?; let documentation = None; - Some(Self {main_line,documentation}) + Some(Self {documentation,main_line}) } /// Obtain documentation text. From 95752b213425ac4f03c3ad36a547c0ff3235804b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Sat, 31 Jul 2021 14:15:33 +0200 Subject: [PATCH 07/19] [wip] --- .../ensogl/lib/text/src/component/area.rs | 25 ++++++--- src/rust/ide/lib/ast/impl/src/macros.rs | 7 +-- src/rust/ide/src/controller/graph.rs | 2 +- .../ide/src/double_representation/comment.rs | 53 +++++++++++++++++++ .../view/graph-editor/src/component/node.rs | 8 ++- 5 files changed, 82 insertions(+), 13 deletions(-) diff --git a/src/rust/ensogl/lib/text/src/component/area.rs b/src/rust/ensogl/lib/text/src/component/area.rs index 85feb1854b..c25786bb17 100644 --- a/src/rust/ensogl/lib/text/src/component/area.rs +++ b/src/rust/ensogl/lib/text/src/component/area.rs @@ -128,7 +128,7 @@ struct Lines { impl Lines { /// The number of visible lines. - pub fn count(&self) -> usize { + pub fn len(&self) -> usize { self.rc.borrow().len() } @@ -260,6 +260,7 @@ ensogl_core::define_endpoints! { Output { pointer_style (cursor::Style), width (f32), + height (f32), changed (Vec), content (Text), hovered (bool), @@ -598,7 +599,7 @@ impl AreaModel { let id = sel.id; let start_line = sel.start.line.as_usize(); let end_line = sel.end.line.as_usize(); - let pos_x = |line:usize, column:Column| if line >= self.lines.count() { + let pos_x = |line:usize, column:Column| if line >= self.lines.len() { self.lines.rc.borrow().last().and_then(|l| l.divs.last().cloned()).unwrap_or(0.0) } else { self.lines.rc.borrow()[line].div_by_column(column) @@ -675,7 +676,7 @@ impl AreaModel { fn get_in_text_location(&self, screen_pos:Vector2) -> Location { let object_space = self.to_object_space(screen_pos); let line_index = (-object_space.y / LINE_HEIGHT) as usize; - let line_index = std::cmp::min(line_index,self.lines.count() - 1); + let line_index = std::cmp::min(line_index,self.lines.len() - 1); let div_index = self.lines.rc.borrow()[line_index].div_index_close_to(object_space.x); let line = line_index.into(); let column = div_index.into(); @@ -688,19 +689,27 @@ impl AreaModel { } /// Redraw the text. - fn redraw(&self, width_may_change:bool) { + fn redraw(&self, size_may_change:bool) { let lines = self.buffer.view_lines(); let line_count = lines.len(); self.lines.resize_with(line_count,|ix| self.new_line(ix)); - let lengths = lines.into_iter().enumerate().map(|(view_line_index,content)|{ + let widths = lines.into_iter().enumerate().map(|(view_line_index,content)|{ self.redraw_line(view_line_index,content) }).collect_vec(); - let length = lengths.into_iter().max_by(|x,y|x.partial_cmp(y).unwrap()).unwrap_or_default(); - if width_may_change { - self.frp_endpoints.source.width.emit(length); + let width = widths.into_iter().max_by(|x, y|x.partial_cmp(y).unwrap()).unwrap_or_default(); + if size_may_change { + let height = self.calculate_height(); + self.frp_endpoints.source.width.emit(width); + self.frp_endpoints.source.height.emit(height); } } + fn calculate_height(&self) -> f32 { + let line_count = self.lines.len(); + let lines_height = line_count as f32 * LINE_HEIGHT; + lines_height + } + fn redraw_line(&self, view_line_index:usize, content:String) -> f32 { let cursor_map = self.selection_map.borrow() .location_map.get(&view_line_index).cloned().unwrap_or_default(); diff --git a/src/rust/ide/lib/ast/impl/src/macros.rs b/src/rust/ide/lib/ast/impl/src/macros.rs index fa3548f928..05bdc7e19e 100644 --- a/src/rust/ide/lib/ast/impl/src/macros.rs +++ b/src/rust/ide/lib/ast/impl/src/macros.rs @@ -92,8 +92,9 @@ impl DocCommentInfo { /// Get the documentation text. pub fn text(&self) -> String { - // This gets us documentation text, however non-first lines have indent whitespace - // maintained. + // This gets us documentation text, however non-first lines have the absolute indent + // whitespace preserved. Thus, we remove spurious indent, to keep only the relative indent + // to the comment's inner block (i.e. the right after the `##` introducer). let repr = self.body.repr(); let indent = self.block_indent + DOCUMENTATION_COMMENT_INTRODUCER.len(); let old = format!("\n{}", " ".repeat(indent)); @@ -102,7 +103,7 @@ impl DocCommentInfo { } /// Get the documentation text. - pub fn pretty_print_text(text:&str) -> String { + pub fn text_to_repr(text:&str) -> String { let mut lines = text.lines(); let first_line = lines.next().map(|line| iformat!("##{line}")); let other_lines = lines .map(|line| iformat!(" {line}")); diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index ad4b13b99a..c4641a5881 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -753,7 +753,7 @@ impl Handle { let expression_ast = self.parse_node_expression(&node.expression)?; let main_line = MainLine::from_ast(&expression_ast).ok_or(FailedToCreateNode)?; let documentation = node.doc_comment.as_ref() - .map(|text| DocCommentInfo::pretty_print_text(&text)) + .map(|text| DocCommentInfo::text_to_repr(&text)) .map(|doc_code| self.parser.parse_line(doc_code)) .transpose()? .map(|doc_ast| DocCommentInfo::new(&doc_ast).ok_or(FailedToCreateNode)) diff --git a/src/rust/ide/src/double_representation/comment.rs b/src/rust/ide/src/double_representation/comment.rs index 7130f21d14..febb6e114e 100644 --- a/src/rust/ide/src/double_representation/comment.rs +++ b/src/rust/ide/src/double_representation/comment.rs @@ -8,8 +8,61 @@ mod tests { use ast::Shape; use crate::double_representation::definition::DefinitionProvider; use ast::macros::DocCommentInfo; + use parser::Parser; + /// Expect `main` method, where first line is a documentation comment. + /// The text of this comment should match the expected one. + fn run_case(parser:&Parser, code:&str, expected_comment_text:&str) { + let ast = parser.parse_module(code,default()).unwrap(); + let main_id = double_representation::definition::Id::new_plain_name("main"); + let module_info = double_representation::module::Info {ast:ast.clone_ref()}; + + let main = double_representation::module::get_definition(&ast,&main_id).unwrap(); + let lines = main.block_lines(); + let first_line_ast = lines[0].elem.as_ref().unwrap(); + let doc = DocCommentInfo::new_indented(first_line_ast,4).unwrap(); + assert_eq!(doc.text(), expected_comment_text); + } + + #[test] + fn parse_single_line_comment() { + let parser = parser::Parser::new_or_panic(); + + // Typical single line case. + let code = r#" +main = + ## Single line + node"#; + let expected = " Single line"; + run_case(&parser, code,expected); + + // Single line case without space after `##`. + let code = r#" +main = + ##Single line + node"#; + let expected = "Single line"; + run_case(&parser, code,expected); + + // Single line case with a single space after `##`. + let code = r#" +main = + ## + node"#; + let expected = " "; + run_case(&parser, code,expected); + + // Single line case without content. + let code = r#" +main = + + ## + node"#; + let expected = ""; + run_case(&parser, code,expected); + + } #[test] fn parse_multi_line_comment() { diff --git a/src/rust/ide/view/graph-editor/src/component/node.rs b/src/rust/ide/view/graph-editor/src/component/node.rs index b1b6fedaa3..964c42bd10 100644 --- a/src/rust/ide/view/graph-editor/src/component/node.rs +++ b/src/rust/ide/view/graph-editor/src/component/node.rs @@ -56,6 +56,9 @@ pub const HEIGHT : f32 = 28.0; pub const PADDING : f32 = 40.0; pub const RADIUS : f32 = 14.0; +/// Space between the documentation comment and the node. +pub const COMMENT_MARGIN : f32 = 14.0; + const INFINITE : f32 = 99999.0; const ERROR_VISUALIZATION_SIZE : (f32,f32) = visualization::container::DEFAULT_SIZE; @@ -470,7 +473,6 @@ impl NodeModel { // TODO: Style the documentation comment properly. let comment = ensogl_text::Area::new(app); - comment.set_position_y(HEIGHT * 1.5); display_object.add_child(&comment); let app = app.clone_ref(); @@ -627,6 +629,10 @@ impl Node { // === Comment === + eval model.comment.width ([model](width) + model.comment.set_position_x(-*width - COMMENT_MARGIN)); + eval model.comment.height ([model](height) + model.comment.set_position_y(*height / 2.0)); model.comment.set_content <+ frp.set_comment; out.source.expression <+ model.comment.content; From 1cfc88704d70bf48f3af679dbde693f88fca68ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Sat, 31 Jul 2021 14:32:56 +0200 Subject: [PATCH 08/19] post merge fixes, warnings --- .../alias_analysis/test_utils.rs | 1 - src/rust/ide/src/double_representation/comment.rs | 5 +---- src/rust/ide/src/double_representation/connection.rs | 3 +-- src/rust/ide/src/double_representation/node.rs | 12 +++--------- .../double_representation/refactorings/collapse.rs | 4 ---- 5 files changed, 5 insertions(+), 20 deletions(-) diff --git a/src/rust/ide/src/double_representation/alias_analysis/test_utils.rs b/src/rust/ide/src/double_representation/alias_analysis/test_utils.rs index 78ea4f3633..e886bed445 100644 --- a/src/rust/ide/src/double_representation/alias_analysis/test_utils.rs +++ b/src/rust/ide/src/double_representation/alias_analysis/test_utils.rs @@ -4,7 +4,6 @@ use crate::prelude::*; use crate::double_representation::identifier::NormalizedName; use crate::double_representation::identifier::LocatedName; -use crate::double_representation::node::NodeInfo; use crate::double_representation::test_utils::MarkdownProcessor; use regex::Captures; diff --git a/src/rust/ide/src/double_representation/comment.rs b/src/rust/ide/src/double_representation/comment.rs index febb6e114e..847d34c590 100644 --- a/src/rust/ide/src/double_representation/comment.rs +++ b/src/rust/ide/src/double_representation/comment.rs @@ -5,7 +5,6 @@ use crate::prelude::*; #[cfg(test)] mod tests { use super::*; - use ast::Shape; use crate::double_representation::definition::DefinitionProvider; use ast::macros::DocCommentInfo; use parser::Parser; @@ -16,8 +15,6 @@ mod tests { fn run_case(parser:&Parser, code:&str, expected_comment_text:&str) { let ast = parser.parse_module(code,default()).unwrap(); let main_id = double_representation::definition::Id::new_plain_name("main"); - let module_info = double_representation::module::Info {ast:ast.clone_ref()}; - let main = double_representation::module::get_definition(&ast,&main_id).unwrap(); let lines = main.block_lines(); let first_line_ast = lines[0].elem.as_ref().unwrap(); @@ -48,7 +45,7 @@ main = // Single line case with a single space after `##`. let code = r#" main = - ## + ## node"#; let expected = " "; run_case(&parser, code,expected); diff --git a/src/rust/ide/src/double_representation/connection.rs b/src/rust/ide/src/double_representation/connection.rs index 08e2ab79e3..c5bd62a8d9 100644 --- a/src/rust/ide/src/double_representation/connection.rs +++ b/src/rust/ide/src/double_representation/connection.rs @@ -43,8 +43,7 @@ impl Endpoint { let definition = DefinitionInfo::from_line_ast(line_ast,ScopeKind::NonRoot,block.indent); let is_non_def = definition.is_none(); let node = is_non_def.and_option_from(|| MainLine::from_ast(line_ast))?.id(); - let node = is_non_def.and_option_from(|| NodeInfo::from_line_ast(line_ast))?.id(); - Some(Endpoint { node, crumbs }) + Some(Endpoint {node,crumbs}) } } diff --git a/src/rust/ide/src/double_representation/node.rs b/src/rust/ide/src/double_representation/node.rs index a5d0c59c36..329af666e1 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -102,12 +102,12 @@ pub fn locate_many<'a> // Skip empty lines, while preserving indices. let lines_iter = lines.into_iter() .enumerate() - .filter_map(|(index,line)| line.elem.as_ref().map(|ast| (index,ast))); + .filter_map(|(index, line)| line.elem.as_ref().map(|ast| (index, ast))); let mut looked_for = looked_for.into_iter().collect::>(); let mut ret = HashMap::new(); - let nodes = NodeIterator {lines_iter}; + let nodes = NodeIterator { lines_iter }; for node in nodes { if looked_for.remove(&node.id()) { ret.insert(node.id(), node); @@ -119,16 +119,10 @@ pub fn locate_many<'a> }; if let Some(id) = looked_for.into_iter().next() { - Err(IdNotFound{id}.into()) + Err(IdNotFound {id}.into()) } else { Ok(ret) } - -/// Searches for `NodeInfo` with the associated `id` index in `lines`. Returns an error if -/// the Id is not found. -pub fn index_in_lines(lines:&[ast::BlockLine>], id:ast::Id) -> FallibleResult { - let position = lines.iter().position(|line| is_node_by_id(line,id)); - position.ok_or_else(|| IdNotFound{id}.into()) } diff --git a/src/rust/ide/src/double_representation/refactorings/collapse.rs b/src/rust/ide/src/double_representation/refactorings/collapse.rs index 26d136c607..46cad46ebb 100644 --- a/src/rust/ide/src/double_representation/refactorings/collapse.rs +++ b/src/rust/ide/src/double_representation/refactorings/collapse.rs @@ -350,10 +350,6 @@ impl Collapser { documentation : None, main_line : expression, }; - } else if node_id == self.replaced_node { - let expression = self.call_to_extracted(extracted_definition)?; - let no_node_err = failure::Error::from(CannotConstructCollapsedNode); - let mut new_node = NodeInfo::new_expression(expression.clone_ref()).ok_or(no_node_err)?; new_node.set_id(self.collapsed_node); if let Some(Output{identifier,..}) = &self.extracted.output { new_node.set_pattern(identifier.with_new_id().into()) From 679c18e1a689003759672fb2ff45c586d83ee917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Sat, 31 Jul 2021 16:24:12 +0200 Subject: [PATCH 09/19] cleanups, CR feedback, minor improvements --- .../ensogl/lib/text/src/component/area.rs | 4 +- src/rust/ide/lib/ast/impl/src/macros.rs | 2 +- src/rust/ide/src/controller/graph.rs | 159 +++++++++--------- .../ide/src/controller/searcher/action.rs | 2 +- src/rust/ide/src/double_representation.rs | 16 +- .../src/double_representation/definition.rs | 5 +- .../ide/src/double_representation/graph.rs | 14 +- .../ide/src/double_representation/node.rs | 20 ++- .../ide/src/ide/integration/file_system.rs | 8 +- 9 files changed, 117 insertions(+), 113 deletions(-) diff --git a/src/rust/ensogl/lib/text/src/component/area.rs b/src/rust/ensogl/lib/text/src/component/area.rs index e0f5026f2c..5c84751cdd 100644 --- a/src/rust/ensogl/lib/text/src/component/area.rs +++ b/src/rust/ensogl/lib/text/src/component/area.rs @@ -707,9 +707,7 @@ impl AreaModel { } fn calculate_height(&self) -> f32 { - let line_count = self.lines.len(); - let lines_height = line_count as f32 * LINE_HEIGHT; - lines_height + self.lines.len() as f32 * LINE_HEIGHT } fn redraw_line(&self, view_line_index:usize, content:String) -> f32 { diff --git a/src/rust/ide/lib/ast/impl/src/macros.rs b/src/rust/ide/lib/ast/impl/src/macros.rs index 80dd755986..1cfa58a8a7 100644 --- a/src/rust/ide/lib/ast/impl/src/macros.rs +++ b/src/rust/ide/lib/ast/impl/src/macros.rs @@ -42,7 +42,7 @@ pub const QUALIFIED_EXPORT_KEYWORD:&str = "export"; /// Try Interpreting the line as disabling comment. Return the text after `#`. pub fn as_disable_comment(ast:&Ast) -> Option { - let r#match = crate::known::Match::try_from(ast).ok()?; + let r#match = crate::known::Match::try_from(ast).ok()?; let first_segment = &r#match.segs.head; if crate::identifier::name(&first_segment.head) == Some(DISABLING_COMMENT_INTRODUCER) { Some(first_segment.body.repr()) diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index ccf048b1f6..ff6981ffdc 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -15,7 +15,7 @@ use crate::double_representation::identifier::generate_name; use crate::double_representation::module; use crate::double_representation::node; use crate::double_representation::node::MainLine; -use crate::double_representation::node::NodeIndex; +use crate::double_representation::node::NodeLocation; use crate::double_representation::node::NodeInfo; use crate::model::module::NodeMetadata; use crate::model::traits::*; @@ -664,7 +664,7 @@ impl Handle { false } }; - let range = NodeIndex::range(node_to_be_after.index,node_to_be_before.index); + let range = NodeLocation::range(node_to_be_after.index, node_to_be_before.index); lines[range].sort_by_key(should_be_at_end); self.update_definition_ast(|mut def| { def.set_block_lines(lines)?; @@ -1294,83 +1294,84 @@ main = }) } -// #[test] -// fn graph_controller_node_operations_node() { -// let mut test = Fixture::set_up(); -// const PROGRAM:&str = r" -// main = -// foo = 2 -// print foo"; -// test.data.code = PROGRAM.into(); -// test.run(|graph| async move { -// // === Initial nodes === -// let nodes = graph.nodes().unwrap(); -// let (node1,node2) = nodes.expect_tuple(); -// assert_eq!(node1.info.expression().repr(), "2"); -// assert_eq!(node2.info.expression().repr(), "print foo"); -// -// -// // === Add node === -// let id = ast::Id::new_v4(); -// let position = Some(model::module::Position::new(10.0,20.0)); -// let metadata = NodeMetadata {position,..default()}; -// let info = NewNodeInfo { -// expression : "a+b".into(), -// metadata : Some(metadata), -// id : Some(id), -// location_hint : LocationHint::End, -// introduce_pattern : false, -// }; -// graph.add_node(info.clone()).unwrap(); -// let expected_program = r" -// main = -// foo = 2 -// print foo -// a+b"; -// -// model::module::test::expect_code(&*graph.module,expected_program); -// let nodes = graph.nodes().unwrap(); -// let (_,_,node3) = nodes.expect_tuple(); -// assert_eq!(node3.info.id(),id); -// assert_eq!(node3.info.expression().repr(), "a+b"); -// let pos = node3.metadata.unwrap().position; -// assert_eq!(pos, position); -// assert!(graph.module.node_metadata(id).is_ok()); -// -// -// // === Edit node === -// graph.set_expression(id, "bar baz").unwrap(); -// let (_,_,node3) = graph.nodes().unwrap().expect_tuple(); -// assert_eq!(node3.info.id(),id); -// assert_eq!(node3.info.expression().repr(), "bar baz"); -// assert_eq!(node3.metadata.unwrap().position, position); -// -// -// // === Remove node === -// graph.remove_node(node3.info.id()).unwrap(); -// let nodes = graph.nodes().unwrap(); -// let (node1,node2) = nodes.expect_tuple(); -// assert_eq!(node1.info.expression().repr(), "2"); -// assert_eq!(node2.info.expression().repr(), "print foo"); -// assert!(graph.module.node_metadata(id).is_err()); -// -// model::module::test::expect_code(&*graph.module, PROGRAM); -// -// -// // === Test adding node with automatically generated pattern === -// let info_w_pattern = NewNodeInfo { -// introduce_pattern : true, -// ..info -// }; -// graph.add_node(info_w_pattern).unwrap(); -// let expected_program = r" -// main = -// foo = 2 -// print foo -// sum1 = a+b"; -// model::module::test::expect_code(&*graph.module,expected_program); -// }) -// } + #[test] + fn graph_controller_node_operations_node() { + let mut test = Fixture::set_up(); + const PROGRAM:&str = r" +main = + foo = 2 + print foo"; + test.data.code = PROGRAM.into(); + test.run(|graph| async move { + // === Initial nodes === + let nodes = graph.nodes().unwrap(); + let (node1,node2) = nodes.expect_tuple(); + assert_eq!(node1.info.expression().repr(), "2"); + assert_eq!(node2.info.expression().repr(), "print foo"); + + + // === Add node === + let id = ast::Id::new_v4(); + let position = Some(model::module::Position::new(10.0,20.0)); + let metadata = NodeMetadata {position,..default()}; + let info = NewNodeInfo { + expression : "a+b".into(), + doc_comment : None, + metadata : Some(metadata), + id : Some(id), + location_hint : LocationHint::End, + introduce_pattern : false, + }; + graph.add_node(info.clone()).unwrap(); + let expected_program = r" +main = + foo = 2 + print foo + a+b"; + + model::module::test::expect_code(&*graph.module,expected_program); + let nodes = graph.nodes().unwrap(); + let (_,_,node3) = nodes.expect_tuple(); + assert_eq!(node3.info.id(),id); + assert_eq!(node3.info.expression().repr(), "a+b"); + let pos = node3.metadata.unwrap().position; + assert_eq!(pos, position); + assert!(graph.module.node_metadata(id).is_ok()); + + + // === Edit node === + graph.set_expression(id, "bar baz").unwrap(); + let (_,_,node3) = graph.nodes().unwrap().expect_tuple(); + assert_eq!(node3.info.id(),id); + assert_eq!(node3.info.expression().repr(), "bar baz"); + assert_eq!(node3.metadata.unwrap().position, position); + + + // === Remove node === + graph.remove_node(node3.info.id()).unwrap(); + let nodes = graph.nodes().unwrap(); + let (node1,node2) = nodes.expect_tuple(); + assert_eq!(node1.info.expression().repr(), "2"); + assert_eq!(node2.info.expression().repr(), "print foo"); + assert!(graph.module.node_metadata(id).is_err()); + + model::module::test::expect_code(&*graph.module, PROGRAM); + + + // === Test adding node with automatically generated pattern === + let info_w_pattern = NewNodeInfo { + introduce_pattern : true, + ..info + }; + graph.add_node(info_w_pattern).unwrap(); + let expected_program = r" +main = + foo = 2 + print foo + sum1 = a+b"; + model::module::test::expect_code(&*graph.module,expected_program); + }) + } #[wasm_bindgen_test] fn graph_controller_connections_listing() { diff --git a/src/rust/ide/src/controller/searcher/action.rs b/src/rust/ide/src/controller/searcher/action.rs index e4cef8275a..96627d5cd8 100644 --- a/src/rust/ide/src/controller/searcher/action.rs +++ b/src/rust/ide/src/controller/searcher/action.rs @@ -388,7 +388,7 @@ impl<'a> CategoryBuilder<'a> { let category = self.category_id; built_list.entries.borrow_mut().extend(iter.into_iter().map(|action| { let match_info = MatchInfo::Matches {subsequence:default()}; - ListEntry{action,category,match_info} + ListEntry{category,match_info,action} })); } } diff --git a/src/rust/ide/src/double_representation.rs b/src/rust/ide/src/double_representation.rs index 5cbb2ea293..d446f9036d 100644 --- a/src/rust/ide/src/double_representation.rs +++ b/src/rust/ide/src/double_representation.rs @@ -26,6 +26,7 @@ pub mod tp; pub mod test_utils; + // ============== // === Consts === // ============== @@ -38,6 +39,12 @@ pub mod test_utils; /// Link: https://github.com/enso-org/enso/blob/main/doc/syntax/encoding.md pub const INDENT : usize = 4; + + +// ======================== +// === Discerning Lines === +// ======================== + /// What kind of node or definition a line should be treated as. #[derive(Clone,Debug)] pub enum LineKind { @@ -45,7 +52,8 @@ pub enum LineKind { Definition { /// Ast of the whole binding. ast : known::Infix, - /// Definition name. If this is an extension method, it might imply an implicit argument. + /// Definition name. If this is an extension method, it might imply an implicit `this` + /// argument. name : Located, /// Explicit arguments. Note that this might be empty when there are implicit arguments. args : Vec>, @@ -70,12 +78,12 @@ pub enum LineKind { impl LineKind { - /// Tell how the given line (described by Ast) should be treated. - /// Returns `None` if line does not match any of the expected entities. + /// Tell how the given line (described by an Ast) should be treated. pub fn discern(ast:&Ast, kind:ScopeKind) -> Self { use LineKind::*; - // First of all, if line is not an infix assignment, it can be only node or nothing. + // First of all, if non-empty line is not an infix (i.e. binding) it can be only a node or + // a documentation comment. let infix = match opr::to_assignment(ast) { Some(infix) => infix, diff --git a/src/rust/ide/src/double_representation/definition.rs b/src/rust/ide/src/double_representation/definition.rs index 3fb7de9e61..a43200a27a 100644 --- a/src/rust/ide/src/double_representation/definition.rs +++ b/src/rust/ide/src/double_representation/definition.rs @@ -367,10 +367,7 @@ impl<'a> DefinitionIterator<'a> { /// Looks up direct child definition by given name. pub fn find_by_name(mut self, name:&DefinitionName) -> Result { let err = || CannotFindChild(name.clone()); - self.find(|child_def| { - INFO!(&*child_def.item.name); - &*child_def.item.name == name - }).ok_or_else(err) + self.find(|child_def| &*child_def.item.name == name).ok_or_else(err) } } diff --git a/src/rust/ide/src/double_representation/graph.rs b/src/rust/ide/src/double_representation/graph.rs index dcecf9957c..96ca074420 100644 --- a/src/rust/ide/src/double_representation/graph.rs +++ b/src/rust/ide/src/double_representation/graph.rs @@ -60,10 +60,10 @@ impl GraphInfo { let body = ast.rarg.clone(); if let Ok(body_block) = known::Block::try_new(body.clone()) { block_nodes(&body_block) - } else if let Some(main_line) = node::MainLine::from_ast(&body) { - // There's no way to attach a documentation comment to an inline node. - let documentation = None; - vec![ NodeInfo {documentation,main_line} ] + } else if let Some(node) = node::NodeInfo::from_main_line_ast(&body) { + // There's no way to attach a documentation comment to an inline node, it consists only + // of the main line. + vec![node] } else { // It should not be possible to have empty definition without any nodes but it is // possible to represent such thing in AST. Anyway, it has no nodes. @@ -447,12 +447,6 @@ main = assert_eq!(nodes[2].ast().repr(), "node2"); assert_eq!(nodes[3].documentation_text(), None); assert_eq!(nodes[3].ast().repr(), "node3"); - - - for node in nodes.iter() { - DEBUG!(node.expression().repr()); - // assert_eq!(node.expression().repr(), "node"); - } assert_eq!(nodes.len(), 4); } diff --git a/src/rust/ide/src/double_representation/node.rs b/src/rust/ide/src/double_representation/node.rs index 329af666e1..d73d7956e4 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -25,21 +25,27 @@ pub type Id = ast::Id; pub struct IdNotFound {pub id:Id} /// Indices of lines belonging to a node. -#[derive(Clone,Copy,Debug,PartialEq)] -pub struct NodeIndex { +#[derive(Clone,Copy,Debug)] +pub struct NodeLocation { /// Documentation comment line index, if present. pub documentation_line : Option, /// Main line is a line that contains the node's expression. pub main_line : usize, } -impl PartialOrd for NodeIndex { +impl PartialEq for NodeLocation { + fn eq(&self, other: &Self) -> bool { + self.partial_cmp(other) == Some(Ordering::Equal) + } +} + +impl PartialOrd for NodeLocation { fn partial_cmp(&self, other: &Self) -> Option { self.main_line.partial_cmp(&other.main_line) } } -impl NodeIndex { +impl NodeLocation { /// Index for the first line belonging to the node. pub fn first(&self) -> usize { self.documentation_line.unwrap_or(self.main_line) @@ -54,7 +60,7 @@ impl NodeIndex { /// /// Note that while a node can contain at most two lines, they may be interspersed by a /// number of blank lines. - pub fn range(start:NodeIndex, last:NodeIndex) -> RangeInclusive { + pub fn range(start: NodeLocation, last: NodeLocation) -> RangeInclusive { start.first() ..= last.last() } } @@ -69,7 +75,7 @@ impl NodeIndex { #[derive(Clone,Debug,Shrinkwrap)] pub struct LocatedNode { /// Line index in the block. Zero for inline definition nodes. - pub index : NodeIndex, + pub index : NodeLocation, #[shrinkwrap(main_field)] /// Information about the node. pub node : NodeInfo, @@ -154,7 +160,7 @@ impl<'a, T:Iterator + 'a> Iterator for NodeIterator<'a, T let ret = LocatedNode { node : NodeInfo {documentation,main_line}, - index : NodeIndex { + index : NodeLocation { main_line : index, documentation_line, } diff --git a/src/rust/ide/src/ide/integration/file_system.rs b/src/rust/ide/src/ide/integration/file_system.rs index e86f3a5194..8de7b07d35 100644 --- a/src/rust/ide/src/ide/integration/file_system.rs +++ b/src/rust/ide/src/ide/integration/file_system.rs @@ -114,7 +114,7 @@ impl FolderContent for FileProvider { DirectoryView::new_from_root(connection,root.clone_ref()).into() } }; - Some(Entry {name,path,type_}) + Some(Entry {type_,name,path}) }); entries_loaded.emit(Rc::new(entries.sorted().collect_vec())); } @@ -162,13 +162,13 @@ impl DirectoryView { type_ : FolderType::Standard, content : sub.into() }; - Entry {name,path,type_} + Entry {type_,name,path} } FileSystemObject::File {name,path} | FileSystemObject::Other {name,path} => { let path = to_file_browser_path(&path).join(&name); let type_ = EntryType::File; - Entry {name,path,type_} + Entry {type_,name,path} } } }); @@ -272,4 +272,4 @@ pub async fn do_file_operation FileOperation::Move => json_rpc.move_file(&ls_source, &dest_full).await?, } Ok(()) -} \ No newline at end of file +} From 94cef3dbf36025550590bb27cc08f31581f71384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Sun, 1 Aug 2021 17:24:23 +0200 Subject: [PATCH 10/19] fixing multi line comments --- src/rust/ide/lib/ast/impl/src/crumbs.rs | 7 +- src/rust/ide/lib/ast/impl/src/lib.rs | 47 +++++++++++ src/rust/ide/lib/ast/impl/src/macros.rs | 54 +++++++++---- src/rust/ide/lib/parser/src/lib.rs | 25 ++++-- src/rust/ide/lib/parser/tests/bugs.rs | 2 +- src/rust/ide/lib/parser/tests/crumbs.rs | 4 +- src/rust/ide/lib/parser/tests/macros.rs | 8 +- src/rust/ide/lib/parser/tests/parsing.rs | 6 +- src/rust/ide/lib/span-tree/src/action.rs | 4 +- src/rust/ide/lib/span-tree/src/generate.rs | 26 +++--- src/rust/ide/src/controller/graph.rs | 44 +++++------ src/rust/ide/src/controller/project.rs | 2 +- src/rust/ide/src/controller/searcher.rs | 20 ++--- src/rust/ide/src/double_representation.rs | 28 +++---- .../double_representation/alias_analysis.rs | 2 +- .../ide/src/double_representation/comment.rs | 26 +++--- .../src/double_representation/definition.rs | 20 ++--- .../ide/src/double_representation/graph.rs | 67 +++++++--------- .../ide/src/double_representation/module.rs | 4 +- .../ide/src/double_representation/node.rs | 79 ++++++++++++++----- .../refactorings/collapse.rs | 1 - .../ide/view/src/debug_scenes/interface.rs | 6 +- 22 files changed, 296 insertions(+), 186 deletions(-) diff --git a/src/rust/ide/lib/ast/impl/src/crumbs.rs b/src/rust/ide/lib/ast/impl/src/crumbs.rs index 6ce7d9cb90..203edcfd1f 100644 --- a/src/rust/ide/lib/ast/impl/src/crumbs.rs +++ b/src/rust/ide/lib/ast/impl/src/crumbs.rs @@ -3,12 +3,13 @@ use crate::prelude::*; -use crate::ShiftedVec1; +use crate::enumerate_non_empty_lines; use crate::known; use crate::Shifted; use crate::MacroPatternMatch; use crate::HasTokens; use crate::Shape; +use crate::ShiftedVec1; use crate::TokenConsumer; use enso_data::text::Index; @@ -1414,9 +1415,7 @@ where for<'t> &'t Shape : TryInto<&'t T, Error=E>, pub fn non_empty_line_indices<'a, T:'a> (iter:impl Iterator>> + 'a) -> impl Iterator + 'a { - iter.enumerate().filter_map(|(line_index,line)| { - line.elem.as_ref().map(|_| line_index) - }) + enumerate_non_empty_lines(iter).map(|(index,_ast)| index) } diff --git a/src/rust/ide/lib/ast/impl/src/lib.rs b/src/rust/ide/lib/ast/impl/src/lib.rs index 9f23466dd1..222c44a30e 100644 --- a/src/rust/ide/lib/ast/impl/src/lib.rs +++ b/src/rust/ide/lib/ast/impl/src/lib.rs @@ -618,6 +618,26 @@ pub enum Escape { // === Block === // ============= + +impl Block { + pub fn indent(&self, parent_indent:usize) -> usize { + parent_indent + self.indent + } + + pub fn enumerate_non_empty_lines(&self) -> impl Iterator)> + '_ { + enumerate_non_empty_lines(&self.lines) + } +} + +pub fn enumerate_non_empty_lines<'a,T:'a>(iter:impl IntoIterator>> + 'a) +-> impl Iterator)> + 'a { + iter.into_iter().enumerate().filter_map(|(index,line):(usize,&BlockLine>)| { + let non_empty_line = line.transpose_ref()?; + Some((index, non_empty_line)) + }) +} + + #[ast_node] pub enum BlockType {Continuous {} , Discontinuous {}} /// Holder for line in `Block` or `Module`. Lines store value of `T` and trailing whitespace info. @@ -629,6 +649,33 @@ pub struct BlockLine { pub off: usize } +impl BlockLine { + pub fn as_ref(&self) -> BlockLine<&T> { + BlockLine { + elem : &self.elem, + off : self.off, + } + } + + pub fn map(self, f:impl FnOnce(T) -> U) -> BlockLine { + BlockLine { + elem : f(self.elem), + off : self.off + } + } +} + +impl BlockLine> { + pub fn transpose(self) -> Option> { + let off = self.off; + self.elem.map(|elem| BlockLine {elem,off}) + } + + pub fn transpose_ref(&self) -> Option> { + self.as_ref().map(Option::as_ref).transpose() + } +} + // ============= diff --git a/src/rust/ide/lib/ast/impl/src/macros.rs b/src/rust/ide/lib/ast/impl/src/macros.rs index 1cfa58a8a7..ff169815db 100644 --- a/src/rust/ide/lib/ast/impl/src/macros.rs +++ b/src/rust/ide/lib/ast/impl/src/macros.rs @@ -7,7 +7,7 @@ use crate::prelude::*; use crate::crumbs::AmbiguousCrumb; use crate::crumbs::Located; use crate::crumbs::MatchCrumb; -use crate::known; +use crate::{known, BlockLine}; use crate::Shifted; @@ -59,35 +59,52 @@ pub fn is_disable_comment(ast:&Ast) -> bool { // === Documentation Comments === +/// Information about AST context necessary to construct `DocCommentInfo` from line's AST. +pub struct DocumentationContext { + /// Absolute indentation of the documentation comment line. + pub block_indent : usize, + /// Trailing whitespace in the documentation line. + pub trailing_whitespace : usize, +} + /// Ast known to be a documentation comment. #[derive(Clone,Debug)] pub struct DocCommentInfo { - ast : known::Match, + line : BlockLine, body : crate::MacroPatternMatch>, /// The absolute indent of the block that contains the line with documentation comment. pub block_indent : usize, } impl DocCommentInfo { - /// Try constructing from AST, return None if this is not a documentation comment. - pub fn new(ast:&Ast) -> Option { - Self::new_indented(ast,0) + pub fn from_ast(ast:&Ast) -> Option Self> { + let ast = crate::known::Match::try_from(ast).ok()?; + let first_segment = &ast.segs.head; + let introducer = crate::identifier::name(&first_segment.head)?; + if introducer == DOCUMENTATION_COMMENT_INTRODUCER { + let body = first_segment.body.clone_ref(); + Some(move |DocumentationContext{block_indent,trailing_whitespace}| { + let line = BlockLine { + elem : ast, + off : trailing_whitespace, + }; + DocCommentInfo {line,body,block_indent} + }) + } else { + None + } } /// Creates a documentation from Ast and information about indentation of the block it belongs /// to. - pub fn new_indented(ast:&Ast, block_indent:usize) -> Option { - let ast = crate::known::Match::try_from(ast).ok()?; - let first_segment = &ast.segs.head; - let introducer = crate::identifier::name(&first_segment.head)?; - let introducer_matches = introducer == DOCUMENTATION_COMMENT_INTRODUCER; - let body = first_segment.body.clone_ref(); - introducer_matches.then(|| DocCommentInfo {ast,body,block_indent}) + pub fn new(line:&BlockLine<&Ast>, block_indent:usize) -> Option { + let context = DocumentationContext{block_indent,trailing_whitespace:line.off}; + Self::from_ast(line.elem).map(|f| f(context)) } /// Get the documentation comment's AST. pub fn ast(&self) -> known::Match { - self.ast.clone_ref() + self.line.elem.clone_ref() } /// Get the documentation text. @@ -95,7 +112,8 @@ impl DocCommentInfo { // This gets us documentation text, however non-first lines have the absolute indent // whitespace preserved. Thus, we remove spurious indent, to keep only the relative indent // to the comment's inner block (i.e. the right after the `##` introducer). - let repr = self.body.repr(); + let mut repr = self.body.repr(); + repr.extend(std::iter::repeat(' ').take(self.line.off)); // trailing whitespace let indent = self.block_indent + DOCUMENTATION_COMMENT_INTRODUCER.len(); let old = format!("\n{}", " ".repeat(indent)); let new = "\n"; @@ -110,11 +128,15 @@ impl DocCommentInfo { let mut out_lines = first_line.into_iter().chain(other_lines); out_lines.join("\n") } + + pub fn line(&self) -> &BlockLine { + &self.line + } } impl AsRef for DocCommentInfo { fn as_ref(&self) -> &Ast { - self.ast.ast() + self.line.elem.ast() } } @@ -126,7 +148,7 @@ impl Display for DocCommentInfo { /// Check if given Ast stores a documentation comment. pub fn is_documentation_comment(ast:&Ast) -> bool { - DocCommentInfo::new(ast).is_some() + DocCommentInfo::from_ast(ast).is_some() } diff --git a/src/rust/ide/lib/parser/src/lib.rs b/src/rust/ide/lib/parser/src/lib.rs index 5e7186a0ed..c1df2e002e 100644 --- a/src/rust/ide/lib/parser/src/lib.rs +++ b/src/rust/ide/lib/parser/src/lib.rs @@ -21,7 +21,7 @@ mod wsclient; use crate::prelude::*; -use ast::Ast; +use ast::{Ast, BlockLine}; use ast::IdMap; use std::panic; use utils::fail::FallibleResult; @@ -101,16 +101,27 @@ impl Parser { /// Program is expected to be single non-empty line module. The line's AST is /// returned. Panics otherwise. The program is parsed with empty IdMap. - pub fn parse_line(&self, program:impl Str) -> FallibleResult { - self.parse_line_with_id_map(program,default()) + pub fn parse_line_ast(&self, program:impl Str) -> FallibleResult { + self.parse_line_with_id_map(program, default()).map(|line| line.elem) } - /// Program is expected to be single non-empty line module. The line's AST is returned. Panics - /// otherwise. - pub fn parse_line_with_id_map(&self, program:impl Str, id_map:IdMap) -> FallibleResult { + + /// Program is expected to be single non-empty line module. The line's AST is + /// returned. Panics otherwise. The program is parsed with empty IdMap. + pub fn parse_line(&self, program:impl Str) -> FallibleResult> { + self.parse_line_with_id_map(program, default()) + } + + /// Program is expected to be single non-empty line module. The line's AST is returned. + pub fn parse_line_ast_with_id_map(&self, program:impl Str, id_map:IdMap) -> FallibleResult { + self.parse_line_with_id_map(program,id_map).map(|line| line.elem) + } + + /// Program is expected to be single non-empty line module. Return the parsed line. + pub fn parse_line_with_id_map(&self, program:impl Str, id_map:IdMap) -> FallibleResult> { let module = self.parse_module(program,id_map)?; let mut lines = module.lines.clone().into_iter().filter_map(|line| { - line.elem + line.map(|elem| elem).transpose() }); if let Some(first_non_empty_line) = lines.next() { if lines.next().is_some() { diff --git a/src/rust/ide/lib/parser/tests/bugs.rs b/src/rust/ide/lib/parser/tests/bugs.rs index c8ca871ffd..2289014dc7 100644 --- a/src/rust/ide/lib/parser/tests/bugs.rs +++ b/src/rust/ide/lib/parser/tests/bugs.rs @@ -17,7 +17,7 @@ fn no_doc_found() { #[wasm_bindgen_test] fn extension_operator_methods() { - let ast = parser::Parser::new_or_panic().parse_line("Int.+").unwrap(); + let ast = parser::Parser::new_or_panic().parse_line_ast("Int.+").unwrap(); use ast::*; if let Shape::Infix(Infix {larg:_larg,loff:_loff,opr,roff:_roff,rarg}, ..) = ast.shape() { diff --git a/src/rust/ide/lib/parser/tests/crumbs.rs b/src/rust/ide/lib/parser/tests/crumbs.rs index 66a6d69166..9db150e7d5 100644 --- a/src/rust/ide/lib/parser/tests/crumbs.rs +++ b/src/rust/ide/lib/parser/tests/crumbs.rs @@ -15,14 +15,14 @@ wasm_bindgen_test_configure!(run_in_browser); #[wasm_bindgen_test] fn macro_crumb_test() { - let ast = Parser::new_or_panic().parse_line("foo -> bar").unwrap(); + let ast = Parser::new_or_panic().parse_line_ast("foo -> bar").unwrap(); let crumbs = ast.iter_subcrumbs().collect_vec(); assert_eq!(ast.get(&crumbs[0]).unwrap().repr(), "foo"); assert_eq!(ast.get(&crumbs[1]).unwrap().repr(), "->"); assert_eq!(ast.get(&crumbs[2]).unwrap().repr(), "bar"); - let ast = Parser::new_or_panic().parse_line("( foo bar )").unwrap(); + let ast = Parser::new_or_panic().parse_line_ast("( foo bar )").unwrap(); let crumbs = ast.iter_subcrumbs().collect_vec(); assert_eq!(ast.get(&crumbs[0]).unwrap().repr(), "("); diff --git a/src/rust/ide/lib/parser/tests/macros.rs b/src/rust/ide/lib/parser/tests/macros.rs index f164843017..aa2caf87f2 100644 --- a/src/rust/ide/lib/parser/tests/macros.rs +++ b/src/rust/ide/lib/parser/tests/macros.rs @@ -14,7 +14,7 @@ fn import_utilities() { let parser = Parser::new_or_panic(); let expect_import = |code:&str| { - let ast = parser.parse_line(code).unwrap(); + let ast = parser.parse_line_ast(code).unwrap(); assert!(is_ast_import(&ast), "Not Ast import: {:?}", ast); let ast_match = ast_as_import_match(&ast).unwrap(); assert_eq!(&ast,ast_match.ast()); @@ -22,7 +22,7 @@ fn import_utilities() { }; let expect_not_import = |code:&str| { - let ast = parser.parse_line(code).unwrap(); + let ast = parser.parse_line_ast(code).unwrap(); assert!(!is_ast_import(&ast)); assert!(ast_as_import_match(&ast).is_none()); }; @@ -52,7 +52,7 @@ fn recognizing_lambdas() { let parser = Parser::new_or_panic(); let expect_lambda = |code:&str, arg:&str, body:&str| { - let ast = parser.parse_line(code).unwrap(); + let ast = parser.parse_line_ast(code).unwrap(); let lambda = ast::macros::as_lambda(&ast).expect("failed to recognize lambda"); assert_eq!(lambda.arg.repr(), arg); assert_eq!(lambda.body.repr(), body); @@ -60,7 +60,7 @@ fn recognizing_lambdas() { assert_eq!(*lambda.body, ast.get_traversing(&lambda.body.crumbs).unwrap()); }; let expect_not_lambda = |code:&str| { - let ast = parser.parse_line(code).unwrap(); + let ast = parser.parse_line_ast(code).unwrap(); assert!(ast::macros::as_lambda_match(&ast).is_none(), "wrongly recognized a lambda"); }; diff --git a/src/rust/ide/lib/parser/tests/parsing.rs b/src/rust/ide/lib/parser/tests/parsing.rs index 94aa77ca09..a010fdc86c 100644 --- a/src/rust/ide/lib/parser/tests/parsing.rs +++ b/src/rust/ide/lib/parser/tests/parsing.rs @@ -83,7 +83,7 @@ impl Fixture { fn test_shape(&mut self, program:&str, tester:F) where for<'t> &'t Shape: TryInto<&'t T>, F : FnOnce(&T) -> () { - let ast = self.parser.parse_line(program).unwrap(); + let ast = self.parser.parse_line_ast(program).unwrap(); let shape = expect_shape(&ast); tester(shape); } @@ -115,7 +115,7 @@ impl Fixture { #[allow(dead_code)] // TODO [mwu] https://github.com/enso-org/enso/issues/1016 fn deserialize_unexpected(&mut self) { let unexpected = "import"; - let ast = self.parser.parse_line(unexpected).unwrap(); + let ast = self.parser.parse_line_ast(unexpected).unwrap(); // This does not deserialize to "Unexpected" but to a very complex macro match tree that has // Unexpected somewhere within. We just make sure that it is somewhere, and that confirms // that we are able to deserialize such node. @@ -416,7 +416,7 @@ impl Fixture { ]; for macro_usage in macro_usages.iter() { - let ast = self.parser.parse_line(*macro_usage).unwrap(); + let ast = self.parser.parse_line_ast(*macro_usage).unwrap(); expect_shape::>(&ast); }; } diff --git a/src/rust/ide/lib/span-tree/src/action.rs b/src/rust/ide/lib/span-tree/src/action.rs index 73da9c6dd6..21aaaaa0a1 100644 --- a/src/rust/ide/lib/span-tree/src/action.rs +++ b/src/rust/ide/lib/span-tree/src/action.rs @@ -243,7 +243,7 @@ mod test { impl Case { fn run(&self, parser:&Parser) { - let ast = parser.parse_line(self.expr).unwrap(); + let ast = parser.parse_line_ast(self.expr).unwrap(); let ast_id = ast.id; let tree = ast.generate_tree(&context::Empty).unwrap() : SpanTree; let span_begin = Index::new(self.span.start); @@ -327,7 +327,7 @@ mod test { impl Case { fn run(&self, parser:&Parser) { - let ast = parser.parse_line(self.expr).unwrap(); + let ast = parser.parse_line_ast(self.expr).unwrap(); let tree : SpanTree = ast.generate_tree(&context::Empty).unwrap(); let span_begin = Index::new(self.span.start); let span_end = Index::new(self.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 c1e633dde6..31672042ee 100644 --- a/src/rust/ide/lib/span-tree/src/generate.rs +++ b/src/rust/ide/lib/span-tree/src/generate.rs @@ -603,7 +603,7 @@ mod test { id_map.generate(12..13); 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 ast = parser.parse_line_ast_with_id_map("2 + foo bar - 3", id_map.clone()).unwrap(); let mut tree = ast.generate_tree(&context::Empty).unwrap() : SpanTree; // Check the expression ids we defined: @@ -643,7 +643,7 @@ mod test { #[wasm_bindgen_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 ast = parser.parse_line_ast("2 + 3 + foo bar baz 13 + 5").unwrap(); let mut tree = ast.generate_tree(&context::Empty).unwrap() : SpanTree; clear_expression_ids(&mut tree.root); @@ -685,7 +685,7 @@ mod test { #[wasm_bindgen_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 ast = parser.parse_line_ast("1,2,3").unwrap(); let mut tree = ast.generate_tree(&context::Empty).unwrap() : SpanTree; clear_expression_ids(&mut tree.root); @@ -711,7 +711,7 @@ mod test { let parser = Parser::new_or_panic(); // 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 ast = parser.parse_line_ast("+ * + + 2 +").unwrap(); let mut tree = ast.generate_tree(&context::Empty).unwrap() : SpanTree; clear_expression_ids(&mut tree.root); @@ -745,7 +745,7 @@ mod test { #[wasm_bindgen_test] fn generating_span_tree_from_right_assoc_section() { let parser = Parser::new_or_panic(); - let ast = parser.parse_line(",2,").unwrap(); + let ast = parser.parse_line_ast(",2,").unwrap(); let mut tree = ast.generate_tree(&context::Empty).unwrap() : SpanTree; clear_expression_ids(&mut tree.root); @@ -771,7 +771,7 @@ mod test { let mut id_map = IdMap::default(); 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 ast = parser.parse_line_ast_with_id_map(expression, id_map.clone()).unwrap(); let mut tree = ast.generate_tree(&context::Empty).unwrap() : SpanTree; // Check if expression id is set @@ -821,7 +821,7 @@ mod test { let parser = Parser::new_or_panic(); let expression = "[a,b]"; - let ast = parser.parse_line(expression).unwrap(); + let ast = parser.parse_line_ast(expression).unwrap(); let mut tree = ast.generate_tree(&context::Empty).unwrap() : SpanTree; // Check the other fields @@ -849,7 +849,7 @@ mod test { let parser = Parser::new_or_panic(); 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 ast = parser.parse_line_ast_with_id_map("(4", id_map.clone()).unwrap(); let mut tree = ast.generate_tree(&context::Empty).unwrap() : SpanTree; // Check the expression id: @@ -871,7 +871,7 @@ mod test { #[wasm_bindgen_test] fn generating_span_tree_for_lambda() { let parser = Parser::new_or_panic(); - let ast = parser.parse_line("foo a-> b + c").unwrap(); + let ast = parser.parse_line_ast("foo a-> b + c").unwrap(); let mut tree = ast.generate_tree(&context::Empty).unwrap() : SpanTree; clear_expression_ids(&mut tree.root); @@ -904,7 +904,7 @@ mod test { // === Single function name === - let ast = parser.parse_line("foo").unwrap(); + let ast = parser.parse_line_ast("foo").unwrap(); let invocation_info = CalledMethodInfo { parameters : vec![this_param.clone()] }; @@ -925,7 +925,7 @@ mod test { // === Complete application chain === - let ast = parser.parse_line("foo here").unwrap(); + let ast = parser.parse_line_ast("foo here").unwrap(); let invocation_info = CalledMethodInfo { parameters : vec![this_param.clone()] }; @@ -946,7 +946,7 @@ mod test { // === Partial application chain === - let ast = parser.parse_line("foo here").unwrap(); + let ast = parser.parse_line_ast("foo here").unwrap(); let invocation_info = CalledMethodInfo { parameters : vec![this_param.clone(), param1.clone(), param2.clone()] }; @@ -977,7 +977,7 @@ mod test { // === Partial application chain - this argument === - let ast = parser.parse_line("here.foo").unwrap(); + let ast = parser.parse_line_ast("here.foo").unwrap(); let invocation_info = CalledMethodInfo { parameters : vec![this_param.clone(), param1.clone(), param2.clone()] }; diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index ff6981ffdc..2501ac694c 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -8,6 +8,7 @@ use crate::prelude::*; use crate::double_representation::connection; use crate::double_representation::definition; +use crate::double_representation::definition::DefinitionProvider; use crate::double_representation::graph::GraphInfo; use crate::double_representation::identifier::LocatedName; use crate::double_representation::identifier::NormalizedName; @@ -476,7 +477,7 @@ impl Handle { ) -> 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()?; + let _ = ret.definition()?; Ok(ret) } @@ -494,15 +495,9 @@ impl Handle { Self::new(parent,module,project.suggestion_db(),project.parser(),definition) } - /// Retrieves double rep information about definition providing this graph. - pub fn graph_definition_info - (&self) -> FallibleResult { - self.module.find_definition(&self.id) - } - /// Get the double representation description of the graph. pub fn graph_info(&self) -> FallibleResult { - self.graph_definition_info().map(GraphInfo::from_definition) + self.definition().map(|definition| GraphInfo::from_definition(definition.item)) } /// Returns double rep information about all nodes in the graph. @@ -567,7 +562,7 @@ impl Handle { /// resolution in the code in this graph. pub fn used_names(&self) -> FallibleResult> { use double_representation::alias_analysis; - let def = self.graph_definition_info()?; + let def = self.definition()?; let body = def.body(); let usage = if matches!(body.shape(),ast::Shape::Block(_)) { alias_analysis::analyze_crumbable(body.item) @@ -645,14 +640,13 @@ impl Handle { /// moved after it, keeping their order. pub fn place_node_and_dependencies_lines_after (&self, node_to_be_before:node::Id, node_to_be_after:node::Id) -> FallibleResult { - let definition = self.graph_definition_info()?; - let definition_ast = &definition.body().item; + let graph = self.graph_info()?; + let definition_ast = &graph.body().item; let dependent_nodes = connection::dependent_nodes_in_def(definition_ast,node_to_be_after); - let mut lines = definition.block_lines(); - let node_to_be_before = node::locate(&lines,node_to_be_before)?; - let node_to_be_after = node::locate(&lines,node_to_be_after)?; - let dependent_nodes = dependent_nodes.iter().map(|id| node::locate(&lines, *id)) + let node_to_be_before = graph.locate_node(node_to_be_before)?; + let node_to_be_after = graph.locate_node(node_to_be_after)?; + let dependent_nodes = dependent_nodes.iter().map(|id| graph.locate_node(*id)) .collect::,_>>()?; if node_to_be_after.index < node_to_be_before.index { @@ -664,6 +658,9 @@ impl Handle { false } }; + + // FIXME block lines are not necessarily in a block + let mut lines = graph.block_lines(); let range = NodeLocation::range(node_to_be_after.index, node_to_be_before.index); lines[range].sort_by_key(should_be_at_end); self.update_definition_ast(|mut def| { @@ -739,7 +736,7 @@ impl Handle { /// Parses given text as a node expression. pub fn parse_node_expression (&self, expression_text:impl Str) -> FallibleResult { - let node_ast = self.parser.parse_line(expression_text.as_ref())?; + let node_ast = self.parser.parse_line_ast(expression_text.as_ref())?; if ast::opr::is_assignment(&node_ast) { Err(BindingExpressionNotAllowed(expression_text.into()).into()) } else { @@ -752,11 +749,12 @@ impl Handle { info!(self.logger, "Adding node with expression `{node.expression}`"); let expression_ast = self.parse_node_expression(&node.expression)?; let main_line = MainLine::from_ast(&expression_ast).ok_or(FailedToCreateNode)?; + let indent = self.definition()?.indent(); let documentation = node.doc_comment.as_ref() .map(|text| DocCommentInfo::text_to_repr(&text)) .map(|doc_code| self.parser.parse_line(doc_code)) .transpose()? - .map(|doc_ast| DocCommentInfo::new(&doc_ast).ok_or(FailedToCreateNode)) + .map(|doc_ast| DocCommentInfo::new(&doc_ast.as_ref(),indent).ok_or(FailedToCreateNode)) .transpose()?; let mut node_info = NodeInfo {documentation,main_line}; @@ -1457,7 +1455,7 @@ main = let destination = Endpoint::new(node1.info.id(),dst_port.to_vec()); let connection = Connection{source,destination}; graph.connect(&connection,&span_tree::generate::context::Empty).unwrap(); - let new_main = graph.graph_definition_info().unwrap().ast.repr(); + let new_main = graph.definition().unwrap().ast.repr(); assert_eq!(new_main,expected,"Case {:?}",this); }) } @@ -1501,7 +1499,7 @@ main = } }; graph.connect(&connection_to_add,&span_tree::generate::context::Empty).unwrap(); - let new_main = graph.graph_definition_info().unwrap().ast.repr(); + let new_main = graph.definition().unwrap().ast.repr(); assert_eq!(new_main,EXPECTED); }) } @@ -1539,7 +1537,7 @@ main = } }; graph.connect(&connection_to_add,&span_tree::generate::context::Empty).unwrap(); - let new_main = graph.graph_definition_info().unwrap().ast.repr(); + let new_main = graph.definition().unwrap().ast.repr(); assert_eq!(new_main,EXPECTED); }) } @@ -1576,7 +1574,7 @@ main = } }; graph.connect(&connection_to_add,&span_tree::generate::context::Empty).unwrap(); - let new_main = graph.graph_definition_info().unwrap().ast.repr(); + let new_main = graph.definition().unwrap().ast.repr(); assert_eq!(new_main,EXPECTED); }) } @@ -1599,7 +1597,7 @@ main = ]; for (code,expected_name) in &cases { - let ast = parser.parse_line(*code).unwrap(); + let ast = parser.parse_line_ast(*code).unwrap(); let node = MainLine::from_ast(&ast).unwrap(); let name = Handle::variable_name_base_for(&node); assert_eq!(&name,expected_name); @@ -1625,7 +1623,7 @@ main = 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(); + let new_main = graph.definition().unwrap().ast.repr(); assert_eq!(new_main,expected,"Case {:?}",this); }) } diff --git a/src/rust/ide/src/controller/project.rs b/src/rust/ide/src/controller/project.rs index e7ecd80675..0e3bce61c6 100644 --- a/src/rust/ide/src/controller/project.rs +++ b/src/rust/ide/src/controller/project.rs @@ -191,7 +191,7 @@ impl Project { if module.lookup_method(project_name,main_ptr).is_err() { let mut info = module.info(); let main_code = default_main_method_code(); - let main_ast = parser.parse_line(main_code)?; + let main_ast = parser.parse_line_ast(main_code)?; info.add_ast(main_ast,double_representation::module::Placement::End)?; module.update_ast(info.ast)?; } diff --git a/src/rust/ide/src/controller/searcher.rs b/src/rust/ide/src/controller/searcher.rs index b69cb9168e..58182d40c2 100644 --- a/src/rust/ide/src/controller/searcher.rs +++ b/src/rust/ide/src/controller/searcher.rs @@ -199,7 +199,7 @@ impl ParsedInput { // // See also `parsed_input` test to see all cases we want to cover. input.push('a'); - let ast = parser.parse_line(input.trim_start())?; + let ast = parser.parse_line_ast(input.trim_start())?; let mut prefix = ast::prefix::Chain::from_ast_non_strict(&ast); if let Some(last_arg) = prefix.args.pop() { let mut last_arg_repr = last_arg.sast.wrapped.repr(); @@ -262,7 +262,7 @@ impl ParsedInput { /// Convert the current input to Prefix Chain representation. pub fn as_prefix_chain (&self, parser:&Parser) -> Option> { - let parsed_pattern = parser.parse_line(&self.pattern).ok(); + let parsed_pattern = parser.parse_line_ast(&self.pattern).ok(); let pattern_sast = parsed_pattern.map(|p| ast::Shifted::new(self.pattern_offset,p)); // If there is an expression part of input, we add current pattern as the last argument. if let Some(chain) = &self.expression { @@ -580,7 +580,7 @@ impl Searcher { let picked_completion = FragmentAddedByPickingSuggestion {id,picked_suggestion}; let code_to_insert = self.code_to_insert(&picked_completion).code; debug!(self.logger, "Code to insert: \"{code_to_insert}\""); - let added_ast = self.ide.parser().parse_line(&code_to_insert)?; + let added_ast = self.ide.parser().parse_line_ast(&code_to_insert)?; let pattern_offset = self.data.borrow().input.pattern_offset; let new_expression = match self.data.borrow_mut().input.expression.take() { None => { @@ -1662,7 +1662,7 @@ pub mod test { fn run(&self) { let parser = Parser::new_or_panic(); - let ast = parser.parse_line(self.before).unwrap(); + let ast = parser.parse_line_ast(self.before).unwrap(); let new_ast = apply_this_argument("foo",&ast); assert_eq!(new_ast.repr(),self.after,"Case {:?} failed: {:?}",self,ast); } @@ -1861,28 +1861,28 @@ pub mod test { fn simple_function_call_parsing() { let parser = Parser::new_or_panic(); - let ast = parser.parse_line("foo").unwrap(); + let ast = parser.parse_line_ast("foo").unwrap(); let call = SimpleFunctionCall::try_new(&ast).expect("Returned None for \"foo\""); assert!(call.this_argument.is_none()); assert_eq!(call.function_name, "foo"); - let ast = parser.parse_line("Main.foo").unwrap(); + let ast = parser.parse_line_ast("Main.foo").unwrap(); let call = SimpleFunctionCall::try_new(&ast).expect("Returned None for \"Main.foo\""); assert_eq!(call.this_argument.unwrap().repr(), "Main"); assert_eq!(call.function_name , "foo"); - let ast = parser.parse_line("(2 + 3).foo").unwrap(); + let ast = parser.parse_line_ast("(2 + 3).foo").unwrap(); let call = SimpleFunctionCall::try_new(&ast).expect("Returned None for \"(2 + 3).foo\""); assert_eq!(call.this_argument.unwrap().repr(), "(2 + 3)"); assert_eq!(call.function_name , "foo"); - let ast = parser.parse_line("foo + 3").unwrap(); + let ast = parser.parse_line_ast("foo + 3").unwrap(); assert!(SimpleFunctionCall::try_new(&ast).is_none()); - let ast = parser.parse_line("foo bar baz").unwrap(); + let ast = parser.parse_line_ast("foo bar baz").unwrap(); assert!(SimpleFunctionCall::try_new(&ast).is_none()); - let ast = parser.parse_line("Main . (foo bar)").unwrap(); + let ast = parser.parse_line_ast("Main . (foo bar)").unwrap(); assert!(SimpleFunctionCall::try_new(&ast).is_none()); } diff --git a/src/rust/ide/src/double_representation.rs b/src/rust/ide/src/double_representation.rs index d446f9036d..47f44b2a45 100644 --- a/src/rust/ide/src/double_representation.rs +++ b/src/rust/ide/src/double_representation.rs @@ -4,10 +4,10 @@ use crate::prelude::*; use ast::{Ast, opr, prefix, known}; -use crate::double_representation::definition::DefinitionName; +use crate::double_representation::definition::{DefinitionName, DefinitionInfo}; use crate::double_representation::definition::ScopeKind; use ast::crumbs::{Located, InfixCrumb}; -use ast::macros::DocCommentInfo; +use ast::macros::{DocCommentInfo, DocumentationContext}; pub mod alias_analysis; pub mod comment; @@ -46,17 +46,10 @@ pub const INDENT : usize = 4; // ======================== /// What kind of node or definition a line should be treated as. -#[derive(Clone,Debug)] pub enum LineKind { /// Definition is a binding, which defines a new entity with arguments. Definition { - /// Ast of the whole binding. - ast : known::Infix, - /// Definition name. If this is an extension method, it might imply an implicit `this` - /// argument. - name : Located, - /// Explicit arguments. Note that this might be empty when there are implicit arguments. - args : Vec>, + get_definition : Box DefinitionInfo> }, /// Node in a binding form. ExpressionAssignment { @@ -72,7 +65,7 @@ pub enum LineKind { /// Instead, they are discovered and processed as part of nodes that follow them. DocumentationComment { /// The comment representation. - comment : DocCommentInfo, + get_documentation : Box DocCommentInfo> } } @@ -88,9 +81,9 @@ impl LineKind { Some(infix) => infix, None => - return if let Some(comment) = DocCommentInfo::new(ast) { + return if let Some(get_documentation) = DocCommentInfo::from_ast(ast) { // e.g. `## My comment.` - DocumentationComment {comment} + DocumentationComment {get_documentation:Box::new(get_documentation)} } else { // The simplest form of node, e.g. `Point 5 10` ExpressionPlain {ast:ast.clone_ref()} @@ -136,7 +129,14 @@ impl LineKind { }; Definition { - args,name,ast:infix.clone_ref() + get_definition: Box::new(|context_indent| { + DefinitionInfo { + context_indent, + ast : infix, + name, + args + } + }) } } } diff --git a/src/rust/ide/src/double_representation/alias_analysis.rs b/src/rust/ide/src/double_representation/alias_analysis.rs index 13e4489726..e33953eb89 100644 --- a/src/rust/ide/src/double_representation/alias_analysis.rs +++ b/src/rust/ide/src/double_representation/alias_analysis.rs @@ -374,7 +374,7 @@ mod tests { fn run_case(parser:&parser::Parser, case:Case) { DEBUG!("\n===========================================================================\n"); DEBUG!("Case: " case.code); - let ast = parser.parse_line(&case.code).unwrap(); + let ast = parser.parse_line_ast(&case.code).unwrap(); let result = analyze_ast(&ast); DEBUG!("Analysis results: {result:?}"); validate_identifiers("introduced",&ast, case.expected_introduced, &result.introduced); diff --git a/src/rust/ide/src/double_representation/comment.rs b/src/rust/ide/src/double_representation/comment.rs index 847d34c590..2d86de094b 100644 --- a/src/rust/ide/src/double_representation/comment.rs +++ b/src/rust/ide/src/double_representation/comment.rs @@ -17,9 +17,16 @@ mod tests { let main_id = double_representation::definition::Id::new_plain_name("main"); let main = double_representation::module::get_definition(&ast,&main_id).unwrap(); let lines = main.block_lines(); - let first_line_ast = lines[0].elem.as_ref().unwrap(); - let doc = DocCommentInfo::new_indented(first_line_ast,4).unwrap(); - assert_eq!(doc.text(), expected_comment_text); + let first_line = lines[0].transpose_ref().unwrap(); + let doc = DocCommentInfo::new(&first_line,main.indent()).unwrap(); + let text = doc.text(); + assert_eq!(text, expected_comment_text); + + // Now, if we convert our pretty text to code, will it be the same as original line? + let code = DocCommentInfo::text_to_repr(&text); + let ast2 = parser.parse_line(&code).unwrap(); + let doc2 = DocCommentInfo::new(&ast2.as_ref(),main.indent()).expect(&format!("Failed to parse `{}` as comment",code)); + assert_eq!(doc.line().repr(), doc2.line().repr()) } #[test] @@ -42,7 +49,7 @@ main = let expected = "Single line"; run_case(&parser, code,expected); - // Single line case with a single space after `##`. + // Single line case with a single trailing space after `##`. let code = r#" main = ## @@ -69,15 +76,8 @@ main = ## First line Second line node"#; - let ast = parser.parse_module(code,default()).unwrap(); - let main_id = double_representation::definition::Id::new_plain_name("main"); - let module_info = double_representation::module::Info {ast:ast.clone_ref()}; - - let main = double_representation::module::get_definition(&ast,&main_id).unwrap(); - let lines = main.block_lines(); - assert_eq!(lines.len(),2); - let doc = ast::macros::DocCommentInfo::new_indented(lines[0].elem.as_ref().unwrap(),4).unwrap(); - assert_eq!(doc.text(), " First line\n Second line"); + let expected = " First line\n Second line"; + run_case(&parser, code,expected); } } diff --git a/src/rust/ide/src/double_representation/definition.rs b/src/rust/ide/src/double_representation/definition.rs index a43200a27a..fbba500f3f 100644 --- a/src/rust/ide/src/double_representation/definition.rs +++ b/src/rust/ide/src/double_representation/definition.rs @@ -2,6 +2,8 @@ use crate::prelude::*; +use crate::double_representation::LineKind; + use ast::crumbs::ChildAst; use ast::crumbs::Crumbable; use ast::crumbs::InfixCrumb; @@ -9,7 +11,7 @@ use ast::crumbs::Located; use ast::known; use ast::opr; use parser::Parser; -use crate::double_representation::LineKind; + // ===================== @@ -178,7 +180,7 @@ impl DefinitionName { pub fn ast(&self, parser:&Parser) -> FallibleResult { // We can't assume that string pieces we have are valid identifiers. // But neither this is our responsibility. If it parses, we can store it in the Ast. - parser.parse_line(self.to_string()) + parser.parse_line_ast(self.to_string()) } /// Checks if the given definition name is a method defined on given expected atom name. @@ -262,7 +264,7 @@ impl DefinitionInfo { // offsets. This is not desirable, as e.g. an empty line in the middle of block is not // possible to express with the current AST (it won't round-trip). - let indent = self.context_indent + double_representation::INDENT; + let indent = self.indent(); let mut empty_lines = Vec::new(); let mut line = lines.pop_front().ok_or(MissingLineWithAst)?; while line.elem.is_none() { @@ -311,8 +313,8 @@ impl DefinitionInfo { /// some binding or other kind of subtree). pub fn from_line_ast (ast:&Ast, kind:ScopeKind, context_indent:usize) -> Option { - if let LineKind::Definition{args,ast,name} = LineKind::discern(ast,kind) { - Some(DefinitionInfo { ast, name, args, context_indent }) + if let LineKind::Definition{get_definition} = LineKind::discern(ast,kind) { + Some(get_definition(context_indent)) } else { None } @@ -578,7 +580,7 @@ mod tests { #[wasm_bindgen_test] fn definition_name_tests() { let parser = parser::Parser::new_or_panic(); - let ast = parser.parse_line("Foo.Bar.baz").unwrap(); + let ast = parser.parse_line_ast("Foo.Bar.baz").unwrap(); let name = DefinitionName::from_ast(&ast).unwrap(); assert_eq!(*name.name, "baz"); @@ -593,14 +595,14 @@ mod tests { #[wasm_bindgen_test] fn definition_name_rejecting_incomplete_names() { let parser = parser::Parser::new_or_panic(); - let ast = parser.parse_line("Foo. .baz").unwrap(); + let ast = parser.parse_line_ast("Foo. .baz").unwrap(); assert!(DefinitionName::from_ast(&ast).is_none()); } #[wasm_bindgen_test] fn definition_info_name() { let parser = parser::Parser::new_or_panic(); - let ast = parser.parse_line("Foo.bar a b c = baz").unwrap(); + let ast = parser.parse_line_ast("Foo.bar a b c = baz").unwrap(); let definition = DefinitionInfo::from_root_line_ast(&ast).unwrap(); assert_eq!(definition.name.to_string(), "Foo.bar"); @@ -610,7 +612,7 @@ mod tests { #[wasm_bindgen_test] fn located_definition_args() { let parser = parser::Parser::new_or_panic(); - let ast = parser.parse_line("foo bar baz = a + b + c").unwrap(); + let ast = parser.parse_line_ast("foo bar baz = a + b + c").unwrap(); let definition = DefinitionInfo::from_root_line_ast(&ast).unwrap(); let (arg0,arg1) = definition.args.expect_tuple(); diff --git a/src/rust/ide/src/double_representation/graph.rs b/src/rust/ide/src/double_representation/graph.rs index 96ca074420..40da63aa77 100644 --- a/src/rust/ide/src/double_representation/graph.rs +++ b/src/rust/ide/src/double_representation/graph.rs @@ -2,7 +2,7 @@ use crate::prelude::*; -use crate::double_representation::definition::DefinitionInfo; +use crate::double_representation::definition::{DefinitionInfo, DefinitionProvider}; use crate::double_representation::node; use crate::double_representation::node::LocatedNode; use crate::double_representation::node::NodeInfo; @@ -43,23 +43,38 @@ pub enum LocationHint { // ================= /// Description of the graph, based on information available in AST. -#[derive(Clone,Debug)] +#[derive(Clone,Debug,Shrinkwrap)] pub struct GraphInfo { /// The definition providing this graph. pub source:DefinitionInfo, } impl GraphInfo { + pub fn locate_node(&self, id:double_representation::node::Id) -> FallibleResult { + let lines = self.source.block_lines(); + double_representation::node::locate(&lines, self.source.context_indent, id) + } + /// Describe graph of the given definition. pub fn from_definition(source:DefinitionInfo) -> GraphInfo { GraphInfo {source} } - /// Lists nodes in the given binding's ast (infix expression). - fn from_function_binding(ast:known::Infix) -> Vec { - let body = ast.rarg.clone(); + /// Gets the AST of this graph definition. + pub fn ast(&self) -> Ast { + self.source.ast.clone().into() + } + + /// Gets all known nodes in this graph (does not include special pseudo-nodes like graph + /// inputs and outputs). + pub fn nodes(&self) -> Vec { + let ast = &self.source.ast; + let body = &ast.rarg; if let Ok(body_block) = known::Block::try_new(body.clone()) { - block_nodes(&body_block) + let context_indent = self.source.indent(); + let lines_iter = body_block.enumerate_non_empty_lines(); + let nodes_iter = node::NodeIterator {context_indent,lines_iter}; + nodes_iter.map(|n| n.node).collect() } else if let Some(node) = node::NodeInfo::from_main_line_ast(&body) { // There's no way to attach a documentation comment to an inline node, it consists only // of the main line. @@ -71,17 +86,6 @@ impl GraphInfo { } } - /// Gets the AST of this graph definition. - pub fn ast(&self) -> Ast { - self.source.ast.clone().into() - } - - /// Gets all known nodes in this graph (does not include special pseudo-nodes like graph - /// inputs and outputs). - pub fn nodes(&self) -> Vec { - Self::from_function_binding(self.source.ast.clone()) - } - /// Gets the list of connections between the nodes in this graph. pub fn connections(&self) -> Vec { double_representation::connection::list(&self.source.ast.rarg) @@ -90,13 +94,13 @@ impl GraphInfo { /// Adds a new node to this graph. pub fn add_line (&mut self, line_ast:Ast, location_hint:LocationHint) -> FallibleResult { - let mut lines = self.source.block_lines(); + let mut lines = self.block_lines(); let last_non_empty = || lines.iter().rposition(|line| line.elem.is_some()); let index = match location_hint { LocationHint::Start => 0, LocationHint::End => last_non_empty().map_or(lines.len(),|ix| ix + 1), - LocationHint::After(id) => node::locate(&lines, id)?.index.last() + 1, - LocationHint::Before(id) => node::locate(&lines, id)?.index.first() + LocationHint::After(id) => self.locate_node(id)?.index.last() + 1, + LocationHint::Before(id) => self.locate_node(id)?.index.first() }; let elem = Some(line_ast); let off = 0; @@ -113,8 +117,8 @@ impl GraphInfo { let index = match location_hint { LocationHint::Start => 0, LocationHint::End => last_non_empty().map_or(lines.len(),|ix| ix + 1), - LocationHint::After(id) => node::locate(&lines, id)?.index.last() + 1, - LocationHint::Before(id) => node::locate(&lines, id)?.index.first(), + LocationHint::After(id) => self.locate_node(id)?.index.last() + 1, + LocationHint::Before(id) => self.locate_node(id)?.index.first(), }; let elem = Some(node.ast().clone_ref()); let off = 0; @@ -154,9 +158,9 @@ impl GraphInfo { /// Sets a new state for the node. The id of the described node must denote already existing /// node. pub fn update_node(&mut self, id:ast::Id, f:impl FnOnce(NodeInfo) -> Option) -> FallibleResult { - let mut lines = self.source.block_lines(); - let LocatedNode{index,node} = node::locate(&lines, id)?; + let LocatedNode{index,node} = self.locate_node(id)?; + let mut lines = self.source.block_lines(); if let Some(updated_node) = f(node) { lines[index.main_line].elem = Some(updated_node.main_line.ast().clone_ref()); match (index.documentation_line, updated_node.documentation) { @@ -205,21 +209,6 @@ impl GraphInfo { -// ===================== -// === Listing nodes === -// ===================== - -/// Collects information about nodes in given code `Block`. -pub fn block_nodes(ast:&known::Block) -> Vec { - // Warning: this uses faux indices, as the empty lines are skipped first. - // It is fine here, since we throw away the result values depending on index. - let lines_iter = ast.iter().enumerate(); - let nodes_iter = node::NodeIterator {lines_iter}; - nodes_iter.map(|n| n.node).collect() -} - - - // ============= // === Tests === // ============= diff --git a/src/rust/ide/src/double_representation/module.rs b/src/rust/ide/src/double_representation/module.rs index 95f18b863a..6039bd829b 100644 --- a/src/rust/ide/src/double_representation/module.rs +++ b/src/rust/ide/src/double_representation/module.rs @@ -485,7 +485,7 @@ impl Info { }).last(); let index_to_place_at = previous_import.map_or(0,|(crumb,_)| crumb.line_index + 1); - let import_ast = parser.parse_line(to_add.to_string()).unwrap(); + let import_ast = parser.parse_line_ast(to_add.to_string()).unwrap(); self.add_line(index_to_place_at,Some(import_ast)); index_to_place_at } @@ -790,7 +790,7 @@ mod tests { let ast = parser.parse_module(code,default()).unwrap(); let mut info = Info { ast }; let import = |code| { - let ast = parser.parse_line(code).unwrap(); + let ast = parser.parse_line_ast(code).unwrap(); ImportInfo::from_ast(&ast).unwrap() }; diff --git a/src/rust/ide/src/double_representation/node.rs b/src/rust/ide/src/double_representation/node.rs index d73d7956e4..6addcac413 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -2,7 +2,7 @@ use crate::prelude::*; -use ast::Ast; +use ast::{Ast, BlockLine, enumerate_non_empty_lines}; use ast::crumbs::Crumbable; use ast::known; use std::cmp::Ordering; @@ -82,7 +82,7 @@ pub struct LocatedNode { } /// Tests if given line contents can be seen as node with a given id -pub fn is_main_line_of(line:&ast::BlockLine>, id:Id) -> bool { +pub fn is_main_line_of(line:&BlockLine>, id:Id) -> bool { let node_info = MainLine::from_block_line(line); node_info.contains_if(|node| node.id() == id) } @@ -91,10 +91,11 @@ pub fn is_main_line_of(line:&ast::BlockLine>, id:Id) -> bool { /// /// Returns an error if the Id is not found. pub fn locate<'a> -( lines : impl IntoIterator>> + 'a +( lines : impl IntoIterator>> + 'a +, context_indent : usize , id : Id ) -> FallibleResult { - Ok(locate_many(lines, [id])?.remove(&id).unwrap()) + Ok(locate_many(lines, context_indent, [id])?.remove(&id).unwrap()) } /// Obtain located node information for multiple nodes in a single pass. @@ -102,18 +103,17 @@ pub fn locate<'a> /// If any of the looked for nodes is not found, `Err` is returned. /// Any `Ok(…)` return value is guaranteed to have length equal to `looked_for` argument. pub fn locate_many<'a> -( lines : impl IntoIterator>> + 'a -, looked_for : impl IntoIterator +( lines : impl IntoIterator>> + 'a +, context_indent : usize +, looked_for : impl IntoIterator ) -> FallibleResult> { - // Skip empty lines, while preserving indices. - let lines_iter = lines.into_iter() - .enumerate() - .filter_map(|(index, line)| line.elem.as_ref().map(|ast| (index, ast))); - let mut looked_for = looked_for.into_iter().collect::>(); let mut ret = HashMap::new(); - let nodes = NodeIterator { lines_iter }; + // Skip empty lines, there are no nodes. + // However, indices are important. + let lines_iter = enumerate_non_empty_lines(lines); + let nodes = NodeIterator {lines_iter,context_indent}; for node in nodes { if looked_for.remove(&node.id()) { ret.insert(node.id(), node); @@ -139,20 +139,61 @@ pub fn locate_many<'a> /// Iterator over indexed line ASTs that yields nodes. #[derive(Clone,Debug)] -pub struct NodeIterator<'a, T:Iterator + 'a> { +pub struct NodeIterator<'a, T:Iterator)> + 'a> { /// Input iterator that yields pairs (line index, line's Ast). - pub lines_iter : T + pub lines_iter : T, + /// + pub context_indent:usize, } -impl<'a, T:Iterator + 'a> Iterator for NodeIterator<'a, T> { +// pub fn iter_lines<'a>(lines:impl IntoIterator>)>, context_indent:usize) +// -> NodeIterator<'a, impl Iterator)> + 'a> { +// +// let lines_iter = lines.into_iter().enumerate().filter_map(|(index, line)| { +// match &line.elem { +// Some(elem) => Some((index, BlockLine {elem,off:line.off})), +// None => None, +// } +// // Some((index,line.transpose()?.as_ref())) +// }); +// +// NodeIterator {lines_iter,context_indent} +// } +// +// pub fn iter_block<'a>(block:&'a ast::Block, parent_indent:usize) +// -> NodeIterator<'a, impl Iterator)> + 'a> { +// iter_lines(&block.lines,block.indent(parent_indent)) +// } + + + +// impl<'a, T:Iterator)> + 'a> NodeIterator<'a, T> { +// pub fn from_all_lines(lines:impl IntoIterator>)>, context_indent:usize) -> Self { +// // let lines_iter = lines.into_iter().enumerate().filter_map(|(index, line)| { +// // Some((index,line.transpose()?)) +// // }); +// +// let lines_iter = lines.into_iter().enumerate().filter_map(|(index, line)| { +// Some((index,line.transpose()?.as_ref())) +// }); +// +// NodeIterator {lines_iter,context_indent} +// } +// +// pub fn from_block(block:&'a ast::Block, parent_indent:usize) -> Self { +// Self::from_all_lines(&block.lines,block.indent(parent_indent)) +// } +// } + +impl<'a, T:Iterator)> + 'a> Iterator for NodeIterator<'a, T> { type Item = LocatedNode; fn next(&mut self) -> Option { let mut documentation = None; while let Some((index,ast)) = self.lines_iter.next() { - if let Some(documentation_info) = DocCommentInfo::new(ast) { + if let Some(documentation_info) = DocCommentInfo::new(&ast,self.context_indent) { documentation = Some((index,documentation_info)); - } else if let Some(main_line) = MainLine::from_ast(ast) { + } else if let Some(main_line) = MainLine::from_ast(&ast.elem) { let (documentation_line,documentation) = match documentation { Some((index,documentation)) => (Some(index),Some(documentation)), None => (None,None) @@ -254,6 +295,8 @@ impl MainLine { // By definition, there are no nodes in the root scope. // Being a node's line, we may assume that this is not a root scope. let scope = ScopeKind::NonRoot; + // Nodes, unlike documentation and definitions, are insensitive to parent block indent. + // Thus, we can just assume any bogus value. match LineKind::discern(ast,scope) { LineKind::ExpressionPlain {ast} => Self::new_expression(ast), LineKind::ExpressionAssignment {ast} => Self::new_binding(ast), @@ -263,7 +306,7 @@ impl MainLine { } /// Tries to interpret AST as node, treating whole AST as an expression. - pub fn from_block_line(line:&ast::BlockLine>) -> Option { + pub fn from_block_line(line:&BlockLine>) -> Option { Self::from_ast(line.elem.as_ref()?) } diff --git a/src/rust/ide/src/double_representation/refactorings/collapse.rs b/src/rust/ide/src/double_representation/refactorings/collapse.rs index 46cad46ebb..2f9fefa8df 100644 --- a/src/rust/ide/src/double_representation/refactorings/collapse.rs +++ b/src/rust/ide/src/double_representation/refactorings/collapse.rs @@ -384,7 +384,6 @@ mod tests { use crate::double_representation::definition::DefinitionName; use crate::double_representation::graph; use crate::double_representation::module; - use crate::double_representation::node::NodeInfo; use ast::crumbs::Crumb; diff --git a/src/rust/ide/view/src/debug_scenes/interface.rs b/src/rust/ide/view/src/debug_scenes/interface.rs index 803246c05a..e80382aaef 100644 --- a/src/rust/ide/view/src/debug_scenes/interface.rs +++ b/src/rust/ide/view/src/debug_scenes/interface.rs @@ -281,7 +281,7 @@ pub fn expression_mock_string(label:&str) -> Expression { let code = format!("\"{}\"", label); let parser = Parser::new_or_panic(); let parameters = vec![]; - let ast = parser.parse_line(&code).unwrap(); + let ast = parser.parse_line_ast(&code).unwrap(); let invocation_info = span_tree::generate::context::CalledMethodInfo {parameters}; let ctx = span_tree::generate::MockContext::new_single(ast.id.unwrap(),invocation_info); let output_span_tree = span_tree::SpanTree::default(); @@ -299,7 +299,7 @@ pub fn expression_mock() -> Expression { tp : Some("Text".to_owned()), }; let parameters = vec![this_param]; - let ast = parser.parse_line(&code).unwrap(); + let ast = parser.parse_line_ast(&code).unwrap(); let invocation_info = span_tree::generate::context::CalledMethodInfo {parameters}; let ctx = span_tree::generate::MockContext::new_single(ast.id.unwrap(),invocation_info); let output_span_tree = span_tree::SpanTree::default(); @@ -373,7 +373,7 @@ pub fn expression_mock3() -> Expression { tp : Some("Vector String".to_owned()), }; let parameters = vec![this_param,param0,param1,param2,param3]; - let ast = parser.parse_line(&code).unwrap(); + let ast = parser.parse_line_ast(&code).unwrap(); let invocation_info = span_tree::generate::context::CalledMethodInfo {parameters}; let ctx = span_tree::generate::MockContext::new_single(ast.id.unwrap(),invocation_info); let output_span_tree = span_tree::SpanTree::new(&ast,&ctx).unwrap();//span_tree::SpanTree::default(); From de20ceb3d4a25aef829abd1d2552826b068e003a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Sun, 1 Aug 2021 23:19:26 +0200 Subject: [PATCH 11/19] dealing with partial information about comments --- CHANGELOG.md | 3 + src/rust/ide/lib/ast/impl/src/lib.rs | 10 +- src/rust/ide/lib/ast/impl/src/macros.rs | 116 ++++++++++++------ src/rust/ide/src/controller/graph.rs | 7 +- src/rust/ide/src/double_representation.rs | 27 +++- .../ide/src/double_representation/comment.rs | 8 +- .../ide/src/double_representation/graph.rs | 12 +- .../ide/src/double_representation/node.rs | 93 ++++++-------- 8 files changed, 162 insertions(+), 114 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b29bfae62..978bfe3889 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ these updates be shipped in a stable release before the end of the year. - [New look of open project dialog.][1700]. Now it has "Open project" title on the top. +- [Documentation cooments are displayed next to the nodes.][1744]. + #### Enso Compiler @@ -31,6 +33,7 @@ these updates be shipped in a stable release before the end of the year. [1700]: https://github.com/enso-org/ide/pull/1700 [1726]: https://github.com/enso-org/ide/pull/1726 [1743]: https://github.com/enso-org/ide/pull/1743 +[1744]: https://github.com/enso-org/ide/pull/1744 # Enso 2.0.0-alpha.10 (2021-07-23) diff --git a/src/rust/ide/lib/ast/impl/src/lib.rs b/src/rust/ide/lib/ast/impl/src/lib.rs index 222c44a30e..6ff2a7b6ce 100644 --- a/src/rust/ide/lib/ast/impl/src/lib.rs +++ b/src/rust/ide/lib/ast/impl/src/lib.rs @@ -624,8 +624,14 @@ impl Block { parent_indent + self.indent } - pub fn enumerate_non_empty_lines(&self) -> impl Iterator)> + '_ { - enumerate_non_empty_lines(&self.lines) + pub fn enumerate_non_empty_lines(&self) -> impl Iterator)> + '_ + where T:CloneRef { + let first_line = std::iter::once((0,self.first_line.as_ref())); + let further_lines = (1..).zip(self.lines.iter()).filter_map(|(index,line):(usize,&BlockLine>)| { + let non_empty_line = line.transpose_ref()?; + Some((index, non_empty_line)) + }); + first_line.chain(further_lines) } } diff --git a/src/rust/ide/lib/ast/impl/src/macros.rs b/src/rust/ide/lib/ast/impl/src/macros.rs index ff169815db..355d5de40a 100644 --- a/src/rust/ide/lib/ast/impl/src/macros.rs +++ b/src/rust/ide/lib/ast/impl/src/macros.rs @@ -34,11 +34,9 @@ pub const QUALIFIED_EXPORT_KEYWORD:&str = "export"; -// ================ -// === Comments === -// ================ - +// ======================== // === Disable Comments === +// ======================== /// Try Interpreting the line as disabling comment. Return the text after `#`. pub fn as_disable_comment(ast:&Ast) -> Option { @@ -57,49 +55,59 @@ pub fn is_disable_comment(ast:&Ast) -> bool { } + +// ============================== // === Documentation Comments === +// ============================== -/// Information about AST context necessary to construct `DocCommentInfo` from line's AST. -pub struct DocumentationContext { - /// Absolute indentation of the documentation comment line. - pub block_indent : usize, - /// Trailing whitespace in the documentation line. - pub trailing_whitespace : usize, -} +// === Ast Description === -/// Ast known to be a documentation comment. -#[derive(Clone,Debug)] -pub struct DocCommentInfo { - line : BlockLine, - body : crate::MacroPatternMatch>, - /// The absolute indent of the block that contains the line with documentation comment. - pub block_indent : usize, +/// Describes the AST of a documentation comment. +pub struct DocumentationCommentAst { + ast : known::Match, + body : crate::MacroPatternMatch>, } -impl DocCommentInfo { - pub fn from_ast(ast:&Ast) -> Option Self> { +impl DocumentationCommentAst { + /// Interpret given Ast as a documentation comment. Return `None` if it is not recognized. + pub fn new(ast:&Ast) -> Option { let ast = crate::known::Match::try_from(ast).ok()?; let first_segment = &ast.segs.head; let introducer = crate::identifier::name(&first_segment.head)?; if introducer == DOCUMENTATION_COMMENT_INTRODUCER { let body = first_segment.body.clone_ref(); - Some(move |DocumentationContext{block_indent,trailing_whitespace}| { - let line = BlockLine { - elem : ast, - off : trailing_whitespace, - }; - DocCommentInfo {line,body,block_indent} - }) + Some(DocumentationCommentAst {ast,body}) } else { None } } +} - /// Creates a documentation from Ast and information about indentation of the block it belongs - /// to. - pub fn new(line:&BlockLine<&Ast>, block_indent:usize) -> Option { - let context = DocumentationContext{block_indent,trailing_whitespace:line.off}; - Self::from_ast(line.elem).map(|f| f(context)) + +// === Line Description === + +/// Describes the line with a documentation comment. +#[derive(Clone,Debug,Shrinkwrap)] +pub struct DocumentationCommentLine { + /// Stores the documentation AST and the trailing whitespace length. + #[shrinkwrap(main_field)] + line : BlockLine, + body : crate::MacroPatternMatch>, +} + +impl DocumentationCommentLine { + /// Try constructing from a line. Return `None` if this line has no documentation comment. + pub fn new(line:&BlockLine<&Ast>) -> Option { + let doc_ast_opt = DocumentationCommentAst::new(line.elem); + doc_ast_opt.map(|doc_ast| Self::from_doc_ast(doc_ast,line.off)) + } + + /// Treat given documentation AST as the line with a given trailing whitespace. + pub fn from_doc_ast(ast_doc:DocumentationCommentAst, off:usize) -> Self { + Self { + line : BlockLine {elem:ast_doc.ast, off}, + body : ast_doc.body, + } } /// Get the documentation comment's AST. @@ -107,6 +115,39 @@ impl DocCommentInfo { self.line.elem.clone_ref() } + /// Get the line with this comment. + pub fn line(&self) -> &BlockLine { + &self.line + } + + /// Convenience function that throws away some information to return the line description that + /// is used in AST blocks. + pub fn block_line(&self) -> BlockLine> { + self.line.as_ref().map(|known_ast| Some(known_ast.ast().clone_ref())) + } +} + + +// === Full Description === + +#[derive(Clone,Debug,Shrinkwrap)] +pub struct DocumentationCommentInfo { + /// Description of the line with the documentation comment. + #[shrinkwrap(main_field)] + pub line : DocumentationCommentLine, + /// The absolute indent of the block that contains the line with documentation comment. + pub block_indent : usize, +} + +impl DocumentationCommentInfo { + /// Try to obtain information about a documentation comment line from block with a given indent. + pub fn new(line:&BlockLine<&Ast>, block_indent:usize) -> Option { + Some(Self { + line : DocumentationCommentLine::new(line)?, + block_indent + }) + } + /// Get the documentation text. pub fn text(&self) -> String { // This gets us documentation text, however non-first lines have the absolute indent @@ -128,19 +169,16 @@ impl DocCommentInfo { let mut out_lines = first_line.into_iter().chain(other_lines); out_lines.join("\n") } - - pub fn line(&self) -> &BlockLine { - &self.line - } } -impl AsRef for DocCommentInfo { + +impl AsRef for DocumentationCommentInfo { fn as_ref(&self) -> &Ast { self.line.elem.ast() } } -impl Display for DocCommentInfo { +impl Display for DocumentationCommentInfo { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f,"{}",self.text()) } @@ -148,7 +186,7 @@ impl Display for DocCommentInfo { /// Check if given Ast stores a documentation comment. pub fn is_documentation_comment(ast:&Ast) -> bool { - DocCommentInfo::from_ast(ast).is_some() + DocumentationCommentAst::new(ast).is_some() } diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 2501ac694c..501f942b48 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -22,7 +22,7 @@ use crate::model::module::NodeMetadata; use crate::model::traits::*; use ast::crumbs::InfixCrumb; -use ast::macros::DocCommentInfo; +use ast::macros::DocumentationCommentInfo; use enso_protocol::language_server; use parser::Parser; use span_tree::SpanTree; @@ -751,10 +751,10 @@ impl Handle { let main_line = MainLine::from_ast(&expression_ast).ok_or(FailedToCreateNode)?; let indent = self.definition()?.indent(); let documentation = node.doc_comment.as_ref() - .map(|text| DocCommentInfo::text_to_repr(&text)) + .map(|text| DocumentationCommentInfo::text_to_repr(&text)) .map(|doc_code| self.parser.parse_line(doc_code)) .transpose()? - .map(|doc_ast| DocCommentInfo::new(&doc_ast.as_ref(),indent).ok_or(FailedToCreateNode)) + .map(|doc_ast| DocumentationCommentInfo::new(&doc_ast.as_ref(),indent).ok_or(FailedToCreateNode)) .transpose()?; let mut node_info = NodeInfo {documentation,main_line}; @@ -1303,6 +1303,7 @@ main = test.run(|graph| async move { // === Initial nodes === let nodes = graph.nodes().unwrap(); + for node in &nodes { DEBUG!(node.repr())}; let (node1,node2) = nodes.expect_tuple(); assert_eq!(node1.info.expression().repr(), "2"); assert_eq!(node2.info.expression().repr(), "print foo"); diff --git a/src/rust/ide/src/double_representation.rs b/src/rust/ide/src/double_representation.rs index 47f44b2a45..e115df3d86 100644 --- a/src/rust/ide/src/double_representation.rs +++ b/src/rust/ide/src/double_representation.rs @@ -6,8 +6,10 @@ use crate::prelude::*; use ast::{Ast, opr, prefix, known}; use crate::double_representation::definition::{DefinitionName, DefinitionInfo}; use crate::double_representation::definition::ScopeKind; -use ast::crumbs::{Located, InfixCrumb}; -use ast::macros::{DocCommentInfo, DocumentationContext}; +use ast::crumbs::InfixCrumb; +use ast::crumbs::Located; +use ast::macros::DocumentationCommentAst; +use crate::double_representation::node::MainLine; pub mod alias_analysis; pub mod comment; @@ -65,11 +67,19 @@ pub enum LineKind { /// Instead, they are discovered and processed as part of nodes that follow them. DocumentationComment { /// The comment representation. - get_documentation : Box DocCommentInfo> + documentation : DocumentationCommentAst } } impl LineKind { + pub fn into_node_main_line(self) -> Option { + match self { + LineKind::ExpressionAssignment {ast} => MainLine::new_binding(ast), + LineKind::ExpressionPlain {ast} => MainLine::new_expression(ast), + LineKind::DocumentationComment {..} => None, + LineKind::Definition {..} => None, + } + } /// Tell how the given line (described by an Ast) should be treated. pub fn discern(ast:&Ast, kind:ScopeKind) -> Self { @@ -81,9 +91,9 @@ impl LineKind { Some(infix) => infix, None => - return if let Some(get_documentation) = DocCommentInfo::from_ast(ast) { + return if let Some(documentation) = DocumentationCommentAst::new(ast) { // e.g. `## My comment.` - DocumentationComment {get_documentation:Box::new(get_documentation)} + DocumentationComment {documentation} } else { // The simplest form of node, e.g. `Point 5 10` ExpressionPlain {ast:ast.clone_ref()} @@ -149,3 +159,10 @@ impl LineKind { // definition scope they are treated as invalid constructs (setter syntax in the old design). // 2. Expression like "foo = 5". In module, this is treated as method definition (with implicit // this parameter). In definition, this is just a node (evaluated expression). + +#[cfg(test)] +mod tests { + use super::*; + + +} diff --git a/src/rust/ide/src/double_representation/comment.rs b/src/rust/ide/src/double_representation/comment.rs index 2d86de094b..3df5b24703 100644 --- a/src/rust/ide/src/double_representation/comment.rs +++ b/src/rust/ide/src/double_representation/comment.rs @@ -6,7 +6,7 @@ use crate::prelude::*; mod tests { use super::*; use crate::double_representation::definition::DefinitionProvider; - use ast::macros::DocCommentInfo; + use ast::macros::DocumentationCommentInfo; use parser::Parser; @@ -18,14 +18,14 @@ mod tests { let main = double_representation::module::get_definition(&ast,&main_id).unwrap(); let lines = main.block_lines(); let first_line = lines[0].transpose_ref().unwrap(); - let doc = DocCommentInfo::new(&first_line,main.indent()).unwrap(); + let doc = DocumentationCommentInfo::new(&first_line,main.indent()).unwrap(); let text = doc.text(); assert_eq!(text, expected_comment_text); // Now, if we convert our pretty text to code, will it be the same as original line? - let code = DocCommentInfo::text_to_repr(&text); + let code = DocumentationCommentInfo::text_to_repr(&text); let ast2 = parser.parse_line(&code).unwrap(); - let doc2 = DocCommentInfo::new(&ast2.as_ref(),main.indent()).expect(&format!("Failed to parse `{}` as comment",code)); + let doc2 = DocumentationCommentInfo::new(&ast2.as_ref(),main.indent()).expect(&format!("Failed to parse `{}` as comment",code)); assert_eq!(doc.line().repr(), doc2.line().repr()) } diff --git a/src/rust/ide/src/double_representation/graph.rs b/src/rust/ide/src/double_representation/graph.rs index 40da63aa77..320087159e 100644 --- a/src/rust/ide/src/double_representation/graph.rs +++ b/src/rust/ide/src/double_representation/graph.rs @@ -168,12 +168,9 @@ impl GraphInfo { lines.remove(old_comment_index); } (Some(old_comment_index),Some(new_comment)) => - lines[old_comment_index].elem = Some(new_comment.as_ref().clone_ref()), + lines[old_comment_index] = new_comment.block_line(), (None,Some(new_comment)) => - lines.insert(index.main_line, BlockLine { - elem : Some(new_comment.as_ref().clone_ref()), - off : 0, - }), + lines.insert(index.main_line, new_comment.block_line()), (None,None) => {}, } } else { @@ -225,7 +222,7 @@ mod tests { use ast::test_utils::expect_single_line; use utils::test::ExpectTuple; use wasm_bindgen_test::wasm_bindgen_test; - use ast::macros::DocCommentInfo; + use ast::macros::DocumentationCommentInfo; wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser); @@ -285,7 +282,8 @@ mod tests { fn assert_same(left:&NodeInfo, right:&NodeInfo) { assert_eq!(left.id(), right.id()); - assert_eq!(left.documentation.as_ref().map(DocCommentInfo::to_string), right.documentation.as_ref().map(DocCommentInfo::to_string)); + assert_eq!( left.documentation.as_ref().map(DocumentationCommentInfo::to_string) + , right.documentation.as_ref().map(DocumentationCommentInfo::to_string)); assert_eq!(left.main_line.repr(), right.main_line.repr()); } diff --git a/src/rust/ide/src/double_representation/node.rs b/src/rust/ide/src/double_representation/node.rs index 6addcac413..b61dba0191 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -6,7 +6,7 @@ use ast::{Ast, BlockLine, enumerate_non_empty_lines}; use ast::crumbs::Crumbable; use ast::known; use std::cmp::Ordering; -use ast::macros::DocCommentInfo; +use ast::macros::{DocumentationCommentInfo, DocumentationCommentLine}; use crate::double_representation::LineKind; use crate::double_representation::definition::ScopeKind; @@ -142,74 +142,59 @@ pub fn locate_many<'a> pub struct NodeIterator<'a, T:Iterator)> + 'a> { /// Input iterator that yields pairs (line index, line's Ast). pub lines_iter : T, - /// + /// Absolute indent of lines in the block we are iterating over. pub context_indent:usize, } -// pub fn iter_lines<'a>(lines:impl IntoIterator>)>, context_indent:usize) -// -> NodeIterator<'a, impl Iterator)> + 'a> { -// -// let lines_iter = lines.into_iter().enumerate().filter_map(|(index, line)| { -// match &line.elem { -// Some(elem) => Some((index, BlockLine {elem,off:line.off})), -// None => None, -// } -// // Some((index,line.transpose()?.as_ref())) -// }); -// -// NodeIterator {lines_iter,context_indent} -// } -// -// pub fn iter_block<'a>(block:&'a ast::Block, parent_indent:usize) -// -> NodeIterator<'a, impl Iterator)> + 'a> { -// iter_lines(&block.lines,block.indent(parent_indent)) -// } - -// impl<'a, T:Iterator)> + 'a> NodeIterator<'a, T> { -// pub fn from_all_lines(lines:impl IntoIterator>)>, context_indent:usize) -> Self { -// // let lines_iter = lines.into_iter().enumerate().filter_map(|(index, line)| { -// // Some((index,line.transpose()?)) -// // }); +// pub fn block_nodes<'a, T:Iterator)> + 'a> { // -// let lines_iter = lines.into_iter().enumerate().filter_map(|(index, line)| { -// Some((index,line.transpose()?.as_ref())) -// }); // +// impl<'a, T:Iterator)> + 'a> NodeIterator<'a, T> { +// pub fn from_block(block:&known::Block, parent_indent:usize) -> Self { +// let lines_iter = block.enumerate_non_empty_lines(); +// let context_indent = block.indent + parent_indent; // NodeIterator {lines_iter,context_indent} // } -// -// pub fn from_block(block:&'a ast::Block, parent_indent:usize) -> Self { -// Self::from_all_lines(&block.lines,block.indent(parent_indent)) -// } // } impl<'a, T:Iterator)> + 'a> Iterator for NodeIterator<'a, T> { type Item = LocatedNode; fn next(&mut self) -> Option { - let mut documentation = None; - while let Some((index,ast)) = self.lines_iter.next() { - if let Some(documentation_info) = DocCommentInfo::new(&ast,self.context_indent) { - documentation = Some((index,documentation_info)); - } else if let Some(main_line) = MainLine::from_ast(&ast.elem) { - let (documentation_line,documentation) = match documentation { - Some((index,documentation)) => (Some(index),Some(documentation)), - None => (None,None) - }; - - let ret = LocatedNode { - node : NodeInfo {documentation,main_line}, - index : NodeLocation { - main_line : index, - documentation_line, + let mut indexed_documentation = None; + while let Some((index, line)) = self.lines_iter.next() { + match LineKind::discern(line.elem, ScopeKind::NonRoot) { + LineKind::DocumentationComment {documentation} => { + let doc_line = DocumentationCommentLine::from_doc_ast(documentation,line.off); + let documentation = DocumentationCommentInfo { + line : doc_line, + block_indent : self.context_indent, + }; + indexed_documentation = Some((index,documentation)); + } + LineKind::Definition { .. } => { + // Non-node entity consumes any previous documentation. + indexed_documentation = None; + } + line => { + if let Some(main_line) = line.into_node_main_line() { + let (documentation_line,documentation) = match indexed_documentation { + Some((index,documentation)) => (Some(index),Some(documentation)), + None => (None,None) + }; + + let ret = LocatedNode { + node: NodeInfo { documentation, main_line }, + index: NodeLocation { + main_line: index, + documentation_line, + } + }; + return Some(ret) } - }; - return Some(ret); - } else { - // Non-node entity consumes any previous documentation. - documentation = None; + } } } None @@ -222,7 +207,7 @@ impl<'a, T:Iterator)> + 'a> Iterator for NodeIter #[shrinkwrap(mutable)] pub struct NodeInfo { /// If the node has doc comment attached, it will be represented here. - pub documentation : Option, + pub documentation : Option, /// Primary node AST that contains node's expression and optional pattern binding. #[shrinkwrap(main_field)] pub main_line : MainLine, From c6cceed81ff6fcb3c45403af9c22da6fda5faa28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Sun, 1 Aug 2021 23:45:24 +0200 Subject: [PATCH 12/19] cleanups --- src/rust/ide/lib/ast/impl/src/lib.rs | 5 ++++ src/rust/ide/lib/ast/impl/src/macros.rs | 23 +++++++++++-------- src/rust/ide/lib/parser/src/lib.rs | 7 +++--- src/rust/ide/src/controller/graph.rs | 20 +++++++++------- .../ide/src/double_representation/comment.rs | 4 ++-- .../ide/src/double_representation/node.rs | 2 +- 6 files changed, 38 insertions(+), 23 deletions(-) diff --git a/src/rust/ide/lib/ast/impl/src/lib.rs b/src/rust/ide/lib/ast/impl/src/lib.rs index 6ff2a7b6ce..948db581a0 100644 --- a/src/rust/ide/lib/ast/impl/src/lib.rs +++ b/src/rust/ide/lib/ast/impl/src/lib.rs @@ -620,10 +620,14 @@ pub enum Escape { impl Block { + /// Calculate absolute indentation of lines in this block. pub fn indent(&self, parent_indent:usize) -> usize { parent_indent + self.indent } + /// Iterate over non-empty lines, while keeping their absolute indices. + /// + /// Note that leading empty lines are never indexed (unlike other empty lines in the block). pub fn enumerate_non_empty_lines(&self) -> impl Iterator)> + '_ where T:CloneRef { let first_line = std::iter::once((0,self.first_line.as_ref())); @@ -635,6 +639,7 @@ impl Block { } } +/// Iterate over non-empty lines, while maintaining their indices. pub fn enumerate_non_empty_lines<'a,T:'a>(iter:impl IntoIterator>> + 'a) -> impl Iterator)> + 'a { iter.into_iter().enumerate().filter_map(|(index,line):(usize,&BlockLine>)| { diff --git a/src/rust/ide/lib/ast/impl/src/macros.rs b/src/rust/ide/lib/ast/impl/src/macros.rs index 355d5de40a..0e0854b3ee 100644 --- a/src/rust/ide/lib/ast/impl/src/macros.rs +++ b/src/rust/ide/lib/ast/impl/src/macros.rs @@ -7,7 +7,8 @@ use crate::prelude::*; use crate::crumbs::AmbiguousCrumb; use crate::crumbs::Located; use crate::crumbs::MatchCrumb; -use crate::{known, BlockLine}; +use crate::known; +use crate::BlockLine; use crate::Shifted; @@ -130,6 +131,8 @@ impl DocumentationCommentLine { // === Full Description === +/// Structure holding the documentation comment AST and related information necessary to deal with +/// them. #[derive(Clone,Debug,Shrinkwrap)] pub struct DocumentationCommentInfo { /// Description of the line with the documentation comment. @@ -149,12 +152,13 @@ impl DocumentationCommentInfo { } /// Get the documentation text. - pub fn text(&self) -> String { - // This gets us documentation text, however non-first lines have the absolute indent - // whitespace preserved. Thus, we remove spurious indent, to keep only the relative indent - // to the comment's inner block (i.e. the right after the `##` introducer). + /// + /// The text is pretty printed as per UI perspective -- all lines leading whitespace is stripped + /// up to the column following comment introducer (`##`). + pub fn pretty_text(&self) -> String { let mut repr = self.body.repr(); - repr.extend(std::iter::repeat(' ').take(self.line.off)); // trailing whitespace + // Trailing whitespace must be maintained. + repr.extend(std::iter::repeat(' ').take(self.line.off)); let indent = self.block_indent + DOCUMENTATION_COMMENT_INTRODUCER.len(); let old = format!("\n{}", " ".repeat(indent)); let new = "\n"; @@ -162,10 +166,11 @@ impl DocumentationCommentInfo { } /// Get the documentation text. - pub fn text_to_repr(text:&str) -> String { + pub fn text_to_repr(context_indent:usize, text:&str) -> String { + let indent = " ".repeat(context_indent); let mut lines = text.lines(); let first_line = lines.next().map(|line| iformat!("##{line}")); - let other_lines = lines .map(|line| iformat!(" {line}")); + let other_lines = lines .map(|line| iformat!("{indent} {line}")); let mut out_lines = first_line.into_iter().chain(other_lines); out_lines.join("\n") } @@ -180,7 +185,7 @@ impl AsRef for DocumentationCommentInfo { impl Display for DocumentationCommentInfo { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f,"{}",self.text()) + write!(f,"{}",self.pretty_text()) } } diff --git a/src/rust/ide/lib/parser/src/lib.rs b/src/rust/ide/lib/parser/src/lib.rs index c1df2e002e..42d4ef328f 100644 --- a/src/rust/ide/lib/parser/src/lib.rs +++ b/src/rust/ide/lib/parser/src/lib.rs @@ -21,7 +21,8 @@ mod wsclient; use crate::prelude::*; -use ast::{Ast, BlockLine}; +use ast::Ast; +use ast::BlockLine; use ast::IdMap; use std::panic; use utils::fail::FallibleResult; @@ -100,13 +101,13 @@ impl Parser { } /// Program is expected to be single non-empty line module. The line's AST is - /// returned. Panics otherwise. The program is parsed with empty IdMap. + /// returned. The program is parsed with empty IdMap. pub fn parse_line_ast(&self, program:impl Str) -> FallibleResult { self.parse_line_with_id_map(program, default()).map(|line| line.elem) } /// Program is expected to be single non-empty line module. The line's AST is - /// returned. Panics otherwise. The program is parsed with empty IdMap. + /// returned. The program is parsed with empty IdMap. pub fn parse_line(&self, program:impl Str) -> FallibleResult> { self.parse_line_with_id_map(program, default()) } diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 501f942b48..5399ca1c2d 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -133,7 +133,6 @@ pub struct NewNodeInfo { pub location_hint : LocationHint, /// Introduce variable name for the node, making it into an assignment line. pub introduce_pattern : bool, - } impl NewNodeInfo { @@ -744,18 +743,23 @@ impl Handle { } } + /// Creates a proper description of a documentation comment in the context of this graph. + pub fn documentation_comment_from_pretty_text + (&self, pretty_text:&str) -> Option { + let indent = self.definition().ok()?.indent(); + let doc_repr = DocumentationCommentInfo::text_to_repr(indent,pretty_text); + let doc_line = self.parser.parse_line(doc_repr).ok()?; + DocumentationCommentInfo::new(&doc_line.as_ref(),indent) + } + /// Adds a new node to the graph and returns information about created node. pub fn add_node(&self, node:NewNodeInfo) -> FallibleResult { info!(self.logger, "Adding node with expression `{node.expression}`"); let expression_ast = self.parse_node_expression(&node.expression)?; let main_line = MainLine::from_ast(&expression_ast).ok_or(FailedToCreateNode)?; - let indent = self.definition()?.indent(); - let documentation = node.doc_comment.as_ref() - .map(|text| DocumentationCommentInfo::text_to_repr(&text)) - .map(|doc_code| self.parser.parse_line(doc_code)) - .transpose()? - .map(|doc_ast| DocumentationCommentInfo::new(&doc_ast.as_ref(),indent).ok_or(FailedToCreateNode)) - .transpose()?; + let documentation = node.doc_comment.as_ref().and_then(|pretty_text| { + self.documentation_comment_from_pretty_text(pretty_text) + }); let mut node_info = NodeInfo {documentation,main_line}; if let Some(desired_id) = node.id { diff --git a/src/rust/ide/src/double_representation/comment.rs b/src/rust/ide/src/double_representation/comment.rs index 3df5b24703..d40872ecc8 100644 --- a/src/rust/ide/src/double_representation/comment.rs +++ b/src/rust/ide/src/double_representation/comment.rs @@ -19,11 +19,11 @@ mod tests { let lines = main.block_lines(); let first_line = lines[0].transpose_ref().unwrap(); let doc = DocumentationCommentInfo::new(&first_line,main.indent()).unwrap(); - let text = doc.text(); + let text = doc.pretty_text(); assert_eq!(text, expected_comment_text); // Now, if we convert our pretty text to code, will it be the same as original line? - let code = DocumentationCommentInfo::text_to_repr(&text); + let code = DocumentationCommentInfo::text_to_repr(main.indent(),&text); let ast2 = parser.parse_line(&code).unwrap(); let doc2 = DocumentationCommentInfo::new(&ast2.as_ref(),main.indent()).expect(&format!("Failed to parse `{}` as comment",code)); assert_eq!(doc.line().repr(), doc2.line().repr()) diff --git a/src/rust/ide/src/double_representation/node.rs b/src/rust/ide/src/double_representation/node.rs index b61dba0191..bb76b9e35e 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -242,7 +242,7 @@ impl NodeInfo { /// Obtain documentation text. pub fn documentation_text(&self) -> Option { - self.documentation.as_ref().map(|doc| doc.text()) + self.documentation.as_ref().map(|doc| doc.pretty_text()) } } From 391143c25821e5df0f59186837587a2c9dac6fcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Sun, 1 Aug 2021 23:47:11 +0200 Subject: [PATCH 13/19] damn --- src/rust/ide/src/double_representation/comment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rust/ide/src/double_representation/comment.rs b/src/rust/ide/src/double_representation/comment.rs index d40872ecc8..98e2138aab 100644 --- a/src/rust/ide/src/double_representation/comment.rs +++ b/src/rust/ide/src/double_representation/comment.rs @@ -52,7 +52,7 @@ main = // Single line case with a single trailing space after `##`. let code = r#" main = - ## + ## node"#; let expected = " "; run_case(&parser, code,expected); From 14d11c0e316286d48e49cd2e8cbbcea8b5a4f6e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Sun, 1 Aug 2021 23:55:14 +0200 Subject: [PATCH 14/19] cleanups --- src/rust/ide/src/double_representation.rs | 11 ++++++++--- src/rust/ide/src/double_representation/node.rs | 12 ++++++++---- src/rust/ide/src/ide/integration/project.rs | 4 ---- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/rust/ide/src/double_representation.rs b/src/rust/ide/src/double_representation.rs index e115df3d86..b8ab021d4e 100644 --- a/src/rust/ide/src/double_representation.rs +++ b/src/rust/ide/src/double_representation.rs @@ -3,13 +3,18 @@ use crate::prelude::*; -use ast::{Ast, opr, prefix, known}; -use crate::double_representation::definition::{DefinitionName, DefinitionInfo}; +use crate::double_representation::definition::DefinitionInfo; +use crate::double_representation::definition::DefinitionName; use crate::double_representation::definition::ScopeKind; +use crate::double_representation::node::MainLine; + +use ast::Ast; use ast::crumbs::InfixCrumb; use ast::crumbs::Located; use ast::macros::DocumentationCommentAst; -use crate::double_representation::node::MainLine; +use ast::known; +use ast::opr; +use ast::prefix; pub mod alias_analysis; pub mod comment; diff --git a/src/rust/ide/src/double_representation/node.rs b/src/rust/ide/src/double_representation/node.rs index bb76b9e35e..807fb179aa 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -2,13 +2,17 @@ use crate::prelude::*; -use ast::{Ast, BlockLine, enumerate_non_empty_lines}; +use crate::double_representation::LineKind; +use crate::double_representation::definition::ScopeKind; + +use ast::Ast; +use ast::BlockLine; use ast::crumbs::Crumbable; +use ast::enumerate_non_empty_lines; use ast::known; +use ast::macros::DocumentationCommentInfo; +use ast::macros::DocumentationCommentLine; use std::cmp::Ordering; -use ast::macros::{DocumentationCommentInfo, DocumentationCommentLine}; -use crate::double_representation::LineKind; -use crate::double_representation::definition::ScopeKind; /// Node Id is the Ast Id attached to the node's expression. pub type Id = ast::Id; diff --git a/src/rust/ide/src/ide/integration/project.rs b/src/rust/ide/src/ide/integration/project.rs index 65e82faea8..6c1e0bc17c 100644 --- a/src/rust/ide/src/ide/integration/project.rs +++ b/src/rust/ide/src/ide/integration/project.rs @@ -858,13 +858,9 @@ impl Model { if let Some(node_view) = self.view.graph().model.nodes.get_cloned_ref(&id) { let comment_as_per_controller = node.info.documentation_text().unwrap_or_default(); let comment_as_per_view = node_view.comment.value(); - DEBUG!("Comment on node {node.info.ast()}: `{comment_as_per_controller}`, while view `{comment_as_per_view}`"); if comment_as_per_controller != comment_as_per_view { - INFO!("Setting comment `{comment_as_per_controller}`"); node_view.set_comment(comment_as_per_controller); } - } else { - DEBUG!("Cannot refresh on node that cannot be found!"); } } From c9f413f23751a7d1c6f60703239fed77520852b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Mon, 2 Aug 2021 02:45:32 +0200 Subject: [PATCH 15/19] cleanups, fixes --- src/rust/ide/lib/ast/impl/src/lib.rs | 116 ++++++++---------- src/rust/ide/lib/ast/impl/src/macros.rs | 13 +- src/rust/ide/src/controller/graph.rs | 2 +- src/rust/ide/src/double_representation.rs | 112 +++++++++++++---- .../ide/src/double_representation/comment.rs | 83 ------------- .../src/double_representation/definition.rs | 6 +- .../ide/src/double_representation/graph.rs | 40 ++---- .../ide/src/double_representation/node.rs | 28 +++-- 8 files changed, 174 insertions(+), 226 deletions(-) delete mode 100644 src/rust/ide/src/double_representation/comment.rs diff --git a/src/rust/ide/lib/ast/impl/src/lib.rs b/src/rust/ide/lib/ast/impl/src/lib.rs index 948db581a0..9d5a0a1552 100644 --- a/src/rust/ide/lib/ast/impl/src/lib.rs +++ b/src/rust/ide/lib/ast/impl/src/lib.rs @@ -618,27 +618,6 @@ pub enum Escape { // === Block === // ============= - -impl Block { - /// Calculate absolute indentation of lines in this block. - pub fn indent(&self, parent_indent:usize) -> usize { - parent_indent + self.indent - } - - /// Iterate over non-empty lines, while keeping their absolute indices. - /// - /// Note that leading empty lines are never indexed (unlike other empty lines in the block). - pub fn enumerate_non_empty_lines(&self) -> impl Iterator)> + '_ - where T:CloneRef { - let first_line = std::iter::once((0,self.first_line.as_ref())); - let further_lines = (1..).zip(self.lines.iter()).filter_map(|(index,line):(usize,&BlockLine>)| { - let non_empty_line = line.transpose_ref()?; - Some((index, non_empty_line)) - }); - first_line.chain(further_lines) - } -} - /// Iterate over non-empty lines, while maintaining their indices. pub fn enumerate_non_empty_lines<'a,T:'a>(iter:impl IntoIterator>> + 'a) -> impl Iterator)> + 'a { @@ -660,34 +639,6 @@ pub struct BlockLine { pub off: usize } -impl BlockLine { - pub fn as_ref(&self) -> BlockLine<&T> { - BlockLine { - elem : &self.elem, - off : self.off, - } - } - - pub fn map(self, f:impl FnOnce(T) -> U) -> BlockLine { - BlockLine { - elem : f(self.elem), - off : self.off - } - } -} - -impl BlockLine> { - pub fn transpose(self) -> Option> { - let off = self.off; - self.elem.map(|elem| BlockLine {elem,off}) - } - - pub fn transpose_ref(&self) -> Option> { - self.as_ref().map(Option::as_ref).transpose() - } -} - - // ============= // === Macro === @@ -1259,29 +1210,68 @@ impl BlockLine { pub fn new(elem:T) -> BlockLine { BlockLine {elem,off:0} } + + pub fn as_ref(&self) -> BlockLine<&T> { + BlockLine { + elem : &self.elem, + off : self.off, + } + } + + pub fn map(self, f:impl FnOnce(T) -> U) -> BlockLine { + BlockLine { + elem : f(self.elem), + off : self.off + } + } } +impl BlockLine> { + pub fn transpose(self) -> Option> { + let off = self.off; + self.elem.map(|elem| BlockLine {elem,off}) + } + + pub fn transpose_ref(&self) -> Option> { + self.as_ref().map(Option::as_ref).transpose() + } + + pub fn map_opt(self, f:impl FnOnce(T) -> U) -> BlockLine> { + self.map(|elem| elem.map(f)) + } +} + + impl Block { - /// Concatenate `Block`'s `first_line` with `lines` and returns a collection with all the lines. - pub fn all_lines(&self) -> Vec>> where T:Clone { - let mut lines = Vec::new(); - for off in &self.empty_lines { + pub fn iter_all_lines(&self) -> impl Iterator>> + '_ { + let indent = self.indent; + let leading_empty_lines = self.empty_lines.iter().map(move |off| { let elem = None; // TODO [mwu] // Empty lines use absolute indent, while BlockLines are relative to Block. // We might lose some data here, as empty lines shorter than block will get filled // with spaces. This is something that should be improved in the future but also // requires changes in the AST. - let off = off.checked_sub(self.indent).unwrap_or(0); - lines.push(BlockLine{elem,off}) - } + let off = off.saturating_sub(indent); + BlockLine {elem,off} + }); + + let first_line = std::iter::once(self.first_line.as_ref().map(|elem| Some(elem))); + let lines = self.lines.iter().map(|line| line.as_ref().map(|elem| elem.as_ref())); + leading_empty_lines.chain(first_line).chain(lines) + } + + /// Calculate absolute indentation of lines in this block. + pub fn indent(&self, parent_indent:usize) -> usize { + parent_indent + self.indent + } - let first_line = self.first_line.clone(); - let elem = Some(first_line.elem); - let off = first_line.off; - lines.push(BlockLine{elem,off}); - lines.extend(self.lines.iter().cloned()); - lines + /// Iterate over non-empty lines, while keeping their absolute indices. + pub fn enumerate_non_empty_lines(&self) -> impl Iterator)> + '_ { + self.iter_all_lines().enumerate().filter_map(|(index,line):(usize,BlockLine>)| { + let non_empty_line = line.transpose()?; + Some((index, non_empty_line)) + }) } } @@ -1726,7 +1716,7 @@ mod tests { let expected_repr = "\n \n head \n tail0 \n \n tail2 "; assert_eq!(block.repr(), expected_repr); - let all_lines = block.all_lines(); + let all_lines = block.iter_all_lines().collect_vec(); let (empty_line,head_line,tail0,tail1,tail2) = all_lines.iter().expect_tuple(); assert!(empty_line.elem.is_none()); assert_eq!(empty_line.off,1); // other 4 indents are provided by Block diff --git a/src/rust/ide/lib/ast/impl/src/macros.rs b/src/rust/ide/lib/ast/impl/src/macros.rs index 0e0854b3ee..6f0d3e9596 100644 --- a/src/rust/ide/lib/ast/impl/src/macros.rs +++ b/src/rust/ide/lib/ast/impl/src/macros.rs @@ -64,6 +64,7 @@ pub fn is_disable_comment(ast:&Ast) -> bool { // === Ast Description === /// Describes the AST of a documentation comment. +#[derive(Clone,Debug)] pub struct DocumentationCommentAst { ast : known::Match, body : crate::MacroPatternMatch>, @@ -82,6 +83,11 @@ impl DocumentationCommentAst { None } } + + /// Get the documentation comment's AST. + pub fn ast(&self) -> known::Match { + self.ast.clone_ref() + } } @@ -169,9 +175,10 @@ impl DocumentationCommentInfo { pub fn text_to_repr(context_indent:usize, text:&str) -> String { let indent = " ".repeat(context_indent); let mut lines = text.lines(); - let first_line = lines.next().map(|line| iformat!("##{line}")); - let other_lines = lines .map(|line| iformat!("{indent} {line}")); - let mut out_lines = first_line.into_iter().chain(other_lines); + // First line must always exist, even for an empty comment. + let first_line = format!("##{}",lines.next().unwrap_or_default()); + let other_lines = lines.map(|line| iformat!("{indent} {line}")); + let mut out_lines = std::iter::once(first_line).chain(other_lines); out_lines.join("\n") } } diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 5399ca1c2d..984fdc08af 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -1296,7 +1296,7 @@ main = }) } - #[test] + #[wasm_bindgen_test] fn graph_controller_node_operations_node() { let mut test = Fixture::set_up(); const PROGRAM:&str = r" diff --git a/src/rust/ide/src/double_representation.rs b/src/rust/ide/src/double_representation.rs index b8ab021d4e..a2fcff1d84 100644 --- a/src/rust/ide/src/double_representation.rs +++ b/src/rust/ide/src/double_representation.rs @@ -3,10 +3,8 @@ use crate::prelude::*; -use crate::double_representation::definition::DefinitionInfo; use crate::double_representation::definition::DefinitionName; use crate::double_representation::definition::ScopeKind; -use crate::double_representation::node::MainLine; use ast::Ast; use ast::crumbs::InfixCrumb; @@ -17,7 +15,6 @@ use ast::opr; use ast::prefix; pub mod alias_analysis; -pub mod comment; pub mod connection; pub mod definition; pub mod graph; @@ -53,10 +50,16 @@ pub const INDENT : usize = 4; // ======================== /// What kind of node or definition a line should be treated as. +#[derive(Clone,Debug)] pub enum LineKind { /// Definition is a binding, which defines a new entity with arguments. Definition { - get_definition : Box DefinitionInfo> + /// The binding that introduces the definition. + ast:known::Infix, + /// Name of this definition. Includes typename, if this is an extension method. + name:Located, + /// Arguments for this definition. Does not include any implicit ones (e.g. no `this`). + args:Vec>, }, /// Node in a binding form. ExpressionAssignment { @@ -77,22 +80,14 @@ pub enum LineKind { } impl LineKind { - pub fn into_node_main_line(self) -> Option { - match self { - LineKind::ExpressionAssignment {ast} => MainLine::new_binding(ast), - LineKind::ExpressionPlain {ast} => MainLine::new_expression(ast), - LineKind::DocumentationComment {..} => None, - LineKind::Definition {..} => None, - } - } - /// Tell how the given line (described by an Ast) should be treated. + // TODO [mwu] This method deserves unit tests of its own. pub fn discern(ast:&Ast, kind:ScopeKind) -> Self { use LineKind::*; // First of all, if non-empty line is not an infix (i.e. binding) it can be only a node or // a documentation comment. - let infix = match opr::to_assignment(ast) { + let ast = match opr::to_assignment(ast) { Some(infix) => infix, None => @@ -109,7 +104,7 @@ impl LineKind { // For definition it is a prefix chain, where first is the name, then arguments (if explicit). // For node it is a pattern, either in a form of Var without args on Cons application. let crumb = InfixCrumb::LeftOperand; - let lhs = Located::new(crumb,prefix::Chain::from_ast_non_strict(&infix.larg)); + let lhs = Located::new(crumb,prefix::Chain::from_ast_non_strict(&ast.larg)); let name = lhs.entered(|chain| { let name_ast = chain.located_func(); name_ast.map(DefinitionName::from_ast) @@ -119,7 +114,7 @@ impl LineKind { // e.g. for `Point x y = get_point …` let name = match name { Some(name) => name, - None => return ExpressionAssignment{ast:infix} + None => return ExpressionAssignment{ast} }; let args = lhs.enumerate_args().map(|Located{crumbs,item}| { @@ -139,20 +134,11 @@ impl LineKind { // e.g. `point = Point 5 10` let is_node = args.is_empty(); if is_setter || is_node { - return ExpressionAssignment{ast:infix} + return ExpressionAssignment{ast} } }; - Definition { - get_definition: Box::new(|context_indent| { - DefinitionInfo { - context_indent, - ast : infix, - name, - args - } - }) - } + Definition {ast,name,args} } } @@ -168,6 +154,78 @@ impl LineKind { #[cfg(test)] mod tests { use super::*; + use crate::double_representation::definition::DefinitionProvider; + use ast::macros::DocumentationCommentInfo; + use parser::Parser; + + + /// Expect `main` method, where first line is a documentation comment. + /// The text of this comment should match the expected one. + fn run_case(parser:&Parser, code:&str, expected_comment_text:&str) { + let ast = parser.parse_module(code,default()).unwrap(); + let main_id = double_representation::definition::Id::new_plain_name("main"); + let main = double_representation::module::get_definition(&ast,&main_id).unwrap(); + let lines = main.block_lines(); + let first_line = lines[0].transpose_ref().unwrap(); + let doc = DocumentationCommentInfo::new(&first_line,main.indent()).unwrap(); + let text = doc.pretty_text(); + assert_eq!(text, expected_comment_text); + + // Now, if we convert our pretty text to code, will it be the same as original line? + let code = DocumentationCommentInfo::text_to_repr(main.indent(),&text); + let ast2 = parser.parse_line(&code).unwrap(); + let doc2 = DocumentationCommentInfo::new(&ast2.as_ref(),main.indent()).expect(&format!("Failed to parse `{}` as comment",code)); + assert_eq!(doc.line().repr(), doc2.line().repr()) + } + #[wasm_bindgen_test] + fn parse_single_line_comment() { + let parser = parser::Parser::new_or_panic(); + + // Typical single line case. + let code = r#" +main = + ## Single line + node"#; + let expected = " Single line"; + run_case(&parser, code,expected); + + // Single line case without space after `##`. + let code = r#" +main = + ##Single line + node"#; + let expected = "Single line"; + run_case(&parser, code,expected); + + // Single line case with a single trailing space after `##`. + let code = r#" +main = + ## + node"#; + let expected = " "; + run_case(&parser, code,expected); + + // Single line case without content. + let code = r#" +main = + ## + node"#; + let expected = ""; + run_case(&parser, code,expected); + + } + + #[wasm_bindgen_test] + fn parse_multi_line_comment() { + let parser = parser::Parser::new_or_panic(); + let code = r#" +main = + ## First line + Second line + node"#; + let expected = " First line\n Second line"; + run_case(&parser, code,expected); + } } diff --git a/src/rust/ide/src/double_representation/comment.rs b/src/rust/ide/src/double_representation/comment.rs deleted file mode 100644 index 98e2138aab..0000000000 --- a/src/rust/ide/src/double_representation/comment.rs +++ /dev/null @@ -1,83 +0,0 @@ -use crate::prelude::*; - -// FIXME move to other parser-related tests - -#[cfg(test)] -mod tests { - use super::*; - use crate::double_representation::definition::DefinitionProvider; - use ast::macros::DocumentationCommentInfo; - use parser::Parser; - - - /// Expect `main` method, where first line is a documentation comment. - /// The text of this comment should match the expected one. - fn run_case(parser:&Parser, code:&str, expected_comment_text:&str) { - let ast = parser.parse_module(code,default()).unwrap(); - let main_id = double_representation::definition::Id::new_plain_name("main"); - let main = double_representation::module::get_definition(&ast,&main_id).unwrap(); - let lines = main.block_lines(); - let first_line = lines[0].transpose_ref().unwrap(); - let doc = DocumentationCommentInfo::new(&first_line,main.indent()).unwrap(); - let text = doc.pretty_text(); - assert_eq!(text, expected_comment_text); - - // Now, if we convert our pretty text to code, will it be the same as original line? - let code = DocumentationCommentInfo::text_to_repr(main.indent(),&text); - let ast2 = parser.parse_line(&code).unwrap(); - let doc2 = DocumentationCommentInfo::new(&ast2.as_ref(),main.indent()).expect(&format!("Failed to parse `{}` as comment",code)); - assert_eq!(doc.line().repr(), doc2.line().repr()) - } - - #[test] - fn parse_single_line_comment() { - let parser = parser::Parser::new_or_panic(); - - // Typical single line case. - let code = r#" -main = - ## Single line - node"#; - let expected = " Single line"; - run_case(&parser, code,expected); - - // Single line case without space after `##`. - let code = r#" -main = - ##Single line - node"#; - let expected = "Single line"; - run_case(&parser, code,expected); - - // Single line case with a single trailing space after `##`. - let code = r#" -main = - ## - node"#; - let expected = " "; - run_case(&parser, code,expected); - - // Single line case without content. - let code = r#" -main = - - ## - node"#; - let expected = ""; - run_case(&parser, code,expected); - - } - - #[test] - fn parse_multi_line_comment() { - let parser = parser::Parser::new_or_panic(); - let code = r#" -main = - ## First line - Second line - node"#; - let expected = " First line\n Second line"; - run_case(&parser, code,expected); - } - -} diff --git a/src/rust/ide/src/double_representation/definition.rs b/src/rust/ide/src/double_representation/definition.rs index fbba500f3f..281e69c564 100644 --- a/src/rust/ide/src/double_representation/definition.rs +++ b/src/rust/ide/src/double_representation/definition.rs @@ -245,7 +245,7 @@ impl DefinitionInfo { /// `Infix`, it returns a single `BlockLine`. pub fn block_lines(&self) -> Vec>> { if let Ok(block) = known::Block::try_from(*self.body()) { - block.all_lines() + block.iter_all_lines().map(|line| line.map_opt(CloneRef::clone_ref)).collect() } else { let elem = Some((*self.body()).clone()); let off = 0; @@ -313,8 +313,8 @@ impl DefinitionInfo { /// some binding or other kind of subtree). pub fn from_line_ast (ast:&Ast, kind:ScopeKind, context_indent:usize) -> Option { - if let LineKind::Definition{get_definition} = LineKind::discern(ast,kind) { - Some(get_definition(context_indent)) + if let LineKind::Definition{ast,args,name} = LineKind::discern(ast,kind) { + Some(DefinitionInfo {ast,args,name,context_indent}) } else { None } diff --git a/src/rust/ide/src/double_representation/graph.rs b/src/rust/ide/src/double_representation/graph.rs index 320087159e..49e945028d 100644 --- a/src/rust/ide/src/double_representation/graph.rs +++ b/src/rust/ide/src/double_representation/graph.rs @@ -50,6 +50,7 @@ pub struct GraphInfo { } impl GraphInfo { + /// Look for a node with given id in the graph. pub fn locate_node(&self, id:double_representation::node::Id) -> FallibleResult { let lines = self.source.block_lines(); double_representation::node::locate(&lines, self.source.context_indent, id) @@ -91,24 +92,6 @@ impl GraphInfo { double_representation::connection::list(&self.source.ast.rarg) } - /// Adds a new node to this graph. - pub fn add_line - (&mut self, line_ast:Ast, location_hint:LocationHint) -> FallibleResult { - let mut lines = self.block_lines(); - let last_non_empty = || lines.iter().rposition(|line| line.elem.is_some()); - let index = match location_hint { - LocationHint::Start => 0, - LocationHint::End => last_non_empty().map_or(lines.len(),|ix| ix + 1), - LocationHint::After(id) => self.locate_node(id)?.index.last() + 1, - LocationHint::Before(id) => self.locate_node(id)?.index.first() - }; - let elem = Some(line_ast); - let off = 0; - lines.insert(index,BlockLine{elem,off}); - self.source.set_block_lines(lines) - // FIXME unify with below? - } - /// Adds a new node to this graph. pub fn add_node (&mut self, node:&NodeInfo, location_hint:LocationHint) -> FallibleResult { @@ -124,10 +107,8 @@ impl GraphInfo { let off = 0; lines.insert(index,BlockLine{elem,off}); if let Some(documentation) = &node.documentation { - let line = BlockLine { - elem : Some(documentation.ast().into()), - off, - }; + let elem = Some(documentation.ast().into()); + let line = BlockLine {elem,off}; lines.insert(index,line); } self.source.set_block_lines(lines) @@ -270,11 +251,8 @@ mod tests { NodeInfo::from_main_line_ast(&line_ast).unwrap() } - fn assert_at(nodes:&[NodeInfo], index:usize, expected:&NodeInfo) { - assert_same(&nodes[index],expected) - } - fn assert_all(nodes:&[NodeInfo], expected:&[NodeInfo]) { + assert_eq!(nodes.len(), expected.len()); for (left,right) in nodes.iter().zip(expected) { assert_same(left,right) } @@ -294,7 +272,8 @@ mod tests { let mut graph = main_graph(&parser, program); let nodes = graph.nodes(); assert_eq!(nodes.len(), 1); - assert_eq!(nodes[0].expression().repr(), "print \"hello\""); + let initial_node = nodes[0].clone(); + assert_eq!(initial_node.expression().repr(), "print \"hello\""); let expr0 = "a + 2"; let expr1 = "b + 3"; @@ -306,12 +285,7 @@ mod tests { graph.add_node(&node_to_add1,LocationHint::Before(graph.nodes()[0].id())).unwrap(); let nodes = graph.nodes(); - assert_eq!(nodes.len(), 3); - assert_eq!(nodes[0].expression().repr(), expr1); - assert_eq!(nodes[0].id(), node_to_add1.id()); - assert_eq!(nodes[1].expression().repr(), expr0); - assert_eq!(nodes[1].id(), node_to_add0.id()); - assert_eq!(nodes[2].expression().repr(), "print \"hello\""); + assert_all(nodes.as_slice(), &[node_to_add1, node_to_add0, initial_node]); } #[wasm_bindgen_test] diff --git a/src/rust/ide/src/double_representation/node.rs b/src/rust/ide/src/double_representation/node.rs index 807fb179aa..73e66ce520 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -183,20 +183,19 @@ impl<'a, T:Iterator)> + 'a> Iterator for NodeIter indexed_documentation = None; } line => { - if let Some(main_line) = line.into_node_main_line() { + if let Some(main_line) = MainLine::from_discerned_line(line) { let (documentation_line,documentation) = match indexed_documentation { Some((index,documentation)) => (Some(index),Some(documentation)), None => (None,None) }; - - let ret = LocatedNode { - node: NodeInfo { documentation, main_line }, - index: NodeLocation { - main_line: index, - documentation_line, - } + + let node = NodeInfo {documentation,main_line}; + let index = NodeLocation { + main_line: index, + documentation_line, }; - return Some(ret) + + return Some(LocatedNode {node,index}) } } } @@ -284,16 +283,19 @@ impl MainLine { // By definition, there are no nodes in the root scope. // Being a node's line, we may assume that this is not a root scope. let scope = ScopeKind::NonRoot; - // Nodes, unlike documentation and definitions, are insensitive to parent block indent. - // Thus, we can just assume any bogus value. - match LineKind::discern(ast,scope) { + Self::from_discerned_line(LineKind::discern(ast,scope)) + } + + /// Try retrieving node information from an already discerned line data. + pub fn from_discerned_line(line:LineKind) -> Option { + match line { LineKind::ExpressionPlain {ast} => Self::new_expression(ast), LineKind::ExpressionAssignment {ast} => Self::new_binding(ast), LineKind::Definition {..} => None, LineKind::DocumentationComment {..} => None, } } - + /// Tries to interpret AST as node, treating whole AST as an expression. pub fn from_block_line(line:&BlockLine>) -> Option { Self::from_ast(line.elem.as_ref()?) From 1beb91e8b28b151310595f25dbc86afc0f68fa34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Mon, 2 Aug 2021 02:47:03 +0200 Subject: [PATCH 16/19] clippy, minor --- src/rust/ide/lib/ast/impl/src/lib.rs | 2 +- src/rust/ide/src/double_representation/definition.rs | 2 +- src/rust/ide/src/double_representation/graph.rs | 5 +++-- src/rust/ide/src/double_representation/node.rs | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/rust/ide/lib/ast/impl/src/lib.rs b/src/rust/ide/lib/ast/impl/src/lib.rs index 9d5a0a1552..f973996cd8 100644 --- a/src/rust/ide/lib/ast/impl/src/lib.rs +++ b/src/rust/ide/lib/ast/impl/src/lib.rs @@ -1256,7 +1256,7 @@ impl Block { BlockLine {elem,off} }); - let first_line = std::iter::once(self.first_line.as_ref().map(|elem| Some(elem))); + let first_line = std::iter::once(self.first_line.as_ref().map(Some)); let lines = self.lines.iter().map(|line| line.as_ref().map(|elem| elem.as_ref())); leading_empty_lines.chain(first_line).chain(lines) } diff --git a/src/rust/ide/src/double_representation/definition.rs b/src/rust/ide/src/double_representation/definition.rs index 281e69c564..fcb9729505 100644 --- a/src/rust/ide/src/double_representation/definition.rs +++ b/src/rust/ide/src/double_representation/definition.rs @@ -314,7 +314,7 @@ impl DefinitionInfo { pub fn from_line_ast (ast:&Ast, kind:ScopeKind, context_indent:usize) -> Option { if let LineKind::Definition{ast,args,name} = LineKind::discern(ast,kind) { - Some(DefinitionInfo {ast,args,name,context_indent}) + Some(DefinitionInfo {ast,name,args,context_indent}) } else { None } diff --git a/src/rust/ide/src/double_representation/graph.rs b/src/rust/ide/src/double_representation/graph.rs index 49e945028d..1e7e2ead39 100644 --- a/src/rust/ide/src/double_representation/graph.rs +++ b/src/rust/ide/src/double_representation/graph.rs @@ -2,7 +2,8 @@ use crate::prelude::*; -use crate::double_representation::definition::{DefinitionInfo, DefinitionProvider}; +use crate::double_representation::definition::DefinitionInfo; +use crate::double_representation::definition::DefinitionProvider; use crate::double_representation::node; use crate::double_representation::node::LocatedNode; use crate::double_representation::node::NodeInfo; @@ -74,7 +75,7 @@ impl GraphInfo { if let Ok(body_block) = known::Block::try_new(body.clone()) { let context_indent = self.source.indent(); let lines_iter = body_block.enumerate_non_empty_lines(); - let nodes_iter = node::NodeIterator {context_indent,lines_iter}; + let nodes_iter = node::NodeIterator {lines_iter,context_indent}; nodes_iter.map(|n| n.node).collect() } else if let Some(node) = node::NodeInfo::from_main_line_ast(&body) { // There's no way to attach a documentation comment to an inline node, it consists only diff --git a/src/rust/ide/src/double_representation/node.rs b/src/rust/ide/src/double_representation/node.rs index 73e66ce520..9c373bdd32 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -195,7 +195,7 @@ impl<'a, T:Iterator)> + 'a> Iterator for NodeIter documentation_line, }; - return Some(LocatedNode {node,index}) + return Some(LocatedNode {index,node}) } } } From 99c1cf90e9a553f3b6bf12f84b88dc951c4c8232 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Mon, 2 Aug 2021 02:48:23 +0200 Subject: [PATCH 17/19] prettier --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 978bfe3889..eddab99f89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,7 @@ these updates be shipped in a stable release before the end of the year. - [New look of open project dialog.][1700]. Now it has "Open project" title on the top. -- [Documentation cooments are displayed next to the nodes.][1744]. - +- [Documentation cooments are displayed next to the nodes.][1744]. #### Enso Compiler From ebf472af6f031521dbb0a5e79a8498de3cf04186 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Mon, 2 Aug 2021 03:02:15 +0200 Subject: [PATCH 18/19] cleanups --- src/rust/ide/lib/ast/impl/src/macros.rs | 2 +- src/rust/ide/src/double_representation/graph.rs | 4 ++-- src/rust/ide/src/double_representation/node.rs | 17 ++--------------- .../ide/view/graph-editor/src/component/node.rs | 4 ++++ 4 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/rust/ide/lib/ast/impl/src/macros.rs b/src/rust/ide/lib/ast/impl/src/macros.rs index 6f0d3e9596..4d4734d28b 100644 --- a/src/rust/ide/lib/ast/impl/src/macros.rs +++ b/src/rust/ide/lib/ast/impl/src/macros.rs @@ -171,7 +171,7 @@ impl DocumentationCommentInfo { repr.replace(&old,new) } - /// Get the documentation text. + /// Generates the source code text of the comment line from a pretty text. pub fn text_to_repr(context_indent:usize, text:&str) -> String { let indent = " ".repeat(context_indent); let mut lines = text.lines(); diff --git a/src/rust/ide/src/double_representation/graph.rs b/src/rust/ide/src/double_representation/graph.rs index 1e7e2ead39..86502ec971 100644 --- a/src/rust/ide/src/double_representation/graph.rs +++ b/src/rust/ide/src/double_representation/graph.rs @@ -77,7 +77,7 @@ impl GraphInfo { let lines_iter = body_block.enumerate_non_empty_lines(); let nodes_iter = node::NodeIterator {lines_iter,context_indent}; nodes_iter.map(|n| n.node).collect() - } else if let Some(node) = node::NodeInfo::from_main_line_ast(&body) { + } else if let Some(node) = node::NodeInfo::from_main_line_ast(body) { // There's no way to attach a documentation comment to an inline node, it consists only // of the main line. vec![node] @@ -201,10 +201,10 @@ mod tests { use crate::double_representation::module::get_definition; use ast::HasRepr; + use ast::macros::DocumentationCommentInfo; use ast::test_utils::expect_single_line; use utils::test::ExpectTuple; use wasm_bindgen_test::wasm_bindgen_test; - use ast::macros::DocumentationCommentInfo; wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser); diff --git a/src/rust/ide/src/double_representation/node.rs b/src/rust/ide/src/double_representation/node.rs index 9c373bdd32..f440a20c6d 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -150,25 +150,12 @@ pub struct NodeIterator<'a, T:Iterator)> + 'a> { pub context_indent:usize, } - - -// pub fn block_nodes<'a, T:Iterator)> + 'a> { -// -// -// impl<'a, T:Iterator)> + 'a> NodeIterator<'a, T> { -// pub fn from_block(block:&known::Block, parent_indent:usize) -> Self { -// let lines_iter = block.enumerate_non_empty_lines(); -// let context_indent = block.indent + parent_indent; -// NodeIterator {lines_iter,context_indent} -// } -// } - impl<'a, T:Iterator)> + 'a> Iterator for NodeIterator<'a, T> { type Item = LocatedNode; fn next(&mut self) -> Option { let mut indexed_documentation = None; - while let Some((index, line)) = self.lines_iter.next() { + for (index,line) in &mut self.lines_iter { match LineKind::discern(line.elem, ScopeKind::NonRoot) { LineKind::DocumentationComment {documentation} => { let doc_line = DocumentationCommentLine::from_doc_ast(documentation,line.off); @@ -231,7 +218,7 @@ impl NodeInfo { expression_id_matches() || doc_comment_id_matches() } - /// TODO should not be needed as a method here + /// Get the ast id of the line with the node comment (if present). pub fn doc_comment_id(&self) -> Option { self.documentation.as_ref().and_then(|comment| comment.ast().id()) } diff --git a/src/rust/ide/view/graph-editor/src/component/node.rs b/src/rust/ide/view/graph-editor/src/component/node.rs index 8f5bf4b6d3..1489ebcad7 100644 --- a/src/rust/ide/view/graph-editor/src/component/node.rs +++ b/src/rust/ide/view/graph-editor/src/component/node.rs @@ -76,6 +76,10 @@ const UNRESOLVED_SYMBOL_TYPE : &str = "Builtins.Main.Unresolved_Symbol"; // === Comment === // =============== +/// String with documentation comment text for this node. +/// +/// This is just a plain string, as this is what text area expects and node just redirects this +/// value, pub type Comment = String; From 9dd201c67e330268b1da11ba20f9f4cd75495762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Mon, 2 Aug 2021 11:18:18 +0200 Subject: [PATCH 19/19] CR feedback --- src/rust/ide/lib/ast/impl/src/lib.rs | 24 ++++++++++++++---------- src/rust/ide/src/controller/graph.rs | 3 +-- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/rust/ide/lib/ast/impl/src/lib.rs b/src/rust/ide/lib/ast/impl/src/lib.rs index f973996cd8..bb57286036 100644 --- a/src/rust/ide/lib/ast/impl/src/lib.rs +++ b/src/rust/ide/lib/ast/impl/src/lib.rs @@ -618,16 +618,6 @@ pub enum Escape { // === Block === // ============= -/// Iterate over non-empty lines, while maintaining their indices. -pub fn enumerate_non_empty_lines<'a,T:'a>(iter:impl IntoIterator>> + 'a) --> impl Iterator)> + 'a { - iter.into_iter().enumerate().filter_map(|(index,line):(usize,&BlockLine>)| { - let non_empty_line = line.transpose_ref()?; - Some((index, non_empty_line)) - }) -} - - #[ast_node] pub enum BlockType {Continuous {} , Discontinuous {}} /// Holder for line in `Block` or `Module`. Lines store value of `T` and trailing whitespace info. @@ -1211,6 +1201,7 @@ impl BlockLine { BlockLine {elem,off:0} } + /// Convert `&BlockLine` into `BlockLine<&T>`. pub fn as_ref(&self) -> BlockLine<&T> { BlockLine { elem : &self.elem, @@ -1218,6 +1209,7 @@ impl BlockLine { } } + /// Maps `BlockLine` into `BlockLine` using the provided function. pub fn map(self, f:impl FnOnce(T) -> U) -> BlockLine { BlockLine { elem : f(self.elem), @@ -1227,22 +1219,34 @@ impl BlockLine { } impl BlockLine> { + /// Transpose a `BlockLine>` into `Option>`. pub fn transpose(self) -> Option> { let off = self.off; self.elem.map(|elem| BlockLine {elem,off}) } + /// Transpose a `&BlockLine>` into `Option>`. pub fn transpose_ref(&self) -> Option> { self.as_ref().map(Option::as_ref).transpose() } + /// Map the inner contents of the line's stored element. pub fn map_opt(self, f:impl FnOnce(T) -> U) -> BlockLine> { self.map(|elem| elem.map(f)) } } +/// Iterate over non-empty lines, while maintaining their indices. +pub fn enumerate_non_empty_lines<'a,T:'a>(iter:impl IntoIterator>> + 'a) +-> impl Iterator)> + 'a { + iter.into_iter().enumerate().filter_map(|(index,line):(usize,&BlockLine>)| { + let non_empty_line = line.transpose_ref()?; + Some((index, non_empty_line)) + }) +} impl Block { + /// Iterates over all lines in the block, including leading empty lines. pub fn iter_all_lines(&self) -> impl Iterator>> + '_ { let indent = self.indent; let leading_empty_lines = self.empty_lines.iter().map(move |off| { diff --git a/src/rust/ide/src/controller/graph.rs b/src/rust/ide/src/controller/graph.rs index 984fdc08af..f40038da38 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -658,9 +658,8 @@ impl Handle { } }; - // FIXME block lines are not necessarily in a block let mut lines = graph.block_lines(); - let range = NodeLocation::range(node_to_be_after.index, node_to_be_before.index); + let range = NodeLocation::range(node_to_be_after.index, node_to_be_before.index); lines[range].sort_by_key(should_be_at_end); self.update_definition_ast(|mut def| { def.set_block_lines(lines)?;