From 666a7430da820ae71c165283a8c432a917c53187 Mon Sep 17 00:00:00 2001 From: Michael Mauderer Date: Thu, 5 Aug 2021 09:33:06 +0100 Subject: [PATCH] Revert "Add support for comments on nodes (https://github.com/enso-org/ide/pull/1744)" This reverts commit f30fb17afe267205c49ee66bf362d04325be3b81. Original commit: https://github.com/enso-org/ide/commit/36a2242f15b43e2741a9958783728f82d0951d9e --- ide/CHANGELOG.md | 2 - .../ensogl/lib/text/src/component/area.rs | 23 +- ide/src/rust/ide/lib/ast/impl/src/crumbs.rs | 7 +- ide/src/rust/ide/lib/ast/impl/src/lib.rs | 82 +---- ide/src/rust/ide/lib/ast/impl/src/macros.rs | 190 +---------- ide/src/rust/ide/lib/parser/src/lib.rs | 26 +- ide/src/rust/ide/lib/parser/tests/bugs.rs | 2 +- ide/src/rust/ide/lib/parser/tests/crumbs.rs | 4 +- ide/src/rust/ide/lib/parser/tests/macros.rs | 8 +- ide/src/rust/ide/lib/parser/tests/parsing.rs | 6 +- ide/src/rust/ide/lib/span-tree/src/action.rs | 4 +- .../rust/ide/lib/span-tree/src/generate.rs | 26 +- ide/src/rust/ide/src/controller/graph.rs | 93 ++--- ide/src/rust/ide/src/controller/project.rs | 2 +- ide/src/rust/ide/src/controller/searcher.rs | 29 +- .../ide/src/controller/searcher/action.rs | 2 +- ide/src/rust/ide/src/controller/upload.rs | 1 - ide/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 +- .../rust/ide/src/ide/integration/project.rs | 13 - .../view/graph-editor/src/component/node.rs | 34 +- ide/src/rust/ide/view/graph-editor/src/lib.rs | 21 +- .../ide/view/src/debug_scenes/interface.rs | 9 +- 31 files changed, 370 insertions(+), 1113 deletions(-) diff --git a/ide/CHANGELOG.md b/ide/CHANGELOG.md index bf6b6505c284..54d9e14a239d 100644 --- a/ide/CHANGELOG.md +++ b/ide/CHANGELOG.md @@ -10,7 +10,6 @@ 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 @@ -32,7 +31,6 @@ 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/ide/src/rust/ensogl/lib/text/src/component/area.rs b/ide/src/rust/ensogl/lib/text/src/component/area.rs index 5c84751cddef..e92ef5587a12 100644 --- a/ide/src/rust/ensogl/lib/text/src/component/area.rs +++ b/ide/src/rust/ensogl/lib/text/src/component/area.rs @@ -128,7 +128,7 @@ struct Lines { impl Lines { /// The number of visible lines. - pub fn len(&self) -> usize { + pub fn count(&self) -> usize { self.rc.borrow().len() } @@ -260,7 +260,6 @@ ensogl_core::define_endpoints! { Output { pointer_style (cursor::Style), width (f32), - height (f32), changed (Vec), content (Text), hovered (bool), @@ -601,7 +600,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.len() { + let pos_x = |line:usize, column:Column| if line >= self.lines.count() { 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) @@ -678,7 +677,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.len() - 1); + let line_index = std::cmp::min(line_index,self.lines.count() - 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(); @@ -691,25 +690,19 @@ impl AreaModel { } /// Redraw the text. - fn redraw(&self, size_may_change:bool) { + fn redraw(&self, width_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 widths = lines.into_iter().enumerate().map(|(view_line_index,content)|{ + let lengths = lines.into_iter().enumerate().map(|(view_line_index,content)|{ self.redraw_line(view_line_index,content) }).collect_vec(); - 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); + 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); } } - 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/ide/src/rust/ide/lib/ast/impl/src/crumbs.rs b/ide/src/rust/ide/lib/ast/impl/src/crumbs.rs index 203edcfd1fa2..6ce7d9cb9096 100644 --- a/ide/src/rust/ide/lib/ast/impl/src/crumbs.rs +++ b/ide/src/rust/ide/lib/ast/impl/src/crumbs.rs @@ -3,13 +3,12 @@ use crate::prelude::*; -use crate::enumerate_non_empty_lines; +use crate::ShiftedVec1; 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; @@ -1415,7 +1414,9 @@ 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 { - enumerate_non_empty_lines(iter).map(|(index,_ast)| index) + iter.enumerate().filter_map(|(line_index,line)| { + line.elem.as_ref().map(|_| line_index) + }) } diff --git a/ide/src/rust/ide/lib/ast/impl/src/lib.rs b/ide/src/rust/ide/lib/ast/impl/src/lib.rs index bb5728603686..9f23466dd1ad 100644 --- a/ide/src/rust/ide/lib/ast/impl/src/lib.rs +++ b/ide/src/rust/ide/lib/ast/impl/src/lib.rs @@ -630,6 +630,7 @@ pub struct BlockLine { } + // ============= // === Macro === // ============= @@ -1200,82 +1201,29 @@ 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 { - /// 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| { + /// 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 { 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.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 off = off.checked_sub(self.indent).unwrap_or(0); + lines.push(BlockLine{elem,off}) + } - /// 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)) - }) + 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 } } @@ -1720,7 +1668,7 @@ mod tests { let expected_repr = "\n \n head \n tail0 \n \n tail2 "; assert_eq!(block.repr(), expected_repr); - let all_lines = block.iter_all_lines().collect_vec(); + let all_lines = block.all_lines(); 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/ide/src/rust/ide/lib/ast/impl/src/macros.rs b/ide/src/rust/ide/lib/ast/impl/src/macros.rs index 4d4734d28b22..7d80dc97a812 100644 --- a/ide/src/rust/ide/lib/ast/impl/src/macros.rs +++ b/ide/src/rust/ide/lib/ast/impl/src/macros.rs @@ -8,205 +8,21 @@ use crate::crumbs::AmbiguousCrumb; use crate::crumbs::Located; use crate::crumbs::MatchCrumb; use crate::known; -use crate::BlockLine; -use crate::Shifted; -// ================================== -// === Recognized Macros Keywords === -// ================================== - -/// The keyword introducing a disabled code line. -pub const DISABLING_COMMENT_INTRODUCER:&str = "#"; - -/// The keyword introducing a documentation block. -pub const DOCUMENTATION_COMMENT_INTRODUCER:&str = "##"; +// =============== +// === Imports === +// =============== /// 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/ide/src/rust/ide/lib/parser/src/lib.rs b/ide/src/rust/ide/lib/parser/src/lib.rs index 42d4ef328f75..5e7186a0ed29 100644 --- a/ide/src/rust/ide/lib/parser/src/lib.rs +++ b/ide/src/rust/ide/lib/parser/src/lib.rs @@ -22,7 +22,6 @@ mod wsclient; use crate::prelude::*; use ast::Ast; -use ast::BlockLine; use ast::IdMap; use std::panic; use utils::fail::FallibleResult; @@ -101,28 +100,17 @@ impl Parser { } /// 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_ast(&self, program:impl Str) -> FallibleResult { - self.parse_line_with_id_map(program, default()).map(|line| line.elem) + /// returned. Panics otherwise. The program is parsed with empty IdMap. + pub fn parse_line(&self, program:impl Str) -> FallibleResult { + self.parse_line_with_id_map(program,default()) } - - /// Program is expected to be single non-empty line module. The line's AST is - /// returned. 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> { + /// 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 { let module = self.parse_module(program,id_map)?; let mut lines = module.lines.clone().into_iter().filter_map(|line| { - line.map(|elem| elem).transpose() + line.elem }); if let Some(first_non_empty_line) = lines.next() { if lines.next().is_some() { diff --git a/ide/src/rust/ide/lib/parser/tests/bugs.rs b/ide/src/rust/ide/lib/parser/tests/bugs.rs index 2289014dc7ce..c8ca871ffda5 100644 --- a/ide/src/rust/ide/lib/parser/tests/bugs.rs +++ b/ide/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_ast("Int.+").unwrap(); + let ast = parser::Parser::new_or_panic().parse_line("Int.+").unwrap(); use ast::*; if let Shape::Infix(Infix {larg:_larg,loff:_loff,opr,roff:_roff,rarg}, ..) = ast.shape() { diff --git a/ide/src/rust/ide/lib/parser/tests/crumbs.rs b/ide/src/rust/ide/lib/parser/tests/crumbs.rs index 9db150e7d591..66a6d6916601 100644 --- a/ide/src/rust/ide/lib/parser/tests/crumbs.rs +++ b/ide/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_ast("foo -> bar").unwrap(); + let ast = Parser::new_or_panic().parse_line("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_ast("( foo bar )").unwrap(); + let ast = Parser::new_or_panic().parse_line("( foo bar )").unwrap(); let crumbs = ast.iter_subcrumbs().collect_vec(); assert_eq!(ast.get(&crumbs[0]).unwrap().repr(), "("); diff --git a/ide/src/rust/ide/lib/parser/tests/macros.rs b/ide/src/rust/ide/lib/parser/tests/macros.rs index aa2caf87f2b3..f164843017e9 100644 --- a/ide/src/rust/ide/lib/parser/tests/macros.rs +++ b/ide/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_ast(code).unwrap(); + let ast = parser.parse_line(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_ast(code).unwrap(); + let ast = parser.parse_line(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_ast(code).unwrap(); + let ast = parser.parse_line(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_ast(code).unwrap(); + let ast = parser.parse_line(code).unwrap(); assert!(ast::macros::as_lambda_match(&ast).is_none(), "wrongly recognized a lambda"); }; diff --git a/ide/src/rust/ide/lib/parser/tests/parsing.rs b/ide/src/rust/ide/lib/parser/tests/parsing.rs index a010fdc86cad..94aa77ca093d 100644 --- a/ide/src/rust/ide/lib/parser/tests/parsing.rs +++ b/ide/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_ast(program).unwrap(); + let ast = self.parser.parse_line(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_ast(unexpected).unwrap(); + let ast = self.parser.parse_line(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_ast(*macro_usage).unwrap(); + let ast = self.parser.parse_line(*macro_usage).unwrap(); expect_shape::>(&ast); }; } diff --git a/ide/src/rust/ide/lib/span-tree/src/action.rs b/ide/src/rust/ide/lib/span-tree/src/action.rs index 21aaaaa0a1eb..73da9c6dd6d2 100644 --- a/ide/src/rust/ide/lib/span-tree/src/action.rs +++ b/ide/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_ast(self.expr).unwrap(); + let ast = parser.parse_line(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_ast(self.expr).unwrap(); + let ast = parser.parse_line(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/ide/src/rust/ide/lib/span-tree/src/generate.rs b/ide/src/rust/ide/lib/span-tree/src/generate.rs index 31672042ee24..c1e633dde620 100644 --- a/ide/src/rust/ide/lib/span-tree/src/generate.rs +++ b/ide/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_ast_with_id_map("2 + foo bar - 3", id_map.clone()).unwrap(); + let ast = parser.parse_line_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_ast("2 + 3 + foo bar baz 13 + 5").unwrap(); + let ast = parser.parse_line("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_ast("1,2,3").unwrap(); + let ast = parser.parse_line("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_ast("+ * + + 2 +").unwrap(); + let ast = parser.parse_line("+ * + + 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_ast(",2,").unwrap(); + let ast = parser.parse_line(",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_ast_with_id_map(expression, id_map.clone()).unwrap(); + let ast = parser.parse_line_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_ast(expression).unwrap(); + let ast = parser.parse_line(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_ast_with_id_map("(4", id_map.clone()).unwrap(); + let ast = parser.parse_line_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_ast("foo a-> b + c").unwrap(); + let ast = parser.parse_line("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_ast("foo").unwrap(); + let ast = parser.parse_line("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_ast("foo here").unwrap(); + let ast = parser.parse_line("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_ast("foo here").unwrap(); + let ast = parser.parse_line("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_ast("here.foo").unwrap(); + let ast = parser.parse_line("here.foo").unwrap(); let invocation_info = CalledMethodInfo { parameters : vec![this_param.clone(), param1.clone(), param2.clone()] }; diff --git a/ide/src/rust/ide/src/controller/graph.rs b/ide/src/rust/ide/src/controller/graph.rs index f40038da3814..cdbb44491b53 100644 --- a/ide/src/rust/ide/src/controller/graph.rs +++ b/ide/src/rust/ide/src/controller/graph.rs @@ -8,21 +8,17 @@ 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; @@ -123,8 +119,6 @@ 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. @@ -133,6 +127,7 @@ 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 { @@ -140,7 +135,6 @@ 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, @@ -476,7 +470,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.definition()?; + let _ = ret.graph_definition_info()?; Ok(ret) } @@ -494,9 +488,15 @@ 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.definition().map(|definition| GraphInfo::from_definition(definition.item)) + self.graph_definition_info().map(GraphInfo::from_definition) } /// 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:&MainLine) -> String { + pub fn variable_name_base_for(node:&NodeInfo) -> 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.definition()?; + let def = self.graph_definition_info()?; let body = def.body(); let usage = if matches!(body.shape(),ast::Shape::Block(_)) { alias_analysis::analyze_crumbable(body.item) - } else if let Some(node) = MainLine::from_ast(&body) { - alias_analysis::analyze_ast(node.ast()) + } else if let Some(node) = NodeInfo::from_line_ast(&body) { + alias_analysis::analyze_node(&node) } 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,28 +639,19 @@ 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 graph = self.graph_info()?; - let definition_ast = &graph.body().item; + let definition = self.graph_definition_info()?; + let definition_ast = &definition.body().item; let dependent_nodes = connection::dependent_nodes_in_def(definition_ast,node_to_be_after); + let mut lines = definition.block_lines()?; - let 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 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 should_be_at_end = |line:&ast::BlockLine>| { - 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 - } + 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 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); + lines[after_node_position..=before_node_position].sort_by_key(should_be_at_end); self.update_definition_ast(|mut def| { def.set_block_lines(lines)?; Ok(def) @@ -734,7 +725,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_ast(expression_text.as_ref())?; + let node_ast = self.parser.parse_line(expression_text.as_ref())?; if ast::opr::is_assignment(&node_ast) { Err(BindingExpressionNotAllowed(expression_text.into()).into()) } else { @@ -742,28 +733,15 @@ impl Handle { } } - /// Creates a proper description of a documentation comment in the context of this graph. - pub fn documentation_comment_from_pretty_text - (&self, pretty_text:&str) -> Option { - let indent = self.definition().ok()?.indent(); - let doc_repr = DocumentationCommentInfo::text_to_repr(indent,pretty_text); - let doc_line = self.parser.parse_line(doc_repr).ok()?; - DocumentationCommentInfo::new(&doc_line.as_ref(),indent) - } - /// Adds a new node to the graph and returns information about created node. pub fn add_node(&self, node:NewNodeInfo) -> FallibleResult { info!(self.logger, "Adding node with expression `{node.expression}`"); - let expression_ast = self.parse_node_expression(&node.expression)?; - let main_line = MainLine::from_ast(&expression_ast).ok_or(FailedToCreateNode)?; - let 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}; + let ast = self.parse_node_expression(&node.expression)?; + let mut node_info = node::NodeInfo::from_line_ast(&ast).ok_or(FailedToCreateNode)?; 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()); @@ -771,7 +749,8 @@ impl Handle { self.update_definition_ast(|definition| { let mut graph = GraphInfo::from_definition(definition); - graph.add_node(&node_info,node.location_hint)?; + let node_ast = node_info.ast().clone(); + graph.add_node(node_ast,node.location_hint)?; Ok(graph.source) })?; @@ -1306,7 +1285,6 @@ 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"); @@ -1318,7 +1296,6 @@ 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, @@ -1459,7 +1436,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.definition().unwrap().ast.repr(); + let new_main = graph.graph_definition_info().unwrap().ast.repr(); assert_eq!(new_main,expected,"Case {:?}",this); }) } @@ -1503,7 +1480,7 @@ main = } }; graph.connect(&connection_to_add,&span_tree::generate::context::Empty).unwrap(); - let new_main = graph.definition().unwrap().ast.repr(); + let new_main = graph.graph_definition_info().unwrap().ast.repr(); assert_eq!(new_main,EXPECTED); }) } @@ -1541,7 +1518,7 @@ main = } }; graph.connect(&connection_to_add,&span_tree::generate::context::Empty).unwrap(); - let new_main = graph.definition().unwrap().ast.repr(); + let new_main = graph.graph_definition_info().unwrap().ast.repr(); assert_eq!(new_main,EXPECTED); }) } @@ -1578,7 +1555,7 @@ main = } }; graph.connect(&connection_to_add,&span_tree::generate::context::Empty).unwrap(); - let new_main = graph.definition().unwrap().ast.repr(); + let new_main = graph.graph_definition_info().unwrap().ast.repr(); assert_eq!(new_main,EXPECTED); }) } @@ -1601,8 +1578,8 @@ main = ]; for (code,expected_name) in &cases { - let ast = parser.parse_line_ast(*code).unwrap(); - let node = MainLine::from_ast(&ast).unwrap(); + let ast = parser.parse_line(*code).unwrap(); + let node = NodeInfo::from_line_ast(&ast).unwrap(); let name = Handle::variable_name_base_for(&node); assert_eq!(&name,expected_name); } @@ -1627,7 +1604,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.definition().unwrap().ast.repr(); + let new_main = graph.graph_definition_info().unwrap().ast.repr(); assert_eq!(new_main,expected,"Case {:?}",this); }) } diff --git a/ide/src/rust/ide/src/controller/project.rs b/ide/src/rust/ide/src/controller/project.rs index 8b39445336cb..857072403558 100644 --- a/ide/src/rust/ide/src/controller/project.rs +++ b/ide/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_ast(main_code)?; + let main_ast = parser.parse_line(main_code)?; info.add_ast(main_ast,double_representation::module::Placement::End)?; module.update_ast(info.ast)?; } diff --git a/ide/src/rust/ide/src/controller/searcher.rs b/ide/src/rust/ide/src/controller/searcher.rs index 58182d40c2f7..10a88a021669 100644 --- a/ide/src/rust/ide/src/controller/searcher.rs +++ b/ide/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_ast(input.trim_start())?; + let ast = parser.parse_line(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_ast(&self.pattern).ok(); + let parsed_pattern = parser.parse_line(&self.pattern).ok(); let pattern_sast = parsed_pattern.map(|p| ast::Shifted::new(self.pattern_offset,p)); // If there is an expression part of input, we add current pattern as the last argument. if let Some(chain) = &self.expression { @@ -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_ast(&code_to_insert)?; + let added_ast = self.ide.parser().parse_line(&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,11 +760,10 @@ impl Searcher { let args = std::iter::empty(); let node_expression = ast::prefix::Chain::new_with_this(new_definition_name,here,args); let node_expression = node_expression.into_ast(); - let node = NodeInfo::from_main_line_ast(&node_expression).ok_or(FailedToCreateNode)?; - let added_node_id = node.id(); + let node = NodeInfo::new_expression(node_expression).ok_or(FailedToCreateNode)?; 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,LocationHint::End)?; + graph_info.add_node(node.ast().clone_ref(), LocationHint::End)?; module.ast = module.ast.set_traversing(&graph_definition.crumbs, graph_info.ast())?; let metadata = NodeMetadata {position,..default()}; @@ -775,9 +774,9 @@ impl Searcher { module.add_module_import(&here,self.ide.parser(),&import); } graph.module.update_ast(module.ast)?; - graph.module.set_node_metadata(added_node_id,metadata)?; + graph.module.set_node_metadata(node.id(),metadata)?; - Ok(added_node_id) + Ok(node.id()) } fn invalidate_fragments_added_by_picking(&self) { @@ -1662,7 +1661,7 @@ pub mod test { fn run(&self) { let parser = Parser::new_or_panic(); - let ast = parser.parse_line_ast(self.before).unwrap(); + let ast = parser.parse_line(self.before).unwrap(); let new_ast = apply_this_argument("foo",&ast); assert_eq!(new_ast.repr(),self.after,"Case {:?} failed: {:?}",self,ast); } @@ -1861,28 +1860,28 @@ pub mod test { fn simple_function_call_parsing() { let parser = Parser::new_or_panic(); - let ast = parser.parse_line_ast("foo").unwrap(); + let ast = parser.parse_line("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_ast("Main.foo").unwrap(); + let ast = parser.parse_line("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_ast("(2 + 3).foo").unwrap(); + let ast = parser.parse_line("(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_ast("foo + 3").unwrap(); + let ast = parser.parse_line("foo + 3").unwrap(); assert!(SimpleFunctionCall::try_new(&ast).is_none()); - let ast = parser.parse_line_ast("foo bar baz").unwrap(); + let ast = parser.parse_line("foo bar baz").unwrap(); assert!(SimpleFunctionCall::try_new(&ast).is_none()); - let ast = parser.parse_line_ast("Main . (foo bar)").unwrap(); + let ast = parser.parse_line("Main . (foo bar)").unwrap(); assert!(SimpleFunctionCall::try_new(&ast).is_none()); } diff --git a/ide/src/rust/ide/src/controller/searcher/action.rs b/ide/src/rust/ide/src/controller/searcher/action.rs index 96627d5cd844..e4cef8275aa1 100644 --- a/ide/src/rust/ide/src/controller/searcher/action.rs +++ b/ide/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{category,match_info,action} + ListEntry{action,category,match_info} })); } } diff --git a/ide/src/rust/ide/src/controller/upload.rs b/ide/src/rust/ide/src/controller/upload.rs index 2d93b95ff6de..47072353bf0a 100644 --- a/ide/src/rust/ide/src/controller/upload.rs +++ b/ide/src/rust/ide/src/controller/upload.rs @@ -197,7 +197,6 @@ 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/ide/src/rust/ide/src/double_representation.rs b/ide/src/rust/ide/src/double_representation.rs index a2fcff1d8421..012751a98155 100644 --- a/ide/src/rust/ide/src/double_representation.rs +++ b/ide/src/rust/ide/src/double_representation.rs @@ -1,19 +1,6 @@ //! 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; @@ -42,190 +29,3 @@ 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/ide/src/rust/ide/src/double_representation/alias_analysis.rs b/ide/src/rust/ide/src/double_representation/alias_analysis.rs index e33953eb897b..ef42990d0995 100644 --- a/ide/src/rust/ide/src/double_representation/alias_analysis.rs +++ b/ide/src/rust/ide/src/double_representation/alias_analysis.rs @@ -7,6 +7,7 @@ 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; @@ -333,12 +334,11 @@ impl AliasAnalyzer { } } -/// 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 { +/// 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 { let mut analyzer = AliasAnalyzer::new(); - analyzer.process_ast(ast); + analyzer.process_ast(node.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, ast:&Ast, expected:Vec>, actual:&Vec) { - let mut checker = IdentifierValidator::new(name,ast,expected); + (name:impl Str, node:&NodeInfo, expected:Vec>, actual:&Vec) { + let mut checker = IdentifierValidator::new(name,node,expected); checker.validate_identifiers(actual); } @@ -374,11 +374,12 @@ mod tests { fn run_case(parser:&parser::Parser, case:Case) { DEBUG!("\n===========================================================================\n"); DEBUG!("Case: " case.code); - let ast = parser.parse_line_ast(&case.code).unwrap(); - let result = analyze_ast(&ast); + let ast = parser.parse_line(&case.code).unwrap(); + let node = NodeInfo::from_line_ast(&ast).unwrap(); + let result = analyze_node(&node); DEBUG!("Analysis results: {result:?}"); - validate_identifiers("introduced",&ast, case.expected_introduced, &result.introduced); - validate_identifiers("used", &ast, case.expected_used, &result.used); + validate_identifiers("introduced",&node, case.expected_introduced, &result.introduced); + validate_identifiers("used", &node, case.expected_used, &result.used); } /// Runs the test for the test case expressed using markdown notation. See `Case` for details. diff --git a/ide/src/rust/ide/src/double_representation/alias_analysis/test_utils.rs b/ide/src/rust/ide/src/double_representation/alias_analysis/test_utils.rs index e886bed445d5..cd1268f1f453 100644 --- a/ide/src/rust/ide/src/double_representation/alias_analysis/test_utils.rs +++ b/ide/src/rust/ide/src/double_representation/alias_analysis/test_utils.rs @@ -4,6 +4,7 @@ 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; @@ -113,23 +114,24 @@ 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, ast:&Ast,spans:Vec>) -> IdentifierValidator { + pub fn new(name:impl Str, node:&NodeInfo,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 {ast,name,validations} + IdentifierValidator {name,node,validations} } /// Marks given identifier as checked. @@ -146,7 +148,7 @@ impl<'a> IdentifierValidator<'a> { self.validate_identifier(&identifier.item); let crumbs = &identifier.crumbs; - let ast_result = self.ast.get_traversing(crumbs); + let ast_result = self.node.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/ide/src/rust/ide/src/double_representation/connection.rs b/ide/src/rust/ide/src/double_representation/connection.rs index c5bd62a8d907..2257d2194452 100644 --- a/ide/src/rust/ide/src/double_representation/connection.rs +++ b/ide/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::MainLine; +use crate::double_representation::node::NodeInfo; 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(|| MainLine::from_ast(line_ast))?.id(); - Some(Endpoint {node,crumbs}) + let node = is_non_def.and_option_from(|| NodeInfo::from_line_ast(line_ast))?.id(); + Some(Endpoint { node, crumbs }) } } diff --git a/ide/src/rust/ide/src/double_representation/definition.rs b/ide/src/rust/ide/src/double_representation/definition.rs index fcb9729505fd..dc52e388b762 100644 --- a/ide/src/rust/ide/src/double_representation/definition.rs +++ b/ide/src/rust/ide/src/double_representation/definition.rs @@ -2,18 +2,16 @@ 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 === // ===================== @@ -180,7 +178,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_ast(self.to_string()) + parser.parse_line(self.to_string()) } /// Checks if the given definition name is a method defined on given expected atom name. @@ -243,13 +241,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) -> Vec>> { + pub fn block_lines(&self) -> FallibleResult>>> { if let Ok(block) = known::Block::try_from(*self.body()) { - block.iter_all_lines().map(|line| line.map_opt(CloneRef::clone_ref)).collect() + Ok(block.all_lines()) } else { let elem = Some((*self.body()).clone()); let off = 0; - vec![ast::BlockLine{elem,off}] + Ok(vec![ast::BlockLine{elem,off}]) } } @@ -264,7 +262,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.indent(); + let indent = self.context_indent + double_representation::INDENT; let mut empty_lines = Vec::new(); let mut line = lines.pop_front().ok_or(MissingLineWithAst)?; while line.elem.is_none() { @@ -313,14 +311,50 @@ impl DefinitionInfo { /// some binding or other kind of subtree). pub fn from_line_ast (ast:&Ast, kind:ScopeKind, context_indent:usize) -> Option { - if let LineKind::Definition{ast,args,name} = LineKind::discern(ast,kind) { - Some(DefinitionInfo {ast,name,args,context_indent}) + 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) + } } else { - None + Some(ret) } } } +// 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; @@ -580,7 +614,7 @@ mod tests { #[wasm_bindgen_test] fn definition_name_tests() { let parser = parser::Parser::new_or_panic(); - let ast = parser.parse_line_ast("Foo.Bar.baz").unwrap(); + let ast = parser.parse_line("Foo.Bar.baz").unwrap(); let name = DefinitionName::from_ast(&ast).unwrap(); assert_eq!(*name.name, "baz"); @@ -595,14 +629,14 @@ mod tests { #[wasm_bindgen_test] fn definition_name_rejecting_incomplete_names() { let parser = parser::Parser::new_or_panic(); - let ast = parser.parse_line_ast("Foo. .baz").unwrap(); + let ast = parser.parse_line("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_ast("Foo.bar a b c = baz").unwrap(); + let ast = parser.parse_line("Foo.bar a b c = baz").unwrap(); let definition = DefinitionInfo::from_root_line_ast(&ast).unwrap(); assert_eq!(definition.name.to_string(), "Foo.bar"); @@ -612,7 +646,7 @@ mod tests { #[wasm_bindgen_test] fn located_definition_args() { let parser = parser::Parser::new_or_panic(); - let ast = parser.parse_line_ast("foo bar baz = a + b + c").unwrap(); + let ast = parser.parse_line("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/ide/src/rust/ide/src/double_representation/graph.rs b/ide/src/rust/ide/src/double_representation/graph.rs index 86502ec97133..e89c201c1a55 100644 --- a/ide/src/rust/ide/src/double_representation/graph.rs +++ b/ide/src/rust/ide/src/double_representation/graph.rs @@ -2,10 +2,9 @@ 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; @@ -15,6 +14,7 @@ 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,24 +44,28 @@ pub enum LocationHint { // ================= /// Description of the graph, based on information available in AST. -#[derive(Clone,Debug,Shrinkwrap)] +#[derive(Clone,Debug)] 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() @@ -70,22 +74,7 @@ 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 { - 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![] - } + Self::from_function_binding(self.source.ast.clone()) } /// Gets the list of connections between the nodes in this graph. @@ -95,23 +84,18 @@ impl GraphInfo { /// Adds a new node to this graph. pub fn add_node - (&mut self, node:&NodeInfo, location_hint:LocationHint) -> FallibleResult { - let mut lines = self.source.block_lines(); + (&mut self, line_ast:Ast, 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) => self.locate_node(id)?.index.last() + 1, - LocationHint::Before(id) => self.locate_node(id)?.index.first(), + LocationHint::After(id) => node::index_in_lines(&lines, id)? + 1, + LocationHint::Before(id) => node::index_in_lines(&lines, id)? }; - let elem = Some(node.ast().clone_ref()); + let elem = Some(line_ast); 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) } @@ -140,35 +124,27 @@ 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 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) => {}, + 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); } - } 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 { + self.source.set_block_lines(lines) } - } - if lines.is_empty() { - self.source.set_body_ast(Self::empty_graph_body()); - Ok(()) } else { - self.source.set_block_lines(lines) + Err(node::IdNotFound {id}.into()) } - - // TODO tests for cases with comments involved } /// Sets expression of the given node. @@ -188,6 +164,30 @@ 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,7 +201,6 @@ 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; @@ -246,24 +245,11 @@ mod tests { } } - fn new_expression_node(parser:&parser::Parser, expression:&str) -> NodeInfo { + fn create_node_ast(parser:&parser::Parser, expression:&str) -> (Ast,ast::Id) { let node_ast = parser.parse(expression.to_string(), default()).unwrap(); let line_ast = expect_single_line(&node_ast).clone(); - 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()); + let id = line_ast.id.expect("line_ast should have an ID"); + (line_ast,id) } #[wasm_bindgen_test] @@ -273,20 +259,24 @@ mod tests { let mut graph = main_graph(&parser, program); let nodes = graph.nodes(); assert_eq!(nodes.len(), 1); - let initial_node = nodes[0].clone(); - assert_eq!(initial_node.expression().repr(), "print \"hello\""); + assert_eq!(nodes[0].expression().repr(), "print \"hello\""); let expr0 = "a + 2"; let expr1 = "b + 3"; - let node_to_add0 = new_expression_node(&parser, expr0); - let node_to_add1 = new_expression_node(&parser, expr1); + let (line_ast0,id0) = create_node_ast(&parser, expr0); + let (line_ast1,id1) = create_node_ast(&parser, expr1); - graph.add_node(&node_to_add0,LocationHint::Start).unwrap(); + graph.add_node(line_ast0, LocationHint::Start).unwrap(); assert_eq!(graph.nodes().len(), 2); - graph.add_node(&node_to_add1,LocationHint::Before(graph.nodes()[0].id())).unwrap(); + graph.add_node(line_ast1, LocationHint::Before(graph.nodes()[0].id())).unwrap(); let nodes = graph.nodes(); - assert_all(nodes.as_slice(), &[node_to_add1, node_to_add0, initial_node]); + 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\""); } #[wasm_bindgen_test] @@ -299,31 +289,29 @@ mod tests { let mut parser = parser::Parser::new_or_panic(); let mut graph = main_graph(&mut parser, program); - 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"); + 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"); - 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. + 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(); let nodes = graph.nodes(); assert_eq!(nodes.len(), 6); assert_eq!(nodes[0].expression().repr(), "a + b"); - assert_eq!(nodes[0].id(), node_to_add1.id()); - // Sic: `node_to_add1` was added at index `0`. + assert_eq!(nodes[0].id(), id1); assert_eq!(nodes[1].expression().repr(), "4 + 4"); - assert_eq!(nodes[1].id(), node_to_add0.id()); + assert_eq!(nodes[1].id(), id0); assert_eq!(nodes[2].expression().repr(), "x * x"); - assert_eq!(nodes[2].id(), node_to_add2.id()); + assert_eq!(nodes[2].id(), id2); 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(), node_to_add3.id()); + assert_eq!(nodes[5].id(), id3); let expected_code = r#"main = a + b @@ -337,10 +325,10 @@ mod tests { let mut graph = find_graph(&mut parser, program, "main.foo"); assert_eq!(graph.nodes().len(), 1); - graph.add_node(&node_to_add4, LocationHint::Start).unwrap(); + graph.add_node(line_ast4, LocationHint::Start).unwrap(); assert_eq!(graph.nodes().len(), 2); assert_eq!(graph.nodes()[0].expression().repr(), "2 - 2"); - assert_eq!(graph.nodes()[0].id(), node_to_add4.id()); + assert_eq!(graph.nodes()[0].id(), id4); assert_eq!(graph.nodes()[1].expression().repr(), "not_node"); } @@ -359,15 +347,15 @@ foo = 5"; let mut graph = main_graph(&mut parser, program); let id2 = graph.nodes()[0].id(); - 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"); + 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"); - 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(); + 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(); let expected_code = r"main = node0 @@ -386,30 +374,19 @@ foo = 5"; let mut parser = parser::Parser::new_or_panic(); let program = r" main = - ## Faux docstring - ## Docstring 0 - foo = node0 - ## Docstring 1 - # disabled node1 + foo = node foo a = not_node - ## Docstring 2 - node2 - node3 + node "; // 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[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); + assert_eq!(nodes.len(), 2); + for node in nodes.iter() { + assert_eq!(node.expression().repr(), "node"); + } } #[wasm_bindgen_test] diff --git a/ide/src/rust/ide/src/double_representation/module.rs b/ide/src/rust/ide/src/double_representation/module.rs index 6039bd829b2c..95f18b863a39 100644 --- a/ide/src/rust/ide/src/double_representation/module.rs +++ b/ide/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_ast(to_add.to_string()).unwrap(); + let import_ast = parser.parse_line(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_ast(code).unwrap(); + let ast = parser.parse_line(code).unwrap(); ImportInfo::from_ast(&ast).unwrap() }; diff --git a/ide/src/rust/ide/src/double_representation/node.rs b/ide/src/rust/ide/src/double_representation/node.rs index f440a20c6dcd..1c562e7a6fe4 100644 --- a/ide/src/rust/ide/src/double_representation/node.rs +++ b/ide/src/rust/ide/src/double_representation/node.rs @@ -2,18 +2,9 @@ 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; @@ -28,111 +19,23 @@ 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_main_line_of(line:&BlockLine>, id:Id) -> bool { - let node_info = MainLine::from_block_line(line); +pub fn is_node_by_id(line:&ast::BlockLine>, id:ast::Id) -> bool { + let node_info = NodeInfo::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 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) - } +/// 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()) } @@ -141,151 +44,43 @@ pub fn locate_many<'a> // === NodeInfo === // ================ -/// Iterator over indexed line ASTs that yields nodes. -#[derive(Clone,Debug)] -pub struct NodeIterator<'a, T:Iterator)> + 'a> { - /// Input iterator that yields pairs (line index, line's Ast). - pub lines_iter : T, - /// 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. +/// Description of the node that consists of all information locally available about node. +/// Nodes are required to bear IDs. This enum should never contain an ast of node without id set. #[derive(Clone,Debug)] #[allow(missing_docs)] -pub enum MainLine { +pub enum NodeInfo { /// 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 MainLine { +impl NodeInfo { /// 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(MainLine::Binding {infix}) + Some(NodeInfo::Binding {infix}) } /// Tries to interpret AST as node, treating whole AST as an expression. - pub fn new_expression(ast:Ast) -> Option { + pub fn new_expression(ast:Ast) -> Option { ast.id?; - // TODO what if we are given an assignment. - Some(MainLine::Expression {ast}) + Some(NodeInfo::Expression {ast}) } - /// Tries to interpret AST as node, treating whole AST as a node's primary line. - pub fn from_ast(ast:&Ast) -> Option { - // By definition, there are no nodes in the root scope. - // Being a node's line, we may assume that this is not a root scope. - let scope = ScopeKind::NonRoot; - 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_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 an expression. - pub fn from_block_line(line:&BlockLine>) -> Option { - Self::from_ast(line.elem.as_ref()?) + pub fn from_block_line(line:&ast::BlockLine>) -> Option { + Self::from_line_ast(line.elem.as_ref()?) } /// Node's unique ID. @@ -298,13 +93,13 @@ impl MainLine { /// Updates the node's AST so the node bears the given ID. pub fn set_id(&mut self, new_id:Id) { match self { - MainLine::Binding{ref mut infix} => { + NodeInfo::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."); } - MainLine::Expression{ref mut ast} => { + NodeInfo::Expression{ref mut ast} => { *ast = ast.with_id(new_id); } }; @@ -313,16 +108,16 @@ impl MainLine { /// AST of the node's expression. pub fn expression(&self) -> &Ast { match self { - MainLine::Binding {infix} => &infix.rarg, - MainLine::Expression{ast} => ast, + NodeInfo::Binding {infix} => &infix.rarg, + NodeInfo::Expression{ast} => ast, } } /// AST of the node's pattern (assignment's left-hand side). pub fn pattern(&self) -> Option<&Ast> { match self { - MainLine::Binding {infix} => Some(&infix.larg), - MainLine::Expression{..} => None, + NodeInfo::Binding {infix} => Some(&infix.larg), + NodeInfo::Expression{..} => None, } } @@ -330,9 +125,9 @@ impl MainLine { pub fn set_expression(&mut self, expression:Ast) { let id = self.id(); match self { - MainLine::Binding{ref mut infix} => + NodeInfo::Binding{ref mut infix} => infix.update_shape(|infix| infix.rarg = expression), - MainLine::Expression{ref mut ast} => *ast = expression, + NodeInfo::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); @@ -341,8 +136,8 @@ impl MainLine { /// The whole AST of node. pub fn ast(&self) -> &Ast { match self { - MainLine::Binding {infix} => infix.into(), - MainLine::Expression{ast} => ast, + NodeInfo::Binding {infix} => infix.into(), + NodeInfo::Expression{ast} => ast, } } @@ -350,11 +145,11 @@ impl MainLine { /// assignment infix will be introduced. pub fn set_pattern(&mut self, pattern:Ast) { match self { - MainLine::Binding {infix} => { + NodeInfo::Binding {infix} => { // Setting infix operand never fails. infix.update_shape(|infix| infix.larg = pattern) } - MainLine::Expression {ast} => { + NodeInfo::Expression {ast} => { let infix = ast::Infix { larg : pattern, loff : 1, @@ -363,7 +158,7 @@ impl MainLine { rarg : ast.clone(), }; let infix = known::Infix::new(infix, None); - *self = MainLine::Binding {infix}; + *self = NodeInfo::Binding {infix}; } } @@ -374,16 +169,16 @@ impl MainLine { /// If it is already an Expression node, no change is done. pub fn clear_pattern(&mut self) { match self { - MainLine::Binding {infix} => { - *self = MainLine::Expression {ast:infix.rarg.clone_ref()} + NodeInfo::Binding {infix} => { + *self = NodeInfo::Expression {ast:infix.rarg.clone_ref()} } - MainLine::Expression {..} => {} + NodeInfo::Expression {..} => {} } } } -impl ast::HasTokens for MainLine { +impl ast::HasTokens for NodeInfo { fn feed_to(&self, consumer:&mut impl ast::TokenConsumer) { self.ast().feed_to(consumer) } @@ -402,7 +197,7 @@ mod tests { use ast::opr::predefined::ASSIGNMENT; fn expect_node(ast:Ast, expression_text:&str, id:Id) { - let node_info = NodeInfo::from_main_line_ast(&ast).expect("expected a node"); + let node_info = NodeInfo::from_line_ast(&ast).expect("expected a node"); assert_eq!(node_info.expression().repr(),expression_text); assert_eq!(node_info.id(), id); } @@ -415,23 +210,12 @@ 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_main_line_ast(&ast).expect("expected a node"); + let mut node = NodeInfo::from_line_ast(&ast).expect("expected a node"); let id = node.id(); node.set_expression(Ast::var("bar")); assert_eq!(node.expression().repr(), "bar"); @@ -444,7 +228,7 @@ mod tests { let ast = Ast::number(4).with_new_id(); assert_eq!(ast.repr(), "4"); - let mut node = NodeInfo::from_main_line_ast(&ast).expect("expected a node"); + let mut node = NodeInfo::from_line_ast(&ast).expect("expected a node"); let id = node.id(); node.set_expression(Ast::var("bar")); assert_eq!(node.expression().repr(), "bar"); @@ -452,6 +236,17 @@ 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` @@ -461,7 +256,7 @@ mod tests { let rarg = Ast::new(number, Some(id)); let ast = Ast::infix(larg,ASSIGNMENT,rarg); - let mut node = NodeInfo::from_main_line_ast(&ast).unwrap(); + let mut node = NodeInfo::from_line_ast(&ast).unwrap(); assert_eq!(node.repr(),"foo = 4"); assert_eq!(node.id(),id); node.clear_pattern(); @@ -476,7 +271,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_main_line_ast(&line_ast).unwrap(); + let mut node = NodeInfo::from_line_ast(&line_ast).unwrap(); assert_eq!(node.repr(), "2"); assert_eq!(node.id(),id); @@ -492,7 +287,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_main_line_ast(&line_ast).unwrap(); + let mut node = NodeInfo::from_line_ast(&line_ast).unwrap(); assert_eq!(node.repr(), "foo = bar"); assert_eq!(node.id(),id); diff --git a/ide/src/rust/ide/src/double_representation/refactorings/collapse.rs b/ide/src/rust/ide/src/double_representation/refactorings/collapse.rs index 2f9fefa8dfaf..47494a7e7f41 100644 --- a/ide/src/rust/ide/src/double_representation/refactorings/collapse.rs +++ b/ide/src/rust/ide/src/double_representation/refactorings/collapse.rs @@ -12,7 +12,6 @@ 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; @@ -138,7 +137,7 @@ impl GraphHelper { -> FallibleResult { let mut updated_definition = self.info.source.clone(); let mut new_lines = Vec::new(); - for line in updated_definition.block_lines() { + for line in updated_definition.block_lines()? { match line_rewriter(&line)? { LineDisposition::Keep => new_lines.push(line), LineDisposition::Remove => {}, @@ -236,9 +235,9 @@ impl Extracted { Ok(Self {inputs,output,extracted_nodes,extracted_nodes_set}) } - /// 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)) + /// 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) } /// Generate AST of a line that needs to be appended to the extracted nodes' Asts. @@ -267,7 +266,7 @@ impl Extracted { // === Collapser === // ================= -/// Collapser rewrites the refactored definition line-by-line. This enum describes action to be +/// Collapser rewrites the refactoring definition line-by-line. This enum describes action to be /// taken for a given line. #[allow(missing_docs)] #[derive(Clone,Debug)] @@ -335,21 +334,17 @@ impl Collapser { pub fn rewrite_line (&self, line:&BlockLine>, extracted_definition:&definition::ToAdd) -> FallibleResult { - let ast = match line.elem.as_ref() { + let node_id = match line.elem.as_ref().and_then(NodeInfo::from_line_ast) { + Some(node_info) => node_info.id(), // We leave lines without nodes (blank lines) intact. - None => return Ok(LineDisposition::Keep), - Some(ast) => ast, + _ => return Ok(LineDisposition::Keep), }; - if !self.extracted.belongs_to_selection(ast) { + if !self.extracted.is_selected(node_id) { Ok(LineDisposition::Keep) - } 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, - }; + } else if node_id == self.replaced_node { + let expression = self.call_to_extracted(extracted_definition)?; + let no_node_err = failure::Error::from(CannotConstructCollapsedNode); + let mut new_node = NodeInfo::new_expression(expression.clone_ref()).ok_or(no_node_err)?; new_node.set_id(self.collapsed_node); if let Some(Output{identifier,..}) = &self.extracted.output { new_node.set_pattern(identifier.with_new_id().into()) @@ -384,6 +379,7 @@ 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; @@ -428,7 +424,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(|node| node.id()).collect_vec(); + let mut selected_nodes = nodes[extracted_lines].iter().map(NodeInfo::id).collect_vec(); run_internal(&selected_nodes); selected_nodes.reverse(); run_internal(&selected_nodes); diff --git a/ide/src/rust/ide/src/ide/integration/file_system.rs b/ide/src/rust/ide/src/ide/integration/file_system.rs index 8de7b07d3529..e86f3a5194ee 100644 --- a/ide/src/rust/ide/src/ide/integration/file_system.rs +++ b/ide/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 {type_,name,path}) + Some(Entry {name,path,type_}) }); entries_loaded.emit(Rc::new(entries.sorted().collect_vec())); } @@ -162,13 +162,13 @@ impl DirectoryView { type_ : FolderType::Standard, content : sub.into() }; - Entry {type_,name,path} + Entry {name,path,type_} } FileSystemObject::File {name,path} | FileSystemObject::Other {name,path} => { let path = to_file_browser_path(&path).join(&name); let type_ = EntryType::File; - Entry {type_,name,path} + Entry {name,path,type_} } } }); @@ -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/ide/src/rust/ide/src/ide/integration/project.rs b/ide/src/rust/ide/src/ide/integration/project.rs index 6c1e0bc17c29..c6fce38b84aa 100644 --- a/ide/src/rust/ide/src/ide/integration/project.rs +++ b/ide/src/rust/ide/src/ide/integration/project.rs @@ -739,7 +739,6 @@ 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), @@ -797,7 +796,6 @@ 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); } @@ -852,17 +850,6 @@ 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/ide/src/rust/ide/view/graph-editor/src/component/node.rs b/ide/src/rust/ide/view/graph-editor/src/component/node.rs index e09cc62fe96e..716050e4c4d7 100644 --- a/ide/src/rust/ide/view/graph-editor/src/component/node.rs +++ b/ide/src/rust/ide/view/graph-editor/src/component/node.rs @@ -56,9 +56,6 @@ 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; @@ -72,18 +69,6 @@ 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 === // ============= @@ -273,7 +258,6 @@ 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 @@ -295,7 +279,6 @@ 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), @@ -401,7 +384,6 @@ pub struct NodeModel { pub action_bar : action_bar::ActionBar, pub vcs_indicator : vcs::StatusIndicator, pub style : StyleWatchFrp, - pub comment : ensogl_text::Area, } impl NodeModel { @@ -476,14 +458,10 @@ impl NodeModel { let style = StyleWatchFrp::new(&app.display.scene().style_sheet); - // TODO: Style the documentation comment properly. - let comment = ensogl_text::Area::new(app); - 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,comment}.init() + ,vcs_indicator,style}.init() } pub fn get_crumbs_by_id(&self, id:ast::Id) -> Option { @@ -632,16 +610,6 @@ 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/ide/src/rust/ide/view/graph-editor/src/lib.rs b/ide/src/rust/ide/view/graph-editor/src/lib.rs index 091223d7877b..7df5bb8a64e3 100644 --- a/ide/src/rust/ide/view/graph-editor/src/lib.rs +++ b/ide/src/rust/ide/view/graph-editor/src/lib.rs @@ -483,7 +483,6 @@ 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)), @@ -556,7 +555,6 @@ 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), @@ -1185,10 +1183,6 @@ 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)); @@ -1585,14 +1579,6 @@ 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) { @@ -2604,12 +2590,6 @@ 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 @@ -3066,6 +3046,7 @@ 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/ide/src/rust/ide/view/src/debug_scenes/interface.rs b/ide/src/rust/ide/view/src/debug_scenes/interface.rs index e80382aaef6c..72532448444d 100644 --- a/ide/src/rust/ide/view/src/debug_scenes/interface.rs +++ b/ide/src/rust/ide/view/src/debug_scenes/interface.rs @@ -118,9 +118,6 @@ 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())); @@ -281,7 +278,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_ast(&code).unwrap(); + let ast = parser.parse_line(&code).unwrap(); let invocation_info = span_tree::generate::context::CalledMethodInfo {parameters}; let ctx = span_tree::generate::MockContext::new_single(ast.id.unwrap(),invocation_info); let output_span_tree = span_tree::SpanTree::default(); @@ -299,7 +296,7 @@ pub fn expression_mock() -> Expression { tp : Some("Text".to_owned()), }; let parameters = vec![this_param]; - let ast = parser.parse_line_ast(&code).unwrap(); + let ast = parser.parse_line(&code).unwrap(); let invocation_info = span_tree::generate::context::CalledMethodInfo {parameters}; let ctx = span_tree::generate::MockContext::new_single(ast.id.unwrap(),invocation_info); let output_span_tree = span_tree::SpanTree::default(); @@ -373,7 +370,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_ast(&code).unwrap(); + let ast = parser.parse_line(&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();