From efea07f31c4786945fba076f8acae2ee825975d1 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 15 Jul 2021 17:02:45 +0200 Subject: [PATCH 1/2] Add nested region folding test --- crates/ide/src/folding_ranges.rs | 59 +++++++++++++++++--------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/crates/ide/src/folding_ranges.rs b/crates/ide/src/folding_ranges.rs index 60f552fd92..66853bea7f 100755 --- a/crates/ide/src/folding_ranges.rs +++ b/crates/ide/src/folding_ranges.rs @@ -7,6 +7,9 @@ use syntax::{ SyntaxNode, TextRange, TextSize, }; +const REGION_START: &str = "// region:"; +const REGION_END: &str = "// endregion"; + #[derive(Debug, PartialEq, Eq)] pub enum FoldKind { Comment, @@ -30,8 +33,8 @@ pub struct Fold { // Feature: Folding // -// Defines folding regions for curly braced blocks, runs of consecutive import -// statements, and `region` / `endregion` comment markers. +// Defines folding regions for curly braced blocks, runs of consecutive use, mod, const or static +// items, and `region` / `endregion` comment markers. pub(crate) fn folding_ranges(file: &SourceFile) -> Vec { let mut res = vec![]; let mut visited_comments = FxHashSet::default(); @@ -39,8 +42,9 @@ pub(crate) fn folding_ranges(file: &SourceFile) -> Vec { let mut visited_mods = FxHashSet::default(); let mut visited_consts = FxHashSet::default(); let mut visited_statics = FxHashSet::default(); + // regions can be nested, here is a LIFO buffer - let mut regions_starts: Vec = vec![]; + let mut region_starts: Vec = vec![]; for element in file.syntax().descendants_with_tokens() { // Fold items that span multiple lines @@ -59,27 +63,23 @@ pub(crate) fn folding_ranges(file: &SourceFile) -> Vec { NodeOrToken::Token(token) => { // Fold groups of comments if let Some(comment) = ast::Comment::cast(token) { - if !visited_comments.contains(&comment) { - // regions are not real comments - if comment.text().trim().starts_with("// region:") { - regions_starts.push(comment.syntax().text_range().start()); - } else if comment.text().trim().starts_with("// endregion") { - if let Some(region) = regions_starts.pop() { - res.push(Fold { - range: TextRange::new( - region, - comment.syntax().text_range().end(), - ), - kind: FoldKind::Region, - }) - } - } else { - if let Some(range) = - contiguous_range_for_comment(comment, &mut visited_comments) - { - res.push(Fold { range, kind: FoldKind::Comment }) - } + if visited_comments.contains(&comment) { + continue; + } + let text = comment.text().trim_start(); + if text.starts_with(REGION_START) { + region_starts.push(comment.syntax().text_range().start()); + } else if text.starts_with(REGION_END) { + if let Some(region) = region_starts.pop() { + res.push(Fold { + range: TextRange::new(region, comment.syntax().text_range().end()), + kind: FoldKind::Region, + }) } + } else if let Some(range) = + contiguous_range_for_comment(comment, &mut visited_comments) + { + res.push(Fold { range, kind: FoldKind::Comment }) } } } @@ -286,7 +286,9 @@ mod tests { let (ranges, text) = extract_tags(ra_fixture, "fold"); let parse = SourceFile::parse(&text); - let folds = folding_ranges(&parse.tree()); + let mut folds = folding_ranges(&parse.tree()); + folds.sort_by_key(|fold| (fold.range.start(), fold.range.end())); + assert_eq!( folds.len(), ranges.len(), @@ -294,8 +296,8 @@ mod tests { ); for (fold, (range, attr)) in folds.iter().zip(ranges.into_iter()) { - assert_eq!(fold.range.start(), range.start()); - assert_eq!(fold.range.end(), range.end()); + assert_eq!(fold.range.start(), range.start(), "mismatched start of folding ranges"); + assert_eq!(fold.range.end(), range.end(), "mismatched end of folding ranges"); let kind = match fold.kind { FoldKind::Comment => "comment", @@ -525,7 +527,10 @@ const FOO: [usize; 4] = [ // 1. some normal comment // region: test // 2. some normal comment -calling_function(x,y); +// region: inner +fn f() {} +// endregion +fn f2() {} // endregion: test "#, ) From ed734609719b0b18906c500c8ce91fc14a61cb19 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 15 Jul 2021 17:44:23 +0200 Subject: [PATCH 2/2] Folding ranges respect item visibilities --- crates/ide/src/folding_ranges.rs | 161 ++++++++++++++----------------- 1 file changed, 75 insertions(+), 86 deletions(-) diff --git a/crates/ide/src/folding_ranges.rs b/crates/ide/src/folding_ranges.rs index 66853bea7f..bfb4ce711c 100755 --- a/crates/ide/src/folding_ranges.rs +++ b/crates/ide/src/folding_ranges.rs @@ -1,12 +1,14 @@ use rustc_hash::FxHashSet; use syntax::{ - ast::{self, AstNode, AstToken, VisibilityOwner}, - Direction, NodeOrToken, SourceFile, + ast::{self, AstNode, AstToken}, + match_ast, Direction, NodeOrToken, SourceFile, SyntaxKind::{self, *}, - SyntaxNode, TextRange, TextSize, + TextRange, TextSize, }; +use std::hash::Hash; + const REGION_START: &str = "// region:"; const REGION_END: &str = "// endregion"; @@ -84,46 +86,39 @@ pub(crate) fn folding_ranges(file: &SourceFile) -> Vec { } } NodeOrToken::Node(node) => { - // Fold groups of imports - if node.kind() == USE && !visited_imports.contains(&node) { - if let Some(range) = contiguous_range_for_group(&node, &mut visited_imports) { - res.push(Fold { range, kind: FoldKind::Imports }) - } - } - - // Fold groups of mods - if let Some(module) = ast::Module::cast(node.clone()) { - if !has_visibility(&node) - && !visited_mods.contains(&node) - && module.item_list().is_none() - { - if let Some(range) = contiguous_range_for_group_unless( - &node, - has_visibility, - &mut visited_mods, - ) { - res.push(Fold { range, kind: FoldKind::Mods }) - } - } - } - - // Fold groups of consts - if node.kind() == CONST && !visited_consts.contains(&node) { - if let Some(range) = contiguous_range_for_group(&node, &mut visited_consts) { - res.push(Fold { range, kind: FoldKind::Consts }) - } - } - // Fold groups of consts - if node.kind() == STATIC && !visited_statics.contains(&node) { - if let Some(range) = contiguous_range_for_group(&node, &mut visited_statics) { - res.push(Fold { range, kind: FoldKind::Statics }) - } - } - - // Fold where clause - if node.kind() == WHERE_CLAUSE { - if let Some(range) = fold_range_for_where_clause(&node) { - res.push(Fold { range, kind: FoldKind::WhereClause }) + match_ast! { + match node { + ast::Module(module) => { + if module.item_list().is_none() { + if let Some(range) = contiguous_range_for_item_group( + module, + &mut visited_mods, + ) { + res.push(Fold { range, kind: FoldKind::Mods }) + } + } + }, + ast::Use(use_) => { + if let Some(range) = contiguous_range_for_item_group(use_, &mut visited_imports) { + res.push(Fold { range, kind: FoldKind::Imports }) + } + }, + ast::Const(konst) => { + if let Some(range) = contiguous_range_for_item_group(konst, &mut visited_consts) { + res.push(Fold { range, kind: FoldKind::Consts }) + } + }, + ast::Static(statik) => { + if let Some(range) = contiguous_range_for_item_group(statik, &mut visited_statics) { + res.push(Fold { range, kind: FoldKind::Statics }) + } + }, + ast::WhereClause(where_clause) => { + if let Some(range) = fold_range_for_where_clause(where_clause) { + res.push(Fold { range, kind: FoldKind::WhereClause }) + } + }, + _ => (), } } } @@ -154,26 +149,16 @@ fn fold_kind(kind: SyntaxKind) -> Option { } } -fn has_visibility(node: &SyntaxNode) -> bool { - ast::Module::cast(node.clone()).and_then(|m| m.visibility()).is_some() -} +fn contiguous_range_for_item_group(first: N, visited: &mut FxHashSet) -> Option +where + N: ast::VisibilityOwner + Clone + Hash + Eq, +{ + if !visited.insert(first.clone()) { + return None; + } -fn contiguous_range_for_group( - first: &SyntaxNode, - visited: &mut FxHashSet, -) -> Option { - contiguous_range_for_group_unless(first, |_| false, visited) -} - -fn contiguous_range_for_group_unless( - first: &SyntaxNode, - unless: impl Fn(&SyntaxNode) -> bool, - visited: &mut FxHashSet, -) -> Option { - visited.insert(first.clone()); - - let mut last = first.clone(); - for element in first.siblings_with_tokens(Direction::Next) { + let (mut last, mut last_vis) = (first.clone(), first.visibility()); + for element in first.syntax().siblings_with_tokens(Direction::Next) { let node = match element { NodeOrToken::Token(token) => { if let Some(ws) = ast::Whitespace::cast(token) { @@ -189,23 +174,35 @@ fn contiguous_range_for_group_unless( NodeOrToken::Node(node) => node, }; - // Stop if we find a node that doesn't belong to the group - if node.kind() != first.kind() || unless(&node) { - break; + if let Some(next) = N::cast(node) { + let next_vis = next.visibility(); + if eq_visibility(next_vis.clone(), last_vis) { + visited.insert(next.clone()); + last_vis = next_vis; + last = next; + continue; + } } - - visited.insert(node.clone()); - last = node; + // Stop if we find an item of a different kind or with a different visibility. + break; } - if first != &last { - Some(TextRange::new(first.text_range().start(), last.text_range().end())) + if first != last { + Some(TextRange::new(first.syntax().text_range().start(), last.syntax().text_range().end())) } else { // The group consists of only one element, therefore it cannot be folded None } } +fn eq_visibility(vis0: Option, vis1: Option) -> bool { + match (vis0, vis1) { + (None, None) => true, + (Some(vis0), Some(vis1)) => vis0.is_eq_to(&vis1), + _ => false, + } +} + fn contiguous_range_for_comment( first: ast::Comment, visited: &mut FxHashSet, @@ -230,12 +227,9 @@ fn contiguous_range_for_comment( } if let Some(c) = ast::Comment::cast(token) { if c.kind() == group_kind { + let text = c.text().trim_start(); // regions are not real comments - if c.text().trim().starts_with("// region:") - || c.text().trim().starts_with("// endregion") - { - break; - } else { + if !(text.starts_with(REGION_START) || text.starts_with(REGION_END)) { visited.insert(c.clone()); last = c; continue; @@ -259,19 +253,14 @@ fn contiguous_range_for_comment( } } -fn fold_range_for_where_clause(node: &SyntaxNode) -> Option { - let first_where_pred = node.first_child(); - let last_where_pred = node.last_child(); +fn fold_range_for_where_clause(where_clause: ast::WhereClause) -> Option { + let first_where_pred = where_clause.predicates().next(); + let last_where_pred = where_clause.predicates().last(); if first_where_pred != last_where_pred { - let mut it = node.descendants_with_tokens(); - if let (Some(_where_clause), Some(where_kw), Some(last_comma)) = - (it.next(), it.next(), it.last()) - { - let start = where_kw.text_range().end(); - let end = last_comma.text_range().end(); - return Some(TextRange::new(start, end)); - } + let start = where_clause.where_token()?.text_range().end(); + let end = where_clause.syntax().text_range().end(); + return Some(TextRange::new(start, end)); } None }