7310: Add assist: add lifetime to type r=matklad a=bnjjj

close #7200 

7395: Obtain `ModuleId`'s `DefMap` through a method r=jonas-schievink a=jonas-schievink

Once `ModuleId` can represent modules inside block expressions, this will be required in order to obtain the correct `DefMap`. Otherwise existing code might use a `LocalModuleId` from the wrong `DefMap` and cause a panic.

bors r+

Co-authored-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
This commit is contained in:
bors[bot] 2021-01-22 15:45:50 +00:00 committed by GitHub
commit b16add934d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 292 additions and 32 deletions

View file

@ -0,0 +1,228 @@
use ast::FieldList;
use syntax::ast::{self, AstNode, GenericParamsOwner, NameOwner, RefType, Type};
use crate::{AssistContext, AssistId, AssistKind, Assists};
// Assist: add_lifetime_to_type
//
// Adds a new lifetime to a struct, enum or union.
//
// ```
// struct Point {
// x: &$0u32,
// y: u32,
// }
// ```
// ->
// ```
// struct Point<'a> {
// x: &'a u32,
// y: u32,
// }
// ```
pub(crate) fn add_lifetime_to_type(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
let ref_type_focused = ctx.find_node_at_offset::<ast::RefType>()?;
if ref_type_focused.lifetime().is_some() {
return None;
}
let node = ctx.find_node_at_offset::<ast::AdtDef>()?;
let has_lifetime = node
.generic_param_list()
.map(|gen_list| gen_list.lifetime_params().count() > 0)
.unwrap_or_default();
if has_lifetime {
return None;
}
let ref_types = fetch_borrowed_types(&node)?;
let target = node.syntax().text_range();
acc.add(
AssistId("add_lifetime_to_type", AssistKind::Generate),
"Add lifetime`",
target,
|builder| {
match node.generic_param_list() {
Some(gen_param) => {
if let Some(left_angle) = gen_param.l_angle_token() {
builder.insert(left_angle.text_range().end(), "'a, ");
}
}
None => {
if let Some(name) = node.name() {
builder.insert(name.syntax().text_range().end(), "<'a>");
}
}
}
for ref_type in ref_types {
if let Some(amp_token) = ref_type.amp_token() {
builder.insert(amp_token.text_range().end(), "'a ");
}
}
},
)
}
fn fetch_borrowed_types(node: &ast::AdtDef) -> Option<Vec<RefType>> {
let ref_types: Vec<RefType> = match node {
ast::AdtDef::Enum(enum_) => {
let variant_list = enum_.variant_list()?;
variant_list
.variants()
.filter_map(|variant| {
let field_list = variant.field_list()?;
find_ref_types_from_field_list(&field_list)
})
.flatten()
.collect()
}
ast::AdtDef::Struct(strukt) => {
let field_list = strukt.field_list()?;
find_ref_types_from_field_list(&field_list)?
}
ast::AdtDef::Union(un) => {
let record_field_list = un.record_field_list()?;
record_field_list
.fields()
.filter_map(|r_field| {
if let Type::RefType(ref_type) = r_field.ty()? {
if ref_type.lifetime().is_none() {
return Some(ref_type);
}
}
None
})
.collect()
}
};
if ref_types.is_empty() {
None
} else {
Some(ref_types)
}
}
fn find_ref_types_from_field_list(field_list: &FieldList) -> Option<Vec<RefType>> {
let ref_types: Vec<RefType> = match field_list {
ast::FieldList::RecordFieldList(record_list) => record_list
.fields()
.filter_map(|f| {
if let Type::RefType(ref_type) = f.ty()? {
if ref_type.lifetime().is_none() {
return Some(ref_type);
}
}
None
})
.collect(),
ast::FieldList::TupleFieldList(tuple_field_list) => tuple_field_list
.fields()
.filter_map(|f| {
if let Type::RefType(ref_type) = f.ty()? {
if ref_type.lifetime().is_none() {
return Some(ref_type);
}
}
None
})
.collect(),
};
if ref_types.is_empty() {
None
} else {
Some(ref_types)
}
}
#[cfg(test)]
mod tests {
use crate::tests::{check_assist, check_assist_not_applicable};
use super::*;
#[test]
fn add_lifetime_to_struct() {
check_assist(
add_lifetime_to_type,
"struct Foo { a: &$0i32 }",
"struct Foo<'a> { a: &'a i32 }",
);
check_assist(
add_lifetime_to_type,
"struct Foo { a: &$0i32, b: &usize }",
"struct Foo<'a> { a: &'a i32, b: &'a usize }",
);
check_assist(
add_lifetime_to_type,
"struct Foo { a: &$0i32, b: usize }",
"struct Foo<'a> { a: &'a i32, b: usize }",
);
check_assist(
add_lifetime_to_type,
"struct Foo<T> { a: &$0T, b: usize }",
"struct Foo<'a, T> { a: &'a T, b: usize }",
);
check_assist_not_applicable(add_lifetime_to_type, "struct Foo<'a> { a: &$0'a i32 }");
check_assist_not_applicable(add_lifetime_to_type, "struct Foo { a: &'a$0 i32 }");
}
#[test]
fn add_lifetime_to_enum() {
check_assist(
add_lifetime_to_type,
"enum Foo { Bar { a: i32 }, Other, Tuple(u32, &$0u32)}",
"enum Foo<'a> { Bar { a: i32 }, Other, Tuple(u32, &'a u32)}",
);
check_assist(
add_lifetime_to_type,
"enum Foo { Bar { a: &$0i32 }}",
"enum Foo<'a> { Bar { a: &'a i32 }}",
);
check_assist(
add_lifetime_to_type,
"enum Foo<T> { Bar { a: &$0i32, b: &T }}",
"enum Foo<'a, T> { Bar { a: &'a i32, b: &'a T }}",
);
check_assist_not_applicable(add_lifetime_to_type, "enum Foo<'a> { Bar { a: &$0'a i32 }}");
check_assist_not_applicable(add_lifetime_to_type, "enum Foo { Bar, $0Misc }");
}
#[test]
fn add_lifetime_to_union() {
check_assist(
add_lifetime_to_type,
"union Foo { a: &$0i32 }",
"union Foo<'a> { a: &'a i32 }",
);
check_assist(
add_lifetime_to_type,
"union Foo { a: &$0i32, b: &usize }",
"union Foo<'a> { a: &'a i32, b: &'a usize }",
);
check_assist(
add_lifetime_to_type,
"union Foo<T> { a: &$0T, b: usize }",
"union Foo<'a, T> { a: &'a T, b: usize }",
);
check_assist_not_applicable(add_lifetime_to_type, "struct Foo<'a> { a: &'a $0i32 }");
}
}

