Skip to content

Commit

Permalink
Merge pull request rust-lang#3284 from scampi/issue-3270
Browse files Browse the repository at this point in the history
recognize strings inside comments in order to avoid indenting them
  • Loading branch information
nrc authored Jan 17, 2019
2 parents a01990c + 083a20f commit d2e91b5
Show file tree
Hide file tree
Showing 9 changed files with 150 additions and 38 deletions.
84 changes: 69 additions & 15 deletions src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1047,21 +1047,28 @@ impl RichChar for (usize, char) {
#[derive(PartialEq, Eq, Debug, Clone, Copy)]
enum CharClassesStatus {
Normal,
/// Character is within a string
LitString,
LitStringEscape,
/// Character is within a raw string
LitRawString(u32),
RawStringPrefix(u32),
RawStringSuffix(u32),
LitChar,
LitCharEscape,
// The u32 is the nesting deepness of the comment
/// Character inside a block comment, with the integer indicating the nesting deepness of the
/// comment
BlockComment(u32),
// Status when the '/' has been consumed, but not yet the '*', deepness is
// the new deepness (after the comment opening).
/// Character inside a block-commented string, with the integer indicating the nesting deepness
/// of the comment
StringInBlockComment(u32),
/// Status when the '/' has been consumed, but not yet the '*', deepness is
/// the new deepness (after the comment opening).
BlockCommentOpening(u32),
// Status when the '*' has been consumed, but not yet the '/', deepness is
// the new deepness (after the comment closing).
/// Status when the '*' has been consumed, but not yet the '/', deepness is
/// the new deepness (after the comment closing).
BlockCommentClosing(u32),
/// Character is within a line comment
LineComment,
}

Expand All @@ -1085,6 +1092,12 @@ pub enum FullCodeCharKind {
InComment,
/// Last character of a comment, '\n' for a line comment, '/' for a block comment.
EndComment,
/// Start of a mutlitine string inside a comment
StartStringCommented,
/// End of a mutlitine string inside a comment
EndStringCommented,
/// Inside a commented string
InStringCommented,
/// Start of a mutlitine string
StartString,
/// End of a mutlitine string
Expand All @@ -1098,7 +1111,21 @@ impl FullCodeCharKind {
match self {
FullCodeCharKind::StartComment
| FullCodeCharKind::InComment
| FullCodeCharKind::EndComment => true,
| FullCodeCharKind::EndComment
| FullCodeCharKind::StartStringCommented
| FullCodeCharKind::InStringCommented
| FullCodeCharKind::EndStringCommented => true,
_ => false,
}
}

/// Returns true if the character is inside a comment
pub fn inside_comment(self) -> bool {
match self {
FullCodeCharKind::InComment
| FullCodeCharKind::StartStringCommented
| FullCodeCharKind::InStringCommented
| FullCodeCharKind::EndStringCommented => true,
_ => false,
}
}
Expand All @@ -1107,6 +1134,12 @@ impl FullCodeCharKind {
self == FullCodeCharKind::InString || self == FullCodeCharKind::StartString
}

/// Returns true if the character is within a commented string
pub fn is_commented_string(self) -> bool {
self == FullCodeCharKind::InStringCommented
|| self == FullCodeCharKind::StartStringCommented
}

fn to_codecharkind(self) -> CodeCharKind {
if self.is_comment() {
CodeCharKind::Comment
Expand Down Expand Up @@ -1250,18 +1283,27 @@ where
},
_ => CharClassesStatus::Normal,
},
CharClassesStatus::StringInBlockComment(deepness) => {
char_kind = FullCodeCharKind::InStringCommented;
if chr == '"' {
CharClassesStatus::BlockComment(deepness)
} else {
CharClassesStatus::StringInBlockComment(deepness)
}
}
CharClassesStatus::BlockComment(deepness) => {
assert_ne!(deepness, 0);
self.status = match self.base.peek() {
char_kind = FullCodeCharKind::InComment;
match self.base.peek() {
Some(next) if next.get_char() == '/' && chr == '*' => {
CharClassesStatus::BlockCommentClosing(deepness - 1)
}
Some(next) if next.get_char() == '*' && chr == '/' => {
CharClassesStatus::BlockCommentOpening(deepness + 1)
}
_ => CharClassesStatus::BlockComment(deepness),
};
return Some((FullCodeCharKind::InComment, item));
_ if chr == '"' => CharClassesStatus::StringInBlockComment(deepness),
_ => self.status,
}
}
CharClassesStatus::BlockCommentOpening(deepness) => {
assert_eq!(chr, '*');
Expand Down Expand Up @@ -1317,26 +1359,33 @@ impl<'a> Iterator for LineClasses<'a> {

let mut line = String::new();

let start_class = match self.base.peek() {
let start_kind = match self.base.peek() {
Some((kind, _)) => *kind,
None => unreachable!(),
};

while let Some((kind, c)) = self.base.next() {
// needed to set the kind of the ending character on the last line
self.kind = kind;
if c == '\n' {
self.kind = match (start_class, kind) {
self.kind = match (start_kind, kind) {
(FullCodeCharKind::Normal, FullCodeCharKind::InString) => {
FullCodeCharKind::StartString
}
(FullCodeCharKind::InString, FullCodeCharKind::Normal) => {
FullCodeCharKind::EndString
}
(FullCodeCharKind::InComment, FullCodeCharKind::InStringCommented) => {
FullCodeCharKind::StartStringCommented
}
(FullCodeCharKind::InStringCommented, FullCodeCharKind::InComment) => {
FullCodeCharKind::EndStringCommented
}
_ => kind,
};
break;
} else {
line.push(c);
}
line.push(c);
}

// Workaround for CRLF newline.
Expand Down Expand Up @@ -1382,7 +1431,12 @@ impl<'a> Iterator for UngroupedCommentCodeSlices<'a> {
}
FullCodeCharKind::StartComment => {
// Consume the whole comment
while let Some((FullCodeCharKind::InComment, (_, _))) = self.iter.next() {}
loop {
match self.iter.next() {
Some((kind, ..)) if kind.inside_comment() => continue,
_ => break,
}
}
}
_ => panic!(),
}
Expand Down
18 changes: 13 additions & 5 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use syntax::source_map::{BytePos, Span, NO_EXPANSION};
use syntax_pos::Mark;

use comment::{filter_normal_code, CharClasses, FullCodeCharKind, LineClasses};
use config::Config;
use config::{Config, Version};
use rewrite::RewriteContext;
use shape::{Indent, Shape};

Expand Down Expand Up @@ -527,18 +527,26 @@ pub fn trim_left_preserve_layout(orig: &str, indent: Indent, config: &Config) ->
Some(get_prefix_space_width(config, &line))
};

let line = if veto_trim || (kind.is_string() && !line.ends_with('\\')) {
veto_trim = kind.is_string() && !line.ends_with('\\');
let new_veto_trim_value = (kind.is_string()
|| (config.version() == Version::Two && kind.is_commented_string()))
&& !line.ends_with('\\');
let line = if veto_trim || new_veto_trim_value {
veto_trim = new_veto_trim_value;
trimmed = false;
line
} else {
line.trim().to_owned()
};
trimmed_lines.push((trimmed, line, prefix_space_width));

// When computing the minimum, do not consider lines within a string.
// The reason is there is a veto against trimming and indenting such lines
// Because there is a veto against trimming and indenting lines within a string,
// such lines should not be taken into account when computing the minimum.
match kind {
FullCodeCharKind::InStringCommented | FullCodeCharKind::EndStringCommented
if config.version() == Version::Two =>
{
None
}
FullCodeCharKind::InString | FullCodeCharKind::EndString => None,
_ => prefix_space_width,
}
Expand Down
13 changes: 0 additions & 13 deletions tests/source/issue-3132.rs

This file was deleted.

12 changes: 12 additions & 0 deletions tests/source/issue-3270/one.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// rustfmt-version: One

pub fn main() {
/* let s = String::from(
"
hello
world
",
); */

assert_eq!(s, "\nhello\nworld\n");
}
12 changes: 12 additions & 0 deletions tests/source/issue-3270/two.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// rustfmt-version: Two

pub fn main() {
/* let s = String::from(
"
hello
world
",
); */

assert_eq!(s, "\nhello\nworld\n");
}
12 changes: 7 additions & 5 deletions tests/target/issue-3132.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
// rustfmt-version: Two

fn test() {
/*
a
*/
let x = 42;
/*
aaa
"line 1
line 2
line 3"
*/
aaa
"line 1
line 2
line 3"
*/
let x = 42;
}
12 changes: 12 additions & 0 deletions tests/target/issue-3270/one.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// rustfmt-version: One

pub fn main() {
/* let s = String::from(
"
hello
world
",
); */

assert_eq!(s, "\nhello\nworld\n");
}
12 changes: 12 additions & 0 deletions tests/target/issue-3270/two.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// rustfmt-version: Two

pub fn main() {
/* let s = String::from(
"
hello
world
",
); */

assert_eq!(s, "\nhello\nworld\n");
}
13 changes: 13 additions & 0 deletions tests/target/issue-3270/wrap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// rustfmt-wrap_comments: true
// rustfmt-version: Two

// check that a line below max_width does not get over the limit when wrapping
// it in a block comment
fn func() {
let x = 42;
/*
let something = "one line line line line line line line line line line line line line
two lines
three lines";
*/
}

0 comments on commit d2e91b5

Please sign in to comment.