Give a deprecation warning on rustfmt_skip and an error on rustfmt:: other than skip

This commit is contained in:
Nick Cameron 2018-05-14 18:01:53 +12:00
parent 51f566062f
commit 390a284851
4 changed files with 102 additions and 25 deletions

View file

@ -37,6 +37,7 @@ extern crate term;
extern crate toml;
extern crate unicode_segmentation;
use std::cell::RefCell;
use std::collections::HashMap;
use std::fmt;
use std::io::{self, stdout, Write};
@ -47,7 +48,7 @@ use std::time::Duration;
use syntax::ast;
pub use syntax::codemap::FileName;
use syntax::codemap::{CodeMap, FilePathMapping};
use syntax::codemap::{CodeMap, FilePathMapping, Span};
use syntax::errors::emitter::{ColorConfig, EmitterWriter};
use syntax::errors::{DiagnosticBuilder, Handler};
use syntax::parse::{self, ParseSess};
@ -125,9 +126,14 @@ pub enum ErrorKind {
// License check has failed
#[fail(display = "license check failed")]
LicenseCheck,
// Used deprecated skip attribute
#[fail(display = "`rustfmt_skip` is deprecated; use `rustfmt::skip`")]
DeprecatedAttr,
// Used a rustfmt:: attribute other than skip
#[fail(display = "invalid attribute")]
BadAttr,
}
// Formatting errors that are identified *after* rustfmt has run.
struct FormattingError {
line: usize,
kind: ErrorKind,
@ -137,11 +143,28 @@ struct FormattingError {
}
impl FormattingError {
fn from_span(span: &Span, codemap: &CodeMap, kind: ErrorKind) -> FormattingError {
FormattingError {
line: codemap.lookup_char_pos(span.lo()).line,
kind,
is_comment: false,
is_string: false,
line_buffer: codemap
.span_to_lines(*span)
.ok()
.and_then(|fl| {
fl.file
.get_line(fl.lines[0].line_index)
.map(|l| l.into_owned())
})
.unwrap_or_else(|| String::new()),
}
}
fn msg_prefix(&self) -> &str {
match self.kind {
ErrorKind::LineOverflow(..) | ErrorKind::TrailingWhitespace => "internal error:",
ErrorKind::LicenseCheck => "error:",
ErrorKind::BadIssue(_) => "warning:",
ErrorKind::LicenseCheck | ErrorKind::BadAttr => "error:",
ErrorKind::BadIssue(_) | ErrorKind::DeprecatedAttr => "warning:",
}
}
@ -158,7 +181,7 @@ impl FormattingError {
fn format_len(&self) -> (usize, usize) {
match self.kind {
ErrorKind::LineOverflow(found, max) => (max, found - max),
ErrorKind::TrailingWhitespace => {
ErrorKind::TrailingWhitespace | ErrorKind::DeprecatedAttr | ErrorKind::BadAttr => {
let trailing_ws_start = self
.line_buffer
.rfind(|c: char| !c.is_whitespace())
@ -174,20 +197,30 @@ impl FormattingError {
}
}
#[derive(Clone)]
pub struct FormatReport {
// Maps stringified file paths to their associated formatting errors.
file_error_map: HashMap<FileName, Vec<FormattingError>>,
file_error_map: Rc<RefCell<HashMap<FileName, Vec<FormattingError>>>>,
}
impl FormatReport {
fn new() -> FormatReport {
FormatReport {
file_error_map: HashMap::new(),
file_error_map: Rc::new(RefCell::new(HashMap::new())),
}
}
fn append(&self, f: FileName, mut v: Vec<FormattingError>) {
self.file_error_map
.borrow_mut()
.entry(f)
.and_modify(|fe| fe.append(&mut v))
.or_insert(v);
}
fn warning_count(&self) -> usize {
self.file_error_map
.borrow()
.iter()
.map(|(_, errors)| errors.len())
.sum()
@ -201,7 +234,7 @@ impl FormatReport {
&self,
mut t: Box<term::Terminal<Output = io::Stderr>>,
) -> Result<(), term::Error> {
for (file, errors) in &self.file_error_map {
for (file, errors) in &*self.file_error_map.borrow() {
for error in errors {
let prefix_space_len = error.line.to_string().len();
let prefix_spaces = " ".repeat(1 + prefix_space_len);
@ -247,7 +280,7 @@ impl FormatReport {
}
}
if !self.file_error_map.is_empty() {
if !self.file_error_map.borrow().is_empty() {
t.attr(term::Attr::Bold)?;
write!(t, "warning: ")?;
t.reset()?;
@ -271,7 +304,7 @@ fn target_str(space_len: usize, target_len: usize) -> String {
impl fmt::Display for FormatReport {
// Prints all the formatting errors.
fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> {
for (file, errors) in &self.file_error_map {
for (file, errors) in &*self.file_error_map.borrow() {
for error in errors {
let prefix_space_len = error.line.to_string().len();
let prefix_spaces = " ".repeat(1 + prefix_space_len);
@ -310,7 +343,7 @@ impl fmt::Display for FormatReport {
)?;
}
}
if !self.file_error_map.is_empty() {
if !self.file_error_map.borrow().is_empty() {
writeln!(
fmt,
"warning: rustfmt may have failed to format. See previous {} errors.",
@ -336,10 +369,11 @@ fn format_ast<F>(
parse_session: &mut ParseSess,
main_file: &FileName,
config: &Config,
report: FormatReport,
mut after_file: F,
) -> Result<(FileMap, bool), io::Error>
where
F: FnMut(&FileName, &mut String, &[(usize, usize)]) -> Result<bool, io::Error>,
F: FnMut(&FileName, &mut String, &[(usize, usize)], &FormatReport) -> Result<bool, io::Error>,
{
let mut result = FileMap::new();
// diff mode: check if any files are differing
@ -357,7 +391,8 @@ where
.file;
let big_snippet = filemap.src.as_ref().unwrap();
let snippet_provider = SnippetProvider::new(filemap.start_pos, big_snippet);
let mut visitor = FmtVisitor::from_codemap(parse_session, config, &snippet_provider);
let mut visitor =
FmtVisitor::from_codemap(parse_session, config, &snippet_provider, report.clone());
// Format inner attributes if available.
if !krate.attrs.is_empty() && path == *main_file {
visitor.skip_empty_lines(filemap.end_pos);
@ -377,8 +412,7 @@ where
::utils::count_newlines(&visitor.buffer)
);
let filename = path.clone();
has_diff |= match after_file(&filename, &mut visitor.buffer, &visitor.skipped_range) {
has_diff |= match after_file(&path, &mut visitor.buffer, &visitor.skipped_range, &report) {
Ok(result) => result,
Err(e) => {
// Create a new error with path_str to help users see which files failed
@ -387,7 +421,7 @@ where
}
};
result.push((filename, visitor.buffer));
result.push((path.clone(), visitor.buffer));
}
Ok((result, has_diff))
@ -426,7 +460,7 @@ fn format_lines(
name: &FileName,
skipped_range: &[(usize, usize)],
config: &Config,
report: &mut FormatReport,
report: &FormatReport,
) {
let mut trims = vec![];
let mut last_wspace: Option<usize> = None;
@ -540,7 +574,7 @@ fn format_lines(
}
}
report.file_error_map.insert(name.clone(), errors);
report.append(name.clone(), errors);
}
fn parse_input<'sess>(
@ -757,19 +791,20 @@ fn format_input_inner<T: Write>(
));
parse_session.span_diagnostic = Handler::with_emitter(true, false, silent_emitter);
let mut report = FormatReport::new();
let report = FormatReport::new();
let format_result = format_ast(
&krate,
&mut parse_session,
&main_file,
config,
|file_name, file, skipped_range| {
report.clone(),
|file_name, file, skipped_range, report| {
// For some reason, the codemap does not include terminating
// newlines so we must add one on for each file. This is sad.
filemap::append_newline(file);
format_lines(file, file_name, skipped_range, config, &mut report);
format_lines(file, file_name, skipped_range, config, report);
if let Some(ref mut out) = out {
return filemap::write_file(file, file_name, out, config);

View file

@ -16,6 +16,7 @@ use syntax::parse::ParseSess;
use config::{Config, IndentStyle};
use shape::Shape;
use visitor::SnippetProvider;
use FormatReport;
use std::cell::RefCell;
@ -38,6 +39,7 @@ pub struct RewriteContext<'a> {
// When rewriting chain, veto going multi line except the last element
pub force_one_line_chain: RefCell<bool>,
pub snippet_provider: &'a SnippetProvider<'a>,
pub report: FormatReport,
}
impl<'a> RewriteContext<'a> {

View file

@ -22,8 +22,8 @@ use config::Color;
use rewrite::RewriteContext;
use shape::Shape;
const DEPR_SKIP_ANNOTATION: &str = "rustfmt_skip";
const SKIP_ANNOTATION: &str = "rustfmt::skip";
pub const DEPR_SKIP_ANNOTATION: &str = "rustfmt_skip";
pub const SKIP_ANNOTATION: &str = "rustfmt::skip";
// Computes the length of a string's last line, minus offset.
pub fn extra_offset(text: &str, shape: Shape) -> usize {

View file

@ -26,7 +26,11 @@ use macros::{rewrite_macro, rewrite_macro_def, MacroPosition};
use rewrite::{Rewrite, RewriteContext};
use shape::{Indent, Shape};
use spanned::Spanned;
use utils::{self, contains_skip, count_newlines, inner_attributes, mk_sp, ptr_vec_to_ref_vec};
use utils::{
self, contains_skip, count_newlines, inner_attributes, mk_sp, ptr_vec_to_ref_vec,
DEPR_SKIP_ANNOTATION,
};
use {ErrorKind, FormatReport, FormattingError};
use std::cell::RefCell;
@ -66,6 +70,7 @@ pub struct FmtVisitor<'a> {
pub snippet_provider: &'a SnippetProvider<'a>,
pub line_number: usize,
pub skipped_range: Vec<(usize, usize)>,
pub report: FormatReport,
}
impl<'b, 'a: 'b> FmtVisitor<'a> {
@ -552,13 +557,19 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
}
pub fn from_context(ctx: &'a RewriteContext) -> FmtVisitor<'a> {
FmtVisitor::from_codemap(ctx.parse_session, ctx.config, ctx.snippet_provider)
FmtVisitor::from_codemap(
ctx.parse_session,
ctx.config,
ctx.snippet_provider,
ctx.report.clone(),
)
}
pub fn from_codemap(
parse_session: &'a ParseSess,
config: &'a Config,
snippet_provider: &'a SnippetProvider,
report: FormatReport,
) -> FmtVisitor<'a> {
FmtVisitor {
parse_session,
@ -571,6 +582,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
snippet_provider,
line_number: 0,
skipped_range: vec![],
report,
}
}
@ -584,6 +596,33 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
// Returns true if we should skip the following item.
pub fn visit_attrs(&mut self, attrs: &[ast::Attribute], style: ast::AttrStyle) -> bool {
for attr in attrs {
if attr.name() == DEPR_SKIP_ANNOTATION {
let file_name = self.codemap.span_to_filename(attr.span);
self.report.append(
file_name,
vec![FormattingError::from_span(
&attr.span,
&self.codemap,
ErrorKind::DeprecatedAttr,
)],
);
} else if attr.path.segments[0].ident.to_string() == "rustfmt" {
if attr.path.segments.len() == 1
|| attr.path.segments[1].ident.to_string() != "skip"
{
let file_name = self.codemap.span_to_filename(attr.span);
self.report.append(
file_name,
vec![FormattingError::from_span(
&attr.span,
&self.codemap,
ErrorKind::BadAttr,
)],
);
}
}
}
if contains_skip(attrs) {
return true;
}
@ -711,6 +750,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
is_if_else_block: RefCell::new(false),
force_one_line_chain: RefCell::new(false),
snippet_provider: self.snippet_provider,
report: self.report.clone(),
}
}
}