Merge branch 'master' into normalize-multiline-doc-attributes

Author: Ruben Schmidmeister

.travis.yml

@@ -26,12 +26,13 @@ matrix:
     - env: INTEGRATION=log
     - env: INTEGRATION=mdbook
     - env: INTEGRATION=packed_simd
-    - env: INTEGRATION=rand
     - env: INTEGRATION=rust-semverver
     - env: INTEGRATION=stdsimd TARGET=x86_64-unknown-linux-gnu
     - env: INTEGRATION=tempdir
     - env: INTEGRATION=futures-rs
   allow_failures:
+    # Using old configuration option
+    - env: INTEGRATION=rand
     # Doesn't build - keep this in allow_failures as it's fragile to breaking changes of rustc.
     - env: INTEGRATION=rust-clippy
     # Doesn't build - seems to be because of an option

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [Unreleased]
 
+- Change option `format_doc_comment` to `format_code_in_doc_comment`.
 - `use_small_heuristics` changed to be an enum and stabilised. Configuration
   options are now ready for 1.0.
 

Configurations.md

@@ -1978,9 +1978,9 @@ fn main() {
 }
 ```
 
-## `format_doc_comments`
+## `format_code_in_doc_comments`
 
-Format doc comments.
+Format code snippet included in doc comments.
 
 - **Default value**: `false`
 - **Possible values**: `true`, `false`

src/attr.rs

@@ -56,12 +56,11 @@ fn is_derive(attr: &ast::Attribute) -> bool {
 }
 
 /// Returns the arguments of `#[derive(...)]`.
-fn get_derive_spans(attr: &ast::Attribute) -> Option<Vec<Span>> {
+fn get_derive_spans<'a>(attr: &'a ast::Attribute) -> Option<impl Iterator<Item = Span> + 'a> {
     attr.meta_item_list().map(|meta_item_list| {
         meta_item_list
-            .iter()
+            .into_iter()
             .map(|nested_meta_item| nested_meta_item.span)
-            .collect()
     })
 }
 
