From b1b684ef2398b073b49498e34c6c6af583ce2a94 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Sat, 29 Sep 2018 21:25:41 -0700 Subject: [PATCH 1/2] structured suggestions for unused-lifetimes lint --- src/librustc/middle/resolve_lifetime.rs | 57 +++++++++++++++++-- .../single-use-lifetime/zero-uses-in-fn.fixed | 24 ++++++++ .../ui/single-use-lifetime/zero-uses-in-fn.rs | 31 +++++----- .../zero-uses-in-fn.stderr | 26 +++++++-- .../single-use-lifetime/zero-uses-in-impl.rs | 19 ++----- .../zero-uses-in-impl.stderr | 8 +-- 6 files changed, 124 insertions(+), 41 deletions(-) create mode 100644 src/test/ui/single-use-lifetime/zero-uses-in-fn.fixed diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index a10b387672ad6..53bfee4ae24bb 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -19,9 +19,9 @@ use hir::def::Def; use hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE}; use hir::map::Map; use hir::{GenericArg, GenericParam, ItemLocalId, LifetimeName, ParamName, Node}; -use ty::{self, TyCtxt, GenericParamDefKind}; +use ty::{self, TyCtxt, DefIdTree, GenericParamDefKind}; -use errors::DiagnosticBuilder; +use errors::{Applicability, DiagnosticBuilder}; use rustc::lint; use rustc_data_structures::sync::Lrc; use session::Session; @@ -1468,12 +1468,61 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { _ => None, } { debug!("id ={:?} span = {:?} name = {:?}", node_id, span, name); - self.tcx.struct_span_lint_node( + let mut err = self.tcx.struct_span_lint_node( lint::builtin::UNUSED_LIFETIMES, id, span, &format!("lifetime parameter `{}` never used", name) - ).emit(); + ); + if let Some(parent_def_id) = self.tcx.parent(def_id) { + if let Some(node_id) = self.tcx.hir.as_local_node_id(parent_def_id) { + if let Some(Node::Item(hir_item)) = self.tcx.hir.find(node_id) { + match hir_item.node { + hir::ItemKind::Fn(_, _, ref generics, _) | + hir::ItemKind::Impl(_, _, _, ref generics, _, _, _) => { + let unused_lt_span = if generics.params.len() == 1 { + // if sole lifetime, remove the `<>` brackets + Some(generics.span) + } else { + generics.params.iter().enumerate() + .find_map(|(i, param)| { + if param.name.ident() == name { + // We also want to delete a leading or + // trailing comma as appropriate + if i >= generics.params.len() - 1 { + Some( + generics.params[i-1] + .span.shrink_to_hi() + .to(param.span) + ) + } else { + Some( + param.span.to( + generics.params[i+1] + .span.shrink_to_lo() + ) + ) + } + } else { + None + } + }) + }; + if let Some(span) = unused_lt_span { + err.span_suggestion_with_applicability( + span, + "remove it", + String::new(), + Applicability::MachineApplicable + ); + } + }, + _ => {} + } + } + } + } + err.emit(); } } } diff --git a/src/test/ui/single-use-lifetime/zero-uses-in-fn.fixed b/src/test/ui/single-use-lifetime/zero-uses-in-fn.fixed new file mode 100644 index 0000000000000..5ba7df8a1e61f --- /dev/null +++ b/src/test/ui/single-use-lifetime/zero-uses-in-fn.fixed @@ -0,0 +1,24 @@ +// run-rustfix + +// Test that we DO warn when lifetime name is not used at all. + +#![deny(unused_lifetimes)] +#![allow(dead_code, unused_variables)] + +fn september() {} +//~^ ERROR lifetime parameter `'a` never used +//~| HELP remove it + +fn october<'b, T>(s: &'b T) -> &'b T { + //~^ ERROR lifetime parameter `'a` never used + //~| HELP remove it + s +} + +fn november<'a>(s: &'a str) -> (&'a str) { + //~^ ERROR lifetime parameter `'b` never used + //~| HELP remove it + s +} + +fn main() {} diff --git a/src/test/ui/single-use-lifetime/zero-uses-in-fn.rs b/src/test/ui/single-use-lifetime/zero-uses-in-fn.rs index 7152d122f79a3..a56d7fa8abc02 100644 --- a/src/test/ui/single-use-lifetime/zero-uses-in-fn.rs +++ b/src/test/ui/single-use-lifetime/zero-uses-in-fn.rs @@ -1,19 +1,24 @@ -// Copyright 2016 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. +// run-rustfix // Test that we DO warn when lifetime name is not used at all. #![deny(unused_lifetimes)] -#![allow(dead_code)] -#![allow(unused_variables)] +#![allow(dead_code, unused_variables)] -fn d<'a>() { } //~ ERROR `'a` never used +fn september<'a>() {} +//~^ ERROR lifetime parameter `'a` never used +//~| HELP remove it -fn main() { } +fn october<'a, 'b, T>(s: &'b T) -> &'b T { + //~^ ERROR lifetime parameter `'a` never used + //~| HELP remove it + s +} + +fn november<'a, 'b>(s: &'a str) -> (&'a str) { + //~^ ERROR lifetime parameter `'b` never used + //~| HELP remove it + s +} + +fn main() {} diff --git a/src/test/ui/single-use-lifetime/zero-uses-in-fn.stderr b/src/test/ui/single-use-lifetime/zero-uses-in-fn.stderr index 322351a0a8e5b..566c841cfa969 100644 --- a/src/test/ui/single-use-lifetime/zero-uses-in-fn.stderr +++ b/src/test/ui/single-use-lifetime/zero-uses-in-fn.stderr @@ -1,14 +1,30 @@ error: lifetime parameter `'a` never used - --> $DIR/zero-uses-in-fn.rs:17:6 + --> $DIR/zero-uses-in-fn.rs:8:14 | -LL | fn d<'a>() { } //~ ERROR `'a` never used - | ^^ +LL | fn september<'a>() {} + | -^^- help: remove it | note: lint level defined here - --> $DIR/zero-uses-in-fn.rs:13:9 + --> $DIR/zero-uses-in-fn.rs:5:9 | LL | #![deny(unused_lifetimes)] | ^^^^^^^^^^^^^^^^ -error: aborting due to previous error +error: lifetime parameter `'a` never used + --> $DIR/zero-uses-in-fn.rs:12:12 + | +LL | fn october<'a, 'b, T>(s: &'b T) -> &'b T { + | ^^-- + | | + | help: remove it + +error: lifetime parameter `'b` never used + --> $DIR/zero-uses-in-fn.rs:18:17 + | +LL | fn november<'a, 'b>(s: &'a str) -> (&'a str) { + | --^^ + | | + | help: remove it + +error: aborting due to 3 previous errors diff --git a/src/test/ui/single-use-lifetime/zero-uses-in-impl.rs b/src/test/ui/single-use-lifetime/zero-uses-in-impl.rs index a231c0bf00331..54803e1d2be42 100644 --- a/src/test/ui/single-use-lifetime/zero-uses-in-impl.rs +++ b/src/test/ui/single-use-lifetime/zero-uses-in-impl.rs @@ -1,21 +1,10 @@ -// Copyright 2016 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - // Test that we DO warn when lifetime name is not used at all. #![deny(unused_lifetimes)] -#![allow(dead_code)] -#![allow(unused_variables)] +#![allow(dead_code, unused_variables)] -struct Foo { } +struct Foo {} -impl<'a> Foo { } //~ ERROR `'a` never used +impl<'a> Foo {} //~ ERROR `'a` never used -fn main() { } +fn main() {} diff --git a/src/test/ui/single-use-lifetime/zero-uses-in-impl.stderr b/src/test/ui/single-use-lifetime/zero-uses-in-impl.stderr index 34cb15c1339d8..1b77ebdec99c6 100644 --- a/src/test/ui/single-use-lifetime/zero-uses-in-impl.stderr +++ b/src/test/ui/single-use-lifetime/zero-uses-in-impl.stderr @@ -1,11 +1,11 @@ error: lifetime parameter `'a` never used - --> $DIR/zero-uses-in-impl.rs:19:6 + --> $DIR/zero-uses-in-impl.rs:8:6 | -LL | impl<'a> Foo { } //~ ERROR `'a` never used - | ^^ +LL | impl<'a> Foo {} //~ ERROR `'a` never used + | -^^- help: remove it | note: lint level defined here - --> $DIR/zero-uses-in-impl.rs:13:9 + --> $DIR/zero-uses-in-impl.rs:3:9 | LL | #![deny(unused_lifetimes)] | ^^^^^^^^^^^^^^^^ From efd7a311506d3a4291dc8f9ed21bda43e39686ae Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Sun, 7 Oct 2018 19:28:37 -0700 Subject: [PATCH 2/2] in which rightward drift is opposed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thanks to reviewers Tyler Mandry (for pointing out that this is ridiculous and we need a helper function), Niko Matsakis (for pointing out that the span-calculation code only has a couple free variables), and Esteban Küber (for pointing out `get_generics`). --- src/librustc/middle/resolve_lifetime.rs | 77 +++++++++++-------------- 1 file changed, 33 insertions(+), 44 deletions(-) diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index 53bfee4ae24bb..2f3fdb7966f25 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -1398,6 +1398,30 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { self.xcrate_object_lifetime_defaults = this.xcrate_object_lifetime_defaults; } + /// helper method to determine the span to remove when suggesting the + /// deletion of a lifetime + fn lifetime_deletion_span(&self, name: ast::Ident, generics: &hir::Generics) -> Option { + if generics.params.len() == 1 { + // if sole lifetime, remove the `<>` brackets + Some(generics.span) + } else { + generics.params.iter().enumerate() + .find_map(|(i, param)| { + if param.name.ident() == name { + // We also want to delete a leading or trailing comma + // as appropriate + if i >= generics.params.len() - 1 { + Some(generics.params[i-1].span.shrink_to_hi().to(param.span)) + } else { + Some(param.span.to(generics.params[i+1].span.shrink_to_lo())) + } + } else { + None + } + }) + } + } + fn check_uses_for_lifetimes_defined_by_scope(&mut self) { let defined_by = match self.scope { Scope::Binder { lifetimes, .. } => lifetimes, @@ -1475,50 +1499,15 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { &format!("lifetime parameter `{}` never used", name) ); if let Some(parent_def_id) = self.tcx.parent(def_id) { - if let Some(node_id) = self.tcx.hir.as_local_node_id(parent_def_id) { - if let Some(Node::Item(hir_item)) = self.tcx.hir.find(node_id) { - match hir_item.node { - hir::ItemKind::Fn(_, _, ref generics, _) | - hir::ItemKind::Impl(_, _, _, ref generics, _, _, _) => { - let unused_lt_span = if generics.params.len() == 1 { - // if sole lifetime, remove the `<>` brackets - Some(generics.span) - } else { - generics.params.iter().enumerate() - .find_map(|(i, param)| { - if param.name.ident() == name { - // We also want to delete a leading or - // trailing comma as appropriate - if i >= generics.params.len() - 1 { - Some( - generics.params[i-1] - .span.shrink_to_hi() - .to(param.span) - ) - } else { - Some( - param.span.to( - generics.params[i+1] - .span.shrink_to_lo() - ) - ) - } - } else { - None - } - }) - }; - if let Some(span) = unused_lt_span { - err.span_suggestion_with_applicability( - span, - "remove it", - String::new(), - Applicability::MachineApplicable - ); - } - }, - _ => {} - } + if let Some(generics) = self.tcx.hir.get_generics(parent_def_id) { + let unused_lt_span = self.lifetime_deletion_span(name, generics); + if let Some(span) = unused_lt_span { + err.span_suggestion_with_applicability( + span, + "remove it", + String::new(), + Applicability::MachineApplicable + ); } } }