Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: rustfmt-1.4.37
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/chains.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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
Expand Down
43 changes: 36 additions & 7 deletions src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
}
}
}
Expand All @@ -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' {
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
);
}
}
20 changes: 5 additions & 15 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,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)) => {
Expand Down Expand Up @@ -1187,11 +1182,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),
}
}

Expand All @@ -1207,7 +1198,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);
}
}

Expand Down Expand Up @@ -1971,8 +1962,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))
}
Expand Down
7 changes: 5 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 2 additions & 6 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion src/pairs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down
2 changes: 1 addition & 1 deletion src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 7 additions & 3 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,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
Expand Down Expand Up @@ -583,7 +587,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);

Expand Down
46 changes: 46 additions & 0 deletions tests/source/issue-4668.rs
Original file line number Diff line number Diff line change
@@ -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 {}
}
53 changes: 53 additions & 0 deletions tests/target/issue-4668.rs
Original file line number Diff line number Diff line change
@@ -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,
};
} else {
}
}