Filter out block comment in filter_normal_code if configured to use rustfmt 2 (#4668)

Add idempotent test cases

Author: ChinYing-Li

src/chains.rs

@@ -442,7 +442,7 @@ impl Rewrite for Chain {
 
  formatter.format_root(&self.parent, context, shape)?;
  if let Some(result) = formatter.pure_root() {
- return wrap_str(result, context.config.max_width(), shape);
+ return wrap_str(result, context.config, shape);
  }
 
  // Decide how to layout the rest of the chain.
@@ -452,7 +452,7 @@ impl Rewrite for Chain {
  formatter.format_last_child(context, shape, child_shape)?;
 
  let result = formatter.join_rewrites(context, child_shape)?;
- wrap_str(result, context.config.max_width(), shape)
+ wrap_str(result, context.config, shape)
  }
 }
 
@@ -813,7 +813,7 @@ impl<'a> ChainFormatter for ChainFormatterVisual<'a> {
  .visual_indent(self.offset)
  .sub_width(self.offset)?;
  let rewrite = item.rewrite(context, child_shape)?;
- match wrap_str(rewrite, context.config.max_width(), shape) {
+ match wrap_str(rewrite, context.config, shape) {
  Some(rewrite) => root_rewrite.push_str(&rewrite),
  None => {
  // We couldn't fit in at the visual indent, try the last

src/comment.rs

@@ -5,7 +5,7 @@ use std::{self, borrow::Cow, iter};
 use itertools::{multipeek, MultiPeek};
 use rustc_span::Span;
 
-use crate::config::Config;
+use crate::config::{Config, Version};
 use crate::rewrite::RewriteContext;
 use crate::shape::{Indent, Shape};
 use crate::string::{rewrite_string, StringFormat};
@@ -1364,13 +1364,15 @@ where
 pub(crate) struct LineClasses<'a> {
  base: iter::Peekable<CharClasses<std::str::Chars<'a>>>,
  kind: FullCodeCharKind,
+ config_version: Version,
 }
 
 impl<'a> LineClasses<'a> {
- pub(crate) fn new(s: &'a str) -> Self {
+ pub(crate) fn new(s: &'a str, config_version: Version) -> Self {
  LineClasses {
  base: CharClasses::new(s.chars()).peekable(),
  kind: FullCodeCharKind::Normal,
+ config_version,
  }
  }
 }
@@ -1388,7 +1390,9 @@ impl<'a> Iterator for LineClasses<'a> {
  None => unreachable!(),
  };
 
+ let mut prev_kind = FullCodeCharKind::Normal;
  while let Some((kind, c)) = self.base.next() {
+ prev_kind = self.kind;
  // needed to set the kind of the ending character on the last line
  self.kind = kind;
  if c == '\n' {
@@ -1405,6 +1409,12 @@ impl<'a> Iterator for LineClasses<'a> {
  (FullCodeCharKind::InStringCommented, FullCodeCharKind::InComment) => {
  FullCodeCharKind::EndStringCommented
  }
+ (_, FullCodeCharKind::Normal)
+ if prev_kind == FullCodeCharKind::EndComment
+ && self.config_version == Version::Two =>
+ {
+ FullCodeCharKind::EndComment
+ }
  _ => kind,
  };
  break;
@@ -1585,9 +1595,9 @@ pub(crate) fn recover_comment_removed(
  }
 }
 
-pub(crate) fn filter_normal_code(code: &str) -> String {
+pub(crate) fn filter_normal_code(code: &str, config_version: Version) -> String {
  let mut buffer = String::with_capacity(code.len());
- LineClasses::new(code).for_each(|(kind, line)| match kind {
+ LineClasses::new(code, config_version).for_each(|(kind, line)| match kind {
  FullCodeCharKind::Normal
  | FullCodeCharKind::StartString
  | FullCodeCharKind::InString
@@ -1905,13 +1915,32 @@ fn main() {
  println!("hello, world");
 }
 "#;
- assert_eq!(s, filter_normal_code(s));
- let s_with_comment = r#"
+ assert_eq!(s, filter_normal_code(s, Version::Two));
+ let s_with_line_comment = r#"
 fn main() {
  // hello, world
  println!("hello, world");
 }
 "#;
- assert_eq!(s, filter_normal_code(s_with_comment));
+ assert_eq!(s, filter_normal_code(s_with_line_comment, Version::Two));
+ let s_with_block_comment = r#"
+fn main() {
+ /* hello, world */
+ println!("hello, world");
+}
+"#;
+ assert_eq!(s, filter_normal_code(s_with_block_comment, Version::Two));
+ let s_with_multi_line_comment = r#"
+fn main() {
+ /* hello,
+ * world
+ */
+ println!("hello, world");
+}
+"#;
+ assert_eq!(
+ s,
+ filter_normal_code(s_with_multi_line_comment, Version::Two)
+ );
  }
 }

src/expr.rs

@@ -203,13 +203,8 @@ pub(crate) fn format_expr(
  rewrite_chain(expr, context, shape)
  }
  ast::ExprKind::MacCall(ref mac) => {
- rewrite_macro(mac, None, context, shape, MacroPosition::Expression).or_else(|| {
- wrap_str(
- context.snippet(expr.span).to_owned(),
- context.config.max_width(),
- shape,
- )
- })
+ rewrite_macro(mac, None, context, shape, MacroPosition::Expression)
+ .or_else(|| wrap_str(context.snippet(expr.span).to_owned(), context.config, shape))
  }
  ast::ExprKind::Ret(None) => Some("return".to_owned()),
  ast::ExprKind::Ret(Some(ref expr)) => {
@@ -1192,11 +1187,7 @@ pub(crate) fn rewrite_literal(
 ) -> Option<String> {
  match l.kind {
  ast::LitKind::Str(_, ast::StrStyle::Cooked) => rewrite_string_lit(context, l.span, shape),
- _ => wrap_str(
- context.snippet(l.span).to_owned(),
- context.config.max_width(),
- shape,
- ),
+ _ => wrap_str(context.snippet(l.span).to_owned(), context.config, shape),
  }
 }
 
@@ -1212,7 +1203,7 @@ fn rewrite_string_lit(context: &RewriteContext<'_>, span: Span, shape: Shape) ->
  {
  return Some(string_lit.to_owned());
  } else {
- return wrap_str(string_lit.to_owned(), context.config.max_width(), shape);
+ return wrap_str(string_lit.to_owned(), context.config, shape);
  }
  }
 
@@ -1976,8 +1967,7 @@ fn choose_rhs<R: Rewrite>(
 
  match (orig_rhs, new_rhs) {
  (Some(ref orig_rhs), Some(ref new_rhs))
- if wrap_str(new_rhs.clone(), context.config.max_width(), new_shape)
- .is_none() =>
+ if wrap_str(new_rhs.clone(), context.config, new_shape).is_none() =>
  {
  Some(format!("{}{}", before_space_str, orig_rhs))
  }

src/lib.rs

@@ -348,7 +348,7 @@ fn format_code_block(
  let mut result = String::with_capacity(s.len() * 2);
  result.push_str(FN_MAIN_PREFIX);
  let mut need_indent = true;
- for (kind, line) in LineClasses::new(s) {
+ for (kind, line) in LineClasses::new(s, config.version()) {
  if need_indent {
  result.push_str(&indent.to_string(config));
  }
@@ -385,7 +385,10 @@ fn format_code_block(
  .unwrap_or_else(|| formatted.snippet.len());
  let mut is_indented = true;
  let indent_str = Indent::from_width(config, config.tab_spaces()).to_string(config);
- for (kind, ref line) in LineClasses::new(&formatted.snippet[FN_MAIN_PREFIX.len()..block_len]) {
+ for (kind, ref line) in LineClasses::new(
+ &formatted.snippet[FN_MAIN_PREFIX.len()..block_len],
+ config.version(),
+ ) {
  if !is_first {
  result.push('\n');
  } else {

src/macros.rs

@@ -1372,15 +1372,11 @@ impl MacroBranch {
  }
  }
  };
- let new_body = wrap_str(
- new_body_snippet.snippet.to_string(),
- config.max_width(),
- shape,
- )?;
+ let new_body = wrap_str(new_body_snippet.snippet.to_string(), &config, shape)?;
 
  // Indent the body since it is in a block.
  let indent_str = body_indent.to_string(&config);
- let mut new_body = LineClasses::new(new_body.trim_end())
+ let mut new_body = LineClasses::new(new_body.trim_end(), config.version())
  .enumerate()
  .fold(
  (String::new(), true),

src/pairs.rs

@@ -86,7 +86,7 @@ fn rewrite_pairs_one_line<T: Rewrite>(
  return None;
  }
 
- wrap_str(result, context.config.max_width(), shape)
+ wrap_str(result, context.config, shape)
 }
 
 fn rewrite_pairs_multiline<T: Rewrite>(

src/string.rs

@@ -150,7 +150,7 @@ pub(crate) fn rewrite_string<'a>(
  }
 
  result.push_str(fmt.closer);
- wrap_str(result, fmt.config.max_width(), fmt.shape)
+ wrap_str(result, fmt.config, fmt.shape)
 }
 
 /// Returns the index to the end of the URL if the split at index of the given string includes an

src/utils.rs

@@ -390,8 +390,12 @@ macro_rules! skip_out_of_file_lines_range_visitor {
 
 // Wraps String in an Option. Returns Some when the string adheres to the
 // Rewrite constraints defined for the Rewrite trait and None otherwise.
-pub(crate) fn wrap_str(s: String, max_width: usize, shape: Shape) -> Option<String> {
- if is_valid_str(&filter_normal_code(&s), max_width, shape) {
+pub(crate) fn wrap_str(s: String, config: &Config, shape: Shape) -> Option<String> {
+ if is_valid_str(
+ &filter_normal_code(&s, config.version()),
+ config.max_width(),
+ shape,
+ ) {
  Some(s)
  } else {
  None
@@ -579,7 +583,7 @@ pub(crate) fn trim_left_preserve_layout(
  indent: Indent,
  config: &Config,
 ) -> Option<String> {
- let mut lines = LineClasses::new(orig);
+ let mut lines = LineClasses::new(orig, config.version());
  let first_line = lines.next().map(|(_, s)| s.trim_end().to_owned())?;
  let mut trimmed_lines = Vec::with_capacity(16);
 

tests/source/issue-4668.rs

@@ -0,0 +1,46 @@
+// rustfmt-version: Two
+
+const C: usize = 0_usize;
+const U: usize = /* A long block-style comment A long block-style comment A long block-style comment A long block-style comment */ if C > 0 { 4 } else { 3 };
+
+fn f() {
+ fn g() -> int {
+ let foo = 12;
+ match foo {
+ 0..=10 => {
+ /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin urna felis, molestie ex. */
+ }
+ _ => {
+ let st = None;
+ let res = st.unwrap_or(
+ /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin urna felis, molestie ex. */ "abc"
+ );
+ }
+ }
+ }
+}
+
+fn h() {
+ let mut y = 0;
+ while let x = vec![1, 2, 3]
+ .iter()
+ /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin urna felis, molestie ex... */
+ .take(3)
+ .skip(1)
+ .to_owned()
+ .next()
+ {
+ y += x.unwrap_or_else(|/* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin urna felis, molestie ex... */_| &0) * 2;
+ }
+}
+
+pub fn t(x: int) {
+ if x == 3 {
+ let y = match context {
+ True => {
+ |x| /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin urna felis, molestie ex... */ x+2
+ }
+ False => |x| x,
+ };
+ } else {}
+}

tests/target/issue-4668.rs

@@ -0,0 +1,53 @@
+// rustfmt-version: Two
+
+const C: usize = 0_usize;
+const U: usize =
+ /* A long block-style comment A long block-style comment A long block-style comment A long block-style comment */
+ if C > 0 { 4 } else { 3 };
+
+fn f() {
+ fn g() -> int {
+ let foo = 12;
+ match foo {
+ 0..=10 => {
+ /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin urna felis, molestie ex. */
+ }
+ _ => {
+ let st = None;
+ let res = st.unwrap_or(
+ /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin urna felis, molestie ex. */
+ "abc",
+ );
+ }
+ }
+ }
+}
+
+fn h() {
+ let mut y = 0;
+ while let x = vec![1, 2, 3]
+ .iter()
+ /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin urna felis, molestie ex... */
+ .take(3)
+ .skip(1)
+ .to_owned()
+ .next()
+ {
+ y += x.unwrap_or_else(
+ |/* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin urna felis, molestie ex... */
+ _| &0,
+ ) * 2;
+ }
+}
+
+pub fn t(x: int) {
+ if x == 3 {
+ let y = match context {
+ True => {
+ |x| /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin urna felis, molestie ex... */ x+2
+ }
+ False => |x| x,
+ };car
+ } else {
+ }
+}