Merge pull request #2257 from topecongiro/error-on-line-overflow-strings

Add error_on_line_overflow_strings config option
This commit is contained in:
Nick Cameron 2017-12-11 16:41:15 +13:00 committed by GitHub
commit 644b60ad85
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 124 additions and 79 deletions

View File

@ -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<Color>,
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
}

View File

@ -585,7 +585,7 @@ pub fn remove_trailing_white_spaces(text: &str) -> String {
buffer
}
struct CharClasses<T>
pub struct CharClasses<T>
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<T> {
pub fn new(base: T) -> CharClasses<T> {
CharClasses {
base: base.peekable(),
status: CharClassesStatus::Normal,

View File

@ -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,

View File

@ -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<char> = 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(),
});
}

View File

@ -1,7 +0,0 @@
// rustfmt-error_on_line_overflow_comments: false
// Error on line overflow comment
// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
fn main() {
// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
}

View File

@ -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 = "
";
}