several highlighting cleanups

* make stuff more type-safe by using `BindPat` instead of just `Pat`
* don't add `mut` into binding hash
* reset shadow counter when we enter a function

6 files changed, 92 insertions, 68 deletions

diff --git a/crates/ra_hir/src/source_binder.rs b/crates/ra_hir/src/source_binder.rs
index e7bc4df97..55eb7da35 100644
--- a/crates/ra_hir/src/source_binder.rs
+++ b/crates/ra_hir/src/source_binder.rs
@@ -181,7 +181,7 @@ pub enum PathResolution {
     /// An item
     Def(crate::ModuleDef),
     /// A local binding (only value namespace)
-    LocalBinding(Either<AstPtr<ast::Pat>, AstPtr<ast::SelfParam>>),
+    LocalBinding(Either<AstPtr<ast::BindPat>, AstPtr<ast::SelfParam>>),
     /// A generic parameter
     GenericParam(u32),
     SelfType(crate::ImplBlock),
@@ -307,7 +307,18 @@ impl SourceAnalyzer {
         let res = match res {
             crate::Resolution::Def(it) => PathResolution::Def(it),
             crate::Resolution::LocalBinding(it) => {
-                PathResolution::LocalBinding(self.body_source_map.as_ref()?.pat_syntax(it)?)
+                // We get a `PatId` from resolver, but it actually can only
+                // point at `BindPat`, and not at the arbitrary pattern.
+                let pat_ptr = self.body_source_map.as_ref()?.pat_syntax(it)?;
+                let pat_ptr = match pat_ptr {
+                    Either::A(pat) => {
+                        let pat: AstPtr<ast::BindPat> =
+                            pat.cast_checking_kind(|kind| kind == BIND_PAT).unwrap();
+                        Either::A(pat)
+                    }
+                    Either::B(self_param) => Either::B(self_param),
+                };
+                PathResolution::LocalBinding(pat_ptr)
             }
             crate::Resolution::GenericParam(it) => PathResolution::GenericParam(it),
             crate::Resolution::SelfType(it) => PathResolution::SelfType(it),
diff --git a/crates/ra_ide_api/src/display/navigation_target.rs b/crates/ra_ide_api/src/display/navigation_target.rs
index 8cc853dd1..8aff5f2cd 100644
--- a/crates/ra_ide_api/src/display/navigation_target.rs
+++ b/crates/ra_ide_api/src/display/navigation_target.rs
@@ -91,24 +91,11 @@ impl NavigationTarget {
     pub(crate) fn from_pat(
         db: &RootDatabase,
         file_id: FileId,
-        pat: AstPtr<ast::Pat>,
+        pat: AstPtr<ast::BindPat>,
     ) -> NavigationTarget {
         let parse = db.parse(file_id);
-        let (name, full_range) = match pat.to_node(parse.tree().syntax()).kind() {
-            ast::PatKind::BindPat(pat) => return NavigationTarget::from_bind_pat(file_id, &pat),
-            _ => ("_".into(), pat.syntax_node_ptr().range()),
-        };
-
-        NavigationTarget {
-            file_id,
-            name,
-            full_range,
-            focus_range: None,
-            kind: NAME,
-            container_name: None,
-            description: None, //< No documentation for Description
-            docs: None,        //< No documentation for Pattern
-        }
+        let pat = pat.to_node(parse.tree().syntax());
+        NavigationTarget::from_bind_pat(file_id, &pat)
     }
 
     pub(crate) fn from_self_param(
diff --git a/crates/ra_ide_api/src/name_ref_kind.rs b/crates/ra_ide_api/src/name_ref_kind.rs
index 6832acf5d..f7db6c826 100644
--- a/crates/ra_ide_api/src/name_ref_kind.rs
+++ b/crates/ra_ide_api/src/name_ref_kind.rs
@@ -11,7 +11,7 @@ pub enum NameRefKind {
     AssocItem(hir::ImplItem),
     Def(hir::ModuleDef),
     SelfType(hir::Ty),
-    Pat(AstPtr<ast::Pat>),
+    Pat(AstPtr<ast::BindPat>),
     SelfParam(AstPtr<ast::SelfParam>),
     GenericParam(u32),
 }
diff --git a/crates/ra_ide_api/src/snapshots/rainbow_highlighting.html b/crates/ra_ide_api/src/snapshots/rainbow_highlighting.html
index c625d47bf..ed664817e 100644
--- a/crates/ra_ide_api/src/snapshots/rainbow_highlighting.html
+++ b/crates/ra_ide_api/src/snapshots/rainbow_highlighting.html
@@ -26,4 +26,8 @@ pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
 
     <span class="keyword">let</span> <span class="variable" data-binding-hash="1903207544374197704" style="color: hsl(58,61%,61%);">x</span> = <span class="string">"other color please!"</span>;
     <span class="keyword">let</span> <span class="variable" data-binding-hash="14878783531007968800" style="color: hsl(265,73%,83%);">y</span> = <span class="variable" data-binding-hash="1903207544374197704" style="color: hsl(58,61%,61%);">x</span>.<span class="text">to_string</span>();
+}
+
+<span class="keyword">fn</span> <span class="function">bar</span>() {
+    <span class="keyword">let</span> <span class="keyword">mut</span> <span class="variable.mut" data-binding-hash="3888301305669440875" style="color: hsl(242,59%,59%);">hello</span> = <span class="string">"hello"</span>;
 }</code></pre>
\ No newline at end of file
diff --git a/crates/ra_ide_api/src/syntax_highlighting.rs b/crates/ra_ide_api/src/syntax_highlighting.rs
index 16a728789..0e5253025 100644
--- a/crates/ra_ide_api/src/syntax_highlighting.rs
+++ b/crates/ra_ide_api/src/syntax_highlighting.rs
@@ -4,10 +4,17 @@ use hir::{Mutability, Ty};
 use ra_db::SourceDatabase;
 use ra_prof::profile;
 use ra_syntax::{
-    ast, AstNode, Direction, SmolStr, SyntaxElement, SyntaxKind, SyntaxKind::*, TextRange, T,
+    ast::{self, NameOwner},
+    AstNode, Direction, SmolStr, SyntaxElement, SyntaxKind,
+    SyntaxKind::*,
+    TextRange, T,
 };
 
-use crate::{db::RootDatabase, FileId};
+use crate::{
+    db::RootDatabase,
+    name_ref_kind::{classify_name_ref, NameRefKind::*},
+    FileId,
+};
 
 #[derive(Debug)]
 pub struct HighlightedRange {
@@ -31,25 +38,24 @@ fn is_control_keyword(kind: SyntaxKind) -> bool {
     }
 }
 
-fn is_variable_mutable(db: &RootDatabase, analyzer: &hir::SourceAnalyzer, pat: ast::Pat) -> bool {
-    let ty = analyzer.type_of_pat(db, &pat).unwrap_or(Ty::Unknown);
-    let is_ty_mut = {
-        if let Some((_, mutability)) = ty.as_reference() {
-            match mutability {
-                Mutability::Shared => false,
-                Mutability::Mut => true,
-            }
-        } else {
-            false
-        }
-    };
-
-    let is_pat_mut = match pat.kind() {
-        ast::PatKind::BindPat(bind_pat) => bind_pat.is_mutable(),
-        _ => false,
-    };
+fn is_variable_mutable(
+    db: &RootDatabase,
+    analyzer: &hir::SourceAnalyzer,
+    pat: ast::BindPat,
+) -> bool {
+    if pat.is_mutable() {
+        return true;
+    }
 
-    is_ty_mut || is_pat_mut
+    let ty = analyzer.type_of_pat(db, &pat.into()).unwrap_or(Ty::Unknown);
+    if let Some((_, mutability)) = ty.as_reference() {
+        match mutability {
+            Mutability::Shared => false,
+            Mutability::Mut => true,
+        }
+    } else {
+        false
+    }
 }
 
 pub(crate) fn highlight(db: &RootDatabase, file_id: FileId) -> Vec<HighlightedRange> {
@@ -81,44 +87,45 @@ pub(crate) fn highlight(db: &RootDatabase, file_id: FileId) -> Vec<HighlightedRa
         }
         let mut binding_hash = None;
         let tag = match node.kind() {
+            FN_DEF => {
+                bindings_shadow_count.clear();
+                continue;
+            }
             COMMENT => "comment",
             STRING | RAW_STRING | RAW_BYTE_STRING | BYTE_STRING => "string",
             ATTR => "attribute",
             NAME_REF => {
                 if let Some(name_ref) = node.as_node().cloned().and_then(ast::NameRef::cast) {
-                    // FIXME: revisit this after #1340
-                    use crate::name_ref_kind::{classify_name_ref, NameRefKind::*};
-                    use hir::{ImplItem, ModuleDef};
-
                     // FIXME: try to reuse the SourceAnalyzers
                     let analyzer = hir::SourceAnalyzer::new(db, file_id, name_ref.syntax(), None);
                     match classify_name_ref(db, &analyzer, &name_ref) {
                         Some(Method(_)) => "function",
                         Some(Macro(_)) => "macro",
                         Some(FieldAccess(_)) => "field",
-                        Some(AssocItem(ImplItem::Method(_))) => "function",
-                        Some(AssocItem(ImplItem::Const(_))) => "constant",
-                        Some(AssocItem(ImplItem::TypeAlias(_))) => "type",
-                        Some(Def(ModuleDef::Module(_))) => "module",
-                        Some(Def(ModuleDef::Function(_))) => "function",
-                        Some(Def(ModuleDef::Struct(_))) => "type",
-                        Some(Def(ModuleDef::Union(_))) => "type",
-                        Some(Def(ModuleDef::Enum(_))) => "type",
-                        Some(Def(ModuleDef::EnumVariant(_))) => "constant",
-                        Some(Def(ModuleDef::Const(_))) => "constant",
-                        Some(Def(ModuleDef::Static(_))) => "constant",
-                        Some(Def(ModuleDef::Trait(_))) => "type",
-                        Some(Def(ModuleDef::TypeAlias(_))) => "type",
-                        Some(Def(ModuleDef::BuiltinType(_))) => "type",
+                        Some(AssocItem(hir::ImplItem::Method(_))) => "function",
+                        Some(AssocItem(hir::ImplItem::Const(_))) => "constant",
+                        Some(AssocItem(hir::ImplItem::TypeAlias(_))) => "type",
+                        Some(Def(hir::ModuleDef::Module(_))) => "module",
+                        Some(Def(hir::ModuleDef::Function(_))) => "function",
+                        Some(Def(hir::ModuleDef::Struct(_))) => "type",
+                        Some(Def(hir::ModuleDef::Union(_))) => "type",
+                        Some(Def(hir::ModuleDef::Enum(_))) => "type",
+                        Some(Def(hir::ModuleDef::EnumVariant(_))) => "constant",
+                        Some(Def(hir::ModuleDef::Const(_))) => "constant",
+                        Some(Def(hir::ModuleDef::Static(_))) => "constant",
+                        Some(Def(hir::ModuleDef::Trait(_))) => "type",
+                        Some(Def(hir::ModuleDef::TypeAlias(_))) => "type",
+                        Some(Def(hir::ModuleDef::BuiltinType(_))) => "type",
                         Some(SelfType(_)) => "type",
                         Some(Pat(ptr)) => {
-                            binding_hash = Some({
-                                let text =
-                                    ptr.syntax_node_ptr().to_node(&root).text().to_smol_string();
+                            let pat = ptr.to_node(&root);
+                            if let Some(name) = pat.name() {
+                                let text = name.text();
                                 let shadow_count =
                                     bindings_shadow_count.entry(text.clone()).or_default();
-                                calc_binding_hash(file_id, &text, *shadow_count)
-                            });
+                                binding_hash =
+                                    Some(calc_binding_hash(file_id, &text, *shadow_count))
+                            }
 
                             if is_variable_mutable(db, &analyzer, ptr.to_node(&root)) {
                                 "variable.mut"
@@ -137,14 +144,14 @@ pub(crate) fn highlight(db: &RootDatabase, file_id: FileId) -> Vec<HighlightedRa
             NAME => {
                 if let Some(name) = node.as_node().cloned().and_then(ast::Name::cast) {
                     let analyzer = hir::SourceAnalyzer::new(db, file_id, name.syntax(), None);
-                    if let Some(pat) = name.syntax().ancestors().find_map(ast::Pat::cast) {
-                        binding_hash = Some({
-                            let text = name.syntax().text().to_smol_string();
+                    if let Some(pat) = name.syntax().ancestors().find_map(ast::BindPat::cast) {
+                        if let Some(name) = pat.name() {
+                            let text = name.text();
                             let shadow_count =
-                                bindings_shadow_count.entry(text.clone()).or_insert(0);
+                                bindings_shadow_count.entry(text.clone()).or_default();
                             *shadow_count += 1;
-                            calc_binding_hash(file_id, &text, *shadow_count)
-                        });
+                            binding_hash = Some(calc_binding_hash(file_id, &text, *shadow_count))
+                        }
 
                         if is_variable_mutable(db, &analyzer, pat) {
                             "variable.mut"
@@ -353,6 +360,10 @@ fn main() {
     let x = "other color please!";
     let y = x.to_string();
 }
+
+fn bar() {
+    let mut hello = "hello";
+}
 "#
             .trim(),
         );
diff --git a/crates/ra_syntax/src/ptr.rs b/crates/ra_syntax/src/ptr.rs
index 016256075..25824722f 100644
--- a/crates/ra_syntax/src/ptr.rs
+++ b/crates/ra_syntax/src/ptr.rs
@@ -60,6 +60,17 @@ impl<N: AstNode> AstPtr<N> {
     pub fn syntax_node_ptr(self) -> SyntaxNodePtr {
         self.raw
     }
+
+    // FIXME: extend AstNode to do this safely
+    pub fn cast_checking_kind<U: AstNode>(
+        self,
+        cond: impl FnOnce(SyntaxKind) -> bool,
+    ) -> Option<AstPtr<U>> {
+        if !cond(self.raw.kind()) {
+            return None;
+        }
+        Some(AstPtr { raw: self.raw, _ty: PhantomData })
+    }
 }
 
 impl<N: AstNode> From<AstPtr<N>> for SyntaxNodePtr {