From 0faa2940c7615fd845b6ad9dd44902c2104477e9 Mon Sep 17 00:00:00 2001 From: Wyatt Herkamp Date: Mon, 15 Apr 2024 10:46:21 -0400 Subject: [PATCH 1/3] Use the text range for the name. Not the entire syntax in Unused Variable Diagnostic. --- .../src/handlers/unused_variables.rs | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/crates/ide-diagnostics/src/handlers/unused_variables.rs b/crates/ide-diagnostics/src/handlers/unused_variables.rs index f69209a10a916..114783d323d73 100644 --- a/crates/ide-diagnostics/src/handlers/unused_variables.rs +++ b/crates/ide-diagnostics/src/handlers/unused_variables.rs @@ -6,6 +6,7 @@ use ide_db::{ source_change::SourceChange, RootDatabase, }; +use syntax::TextRange; use text_edit::TextEdit; use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext}; @@ -23,6 +24,13 @@ pub(crate) fn unused_variables( return None; } let diagnostic_range = ctx.sema.diagnostics_display_range(ast); + // The range for the Actual Name. We don't want to replace the entire declarition. Using the diagnostic range causes issues within in Array Destructuring. + let name_range = + d.local.primary_source(ctx.sema.db).name().map(|v| v.syntax().value.text_range())?; + // Make sure we are within the diagnostic range for the variable + if !diagnostic_range.range.contains_range(name_range) { + return None; + } let var_name = d.local.name(ctx.sema.db); Some( Diagnostic::new_with_syntax_node_ptr( @@ -31,7 +39,13 @@ pub(crate) fn unused_variables( "unused variable", ast, ) - .with_fixes(fixes(ctx.sema.db, var_name, diagnostic_range, ast.file_id.is_macro())) + .with_fixes(fixes( + ctx.sema.db, + var_name, + name_range, + diagnostic_range, + ast.file_id.is_macro(), + )) .experimental(), ) } @@ -39,12 +53,14 @@ pub(crate) fn unused_variables( fn fixes( db: &RootDatabase, var_name: Name, + name_range: TextRange, diagnostic_range: FileRange, is_in_marco: bool, ) -> Option> { if is_in_marco { return None; } + Some(vec![Assist { id: AssistId("unscore_unused_variable_name", AssistKind::QuickFix), label: Label::new(format!( @@ -56,7 +72,7 @@ fn fixes( target: diagnostic_range.range, source_change: Some(SourceChange::from_text_edit( diagnostic_range.file_id, - TextEdit::replace(diagnostic_range.range, format!("_{}", var_name.display(db))), + TextEdit::replace(name_range, format!("_{}", var_name.display(db))), )), trigger_signature_help: false, }]) @@ -221,6 +237,23 @@ macro_rules! my_macro { fn main() { my_macro!(); } +"#, + ); + } + #[test] + fn unused_variable_in_array_destructure() { + check_fix( + r#" +fn main() { + let arr = [1, 2, 3, 4, 5]; + let [_x, y$0 @ ..] = arr; +} +"#, + r#" +fn main() { + let arr = [1, 2, 3, 4, 5]; + let [_x, _y @ ..] = arr; +} "#, ); } From 701068daf28d6a13d443b53170df70153ea4602e Mon Sep 17 00:00:00 2001 From: Wyatt Herkamp Date: Mon, 15 Apr 2024 14:11:45 -0400 Subject: [PATCH 2/3] Verify we are not in a macro attempt 2 --- .../src/handlers/unused_variables.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/ide-diagnostics/src/handlers/unused_variables.rs b/crates/ide-diagnostics/src/handlers/unused_variables.rs index 114783d323d73..0b22ac8ead030 100644 --- a/crates/ide-diagnostics/src/handlers/unused_variables.rs +++ b/crates/ide-diagnostics/src/handlers/unused_variables.rs @@ -25,10 +25,13 @@ pub(crate) fn unused_variables( } let diagnostic_range = ctx.sema.diagnostics_display_range(ast); // The range for the Actual Name. We don't want to replace the entire declarition. Using the diagnostic range causes issues within in Array Destructuring. - let name_range = - d.local.primary_source(ctx.sema.db).name().map(|v| v.syntax().value.text_range())?; - // Make sure we are within the diagnostic range for the variable - if !diagnostic_range.range.contains_range(name_range) { + let name_range = d + .local + .primary_source(ctx.sema.db) + .name() + .map(|v| v.syntax().original_file_range_rooted(ctx.sema.db)) + .filter(|it| Some(it.file_id) == ast.file_id.file_id())?; + if !diagnostic_range.range.contains_range(name_range.range) { return None; } let var_name = d.local.name(ctx.sema.db); @@ -42,7 +45,7 @@ pub(crate) fn unused_variables( .with_fixes(fixes( ctx.sema.db, var_name, - name_range, + name_range.range, diagnostic_range, ast.file_id.is_macro(), )) From 0eb7b6c7b5846d93a5d5e0a42b9aba6e26feef61 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 17 Apr 2024 12:05:35 +0200 Subject: [PATCH 3/3] Don't show unused_variables fixes if the name comes from a macro definition --- .../src/handlers/unused_variables.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/crates/ide-diagnostics/src/handlers/unused_variables.rs b/crates/ide-diagnostics/src/handlers/unused_variables.rs index 0b22ac8ead030..fdd4e862cafcd 100644 --- a/crates/ide-diagnostics/src/handlers/unused_variables.rs +++ b/crates/ide-diagnostics/src/handlers/unused_variables.rs @@ -30,10 +30,10 @@ pub(crate) fn unused_variables( .primary_source(ctx.sema.db) .name() .map(|v| v.syntax().original_file_range_rooted(ctx.sema.db)) - .filter(|it| Some(it.file_id) == ast.file_id.file_id())?; - if !diagnostic_range.range.contains_range(name_range.range) { - return None; - } + .filter(|it| { + Some(it.file_id) == ast.file_id.file_id() + && diagnostic_range.range.contains_range(it.range) + }); let var_name = d.local.name(ctx.sema.db); Some( Diagnostic::new_with_syntax_node_ptr( @@ -42,13 +42,9 @@ pub(crate) fn unused_variables( "unused variable", ast, ) - .with_fixes(fixes( - ctx.sema.db, - var_name, - name_range.range, - diagnostic_range, - ast.file_id.is_macro(), - )) + .with_fixes(name_range.and_then(|it| { + fixes(ctx.sema.db, var_name, it.range, diagnostic_range, ast.file_id.is_macro()) + })) .experimental(), ) }