From a4660dfea230638dea504c24ca67b49a1d67e1a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 15 Jan 2018 21:38:12 -0800 Subject: [PATCH 1/2] Point at unused arguments for format string Avoid overlapping spans by only pointing at the arguments that are not being used in the argument string. Enable libsyntax to have diagnostics with multiple primary spans by accepting `Into` instead of `Span`. --- src/libsyntax/ext/base.rs | 38 ++++++------- src/libsyntax/parse/parser.rs | 25 +++++---- src/libsyntax_ext/format.rs | 12 +---- src/test/ui/macros/format-foreign.rs | 2 +- src/test/ui/macros/format-foreign.stderr | 11 ++-- src/test/ui/macros/format-unused-lables.rs | 5 +- .../ui/macros/format-unused-lables.stderr | 53 +++++++------------ 7 files changed, 62 insertions(+), 84 deletions(-) diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 0d3be28ffefe5..612d8501fb2af 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -13,7 +13,7 @@ pub use self::SyntaxExtension::*; use ast::{self, Attribute, Name, PatKind, MetaItem}; use attr::HasAttrs; use codemap::{self, CodeMap, Spanned, respan}; -use syntax_pos::{Span, DUMMY_SP}; +use syntax_pos::{Span, MultiSpan, DUMMY_SP}; use errors::DiagnosticBuilder; use ext::expand::{self, Expansion, Invocation}; use ext::hygiene::{Mark, SyntaxContext}; @@ -754,22 +754,22 @@ impl<'a> ExtCtxt<'a> { last_macro } - pub fn struct_span_warn(&self, - sp: Span, - msg: &str) - -> DiagnosticBuilder<'a> { + pub fn struct_span_warn>(&self, + sp: S, + msg: &str) + -> DiagnosticBuilder<'a> { self.parse_sess.span_diagnostic.struct_span_warn(sp, msg) } - pub fn struct_span_err(&self, - sp: Span, - msg: &str) - -> DiagnosticBuilder<'a> { + pub fn struct_span_err>(&self, + sp: S, + msg: &str) + -> DiagnosticBuilder<'a> { self.parse_sess.span_diagnostic.struct_span_err(sp, msg) } - pub fn struct_span_fatal(&self, - sp: Span, - msg: &str) - -> DiagnosticBuilder<'a> { + pub fn struct_span_fatal>(&self, + sp: S, + msg: &str) + -> DiagnosticBuilder<'a> { self.parse_sess.span_diagnostic.struct_span_fatal(sp, msg) } @@ -785,7 +785,7 @@ impl<'a> ExtCtxt<'a> { /// in most cases one can construct a dummy expression/item to /// substitute; we never hit resolve/type-checking so the dummy /// value doesn't have to match anything) - pub fn span_fatal(&self, sp: Span, msg: &str) -> ! { + pub fn span_fatal>(&self, sp: S, msg: &str) -> ! { panic!(self.parse_sess.span_diagnostic.span_fatal(sp, msg)); } @@ -794,20 +794,20 @@ impl<'a> ExtCtxt<'a> { /// /// Compilation will be stopped in the near future (at the end of /// the macro expansion phase). - pub fn span_err(&self, sp: Span, msg: &str) { + pub fn span_err>(&self, sp: S, msg: &str) { self.parse_sess.span_diagnostic.span_err(sp, msg); } - pub fn mut_span_err(&self, sp: Span, msg: &str) + pub fn mut_span_err>(&self, sp: S, msg: &str) -> DiagnosticBuilder<'a> { self.parse_sess.span_diagnostic.mut_span_err(sp, msg) } - pub fn span_warn(&self, sp: Span, msg: &str) { + pub fn span_warn>(&self, sp: S, msg: &str) { self.parse_sess.span_diagnostic.span_warn(sp, msg); } - pub fn span_unimpl(&self, sp: Span, msg: &str) -> ! { + pub fn span_unimpl>(&self, sp: S, msg: &str) -> ! { self.parse_sess.span_diagnostic.span_unimpl(sp, msg); } - pub fn span_bug(&self, sp: Span, msg: &str) -> ! { + pub fn span_bug>(&self, sp: S, msg: &str) -> ! { self.parse_sess.span_diagnostic.span_bug(sp, msg); } pub fn trace_macros_diag(&mut self) { diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index e7565d357397c..3d58104260f9a 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -42,7 +42,7 @@ use ast::{BinOpKind, UnOp}; use ast::{RangeEnd, RangeSyntax}; use {ast, attr}; use codemap::{self, CodeMap, Spanned, respan}; -use syntax_pos::{self, Span, BytePos, FileName, DUMMY_SP}; +use syntax_pos::{self, Span, MultiSpan, BytePos, FileName, DUMMY_SP}; use errors::{self, DiagnosticBuilder}; use parse::{self, classify, token}; use parse::common::SeqSep; @@ -447,7 +447,9 @@ pub enum Error { } impl Error { - pub fn span_err(self, sp: Span, handler: &errors::Handler) -> DiagnosticBuilder { + pub fn span_err>(self, + sp: S, + handler: &errors::Handler) -> DiagnosticBuilder { match self { Error::FileNotFoundForModule { ref mod_name, ref default_path, @@ -1266,13 +1268,16 @@ impl<'a> Parser<'a> { pub fn fatal(&self, m: &str) -> DiagnosticBuilder<'a> { self.sess.span_diagnostic.struct_span_fatal(self.span, m) } - pub fn span_fatal(&self, sp: Span, m: &str) -> DiagnosticBuilder<'a> { + pub fn span_fatal>(&self, sp: S, m: &str) -> DiagnosticBuilder<'a> { self.sess.span_diagnostic.struct_span_fatal(sp, m) } - pub fn span_fatal_err(&self, sp: Span, err: Error) -> DiagnosticBuilder<'a> { + pub fn span_fatal_err>(&self, sp: S, err: Error) -> DiagnosticBuilder<'a> { err.span_err(sp, self.diagnostic()) } - pub fn span_fatal_help(&self, sp: Span, m: &str, help: &str) -> DiagnosticBuilder<'a> { + pub fn span_fatal_help>(&self, + sp: S, + m: &str, + help: &str) -> DiagnosticBuilder<'a> { let mut err = self.sess.span_diagnostic.struct_span_fatal(sp, m); err.help(help); err @@ -1283,21 +1288,21 @@ impl<'a> Parser<'a> { pub fn warn(&self, m: &str) { self.sess.span_diagnostic.span_warn(self.span, m) } - pub fn span_warn(&self, sp: Span, m: &str) { + pub fn span_warn>(&self, sp: S, m: &str) { self.sess.span_diagnostic.span_warn(sp, m) } - pub fn span_err(&self, sp: Span, m: &str) { + pub fn span_err>(&self, sp: S, m: &str) { self.sess.span_diagnostic.span_err(sp, m) } - pub fn struct_span_err(&self, sp: Span, m: &str) -> DiagnosticBuilder<'a> { + pub fn struct_span_err>(&self, sp: S, m: &str) -> DiagnosticBuilder<'a> { self.sess.span_diagnostic.struct_span_err(sp, m) } - pub fn span_err_help(&self, sp: Span, m: &str, h: &str) { + pub fn span_err_help>(&self, sp: S, m: &str, h: &str) { let mut err = self.sess.span_diagnostic.mut_span_err(sp, m); err.help(h); err.emit(); } - pub fn span_bug(&self, sp: Span, m: &str) -> ! { + pub fn span_bug>(&self, sp: S, m: &str) -> ! { self.sess.span_diagnostic.span_bug(sp, m) } pub fn abort_if_errors(&self) { diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index ad5bd39a45341..29280d203c79f 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -814,16 +814,8 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt, let (sp, msg) = errs.into_iter().next().unwrap(); cx.ecx.struct_span_err(sp, msg) } else { - let mut diag = cx.ecx.struct_span_err(cx.fmtsp, - "multiple unused formatting arguments"); - - // Ignoring message, as it gets repetitive - // Then use MultiSpan to not clutter up errors - for (sp, _) in errs { - diag.span_label(sp, "unused"); - } - - diag + cx.ecx.struct_span_err(errs.iter().map(|&(sp, _)| sp).collect::>(), + "multiple unused formatting arguments") } }; diff --git a/src/test/ui/macros/format-foreign.rs b/src/test/ui/macros/format-foreign.rs index 91ca8f5ff7602..ec0eaed43aea6 100644 --- a/src/test/ui/macros/format-foreign.rs +++ b/src/test/ui/macros/format-foreign.rs @@ -9,7 +9,7 @@ // except according to those terms. fn main() { - println!("%.*3$s %s!\n", "Hello,", "World", 4); + println!("%.*3$s %s!\n", "Hello,", "World", 4); //~ ERROR multiple unused formatting arguments println!("%1$*2$.*3$f", 123.456); //~ ERROR never used // This should *not* produce hints, on the basis that there's equally as diff --git a/src/test/ui/macros/format-foreign.stderr b/src/test/ui/macros/format-foreign.stderr index d0229957b682e..31804ca4c40d1 100644 --- a/src/test/ui/macros/format-foreign.stderr +++ b/src/test/ui/macros/format-foreign.stderr @@ -1,17 +1,12 @@ error: multiple unused formatting arguments - --> $DIR/format-foreign.rs:12:5 + --> $DIR/format-foreign.rs:12:30 | -12 | println!("%.*3$s %s!/n", "Hello,", "World", 4); - | ^^^^^^^^^^^^^^^^^^^^^^^^^--------^^-------^^-^^ - | | | | - | | | unused - | | unused - | unused +12 | println!("%.*3$s %s!/n", "Hello,", "World", 4); //~ ERROR multiple unused formatting arguments + | ^^^^^^^^ ^^^^^^^ ^ | = help: `%.*3$s` should be written as `{:.2$}` = help: `%s` should be written as `{}` = note: printf formatting not supported; see the documentation for `std::fmt` - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: argument never used --> $DIR/format-foreign.rs:13:29 diff --git a/src/test/ui/macros/format-unused-lables.rs b/src/test/ui/macros/format-unused-lables.rs index 7a32d932ba386..3347a1215cede 100644 --- a/src/test/ui/macros/format-unused-lables.rs +++ b/src/test/ui/macros/format-unused-lables.rs @@ -10,9 +10,10 @@ fn main() { println!("Test", 123, 456, 789); + //~^ ERROR multiple unused formatting arguments println!("Test2", - 123, + 123, //~ ERROR multiple unused formatting arguments 456, 789 ); @@ -20,7 +21,7 @@ fn main() { println!("Some stuff", UNUSED="args"); //~ ERROR named argument never used println!("Some more $STUFF", - "woo!", + "woo!", //~ ERROR multiple unused formatting arguments STUFF= "things" , UNUSED="args"); diff --git a/src/test/ui/macros/format-unused-lables.stderr b/src/test/ui/macros/format-unused-lables.stderr index 9efdca12dea03..59aed9d8bc6fb 100644 --- a/src/test/ui/macros/format-unused-lables.stderr +++ b/src/test/ui/macros/format-unused-lables.stderr @@ -1,53 +1,38 @@ error: multiple unused formatting arguments - --> $DIR/format-unused-lables.rs:12:5 + --> $DIR/format-unused-lables.rs:12:22 | 12 | println!("Test", 123, 456, 789); - | ^^^^^^^^^^^^^^^^^---^^---^^---^^ - | | | | - | | | unused - | | unused - | unused - | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + | ^^^ ^^^ ^^^ error: multiple unused formatting arguments - --> $DIR/format-unused-lables.rs:14:5 - | -14 | / println!("Test2", -15 | | 123, - | | --- unused -16 | | 456, - | | --- unused -17 | | 789 - | | --- unused -18 | | ); - | |______^ + --> $DIR/format-unused-lables.rs:16:9 | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) +16 | 123, //~ ERROR multiple unused formatting arguments + | ^^^ +17 | 456, + | ^^^ +18 | 789 + | ^^^ error: named argument never used - --> $DIR/format-unused-lables.rs:20:35 + --> $DIR/format-unused-lables.rs:21:35 | -20 | println!("Some stuff", UNUSED="args"); //~ ERROR named argument never used +21 | println!("Some stuff", UNUSED="args"); //~ ERROR named argument never used | ^^^^^^ error: multiple unused formatting arguments - --> $DIR/format-unused-lables.rs:22:5 + --> $DIR/format-unused-lables.rs:24:9 | -22 | / println!("Some more $STUFF", -23 | | "woo!", - | | ------ unused -24 | | STUFF= -25 | | "things" - | | -------- unused -26 | | , UNUSED="args"); - | |_______________________------_^ - | | - | unused +24 | "woo!", //~ ERROR multiple unused formatting arguments + | ^^^^^^ +25 | STUFF= +26 | "things" + | ^^^^^^^^ +27 | , UNUSED="args"); + | ^^^^^^ | = help: `$STUFF` should be written as `{STUFF}` = note: shell formatting not supported; see the documentation for `std::fmt` - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to 4 previous errors From eb3da09333870e94b122d863402d993fb7ecd78f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 16 Jan 2018 18:11:35 -0800 Subject: [PATCH 2/2] Add secondary span pointing at the statement (error span) --- src/libsyntax_ext/format.rs | 8 +++- src/test/ui/macros/format-foreign.stderr | 3 +- .../ui/macros/format-unused-lables.stderr | 37 ++++++++++++------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index 29280d203c79f..a7822414c6959 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -814,8 +814,12 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt, let (sp, msg) = errs.into_iter().next().unwrap(); cx.ecx.struct_span_err(sp, msg) } else { - cx.ecx.struct_span_err(errs.iter().map(|&(sp, _)| sp).collect::>(), - "multiple unused formatting arguments") + let mut diag = cx.ecx.struct_span_err( + errs.iter().map(|&(sp, _)| sp).collect::>(), + "multiple unused formatting arguments" + ); + diag.span_label(cx.fmtsp, "multiple unused arguments in this statement"); + diag } }; diff --git a/src/test/ui/macros/format-foreign.stderr b/src/test/ui/macros/format-foreign.stderr index 31804ca4c40d1..f9852c5477332 100644 --- a/src/test/ui/macros/format-foreign.stderr +++ b/src/test/ui/macros/format-foreign.stderr @@ -2,11 +2,12 @@ error: multiple unused formatting arguments --> $DIR/format-foreign.rs:12:30 | 12 | println!("%.*3$s %s!/n", "Hello,", "World", 4); //~ ERROR multiple unused formatting arguments - | ^^^^^^^^ ^^^^^^^ ^ + | -------------------------^^^^^^^^--^^^^^^^--^-- multiple unused arguments in this statement | = help: `%.*3$s` should be written as `{:.2$}` = help: `%s` should be written as `{}` = note: printf formatting not supported; see the documentation for `std::fmt` + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: argument never used --> $DIR/format-foreign.rs:13:29 diff --git a/src/test/ui/macros/format-unused-lables.stderr b/src/test/ui/macros/format-unused-lables.stderr index 59aed9d8bc6fb..64ea5626c1d62 100644 --- a/src/test/ui/macros/format-unused-lables.stderr +++ b/src/test/ui/macros/format-unused-lables.stderr @@ -2,17 +2,24 @@ error: multiple unused formatting arguments --> $DIR/format-unused-lables.rs:12:22 | 12 | println!("Test", 123, 456, 789); - | ^^^ ^^^ ^^^ + | -----------------^^^--^^^--^^^-- multiple unused arguments in this statement + | + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: multiple unused formatting arguments --> $DIR/format-unused-lables.rs:16:9 | -16 | 123, //~ ERROR multiple unused formatting arguments - | ^^^ -17 | 456, - | ^^^ -18 | 789 - | ^^^ +15 | / println!("Test2", +16 | | 123, //~ ERROR multiple unused formatting arguments + | | ^^^ +17 | | 456, + | | ^^^ +18 | | 789 + | | ^^^ +19 | | ); + | |______- multiple unused arguments in this statement + | + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: named argument never used --> $DIR/format-unused-lables.rs:21:35 @@ -23,16 +30,18 @@ error: named argument never used error: multiple unused formatting arguments --> $DIR/format-unused-lables.rs:24:9 | -24 | "woo!", //~ ERROR multiple unused formatting arguments - | ^^^^^^ -25 | STUFF= -26 | "things" - | ^^^^^^^^ -27 | , UNUSED="args"); - | ^^^^^^ +23 | / println!("Some more $STUFF", +24 | | "woo!", //~ ERROR multiple unused formatting arguments + | | ^^^^^^ +25 | | STUFF= +26 | | "things" + | | ^^^^^^^^ +27 | | , UNUSED="args"); + | |_______________________^^^^^^_- multiple unused arguments in this statement | = help: `$STUFF` should be written as `{STUFF}` = note: shell formatting not supported; see the documentation for `std::fmt` + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: aborting due to 4 previous errors