@@ -412,10 +411,11 @@ impl<'a> Rewrite for [ast::Attribute] {
             // Handle derives if we will merge them.
             if context.config.merge_derives() && is_derive(&attrs[0]) {
                 let derives = take_while_with_pred(context, attrs, is_derive);
-                let mut derive_spans = vec![];
-                for derive in derives {
-                    derive_spans.append(&mut get_derive_spans(derive)?);
-                }
+                let derive_spans: Vec<_> = derives
+                    .iter()
+                    .filter_map(get_derive_spans)
+                    .flatten()
+                    .collect();
                 let derive_str =
                     format_derive(&derive_spans, attr_prefix(&attrs[0]), shape, context)?;
                 result.push_str(&derive_str);

src/checkstyle.rs

@@ -1,3 +1,4 @@
+use std::fmt::{self, Display};
 use std::io::{self, Write};
 use std::path::Path;
 
@@ -9,9 +10,9 @@ use crate::rustfmt_diff::{DiffLine, Mismatch};
 /// future version of Rustfmt.
 pub(crate) fn header() -> String {
     let mut xml_heading = String::new();
-    xml_heading.push_str("<?xml version=\"1.0\" encoding=\"utf-8\"?>");
+    xml_heading.push_str(r#"<?xml version="1.0" encoding="utf-8"?>"#);
     xml_heading.push_str("\n");
-    xml_heading.push_str("<checkstyle version=\"4.3\">");
+    xml_heading.push_str(r#"<checkstyle version="4.3">"#);
     xml_heading
 }
 
@@ -31,17 +32,16 @@ pub(crate) fn output_checkstyle_file<T>(
 where
     T: Write,
 {
-    write!(writer, "<file name=\"{}\">", filename.display())?;
+    write!(writer, r#"<file name="{}">"#, filename.display())?;
     for mismatch in diff {
         for line in mismatch.lines {
             // Do nothing with `DiffLine::Context` and `DiffLine::Resulting`.
-            if let DiffLine::Expected(ref str) = line {
-                let message = xml_escape_str(str);
+            if let DiffLine::Expected(message) = line {
                 write!(
                     writer,
-                    "<error line=\"{}\" severity=\"warning\" message=\"Should be `{}`\" \
-                     />",
-                    mismatch.line_number, message
+                    r#"<error line="{}" severity="warning" message="Should be `{}`" />"#,
+                    mismatch.line_number,
+                    XmlEscaped(&message)
                 )?;
             }
         }
@@ -50,19 +50,53 @@ where
     Ok(())
 }
 
-// Convert special characters into XML entities.
-// This is needed for checkstyle output.
-fn xml_escape_str(string: &str) -> String {
-    let mut out = String::new();
-    for c in string.chars() {
-        match c {
-            '<' => out.push_str("&lt;"),
-            '>' => out.push_str("&gt;"),
-            '"' => out.push_str("&quot;"),
-            '\'' => out.push_str("&apos;"),
-            '&' => out.push_str("&amp;"),
-            _ => out.push(c),
+/// Convert special characters into XML entities.
+/// This is needed for checkstyle output.
+struct XmlEscaped<'a>(&'a str);
+
+impl<'a> Display for XmlEscaped<'a> {
+    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+        for char in self.0.chars() {
+            match char {
+                '<' => write!(formatter, "&lt;"),
+                '>' => write!(formatter, "&gt;"),
+                '"' => write!(formatter, "&quot;"),
+                '\'' => write!(formatter, "&apos;"),
+                '&' => write!(formatter, "&amp;"),
+                _ => write!(formatter, "{}", char),
+            }?;
         }
+
+        Ok(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn special_characters_are_escaped() {
+        assert_eq!(
+            "&lt;&gt;&quot;&apos;&amp;",
+            format!("{}", XmlEscaped(r#"<>"'&"#)),
+        );
+    }
+
+    #[test]
+    fn special_characters_are_escaped_in_string_with_other_characters() {
+        assert_eq!(
+            "The quick brown &quot;🦊&quot; jumps &lt;over&gt; the lazy 🐶",
+            format!(
+                "{}",
+                XmlEscaped(r#"The quick brown "🦊" jumps <over> the lazy 🐶"#)
+            ),
+        );
+    }
+
+    #[test]
+    fn other_characters_are_not_escaped() {
+        let string = "The quick brown 🦊 jumps over the lazy 🐶";
+        assert_eq!(string, format!("{}", XmlEscaped(string)));
     }
-    out
 }

src/comment.rs

@@ -353,7 +353,7 @@ fn identify_comment(
             trim_left_preserve_layout(first_group, shape.indent, config)?
         } else if !config.normalize_comments()
             && !config.wrap_comments()
-            && !config.format_doc_comments()
+            && !config.format_code_in_doc_comments()
         {
             light_rewrite_comment(first_group, shape.indent, config, is_doc_comment)
         } else {
@@ -656,9 +656,16 @@ impl<'a> CommentRewrite<'a> {
                     _ => {
                         let mut config = self.fmt.config.clone();
                         config.set().wrap_comments(false);
-                        match crate::format_code_block(&self.code_block_buffer, &config) {
-                            Some(ref s) => trim_custom_comment_prefix(&s.snippet),
-                            None => trim_custom_comment_prefix(&self.code_block_buffer),
+                        if config.format_code_in_doc_comments() {
+                            if let Some(s) =
+                                crate::format_code_block(&self.code_block_buffer, &config)
+                            {
+                                trim_custom_comment_prefix(&s.snippet)
+                            } else {
+                                trim_custom_comment_prefix(&self.code_block_buffer)
+                            }
+                        } else {
+                            trim_custom_comment_prefix(&self.code_block_buffer)
                         }
                     }
                 };

src/config/config_type.rs

@@ -50,22 +50,6 @@ impl ConfigType for IgnoreList {
     }
 }
 
-/// Checks if we're in a nightly build.
-///
-/// The environment variable `CFG_RELEASE_CHANNEL` is set during the rustc bootstrap
-/// to "stable", "beta", or "nightly" depending on what toolchain is being built.
-/// If we are being built as part of the stable or beta toolchains, we want
-/// to disable unstable configuration options.
-///
-/// If we're being built by cargo (e.g., `cargo +nightly install rustfmt-nightly`),
-/// `CFG_RELEASE_CHANNEL` is not set. As we only support being built against the
-/// nightly compiler when installed from crates.io, default to nightly mode.
-macro_rules! is_nightly_channel {
-    () => {
-        option_env!("CFG_RELEASE_CHANNEL").map_or(true, |c| c == "nightly" || c == "dev")
-    };
-}
-
 macro_rules! create_config {
     ($($i:ident: $ty:ty, $def:expr, $stb:expr, $( $dstring:expr ),+ );+ $(;)*) => (
         #[cfg(test)]
@@ -159,7 +143,7 @@ macro_rules! create_config {
                         self.$i.1 = true;
                         self.$i.2 = val;
                     } else {
-                        if is_nightly_channel!() {
+                        if crate::is_nightly_channel!() {
                             self.$i.1 = true;
                             self.$i.2 = val;
                         } else {

src/config/mod.rs

@@ -40,7 +40,7 @@ create_config! {
 
     // Comments. macros, and strings
     wrap_comments: bool, false, false, "Break comments to fit on the line";
-    format_doc_comments: bool, false, false, "Format doc comments.";
+    format_code_in_doc_comments: bool, false, false, "Format the code snippet in doc comments.";
     comment_width: usize, 80, false,
         "Maximum length of comments. No effect unless wrap_comments = true";
     normalize_comments: bool, false, false, "Convert /* */ comments to // comments where possible";

src/formatting.rs

@@ -458,8 +458,7 @@ struct FormatLines<'a> {
     errors: Vec<FormattingError>,
     issue_seeker: BadIssueSeeker,
     line_buffer: String,
-    // `true` if the current line contains a string literal.
-    is_string: bool,
+    current_line_contains_string_literal: bool,
     format_line: bool,
     allow_issue_seek: bool,
     config: &'a Config,
@@ -483,7 +482,7 @@ impl<'a> FormatLines<'a> {
             allow_issue_seek: !issue_seeker.is_disabled(),
             issue_seeker,
             line_buffer: String::with_capacity(config.max_width() * 2),
-            is_string: false,
+            current_line_contains_string_literal: false,
             format_line: config.file_lines().contains_line(name, 1),
             config,
         }
@@ -547,7 +546,7 @@ impl<'a> FormatLines<'a> {
                 && !self.is_skipped_line()
                 && self.should_report_error(kind, &error_kind)
             {
-                let is_string = self.is_string;
+                let is_string = self.current_line_contains_string_literal;
                 self.push_err(error_kind, kind.is_comment(), is_string);
             }
         }
@@ -561,7 +560,7 @@ impl<'a> FormatLines<'a> {
         self.newline_count += 1;
         self.last_was_space = false;
         self.line_buffer.clear();
-        self.is_string = false;
+        self.current_line_contains_string_literal = false;
     }
 
     fn char(&mut self, c: char, kind: FullCodeCharKind) {
@@ -574,7 +573,7 @@ impl<'a> FormatLines<'a> {
         self.last_was_space = c.is_whitespace();
         self.line_buffer.push(c);
         if kind.is_string() {
-            self.is_string = true;
+            self.current_line_contains_string_literal = true;
         }
     }
 
@@ -589,12 +588,14 @@ impl<'a> FormatLines<'a> {
     }
 
     fn should_report_error(&self, char_kind: FullCodeCharKind, error_kind: &ErrorKind) -> bool {
-        let allow_error_report =
-            if char_kind.is_comment() || self.is_string || error_kind.is_comment() {
-                self.config.error_on_unformatted()
-            } else {
-                true
-            };
+        let allow_error_report = if char_kind.is_comment()
+            || self.current_line_contains_string_literal
+            || error_kind.is_comment()
+        {
+            self.config.error_on_unformatted()
+        } else {
+            true
+        };
 
         match error_kind {
             ErrorKind::LineOverflow(..) => {

src/items.rs

@@ -664,7 +664,6 @@ pub(crate) fn format_impl(
     context: &RewriteContext<'_>,
     item: &ast::Item,
     offset: Indent,
-    where_span_end: Option<BytePos>,
 ) -> Option<String> {
     if let ast::ItemKind::Impl(_, _, _, ref generics, _, ref self_ty, ref items) = item.node {
         let mut result = String::with_capacity(128);
@@ -691,6 +690,8 @@ pub(crate) fn format_impl(
             option.compress_where();
         }
 
+        let misssing_span = mk_sp(self_ty.span.hi(), item.span.hi());
+        let where_span_end = context.snippet_provider.opt_span_before(misssing_span, "{");
         let where_clause_str = rewrite_where_clause(
             context,
             &generics.where_clause,

src/lib.rs

@@ -40,6 +40,9 @@ pub use crate::rustfmt_diff::{ModifiedChunk, ModifiedLines};
 #[macro_use]
 mod utils;
 
+#[macro_use]
+mod release_channel;
+
 mod attr;
 mod chains;
 pub(crate) mod checkstyle;

src/release_channel.rs

@@ -0,0 +1,16 @@
+/// Checks if we're in a nightly build.
+///
+/// The environment variable `CFG_RELEASE_CHANNEL` is set during the rustc bootstrap
+/// to "stable", "beta", or "nightly" depending on what toolchain is being built.
+/// If we are being built as part of the stable or beta toolchains, we want
+/// to disable unstable configuration options.
+///
+/// If we're being built by cargo (e.g., `cargo +nightly install rustfmt-nightly`),
+/// `CFG_RELEASE_CHANNEL` is not set. As we only support being built against the
+/// nightly compiler when installed from crates.io, default to nightly mode.
+#[macro_export]
+macro_rules! is_nightly_channel {
+    () => {
+        option_env!("CFG_RELEASE_CHANNEL").map_or(true, |c| c == "nightly" || c == "dev")
+    };
+}