From fbdb32adfc49e0d69b7fd8e44135bea59382d2cb Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 12 Jan 2021 00:05:07 +0100 Subject: Group references by FileId --- .../handlers/extract_struct_from_enum_variant.rs | 59 ++++++++------ .../assists/src/handlers/inline_local_variable.rs | 95 +++++++++++----------- crates/assists/src/handlers/remove_unused_param.rs | 43 ++++++---- 3 files changed, 107 insertions(+), 90 deletions(-) (limited to 'crates/assists/src') diff --git a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs index 40028fc01..21b13977b 100644 --- a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs @@ -2,12 +2,16 @@ use std::iter; use either::Either; use hir::{AsName, Module, ModuleDef, Name, Variant}; -use ide_db::helpers::{ - insert_use::{insert_use, ImportScope}, - mod_path_to_ast, +use ide_db::{ + defs::Definition, + helpers::{ + insert_use::{insert_use, ImportScope}, + mod_path_to_ast, + }, + search::{FileReference, FileReferences}, + RootDatabase, }; -use ide_db::{defs::Definition, search::Reference, RootDatabase}; -use rustc_hash::{FxHashMap, FxHashSet}; +use rustc_hash::FxHashSet; use syntax::{ algo::{find_node_at_offset, SyntaxRewriter}, ast::{self, edit::IndentLevel, make, AstNode, NameOwner, VisibilityOwner}, @@ -58,29 +62,29 @@ pub(crate) fn extract_struct_from_enum_variant( let mut visited_modules_set = FxHashSet::default(); let current_module = enum_hir.module(ctx.db()); visited_modules_set.insert(current_module); - let mut rewriters = FxHashMap::default(); - for reference in usages { - let rewriter = rewriters - .entry(reference.file_range.file_id) - .or_insert_with(SyntaxRewriter::default); - let source_file = ctx.sema.parse(reference.file_range.file_id); - update_reference( - ctx, - rewriter, - reference, - &source_file, - &enum_module_def, - &variant_hir_name, - &mut visited_modules_set, - ); - } - let mut rewriter = - rewriters.remove(&ctx.frange.file_id).unwrap_or_else(SyntaxRewriter::default); - for (file_id, rewriter) in rewriters { + let mut def_rewriter = None; + for FileReferences { file_id, references: refs } in usages { + let mut rewriter = SyntaxRewriter::default(); + let source_file = ctx.sema.parse(file_id); + for reference in refs { + update_reference( + ctx, + &mut rewriter, + reference, + &source_file, + &enum_module_def, + &variant_hir_name, + &mut visited_modules_set, + ); + } + if file_id == ctx.frange.file_id { + def_rewriter = Some(rewriter); + continue; + } builder.edit_file(file_id); builder.rewrite(rewriter); } - builder.edit_file(ctx.frange.file_id); + let mut rewriter = def_rewriter.unwrap_or_default(); update_variant(&mut rewriter, &variant); extract_struct_def( &mut rewriter, @@ -90,6 +94,7 @@ pub(crate) fn extract_struct_from_enum_variant( &variant.parent_enum().syntax().clone().into(), enum_ast.visibility(), ); + builder.edit_file(ctx.frange.file_id); builder.rewrite(rewriter); }, ) @@ -205,13 +210,13 @@ fn update_variant(rewriter: &mut SyntaxRewriter, variant: &ast::Variant) -> Opti fn update_reference( ctx: &AssistContext, rewriter: &mut SyntaxRewriter, - reference: Reference, + reference: FileReference, source_file: &SourceFile, enum_module_def: &ModuleDef, variant_hir_name: &Name, visited_modules_set: &mut FxHashSet, ) -> Option<()> { - let offset = reference.file_range.range.start(); + let offset = reference.range.start(); let (segment, expr) = if let Some(path_expr) = find_node_at_offset::(source_file.syntax(), offset) { diff --git a/crates/assists/src/handlers/inline_local_variable.rs b/crates/assists/src/handlers/inline_local_variable.rs index d559be9cb..928df6825 100644 --- a/crates/assists/src/handlers/inline_local_variable.rs +++ b/crates/assists/src/handlers/inline_local_variable.rs @@ -1,4 +1,7 @@ -use ide_db::{defs::Definition, search::ReferenceKind}; +use ide_db::{ + defs::Definition, + search::{FileReference, ReferenceKind}, +}; use syntax::{ ast::{self, AstNode, AstToken}, TextRange, @@ -63,48 +66,44 @@ pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext) -> O let_stmt.syntax().text_range() }; - let mut wrap_in_parens = vec![true; refs.len()]; - - for (i, desc) in refs.iter().enumerate() { - let usage_node = ctx - .covering_node_for_range(desc.file_range.range) - .ancestors() - .find_map(ast::PathExpr::cast)?; - let usage_parent_option = usage_node.syntax().parent().and_then(ast::Expr::cast); - let usage_parent = match usage_parent_option { - Some(u) => u, - None => { - wrap_in_parens[i] = false; - continue; - } - }; - - wrap_in_parens[i] = match (&initializer_expr, usage_parent) { - (ast::Expr::CallExpr(_), _) - | (ast::Expr::IndexExpr(_), _) - | (ast::Expr::MethodCallExpr(_), _) - | (ast::Expr::FieldExpr(_), _) - | (ast::Expr::TryExpr(_), _) - | (ast::Expr::RefExpr(_), _) - | (ast::Expr::Literal(_), _) - | (ast::Expr::TupleExpr(_), _) - | (ast::Expr::ArrayExpr(_), _) - | (ast::Expr::ParenExpr(_), _) - | (ast::Expr::PathExpr(_), _) - | (ast::Expr::BlockExpr(_), _) - | (ast::Expr::EffectExpr(_), _) - | (_, ast::Expr::CallExpr(_)) - | (_, ast::Expr::TupleExpr(_)) - | (_, ast::Expr::ArrayExpr(_)) - | (_, ast::Expr::ParenExpr(_)) - | (_, ast::Expr::ForExpr(_)) - | (_, ast::Expr::WhileExpr(_)) - | (_, ast::Expr::BreakExpr(_)) - | (_, ast::Expr::ReturnExpr(_)) - | (_, ast::Expr::MatchExpr(_)) => false, - _ => true, - }; - } + let wrap_in_parens = refs + .iter() + .flat_map(|refs| &refs.references) + .map(|&FileReference { range, .. }| { + let usage_node = + ctx.covering_node_for_range(range).ancestors().find_map(ast::PathExpr::cast)?; + let usage_parent_option = usage_node.syntax().parent().and_then(ast::Expr::cast); + let usage_parent = match usage_parent_option { + Some(u) => u, + None => return Ok(false), + }; + + Ok(!matches!((&initializer_expr, usage_parent), + (ast::Expr::CallExpr(_), _) + | (ast::Expr::IndexExpr(_), _) + | (ast::Expr::MethodCallExpr(_), _) + | (ast::Expr::FieldExpr(_), _) + | (ast::Expr::TryExpr(_), _) + | (ast::Expr::RefExpr(_), _) + | (ast::Expr::Literal(_), _) + | (ast::Expr::TupleExpr(_), _) + | (ast::Expr::ArrayExpr(_), _) + | (ast::Expr::ParenExpr(_), _) + | (ast::Expr::PathExpr(_), _) + | (ast::Expr::BlockExpr(_), _) + | (ast::Expr::EffectExpr(_), _) + | (_, ast::Expr::CallExpr(_)) + | (_, ast::Expr::TupleExpr(_)) + | (_, ast::Expr::ArrayExpr(_)) + | (_, ast::Expr::ParenExpr(_)) + | (_, ast::Expr::ForExpr(_)) + | (_, ast::Expr::WhileExpr(_)) + | (_, ast::Expr::BreakExpr(_)) + | (_, ast::Expr::ReturnExpr(_)) + | (_, ast::Expr::MatchExpr(_)) + )) + }) + .collect::, _>>()?; let init_str = initializer_expr.syntax().text().to_string(); let init_in_paren = format!("({})", &init_str); @@ -116,15 +115,17 @@ pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext) -> O target, move |builder| { builder.delete(delete_range); - for (desc, should_wrap) in refs.iter().zip(wrap_in_parens) { + for (reference, should_wrap) in + refs.iter().flat_map(|refs| &refs.references).zip(wrap_in_parens) + { let replacement = if should_wrap { init_in_paren.clone() } else { init_str.clone() }; - match desc.kind { + match reference.kind { ReferenceKind::FieldShorthandForLocal => { mark::hit!(inline_field_shorthand); - builder.insert(desc.file_range.range.end(), format!(": {}", replacement)) + builder.insert(reference.range.end(), format!(": {}", replacement)) } - _ => builder.replace(desc.file_range.range, replacement), + _ => builder.replace(reference.range, replacement), } } }, diff --git a/crates/assists/src/handlers/remove_unused_param.rs b/crates/assists/src/handlers/remove_unused_param.rs index 56e8b5229..4f3b8ac46 100644 --- a/crates/assists/src/handlers/remove_unused_param.rs +++ b/crates/assists/src/handlers/remove_unused_param.rs @@ -1,8 +1,11 @@ -use ide_db::{defs::Definition, search::Reference}; +use ide_db::{ + defs::Definition, + search::{FileReference, FileReferences}, +}; use syntax::{ algo::find_node_at_range, ast::{self, ArgListOwner}, - AstNode, SyntaxKind, SyntaxNode, TextRange, T, + AstNode, SourceFile, SyntaxKind, SyntaxNode, TextRange, T, }; use test_utils::mark; use SyntaxKind::WHITESPACE; @@ -58,32 +61,40 @@ pub(crate) fn remove_unused_param(acc: &mut Assists, ctx: &AssistContext) -> Opt param.syntax().text_range(), |builder| { builder.delete(range_to_remove(param.syntax())); - for usage in fn_def.usages(&ctx.sema).all() { - process_usage(ctx, builder, usage, param_position); + for usages in fn_def.usages(&ctx.sema).all() { + process_usages(ctx, builder, usages, param_position); } }, ) } -fn process_usage( +fn process_usages( ctx: &AssistContext, builder: &mut AssistBuilder, - usage: Reference, + usages: FileReferences, + arg_to_remove: usize, +) { + let source_file = ctx.sema.parse(usages.file_id); + builder.edit_file(usages.file_id); + for usage in usages.references { + if let Some(text_range) = process_usage(&source_file, usage, arg_to_remove) { + builder.delete(text_range); + } + } +} + +fn process_usage( + source_file: &SourceFile, + FileReference { range, .. }: FileReference, arg_to_remove: usize, -) -> Option<()> { - let source_file = ctx.sema.parse(usage.file_range.file_id); - let call_expr: ast::CallExpr = - find_node_at_range(source_file.syntax(), usage.file_range.range)?; +) -> Option { + let call_expr: ast::CallExpr = find_node_at_range(source_file.syntax(), range)?; let call_expr_range = call_expr.expr()?.syntax().text_range(); - if !call_expr_range.contains_range(usage.file_range.range) { + if !call_expr_range.contains_range(range) { return None; } let arg = call_expr.arg_list()?.args().nth(arg_to_remove)?; - - builder.edit_file(usage.file_range.file_id); - builder.delete(range_to_remove(arg.syntax())); - - Some(()) + Some(range_to_remove(arg.syntax())) } fn range_to_remove(node: &SyntaxNode) -> TextRange { -- cgit v1.2.3 From 2c1777a2e264e58fccd5ace94b238c8a497ddbda Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 12 Jan 2021 15:51:02 +0100 Subject: Ensure uniqueness of file ids in reference search via hashmap --- .../src/handlers/extract_struct_from_enum_variant.rs | 6 +++--- crates/assists/src/handlers/inline_local_variable.rs | 14 +++++++------- crates/assists/src/handlers/remove_unused_param.rs | 18 ++++++++---------- 3 files changed, 18 insertions(+), 20 deletions(-) (limited to 'crates/assists/src') diff --git a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs index 21b13977b..e3ef04932 100644 --- a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs @@ -8,7 +8,7 @@ use ide_db::{ insert_use::{insert_use, ImportScope}, mod_path_to_ast, }, - search::{FileReference, FileReferences}, + search::FileReference, RootDatabase, }; use rustc_hash::FxHashSet; @@ -63,10 +63,10 @@ pub(crate) fn extract_struct_from_enum_variant( let current_module = enum_hir.module(ctx.db()); visited_modules_set.insert(current_module); let mut def_rewriter = None; - for FileReferences { file_id, references: refs } in usages { + for (file_id, references) in usages { let mut rewriter = SyntaxRewriter::default(); let source_file = ctx.sema.parse(file_id); - for reference in refs { + for reference in references { update_reference( ctx, &mut rewriter, diff --git a/crates/assists/src/handlers/inline_local_variable.rs b/crates/assists/src/handlers/inline_local_variable.rs index 928df6825..dc798daaa 100644 --- a/crates/assists/src/handlers/inline_local_variable.rs +++ b/crates/assists/src/handlers/inline_local_variable.rs @@ -47,8 +47,8 @@ pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext) -> O let def = ctx.sema.to_def(&bind_pat)?; let def = Definition::Local(def); - let refs = def.usages(&ctx.sema).all(); - if refs.is_empty() { + let usages = def.usages(&ctx.sema).all(); + if usages.is_empty() { mark::hit!(test_not_applicable_if_variable_unused); return None; }; @@ -66,9 +66,10 @@ pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext) -> O let_stmt.syntax().text_range() }; - let wrap_in_parens = refs - .iter() - .flat_map(|refs| &refs.references) + let wrap_in_parens = usages + .references + .values() + .flatten() .map(|&FileReference { range, .. }| { let usage_node = ctx.covering_node_for_range(range).ancestors().find_map(ast::PathExpr::cast)?; @@ -115,8 +116,7 @@ pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext) -> O target, move |builder| { builder.delete(delete_range); - for (reference, should_wrap) in - refs.iter().flat_map(|refs| &refs.references).zip(wrap_in_parens) + for (reference, should_wrap) in usages.references.values().flatten().zip(wrap_in_parens) { let replacement = if should_wrap { init_in_paren.clone() } else { init_str.clone() }; diff --git a/crates/assists/src/handlers/remove_unused_param.rs b/crates/assists/src/handlers/remove_unused_param.rs index 4f3b8ac46..c961680e2 100644 --- a/crates/assists/src/handlers/remove_unused_param.rs +++ b/crates/assists/src/handlers/remove_unused_param.rs @@ -1,7 +1,4 @@ -use ide_db::{ - defs::Definition, - search::{FileReference, FileReferences}, -}; +use ide_db::{base_db::FileId, defs::Definition, search::FileReference}; use syntax::{ algo::find_node_at_range, ast::{self, ArgListOwner}, @@ -61,8 +58,8 @@ pub(crate) fn remove_unused_param(acc: &mut Assists, ctx: &AssistContext) -> Opt param.syntax().text_range(), |builder| { builder.delete(range_to_remove(param.syntax())); - for usages in fn_def.usages(&ctx.sema).all() { - process_usages(ctx, builder, usages, param_position); + for (file_id, references) in fn_def.usages(&ctx.sema).all() { + process_usages(ctx, builder, file_id, references, param_position); } }, ) @@ -71,12 +68,13 @@ pub(crate) fn remove_unused_param(acc: &mut Assists, ctx: &AssistContext) -> Opt fn process_usages( ctx: &AssistContext, builder: &mut AssistBuilder, - usages: FileReferences, + file_id: FileId, + references: Vec, arg_to_remove: usize, ) { - let source_file = ctx.sema.parse(usages.file_id); - builder.edit_file(usages.file_id); - for usage in usages.references { + let source_file = ctx.sema.parse(file_id); + builder.edit_file(file_id); + for usage in references { if let Some(text_range) = process_usage(&source_file, usage, arg_to_remove) { builder.delete(text_range); } -- cgit v1.2.3