diff --git a/src/bin/rustfmt.rs b/src/bin/rustfmt.rs index 9bf26887881..aebd19d8e4b 100644 --- a/src/bin/rustfmt.rs +++ b/src/bin/rustfmt.rs @@ -20,7 +20,7 @@ use std::io::{self, Read, Write}; use std::path::{Path, PathBuf}; use std::str::FromStr; -use getopts::{HasArg, Matches, Occur, Options}; +use getopts::{Matches, Options}; use rustfmt::{run, Input, Summary}; use rustfmt::file_lines::FileLines; @@ -63,6 +63,7 @@ struct CliOptions { color: Option, file_lines: FileLines, // Default is all lines in all files. unstable_features: bool, + error_on_unformatted: bool, } impl CliOptions { @@ -104,6 +105,10 @@ impl CliOptions { options.file_lines = file_lines.parse()?; } + if matches.opt_present("error-on-unformatted") { + options.error_on_unformatted = true; + } + Ok(options) } @@ -112,6 +117,7 @@ impl CliOptions { config.set().verbose(self.verbose); config.set().file_lines(self.file_lines); config.set().unstable_features(self.unstable_features); + config.set().error_on_unformatted(self.error_on_unformatted); if let Some(write_mode) = self.write_mode { config.set().write_mode(write_mode); } @@ -135,47 +141,18 @@ fn match_cli_path_or_file( fn make_opts() -> Options { let mut opts = Options::new(); - opts.optflag("h", "help", "show this message"); - opts.optflag("V", "version", "show version information"); - opts.optflag("v", "verbose", "print verbose output"); - opts.optopt( - "", - "write-mode", - "how to write output (not usable when piping from stdin)", - "[replace|overwrite|display|plain|diff|coverage|checkstyle]", - ); + + // Sorted in alphabetical order. opts.optopt( "", "color", - "use colored output (if supported)", + "Use colored output (if supported)", "[always|never|auto]", ); - opts.optflag("", "skip-children", "don't reformat child modules"); - - opts.optflag( - "", - "unstable-features", - "Enables unstable features. Only available on nightly channel", - ); - opts.optflag( "", "config-help", - "show details of rustfmt configuration options", - ); - opts.opt( - "", - "dump-default-config", - "Dumps default configuration to PATH. PATH defaults to stdout, if omitted.", - "PATH", - HasArg::Maybe, - Occur::Optional, - ); - opts.optopt( - "", - "dump-minimal-config", - "Dumps configuration options that were checked during formatting to a file.", - "PATH", + "Show details of rustfmt configuration options", ); opts.optopt( "", @@ -184,12 +161,45 @@ fn make_opts() -> Options { found reverts to the input file path", "[Path for the configuration file]", ); + opts.optopt( + "", + "dump-default-config", + "Dumps default configuration to PATH. PATH defaults to stdout, if omitted.", + "PATH", + ); + opts.optopt( + "", + "dump-minimal-config", + "Dumps configuration options that were checked during formatting to a file.", + "PATH", + ); + opts.optflag( + "", + "error-on-unformatted", + "Error if unable to get comments or string literals within max_width, \ + or they are left with trailing whitespaces", + ); opts.optopt( "", "file-lines", "Format specified line ranges. See README for more detail on the JSON format.", "JSON", ); + opts.optflag("h", "help", "Show this message"); + opts.optflag("", "skip-children", "Don't reformat child modules"); + opts.optflag( + "", + "unstable-features", + "Enables unstable features. Only available on nightly channel", + ); + opts.optflag("v", "verbose", "Print verbose output"); + opts.optflag("V", "version", "Show version information"); + opts.optopt( + "", + "write-mode", + "How to write output (not usable when piping from stdin)", + "[replace|overwrite|display|plain|diff|coverage|checkstyle]", + ); opts } diff --git a/src/comment.rs b/src/comment.rs index 4a44c77a4a2..a9fc31ea0b0 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -585,7 +585,7 @@ pub fn remove_trailing_white_spaces(text: &str) -> String { buffer } -struct CharClasses +pub struct CharClasses where T: Iterator, T::Item: RichChar, @@ -594,7 +594,7 @@ where status: CharClassesStatus, } -trait RichChar { +pub trait RichChar { fn get_char(&self) -> char; } @@ -610,6 +610,12 @@ impl RichChar for (usize, char) { } } +impl RichChar for (char, usize) { + fn get_char(&self) -> char { + self.0 + } +} + #[derive(PartialEq, Eq, Debug, Clone, Copy)] enum CharClassesStatus { Normal, @@ -639,7 +645,7 @@ pub enum CodeCharKind { /// describing opening and closing of comments for ease when chunking /// code from tagged characters #[derive(PartialEq, Eq, Debug, Clone, Copy)] -enum FullCodeCharKind { +pub enum FullCodeCharKind { Normal, /// The first character of a comment, there is only one for a comment (always '/') StartComment, @@ -653,7 +659,7 @@ enum FullCodeCharKind { } impl FullCodeCharKind { - fn is_comment(&self) -> bool { + pub fn is_comment(&self) -> bool { match *self { FullCodeCharKind::StartComment | FullCodeCharKind::InComment @@ -662,6 +668,10 @@ impl FullCodeCharKind { } } + pub fn is_string(&self) -> bool { + *self == FullCodeCharKind::InString + } + fn to_codecharkind(&self) -> CodeCharKind { if self.is_comment() { CodeCharKind::Comment @@ -676,7 +686,7 @@ where T: Iterator, T::Item: RichChar, { - fn new(base: T) -> CharClasses { + pub fn new(base: T) -> CharClasses { CharClasses { base: base.peekable(), status: CharClassesStatus::Normal, diff --git a/src/config.rs b/src/config.rs index 6dc1056b5af..ed44df022a9 100644 --- a/src/config.rs +++ b/src/config.rs @@ -682,8 +682,9 @@ create_config! { disable_all_formatting: bool, false, false, "Don't reformat anything"; skip_children: bool, false, false, "Don't reformat out of line modules"; error_on_line_overflow: bool, true, false, "Error if unable to get all lines within max_width"; - error_on_line_overflow_comments: bool, true, false, - "Error if unable to get comments within max_width"; + error_on_unformatted: bool, false, false, + "Error if unable to get comments or string literals within max_width, \ + or they are left with trailing whitespaces"; report_todo: ReportTactic, ReportTactic::Never, false, "Report all, none or unnumbered occurrences of TODO in source file comments"; report_fixme: ReportTactic, ReportTactic::Never, false, diff --git a/src/lib.rs b/src/lib.rs index a300e961421..b00caf40cd5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -39,6 +39,7 @@ use syntax::codemap::{CodeMap, FilePathMapping}; use syntax::parse::{self, ParseSess}; use checkstyle::{output_footer, output_header}; +use comment::{CharClasses, FullCodeCharKind}; use config::Config; use filemap::FileMap; use issues::{BadIssueSeeker, Issue}; @@ -76,6 +77,7 @@ mod patterns; mod summary; mod vertical; +#[derive(Clone, Copy)] pub enum ErrorKind { // Line has exceeded character limit (found, maximum) LineOverflow(usize, usize), @@ -104,6 +106,7 @@ pub struct FormattingError { line: usize, kind: ErrorKind, is_comment: bool, + is_string: bool, line_buffer: String, } @@ -116,12 +119,11 @@ impl FormattingError { } fn msg_suffix(&self) -> &str { - match self.kind { - ErrorKind::LineOverflow(..) if self.is_comment => { - "use `error_on_line_overflow_comments = false` to suppress \ - the warning against line comments\n" - } - _ => "", + if self.is_comment || self.is_string { + "set `error_on_unformatted = false` to suppress \ + the warning against comments or string literals\n" + } else { + "" } } @@ -363,6 +365,25 @@ fn is_skipped_line(line_number: usize, skipped_range: &[(usize, usize)]) -> bool .any(|&(lo, hi)| lo <= line_number && line_number <= hi) } +fn should_report_error( + config: &Config, + char_kind: FullCodeCharKind, + is_string: bool, + error_kind: ErrorKind, +) -> bool { + let allow_error_report = if char_kind.is_comment() || is_string { + config.error_on_unformatted() + } else { + true + }; + + match error_kind { + ErrorKind::LineOverflow(..) => config.error_on_line_overflow() && allow_error_report, + ErrorKind::TrailingWhitespace => allow_error_report, + _ => true, + } +} + // Formatting done on a char by char or line by line basis. // FIXME(#209) warn on bad license // FIXME(#20) other stuff for parity with make tidy @@ -381,19 +402,15 @@ fn format_lines( let mut newline_count = 0; let mut errors = vec![]; let mut issue_seeker = BadIssueSeeker::new(config.report_todo(), config.report_fixme()); - let mut prev_char: Option = None; - let mut is_comment = false; let mut line_buffer = String::with_capacity(config.max_width() * 2); - let mut b = 0; + let mut is_string = false; // true if the current line contains a string literal. + let mut format_line = config.file_lines().contains_line(name, cur_line); - for c in text.chars() { - b += 1; + for (kind, (b, c)) in CharClasses::new(text.chars().enumerate()) { if c == '\r' { continue; } - let format_line = config.file_lines().contains_line(name, cur_line as usize); - if format_line { // Add warnings for bad todos/ fixmes if let Some(issue) = issue_seeker.inspect(c) { @@ -401,6 +418,7 @@ fn format_lines( line: cur_line, kind: ErrorKind::BadIssue(issue), is_comment: false, + is_string: false, line_buffer: String::new(), }); } @@ -409,20 +427,23 @@ fn format_lines( if c == '\n' { if format_line { // Check for (and record) trailing whitespace. - if let Some(lw) = last_wspace { - trims.push((cur_line, lw, b, line_buffer.clone())); + if let Some(..) = last_wspace { + if should_report_error(config, kind, is_string, ErrorKind::TrailingWhitespace) { + trims.push((cur_line, kind, line_buffer.clone())); + } line_len -= 1; } // Check for any line width errors we couldn't correct. - let report_error_on_line_overflow = config.error_on_line_overflow() - && !is_skipped_line(cur_line, skipped_range) - && (config.error_on_line_overflow_comments() || !is_comment); - if report_error_on_line_overflow && line_len > config.max_width() { + let error_kind = ErrorKind::LineOverflow(line_len, config.max_width()); + if line_len > config.max_width() && !is_skipped_line(cur_line, skipped_range) + && should_report_error(config, kind, is_string, error_kind) + { errors.push(FormattingError { line: cur_line, - kind: ErrorKind::LineOverflow(line_len, config.max_width()), - is_comment: is_comment, + kind: error_kind, + is_comment: kind.is_comment(), + is_string: is_string, line_buffer: line_buffer.clone(), }); } @@ -430,11 +451,11 @@ fn format_lines( line_len = 0; cur_line += 1; + format_line = config.file_lines().contains_line(name, cur_line); newline_count += 1; last_wspace = None; - prev_char = None; - is_comment = false; line_buffer.clear(); + is_string = false; } else { newline_count = 0; line_len += 1; @@ -442,16 +463,13 @@ fn format_lines( if last_wspace.is_none() { last_wspace = Some(b); } - } else if c == '/' { - if let Some('/') = prev_char { - is_comment = true; - } - last_wspace = None; } else { last_wspace = None; } - prev_char = Some(c); line_buffer.push(c); + if kind.is_string() { + is_string = true; + } } } @@ -461,12 +479,13 @@ fn format_lines( text.truncate(line); } - for &(l, _, _, ref b) in &trims { + for &(l, kind, ref b) in &trims { if !is_skipped_line(l, skipped_range) { errors.push(FormattingError { line: l, kind: ErrorKind::TrailingWhitespace, - is_comment: false, + is_comment: kind.is_comment(), + is_string: kind.is_string(), line_buffer: b.clone(), }); } diff --git a/tests/target/configs-error_on_line_overflow_comment-false.rs b/tests/target/configs-error_on_line_overflow_comment-false.rs deleted file mode 100644 index 9fd9e01e274..00000000000 --- a/tests/target/configs-error_on_line_overflow_comment-false.rs +++ /dev/null @@ -1,7 +0,0 @@ -// rustfmt-error_on_line_overflow_comments: false -// Error on line overflow comment - -// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -fn main() { - // aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -} diff --git a/tests/target/configs-error_on_unformatted-false.rs b/tests/target/configs-error_on_unformatted-false.rs new file mode 100644 index 00000000000..6a78374e2a2 --- /dev/null +++ b/tests/target/configs-error_on_unformatted-false.rs @@ -0,0 +1,12 @@ +// rustfmt-error_on_unformatted: false +// Error on line overflow comment or string literals. + +// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +fn main() { + // aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + + let x = " "; + let a = " + +"; +}