Move explicit range handling out of goto_definition, use OperatorClass instead

Author: duncpro

crates/hir/src/semantics.rs

@@ -17,7 +17,7 @@ use hir_def::{
     path::ModPath,
     resolver::{self, HasResolver, Resolver, TypeNs},
     type_ref::Mutability,
-    AsMacroCall, DefWithBodyId, FunctionId, MacroId, TraitId, VariantId,
+    AsMacroCall, DefWithBodyId, FunctionId, MacroId, StructId, TraitId, VariantId,
 };
 use hir_expand::{
     attrs::collect_attrs,
@@ -203,6 +203,10 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
         self.imp.descend_node_at_offset(node, offset).filter_map(|mut it| it.find_map(N::cast))
     }
 
+    pub fn resolve_range_expr(&self, range_expr: &ast::RangeExpr) -> Option<Struct> {
+        self.imp.resolve_range_expr(range_expr).map(Struct::from)
+    }
+
     pub fn resolve_await_to_poll(&self, await_expr: &ast::AwaitExpr) -> Option<Function> {
         self.imp.resolve_await_to_poll(await_expr).map(Function::from)
     }
@@ -1357,6 +1361,10 @@ impl<'db> SemanticsImpl<'db> {
         self.analyze(call.syntax())?.resolve_method_call_fallback(self.db, call)
     }
 
+    fn resolve_range_expr(&self, range_expr: &ast::RangeExpr) -> Option<StructId> {
+        self.analyze(range_expr.syntax())?.resolve_range_expr(self.db, range_expr)
+    }
+
     fn resolve_await_to_poll(&self, await_expr: &ast::AwaitExpr) -> Option<FunctionId> {
         self.analyze(await_expr.syntax())?.resolve_await_to_poll(self.db, await_expr)
     }

crates/hir/src/source_analyzer.rs

@@ -7,6 +7,11 @@
 //! purely for "IDE needs".
 use std::iter::{self, once};
 
