Always include a position span in rustc_parse_format::Argument

matthiaskrgr · Jul 31, 2022 · 2a0b51d · 2a0b51d
1 parent 482153b
commit 2a0b51d
Show file tree

Hide file tree

Showing 6 changed files with 96 additions and 41 deletions.
diff --git a/compiler/rustc_builtin_macros/src/asm.rs b/compiler/rustc_builtin_macros/src/asm.rs
@@ -656,7 +656,7 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, args: AsmArgs) -> Option<ast::Inl
                     let span = arg_spans.next().unwrap_or(template_sp);
 
                     let operand_idx = match arg.position {
-                        parse::ArgumentIs(idx, _) | parse::ArgumentImplicitlyIs(idx) => {
+                        parse::ArgumentIs(idx) | parse::ArgumentImplicitlyIs(idx) => {
                             if idx >= args.operands.len()
                                 || named_pos.contains_key(&idx)
                                 || args.reg_args.contains(&idx)
@@ -702,11 +702,12 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, args: AsmArgs) -> Option<ast::Inl
                                 Some(idx)
                             }
                         }
-                        parse::ArgumentNamed(name, span) => {
+                        parse::ArgumentNamed(name) => {
                             match args.named_args.get(&Symbol::intern(name)) {
                                 Some(&idx) => Some(idx),
                                 None => {
                                     let msg = format!("there is no argument named `{}`", name);
+                                    let span = arg.position_span;
                                     ecx.struct_span_err(
                                         template_span
                                             .from_inner(InnerSpan::new(span.start, span.end)),

diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs
@@ -381,8 +381,8 @@ impl<'a, 'b> Context<'a, 'b> {
         match *p {
             parse::String(_) => {}
             parse::NextArgument(ref mut arg) => {
-                if let parse::ArgumentNamed(s, _) = arg.position {
-                    arg.position = parse::ArgumentIs(lookup(s), None);
+                if let parse::ArgumentNamed(s) = arg.position {
+                    arg.position = parse::ArgumentIs(lookup(s));
                 }
                 if let parse::CountIsName(s, _) = arg.format.width {
                     arg.format.width = parse::CountIsParam(lookup(s));
@@ -417,14 +417,14 @@ impl<'a, 'b> Context<'a, 'b> {
                 // argument second, if it's an implicit positional parameter
                 // it's written second, so it should come after width/precision.
                 let pos = match arg.position {
-                    parse::ArgumentIs(i, arg_end) => {
+                    parse::ArgumentIs(i) => {
                         self.unused_names_lint.maybe_add_positional_named_arg(
                             i,
                             self.args.len(),
                             i,
                             PositionalNamedArgType::Arg,
                             self.curpiece,
-                            arg_end,
+                            Some(arg.position_span),
                             &self.names,
                         );
 
@@ -442,8 +442,9 @@ impl<'a, 'b> Context<'a, 'b> {
                         );
                         Exact(i)
                     }
-                    parse::ArgumentNamed(s, span) => {
+                    parse::ArgumentNamed(s) => {
                         let symbol = Symbol::intern(s);
+                        let span = arg.position_span;
                         Named(symbol, InnerSpan::new(span.start, span.end))
                     }
                 };
@@ -878,8 +879,9 @@ impl<'a, 'b> Context<'a, 'b> {
                         // track the current argument ourselves.
                         let i = self.curarg;
                         self.curarg += 1;
-                        parse::ArgumentIs(i, None)
+                        parse::ArgumentIs(i)
                     },
+                    position_span: arg.position_span,
                     format: parse::FormatSpec {
                         fill: arg.format.fill,
                         align: parse::AlignUnknown,

diff --git a/compiler/rustc_parse_format/src/lib.rs b/compiler/rustc_parse_format/src/lib.rs
@@ -70,6 +70,9 @@ pub enum Piece<'a> {
 pub struct Argument<'a> {
     /// Where to find this argument
     pub position: Position<'a>,
+    /// The span of the position indicator. Includes any whitespace in implicit
+    /// positions (`{  }`).
+    pub position_span: InnerSpan,
     /// How to format the argument
     pub format: FormatSpec<'a>,
 }
@@ -105,9 +108,9 @@ pub enum Position<'a> {
     /// The argument is implied to be located at an index
     ArgumentImplicitlyIs(usize),
     /// The argument is located at a specific index given in the format,
-    ArgumentIs(usize, Option<InnerSpan>),
+    ArgumentIs(usize),
     /// The argument has a name.
-    ArgumentNamed(&'a str, InnerSpan),
+    ArgumentNamed(&'a str),
 }
 
 impl Position<'_> {
@@ -216,14 +219,15 @@ impl<'a> Iterator for Parser<'a> {
                 '{' => {
                     let curr_last_brace = self.last_opening_brace;
                     let byte_pos = self.to_span_index(pos);
-                    self.last_opening_brace = Some(byte_pos.to(InnerOffset(byte_pos.0 + 1)));
+                    let lbrace_end = InnerOffset(byte_pos.0 + 1);
+                    self.last_opening_brace = Some(byte_pos.to(lbrace_end));
                     self.cur.next();
                     if self.consume('{') {
                         self.last_opening_brace = curr_last_brace;
 
                         Some(String(self.string(pos + 1)))
                     } else {
-                        let arg = self.argument();
+                        let arg = self.argument(lbrace_end);
                         if let Some(rbrace_byte_idx) = self.must_consume('}') {
                             let lbrace_inner_offset = self.to_span_index(pos);
                             let rbrace_inner_offset = self.to_span_index(rbrace_byte_idx);
@@ -477,8 +481,16 @@ impl<'a> Parser<'a> {
     }
 
     /// Parses an `Argument` structure, or what's contained within braces inside the format string.
-    fn argument(&mut self) -> Argument<'a> {
+    fn argument(&mut self, start: InnerOffset) -> Argument<'a> {
         let pos = self.position();
+
+        let end = self
+            .cur
+            .clone()
+            .find(|(_, ch)| !ch.is_whitespace())
+            .map_or(start, |(end, _)| self.to_span_index(end));
+        let position_span = start.to(end);
+
         let format = match self.mode {
             ParseMode::Format => self.format(),
             ParseMode::InlineAsm => self.inline_asm(),
@@ -494,31 +506,19 @@ impl<'a> Parser<'a> {
             }
         };
 
-        Argument { position: pos, format }
+        Argument { position: pos, position_span, format }
     }
 
     /// Parses a positional argument for a format. This could either be an
     /// integer index of an argument, a named argument, or a blank string.
     /// Returns `Some(parsed_position)` if the position is not implicitly
     /// consuming a macro argument, `None` if it's the case.
     fn position(&mut self) -> Option<Position<'a>> {
-        let start_position = self.cur.peek().map(|item| item.0);
         if let Some(i) = self.integer() {
-            let inner_span = start_position.and_then(|start| {
-                self.cur
-                    .peek()
-                    .cloned()
-                    .and_then(|item| Some(self.to_span_index(start).to(self.to_span_index(item.0))))
-            });
-            Some(ArgumentIs(i, inner_span))
+            Some(ArgumentIs(i))
         } else {
             match self.cur.peek() {
-                Some(&(start, c)) if rustc_lexer::is_id_start(c) => {
-                    let word = self.word();
-                    let end = start + word.len();
-                    let span = self.to_span_index(start).to(self.to_span_index(end));
-                    Some(ArgumentNamed(word, span))
-                }
+                Some(&(_, c)) if rustc_lexer::is_id_start(c) => Some(ArgumentNamed(self.word())),
 
                 // This is an `ArgumentNext`.
                 // Record the fact and do the resolution after parsing the

diff --git a/compiler/rustc_parse_format/src/tests.rs b/compiler/rustc_parse_format/src/tests.rs
@@ -58,14 +58,22 @@ fn invalid06() {
 
 #[test]
 fn format_nothing() {
-    same("{}", &[NextArgument(Argument { position: ArgumentImplicitlyIs(0), format: fmtdflt() })]);
+    same(
+        "{}",
+        &[NextArgument(Argument {
+            position: ArgumentImplicitlyIs(0),
+            position_span: InnerSpan { start: 2, end: 2 },
+            format: fmtdflt(),
+        })],
+    );
 }
 #[test]
 fn format_position() {
     same(
         "{3}",
         &[NextArgument(Argument {
-            position: ArgumentIs(3, Some(InnerSpan { start: 2, end: 3 })),
+            position: ArgumentIs(3),
+            position_span: InnerSpan { start: 2, end: 3 },
             format: fmtdflt(),
         })],
     );
@@ -75,17 +83,30 @@ fn format_position_nothing_else() {
     same(
         "{3:}",
         &[NextArgument(Argument {
-            position: ArgumentIs(3, Some(InnerSpan { start: 2, end: 3 })),
+            position: ArgumentIs(3),
+            position_span: InnerSpan { start: 2, end: 3 },
             format: fmtdflt(),
         })],
     );
 }
 #[test]
+fn format_named() {
+    same(
+        "{name}",
+        &[NextArgument(Argument {
+            position: ArgumentNamed("name"),
+            position_span: InnerSpan { start: 2, end: 6 },
+            format: fmtdflt(),
+        })],
+    )
+}
+#[test]
 fn format_type() {
     same(
         "{3:x}",
         &[NextArgument(Argument {
-            position: ArgumentIs(3, Some(InnerSpan { start: 2, end: 3 })),
+            position: ArgumentIs(3),
+            position_span: InnerSpan { start: 2, end: 3 },
             format: FormatSpec {
                 fill: None,
                 align: AlignUnknown,
@@ -105,7 +126,8 @@ fn format_align_fill() {
     same(
         "{3:>}",
         &[NextArgument(Argument {
-            position: ArgumentIs(3, Some(InnerSpan { start: 2, end: 3 })),
+            position: ArgumentIs(3),
+            position_span: InnerSpan { start: 2, end: 3 },
             format: FormatSpec {
                 fill: None,
                 align: AlignRight,
@@ -122,7 +144,8 @@ fn format_align_fill() {
     same(
         "{3:0<}",
         &[NextArgument(Argument {
-            position: ArgumentIs(3, Some(InnerSpan { start: 2, end: 3 })),
+            position: ArgumentIs(3),
+            position_span: InnerSpan { start: 2, end: 3 },
             format: FormatSpec {
                 fill: Some('0'),
                 align: AlignLeft,
@@ -139,7 +162,8 @@ fn format_align_fill() {
     same(
         "{3:*<abcd}",
         &[NextArgument(Argument {
-            position: ArgumentIs(3, Some(InnerSpan { start: 2, end: 3 })),
+            position: ArgumentIs(3),
+            position_span: InnerSpan { start: 2, end: 3 },
             format: FormatSpec {
                 fill: Some('*'),
                 align: AlignLeft,
@@ -160,6 +184,7 @@ fn format_counts() {
         "{:10x}",
         &[NextArgument(Argument {
             position: ArgumentImplicitlyIs(0),
+            position_span: InnerSpan { start: 2, end: 2 },
             format: FormatSpec {
                 fill: None,
                 align: AlignUnknown,
@@ -177,6 +202,7 @@ fn format_counts() {
         "{:10$.10x}",
         &[NextArgument(Argument {
             position: ArgumentImplicitlyIs(0),
+            position_span: InnerSpan { start: 2, end: 2 },
             format: FormatSpec {
                 fill: None,
                 align: AlignUnknown,
@@ -193,7 +219,8 @@ fn format_counts() {
     same(
         "{1:0$.10x}",
         &[NextArgument(Argument {
-            position: ArgumentIs(1, Some(InnerSpan { start: 2, end: 3 })),
+            position: ArgumentIs(1),
+            position_span: InnerSpan { start: 2, end: 3 },
             format: FormatSpec {
                 fill: None,
                 align: AlignUnknown,
@@ -211,6 +238,7 @@ fn format_counts() {
         "{:.*x}",
         &[NextArgument(Argument {
             position: ArgumentImplicitlyIs(1),
+            position_span: InnerSpan { start: 2, end: 2 },
             format: FormatSpec {
                 fill: None,
                 align: AlignUnknown,
@@ -228,6 +256,7 @@ fn format_counts() {
         "{:.10$x}",
         &[NextArgument(Argument {
             position: ArgumentImplicitlyIs(0),
+            position_span: InnerSpan { start: 2, end: 2 },
             format: FormatSpec {
                 fill: None,
                 align: AlignUnknown,
@@ -245,6 +274,7 @@ fn format_counts() {
         "{:a$.b$?}",
         &[NextArgument(Argument {
             position: ArgumentImplicitlyIs(0),
+            position_span: InnerSpan { start: 2, end: 2 },
             format: FormatSpec {
                 fill: None,
                 align: AlignUnknown,
@@ -265,6 +295,7 @@ fn format_flags() {
         "{:-}",
         &[NextArgument(Argument {
             position: ArgumentImplicitlyIs(0),
+            position_span: InnerSpan { start: 2, end: 2 },
             format: FormatSpec {
                 fill: None,
                 align: AlignUnknown,
@@ -282,6 +313,7 @@ fn format_flags() {
         "{:+#}",
         &[NextArgument(Argument {
             position: ArgumentImplicitlyIs(0),
+            position_span: InnerSpan { start: 2, end: 2 },
             format: FormatSpec {
                 fill: None,
                 align: AlignUnknown,
@@ -303,7 +335,8 @@ fn format_mixture() {
         &[
             String("abcd "),
             NextArgument(Argument {
-                position: ArgumentIs(3, Some(InnerSpan { start: 7, end: 8 })),
+                position: ArgumentIs(3),
+                position_span: InnerSpan { start: 7, end: 8 },
                 format: FormatSpec {
                     fill: None,
                     align: AlignUnknown,
@@ -320,3 +353,22 @@ fn format_mixture() {
         ],
     );
 }
+#[test]
+fn format_whitespace() {
+    same(
+        "{ }",
+        &[NextArgument(Argument {
+            position: ArgumentImplicitlyIs(0),
+            position_span: InnerSpan { start: 2, end: 3 },
+            format: fmtdflt(),
+        })],
+    );
+    same(
+        "{  }",
+        &[NextArgument(Argument {
+            position: ArgumentImplicitlyIs(0),
+            position_span: InnerSpan { start: 2, end: 4 },
+            format: fmtdflt(),
+        })],
+    );
+}
diff --git a/compiler/rustc_trait_selection/src/traits/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/on_unimplemented.rs
@@ -300,7 +300,7 @@ impl<'tcx> OnUnimplementedFormatString {
             match token {
                 Piece::String(_) => (), // Normal string, no need to check it
                 Piece::NextArgument(a) => match a.position {
-                    Position::ArgumentNamed(s, _) => {
+                    Position::ArgumentNamed(s) => {
                         match Symbol::intern(s) {
                             // `{Self}` is allowed
                             kw::SelfUpper => (),
@@ -386,7 +386,7 @@ impl<'tcx> OnUnimplementedFormatString {
             .map(|p| match p {
                 Piece::String(s) => s,
                 Piece::NextArgument(a) => match a.position {
-                    Position::ArgumentNamed(s, _) => {
+                    Position::ArgumentNamed(s) => {
                         let s = Symbol::intern(s);
                         match generic_map.get(&s) {
                             Some(val) => val,

diff --git a/src/tools/clippy/clippy_lints/src/write.rs b/src/tools/clippy/clippy_lints/src/write.rs
@@ -441,7 +441,7 @@ impl SimpleFormatArgs {
         };
 
         match arg.position {
-            ArgumentIs(n, _) | ArgumentImplicitlyIs(n) => {
+            ArgumentIs(n) | ArgumentImplicitlyIs(n) => {
                 if self.unnamed.len() <= n {
                     // Use a dummy span to mark all unseen arguments.
                     self.unnamed.resize_with(n, || vec![DUMMY_SP]);
@@ -462,7 +462,7 @@ impl SimpleFormatArgs {
                     }
                 }
             },
-            ArgumentNamed(n, _) => {
+            ArgumentNamed(n) => {
                 let n = Symbol::intern(n);
                 if let Some(x) = self.named.iter_mut().find(|x| x.0 == n) {
                     match x.1.as_slice() {