Rollup merge of rust-lang#47481 - estebank:unused-args, r=arielb1

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<MultiSpan>` instead of `Span`. Partially addresses rust-lang#41850.
kennytm · Jan 17, 2018 · 35bf7f8 · 35bf7f8
2 parents b2c5484 + eb3da09
commit 35bf7f8
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 77 deletions.
diff --git 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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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
@@ -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<S: Into<MultiSpan>>(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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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<S: Into<MultiSpan>>(&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
@@ -814,15 +814,11 @@ 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");
-                }
-
+                let mut diag = cx.ecx.struct_span_err(
+                    errs.iter().map(|&(sp, _)| sp).collect::<Vec<Span>>(),
+                    "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.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
@@ -1,12 +1,8 @@
 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
+   |     -------------------------^^^^^^^^--^^^^^^^--^-- multiple unused arguments in this statement
    |
    = help: `%.*3$s` should be written as `{:.2$}`
    = help: `%s` should be written as `{}`

diff --git a/src/test/ui/macros/format-unused-lables.rs b/src/test/ui/macros/format-unused-lables.rs
@@ -10,17 +10,18 @@
 
 fn main() {
     println!("Test", 123, 456, 789);
+    //~^ ERROR multiple unused formatting arguments
 
     println!("Test2",
-        123,
+        123,  //~ ERROR multiple unused formatting arguments
         456,
         789
     );
 
     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
@@ -1,49 +1,43 @@
 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
+   |     -----------------^^^--^^^--^^^-- 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:14:5
+  --> $DIR/format-unused-lables.rs:16:9
    |
-14 | /     println!("Test2",
-15 | |         123,
-   | |         --- unused
-16 | |         456,
-   | |         --- unused
-17 | |         789
-   | |         --- unused
-18 | |     );
-   | |______^
+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: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
+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`