+use crate::{
+    db::HirDatabase, semantics::PathResolution, Adt, AssocItem, BindingMode, BuiltinAttr,
+    BuiltinType, Callable, Const, DeriveHelper, Field, Function, Local, Macro, ModuleDef, Static,
+    Struct, ToolModule, Trait, TraitAlias, TupleField, Type, TypeAlias, Variant,
+};
 use either::Either;
 use hir_def::{
     body::{
@@ -21,7 +26,7 @@ use hir_def::{
     resolver::{resolver_for_scope, Resolver, TypeNs, ValueNs},
     type_ref::Mutability,
     AsMacroCall, AssocItemId, ConstId, DefWithBodyId, FieldId, FunctionId, ItemContainerId,
-    LocalFieldId, Lookup, ModuleDefId, TraitId, VariantId,
+    LocalFieldId, Lookup, ModuleDefId, StructId, TraitId, VariantId,
 };
 use hir_expand::{
     mod_path::path,
@@ -40,18 +45,13 @@ use hir_ty::{
 use intern::sym;
 use itertools::Itertools;
 use smallvec::SmallVec;
+use syntax::ast::{RangeItem, RangeOp};
 use syntax::{
     ast::{self, AstNode},
     SyntaxKind, SyntaxNode, TextRange, TextSize,
 };
 use triomphe::Arc;
 
-use crate::{
-    db::HirDatabase, semantics::PathResolution, Adt, AssocItem, BindingMode, BuiltinAttr,
-    BuiltinType, Callable, Const, DeriveHelper, Field, Function, Local, Macro, ModuleDef, Static,
-    Struct, ToolModule, Trait, TraitAlias, TupleField, Type, TypeAlias, Variant,
-};
-
 /// `SourceAnalyzer` is a convenience wrapper which exposes HIR API in terms of
 /// original source files. It should not be used inside the HIR itself.
 #[derive(Debug)]
@@ -348,6 +348,18 @@ impl SourceAnalyzer {
         }
     }
 
+    pub(crate) fn resolve_range_expr(
+        &self,
+        db: &dyn HirDatabase,
+        range_expr: &ast::RangeExpr,
+    ) -> Option<StructId> {
+        let path = match range_expr.op_kind()? {
+            RangeOp::Exclusive => path![core::ops::Range],
+            RangeOp::Inclusive => path![core::ops::RangeInclusive],
+        };
+        self.resolver.resolve_known_struct(db.upcast(), &path)
+    }
+
     pub(crate) fn resolve_await_to_poll(
         &self,
         db: &dyn HirDatabase,

crates/ide-db/src/defs.rs

@@ -5,14 +5,17 @@
 
 // FIXME: this badly needs rename/rewrite (matklad, 2020-02-06).
 
+use crate::documentation::{Documentation, HasDocs};
+use crate::famous_defs::FamousDefs;
+use crate::RootDatabase;
 use arrayvec::ArrayVec;
 use either::Either;
 use hir::{
     Adt, AsAssocItem, AsExternAssocItem, AssocItem, AttributeTemplate, BuiltinAttr, BuiltinType,
     Const, Crate, DefWithBody, DeriveHelper, DocLinkDef, ExternAssocItem, ExternCrateDecl, Field,
     Function, GenericParam, HasVisibility, HirDisplay, Impl, InlineAsmOperand, Label, Local, Macro,
-    Module, ModuleDef, Name, PathResolution, Semantics, Static, StaticLifetime, ToolModule, Trait,
-    TraitAlias, TupleField, TypeAlias, Variant, VariantDef, Visibility,
+    Module, ModuleDef, Name, PathResolution, Semantics, Static, StaticLifetime, Struct, ToolModule,
+    Trait, TraitAlias, TupleField, TypeAlias, Variant, VariantDef, Visibility,
 };
 use span::Edition;
 use stdx::{format_to, impl_from};
@@ -21,10 +24,6 @@ use syntax::{
     match_ast, SyntaxKind, SyntaxNode, SyntaxToken,
 };
 
-use crate::documentation::{Documentation, HasDocs};
-use crate::famous_defs::FamousDefs;
-use crate::RootDatabase;
-
 // FIXME: a more precise name would probably be `Symbol`?
 #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)]
 pub enum Definition {
@@ -319,6 +318,7 @@ impl IdentClass {
                         .map(IdentClass::NameClass)
                         .or_else(|| NameRefClass::classify_lifetime(sema, &lifetime).map(IdentClass::NameRefClass))
                 },
+                ast::RangeExpr(range_expr) => OperatorClass::classify_range(sema, &range_expr).map(IdentClass::Operator),
                 ast::AwaitExpr(await_expr) => OperatorClass::classify_await(sema, &await_expr).map(IdentClass::Operator),
                 ast::BinExpr(bin_expr) => OperatorClass::classify_bin(sema, &bin_expr).map(IdentClass::Operator),
                 ast::IndexExpr(index_expr) => OperatorClass::classify_index(sema, &index_expr).map(IdentClass::Operator),
@@ -372,6 +372,9 @@ impl IdentClass {
                 | OperatorClass::Index(func)
                 | OperatorClass::Try(func),
             ) => res.push(Definition::Function(func)),
+            IdentClass::Operator(OperatorClass::Range(struct0)) => {
+                res.push(Definition::Adt(Adt::Struct(struct0)))
+            }
         }
         res
     }
@@ -546,6 +549,7 @@ impl NameClass {
 
 #[derive(Debug)]
 pub enum OperatorClass {
+    Range(Struct),
     Await(Function),
     Prefix(Function),
     Index(Function),
@@ -554,6 +558,13 @@ pub enum OperatorClass {
 }
 
 impl OperatorClass {
+    pub fn classify_range(
+        sema: &Semantics<'_, RootDatabase>,
+        range_expr: &ast::RangeExpr,
+    ) -> Option<OperatorClass> {
+        sema.resolve_range_expr(range_expr).map(OperatorClass::Range)
+    }
+
     pub fn classify_await(
         sema: &Semantics<'_, RootDatabase>,
         await_expr: &ast::AwaitExpr,

crates/ide-db/src/famous_defs.rs

@@ -1,7 +1,7 @@
 //! See [`FamousDefs`].
 
 use base_db::{CrateOrigin, LangCrateOrigin, SourceDatabase};
-use hir::{Crate, Enum, Function, Macro, Module, ScopeDef, Semantics, Struct, Trait};
+use hir::{Crate, Enum, Function, Macro, Module, ScopeDef, Semantics, Trait};
 
 use crate::RootDatabase;
 
@@ -102,14 +102,6 @@ impl FamousDefs<'_, '_> {
         self.find_trait("core:ops:Drop")
     }
 
-    pub fn core_ops_Range(&self) -> Option<Struct> {
-        self.find_struct("core:ops:Range")
-    }
-
-    pub fn core_ops_RangeInclusive(&self) -> Option<Struct> {
-        self.find_struct("core:ops:RangeInclusive")
-    }
-
     pub fn core_marker_Copy(&self) -> Option<Trait> {
         self.find_trait("core:marker:Copy")
     }
@@ -145,13 +137,6 @@ impl FamousDefs<'_, '_> {
         .flatten()
     }
 
-    fn find_struct(&self, path: &str) -> Option<Struct> {
-        match self.find_def(path)? {
-            hir::ScopeDef::ModuleDef(hir::ModuleDef::Adt(hir::Adt::Struct(it))) => Some(it),
-            _ => None,
-        }
-    }
-
     fn find_trait(&self, path: &str) -> Option<Trait> {
         match self.find_def(path)? {
             hir::ScopeDef::ModuleDef(hir::ModuleDef::Trait(it)) => Some(it),

crates/ide/src/goto_definition.rs

@@ -5,15 +5,14 @@ use crate::{
     navigation_target::{self, ToNav},
     FilePosition, NavigationTarget, RangeInfo, TryToNav, UpmappingResult,
 };
-use hir::{Adt, AsAssocItem, AssocItem, FileRange, InFile, MacroFileIdExt, ModuleDef, Semantics};
+use hir::{AsAssocItem, AssocItem, FileRange, InFile, MacroFileIdExt, ModuleDef, Semantics};
 use ide_db::{
     base_db::{AnchoredPath, FileLoader, SourceDatabase},
     defs::{Definition, IdentClass},
     helpers::pick_best_token,
     RootDatabase, SymbolKind,
 };
 use itertools::Itertools;
-use ide_db::famous_defs::FamousDefs;
 use span::{Edition, FileId};
 use syntax::{
     ast::{self, HasLoopBody},
@@ -41,22 +40,6 @@ pub(crate) fn goto_definition(
 ) -> Option<RangeInfo<Vec<NavigationTarget>>> {
     let sema = &Semantics::new(db);
     let file = sema.parse_guess_edition(file_id).syntax().clone();
-
-    if let syntax::TokenAtOffset::Single(tok) = file.token_at_offset(offset) {
-        if let Some(module) = sema.file_to_module_def(file_id) {
-            let famous_defs = FamousDefs(sema, module.krate());
-            let maybe_famous_struct = match tok.kind() {
-                T![..]  => famous_defs.core_ops_Range(),
-                T![..=] => famous_defs.core_ops_RangeInclusive(),
-                _ => None
-            };
-            if let Some(fstruct) = maybe_famous_struct {
-                let target = def_to_nav(db, Definition::Adt(Adt::Struct(fstruct)));
-                return Some(RangeInfo::new(tok.text_range(), target));
-            }
-        }
-    }
-
     let edition =
         sema.attach_first_edition(file_id).map(|it| it.edition()).unwrap_or(Edition::CURRENT);
     let original_token = pick_best_token(file.token_at_offset(offset), |kind| match kind {
@@ -434,10 +417,10 @@ fn expr_to_nav(
 
 #[cfg(test)]
 mod tests {
+    use crate::fixture;
     use ide_db::FileRange;
     use itertools::Itertools;
     use syntax::SmolStr;
-    use crate::fixture;
 
     #[track_caller]
     fn check(ra_fixture: &str) {
@@ -466,9 +449,25 @@ mod tests {
         assert!(navs.is_empty(), "didn't expect this to resolve anywhere: {navs:?}")
     }
 
+    #[test]
+    fn goto_def_range_inclusive_0() {
+        let ra_fixture = r#"
+//- minicore: range
+fn f(a: usize, b: usize) {
+    for _ in a.$0.=b {
+
+    }
+}
+"#;
+        let (analysis, position, _) = fixture::annotations(ra_fixture);
+        let mut navs =
+            analysis.goto_definition(position).unwrap().expect("no definition found").info;
+        let Some(target) = navs.pop() else { panic!("no target found") };
+        assert_eq!(target.name, SmolStr::new_inline("RangeInclusive"));
+    }
 
     #[test]
-    fn goto_def_range_inclusive() {
+    fn goto_def_range_inclusive_1() {
         let ra_fixture = r#"
 //- minicore: range
 fn f(a: usize, b: usize) {
@@ -478,13 +477,14 @@ fn f(a: usize, b: usize) {
 }
 "#;
         let (analysis, position, _) = fixture::annotations(ra_fixture);
-        let mut navs = analysis.goto_definition(position).unwrap().expect("no definition found").info;
+        let mut navs =
+            analysis.goto_definition(position).unwrap().expect("no definition found").info;
         let Some(target) = navs.pop() else { panic!("no target found") };
         assert_eq!(target.name, SmolStr::new_inline("RangeInclusive"));
     }
 
     #[test]
-    fn goto_def_range_half_open() {
+    fn goto_def_range() {
         let ra_fixture = r#"
 //- minicore: range
 fn f(a: usize, b: usize) {
@@ -494,7 +494,8 @@ fn f(a: usize, b: usize) {
 }
 "#;
         let (analysis, position, _) = fixture::annotations(ra_fixture);
-        let mut navs = analysis.goto_definition(position).unwrap().expect("no definition found").info;
+        let mut navs =
+            analysis.goto_definition(position).unwrap().expect("no definition found").info;
         let Some(target) = navs.pop() else { panic!("no target found") };
         assert_eq!(target.name, SmolStr::new_inline("Range"));
     }