From fdeed13e6cf418405e0bbc3efeabb4805e79735e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wawrzyniec=20Urba=C5=84czyk?= Date: Mon, 2 Aug 2021 12:27:25 +0200 Subject: [PATCH] Add support for comments on nodes (#1744) --- CHANGELOG.md | 2 + .../ensogl/lib/text/src/component/area.rs | 23 +- src/rust/ide/lib/ast/impl/src/crumbs.rs | 7 +- src/rust/ide/lib/ast/impl/src/lib.rs | 82 ++++- src/rust/ide/lib/ast/impl/src/macros.rs | 190 ++++++++++- src/rust/ide/lib/parser/src/lib.rs | 26 +- 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 | 93 +++-- src/rust/ide/src/controller/project.rs | 2 +- src/rust/ide/src/controller/searcher.rs | 29 +- .../ide/src/controller/searcher/action.rs | 2 +- src/rust/ide/src/controller/upload.rs | 1 + src/rust/ide/src/double_representation.rs | 200 +++++++++++ .../double_representation/alias_analysis.rs | 23 +- .../alias_analysis/test_utils.rs | 10 +- .../src/double_representation/connection.rs | 6 +- .../src/double_representation/definition.rs | 64 +--- .../ide/src/double_representation/graph.rs | 229 +++++++------ .../ide/src/double_representation/module.rs | 4 +- .../ide/src/double_representation/node.rs | 321 ++++++++++++++---- .../refactorings/collapse.rs | 34 +- .../ide/src/ide/integration/file_system.rs | 8 +- src/rust/ide/src/ide/integration/project.rs | 13 + .../view/graph-editor/src/component/node.rs | 34 +- src/rust/ide/view/graph-editor/src/lib.rs | 21 +- .../ide/view/src/debug_scenes/interface.rs | 9 +- 31 files changed, 1113 insertions(+), 370 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b29bfae62..eddab99f89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +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]. #### Enso Compiler @@ -31,6 +32,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/ensogl/lib/text/src/component/area.rs b/src/rust/ensogl/lib/text/src/component/area.rs index e92ef5587a..5c84751cdd 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), @@ -600,7 +601,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) @@ -677,7 +678,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(); @@ -690,19 +691,25 @@ 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 { + self.lines.len() as f32 * LINE_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/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..bb57286036 100644 --- a/src/rust/ide/lib/ast/impl/src/lib.rs +++ b/src/rust/ide/lib/ast/impl/src/lib.rs @@ -630,7 +630,6 @@ pub struct BlockLine { } - // ============= // === Macro === // ============= @@ -1201,29 +1200,82 @@ impl BlockLine { pub fn new(elem:T) -> BlockLine { BlockLine {elem,off:0} } + + /// Convert `&BlockLine` into `BlockLine<&T>`. + pub fn as_ref(&self) -> BlockLine<&T> { + BlockLine { + elem : &self.elem, + off : self.off, + } + } + + /// Maps `BlockLine` into `BlockLine` using the provided function. + pub fn map(self, f:impl FnOnce(T) -> U) -> BlockLine { + BlockLine { + elem : f(self.elem), + off : self.off + } + } +} + +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 { - /// 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 { + /// 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| { 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(Some)); + 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)) + }) } } @@ -1668,7 +1720,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 7d80dc97a8..4d4734d28b 100644 --- a/src/rust/ide/lib/ast/impl/src/macros.rs +++ b/src/rust/ide/lib/ast/impl/src/macros.rs @@ -8,21 +8,205 @@ use crate::crumbs::AmbiguousCrumb; use crate::crumbs::Located; use crate::crumbs::MatchCrumb; use crate::known; +use crate::BlockLine; +use crate::Shifted; -// =============== -// === Imports === -// =============== +// ================================== +// === 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: /// 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"; + + +// ======================== +// === 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; + if crate::identifier::name(&first_segment.head) == Some(DISABLING_COMMENT_INTRODUCER) { + Some(first_segment.body.repr()) + } else { + None + } +} + +/// Check if this AST is a disabling comment. +pub fn is_disable_comment(ast:&Ast) -> bool { + as_disable_comment(ast).is_some() +} + + + +// ============================== +// === Documentation Comments === +// ============================== + +// === Ast Description === + +/// Describes the AST of a documentation comment. +#[derive(Clone,Debug)] +pub struct DocumentationCommentAst { + ast : known::Match, + body : crate::MacroPatternMatch>, +} + +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(DocumentationCommentAst {ast,body}) + } else { + None + } + } + + /// Get the documentation comment's AST. + pub fn ast(&self) -> known::Match { + self.ast.clone_ref() + } +} + + +// === 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. + pub fn ast(&self) -> known::Match { + 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 === + +/// 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. + #[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. + /// + /// 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(); + // 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"; + repr.replace(&old,new) + } + + /// 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(); + // 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") + } +} + + +impl AsRef for DocumentationCommentInfo { + fn as_ref(&self) -> &Ast { + self.line.elem.ast() + } +} + +impl Display for DocumentationCommentInfo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f,"{}",self.pretty_text()) + } +} + +/// Check if given Ast stores a documentation comment. +pub fn is_documentation_comment(ast:&Ast) -> bool { + DocumentationCommentAst::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/lib/parser/src/lib.rs b/src/rust/ide/lib/parser/src/lib.rs index 5e7186a0ed..42d4ef328f 100644 --- a/src/rust/ide/lib/parser/src/lib.rs +++ b/src/rust/ide/lib/parser/src/lib.rs @@ -22,6 +22,7 @@ mod wsclient; use crate::prelude::*; use ast::Ast; +use ast::BlockLine; use ast::IdMap; use std::panic; use utils::fail::FallibleResult; @@ -100,17 +101,28 @@ 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()) + /// 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. - 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. 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 cdbb44491b..f40038da38 100644 --- a/src/rust/ide/src/controller/graph.rs +++ b/src/rust/ide/src/controller/graph.rs @@ -8,17 +8,21 @@ 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; 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::NodeLocation; use crate::double_representation::node::NodeInfo; use crate::model::module::NodeMetadata; use crate::model::traits::*; use ast::crumbs::InfixCrumb; +use ast::macros::DocumentationCommentInfo; use enso_protocol::language_server; use parser::Parser; use span_tree::SpanTree; @@ -119,6 +123,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. @@ -127,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 { @@ -135,6 +140,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, @@ -470,7 +476,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) } @@ -488,15 +494,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. @@ -551,7 +551,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:&MainLine) -> String { name_for_ast(node.expression()) } @@ -561,12 +561,12 @@ 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) - } else if let Some(node) = NodeInfo::from_line_ast(&body) { - alias_analysis::analyze_node(&node) + } 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 // body, then there is nothing that could use any symbols, so nothing is used. @@ -639,19 +639,28 @@ 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 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 = 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 { 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 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| { def.set_block_lines(lines)?; Ok(def) @@ -725,7 +734,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 { @@ -733,15 +742,28 @@ 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 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 = MainLine::from_ast(&expression_ast).ok_or(FailedToCreateNode)?; + 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 { 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 +771,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) })?; @@ -1285,6 +1306,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"); @@ -1296,6 +1318,7 @@ main = 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, @@ -1436,7 +1459,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); }) } @@ -1480,7 +1503,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); }) } @@ -1518,7 +1541,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); }) } @@ -1555,7 +1578,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); }) } @@ -1578,8 +1601,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_ast(*code).unwrap(); + let node = MainLine::from_ast(&ast).unwrap(); let name = Handle::variable_name_base_for(&node); assert_eq!(&name,expected_name); } @@ -1604,7 +1627,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 10a88a0216..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 => { @@ -760,10 +760,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_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); - 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()}; @@ -774,9 +775,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) { @@ -1661,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); } @@ -1860,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/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/controller/upload.rs b/src/rust/ide/src/controller/upload.rs index 47072353bf..2d93b95ff6 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 012751a981..a2fcff1d84 100644 --- a/src/rust/ide/src/double_representation.rs +++ b/src/rust/ide/src/double_representation.rs @@ -1,6 +1,19 @@ //! A module with all functions used to synchronize different representations of our language //! module. +use crate::prelude::*; + +use crate::double_representation::definition::DefinitionName; +use crate::double_representation::definition::ScopeKind; + +use ast::Ast; +use ast::crumbs::InfixCrumb; +use ast::crumbs::Located; +use ast::macros::DocumentationCommentAst; +use ast::known; +use ast::opr; +use ast::prefix; + pub mod alias_analysis; pub mod connection; pub mod definition; @@ -29,3 +42,190 @@ 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 { + /// Definition is a binding, which defines a new entity with arguments. + Definition { + /// 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 { + /// 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. + documentation : DocumentationCommentAst + } +} + +impl LineKind { + /// 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 ast = match opr::to_assignment(ast) { + Some(infix) => + infix, + None => + return if let Some(documentation) = DocumentationCommentAst::new(ast) { + // e.g. `## My comment.` + DocumentationComment {documentation} + } 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(&ast.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} + }; + + 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} + } + }; + + Definition {ast,name,args} + } +} + +// 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). + +#[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/alias_analysis.rs b/src/rust/ide/src/double_representation/alias_analysis.rs index ef42990d09..e33953eb89 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); } @@ -374,12 +374,11 @@ 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 node = NodeInfo::from_line_ast(&ast).unwrap(); - let result = analyze_node(&node); + let ast = parser.parse_line_ast(&case.code).unwrap(); + 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..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; @@ -114,24 +113,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 +146,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/connection.rs b/src/rust/ide/src/double_representation/connection.rs index 2257d21944..c5bd62a8d9 100644 --- a/src/rust/ide/src/double_representation/connection.rs +++ b/src/rust/ide/src/double_representation/connection.rs @@ -7,7 +7,7 @@ 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::NodeInfo; +use crate::double_representation::node::MainLine; use ast::crumbs::Crumb; use ast::crumbs::Crumbs; @@ -42,8 +42,8 @@ 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(); - Some(Endpoint { node, crumbs }) + 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 dc52e388b7..fcb9729505 100644 --- a/src/rust/ide/src/double_representation/definition.rs +++ b/src/rust/ide/src/double_representation/definition.rs @@ -2,16 +2,18 @@ use crate::prelude::*; +use crate::double_representation::LineKind; + use ast::crumbs::ChildAst; use ast::crumbs::Crumbable; use ast::crumbs::InfixCrumb; use ast::crumbs::Located; use ast::known; -use ast::prefix; use ast::opr; use parser::Parser; + // ===================== // === Definition Id === // ===================== @@ -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. @@ -241,13 +243,13 @@ impl DefinitionInfo { /// 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.iter_all_lines().map(|line| line.map_opt(CloneRef::clone_ref)).collect() } else { let elem = Some((*self.body()).clone()); let off = 0; - Ok(vec![ast::BlockLine{elem,off}]) + vec![ast::BlockLine{elem,off}] } } @@ -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,50 +313,14 @@ impl DefinitionInfo { /// 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 LineKind::Definition{ast,args,name} = LineKind::discern(ast,kind) { + Some(DefinitionInfo {ast,name,args,context_indent}) } else { - Some(ret) + None } } } -// 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; @@ -614,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"); @@ -629,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"); @@ -646,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 e89c201c1a..86502ec971 100644 --- a/src/rust/ide/src/double_representation/graph.rs +++ b/src/rust/ide/src/double_representation/graph.rs @@ -2,9 +2,10 @@ use crate::prelude::*; -use crate::double_representation::definition; 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; use ast::Ast; @@ -14,7 +15,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; @@ -44,28 +44,24 @@ 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 { + /// 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) + } + /// 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(); - if let Ok(body_block) = known::Block::try_new(body.clone()) { - block_nodes(&body_block) - } else { - expression_node(body) - } - } - /// Gets the AST of this graph definition. pub fn ast(&self) -> Ast { self.source.ast.clone().into() @@ -74,7 +70,22 @@ impl GraphInfo { /// 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()) + let ast = &self.source.ast; + let body = &ast.rarg; + 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 {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 + // 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. + vec![] + } } /// Gets the list of connections between the nodes in this graph. @@ -84,18 +95,23 @@ impl GraphInfo { /// Adds a new node to this graph. pub fn add_node - (&mut self, line_ast:Ast, location_hint:LocationHint) -> FallibleResult { - let mut lines = self.source.block_lines()?; + (&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)? + 1, - LocationHint::Before(id) => node::index_in_lines(&lines, id)? + 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 elem = Some(node.ast().clone_ref()); let off = 0; lines.insert(index,BlockLine{elem,off}); + if let Some(documentation) = &node.documentation { + let elem = Some(documentation.ast().into()); + let line = BlockLine {elem,off}; + lines.insert(index,line); + } self.source.set_block_lines(lines) } @@ -124,27 +140,35 @@ 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 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) { + (Some(old_comment_index),None) => { + lines.remove(old_comment_index); + } + (Some(old_comment_index),Some(new_comment)) => + lines[old_comment_index] = new_comment.block_line(), + (None,Some(new_comment)) => + lines.insert(index.main_line, new_comment.block_line()), + (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) } + + // TODO tests for cases with comments involved } /// Sets expression of the given node. @@ -164,30 +188,6 @@ impl GraphInfo { -// ===================== -// === Listing nodes === -// ===================== - -/// 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() -} - - - // ============= // === Tests === // ============= @@ -201,6 +201,7 @@ 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; @@ -245,11 +246,24 @@ 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_main_line_ast(&line_ast).unwrap() + } + + 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) + } + } + + fn assert_same(left:&NodeInfo, right:&NodeInfo) { + assert_eq!(left.id(), right.id()); + 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()); } #[wasm_bindgen_test] @@ -259,24 +273,20 @@ 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"; - 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[1].expression().repr(), expr0); - assert_eq!(nodes[1].id(), id0); - assert_eq!(nodes[2].expression().repr(), "print \"hello\""); + assert_all(nodes.as_slice(), &[node_to_add1, node_to_add0, initial_node]); } #[wasm_bindgen_test] @@ -289,29 +299,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 +337,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 +359,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 @@ -374,19 +386,30 @@ foo = 5"; 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); - for node in nodes.iter() { - assert_eq!(node.expression().repr(), "node"); - } + 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"); + assert_eq!(nodes.len(), 4); } #[wasm_bindgen_test] 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 1c562e7a6f..f440a20c6d 100644 --- a/src/rust/ide/src/double_representation/node.rs +++ b/src/rust/ide/src/double_representation/node.rs @@ -2,9 +2,18 @@ use crate::prelude::*; +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; + /// Node Id is the Ast Id attached to the node's expression. pub type Id = ast::Id; @@ -19,23 +28,111 @@ 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)] +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 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 NodeLocation { + /// 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: NodeLocation, last: NodeLocation) -> RangeInclusive { + start.first() ..= last.last() + } +} + // =============== // === 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 : NodeLocation, + #[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 is_node_by_id(line:&ast::BlockLine>, id:ast::Id) -> bool { - let node_info = NodeInfo::from_block_line(line); +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) } -/// 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()) +/// 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 +, context_indent : usize +, id : Id +) -> FallibleResult { + Ok(locate_many(lines, context_indent, [id])?.remove(&id).unwrap()) +} + +/// 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 +, context_indent : usize +, looked_for : impl IntoIterator +) -> FallibleResult> { + let mut looked_for = looked_for.into_iter().collect::>(); + + let mut ret = HashMap::new(); + // 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); + } + + if looked_for.is_empty() { + break + } + }; + + if let Some(id) = looked_for.into_iter().next() { + Err(IdNotFound {id}.into()) + } else { + Ok(ret) + } } @@ -44,43 +141,151 @@ pub fn index_in_lines(lines:&[ast::BlockLine>], id:ast::Id) -> Falli // === 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. +/// 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, + /// Absolute indent of lines in the block we are iterating over. + pub context_indent:usize, +} + +impl<'a, T:Iterator)> + 'a> Iterator for NodeIterator<'a, T> { + type Item = LocatedNode; + + fn next(&mut self) -> Option { + let mut indexed_documentation = None; + 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); + 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) = MainLine::from_discerned_line(line) { + let (documentation_line,documentation) = match indexed_documentation { + Some((index,documentation)) => (Some(index),Some(documentation)), + None => (None,None) + }; + + let node = NodeInfo {documentation,main_line}; + let index = NodeLocation { + main_line: index, + documentation_line, + }; + + return Some(LocatedNode {index,node}) + } + } + } + } + None + } +} + +/// 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 { + /// 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 : 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 = || MainLine::from_ast(line_ast) + .as_ref() + .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, + _ => false, + }; + expression_id_matches() || doc_comment_id_matches() + } + + /// 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()) + } + + /// 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 {documentation,main_line}) + } + + /// Obtain documentation text. + pub fn documentation_text(&self) -> Option { + self.documentation.as_ref().map(|doc| doc.pretty_text()) + } +} + +/// 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 NodeInfo { +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 NodeInfo { +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(NodeInfo::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?; - Some(NodeInfo::Expression {ast}) + // TODO what if we are given an assignment. + Some(MainLine::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()) + /// 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; + 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:&ast::BlockLine>) -> Option { - Self::from_line_ast(line.elem.as_ref()?) + pub fn from_block_line(line:&BlockLine>) -> Option { + Self::from_ast(line.elem.as_ref()?) } /// Node's unique ID. @@ -93,13 +298,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} => { + 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."); } - NodeInfo::Expression{ref mut ast} => { + MainLine::Expression{ref mut ast} => { *ast = ast.with_id(new_id); } }; @@ -108,16 +313,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, + 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 { - NodeInfo::Binding {infix} => Some(&infix.larg), - NodeInfo::Expression{..} => None, + MainLine::Binding {infix} => Some(&infix.larg), + MainLine::Expression{..} => None, } } @@ -125,9 +330,9 @@ impl NodeInfo { pub fn set_expression(&mut self, expression:Ast) { let id = self.id(); match self { - NodeInfo::Binding{ref mut infix} => + MainLine::Binding{ref mut infix} => infix.update_shape(|infix| infix.rarg = expression), - NodeInfo::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); @@ -136,8 +341,8 @@ impl NodeInfo { /// The whole AST of node. pub fn ast(&self) -> &Ast { match self { - NodeInfo::Binding {infix} => infix.into(), - NodeInfo::Expression{ast} => ast, + MainLine::Binding {infix} => infix.into(), + MainLine::Expression{ast} => ast, } } @@ -145,11 +350,11 @@ impl NodeInfo { /// assignment infix will be introduced. pub fn set_pattern(&mut self, pattern:Ast) { match self { - NodeInfo::Binding {infix} => { + MainLine::Binding {infix} => { // Setting infix operand never fails. infix.update_shape(|infix| infix.larg = pattern) } - NodeInfo::Expression {ast} => { + MainLine::Expression {ast} => { let infix = ast::Infix { larg : pattern, loff : 1, @@ -158,7 +363,7 @@ impl NodeInfo { rarg : ast.clone(), }; let infix = known::Infix::new(infix, None); - *self = NodeInfo::Binding {infix}; + *self = MainLine::Binding {infix}; } } @@ -169,16 +374,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()} + MainLine::Binding {infix} => { + *self = MainLine::Expression {ast:infix.rarg.clone_ref()} } - NodeInfo::Expression {..} => {} + MainLine::Expression {..} => {} } } } -impl ast::HasTokens for NodeInfo { +impl ast::HasTokens for MainLine { fn feed_to(&self, consumer:&mut impl ast::TokenConsumer) { self.ast().feed_to(consumer) } @@ -197,7 +402,7 @@ mod tests { 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"); + 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); } @@ -210,12 +415,23 @@ mod tests { 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_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"); @@ -228,7 +444,7 @@ mod tests { 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 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"); @@ -236,17 +452,6 @@ mod tests { 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` @@ -256,7 +461,7 @@ mod tests { let rarg = Ast::new(number, Some(id)); let ast = Ast::infix(larg,ASSIGNMENT,rarg); - let mut node = NodeInfo::from_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(); @@ -271,7 +476,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_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); @@ -287,7 +492,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_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 47494a7e7f..2f9fefa8df 100644 --- a/src/rust/ide/src/double_representation/refactorings/collapse.rs +++ b/src/rust/ide/src/double_representation/refactorings/collapse.rs @@ -12,6 +12,7 @@ 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::MainLine; use crate::double_representation::graph::GraphInfo; use ast::crumbs::Located; @@ -137,7 +138,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 +236,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 +267,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 +335,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 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 = MainLine::from_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()) @@ -379,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; @@ -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/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 +} diff --git a/src/rust/ide/src/ide/integration/project.rs b/src/rust/ide/src/ide/integration/project.rs index c6fce38b84..6c1e0bc17c 100644 --- a/src/rust/ide/src/ide/integration/project.rs +++ b/src/rust/ide/src/ide/integration/project.rs @@ -739,6 +739,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), @@ -796,6 +797,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); } @@ -850,6 +852,17 @@ 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(); + if comment_as_per_controller != comment_as_per_view { + node_view.set_comment(comment_as_per_controller); + } + } + } /// Update the expression of the node and all related properties e.g., types, ports). fn refresh_node_expression 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 80980f8299..1489ebcad7 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; @@ -69,6 +72,18 @@ 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; + + + // ============= // === Shape === // ============= @@ -258,6 +273,7 @@ ensogl::define_endpoints! { set_disabled (bool), set_input_connected (span_tree::Crumbs,Option,bool), 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,6 +295,7 @@ ensogl::define_endpoints! { /// background. In edit mode, the whole node area is considered non-active. background_press (), expression (Text), + comment (Comment), skip (bool), freeze (bool), hover (bool), @@ -384,6 +401,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 +475,14 @@ impl NodeModel { let style = StyleWatchFrp::new(&app.display.scene().style_sheet); + // TODO: Style the documentation comment properly. + let comment = ensogl_text::Area::new(app); + display_object.add_child(&comment); + 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 { @@ -609,6 +631,16 @@ impl Node { model.output.set_expression_visibility <+ frp.set_output_expression_visibility; + // === 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; + + // === 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 7df5bb8a64..091223d787 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 7253244844..e80382aaef 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())); @@ -278,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(); @@ -296,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(); @@ -370,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();