From f4306ffbfcb2280f180e51ce6421e1c6c28dccec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 19 Jul 2018 23:14:00 -0700 Subject: [PATCH] Use correct spans for format string errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When encountering format string errors in a raw string, or regular string literal with embedded newlines, account for the positional change to use correct spans. :drive by fix: 🚗 --- src/Cargo.lock | 3 ++ src/libfmt_macros/Cargo.toml | 3 ++ src/libfmt_macros/lib.rs | 40 ++++++++++++---- src/librustc/traits/on_unimplemented.rs | 6 +-- src/libsyntax_ext/format.rs | 2 +- src/test/ui/fmt/format-string-error.rs | 34 +++++++++++++ src/test/ui/fmt/format-string-error.stderr | 56 ++++++++++++++++++---- 7 files changed, 123 insertions(+), 21 deletions(-) diff --git a/src/Cargo.lock b/src/Cargo.lock index 8299dea1c4b0..c59c11084957 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -715,6 +715,9 @@ dependencies = [ [[package]] name = "fmt_macros" version = "0.0.0" +dependencies = [ + "syntax 0.0.0", +] [[package]] name = "fnv" diff --git a/src/libfmt_macros/Cargo.toml b/src/libfmt_macros/Cargo.toml index b3f4d2deae2f..6e6af9c2ff5e 100644 --- a/src/libfmt_macros/Cargo.toml +++ b/src/libfmt_macros/Cargo.toml @@ -7,3 +7,6 @@ version = "0.0.0" name = "fmt_macros" path = "lib.rs" crate-type = ["dylib"] + +[dependencies] +syntax = { path = "../libsyntax" } diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs index ee590bc3b5ec..54cf3d24173f 100644 --- a/src/libfmt_macros/lib.rs +++ b/src/libfmt_macros/lib.rs @@ -28,6 +28,8 @@ pub use self::Alignment::*; pub use self::Flag::*; pub use self::Count::*; +extern crate syntax; + use std::str; use std::string; use std::iter; @@ -150,18 +152,27 @@ pub struct Parser<'a> { pub errors: Vec, /// Current position of implicit positional argument pointer curarg: usize, + /// The style of the string (raw or not), used to position spans correctly + style: syntax::ast::StrStyle, + /// How many newlines have been seen in the string so far, to adjust the error spans + seen_newlines: usize, } impl<'a> Iterator for Parser<'a> { type Item = Piece<'a>; fn next(&mut self) -> Option> { + let raw = match self.style { + syntax::ast::StrStyle::Raw(raw) => raw as usize + self.seen_newlines, + _ => 0, + }; if let Some(&(pos, c)) = self.cur.peek() { match c { '{' => { + let pos = pos + raw + 1; self.cur.next(); if self.consume('{') { - Some(String(self.string(pos + 1))) + Some(String(self.string(pos))) } else { let ret = Some(NextArgument(self.argument())); self.must_consume('}'); @@ -169,8 +180,8 @@ impl<'a> Iterator for Parser<'a> { } } '}' => { + let pos = pos + raw + 1; self.cur.next(); - let pos = pos + 1; if self.consume('}') { Some(String(self.string(pos))) } else { @@ -184,6 +195,10 @@ impl<'a> Iterator for Parser<'a> { None } } + '\n' => { + self.seen_newlines += 1; + Some(String(self.string(pos))) + } _ => Some(String(self.string(pos))), } } else { @@ -194,12 +209,14 @@ impl<'a> Iterator for Parser<'a> { impl<'a> Parser<'a> { /// Creates a new parser for the given format string - pub fn new(s: &'a str) -> Parser<'a> { + pub fn new(s: &'a str, style: syntax::ast::StrStyle) -> Parser<'a> { Parser { input: s, cur: s.char_indices().peekable(), errors: vec![], curarg: 0, + style, + seen_newlines: 0, } } @@ -262,14 +279,19 @@ impl<'a> Parser<'a> { /// found, an error is emitted. fn must_consume(&mut self, c: char) { self.ws(); + let raw = match self.style { + syntax::ast::StrStyle::Raw(raw) => raw as usize, + _ => 0, + }; + let padding = raw + self.seen_newlines; if let Some(&(pos, maybe)) = self.cur.peek() { if c == maybe { self.cur.next(); } else { self.err(format!("expected `{:?}`, found `{:?}`", c, maybe), format!("expected `{}`", c), - pos + 1, - pos + 1); + pos + padding + 1, + pos + padding + 1); } } else { let msg = format!("expected `{:?}` but string was terminated", c); @@ -282,8 +304,8 @@ impl<'a> Parser<'a> { self.err_with_note(msg, format!("expected `{:?}`", c), "if you intended to print `{`, you can escape it using `{{`", - pos, - pos); + pos + padding, + pos + padding); } else { self.err(msg, format!("expected `{:?}`", c), pos, pos); } @@ -540,7 +562,7 @@ mod tests { use super::*; fn same(fmt: &'static str, p: &[Piece<'static>]) { - let parser = Parser::new(fmt); + let parser = Parser::new(fmt, syntax::ast::StrStyle::Cooked); assert!(parser.collect::>>() == p); } @@ -556,7 +578,7 @@ mod tests { } fn musterr(s: &str) { - let mut p = Parser::new(s); + let mut p = Parser::new(fmt, syntax::ast::StrStyle::Cooked); p.next(); assert!(!p.errors.is_empty()); } diff --git a/src/librustc/traits/on_unimplemented.rs b/src/librustc/traits/on_unimplemented.rs index 0550cf7c6d2c..33d5502eba00 100644 --- a/src/librustc/traits/on_unimplemented.rs +++ b/src/librustc/traits/on_unimplemented.rs @@ -15,7 +15,7 @@ use ty::{self, TyCtxt, GenericParamDefKind}; use util::common::ErrorReported; use util::nodemap::FxHashMap; -use syntax::ast::{MetaItem, NestedMetaItem}; +use syntax::ast::{self, MetaItem, NestedMetaItem}; use syntax::attr; use syntax_pos::Span; use syntax_pos::symbol::LocalInternedString; @@ -242,7 +242,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString { { let name = tcx.item_name(trait_def_id); let generics = tcx.generics_of(trait_def_id); - let parser = Parser::new(&self.0); + let parser = Parser::new(&self.0, ast::StrStyle::Cooked); let mut result = Ok(()); for token in parser { match token { @@ -298,7 +298,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString { Some((name, value)) }).collect::>(); - let parser = Parser::new(&self.0); + let parser = Parser::new(&self.0, ast::StrStyle::Cooked); parser.map(|p| { match p { Piece::String(s) => s, diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index d3e5adf68350..7acfb08020fd 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -763,7 +763,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt, }; let fmt_str = &*fmt.node.0.as_str(); - let mut parser = parse::Parser::new(fmt_str); + let mut parser = parse::Parser::new(fmt_str, fmt.node.1); let mut pieces = vec![]; while let Some(mut piece) = parser.next() { diff --git a/src/test/ui/fmt/format-string-error.rs b/src/test/ui/fmt/format-string-error.rs index e48aa489f435..f39110ebc5dc 100644 --- a/src/test/ui/fmt/format-string-error.rs +++ b/src/test/ui/fmt/format-string-error.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// ignore-tidy-tab + fn main() { println!("{"); //~^ ERROR invalid format string: expected `'}'` but string was terminated @@ -24,4 +26,36 @@ fn main() { //~^ ERROR invalid format string: unmatched `}` found let _ = format!("{\\}"); //~^ ERROR invalid format string: expected `'}'`, found `'\\'` + let _ = format!("\n\n\n{\n\n\n"); + //~^ ERROR invalid format string + let _ = format!(r###" + + + + {"###); + //~^ ERROR invalid format string + let _ = format!(r###" + + + + { + +"###); + //~^^ ERROR invalid format string + let _ = format!(r###" + + + + } + +"###); + //~^^^ ERROR invalid format string + let _ = format!(r###" + + + + } + +"###); + //~^^^ ERROR invalid format string: unmatched `}` found } diff --git a/src/test/ui/fmt/format-string-error.stderr b/src/test/ui/fmt/format-string-error.stderr index 4c7ef11b29ef..9c84fd252189 100644 --- a/src/test/ui/fmt/format-string-error.stderr +++ b/src/test/ui/fmt/format-string-error.stderr @@ -1,5 +1,5 @@ error: invalid format string: expected `'}'` but string was terminated - --> $DIR/format-string-error.rs:12:16 + --> $DIR/format-string-error.rs:14:16 | LL | println!("{"); | ^ expected `'}'` in format string @@ -7,7 +7,7 @@ LL | println!("{"); = note: if you intended to print `{`, you can escape it using `{{` error: invalid format string: unmatched `}` found - --> $DIR/format-string-error.rs:15:15 + --> $DIR/format-string-error.rs:17:15 | LL | println!("}"); | ^ unmatched `}` in format string @@ -15,7 +15,7 @@ LL | println!("}"); = note: if you intended to print `}`, you can escape it using `}}` error: invalid format string: invalid argument name `_foo` - --> $DIR/format-string-error.rs:17:23 + --> $DIR/format-string-error.rs:19:23 | LL | let _ = format!("{_foo}", _foo = 6usize); | ^^^^ invalid argument name in format string @@ -23,7 +23,7 @@ LL | let _ = format!("{_foo}", _foo = 6usize); = note: argument names cannot start with an underscore error: invalid format string: invalid argument name `_` - --> $DIR/format-string-error.rs:19:23 + --> $DIR/format-string-error.rs:21:23 | LL | let _ = format!("{_}", _ = 6usize); | ^ invalid argument name in format string @@ -31,7 +31,7 @@ LL | let _ = format!("{_}", _ = 6usize); = note: argument names cannot start with an underscore error: invalid format string: expected `'}'` but string was terminated - --> $DIR/format-string-error.rs:21:23 + --> $DIR/format-string-error.rs:23:23 | LL | let _ = format!("{"); | ^ expected `'}'` in format string @@ -39,7 +39,7 @@ LL | let _ = format!("{"); = note: if you intended to print `{`, you can escape it using `{{` error: invalid format string: unmatched `}` found - --> $DIR/format-string-error.rs:23:22 + --> $DIR/format-string-error.rs:25:22 | LL | let _ = format!("}"); | ^ unmatched `}` in format string @@ -47,10 +47,50 @@ LL | let _ = format!("}"); = note: if you intended to print `}`, you can escape it using `}}` error: invalid format string: expected `'}'`, found `'/'` - --> $DIR/format-string-error.rs:25:23 + --> $DIR/format-string-error.rs:27:23 | LL | let _ = format!("{/}"); | ^ expected `}` in format string -error: aborting due to 7 previous errors +error: invalid format string: expected `'}'` but string was terminated + --> $DIR/format-string-error.rs:29:29 + | +LL | let _ = format!("/n/n/n{/n/n/n"); + | ^ expected `'}'` in format string + | + = note: if you intended to print `{`, you can escape it using `{{` + +error: invalid format string: expected `'}'` but string was terminated + --> $DIR/format-string-error.rs:35:3 + | +LL | {"###); + | ^ expected `'}'` in format string + | + = note: if you intended to print `{`, you can escape it using `{{` + +error: invalid format string: expected `'}'` but string was terminated + --> $DIR/format-string-error.rs:42:1 + | +LL | + | ^ expected `'}'` in format string + | + = note: if you intended to print `{`, you can escape it using `{{` + +error: invalid format string: unmatched `}` found + --> $DIR/format-string-error.rs:49:2 + | +LL | } + | ^ unmatched `}` in format string + | + = note: if you intended to print `}`, you can escape it using `}}` + +error: invalid format string: unmatched `}` found + --> $DIR/format-string-error.rs:57:9 + | +LL | } + | ^ unmatched `}` in format string + | + = note: if you intended to print `}`, you can escape it using `}}` + +error: aborting due to 12 previous errors