From 37c2bb8c947468bff1b6a9162843e57bc19cb3ad Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Sat, 6 Feb 2016 00:47:20 +0100 Subject: [PATCH 1/2] Use MultiSpan for lints --- src/librustc/lint/context.rs | 37 ++++++++++++++++++++---------------- src/librustc/session/mod.rs | 10 +++++----- src/libsyntax/codemap.rs | 8 ++++++++ 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 55782041be687..a5d41475d5890 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -41,7 +41,7 @@ use std::default::Default as StdDefault; use std::mem; use syntax::ast_util::{self, IdVisitingOperation}; use syntax::attr::{self, AttrMetaMethods}; -use syntax::codemap::Span; +use syntax::codemap::{Span, MultiSpan}; use syntax::errors::DiagnosticBuilder; use syntax::parse::token::InternedString; use syntax::ast; @@ -398,20 +398,20 @@ pub fn gather_attr(attr: &ast::Attribute) /// in trans that run after the main lint pass is finished. Most /// lints elsewhere in the compiler should call /// `Session::add_lint()` instead. -pub fn raw_emit_lint(sess: &Session, +pub fn raw_emit_lint>(sess: &Session, lints: &LintStore, lint: &'static Lint, lvlsrc: LevelSource, - span: Option, + span: Option, msg: &str) { raw_struct_lint(sess, lints, lint, lvlsrc, span, msg).emit(); } -pub fn raw_struct_lint<'a>(sess: &'a Session, +pub fn raw_struct_lint<'a, S: Into>(sess: &'a Session, lints: &LintStore, lint: &'static Lint, lvlsrc: LevelSource, - span: Option, + span: Option, msg: &str) -> DiagnosticBuilder<'a> { let (mut level, source) = lvlsrc; @@ -442,10 +442,15 @@ pub fn raw_struct_lint<'a>(sess: &'a Session, // For purposes of printing, we can treat forbid as deny. if level == Forbid { level = Deny; } - let mut err = match (level, span) { - (Warn, Some(sp)) => sess.struct_span_warn(sp, &msg[..]), + let span = span.map(|s| s.into()); + let mut err = match (level, span.clone()) { + (Warn, Some(sp)) => { + sess.struct_span_warn(sp, &msg[..]) + }, (Warn, None) => sess.struct_warn(&msg[..]), - (Deny, Some(sp)) => sess.struct_span_err(sp, &msg[..]), + (Deny, Some(sp)) => { + sess.struct_span_err(sp, &msg[..]) + }, (Deny, None) => sess.struct_err(&msg[..]), _ => sess.bug("impossible level in raw_emit_lint"), }; @@ -458,7 +463,7 @@ pub fn raw_struct_lint<'a>(sess: &'a Session, let citation = format!("for more information, see {}", future_incompatible.reference); if let Some(sp) = span { - err.fileline_warn(sp, &explanation); + err.fileline_warn(sp.clone(), &explanation); err.fileline_note(sp, &citation); } else { err.warn(&explanation); @@ -497,7 +502,7 @@ pub trait LintContext: Sized { }) } - fn lookup_and_emit(&self, lint: &'static Lint, span: Option, msg: &str) { + fn lookup_and_emit(&self, lint: &'static Lint, span: Option, msg: &str) { let (level, src) = match self.level_src(lint) { None => return, Some(pair) => pair, @@ -520,8 +525,8 @@ pub trait LintContext: Sized { } /// Emit a lint at the appropriate level, for a particular span. - fn span_lint(&self, lint: &'static Lint, span: Span, msg: &str) { - self.lookup_and_emit(lint, Some(span), msg); + fn span_lint>(&self, lint: &'static Lint, span: S, msg: &str) { + self.lookup_and_emit(lint, Some(span.into()), msg); } fn struct_span_lint(&self, @@ -1258,8 +1263,8 @@ pub fn check_crate(tcx: &ty::ctxt, access_levels: &AccessLevels) { // If we missed any lints added to the session, then there's a bug somewhere // in the iteration code. for (id, v) in tcx.sess.lints.borrow().iter() { - for &(lint, span, ref msg) in v { - tcx.sess.span_bug(span, + for &(lint, ref span, ref msg) in v { + tcx.sess.span_bug(span.clone(), &format!("unprocessed lint {} at {}: {}", lint.as_str(), tcx.map.node_to_string(*id), *msg)) } @@ -1292,8 +1297,8 @@ pub fn check_ast_crate(sess: &Session, krate: &ast::Crate) { // If we missed any lints added to the session, then there's a bug somewhere // in the iteration code. for (_, v) in sess.lints.borrow().iter() { - for &(lint, span, ref msg) in v { - sess.span_bug(span, + for &(lint, ref span, ref msg) in v { + sess.span_bug(span.clone(), &format!("unprocessed lint {}: {}", lint.as_str(), *msg)) } diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index f0c7182594240..c3d3f18e2a782 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -57,7 +57,7 @@ pub struct Session { pub local_crate_source_file: Option, pub working_dir: PathBuf, pub lint_store: RefCell, - pub lints: RefCell>>, + pub lints: RefCell>>, pub plugin_llvm_passes: RefCell>, pub plugin_attributes: RefCell>, pub crate_types: RefCell>, @@ -236,18 +236,18 @@ impl Session { pub fn unimpl(&self, msg: &str) -> ! { self.diagnostic().unimpl(msg) } - pub fn add_lint(&self, + pub fn add_lint>(&self, lint: &'static lint::Lint, id: ast::NodeId, - sp: Span, + sp: S, msg: String) { let lint_id = lint::LintId::of(lint); let mut lints = self.lints.borrow_mut(); match lints.get_mut(&id) { - Some(arr) => { arr.push((lint_id, sp, msg)); return; } + Some(arr) => { arr.push((lint_id, sp.into(), msg)); return; } None => {} } - lints.insert(id, vec!((lint_id, sp, msg))); + lints.insert(id, vec!((lint_id, sp.into(), msg))); } pub fn reserve_node_ids(&self, count: ast::NodeId) -> ast::NodeId { let id = self.next_node_id.get(); diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index 74302e2f6a0fd..dfc7f4ca868ea 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -364,6 +364,14 @@ impl MultiSpan { } } +impl fmt::Debug for MultiSpan { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, + "MultiSpan ({})", + self.spans.iter().map(|s| format!("{:?}", s)).collect::>().join(", ")) + } +} + impl From for MultiSpan { fn from(span: Span) -> MultiSpan { MultiSpan { spans: vec![span] } From 2d32c145a2799f668133ab3ce4ef55648ab3e113 Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Sat, 6 Feb 2016 00:48:00 +0100 Subject: [PATCH 2/2] Emit unused import lints in a multispan, closes #16132 --- src/librustc_resolve/check_unused.rs | 38 ++++++++++++++++++---------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/src/librustc_resolve/check_unused.rs b/src/librustc_resolve/check_unused.rs index 178e2a4d1bc78..2ae4e8af33163 100644 --- a/src/librustc_resolve/check_unused.rs +++ b/src/librustc_resolve/check_unused.rs @@ -18,6 +18,7 @@ // use std::ops::{Deref, DerefMut}; +use std::collections::HashMap; use Resolver; use Namespace::{TypeNS, ValueNS}; @@ -25,7 +26,7 @@ use Namespace::{TypeNS, ValueNS}; use rustc::lint; use rustc::middle::privacy::{DependsOn, LastImport, Used, Unused}; use syntax::ast; -use syntax::codemap::{Span, DUMMY_SP}; +use syntax::codemap::{Span, MultiSpan, DUMMY_SP}; use rustc_front::hir; use rustc_front::hir::{ViewPathGlob, ViewPathList, ViewPathSimple}; @@ -33,6 +34,7 @@ use rustc_front::intravisit::Visitor; struct UnusedImportCheckVisitor<'a, 'b: 'a, 'tcx: 'b> { resolver: &'a mut Resolver<'b, 'tcx>, + unused_imports: HashMap>, } // Deref and DerefMut impls allow treating UnusedImportCheckVisitor as Resolver. @@ -58,16 +60,13 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> { // only check imports and namespaces which are used. In particular, this // means that if an import could name either a public or private item, we // will check the correct thing, dependent on how the import is used. - fn finalize_import(&mut self, id: ast::NodeId, span: Span) { + fn finalize_import(&mut self, item_id: ast::NodeId, id: ast::NodeId, span: Span) { debug!("finalizing import uses for {:?}", self.session.codemap().span_to_snippet(span)); if !self.used_imports.contains(&(id, TypeNS)) && !self.used_imports.contains(&(id, ValueNS)) { - self.session.add_lint(lint::builtin::UNUSED_IMPORTS, - id, - span, - "unused import".to_string()); + self.unused_imports.entry(item_id).or_insert_with(Vec::new).push(span); } let mut def_map = self.def_map.borrow_mut(); @@ -135,22 +134,19 @@ impl<'a, 'b, 'v, 'tcx> Visitor<'v> for UnusedImportCheckVisitor<'a, 'b, 'tcx> { hir::ItemUse(ref p) => { match p.node { ViewPathSimple(_, _) => { - self.finalize_import(item.id, p.span) + self.finalize_import(item.id, item.id, p.span) } ViewPathList(_, ref list) => { for i in list { - self.finalize_import(i.node.id(), i.span); + self.finalize_import(item.id, i.node.id(), i.span); } } ViewPathGlob(_) => { if !self.used_imports.contains(&(item.id, TypeNS)) && !self.used_imports.contains(&(item.id, ValueNS)) { - self.session - .add_lint(lint::builtin::UNUSED_IMPORTS, - item.id, - p.span, - "unused import".to_string()); + self.unused_imports.entry(item.id).or_insert_with(Vec::new) + .push(item.span); } } } @@ -161,6 +157,20 @@ impl<'a, 'b, 'v, 'tcx> Visitor<'v> for UnusedImportCheckVisitor<'a, 'b, 'tcx> { } pub fn check_crate(resolver: &mut Resolver, krate: &hir::Crate) { - let mut visitor = UnusedImportCheckVisitor { resolver: resolver }; + let mut visitor = UnusedImportCheckVisitor { resolver: resolver, + unused_imports: HashMap::new() }; krate.visit_all_items(&mut visitor); + + for (id, spans) in &visitor.unused_imports { + let ms = spans.iter().fold(MultiSpan::new(), |mut acc, &sp| { acc.push_merge(sp); acc }); + + visitor.session.add_lint(lint::builtin::UNUSED_IMPORTS, + *id, + ms, + if spans.len() > 1 { + "unused imports".to_string() + } else { + "unused import".to_string() + }); + } }