View file

@ -108,6 +108,7 @@ mod handlers {
pub(crate) type Handler = fn(&mut Assists, &AssistContext) -> Option<()>;
mod add_explicit_type;
mod add_lifetime_to_type;
mod add_missing_impl_members;
mod add_turbo_fish;
mod apply_demorgan;
@ -164,6 +165,7 @@ mod handlers {
&[
// These are alphabetic for the foolish consistency
add_explicit_type::add_explicit_type,
add_lifetime_to_type::add_lifetime_to_type,
add_turbo_fish::add_turbo_fish,
apply_demorgan::apply_demorgan,
auto_import::auto_import,

View file

@ -103,6 +103,25 @@ impl Trait<u32> for () {
)
}
#[test]
fn doctest_add_lifetime_to_type() {
check_doc_test(
"add_lifetime_to_type",
r#####"
struct Point {
x: &$0u32,
y: u32,
}
"#####,
r#####"
struct Point<'a> {
x: &'a u32,
y: u32,
}
"#####,
)
}
#[test]
fn doctest_add_turbo_fish() {
check_doc_test(

View file

@ -281,7 +281,7 @@ impl Module {
/// Name of this module.
pub fn name(self, db: &dyn HirDatabase) -> Option<Name> {
let def_map = db.crate_def_map(self.id.krate);
let def_map = self.id.def_map(db.upcast());
let parent = def_map[self.id.local_id].parent?;
def_map[parent].children.iter().find_map(|(name, module_id)| {
if *module_id == self.id.local_id {
@ -307,7 +307,7 @@ impl Module {
/// Iterates over all child modules.
pub fn children(self, db: &dyn HirDatabase) -> impl Iterator<Item = Module> {
let def_map = db.crate_def_map(self.id.krate);
let def_map = self.id.def_map(db.upcast());
let children = def_map[self.id.local_id]
.children
.iter()
@ -318,7 +318,7 @@ impl Module {
/// Finds a parent module.
pub fn parent(self, db: &dyn HirDatabase) -> Option<Module> {
let def_map = db.crate_def_map(self.id.krate);
let def_map = self.id.def_map(db.upcast());
let parent_id = def_map[self.id.local_id].parent?;
Some(self.with_module_id(parent_id))
}
@ -339,7 +339,7 @@ impl Module {
db: &dyn HirDatabase,
visible_from: Option<Module>,
) -> Vec<(Name, ScopeDef)> {
db.crate_def_map(self.id.krate)[self.id.local_id]
self.id.def_map(db.upcast())[self.id.local_id]
.scope
.entries()
.filter_map(|(name, def)| {
@ -362,14 +362,14 @@ impl Module {
}
pub fn visibility_of(self, db: &dyn HirDatabase, def: &ModuleDef) -> Option<Visibility> {
db.crate_def_map(self.id.krate)[self.id.local_id].scope.visibility_of(def.clone().into())
self.id.def_map(db.upcast())[self.id.local_id].scope.visibility_of(def.clone().into())
}
pub fn diagnostics(self, db: &dyn HirDatabase, sink: &mut DiagnosticSink) {
let _p = profile::span("Module::diagnostics").detail(|| {
format!("{:?}", self.name(db).map_or("<unknown>".into(), |name| name.to_string()))
});
let crate_def_map = db.crate_def_map(self.id.krate);
let crate_def_map = self.id.def_map(db.upcast());
crate_def_map.add_diagnostics(db.upcast(), self.id.local_id, sink);
for decl in self.declarations(db) {
match decl {
@ -396,12 +396,12 @@ impl Module {
}
pub fn declarations(self, db: &dyn HirDatabase) -> Vec<ModuleDef> {
let def_map = db.crate_def_map(self.id.krate);
let def_map = self.id.def_map(db.upcast());
def_map[self.id.local_id].scope.declarations().map(ModuleDef::from).collect()
}
pub fn impl_defs(self, db: &dyn HirDatabase) -> Vec<Impl> {
let def_map = db.crate_def_map(self.id.krate);
let def_map = self.id.def_map(db.upcast());
def_map[self.id.local_id].scope.impls().map(Impl::from).collect()
}

View file

@ -24,12 +24,12 @@ pub trait HasSource {
impl Module {
/// Returns a node which defines this module. That is, a file or a `mod foo {}` with items.
pub fn definition_source(self, db: &dyn HirDatabase) -> InFile<ModuleSource> {
let def_map = db.crate_def_map(self.id.krate);
let def_map = self.id.def_map(db.upcast());
def_map[self.id.local_id].definition_source(db.upcast())
}
pub fn is_mod_rs(self, db: &dyn HirDatabase) -> bool {
let def_map = db.crate_def_map(self.id.krate);
let def_map = self.id.def_map(db.upcast());
match def_map[self.id.local_id].origin {
ModuleOrigin::File { is_mod_rs, .. } => is_mod_rs,
_ => false,
@ -39,7 +39,7 @@ impl Module {
/// Returns a node which declares this module, either a `mod foo;` or a `mod foo {}`.
/// `None` for the crate root.
pub fn declaration_source(self, db: &dyn HirDatabase) -> Option<InFile<ast::Module>> {
let def_map = db.crate_def_map(self.id.krate);
let def_map = self.id.def_map(db.upcast());
def_map[self.id.local_id].declaration_source(db.upcast())
}
}

View file

@ -31,6 +31,7 @@ impl SourceToDefCtx<'_, '_> {
pub(super) fn file_to_def(&mut self, file: FileId) -> Option<ModuleId> {
let _p = profile::span("SourceBinder::to_module_def");
let (krate, local_id) = self.db.relevant_crates(file).iter().find_map(|&crate_id| {
// FIXME: inner items
let crate_def_map = self.db.crate_def_map(crate_id);
let local_id = crate_def_map.modules_for_file(file).next()?;
Some((crate_id, local_id))
@ -60,7 +61,7 @@ impl SourceToDefCtx<'_, '_> {
}?;
let child_name = src.value.name()?.as_name();
let def_map = self.db.crate_def_map(parent_module.krate);
let def_map = parent_module.def_map(self.db.upcast());
let child_id = *def_map[parent_module.local_id].children.get(&child_name)?;
Some(ModuleId { krate: parent_module.krate, local_id: child_id })
}

View file

@ -196,7 +196,7 @@ impl Attrs {
pub(crate) fn attrs_query(db: &dyn DefDatabase, def: AttrDefId) -> Attrs {
let raw_attrs = match def {
AttrDefId::ModuleId(module) => {
let def_map = db.crate_def_map(module.krate);
let def_map = module.def_map(db);
let mod_data = &def_map[module.local_id];
match mod_data.declaration_source(db) {
Some(it) => {

View file

@ -86,7 +86,7 @@ impl Expander {
module: ModuleId,
) -> Expander {
let cfg_expander = CfgExpander::new(db, current_file_id, module.krate);
let crate_def_map = db.crate_def_map(module.krate);
let crate_def_map = module.def_map(db);
let ast_id_map = db.ast_id_map(current_file_id);
Expander {
cfg_expander,

View file

@ -74,7 +74,7 @@ impl ChildBySource for ImplId {
impl ChildBySource for ModuleId {
fn child_by_source(&self, db: &dyn DefDatabase) -> DynMap {
let crate_def_map = db.crate_def_map(self.krate);
let crate_def_map = self.def_map(db);
let module_data = &crate_def_map[self.local_id];
module_data.scope.child_by_source(db)
}

View file

@ -110,7 +110,7 @@ fn find_path_inner(
// Base cases:
// - if the item is already in scope, return the name under which it is
let def_map = db.crate_def_map(from.krate);
let def_map = from.def_map(db);
let from_scope: &crate::item_scope::ItemScope = &def_map[from.local_id].scope;
let scope_name =
if let Some((name, _)) = from_scope.name_of(item) { Some(name.clone()) } else { None };
@ -145,7 +145,7 @@ fn find_path_inner(
// - if the item is in the prelude, return the name from there
if let Some(prelude_module) = def_map.prelude() {
let prelude_def_map = db.crate_def_map(prelude_module.krate);
let prelude_def_map = prelude_module.def_map(db);
let prelude_scope: &crate::item_scope::ItemScope =
&prelude_def_map[prelude_module.local_id].scope;
if let Some((name, vis)) = prelude_scope.name_of(item) {
@ -283,7 +283,7 @@ fn find_local_import_locations(
// above `from` with any visibility. That means we do not need to descend into private siblings
// of `from` (and similar).
let def_map = db.crate_def_map(from.krate);
let def_map = from.def_map(db);
// Compute the initial worklist. We start with all direct child modules of `from` as well as all
// of its (recursive) parent modules.
@ -312,7 +312,7 @@ fn find_local_import_locations(
&def_map[module.local_id]
} else {
// The crate might reexport a module defined in another crate.
ext_def_map = db.crate_def_map(module.krate);
ext_def_map = module.def_map(db);
&ext_def_map[module.local_id]
};
@ -375,7 +375,7 @@ mod tests {
parsed_path_file.syntax_node().descendants().find_map(syntax::ast::Path::cast).unwrap();
let mod_path = ModPath::from_src(ast_path, &Hygiene::new_unhygienic()).unwrap();
let crate_def_map = db.crate_def_map(module.krate);
let crate_def_map = module.def_map(&db);
let resolved = crate_def_map
.resolve_path(
&db,

View file

@ -83,7 +83,7 @@ impl ImportMap {
&def_map[module.local_id]
} else {
// The crate might reexport a module defined in another crate.
ext_def_map = db.crate_def_map(module.krate);
ext_def_map = module.def_map(db);
&ext_def_map[module.local_id]
};

View file

@ -50,7 +50,10 @@ pub mod import_map;
#[cfg(test)]
mod test_db;
use std::hash::{Hash, Hasher};
use std::{
hash::{Hash, Hasher},
sync::Arc,
};
use base_db::{impl_intern_key, salsa, CrateId};
use hir_expand::{
@ -58,6 +61,7 @@ use hir_expand::{
MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
};
use la_arena::Idx;
use nameres::DefMap;
use syntax::ast;
use crate::builtin_type::BuiltinType;
@ -73,6 +77,12 @@ pub struct ModuleId {
pub local_id: LocalModuleId,
}
impl ModuleId {
pub fn def_map(&self, db: &dyn db::DefDatabase) -> Arc<DefMap> {
db.crate_def_map(self.krate)
}
}
/// An ID of a module, **local** to a specific crate
pub type LocalModuleId = Idx<nameres::ModuleData>;

View file

@ -578,7 +578,7 @@ impl DefCollector<'_> {
} else if m.krate != self.def_map.krate {
mark::hit!(glob_across_crates);
// glob import from other crate => we can just import everything once
let item_map = self.db.crate_def_map(m.krate);
let item_map = m.def_map(self.db);
let scope = &item_map[m.local_id].scope;
// Module scoped macros is included

View file

@ -243,7 +243,7 @@ impl DefMap {
kind: PathKind::Super(0),
};
log::debug!("resolving {:?} in other crate", path);
let defp_map = db.crate_def_map(module.krate);
let defp_map = module.def_map(db);
let (def, s) = defp_map.resolve_path(db, module.local_id, &path, shadow);
return ResolvePathResult::with(
def,
@ -356,7 +356,7 @@ impl DefMap {
self
} else {
// Extend lifetime
keep = db.crate_def_map(prelude.krate);
keep = prelude.def_map(db);
&keep
};
def_map[prelude.local_id].scope.get(name)

View file

@ -430,7 +430,7 @@ impl Resolver {
for scope in &self.scopes {
if let Scope::ModuleScope(m) = scope {
if let Some(prelude) = m.crate_def_map.prelude() {
let prelude_def_map = db.crate_def_map(prelude.krate);
let prelude_def_map = prelude.def_map(db);
traits.extend(prelude_def_map[prelude.local_id].scope.traits());
}
traits.extend(m.crate_def_map[m.module_id].scope.traits());
@ -529,7 +529,7 @@ impl Scope {
f(name.clone(), ScopeDef::PerNs(def));
});
if let Some(prelude) = m.crate_def_map.prelude() {
let prelude_def_map = db.crate_def_map(prelude.krate);
let prelude_def_map = prelude.def_map(db);
prelude_def_map[prelude.local_id].scope.entries().for_each(|(name, def)| {
let seen_tuple = (name.clone(), def);
if !seen.contains(&seen_tuple) {
@ -633,7 +633,7 @@ pub trait HasResolver: Copy {
impl HasResolver for ModuleId {
fn resolver(self, db: &dyn DefDatabase) -> Resolver {
let def_map = db.crate_def_map(self.krate);
let def_map = self.def_map(db);
Resolver::default().push_module_scope(def_map, self.local_id)
}
}

View file

@ -103,7 +103,7 @@ impl Visibility {
if from_module.krate != to_module.krate {
return false;
}
let def_map = db.crate_def_map(from_module.krate);
let def_map = from_module.def_map(db);
self.is_visible_from_def_map(&def_map, from_module.local_id)
}

View file

@ -188,10 +188,10 @@ fn infer_with_mismatches(content: &str, include_mismatches: bool) -> String {
};
let module = db.module_for_file(file_id);
let crate_def_map = db.crate_def_map(module.krate);
let def_map = module.def_map(&db);
let mut defs: Vec<DefWithBodyId> = Vec::new();
visit_module(&db, &crate_def_map, module.local_id, &mut |it| defs.push(it));
visit_module(&db, &def_map, module.local_id, &mut |it| defs.push(it));
defs.sort_by_key(|def| match def {
DefWithBodyId::FunctionId(it) => {
let loc = it.lookup(&db);
@ -321,7 +321,7 @@ fn typing_whitespace_inside_a_function_should_not_invalidate_types() {
{
let events = db.log_executed(|| {
let module = db.module_for_file(pos.file_id);
let crate_def_map = db.crate_def_map(module.krate);
let crate_def_map = module.def_map(&db);
visit_module(&db, &crate_def_map, module.local_id, &mut |def| {
db.infer(def);
});