Remove inherent methods from ast node that carry semantic meaning

crates/ide/src/folding_ranges.rs

@@ -1,3 +1,4 @@
+use ide_db::helpers::node_ext::vis_eq;
 use rustc_hash::FxHashSet;
 
 use syntax::{
@@ -198,7 +199,7 @@ where
 fn eq_visibility(vis0: Option<ast::Visibility>, vis1: Option<ast::Visibility>) -> bool {
     match (vis0, vis1) {
         (None, None) => true,
-        (Some(vis0), Some(vis1)) => vis0.is_eq_to(&vis1),
+        (Some(vis0), Some(vis1)) => vis_eq(&vis0, &vis1),
         _ => false,
     }
 }

crates/ide/src/highlight_related.rs

@@ -2,7 +2,7 @@ use hir::Semantics;
 use ide_db::{
     base_db::FilePosition,
     defs::{Definition, NameClass, NameRefClass},
-    helpers::{for_each_break_expr, for_each_tail_expr, pick_best_token},
+    helpers::{for_each_break_expr, for_each_tail_expr, node_ext::walk_expr, pick_best_token},
     search::{FileReference, ReferenceAccess, SearchScope},
     RootDatabase,
 };
@@ -122,7 +122,7 @@ fn highlight_exit_points(
     ) -> Option<Vec<HighlightedRange>> {
         let mut highlights = Vec::new();
         let body = body?;
-        body.walk(&mut |expr| match expr {
+        walk_expr(&body, &mut |expr| match expr {
             ast::Expr::ReturnExpr(expr) => {
                 if let Some(token) = expr.return_token() {
                     highlights.push(HighlightedRange { access: None, range: token.text_range() });
@@ -243,7 +243,7 @@ fn highlight_yield_points(token: SyntaxToken) -> Option<Vec<HighlightedRange>> {
         let mut highlights =
             vec![HighlightedRange { access: None, range: async_token?.text_range() }];
         if let Some(body) = body {
-            body.walk(&mut |expr| {
+            walk_expr(&body, &mut |expr| {
                 if let ast::Expr::AwaitExpr(expr) = expr {
                     if let Some(token) = expr.await_token() {
                         highlights

crates/ide/src/join_lines.rs

@@ -1,6 +1,7 @@
 use std::convert::TryFrom;
 
 use ide_assists::utils::extract_trivial_expression;
+use ide_db::helpers::node_ext::expr_as_name_ref;
 use itertools::Itertools;
 use syntax::{
     ast::{self, AstNode, AstToken, IsString},
@@ -263,7 +264,7 @@ fn join_assignments(
         return None;
     }
     let lhs = bin_expr.lhs()?;
-    let name_ref = lhs.name_ref()?;
+    let name_ref = expr_as_name_ref(&lhs)?;
 
     if name_ref.to_string() != let_ident_pat.syntax().to_string() {
         cov_mark::hit!(join_assignments_mismatch);

crates/ide_assists/src/handlers/add_explicit_type.rs

@@ -1,4 +1,5 @@
 use hir::HirDisplay;
+use ide_db::helpers::node_ext::walk_ty;
 use syntax::ast::{self, AstNode, LetStmt, Param};
 
 use crate::{AssistContext, AssistId, AssistKind, Assists};
@@ -46,7 +47,7 @@ pub(crate) fn add_explicit_type(acc: &mut Assists, ctx: &AssistContext) -> Optio
     // Don't enable the assist if there is a type ascription without any placeholders
     if let Some(ty) = &ascribed_ty {
         let mut contains_infer_ty = false;
-        ty.walk(&mut |ty| contains_infer_ty |= matches!(ty, ast::Type::InferType(_)));
+        walk_ty(ty, &mut |ty| contains_infer_ty |= matches!(ty, ast::Type::InferType(_)));
         if !contains_infer_ty {
             cov_mark::hit!(add_explicit_type_not_applicable_if_ty_already_specified);
             return None;

crates/ide_assists/src/handlers/convert_bool_then.rs

@@ -1,6 +1,10 @@
 use hir::{known, AsAssocItem, Semantics};
 use ide_db::{
-    helpers::{for_each_tail_expr, FamousDefs},
+    helpers::{
+        for_each_tail_expr,
+        node_ext::{block_as_lone_tail, preorder_expr},
+        FamousDefs,
+    },
     RootDatabase,
 };
 use itertools::Itertools;
@@ -218,7 +222,7 @@ fn is_invalid_body(
     expr: &ast::Expr,
 ) -> bool {
     let mut invalid = false;
-    expr.preorder(&mut |e| {
+    preorder_expr(expr, &mut |e| {
         invalid |=
             matches!(e, syntax::WalkEvent::Enter(ast::Expr::TryExpr(_) | ast::Expr::ReturnExpr(_)));
         invalid
@@ -252,7 +256,7 @@ fn block_is_none_variant(
     block: &ast::BlockExpr,
     none_variant: hir::Variant,
 ) -> bool {
-    block.as_lone_tail().and_then(|e| match e {
+    block_as_lone_tail(block).and_then(|e| match e {
         ast::Expr::PathExpr(pat) => match sema.resolve_path(&pat.path()?)? {
             hir::PathResolution::Def(hir::ModuleDef::Variant(v)) => Some(v),
             _ => None,

crates/ide_assists/src/handlers/extract_function.rs

@@ -5,6 +5,7 @@ use either::Either;
 use hir::{HirDisplay, InFile, Local, Semantics, TypeInfo};
 use ide_db::{
     defs::{Definition, NameRefClass},
+    helpers::node_ext::{preorder_expr, walk_expr, walk_pat, walk_patterns_in_expr},
     search::{FileReference, ReferenceAccess, SearchScope},
     RootDatabase,
 };
@@ -478,7 +479,7 @@ impl FunctionBody {
 
     fn walk_expr(&self, cb: &mut dyn FnMut(ast::Expr)) {
         match self {
-            FunctionBody::Expr(expr) => expr.walk(cb),
+            FunctionBody::Expr(expr) => walk_expr(expr, cb),
             FunctionBody::Span { parent, text_range } => {
                 parent
                     .statements()
@@ -488,12 +489,12 @@ impl FunctionBody {
                         ast::Stmt::Item(_) => None,
                         ast::Stmt::LetStmt(stmt) => stmt.initializer(),
                     })
-                    .for_each(|expr| expr.walk(cb));
+                    .for_each(|expr| walk_expr(&expr, cb));
                 if let Some(expr) = parent
                     .tail_expr()
                     .filter(|it| text_range.contains_range(it.syntax().text_range()))
                 {
-                    expr.walk(cb);
+                    walk_expr(&expr, cb);
                 }
             }
         }
@@ -501,7 +502,7 @@ impl FunctionBody {
 
     fn preorder_expr(&self, cb: &mut dyn FnMut(WalkEvent<ast::Expr>) -> bool) {
         match self {
-            FunctionBody::Expr(expr) => expr.preorder(cb),
+            FunctionBody::Expr(expr) => preorder_expr(expr, cb),
             FunctionBody::Span { parent, text_range } => {
                 parent
                     .statements()
@@ -511,12 +512,12 @@ impl FunctionBody {
                         ast::Stmt::Item(_) => None,
                         ast::Stmt::LetStmt(stmt) => stmt.initializer(),
                     })
-                    .for_each(|expr| expr.preorder(cb));
+                    .for_each(|expr| preorder_expr(&expr, cb));
                 if let Some(expr) = parent
                     .tail_expr()
                     .filter(|it| text_range.contains_range(it.syntax().text_range()))
                 {
-                    expr.preorder(cb);
+                    preorder_expr(&expr, cb);
                 }
             }
         }
@@ -524,7 +525,7 @@ impl FunctionBody {
 
     fn walk_pat(&self, cb: &mut dyn FnMut(ast::Pat)) {
         match self {
-            FunctionBody::Expr(expr) => expr.walk_patterns(cb),
+            FunctionBody::Expr(expr) => walk_patterns_in_expr(expr, cb),
             FunctionBody::Span { parent, text_range } => {
                 parent
                     .statements()
@@ -532,16 +533,16 @@ impl FunctionBody {
                     .for_each(|stmt| match stmt {
                         ast::Stmt::ExprStmt(expr_stmt) => {
                             if let Some(expr) = expr_stmt.expr() {
-                                expr.walk_patterns(cb)
+                                walk_patterns_in_expr(&expr, cb)
                             }
                         }
                         ast::Stmt::Item(_) => (),
                         ast::Stmt::LetStmt(stmt) => {
                             if let Some(pat) = stmt.pat() {
-                                pat.walk(cb);
+                                walk_pat(&pat, cb);
                             }
                             if let Some(expr) = stmt.initializer() {
-                                expr.walk_patterns(cb);
+                                walk_patterns_in_expr(&expr, cb);
                             }
                         }
                     });
@@ -549,7 +550,7 @@ impl FunctionBody {
                     .tail_expr()
                     .filter(|it| text_range.contains_range(it.syntax().text_range()))
                 {
-                    expr.walk_patterns(cb);
+                    walk_patterns_in_expr(&expr, cb);
                 }
             }
         }

crates/ide_assists/src/handlers/extract_type_alias.rs

@@ -1,4 +1,5 @@
 use either::Either;
+use ide_db::helpers::node_ext::walk_ty;
 use itertools::Itertools;
 use syntax::{
     ast::{self, edit::IndentLevel, AstNode, GenericParamsOwner, NameOwner},
@@ -120,7 +121,7 @@ fn collect_used_generics<'gp>(
     }
 
     let mut generics = Vec::new();
-    ty.walk(&mut |ty| match ty {
+    walk_ty(ty, &mut |ty| match ty {
         ast::Type::PathType(ty) => {
             if let Some(path) = ty.path() {
                 if let Some(name_ref) = path.as_single_name_ref() {

crates/ide_assists/src/handlers/wrap_return_type_in_result.rs

@@ -1,6 +1,6 @@
 use std::iter;
 
-use ide_db::helpers::{for_each_tail_expr, FamousDefs};
+use ide_db::helpers::{for_each_tail_expr, node_ext::walk_expr, FamousDefs};
 use syntax::{
     ast::{self, make, Expr},
     match_ast, AstNode,
@@ -54,7 +54,7 @@ pub(crate) fn wrap_return_type_in_result(acc: &mut Assists, ctx: &AssistContext)
 
             let mut exprs_to_wrap = Vec::new();
             let tail_cb = &mut |e: &_| tail_cb_impl(&mut exprs_to_wrap, e);
-            body.walk(&mut |expr| {
+            walk_expr(&body, &mut |expr| {
                 if let Expr::ReturnExpr(ret_expr) = expr {
                     if let Some(ret_expr_arg) = &ret_expr.expr() {
                         for_each_tail_expr(ret_expr_arg, tail_cb);

crates/ide_db/src/helpers.rs

@@ -1,15 +1,17 @@
 //! A module with ide helpers for high-level ide features.
+pub mod famous_defs;
+pub mod generated_lints;
 pub mod import_assets;
 pub mod insert_use;
 pub mod merge_imports;
+pub mod node_ext;
 pub mod rust_doc;
-pub mod generated_lints;
 
 use std::collections::VecDeque;
 
 use base_db::FileId;
 use either::Either;
-use hir::{Crate, Enum, ItemInNs, MacroDef, Module, ModuleDef, Name, ScopeDef, Semantics, Trait};
+use hir::{ItemInNs, MacroDef, ModuleDef, Name, Semantics};
 use syntax::{
     ast::{self, make, LoopBodyOwner},
     AstNode, Direction, SyntaxElement, SyntaxKind, SyntaxToken, TokenAtOffset, WalkEvent, T,
@@ -17,6 +19,8 @@ use syntax::{
 
 use crate::RootDatabase;
 
+pub use self::famous_defs::FamousDefs;
+
 pub fn item_name(db: &RootDatabase, item: ItemInNs) -> Option<Name> {
     match item {
         ItemInNs::Types(module_def_id) => ModuleDef::from(module_def_id).name(db),
@@ -27,7 +31,7 @@ pub fn item_name(db: &RootDatabase, item: ItemInNs) -> Option<Name> {
 
 /// Resolves the path at the cursor token as a derive macro if it inside a token tree of a derive attribute.
 pub fn try_resolve_derive_input_at(
-    sema: &Semantics<RootDatabase>,
+    sema: &hir::Semantics<RootDatabase>,
     derive_attr: &ast::Attr,
     cursor: &SyntaxToken,
 ) -> Option<MacroDef> {
@@ -113,123 +117,6 @@ pub fn visit_file_defs(
     module.impl_defs(db).into_iter().for_each(|impl_| cb(Either::Right(impl_)));
 }
 
-/// Helps with finding well-know things inside the standard library. This is
-/// somewhat similar to the known paths infra inside hir, but it different; We
-/// want to make sure that IDE specific paths don't become interesting inside
-/// the compiler itself as well.
-///
-/// Note that, by default, rust-analyzer tests **do not** include core or std
-/// libraries. If you are writing tests for functionality using [`FamousDefs`],
-/// you'd want to include minicore (see `test_utils::MiniCore`) declaration at
-/// the start of your tests:
-///
-/// ```
-/// //- minicore: iterator, ord, derive
-/// ```
-pub struct FamousDefs<'a, 'b>(pub &'a Semantics<'b, RootDatabase>, pub Option<Crate>);
-
-#[allow(non_snake_case)]
-impl FamousDefs<'_, '_> {
-    pub fn std(&self) -> Option<Crate> {
-        self.find_crate("std")
-    }
-
-    pub fn core(&self) -> Option<Crate> {
-        self.find_crate("core")
-    }
-
-    pub fn core_cmp_Ord(&self) -> Option<Trait> {
-        self.find_trait("core:cmp:Ord")
-    }
-
-    pub fn core_convert_From(&self) -> Option<Trait> {
-        self.find_trait("core:convert:From")
-    }
-
-    pub fn core_convert_Into(&self) -> Option<Trait> {
-        self.find_trait("core:convert:Into")
-    }
-
-    pub fn core_option_Option(&self) -> Option<Enum> {
-        self.find_enum("core:option:Option")
-    }
-
-    pub fn core_result_Result(&self) -> Option<Enum> {
-        self.find_enum("core:result:Result")
-    }
-
-    pub fn core_default_Default(&self) -> Option<Trait> {
-        self.find_trait("core:default:Default")
-    }
-
-    pub fn core_iter_Iterator(&self) -> Option<Trait> {
-        self.find_trait("core:iter:traits:iterator:Iterator")
-    }
-
-    pub fn core_iter_IntoIterator(&self) -> Option<Trait> {
-        self.find_trait("core:iter:traits:collect:IntoIterator")
-    }
-
-    pub fn core_iter(&self) -> Option<Module> {
-        self.find_module("core:iter")
-    }
-
-    pub fn core_ops_Deref(&self) -> Option<Trait> {
-        self.find_trait("core:ops:Deref")
-    }
-
-    fn find_trait(&self, path: &str) -> Option<Trait> {
-        match self.find_def(path)? {
-            hir::ScopeDef::ModuleDef(hir::ModuleDef::Trait(it)) => Some(it),
-            _ => None,
-        }
-    }
-
-    fn find_enum(&self, path: &str) -> Option<Enum> {
-        match self.find_def(path)? {
-            hir::ScopeDef::ModuleDef(hir::ModuleDef::Adt(hir::Adt::Enum(it))) => Some(it),
-            _ => None,
-        }
-    }
-
-    fn find_module(&self, path: &str) -> Option<Module> {
-        match self.find_def(path)? {
-            hir::ScopeDef::ModuleDef(hir::ModuleDef::Module(it)) => Some(it),
-            _ => None,
-        }
-    }
-
-    fn find_crate(&self, name: &str) -> Option<Crate> {
-        let krate = self.1?;
-        let db = self.0.db;
-        let res =
-            krate.dependencies(db).into_iter().find(|dep| dep.name.to_string() == name)?.krate;
-        Some(res)
-    }
-
-    fn find_def(&self, path: &str) -> Option<ScopeDef> {
-        let db = self.0.db;
-        let mut path = path.split(':');
-        let trait_ = path.next_back()?;
-        let std_crate = path.next()?;
-        let std_crate = self.find_crate(std_crate)?;
-        let mut module = std_crate.root_module(db);
-        for segment in path {
-            module = module.children(db).find_map(|child| {
-                let name = child.name(db)?;
-                if name.to_string() == segment {
-                    Some(child)
-                } else {
-                    None
-                }
-            })?;
-        }
-        let def =
-            module.scope(db, None).into_iter().find(|(name, _def)| name.to_string() == trait_)?.1;
-        Some(def)
-    }
-}
-
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub struct SnippetCap {
     _private: (),

crates/ide_db/src/helpers/famous_defs.rs

@@ -0,0 +1,121 @@
+//! See [`FamousDefs`].
+use hir::{Crate, Enum, Module, ScopeDef, Semantics, Trait};
+
+use crate::RootDatabase;
+
+/// Helps with finding well-know things inside the standard library. This is
+/// somewhat similar to the known paths infra inside hir, but it different; We
+/// want to make sure that IDE specific paths don't become interesting inside
+/// the compiler itself as well.
+///
+/// Note that, by default, rust-analyzer tests **do not** include core or std
+/// libraries. If you are writing tests for functionality using [`FamousDefs`],
+/// you'd want to include minicore (see `test_utils::MiniCore`) declaration at
+/// the start of your tests:
+///
+/// ```
+/// //- minicore: iterator, ord, derive
+/// ```
+pub struct FamousDefs<'a, 'b>(pub &'a Semantics<'b, RootDatabase>, pub Option<Crate>);
+
+#[allow(non_snake_case)]
+impl FamousDefs<'_, '_> {
+    pub fn std(&self) -> Option<Crate> {
+        self.find_crate("std")
+    }
+
+    pub fn core(&self) -> Option<Crate> {
+        self.find_crate("core")
+    }
+
+    pub fn core_cmp_Ord(&self) -> Option<Trait> {
+        self.find_trait("core:cmp:Ord")
+    }
+
+    pub fn core_convert_From(&self) -> Option<Trait> {
+        self.find_trait("core:convert:From")
+    }
+
+    pub fn core_convert_Into(&self) -> Option<Trait> {
+        self.find_trait("core:convert:Into")
+    }
+
+    pub fn core_option_Option(&self) -> Option<Enum> {
+        self.find_enum("core:option:Option")
+    }
+
+    pub fn core_result_Result(&self) -> Option<Enum> {
+        self.find_enum("core:result:Result")
+    }
+
+    pub fn core_default_Default(&self) -> Option<Trait> {
+        self.find_trait("core:default:Default")
+    }
+
+    pub fn core_iter_Iterator(&self) -> Option<Trait> {
+        self.find_trait("core:iter:traits:iterator:Iterator")
+    }
+
+    pub fn core_iter_IntoIterator(&self) -> Option<Trait> {
+        self.find_trait("core:iter:traits:collect:IntoIterator")
+    }
+
+    pub fn core_iter(&self) -> Option<Module> {
+        self.find_module("core:iter")
+    }
+
+    pub fn core_ops_Deref(&self) -> Option<Trait> {
+        self.find_trait("core:ops:Deref")
+    }
+
+    fn find_trait(&self, path: &str) -> Option<Trait> {
+        match self.find_def(path)? {
+            hir::ScopeDef::ModuleDef(hir::ModuleDef::Trait(it)) => Some(it),
+            _ => None,
+        }
+    }
+
+    fn find_enum(&self, path: &str) -> Option<Enum> {
+        match self.find_def(path)? {
+            hir::ScopeDef::ModuleDef(hir::ModuleDef::Adt(hir::Adt::Enum(it))) => Some(it),
+            _ => None,
+        }
+    }
+
+    fn find_module(&self, path: &str) -> Option<Module> {
+        match self.find_def(path)? {
+            hir::ScopeDef::ModuleDef(hir::ModuleDef::Module(it)) => Some(it),
+            _ => None,
+        }
+    }
+
+    fn find_crate(&self, name: &str) -> Option<Crate> {
+        let krate = self.1?;
+        let db = self.0.db;
+        let res =
+            krate.dependencies(db).into_iter().find(|dep| dep.name.to_string() == name)?.krate;
+        Some(res)
+    }
+
+    fn find_def(&self, path: &str) -> Option<ScopeDef> {
+        let db = self.0.db;
+        let mut path = path.split(':');
+        let trait_ = path.next_back()?;
+        let std_crate = path.next()?;
+        let std_crate = self.find_crate(std_crate)?;
+        let mut module = std_crate.root_module(db);
+        for segment in path {
+            module = module.children(db).find_map(|child| {
+                let name = child.name(db)?;
+                if name.to_string() == segment {
+                    Some(child)
+                } else {
+                    None
+                }
+            })?;
+        }
+        let def =
+            module.scope(db, None).into_iter().find(|(name, _def)| name.to_string() == trait_)?.1;
+        Some(def)
+    }
+}

crates/ide_db/src/helpers/merge_imports.rs

@@ -7,6 +7,8 @@ use syntax::{
     ted,
 };
 
+use crate::helpers::node_ext::vis_eq;
+
 /// What type of merges are allowed.
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum MergeBehavior {
@@ -292,7 +294,7 @@ fn path_segment_cmp(a: &ast::PathSegment, b: &ast::PathSegment) -> Ordering {
 pub fn eq_visibility(vis0: Option<ast::Visibility>, vis1: Option<ast::Visibility>) -> bool {
     match (vis0, vis1) {
         (None, None) => true,
-        (Some(vis0), Some(vis1)) => vis0.is_eq_to(&vis1),
+        (Some(vis0), Some(vis1)) => vis_eq(&vis0, &vis1),
         _ => false,
     }
 }

crates/ide_db/src/helpers/node_ext.rs

@@ -0,0 +1,212 @@
+//! Various helper functions to work with SyntaxNodes.
+use syntax::{
+    ast::{self, PathSegmentKind, VisibilityKind},
+    AstNode, WalkEvent,
+};
+
+pub fn expr_as_name_ref(expr: &ast::Expr) -> Option<ast::NameRef> {
+    if let ast::Expr::PathExpr(expr) = expr {
+        let path = expr.path()?;
+        let segment = path.segment()?;
+        let name_ref = segment.name_ref()?;
+        if path.qualifier().is_none() {
+            return Some(name_ref);
+        }
+    }
+    None
+}
+
+pub fn block_as_lone_tail(block: &ast::BlockExpr) -> Option<ast::Expr> {
+    block.statements().next().is_none().then(|| block.tail_expr()).flatten()
+}
+/// Preorder walk all the expression's child expressions.
+pub fn walk_expr(expr: &ast::Expr, cb: &mut dyn FnMut(ast::Expr)) {
+    preorder_expr(expr, &mut |ev| {
+        if let WalkEvent::Enter(expr) = ev {
+            cb(expr);
+        }
+        false
+    })
+}
+
+/// Preorder walk all the expression's child expressions preserving events.
+/// If the callback returns true on an [`WalkEvent::Enter`], the subtree of the expression will be skipped.
+/// Note that the subtree may already be skipped due to the context analysis this function does.
+pub fn preorder_expr(expr: &ast::Expr, cb: &mut dyn FnMut(WalkEvent<ast::Expr>) -> bool) {
+    let mut preorder = expr.syntax().preorder();
+    while let Some(event) = preorder.next() {
+        let node = match event {
+            WalkEvent::Enter(node) => node,
+            WalkEvent::Leave(node) => {
+                if let Some(expr) = ast::Expr::cast(node) {
+                    cb(WalkEvent::Leave(expr));
+                }
+                continue;
+            }
+        };
+        match ast::Stmt::cast(node.clone()) {
+            // recursively walk the initializer, skipping potential const pat expressions
+            // let statements aren't usually nested too deeply so this is fine to recurse on
+            Some(ast::Stmt::LetStmt(l)) => {
+                if let Some(expr) = l.initializer() {
+                    preorder_expr(&expr, cb);
+                }
+                preorder.skip_subtree();
+            }
+            // Don't skip subtree since we want to process the expression child next
+            Some(ast::Stmt::ExprStmt(_)) => (),
+            // This might be an expression
+            Some(ast::Stmt::Item(ast::Item::MacroCall(mcall))) => {
+                cb(WalkEvent::Enter(ast::Expr::MacroCall(mcall)));
+                preorder.skip_subtree();
+            }
+            // skip inner items which might have their own expressions
+            Some(ast::Stmt::Item(_)) => preorder.skip_subtree(),
+            None => {
+                // skip const args, those expressions are a different context
+                if ast::GenericArg::can_cast(node.kind()) {
+                    preorder.skip_subtree();
+                } else if let Some(expr) = ast::Expr::cast(node) {
+                    let is_different_context = match &expr {
+                        ast::Expr::EffectExpr(effect) => {
+                            matches!(
+                                effect.effect(),
+                                ast::Effect::Async(_) | ast::Effect::Try(_) | ast::Effect::Const(_)
+                            )
+                        }
+                        ast::Expr::ClosureExpr(_) => true,
+                        _ => false,
+                    };
+                    let skip = cb(WalkEvent::Enter(expr));
+                    if skip || is_different_context {
+                        preorder.skip_subtree();
+                    }
+                }
+            }
+        }
+    }
+}
+
+/// Preorder walk all the expression's child patterns.
+pub fn walk_patterns_in_expr(expr: &ast::Expr, cb: &mut dyn FnMut(ast::Pat)) {
+    let mut preorder = expr.syntax().preorder();
+    while let Some(event) = preorder.next() {
+        let node = match event {
+            WalkEvent::Enter(node) => node,
+            WalkEvent::Leave(_) => continue,
+        };
+        match ast::Stmt::cast(node.clone()) {
+            Some(ast::Stmt::LetStmt(l)) => {
+                if let Some(pat) = l.pat() {
+                    walk_pat(&pat, cb);
+                }
+                if let Some(expr) = l.initializer() {
+                    walk_patterns_in_expr(&expr, cb);
+                }
+                preorder.skip_subtree();
+            }
+            // Don't skip subtree since we want to process the expression child next
+            Some(ast::Stmt::ExprStmt(_)) => (),
+            // skip inner items which might have their own patterns
+            Some(ast::Stmt::Item(_)) => preorder.skip_subtree(),
+            None => {
+                // skip const args, those are a different context
+                if ast::GenericArg::can_cast(node.kind()) {
+                    preorder.skip_subtree();
+                } else if let Some(expr) = ast::Expr::cast(node.clone()) {
+                    let is_different_context = match &expr {
+                        ast::Expr::EffectExpr(effect) => match effect.effect() {
+                            ast::Effect::Async(_) | ast::Effect::Try(_) | ast::Effect::Const(_) => {
+                                true
+                            }
+                            ast::Effect::Unsafe(_) | ast::Effect::Label(_) => false,
+                        },
+                        ast::Expr::ClosureExpr(_) => true,
+                        _ => false,
+                    };
+                    if is_different_context {
+                        preorder.skip_subtree();
+                    }
+                } else if let Some(pat) = ast::Pat::cast(node) {
+                    preorder.skip_subtree();
+                    walk_pat(&pat, cb);
+                }
+            }
+        }
+    }
+}
+
+/// Preorder walk all the pattern's sub patterns.
+pub fn walk_pat(pat: &ast::Pat, cb: &mut dyn FnMut(ast::Pat)) {
+    let mut preorder = pat.syntax().preorder();
+    while let Some(event) = preorder.next() {
+        let node = match event {
+            WalkEvent::Enter(node) => node,
+            WalkEvent::Leave(_) => continue,
+        };
+        let kind = node.kind();
+        match ast::Pat::cast(node) {
+            Some(pat @ ast::Pat::ConstBlockPat(_)) => {
+                preorder.skip_subtree();
+                cb(pat);
+            }
+            Some(pat) => {
+                cb(pat);
+            }
+            // skip const args
+            None if ast::GenericArg::can_cast(kind) => {
+                preorder.skip_subtree();
+            }
+            None => (),
+        }
+    }
+}
+
+/// Preorder walk all the type's sub types.
+pub fn walk_ty(ty: &ast::Type, cb: &mut dyn FnMut(ast::Type)) {
+    let mut preorder = ty.syntax().preorder();
+    while let Some(event) = preorder.next() {
+        let node = match event {
+            WalkEvent::Enter(node) => node,
+            WalkEvent::Leave(_) => continue,
+        };
+        let kind = node.kind();
+        match ast::Type::cast(node) {
+            Some(ty @ ast::Type::MacroType(_)) => {
+                preorder.skip_subtree();
+                cb(ty)
+            }
+            Some(ty) => {
+                cb(ty);
+            }
+            // skip const args
+            None if ast::ConstArg::can_cast(kind) => {
+                preorder.skip_subtree();
+            }
+            None => (),
+        }
+    }
+}
+
+pub fn vis_eq(this: &ast::Visibility, other: &ast::Visibility) -> bool {
+    match (this.kind(), other.kind()) {
+        (VisibilityKind::In(this), VisibilityKind::In(other)) => {
+            stdx::iter_eq_by(this.segments(), other.segments(), |lhs, rhs| {
+                lhs.kind().zip(rhs.kind()).map_or(false, |it| match it {
+                    (PathSegmentKind::CrateKw, PathSegmentKind::CrateKw)
+                    | (PathSegmentKind::SelfKw, PathSegmentKind::SelfKw)
+                    | (PathSegmentKind::SuperKw, PathSegmentKind::SuperKw) => true,
+                    (PathSegmentKind::Name(lhs), PathSegmentKind::Name(rhs)) => {
+                        lhs.text() == rhs.text()
+                    }
+                    _ => false,
+                })
+            })
+        }
+        (VisibilityKind::PubSelf, VisibilityKind::PubSelf)
+        | (VisibilityKind::PubSuper, VisibilityKind::PubSuper)
+        | (VisibilityKind::PubCrate, VisibilityKind::PubCrate)
+        | (VisibilityKind::Pub, VisibilityKind::Pub) => true,
+        _ => false,
+    }
+}

crates/ide_db/src/rename.rs

@@ -34,6 +34,7 @@ use text_edit::{TextEdit, TextEditBuilder};
 
 use crate::{
     defs::Definition,
+    helpers::node_ext::expr_as_name_ref,
     search::FileReference,
     source_change::{FileSystemEdit, SourceChange},
     RootDatabase,
@@ -339,7 +340,7 @@ fn source_edit_from_name_ref(
     if let Some(record_field) = ast::RecordExprField::for_name_ref(name_ref) {
         let rcf_name_ref = record_field.name_ref();
         let rcf_expr = record_field.expr();
-        match &(rcf_name_ref, rcf_expr.and_then(|it| it.name_ref())) {
+        match &(rcf_name_ref, rcf_expr.and_then(|it| expr_as_name_ref(&it))) {
             // field: init-expr, check if we can use a field init shorthand
             (Some(field_name), Some(init)) => {
                 if field_name == name_ref {

crates/syntax/src/ast/expr_ext.rs

@@ -1,6 +1,6 @@
 //! Various extension methods to ast Expr Nodes, which are hard to code-generate.
-
+//!
-use rowan::WalkEvent;
+//! These methods should only do simple, shallow tasks related to the syntax of the node itself.
 
 use crate::{
     ast::{
@@ -28,139 +28,6 @@ impl ast::Expr {
                 | ast::Expr::EffectExpr(_)
         )
     }
-
-    pub fn name_ref(&self) -> Option<ast::NameRef> {
-        if let ast::Expr::PathExpr(expr) = self {
-            let path = expr.path()?;
-            let segment = path.segment()?;
-            let name_ref = segment.name_ref()?;
-            if path.qualifier().is_none() {
-                return Some(name_ref);
-            }
-        }
-        None
-    }
-
-    /// Preorder walk all the expression's child expressions.
-    pub fn walk(&self, cb: &mut dyn FnMut(ast::Expr)) {
-        self.preorder(&mut |ev| {
-            if let WalkEvent::Enter(expr) = ev {
-                cb(expr);
-            }
-            false
-        })
-    }
-
-    /// Preorder walk all the expression's child expressions preserving events.
-    /// If the callback returns true on an [`WalkEvent::Enter`], the subtree of the expression will be skipped.
-    /// Note that the subtree may already be skipped due to the context analysis this function does.
-    pub fn preorder(&self, cb: &mut dyn FnMut(WalkEvent<ast::Expr>) -> bool) {
-        let mut preorder = self.syntax().preorder();
-        while let Some(event) = preorder.next() {
-            let node = match event {
-                WalkEvent::Enter(node) => node,
-                WalkEvent::Leave(node) => {
-                    if let Some(expr) = ast::Expr::cast(node) {
-                        cb(WalkEvent::Leave(expr));
-                    }
-                    continue;
-                }
-            };
-            match ast::Stmt::cast(node.clone()) {
-                // recursively walk the initializer, skipping potential const pat expressions
-                // let statements aren't usually nested too deeply so this is fine to recurse on
-                Some(ast::Stmt::LetStmt(l)) => {
-                    if let Some(expr) = l.initializer() {
-                        expr.preorder(cb);
-                    }
-                    preorder.skip_subtree();
-                }
-                // Don't skip subtree since we want to process the expression child next
-                Some(ast::Stmt::ExprStmt(_)) => (),
-                // This might be an expression
-                Some(ast::Stmt::Item(ast::Item::MacroCall(mcall))) => {
-                    cb(WalkEvent::Enter(ast::Expr::MacroCall(mcall)));
-                    preorder.skip_subtree();
-                }
-                // skip inner items which might have their own expressions
-                Some(ast::Stmt::Item(_)) => preorder.skip_subtree(),
-                None => {
-                    // skip const args, those expressions are a different context
-                    if ast::GenericArg::can_cast(node.kind()) {
-                        preorder.skip_subtree();
-                    } else if let Some(expr) = ast::Expr::cast(node) {
-                        let is_different_context = match &expr {
-                            ast::Expr::EffectExpr(effect) => {
-                                matches!(
-                                    effect.effect(),
-                                    ast::Effect::Async(_)
-                                        | ast::Effect::Try(_)
-                                        | ast::Effect::Const(_)
-                                )
-                            }
-                            ast::Expr::ClosureExpr(_) => true,
-                            _ => false,
-                        };
-                        let skip = cb(WalkEvent::Enter(expr));
-                        if skip || is_different_context {
-                            preorder.skip_subtree();
-                        }
-                    }
-                }
-            }
-        }
-    }
-
-    /// Preorder walk all the expression's child patterns.
-    pub fn walk_patterns(&self, cb: &mut dyn FnMut(ast::Pat)) {
-        let mut preorder = self.syntax().preorder();
-        while let Some(event) = preorder.next() {
-            let node = match event {
-                WalkEvent::Enter(node) => node,
-                WalkEvent::Leave(_) => continue,
-            };
-            match ast::Stmt::cast(node.clone()) {
-                Some(ast::Stmt::LetStmt(l)) => {
-                    if let Some(pat) = l.pat() {
-                        pat.walk(cb);
-                    }
-                    if let Some(expr) = l.initializer() {
-                        expr.walk_patterns(cb);
-                    }
-                    preorder.skip_subtree();
-                }
-                // Don't skip subtree since we want to process the expression child next
-                Some(ast::Stmt::ExprStmt(_)) => (),
-                // skip inner items which might have their own patterns
-                Some(ast::Stmt::Item(_)) => preorder.skip_subtree(),
-                None => {
-                    // skip const args, those are a different context
-                    if ast::GenericArg::can_cast(node.kind()) {
-                        preorder.skip_subtree();
-                    } else if let Some(expr) = ast::Expr::cast(node.clone()) {
-                        let is_different_context = match &expr {
-                            ast::Expr::EffectExpr(effect) => {
-                                matches!(
-                                    effect.effect(),
-                                    ast::Effect::Async(_)
-                                        | ast::Effect::Try(_)
-                                        | ast::Effect::Const(_)
-                                )
-                            }
-                            ast::Expr::ClosureExpr(_) => true,
-                            _ => false,
-                        };
-                        if is_different_context {
-                            preorder.skip_subtree();
-                        }
-                    } else if let Some(pat) = ast::Pat::cast(node) {
-                        preorder.skip_subtree();
-                        pat.walk(cb);
-                    }
-                }
-            }
-        }
-    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -374,6 +241,7 @@ impl ast::Literal {
             .and_then(|e| e.into_token())
             .unwrap()
     }
+
     pub fn kind(&self) -> LiteralKind {
         let token = self.token();
 

crates/syntax/src/ast/node_ext.rs

@@ -1,11 +1,13 @@
 //! Various extension methods to ast Nodes, which are hard to code-generate.
 //! Extensions for various expressions live in a sibling `expr_extensions` module.
+//!
+//! These methods should only do simple, shallow tasks related to the syntax of the node itself.
 
 use std::{borrow::Cow, fmt, iter::successors};
 
 use itertools::Itertools;
 use parser::SyntaxKind;
-use rowan::{GreenNodeData, GreenTokenData, WalkEvent};
+use rowan::{GreenNodeData, GreenTokenData};
 
 use crate::{
     ast::{
@@ -56,66 +58,6 @@ impl ast::BlockExpr {
     pub fn is_empty(&self) -> bool {
         self.statements().next().is_none() && self.tail_expr().is_none()
     }
-
-    pub fn as_lone_tail(&self) -> Option<ast::Expr> {
-        self.statements().next().is_none().then(|| self.tail_expr()).flatten()
-    }
-}
-
-impl ast::Pat {
-    /// Preorder walk all the pattern's sub patterns.
-    pub fn walk(&self, cb: &mut dyn FnMut(ast::Pat)) {
-        let mut preorder = self.syntax().preorder();
-        while let Some(event) = preorder.next() {
-            let node = match event {
-                WalkEvent::Enter(node) => node,
-                WalkEvent::Leave(_) => continue,
-            };
-            let kind = node.kind();
-            match ast::Pat::cast(node) {
-                Some(pat @ ast::Pat::ConstBlockPat(_)) => {
-                    preorder.skip_subtree();
-                    cb(pat);
-                }
-                Some(pat) => {
-                    cb(pat);
-                }
-                // skip const args
-                None if ast::GenericArg::can_cast(kind) => {
-                    preorder.skip_subtree();
-                }
-                None => (),
-            }
-        }
-    }
-}
-
-impl ast::Type {
-    /// Preorder walk all the type's sub types.
-    pub fn walk(&self, cb: &mut dyn FnMut(ast::Type)) {
-        let mut preorder = self.syntax().preorder();
-        while let Some(event) = preorder.next() {
-            let node = match event {
-                WalkEvent::Enter(node) => node,
-                WalkEvent::Leave(_) => continue,
-            };
-            let kind = node.kind();
-            match ast::Type::cast(node) {
-                Some(ty @ ast::Type::MacroType(_)) => {
-                    preorder.skip_subtree();
-                    cb(ty)
-                }
-                Some(ty) => {
-                    cb(ty);
-                }
-                // skip const args
-                None if ast::ConstArg::can_cast(kind) => {
-                    preorder.skip_subtree();
-                }
-                None => (),
-            }
-        }
-    }
 }
 
 #[derive(Debug, PartialEq, Eq, Clone)]
@@ -443,7 +385,15 @@ impl ast::RecordExprField {
         if let Some(name_ref) = self.name_ref() {
             return Some(name_ref);
         }
-        self.expr()?.name_ref()
+        if let ast::Expr::PathExpr(expr) = self.expr()? {
+            let path = expr.path()?;
+            let segment = path.segment()?;
+            let name_ref = segment.name_ref()?;
+            if path.qualifier().is_none() {
+                return Some(name_ref);
+            }
+        }
+        None
     }
 }
 
@@ -721,29 +671,6 @@ impl ast::Visibility {
             None => VisibilityKind::Pub,
         }
     }
-
-    pub fn is_eq_to(&self, other: &Self) -> bool {
-        match (self.kind(), other.kind()) {
-            (VisibilityKind::In(this), VisibilityKind::In(other)) => {
-                stdx::iter_eq_by(this.segments(), other.segments(), |lhs, rhs| {
-                    lhs.kind().zip(rhs.kind()).map_or(false, |it| match it {
-                        (PathSegmentKind::CrateKw, PathSegmentKind::CrateKw)
-                        | (PathSegmentKind::SelfKw, PathSegmentKind::SelfKw)
-                        | (PathSegmentKind::SuperKw, PathSegmentKind::SuperKw) => true,
-                        (PathSegmentKind::Name(lhs), PathSegmentKind::Name(rhs)) => {
-                            lhs.text() == rhs.text()
-                        }
-                        _ => false,
-                    })
-                })
-            }
-            (VisibilityKind::PubSelf, VisibilityKind::PubSelf)
-            | (VisibilityKind::PubSuper, VisibilityKind::PubSuper)
-            | (VisibilityKind::PubCrate, VisibilityKind::PubCrate)
-            | (VisibilityKind::Pub, VisibilityKind::Pub) => true,
-            _ => false,
-        }
-    }
 }
 
 impl ast::LifetimeParam {