From a5114e5eebb62a4605098eec2aa7b908c246ccf9 Mon Sep 17 00:00:00 2001 From: Michele d'Amico Date: Wed, 3 Jul 2019 17:54:00 +0200 Subject: [PATCH 01/24] #3665 TEST: Test files for system and idempotent --- tests/source/issue-3665/lib.rs | 33 +++++++++++++++++++ tests/source/issue-3665/not_skip_attribute.rs | 4 +++ tests/source/issue-3665/sub_mod.rs | 14 ++++++++ tests/target/issue-3665/lib.rs | 31 +++++++++++++++++ tests/target/issue-3665/not_skip_attribute.rs | 4 +++ tests/target/issue-3665/sub_mod.rs | 13 ++++++++ 6 files changed, 99 insertions(+) create mode 100644 tests/source/issue-3665/lib.rs create mode 100644 tests/source/issue-3665/not_skip_attribute.rs create mode 100644 tests/source/issue-3665/sub_mod.rs create mode 100644 tests/target/issue-3665/lib.rs create mode 100644 tests/target/issue-3665/not_skip_attribute.rs create mode 100644 tests/target/issue-3665/sub_mod.rs diff --git a/tests/source/issue-3665/lib.rs b/tests/source/issue-3665/lib.rs new file mode 100644 index 00000000000..e049fbc5680 --- /dev/null +++ b/tests/source/issue-3665/lib.rs @@ -0,0 +1,33 @@ +#![rustfmt::skip::attributes(skip_mod_attr)] + +mod sub_mod; + +#[rustfmt::skip::attributes(other, skip_attr)] +fn main() { + #[other(should, +skip, + this, format)] + struct S {} + + #[skip_attr(should, skip, +this, format,too)] + fn doesnt_mater() {} + + #[skip_mod_attr(should, skip, +this, format, + enerywhere)] + fn more() {} + + #[not_skip(not, +skip, me)] + struct B {} +} + +#[other(should, not, skip, +this, format, here)] +fn foo() {} + +#[skip_mod_attr(should, skip, +this, format,in, master, + and, sub, module)] +fn bar() {} diff --git a/tests/source/issue-3665/not_skip_attribute.rs b/tests/source/issue-3665/not_skip_attribute.rs new file mode 100644 index 00000000000..14985259a50 --- /dev/null +++ b/tests/source/issue-3665/not_skip_attribute.rs @@ -0,0 +1,4 @@ +#![this::is::not::skip::attribute(ouch)] + +#[ouch(not, skip, me)] +fn main() {} diff --git a/tests/source/issue-3665/sub_mod.rs b/tests/source/issue-3665/sub_mod.rs new file mode 100644 index 00000000000..75fb24b4a65 --- /dev/null +++ b/tests/source/issue-3665/sub_mod.rs @@ -0,0 +1,14 @@ +#[rustfmt::skip::attributes(more_skip)] +#[more_skip(should, + skip, +this, format)] +fn foo() {} + +#[skip_mod_attr(should, skip, +this, format,in, master, + and, sub, module)] +fn bar() {} + +#[skip_attr(should, not, + skip, this, attribute, here)] +fn baz() {} diff --git a/tests/target/issue-3665/lib.rs b/tests/target/issue-3665/lib.rs new file mode 100644 index 00000000000..c313f320368 --- /dev/null +++ b/tests/target/issue-3665/lib.rs @@ -0,0 +1,31 @@ +#![rustfmt::skip::attributes(skip_mod_attr)] + +mod sub_mod; + +#[rustfmt::skip::attributes(other, skip_attr)] +fn main() { + #[other(should, +skip, + this, format)] + struct S {} + + #[skip_attr(should, skip, +this, format,too)] + fn doesnt_mater() {} + + #[skip_mod_attr(should, skip, +this, format, + enerywhere)] + fn more() {} + + #[not_skip(not, skip, me)] + struct B {} +} + +#[other(should, not, skip, this, format, here)] +fn foo() {} + +#[skip_mod_attr(should, skip, +this, format,in, master, + and, sub, module)] +fn bar() {} diff --git a/tests/target/issue-3665/not_skip_attribute.rs b/tests/target/issue-3665/not_skip_attribute.rs new file mode 100644 index 00000000000..a4e8b948736 --- /dev/null +++ b/tests/target/issue-3665/not_skip_attribute.rs @@ -0,0 +1,4 @@ +#![this::is::not::skip::attribute(ouch)] + +#[ouch(not, skip, me)] +fn main() {} diff --git a/tests/target/issue-3665/sub_mod.rs b/tests/target/issue-3665/sub_mod.rs new file mode 100644 index 00000000000..30a2b0fd9d9 --- /dev/null +++ b/tests/target/issue-3665/sub_mod.rs @@ -0,0 +1,13 @@ +#[rustfmt::skip::attributes(more_skip)] +#[more_skip(should, + skip, +this, format)] +fn foo() {} + +#[skip_mod_attr(should, skip, +this, format,in, master, + and, sub, module)] +fn bar() {} + +#[skip_attr(should, not, skip, this, attribute, here)] +fn baz() {} From 4c30941695212f4b1cd9289e5bc5fa07819d58eb Mon Sep 17 00:00:00 2001 From: Michele d'Amico Date: Wed, 3 Jul 2019 18:01:36 +0200 Subject: [PATCH 02/24] #3665 WIP: mock up should_skip logic (hardcoded tests values) --- src/attr.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/attr.rs b/src/attr.rs index 8497917ab06..6074502fa1e 100644 --- a/src/attr.rs +++ b/src/attr.rs @@ -319,9 +319,13 @@ impl Rewrite for ast::Attribute { if self.is_sugared_doc { rewrite_doc_comment(snippet, shape.comment(context.config), context.config) } else { + let should_skip = self + .ident() + .map(|s| s.name.as_str() == "skip_attr" || s.name.as_str() == "skip_mod_attr" + ).unwrap_or(false); let prefix = attr_prefix(self); - if contains_comment(snippet) { + if should_skip || contains_comment(snippet) { return Some(snippet.to_owned()); } From 34508acf265989db53ad8d88976e7fc3f78e50e0 Mon Sep 17 00:00:00 2001 From: Michele d'Amico Date: Wed, 3 Jul 2019 18:02:44 +0200 Subject: [PATCH 03/24] #3665 WIP: Accept "attributes" as skip attribute too --- src/visitor.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/visitor.rs b/src/visitor.rs index 641ff1b62fd..a6aeced74be 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -728,7 +728,9 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { match segments.len() { 2 => segments[1].ident.to_string() != "skip", 3 => { - segments[1].ident.to_string() != "skip" || segments[2].ident.to_string() != "macros" + segments[1].ident.to_string() != "skip" + || (segments[2].ident.to_string() != "macros" + && segments[2].ident.to_string() != "attributes") } _ => false, } From ebb933ab8d0cfa2aa322e3ca120da94d2de4e6e8 Mon Sep 17 00:00:00 2001 From: michele Date: Wed, 3 Jul 2019 22:39:26 +0200 Subject: [PATCH 04/24] #3665: fix format --- src/attr.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/attr.rs b/src/attr.rs index 6074502fa1e..6a176e072f3 100644 --- a/src/attr.rs +++ b/src/attr.rs @@ -321,8 +321,8 @@ impl Rewrite for ast::Attribute { } else { let should_skip = self .ident() - .map(|s| s.name.as_str() == "skip_attr" || s.name.as_str() == "skip_mod_attr" - ).unwrap_or(false); + .map(|s| s.name.as_str() == "skip_attr" || s.name.as_str() == "skip_mod_attr") + .unwrap_or(false); let prefix = attr_prefix(self); if should_skip || contains_comment(snippet) { From 8182ec4a813501f603e0874b70525189907f31e9 Mon Sep 17 00:00:00 2001 From: michele Date: Thu, 4 Jul 2019 00:04:12 +0200 Subject: [PATCH 05/24] #3665: Dirty but works --- src/attr.rs | 10 +++++++++- src/formatting.rs | 6 +++++- src/rewrite.rs | 1 + src/test/mod.rs | 1 + src/utils.rs | 20 ++++++++++++++++++++ src/visitor.rs | 16 ++++++++++++++-- 6 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/attr.rs b/src/attr.rs index 6a176e072f3..e1055d9793e 100644 --- a/src/attr.rs +++ b/src/attr.rs @@ -321,7 +321,15 @@ impl Rewrite for ast::Attribute { } else { let should_skip = self .ident() - .map(|s| s.name.as_str() == "skip_attr" || s.name.as_str() == "skip_mod_attr") + .map(|s| { + context + .skip_attribute_names + .borrow() + .iter() + .filter(|&name| s.name.as_str() == name.as_str()) + .next() + .is_some() + }) .unwrap_or(false); let prefix = attr_prefix(self); diff --git a/src/formatting.rs b/src/formatting.rs index 0faeb5102f2..1dce743410e 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -17,7 +17,7 @@ use crate::comment::{CharClasses, FullCodeCharKind}; use crate::config::{Config, FileName, Verbosity}; use crate::ignore_path::IgnorePathSet; use crate::issues::BadIssueSeeker; -use crate::utils::{count_newlines, get_skip_macro_names}; +use crate::utils::{count_newlines, get_skip_attribute_names, get_skip_macro_names}; use crate::visitor::{FmtVisitor, SnippetProvider}; use crate::{modules, source_file, ErrorKind, FormatReport, Input, Session}; @@ -162,6 +162,10 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> { .skip_macro_names .borrow_mut() .append(&mut get_skip_macro_names(&self.krate.attrs)); + visitor + .skip_attribute_names + .borrow_mut() + .append(&mut get_skip_attribute_names(&self.krate.attrs)); // Format inner attributes if available. if !self.krate.attrs.is_empty() && is_root { diff --git a/src/rewrite.rs b/src/rewrite.rs index 9a955dbd91b..948414915bd 100644 --- a/src/rewrite.rs +++ b/src/rewrite.rs @@ -40,6 +40,7 @@ pub(crate) struct RewriteContext<'a> { pub(crate) macro_rewrite_failure: RefCell, pub(crate) report: FormatReport, pub(crate) skip_macro_names: RefCell>, + pub(crate) skip_attribute_names: RefCell>, } impl<'a> RewriteContext<'a> { diff --git a/src/test/mod.rs b/src/test/mod.rs index 89bde8c0467..0401df290a7 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -25,6 +25,7 @@ const SKIP_FILE_WHITE_LIST: &[&str] = &[ // so we do not want to test this file directly. "configs/skip_children/foo/mod.rs", "issue-3434/no_entry.rs", + "issue-3665/sub_mod.rs", // These files and directory are a part of modules defined inside `cfg_if!`. "cfg_if/mod.rs", "cfg_if/detect", diff --git a/src/utils.rs b/src/utils.rs index afa64e6c045..66d02ae9c82 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -643,6 +643,26 @@ pub(crate) fn get_skip_macro_names(attrs: &[ast::Attribute]) -> Vec { skip_macro_names } +pub(crate) fn get_skip_attribute_names(attrs: &[ast::Attribute]) -> Vec { + let mut skip_attribute_names = vec![]; + for attr in attrs { + // syntax::ast::Path is implemented partialEq + // but it is designed for segments.len() == 1 + if format!("{}", attr.path) != "rustfmt::skip::attributes" { + continue; + } + + if let Some(list) = attr.meta_item_list() { + for nested_meta_item in list { + if let Some(name) = nested_meta_item.ident() { + skip_attribute_names.push(name.to_string()); + } + } + } + } + skip_attribute_names +} + #[cfg(test)] mod test { use super::*; diff --git a/src/visitor.rs b/src/visitor.rs index a6aeced74be..01efd16c230 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -21,8 +21,8 @@ use crate::source_map::{LineRangeUtils, SpanUtils}; use crate::spanned::Spanned; use crate::stmt::Stmt; use crate::utils::{ - self, contains_skip, count_newlines, depr_skip_annotation, get_skip_macro_names, - inner_attributes, mk_sp, ptr_vec_to_ref_vec, rewrite_ident, stmt_expr, + self, contains_skip, count_newlines, depr_skip_annotation, get_skip_attribute_names, + get_skip_macro_names, inner_attributes, mk_sp, ptr_vec_to_ref_vec, rewrite_ident, stmt_expr, }; use crate::{ErrorKind, FormatReport, FormattingError}; @@ -68,6 +68,7 @@ pub(crate) struct FmtVisitor<'a> { pub(crate) macro_rewrite_failure: bool, pub(crate) report: FormatReport, pub(crate) skip_macro_names: RefCell>, + pub(crate) skip_attribute_names: RefCell>, } impl<'a> Drop for FmtVisitor<'a> { @@ -334,6 +335,10 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.skip_macro_names .borrow_mut() .append(&mut get_skip_macro_names(&attrs)); + let temp_skip_attribute_names = self.skip_attribute_names.clone(); + self.skip_attribute_names + .borrow_mut() + .append(&mut get_skip_attribute_names(&attrs)); let should_visit_node_again = match item.node { // For use/extern crate items, skip rewriting attributes but check for a skip attribute. @@ -485,6 +490,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { }; } self.skip_macro_names = temp_skip_macro_names; + self.skip_attribute_names = temp_skip_attribute_names; } pub(crate) fn visit_trait_item(&mut self, ti: &ast::TraitItem) { @@ -643,6 +649,10 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { .skip_macro_names .borrow_mut() .append(&mut ctx.skip_macro_names.borrow().clone()); + visitor + .skip_attribute_names + .borrow_mut() + .append(&mut ctx.skip_attribute_names.borrow().clone()); visitor.set_parent_context(ctx); visitor } @@ -668,6 +678,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { macro_rewrite_failure: false, report, skip_macro_names: RefCell::new(vec![]), + skip_attribute_names: RefCell::new(vec![]), } } @@ -866,6 +877,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { macro_rewrite_failure: RefCell::new(false), report: self.report.clone(), skip_macro_names: self.skip_macro_names.clone(), + skip_attribute_names: self.skip_attribute_names.clone(), } } } From 86e87cb691e405d4584f554501fab7246eed76ef Mon Sep 17 00:00:00 2001 From: Michele d'Amico Date: Thu, 4 Jul 2019 13:50:41 +0200 Subject: [PATCH 06/24] #3665 Refactor: move attribute skip stuff in a dedicated struct --- src/attr.rs | 10 +--------- src/formatting.rs | 11 ++--------- src/macros.rs | 5 ++--- src/rewrite.rs | 4 ++-- src/utils.rs | 50 +++++++++++++++++++++++++++-------------------- src/visitor.rs | 35 +++++++++------------------------ 6 files changed, 45 insertions(+), 70 deletions(-) diff --git a/src/attr.rs b/src/attr.rs index e1055d9793e..79426c663e8 100644 --- a/src/attr.rs +++ b/src/attr.rs @@ -321,15 +321,7 @@ impl Rewrite for ast::Attribute { } else { let should_skip = self .ident() - .map(|s| { - context - .skip_attribute_names - .borrow() - .iter() - .filter(|&name| s.name.as_str() == name.as_str()) - .next() - .is_some() - }) + .map(|s| context.skip_context.attributes_skip(&*s.name.as_str())) .unwrap_or(false); let prefix = attr_prefix(self); diff --git a/src/formatting.rs b/src/formatting.rs index 1dce743410e..62b87b12a5b 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -17,7 +17,7 @@ use crate::comment::{CharClasses, FullCodeCharKind}; use crate::config::{Config, FileName, Verbosity}; use crate::ignore_path::IgnorePathSet; use crate::issues::BadIssueSeeker; -use crate::utils::{count_newlines, get_skip_attribute_names, get_skip_macro_names}; +use crate::utils::count_newlines; use crate::visitor::{FmtVisitor, SnippetProvider}; use crate::{modules, source_file, ErrorKind, FormatReport, Input, Session}; @@ -158,14 +158,7 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> { &snippet_provider, self.report.clone(), ); - visitor - .skip_macro_names - .borrow_mut() - .append(&mut get_skip_macro_names(&self.krate.attrs)); - visitor - .skip_attribute_names - .borrow_mut() - .append(&mut get_skip_attribute_names(&self.krate.attrs)); + visitor.skip_context.update_by_attrs(&self.krate.attrs); // Format inner attributes if available. if !self.krate.attrs.is_empty() && is_root { diff --git a/src/macros.rs b/src/macros.rs index ac00e4452a7..d4f1e880376 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -211,9 +211,8 @@ pub(crate) fn rewrite_macro( position: MacroPosition, ) -> Option { let should_skip = context - .skip_macro_names - .borrow() - .contains(&context.snippet(mac.node.path.span).to_owned()); + .skip_context + .macro_skip(&context.snippet(mac.node.path.span).to_owned()); if should_skip { None } else { diff --git a/src/rewrite.rs b/src/rewrite.rs index 948414915bd..47c6ae4efbd 100644 --- a/src/rewrite.rs +++ b/src/rewrite.rs @@ -8,6 +8,7 @@ use syntax::source_map::{SourceMap, Span}; use crate::config::{Config, IndentStyle}; use crate::shape::Shape; +use crate::utils::SkipContext; use crate::visitor::SnippetProvider; use crate::FormatReport; @@ -39,8 +40,7 @@ pub(crate) struct RewriteContext<'a> { // Used for `format_snippet` pub(crate) macro_rewrite_failure: RefCell, pub(crate) report: FormatReport, - pub(crate) skip_macro_names: RefCell>, - pub(crate) skip_attribute_names: RefCell>, + pub(crate) skip_context: SkipContext, } impl<'a> RewriteContext<'a> { diff --git a/src/utils.rs b/src/utils.rs index 66d02ae9c82..2fc334e7268 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -623,44 +623,52 @@ pub(crate) fn unicode_str_width(s: &str) -> usize { s.width() } -pub(crate) fn get_skip_macro_names(attrs: &[ast::Attribute]) -> Vec { - let mut skip_macro_names = vec![]; - for attr in attrs { - // syntax::ast::Path is implemented partialEq - // but it is designed for segments.len() == 1 - if format!("{}", attr.path) != "rustfmt::skip::macros" { - continue; - } +#[derive(Default, Clone)] +pub(crate) struct SkipContext { + macros: Vec, + attributes: Vec, +} - if let Some(list) = attr.meta_item_list() { - for nested_meta_item in list { - if let Some(name) = nested_meta_item.ident() { - skip_macro_names.push(name.to_string()); - } - } - } +impl SkipContext { + pub(crate) fn update_by_attrs(&mut self, attrs: &[ast::Attribute]) { + self.macros.append(&mut get_skip_names("macros", attrs)); + self.attributes + .append(&mut get_skip_names("attributes", attrs)); + } + + pub(crate) fn update(&mut self, mut other: SkipContext) { + self.macros.append(&mut other.macros); + self.attributes.append(&mut other.attributes); + } + + pub(crate) fn macro_skip(&self, name: &str) -> bool { + self.macros.iter().any(|n| n == name) + } + + pub(crate) fn attributes_skip(&self, name: &str) -> bool { + self.attributes.iter().any(|n| n == name) } - skip_macro_names } -pub(crate) fn get_skip_attribute_names(attrs: &[ast::Attribute]) -> Vec { - let mut skip_attribute_names = vec![]; +fn get_skip_names(kind: &str, attrs: &[ast::Attribute]) -> Vec { + let mut skip_names = vec![]; + let path = format!("rustfmt::skip::{}", kind); for attr in attrs { // syntax::ast::Path is implemented partialEq // but it is designed for segments.len() == 1 - if format!("{}", attr.path) != "rustfmt::skip::attributes" { + if format!("{}", attr.path) != path { continue; } if let Some(list) = attr.meta_item_list() { for nested_meta_item in list { if let Some(name) = nested_meta_item.ident() { - skip_attribute_names.push(name.to_string()); + skip_names.push(name.to_string()); } } } } - skip_attribute_names + skip_names } #[cfg(test)] diff --git a/src/visitor.rs b/src/visitor.rs index 01efd16c230..260007690a3 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -21,8 +21,8 @@ use crate::source_map::{LineRangeUtils, SpanUtils}; use crate::spanned::Spanned; use crate::stmt::Stmt; use crate::utils::{ - self, contains_skip, count_newlines, depr_skip_annotation, get_skip_attribute_names, - get_skip_macro_names, inner_attributes, mk_sp, ptr_vec_to_ref_vec, rewrite_ident, stmt_expr, + self, contains_skip, count_newlines, depr_skip_annotation, inner_attributes, mk_sp, + ptr_vec_to_ref_vec, rewrite_ident, stmt_expr, SkipContext, }; use crate::{ErrorKind, FormatReport, FormattingError}; @@ -67,8 +67,7 @@ pub(crate) struct FmtVisitor<'a> { pub(crate) skipped_range: Vec<(usize, usize)>, pub(crate) macro_rewrite_failure: bool, pub(crate) report: FormatReport, - pub(crate) skip_macro_names: RefCell>, - pub(crate) skip_attribute_names: RefCell>, + pub(crate) skip_context: SkipContext, } impl<'a> Drop for FmtVisitor<'a> { @@ -331,14 +330,8 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { // the AST lumps them all together. let filtered_attrs; let mut attrs = &item.attrs; - let temp_skip_macro_names = self.skip_macro_names.clone(); - self.skip_macro_names - .borrow_mut() - .append(&mut get_skip_macro_names(&attrs)); - let temp_skip_attribute_names = self.skip_attribute_names.clone(); - self.skip_attribute_names - .borrow_mut() - .append(&mut get_skip_attribute_names(&attrs)); + let skip_context_saved = self.skip_context.clone(); + self.skip_context.update_by_attrs(&attrs); let should_visit_node_again = match item.node { // For use/extern crate items, skip rewriting attributes but check for a skip attribute. @@ -489,8 +482,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { } }; } - self.skip_macro_names = temp_skip_macro_names; - self.skip_attribute_names = temp_skip_attribute_names; + self.skip_context = skip_context_saved; } pub(crate) fn visit_trait_item(&mut self, ti: &ast::TraitItem) { @@ -645,14 +637,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { ctx.snippet_provider, ctx.report.clone(), ); - visitor - .skip_macro_names - .borrow_mut() - .append(&mut ctx.skip_macro_names.borrow().clone()); - visitor - .skip_attribute_names - .borrow_mut() - .append(&mut ctx.skip_attribute_names.borrow().clone()); + visitor.skip_context.update(ctx.skip_context.clone()); visitor.set_parent_context(ctx); visitor } @@ -677,8 +662,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { skipped_range: vec![], macro_rewrite_failure: false, report, - skip_macro_names: RefCell::new(vec![]), - skip_attribute_names: RefCell::new(vec![]), + skip_context: Default::default(), } } @@ -876,8 +860,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { snippet_provider: self.snippet_provider, macro_rewrite_failure: RefCell::new(false), report: self.report.clone(), - skip_macro_names: self.skip_macro_names.clone(), - skip_attribute_names: self.skip_attribute_names.clone(), + skip_context: self.skip_context.clone().into(), } } } From 32159da58dd8416fc86fc49a90d5f833c6e81981 Mon Sep 17 00:00:00 2001 From: Michele d'Amico Date: Thu, 4 Jul 2019 14:02:57 +0200 Subject: [PATCH 07/24] #3665 Add tip in README.md --- README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 381dc73209e..292fcf644d2 100644 --- a/README.md +++ b/README.md @@ -179,12 +179,16 @@ needs to be specified in `rustfmt.toml`, e.g., with `edition = "2018"`. ## Tips * For things you do not want rustfmt to mangle, use `#[rustfmt::skip]` -* To prevent rustfmt from formatting a macro, - use `#[rustfmt::skip::macros(target_macro_name)]` +* To prevent rustfmt from formatting a macro or an attribute, + use `#[rustfmt::skip::macros(target_macro_name)]` or + `#[rustfmt::skip::attributes(target_attribute_name)]` Example: ```rust + #![rustfmt::skip::attributes(custom_attribute)] + + #[custom_attribute(formatting , here , should , be , Skipped)] #[rustfmt::skip::macros(html)] fn main() { let macro_result1 = html! {
From 94130158552de786078824562ffbc68aee018558 Mon Sep 17 00:00:00 2001 From: Michele d'Amico Date: Thu, 4 Jul 2019 18:31:13 +0200 Subject: [PATCH 08/24] #3665 Refactoring: Move skip related stuff in skip module. Removed useless `into()` --- src/lib.rs | 1 + src/rewrite.rs | 2 +- src/skip.rs | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/utils.rs | 48 ----------------------------------- src/visitor.rs | 16 +++--------- 5 files changed, 74 insertions(+), 61 deletions(-) create mode 100644 src/skip.rs diff --git a/src/lib.rs b/src/lib.rs index 1b4ce663313..a672e2cc8aa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -69,6 +69,7 @@ mod reorder; mod rewrite; pub(crate) mod rustfmt_diff; mod shape; +mod skip; pub(crate) mod source_file; pub(crate) mod source_map; mod spanned; diff --git a/src/rewrite.rs b/src/rewrite.rs index 47c6ae4efbd..606aafc9ebd 100644 --- a/src/rewrite.rs +++ b/src/rewrite.rs @@ -8,7 +8,7 @@ use syntax::source_map::{SourceMap, Span}; use crate::config::{Config, IndentStyle}; use crate::shape::Shape; -use crate::utils::SkipContext; +use crate::skip::SkipContext; use crate::visitor::SnippetProvider; use crate::FormatReport; diff --git a/src/skip.rs b/src/skip.rs new file mode 100644 index 00000000000..436f0c48123 --- /dev/null +++ b/src/skip.rs @@ -0,0 +1,68 @@ +use syntax::ast; + +#[derive(Default, Clone)] +pub(crate) struct SkipContext { + macros: Vec, + attributes: Vec, +} + +impl SkipContext { + pub(crate) fn update_by_attrs(&mut self, attrs: &[ast::Attribute]) { + self.macros.append(&mut get_skip_names("macros", attrs)); + self.attributes + .append(&mut get_skip_names("attributes", attrs)); + } + + pub(crate) fn update(&mut self, mut other: SkipContext) { + self.macros.append(&mut other.macros); + self.attributes.append(&mut other.attributes); + } + + pub(crate) fn macro_skip(&self, name: &str) -> bool { + self.macros.iter().any(|n| n == name) + } + + pub(crate) fn attributes_skip(&self, name: &str) -> bool { + self.attributes.iter().any(|n| n == name) + } +} + +static RUSTFMT: &'static str = "rustfmt"; +static SKIP: &'static str = "skip"; + +pub(crate) fn is_skip_attr(segments: &[ast::PathSegment]) -> bool { + if segments.len() < 2 || segments[0].ident.to_string() != RUSTFMT { + return false; + } + match segments.len() { + 2 => segments[1].ident.to_string() == SKIP, + 3 => { + segments[1].ident.to_string() == SKIP + && ["macros", "attributes"] + .iter() + .any(|&n| n == &segments[2].ident.name.as_str()) + } + _ => false, + } +} + +fn get_skip_names(kind: &str, attrs: &[ast::Attribute]) -> Vec { + let mut skip_names = vec![]; + let path = format!("{}::{}::{}", RUSTFMT, SKIP, kind); + for attr in attrs { + // syntax::ast::Path is implemented partialEq + // but it is designed for segments.len() == 1 + if format!("{}", attr.path) != path { + continue; + } + + if let Some(list) = attr.meta_item_list() { + for nested_meta_item in list { + if let Some(name) = nested_meta_item.ident() { + skip_names.push(name.to_string()); + } + } + } + } + skip_names +} diff --git a/src/utils.rs b/src/utils.rs index 2fc334e7268..d47e528d485 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -623,54 +623,6 @@ pub(crate) fn unicode_str_width(s: &str) -> usize { s.width() } -#[derive(Default, Clone)] -pub(crate) struct SkipContext { - macros: Vec, - attributes: Vec, -} - -impl SkipContext { - pub(crate) fn update_by_attrs(&mut self, attrs: &[ast::Attribute]) { - self.macros.append(&mut get_skip_names("macros", attrs)); - self.attributes - .append(&mut get_skip_names("attributes", attrs)); - } - - pub(crate) fn update(&mut self, mut other: SkipContext) { - self.macros.append(&mut other.macros); - self.attributes.append(&mut other.attributes); - } - - pub(crate) fn macro_skip(&self, name: &str) -> bool { - self.macros.iter().any(|n| n == name) - } - - pub(crate) fn attributes_skip(&self, name: &str) -> bool { - self.attributes.iter().any(|n| n == name) - } -} - -fn get_skip_names(kind: &str, attrs: &[ast::Attribute]) -> Vec { - let mut skip_names = vec![]; - let path = format!("rustfmt::skip::{}", kind); - for attr in attrs { - // syntax::ast::Path is implemented partialEq - // but it is designed for segments.len() == 1 - if format!("{}", attr.path) != path { - continue; - } - - if let Some(list) = attr.meta_item_list() { - for nested_meta_item in list { - if let Some(name) = nested_meta_item.ident() { - skip_names.push(name.to_string()); - } - } - } - } - skip_names -} - #[cfg(test)] mod test { use super::*; diff --git a/src/visitor.rs b/src/visitor.rs index 260007690a3..1e4abd9d139 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -17,12 +17,13 @@ use crate::items::{ use crate::macros::{rewrite_macro, rewrite_macro_def, MacroPosition}; use crate::rewrite::{Rewrite, RewriteContext}; use crate::shape::{Indent, Shape}; +use crate::skip::{is_skip_attr, SkipContext}; use crate::source_map::{LineRangeUtils, SpanUtils}; use crate::spanned::Spanned; use crate::stmt::Stmt; use crate::utils::{ self, contains_skip, count_newlines, depr_skip_annotation, inner_attributes, mk_sp, - ptr_vec_to_ref_vec, rewrite_ident, stmt_expr, SkipContext, + ptr_vec_to_ref_vec, rewrite_ident, stmt_expr, }; use crate::{ErrorKind, FormatReport, FormattingError}; @@ -719,16 +720,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { if segments[0].ident.to_string() != "rustfmt" { return false; } - - match segments.len() { - 2 => segments[1].ident.to_string() != "skip", - 3 => { - segments[1].ident.to_string() != "skip" - || (segments[2].ident.to_string() != "macros" - && segments[2].ident.to_string() != "attributes") - } - _ => false, - } + !is_skip_attr(segments) } fn walk_mod_items(&mut self, m: &ast::Mod) { @@ -860,7 +852,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { snippet_provider: self.snippet_provider, macro_rewrite_failure: RefCell::new(false), report: self.report.clone(), - skip_context: self.skip_context.clone().into(), + skip_context: self.skip_context.clone(), } } } From 0b533e0624efe44d9cd2e3a99987f6f0051aaf2f Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Sat, 6 Jul 2019 00:17:35 -0500 Subject: [PATCH 09/24] fix: handling of --all when dep name and dir name differ (#3664) --- src/cargo-fmt/main.rs | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/cargo-fmt/main.rs b/src/cargo-fmt/main.rs index ec6344e0695..c063d8eb419 100644 --- a/src/cargo-fmt/main.rs +++ b/src/cargo-fmt/main.rs @@ -247,7 +247,7 @@ fn get_targets(strategy: &CargoFmtStrategy) -> Result, io::Erro } fn get_targets_root_only(targets: &mut BTreeSet) -> Result<(), io::Error> { - let metadata = get_cargo_metadata(None)?; + let metadata = get_cargo_metadata(None, false)?; let current_dir = env::current_dir()?.canonicalize()?; let current_dir_manifest = current_dir.join("Cargo.toml"); let workspace_root_path = PathBuf::from(&metadata.workspace_root).canonicalize()?; @@ -282,7 +282,8 @@ fn get_targets_recursive( mut targets: &mut BTreeSet, visited: &mut BTreeSet, ) -> Result<(), io::Error> { - let metadata = get_cargo_metadata(manifest_path)?; + let metadata = get_cargo_metadata(manifest_path, false)?; + let metadata_with_deps = get_cargo_metadata(manifest_path, true)?; for package in metadata.packages { add_targets(&package.targets, &mut targets); @@ -293,11 +294,19 @@ fn get_targets_recursive( continue; } - let mut manifest_path = PathBuf::from(&package.manifest_path); - - manifest_path.pop(); - manifest_path.push(&dependency.name); - manifest_path.push("Cargo.toml"); + let dependency_package = metadata_with_deps + .packages + .iter() + .find(|p| p.name == dependency.name); + let manifest_path = if dependency_package.is_some() { + PathBuf::from(&dependency_package.unwrap().manifest_path) + } else { + let mut package_manifest_path = PathBuf::from(&package.manifest_path); + package_manifest_path.pop(); + package_manifest_path.push(&dependency.name); + package_manifest_path.push("Cargo.toml"); + package_manifest_path + }; if manifest_path.exists() { visited.insert(dependency.name); @@ -313,7 +322,7 @@ fn get_targets_with_hitlist( hitlist: &[String], targets: &mut BTreeSet, ) -> Result<(), io::Error> { - let metadata = get_cargo_metadata(None)?; + let metadata = get_cargo_metadata(None, false)?; let mut workspace_hitlist: BTreeSet<&String> = BTreeSet::from_iter(hitlist); @@ -399,9 +408,14 @@ fn run_rustfmt( .unwrap_or(SUCCESS)) } -fn get_cargo_metadata(manifest_path: Option<&Path>) -> Result { +fn get_cargo_metadata( + manifest_path: Option<&Path>, + include_deps: bool, +) -> Result { let mut cmd = cargo_metadata::MetadataCommand::new(); - cmd.no_deps(); + if !include_deps { + cmd.no_deps(); + } if let Some(manifest_path) = manifest_path { cmd.manifest_path(manifest_path); } From 950e9f920d852e589008bde5fcd41585bc630837 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Sat, 6 Jul 2019 14:17:53 +0900 Subject: [PATCH 10/24] Do not consider macro-origin await as chain item (#3671) --- src/chains.rs | 6 ++++-- tests/source/async_fn.rs | 5 +++++ tests/target/async_fn.rs | 4 ++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/chains.rs b/src/chains.rs index 6cac55c6e86..1ca0ac582b7 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -168,7 +168,7 @@ impl ChainItemKind { let span = mk_sp(nested.span.hi(), field.span.hi()); (kind, span) } - ast::ExprKind::Await(_, ref nested) => { + ast::ExprKind::Await(ast::AwaitOrigin::FieldLike, ref nested) => { let span = mk_sp(nested.span.hi(), expr.span.hi()); (ChainItemKind::Await, span) } @@ -396,7 +396,9 @@ impl Chain { } ast::ExprKind::Field(ref subexpr, _) | ast::ExprKind::Try(ref subexpr) - | ast::ExprKind::Await(_, ref subexpr) => Some(Self::convert_try(subexpr, context)), + | ast::ExprKind::Await(ast::AwaitOrigin::FieldLike, ref subexpr) => { + Some(Self::convert_try(subexpr, context)) + } _ => None, } } diff --git a/tests/source/async_fn.rs b/tests/source/async_fn.rs index f6c5f5e1c9a..b4dc9fac5b2 100644 --- a/tests/source/async_fn.rs +++ b/tests/source/async_fn.rs @@ -19,3 +19,8 @@ async unsafe fn rust() { Ok(()) } } + +async fn await_macro() { + await ! ( + something)?; +} diff --git a/tests/target/async_fn.rs b/tests/target/async_fn.rs index 7992e63c080..82be8c76785 100644 --- a/tests/target/async_fn.rs +++ b/tests/target/async_fn.rs @@ -18,3 +18,7 @@ async unsafe fn rust() { Ok(()) } } + +async fn await_macro() { + await!(something)?; +} From d4a3694bdbad588a104838ee7ed74a4da90abbb9 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Sat, 6 Jul 2019 14:22:19 +0900 Subject: [PATCH 11/24] Update CHANGELOG.md --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07ddc87d170..9fa24eee62c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## [Unreleased] +## [1.3.2] 2019-07-06 + +### Fixed + +- Fix rustfmt from crashing when `async!` macro call is used in a method chain. +- Fix rustfmt not recognizing a package whose name differs from its directory's name. + ## [1.3.1] 2019-06-30 ### Added From 4b28c36103ae05982b208469a704d03fc6f544b1 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Sat, 6 Jul 2019 14:22:30 +0900 Subject: [PATCH 12/24] Release 1.3.2 --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/config/mod.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d6c1cef8d84..9568dbcfd56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -838,7 +838,7 @@ dependencies = [ [[package]] name = "rustfmt-nightly" -version = "1.3.1" +version = "1.3.2" dependencies = [ "annotate-snippets 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", "atty 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index 99a0890574e..56815a1e5b1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "rustfmt-nightly" -version = "1.3.1" +version = "1.3.2" authors = ["Nicholas Cameron ", "The Rustfmt developers"] description = "Tool to find and fix Rust formatting issues" repository = "https://github.com/rust-lang/rustfmt" diff --git a/src/config/mod.rs b/src/config/mod.rs index 4b4f9921d80..01ce1df8a9a 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -519,7 +519,7 @@ use_field_init_shorthand = false force_explicit_abi = true condense_wildcard_suffixes = false color = "Auto" -required_version = "1.3.1" +required_version = "1.3.2" unstable_features = false disable_all_formatting = false skip_children = false From 0978f8de17d9e68d1188e752b1872532d14887eb Mon Sep 17 00:00:00 2001 From: calebcartwright Date: Fri, 12 Jul 2019 20:59:29 -0500 Subject: [PATCH 13/24] feat: add --manifest-path support to cargo fmt --- src/cargo-fmt/main.rs | 67 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 55 insertions(+), 12 deletions(-) diff --git a/src/cargo-fmt/main.rs b/src/cargo-fmt/main.rs index c063d8eb419..f32f7b5f1be 100644 --- a/src/cargo-fmt/main.rs +++ b/src/cargo-fmt/main.rs @@ -41,6 +41,10 @@ pub struct Opts { #[structopt(short = "p", long = "package", value_name = "package")] packages: Vec, + /// Specify path to Cargo.toml + #[structopt(long = "manifest-path", value_name = "manifest-path")] + manifest_path: Option, + /// Options passed to rustfmt // 'raw = true' to make `--` explicit. #[structopt(name = "rustfmt_options", raw(raw = "true"))] @@ -90,7 +94,27 @@ fn execute() -> i32 { let strategy = CargoFmtStrategy::from_opts(&opts); - handle_command_status(format_crate(verbosity, &strategy, opts.rustfmt_options)) + if opts.manifest_path.is_some() { + let specified_manifest_path = opts.manifest_path.unwrap(); + if !specified_manifest_path.ends_with("Cargo.toml") { + print_usage_to_stderr("the manifest-path must be a path to a Cargo.toml file"); + return FAILURE; + } + let manifest_path = PathBuf::from(specified_manifest_path); + handle_command_status(format_crate( + verbosity, + &strategy, + opts.rustfmt_options, + Some(&manifest_path), + )) + } else { + handle_command_status(format_crate( + verbosity, + &strategy, + opts.rustfmt_options, + None, + )) + } } fn print_usage_to_stderr(reason: &str) { @@ -142,6 +166,7 @@ fn format_crate( verbosity: Verbosity, strategy: &CargoFmtStrategy, rustfmt_args: Vec, + manifest_path: Option<&Path>, ) -> Result { let targets = if rustfmt_args .iter() @@ -149,7 +174,7 @@ fn format_crate( { BTreeSet::new() } else { - get_targets(strategy)? + get_targets(strategy, manifest_path)? }; // Currently only bin and lib files get formatted. @@ -227,13 +252,20 @@ impl CargoFmtStrategy { } /// Based on the specified `CargoFmtStrategy`, returns a set of main source files. -fn get_targets(strategy: &CargoFmtStrategy) -> Result, io::Error> { +fn get_targets( + strategy: &CargoFmtStrategy, + manifest_path: Option<&Path>, +) -> Result, io::Error> { let mut targets = BTreeSet::new(); match *strategy { - CargoFmtStrategy::Root => get_targets_root_only(&mut targets)?, - CargoFmtStrategy::All => get_targets_recursive(None, &mut targets, &mut BTreeSet::new())?, - CargoFmtStrategy::Some(ref hitlist) => get_targets_with_hitlist(hitlist, &mut targets)?, + CargoFmtStrategy::Root => get_targets_root_only(manifest_path, &mut targets)?, + CargoFmtStrategy::All => { + get_targets_recursive(manifest_path, &mut targets, &mut BTreeSet::new())? + } + CargoFmtStrategy::Some(ref hitlist) => { + get_targets_with_hitlist(manifest_path, hitlist, &mut targets)? + } } if targets.is_empty() { @@ -246,12 +278,22 @@ fn get_targets(strategy: &CargoFmtStrategy) -> Result, io::Erro } } -fn get_targets_root_only(targets: &mut BTreeSet) -> Result<(), io::Error> { - let metadata = get_cargo_metadata(None, false)?; - let current_dir = env::current_dir()?.canonicalize()?; - let current_dir_manifest = current_dir.join("Cargo.toml"); +fn get_targets_root_only( + manifest_path: Option<&Path>, + targets: &mut BTreeSet, +) -> Result<(), io::Error> { + let metadata = get_cargo_metadata(manifest_path, false)?; let workspace_root_path = PathBuf::from(&metadata.workspace_root).canonicalize()?; - let in_workspace_root = workspace_root_path == current_dir; + let (in_workspace_root, current_dir_manifest) = if manifest_path.is_some() { + let target_manifest = manifest_path.unwrap().canonicalize()?; + (workspace_root_path == target_manifest, target_manifest) + } else { + let current_dir = env::current_dir()?.canonicalize()?; + ( + workspace_root_path == current_dir, + current_dir.join("Cargo.toml"), + ) + }; let package_targets = match metadata.packages.len() { 1 => metadata.packages.into_iter().next().unwrap().targets, @@ -319,10 +361,11 @@ fn get_targets_recursive( } fn get_targets_with_hitlist( + manifest_path: Option<&Path>, hitlist: &[String], targets: &mut BTreeSet, ) -> Result<(), io::Error> { - let metadata = get_cargo_metadata(None, false)?; + let metadata = get_cargo_metadata(manifest_path, false)?; let mut workspace_hitlist: BTreeSet<&String> = BTreeSet::from_iter(hitlist); From 899e2797600849c9f43aa221ecbe4b2de08ae59d Mon Sep 17 00:00:00 2001 From: Ruben Schmidmeister Date: Sun, 14 Jul 2019 03:22:18 +0200 Subject: [PATCH 14/24] Extract configuration snippet tests into own module (#3667) * Extract configuration snippet tests into own module * Move helper function outside of test function --- src/test/configuration_snippet.rs | 286 ++++++++++++++++++++++++++++++ src/test/mod.rs | 284 +---------------------------- 2 files changed, 291 insertions(+), 279 deletions(-) create mode 100644 src/test/configuration_snippet.rs diff --git a/src/test/configuration_snippet.rs b/src/test/configuration_snippet.rs new file mode 100644 index 00000000000..55289f55537 --- /dev/null +++ b/src/test/configuration_snippet.rs @@ -0,0 +1,286 @@ +use std::collections::{HashMap, HashSet}; +use std::fs; +use std::io::{BufRead, BufReader, Write}; +use std::iter::Enumerate; +use std::path::{Path, PathBuf}; + +use super::{print_mismatches, write_message, DIFF_CONTEXT_SIZE}; +use crate::config::{Config, EmitMode, Verbosity}; +use crate::rustfmt_diff::{make_diff, Mismatch}; +use crate::{Input, Session}; + +const CONFIGURATIONS_FILE_NAME: &str = "Configurations.md"; + +// This enum is used to represent one of three text features in Configurations.md: a block of code +// with its starting line number, the name of a rustfmt configuration option, or the value of a +// rustfmt configuration option. +enum ConfigurationSection { + CodeBlock((String, u32)), // (String: block of code, u32: line number of code block start) + ConfigName(String), + ConfigValue(String), +} + +impl ConfigurationSection { + fn get_section>( + file: &mut Enumerate, + ) -> Option { + lazy_static! { + static ref CONFIG_NAME_REGEX: regex::Regex = + regex::Regex::new(r"^## `([^`]+)`").expect("failed creating configuration pattern"); + static ref CONFIG_VALUE_REGEX: regex::Regex = + regex::Regex::new(r#"^#### `"?([^`"]+)"?`"#) + .expect("failed creating configuration value pattern"); + } + + loop { + match file.next() { + Some((i, line)) => { + if line.starts_with("```rust") { + // Get the lines of the code block. + let lines: Vec = file + .map(|(_i, l)| l) + .take_while(|l| !l.starts_with("```")) + .collect(); + let block = format!("{}\n", lines.join("\n")); + + // +1 to translate to one-based indexing + // +1 to get to first line of code (line after "```") + let start_line = (i + 2) as u32; + + return Some(ConfigurationSection::CodeBlock((block, start_line))); + } else if let Some(c) = CONFIG_NAME_REGEX.captures(&line) { + return Some(ConfigurationSection::ConfigName(String::from(&c[1]))); + } else if let Some(c) = CONFIG_VALUE_REGEX.captures(&line) { + return Some(ConfigurationSection::ConfigValue(String::from(&c[1]))); + } + } + None => return None, // reached the end of the file + } + } + } +} + +// This struct stores the information about code blocks in the configurations +// file, formats the code blocks, and prints formatting errors. +struct ConfigCodeBlock { + config_name: Option, + config_value: Option, + code_block: Option, + code_block_start: Option, +} + +impl ConfigCodeBlock { + fn new() -> ConfigCodeBlock { + ConfigCodeBlock { + config_name: None, + config_value: None, + code_block: None, + code_block_start: None, + } + } + + fn set_config_name(&mut self, name: Option) { + self.config_name = name; + self.config_value = None; + } + + fn set_config_value(&mut self, value: Option) { + self.config_value = value; + } + + fn set_code_block(&mut self, code_block: String, code_block_start: u32) { + self.code_block = Some(code_block); + self.code_block_start = Some(code_block_start); + } + + fn get_block_config(&self) -> Config { + let mut config = Config::default(); + config.set().verbose(Verbosity::Quiet); + if self.config_name.is_some() && self.config_value.is_some() { + config.override_value( + self.config_name.as_ref().unwrap(), + self.config_value.as_ref().unwrap(), + ); + } + config + } + + fn code_block_valid(&self) -> bool { + // We never expect to not have a code block. + assert!(self.code_block.is_some() && self.code_block_start.is_some()); + + // See if code block begins with #![rustfmt::skip]. + let fmt_skip = self + .code_block + .as_ref() + .unwrap() + .lines() + .nth(0) + .unwrap_or("") + == "#![rustfmt::skip]"; + + if self.config_name.is_none() && !fmt_skip { + write_message(&format!( + "No configuration name for {}:{}", + CONFIGURATIONS_FILE_NAME, + self.code_block_start.unwrap() + )); + return false; + } + if self.config_value.is_none() && !fmt_skip { + write_message(&format!( + "No configuration value for {}:{}", + CONFIGURATIONS_FILE_NAME, + self.code_block_start.unwrap() + )); + return false; + } + true + } + + fn has_parsing_errors(&self, session: &Session<'_, T>) -> bool { + if session.has_parsing_errors() { + write_message(&format!( + "\u{261d}\u{1f3fd} Cannot format {}:{}", + CONFIGURATIONS_FILE_NAME, + self.code_block_start.unwrap() + )); + return true; + } + + false + } + + fn print_diff(&self, compare: Vec) { + let mut mismatches = HashMap::new(); + mismatches.insert(PathBuf::from(CONFIGURATIONS_FILE_NAME), compare); + print_mismatches(mismatches, |line_num| { + format!( + "\nMismatch at {}:{}:", + CONFIGURATIONS_FILE_NAME, + line_num + self.code_block_start.unwrap() - 1 + ) + }); + } + + fn formatted_has_diff(&self, text: &str) -> bool { + let compare = make_diff(self.code_block.as_ref().unwrap(), text, DIFF_CONTEXT_SIZE); + if !compare.is_empty() { + self.print_diff(compare); + return true; + } + + false + } + + // Return a bool indicating if formatting this code block is an idempotent + // operation. This function also triggers printing any formatting failure + // messages. + fn formatted_is_idempotent(&self) -> bool { + // Verify that we have all of the expected information. + if !self.code_block_valid() { + return false; + } + + let input = Input::Text(self.code_block.as_ref().unwrap().to_owned()); + let mut config = self.get_block_config(); + config.set().emit_mode(EmitMode::Stdout); + let mut buf: Vec = vec![]; + + { + let mut session = Session::new(config, Some(&mut buf)); + session.format(input).unwrap(); + if self.has_parsing_errors(&session) { + return false; + } + } + + !self.formatted_has_diff(&String::from_utf8(buf).unwrap()) + } + + // Extract a code block from the iterator. Behavior: + // - Rust code blocks are identifed by lines beginning with "```rust". + // - One explicit configuration setting is supported per code block. + // - Rust code blocks with no configuration setting are illegal and cause an + // assertion failure, unless the snippet begins with #![rustfmt::skip]. + // - Configuration names in Configurations.md must be in the form of + // "## `NAME`". + // - Configuration values in Configurations.md must be in the form of + // "#### `VALUE`". + fn extract>( + file: &mut Enumerate, + prev: Option<&ConfigCodeBlock>, + hash_set: &mut HashSet, + ) -> Option { + let mut code_block = ConfigCodeBlock::new(); + code_block.config_name = prev.and_then(|cb| cb.config_name.clone()); + + loop { + match ConfigurationSection::get_section(file) { + Some(ConfigurationSection::CodeBlock((block, start_line))) => { + code_block.set_code_block(block, start_line); + break; + } + Some(ConfigurationSection::ConfigName(name)) => { + assert!( + Config::is_valid_name(&name), + "an unknown configuration option was found: {}", + name + ); + assert!( + hash_set.remove(&name), + "multiple configuration guides found for option {}", + name + ); + code_block.set_config_name(Some(name)); + } + Some(ConfigurationSection::ConfigValue(value)) => { + code_block.set_config_value(Some(value)); + } + None => return None, // end of file was reached + } + } + + Some(code_block) + } +} + +#[test] +fn configuration_snippet_tests() { + let blocks = get_code_blocks(); + let failures = blocks + .iter() + .map(ConfigCodeBlock::formatted_is_idempotent) + .fold(0, |acc, r| acc + (!r as u32)); + + // Display results. + println!("Ran {} configurations tests.", blocks.len()); + assert_eq!(failures, 0, "{} configurations tests failed", failures); +} + +// Read Configurations.md and build a `Vec` of `ConfigCodeBlock` structs with one +// entry for each Rust code block found. +fn get_code_blocks() -> Vec { + let mut file_iter = BufReader::new( + fs::File::open(Path::new(CONFIGURATIONS_FILE_NAME)) + .unwrap_or_else(|_| panic!("couldn't read file {}", CONFIGURATIONS_FILE_NAME)), + ) + .lines() + .map(Result::unwrap) + .enumerate(); + let mut code_blocks: Vec = Vec::new(); + let mut hash_set = Config::hash_set(); + + while let Some(cb) = ConfigCodeBlock::extract(&mut file_iter, code_blocks.last(), &mut hash_set) + { + code_blocks.push(cb); + } + + for name in hash_set { + if !Config::is_hidden_option(&name) { + panic!("{} does not have a configuration guide", name); + } + } + + code_blocks +} diff --git a/src/test/mod.rs b/src/test/mod.rs index 0401df290a7..a7063c76f28 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -1,23 +1,24 @@ -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::env; use std::fs; use std::io::{self, BufRead, BufReader, Read, Write}; -use std::iter::{Enumerate, Peekable}; +use std::iter::Peekable; use std::mem; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::str::Chars; use std::thread; -use crate::config::{Color, Config, EmitMode, FileName, NewlineStyle, ReportTactic, Verbosity}; +use crate::config::{Color, Config, EmitMode, FileName, NewlineStyle, ReportTactic}; use crate::formatting::{ReportedErrors, SourceFile}; use crate::is_nightly_channel; use crate::rustfmt_diff::{make_diff, print_diff, DiffLine, Mismatch, ModifiedChunk, OutputWriter}; use crate::source_file; use crate::{FormatReport, FormatReportFormatterBuilder, Input, Session}; +mod configuration_snippet; + const DIFF_CONTEXT_SIZE: usize = 3; -const CONFIGURATIONS_FILE_NAME: &str = "Configurations.md"; // A list of files on which we want to skip testing. const SKIP_FILE_WHITE_LIST: &[&str] = &[ @@ -769,281 +770,6 @@ fn string_eq_ignore_newline_repr_test() { assert!(!string_eq_ignore_newline_repr("a\r\nbcd", "a\nbcdefghijk")); } -// This enum is used to represent one of three text features in Configurations.md: a block of code -// with its starting line number, the name of a rustfmt configuration option, or the value of a -// rustfmt configuration option. -enum ConfigurationSection { - CodeBlock((String, u32)), // (String: block of code, u32: line number of code block start) - ConfigName(String), - ConfigValue(String), -} - -impl ConfigurationSection { - fn get_section>( - file: &mut Enumerate, - ) -> Option { - lazy_static! { - static ref CONFIG_NAME_REGEX: regex::Regex = - regex::Regex::new(r"^## `([^`]+)`").expect("failed creating configuration pattern"); - static ref CONFIG_VALUE_REGEX: regex::Regex = - regex::Regex::new(r#"^#### `"?([^`"]+)"?`"#) - .expect("failed creating configuration value pattern"); - } - - loop { - match file.next() { - Some((i, line)) => { - if line.starts_with("```rust") { - // Get the lines of the code block. - let lines: Vec = file - .map(|(_i, l)| l) - .take_while(|l| !l.starts_with("```")) - .collect(); - let block = format!("{}\n", lines.join("\n")); - - // +1 to translate to one-based indexing - // +1 to get to first line of code (line after "```") - let start_line = (i + 2) as u32; - - return Some(ConfigurationSection::CodeBlock((block, start_line))); - } else if let Some(c) = CONFIG_NAME_REGEX.captures(&line) { - return Some(ConfigurationSection::ConfigName(String::from(&c[1]))); - } else if let Some(c) = CONFIG_VALUE_REGEX.captures(&line) { - return Some(ConfigurationSection::ConfigValue(String::from(&c[1]))); - } - } - None => return None, // reached the end of the file - } - } - } -} - -// This struct stores the information about code blocks in the configurations -// file, formats the code blocks, and prints formatting errors. -struct ConfigCodeBlock { - config_name: Option, - config_value: Option, - code_block: Option, - code_block_start: Option, -} - -impl ConfigCodeBlock { - fn new() -> ConfigCodeBlock { - ConfigCodeBlock { - config_name: None, - config_value: None, - code_block: None, - code_block_start: None, - } - } - - fn set_config_name(&mut self, name: Option) { - self.config_name = name; - self.config_value = None; - } - - fn set_config_value(&mut self, value: Option) { - self.config_value = value; - } - - fn set_code_block(&mut self, code_block: String, code_block_start: u32) { - self.code_block = Some(code_block); - self.code_block_start = Some(code_block_start); - } - - fn get_block_config(&self) -> Config { - let mut config = Config::default(); - config.set().verbose(Verbosity::Quiet); - if self.config_name.is_some() && self.config_value.is_some() { - config.override_value( - self.config_name.as_ref().unwrap(), - self.config_value.as_ref().unwrap(), - ); - } - config - } - - fn code_block_valid(&self) -> bool { - // We never expect to not have a code block. - assert!(self.code_block.is_some() && self.code_block_start.is_some()); - - // See if code block begins with #![rustfmt::skip]. - let fmt_skip = self - .code_block - .as_ref() - .unwrap() - .lines() - .nth(0) - .unwrap_or("") - == "#![rustfmt::skip]"; - - if self.config_name.is_none() && !fmt_skip { - write_message(&format!( - "No configuration name for {}:{}", - CONFIGURATIONS_FILE_NAME, - self.code_block_start.unwrap() - )); - return false; - } - if self.config_value.is_none() && !fmt_skip { - write_message(&format!( - "No configuration value for {}:{}", - CONFIGURATIONS_FILE_NAME, - self.code_block_start.unwrap() - )); - return false; - } - true - } - - fn has_parsing_errors(&self, session: &Session<'_, T>) -> bool { - if session.has_parsing_errors() { - write_message(&format!( - "\u{261d}\u{1f3fd} Cannot format {}:{}", - CONFIGURATIONS_FILE_NAME, - self.code_block_start.unwrap() - )); - return true; - } - - false - } - - fn print_diff(&self, compare: Vec) { - let mut mismatches = HashMap::new(); - mismatches.insert(PathBuf::from(CONFIGURATIONS_FILE_NAME), compare); - print_mismatches(mismatches, |line_num| { - format!( - "\nMismatch at {}:{}:", - CONFIGURATIONS_FILE_NAME, - line_num + self.code_block_start.unwrap() - 1 - ) - }); - } - - fn formatted_has_diff(&self, text: &str) -> bool { - let compare = make_diff(self.code_block.as_ref().unwrap(), text, DIFF_CONTEXT_SIZE); - if !compare.is_empty() { - self.print_diff(compare); - return true; - } - - false - } - - // Return a bool indicating if formatting this code block is an idempotent - // operation. This function also triggers printing any formatting failure - // messages. - fn formatted_is_idempotent(&self) -> bool { - // Verify that we have all of the expected information. - if !self.code_block_valid() { - return false; - } - - let input = Input::Text(self.code_block.as_ref().unwrap().to_owned()); - let mut config = self.get_block_config(); - config.set().emit_mode(EmitMode::Stdout); - let mut buf: Vec = vec![]; - - { - let mut session = Session::new(config, Some(&mut buf)); - session.format(input).unwrap(); - if self.has_parsing_errors(&session) { - return false; - } - } - - !self.formatted_has_diff(&String::from_utf8(buf).unwrap()) - } - - // Extract a code block from the iterator. Behavior: - // - Rust code blocks are identifed by lines beginning with "```rust". - // - One explicit configuration setting is supported per code block. - // - Rust code blocks with no configuration setting are illegal and cause an - // assertion failure, unless the snippet begins with #![rustfmt::skip]. - // - Configuration names in Configurations.md must be in the form of - // "## `NAME`". - // - Configuration values in Configurations.md must be in the form of - // "#### `VALUE`". - fn extract>( - file: &mut Enumerate, - prev: Option<&ConfigCodeBlock>, - hash_set: &mut HashSet, - ) -> Option { - let mut code_block = ConfigCodeBlock::new(); - code_block.config_name = prev.and_then(|cb| cb.config_name.clone()); - - loop { - match ConfigurationSection::get_section(file) { - Some(ConfigurationSection::CodeBlock((block, start_line))) => { - code_block.set_code_block(block, start_line); - break; - } - Some(ConfigurationSection::ConfigName(name)) => { - assert!( - Config::is_valid_name(&name), - "an unknown configuration option was found: {}", - name - ); - assert!( - hash_set.remove(&name), - "multiple configuration guides found for option {}", - name - ); - code_block.set_config_name(Some(name)); - } - Some(ConfigurationSection::ConfigValue(value)) => { - code_block.set_config_value(Some(value)); - } - None => return None, // end of file was reached - } - } - - Some(code_block) - } -} - -#[test] -fn configuration_snippet_tests() { - // Read Configurations.md and build a `Vec` of `ConfigCodeBlock` structs with one - // entry for each Rust code block found. - fn get_code_blocks() -> Vec { - let mut file_iter = BufReader::new( - fs::File::open(Path::new(CONFIGURATIONS_FILE_NAME)) - .unwrap_or_else(|_| panic!("couldn't read file {}", CONFIGURATIONS_FILE_NAME)), - ) - .lines() - .map(Result::unwrap) - .enumerate(); - let mut code_blocks: Vec = Vec::new(); - let mut hash_set = Config::hash_set(); - - while let Some(cb) = - ConfigCodeBlock::extract(&mut file_iter, code_blocks.last(), &mut hash_set) - { - code_blocks.push(cb); - } - - for name in hash_set { - if !Config::is_hidden_option(&name) { - panic!("{} does not have a configuration guide", name); - } - } - - code_blocks - } - - let blocks = get_code_blocks(); - let failures = blocks - .iter() - .map(ConfigCodeBlock::formatted_is_idempotent) - .fold(0, |acc, r| acc + (!r as u32)); - - // Display results. - println!("Ran {} configurations tests.", blocks.len()); - assert_eq!(failures, 0, "{} configurations tests failed", failures); -} - struct TempFile { path: PathBuf, } From bee3946cdb96e4aca797de8ab04c2e5c71345fe9 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sat, 13 Jul 2019 18:25:53 -0700 Subject: [PATCH 15/24] Fix using --help, --verbose, etc. (#3620) --- tests/cargo-fmt/main.rs | 70 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 tests/cargo-fmt/main.rs diff --git a/tests/cargo-fmt/main.rs b/tests/cargo-fmt/main.rs new file mode 100644 index 00000000000..d1ba2e689b6 --- /dev/null +++ b/tests/cargo-fmt/main.rs @@ -0,0 +1,70 @@ +// Integration tests for cargo-fmt. + +use std::env; +use std::process::Command; + +/// Run the cargo-fmt executable and return its output. +fn cargo_fmt(args: &[&str]) -> (String, String) { + let mut bin_dir = env::current_exe().unwrap(); + bin_dir.pop(); // chop off test exe name + if bin_dir.ends_with("deps") { + bin_dir.pop(); + } + let cmd = bin_dir.join(format!("cargo-fmt{}", env::consts::EXE_SUFFIX)); + + // Ensure cargo-fmt runs the rustfmt binary from the local target dir. + let path = env::var_os("PATH").unwrap_or_default(); + let mut paths = env::split_paths(&path).collect::>(); + paths.insert(0, bin_dir); + let new_path = env::join_paths(paths).unwrap(); + + match Command::new(&cmd).args(args).env("PATH", new_path).output() { + Ok(output) => ( + String::from_utf8(output.stdout).expect("utf-8"), + String::from_utf8(output.stderr).expect("utf-8"), + ), + Err(e) => panic!("failed to run `{:?} {:?}`: {}", cmd, args, e), + } +} + +macro_rules! assert_that { + ($args:expr, $check:ident $check_args:tt) => { + let (stdout, stderr) = cargo_fmt($args); + if !stdout.$check$check_args { + panic!( + "Output not expected for cargo-fmt {:?}\n\ + expected: {}{}\n\ + actual stdout:\n{}\n\ + actual stderr:\n{}", + $args, + stringify!($check), + stringify!($check_args), + stdout, + stderr + ); + } + }; +} + +#[test] +fn version() { + assert_that!(&["--version"], starts_with("rustfmt ")); + assert_that!(&["--version"], starts_with("rustfmt ")); + assert_that!(&["--", "-V"], starts_with("rustfmt ")); + assert_that!(&["--", "--version"], starts_with("rustfmt ")); +} + +#[test] +fn print_config() { + assert_that!( + &["--", "--print-config", "current", "."], + contains("max_width = ") + ); +} + +#[test] +fn rustfmt_help() { + assert_that!(&["--", "--help"], contains("Format Rust code")); + assert_that!(&["--", "-h"], contains("Format Rust code")); + assert_that!(&["--", "--help=config"], contains("Configuration Options:")); +} From 670c11f5482d4a8203a2a0c41177199f73eb1c9e Mon Sep 17 00:00:00 2001 From: rChaser53 Date: Sun, 14 Jul 2019 22:16:47 +0900 Subject: [PATCH 16/24] fix 'extra comma inserted due to comment' (#3677) --- src/lists.rs | 6 +++++- tests/source/issue-3675.rs | 5 +++++ tests/target/issue-3675.rs | 6 ++++++ 3 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 tests/source/issue-3675.rs create mode 100644 tests/target/issue-3675.rs diff --git a/src/lists.rs b/src/lists.rs index 4c31f670d4d..ac83d7082c8 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -612,7 +612,11 @@ pub(crate) fn extract_post_comment( post_snippet[1..].trim_matches(white_space) } else if post_snippet.starts_with(separator) { post_snippet[separator.len()..].trim_matches(white_space) - } else if post_snippet.ends_with(',') && !post_snippet.trim().starts_with("//") { + } + // not comment or over two lines + else if post_snippet.ends_with(',') + && (!post_snippet.trim().starts_with("//") || post_snippet.trim().contains('\n')) + { post_snippet[..(post_snippet.len() - 1)].trim_matches(white_space) } else { post_snippet diff --git a/tests/source/issue-3675.rs b/tests/source/issue-3675.rs new file mode 100644 index 00000000000..f16efb2dcac --- /dev/null +++ b/tests/source/issue-3675.rs @@ -0,0 +1,5 @@ + fn main() { + println!("{}" + // comment + , 111); + } \ No newline at end of file diff --git a/tests/target/issue-3675.rs b/tests/target/issue-3675.rs new file mode 100644 index 00000000000..62d986e7734 --- /dev/null +++ b/tests/target/issue-3675.rs @@ -0,0 +1,6 @@ +fn main() { + println!( + "{}", // comment + 111 + ); +} From b949ac2258f9dc6b69250e20797ea481bdcdafa9 Mon Sep 17 00:00:00 2001 From: calebcartwright Date: Sun, 14 Jul 2019 09:56:07 -0500 Subject: [PATCH 17/24] refactor: simplify manifest_path option checks --- src/cargo-fmt/main.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cargo-fmt/main.rs b/src/cargo-fmt/main.rs index f32f7b5f1be..086dca6e7a7 100644 --- a/src/cargo-fmt/main.rs +++ b/src/cargo-fmt/main.rs @@ -94,8 +94,7 @@ fn execute() -> i32 { let strategy = CargoFmtStrategy::from_opts(&opts); - if opts.manifest_path.is_some() { - let specified_manifest_path = opts.manifest_path.unwrap(); + if let Some(specified_manifest_path) = opts.manifest_path { if !specified_manifest_path.ends_with("Cargo.toml") { print_usage_to_stderr("the manifest-path must be a path to a Cargo.toml file"); return FAILURE; @@ -284,9 +283,11 @@ fn get_targets_root_only( ) -> Result<(), io::Error> { let metadata = get_cargo_metadata(manifest_path, false)?; let workspace_root_path = PathBuf::from(&metadata.workspace_root).canonicalize()?; - let (in_workspace_root, current_dir_manifest) = if manifest_path.is_some() { - let target_manifest = manifest_path.unwrap().canonicalize()?; - (workspace_root_path == target_manifest, target_manifest) + let (in_workspace_root, current_dir_manifest) = if let Some(target_manifest) = manifest_path { + ( + workspace_root_path == target_manifest, + target_manifest.canonicalize()?, + ) } else { let current_dir = env::current_dir()?.canonicalize()?; ( From 3591a34d1a145915f281ccb314efd2d5b556e166 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Mon, 15 Jul 2019 13:58:54 +0200 Subject: [PATCH 18/24] fix print-config minimal option (#3687) --- src/bin/main.rs | 109 +++++++++++++++++++++++++----------------- tests/rustfmt/main.rs | 77 +++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 45 deletions(-) create mode 100644 tests/rustfmt/main.rs diff --git a/src/bin/main.rs b/src/bin/main.rs index 5b69a440b65..813504cdd6d 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -1,6 +1,6 @@ use env_logger; -#[macro_use] -extern crate failure; +use failure::{err_msg, format_err, Error as FailureError, Fail}; +use io::Error as IoError; use rustfmt_nightly as rustfmt; @@ -10,12 +10,10 @@ use std::io::{self, stdout, Read, Write}; use std::path::{Path, PathBuf}; use std::str::FromStr; -use failure::err_msg; - use getopts::{Matches, Options}; use crate::rustfmt::{ - load_config, CliOptions, Color, Config, Edition, EmitMode, ErrorKind, FileLines, FileName, + load_config, CliOptions, Color, Config, Edition, EmitMode, FileLines, FileName, FormatReportFormatterBuilder, Input, Session, Verbosity, }; @@ -49,20 +47,37 @@ enum Operation { }, /// Print the help message. Help(HelpOp), - // Print version information + /// Print version information Version, /// Output default config to a file, or stdout if None - ConfigOutputDefault { - path: Option, - }, + ConfigOutputDefault { path: Option }, /// Output current config (as if formatting to a file) to stdout - ConfigOutputCurrent { - path: Option, - }, + ConfigOutputCurrent { path: Option }, /// No file specified, read from stdin - Stdin { - input: String, - }, + Stdin { input: String }, +} + +/// Rustfmt operations errors. +#[derive(Fail, Debug)] +pub enum OperationError { + /// An unknown help topic was requested. + #[fail(display = "Unknown help topic: `{}`.", _0)] + UnknownHelpTopic(String), + /// An unknown print-config option was requested. + #[fail(display = "Unknown print-config option: `{}`.", _0)] + UnknownPrintConfigTopic(String), + /// Attempt to generate a minimal config from standard input. + #[fail(display = "The `--print-config=minimal` option doesn't work with standard input.")] + MinimalPathWithStdin, + /// An io error during reading or writing. + #[fail(display = "io error: {}", _0)] + IoError(IoError), +} + +impl From for OperationError { + fn from(e: IoError) -> OperationError { + OperationError::IoError(e) + } } /// Arguments to `--help` @@ -156,7 +171,7 @@ fn is_nightly() -> bool { } // Returned i32 is an exit code -fn execute(opts: &Options) -> Result { +fn execute(opts: &Options) -> Result { let matches = opts.parse(env::args().skip(1))?; let options = GetOptsOptions::from_matches(&matches)?; @@ -210,7 +225,7 @@ fn execute(opts: &Options) -> Result { } } -fn format_string(input: String, options: GetOptsOptions) -> Result { +fn format_string(input: String, options: GetOptsOptions) -> Result { // try to read config from local directory let (mut config, _) = load_config(Some(Path::new(".")), Some(options.clone()))?; @@ -243,7 +258,7 @@ fn format( files: Vec, minimal_config_path: Option, options: &GetOptsOptions, -) -> Result { +) -> Result { options.verify_file_lines(&files); let (config, config_path) = load_config(None, Some(options.clone()))?; @@ -385,7 +400,7 @@ fn print_version() { println!("rustfmt {}", version_info); } -fn determine_operation(matches: &Matches) -> Result { +fn determine_operation(matches: &Matches) -> Result { if matches.opt_present("h") { let topic = matches.opt_str("h"); if topic == None { @@ -395,22 +410,25 @@ fn determine_operation(matches: &Matches) -> Result { } else if topic == Some("file-lines".to_owned()) { return Ok(Operation::Help(HelpOp::FileLines)); } else { - println!("Unknown help topic: `{}`\n", topic.unwrap()); - return Ok(Operation::Help(HelpOp::None)); + return Err(OperationError::UnknownHelpTopic(topic.unwrap())); } } + let mut free_matches = matches.free.iter(); let mut minimal_config_path = None; - if let Some(ref kind) = matches.opt_str("print-config") { - let path = matches.free.get(0).cloned(); - if kind == "default" { - return Ok(Operation::ConfigOutputDefault { path }); - } else if kind == "current" { - return Ok(Operation::ConfigOutputCurrent { path }); - } else if kind == "minimal" { - minimal_config_path = path; - if minimal_config_path.is_none() { - println!("WARNING: PATH required for `--print-config minimal`"); + if let Some(kind) = matches.opt_str("print-config") { + let path = free_matches.next().cloned(); + match kind.as_str() { + "default" => return Ok(Operation::ConfigOutputDefault { path }), + "current" => return Ok(Operation::ConfigOutputCurrent { path }), + "minimal" => { + minimal_config_path = path; + if minimal_config_path.is_none() { + eprintln!("WARNING: PATH required for `--print-config minimal`."); + } + } + _ => { + return Err(OperationError::UnknownPrintConfigTopic(kind)); } } } @@ -419,17 +437,7 @@ fn determine_operation(matches: &Matches) -> Result { return Ok(Operation::Version); } - // if no file argument is supplied, read from stdin - if matches.free.is_empty() { - let mut buffer = String::new(); - io::stdin().read_to_string(&mut buffer)?; - - return Ok(Operation::Stdin { input: buffer }); - } - - let files: Vec<_> = matches - .free - .iter() + let files: Vec<_> = free_matches .map(|s| { let p = PathBuf::from(s); // we will do comparison later, so here tries to canonicalize first @@ -438,6 +446,17 @@ fn determine_operation(matches: &Matches) -> Result { }) .collect(); + // if no file argument is supplied, read from stdin + if files.is_empty() { + if minimal_config_path.is_some() { + return Err(OperationError::MinimalPathWithStdin); + } + let mut buffer = String::new(); + io::stdin().read_to_string(&mut buffer)?; + + return Ok(Operation::Stdin { input: buffer }); + } + Ok(Operation::Format { files, minimal_config_path, @@ -464,7 +483,7 @@ struct GetOptsOptions { } impl GetOptsOptions { - pub fn from_matches(matches: &Matches) -> Result { + pub fn from_matches(matches: &Matches) -> Result { let mut options = GetOptsOptions::default(); options.verbose = matches.opt_present("verbose"); options.quiet = matches.opt_present("quiet"); @@ -598,7 +617,7 @@ impl CliOptions for GetOptsOptions { } } -fn edition_from_edition_str(edition_str: &str) -> Result { +fn edition_from_edition_str(edition_str: &str) -> Result { match edition_str { "2015" => Ok(Edition::Edition2015), "2018" => Ok(Edition::Edition2018), @@ -606,7 +625,7 @@ fn edition_from_edition_str(edition_str: &str) -> Result Result { +fn emit_mode_from_emit_str(emit_str: &str) -> Result { match emit_str { "files" => Ok(EmitMode::Files), "stdout" => Ok(EmitMode::Stdout), diff --git a/tests/rustfmt/main.rs b/tests/rustfmt/main.rs new file mode 100644 index 00000000000..51f311cd08d --- /dev/null +++ b/tests/rustfmt/main.rs @@ -0,0 +1,77 @@ +//! Integration tests for rustfmt. + +use std::env; +use std::fs::remove_file; +use std::path::Path; +use std::process::Command; + +/// Run the rustfmt executable and return its output. +fn rustfmt(args: &[&str]) -> (String, String) { + let mut bin_dir = env::current_exe().unwrap(); + bin_dir.pop(); // chop off test exe name + if bin_dir.ends_with("deps") { + bin_dir.pop(); + } + let cmd = bin_dir.join(format!("rustfmt{}", env::consts::EXE_SUFFIX)); + + // Ensure the rustfmt binary runs from the local target dir. + let path = env::var_os("PATH").unwrap_or_default(); + let mut paths = env::split_paths(&path).collect::>(); + paths.insert(0, bin_dir); + let new_path = env::join_paths(paths).unwrap(); + + match Command::new(&cmd).args(args).env("PATH", new_path).output() { + Ok(output) => ( + String::from_utf8(output.stdout).expect("utf-8"), + String::from_utf8(output.stderr).expect("utf-8"), + ), + Err(e) => panic!("failed to run `{:?} {:?}`: {}", cmd, args, e), + } +} + +macro_rules! assert_that { + ($args:expr, $check:ident $check_args:tt) => { + let (stdout, stderr) = rustfmt($args); + if !stdout.$check$check_args && !stderr.$check$check_args { + panic!( + "Output not expected for rustfmt {:?}\n\ + expected: {}{}\n\ + actual stdout:\n{}\n\ + actual stderr:\n{}", + $args, + stringify!($check), + stringify!($check_args), + stdout, + stderr + ); + } + }; +} + +#[test] +fn print_config() { + assert_that!( + &["--print-config", "unknown"], + starts_with("Unknown print-config option") + ); + assert_that!(&["--print-config", "default"], contains("max_width = 100")); + assert_that!(&["--print-config", "minimal"], contains("PATH required")); + assert_that!( + &["--print-config", "minimal", "minimal-config"], + contains("doesn't work with standard input.") + ); + + let (stdout, stderr) = rustfmt(&[ + "--print-config", + "minimal", + "minimal-config", + "src/shape.rs", + ]); + assert!( + Path::new("minimal-config").exists(), + "stdout:\n{}\nstderr:\n{}", + stdout, + stderr + ); + remove_file("minimal-config").unwrap(); +} From 4b54abdf35af4fcb28514fc83647f9ca887e140b Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Mon, 15 Jul 2019 22:41:56 +0900 Subject: [PATCH 19/24] Fix bugs related to file-lines (#3684) --- src/bin/main.rs | 2 +- src/config/file_lines.rs | 13 ++- src/missed_spans.rs | 50 +++++------- src/source_map.rs | 3 +- src/utils.rs | 17 +++- src/visitor.rs | 140 ++++++++++++++++++-------------- tests/source/file-lines-7.rs | 24 ++++++ tests/source/issue-3494/crlf.rs | 8 ++ tests/source/issue-3494/lf.rs | 8 ++ tests/source/issue-3636.rs | 4 +- tests/target/file-lines-7.rs | 21 +++++ tests/target/issue-3494/crlf.rs | 8 ++ tests/target/issue-3494/lf.rs | 8 ++ tests/target/issue-3636.rs | 4 +- 14 files changed, 211 insertions(+), 99 deletions(-) create mode 100644 tests/source/file-lines-7.rs create mode 100644 tests/source/issue-3494/crlf.rs create mode 100644 tests/source/issue-3494/lf.rs create mode 100644 tests/target/file-lines-7.rs create mode 100644 tests/target/issue-3494/crlf.rs create mode 100644 tests/target/issue-3494/lf.rs diff --git a/src/bin/main.rs b/src/bin/main.rs index 813504cdd6d..70990aa798d 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -167,7 +167,7 @@ fn make_opts() -> Options { } fn is_nightly() -> bool { - option_env!("CFG_RELEASE_CHANNEL").map_or(false, |c| c == "nightly" || c == "dev") + option_env!("CFG_RELEASE_CHANNEL").map_or(true, |c| c == "nightly" || c == "dev") } // Returned i32 is an exit code diff --git a/src/config/file_lines.rs b/src/config/file_lines.rs index f0dc6c66597..f58d3c16ac2 100644 --- a/src/config/file_lines.rs +++ b/src/config/file_lines.rs @@ -9,7 +9,7 @@ use std::{cmp, fmt, iter, str}; use serde::{ser, Deserialize, Deserializer, Serialize, Serializer}; use serde_json as json; -use syntax::source_map::{self, SourceFile}; +use syntax::source_map::{self, SourceFile, SourceMap, Span}; /// A range of lines in a file, inclusive of both ends. pub struct LineRange { @@ -78,6 +78,17 @@ impl LineRange { pub fn file_name(&self) -> FileName { self.file.name.clone().into() } + + pub(crate) fn from_span(source_map: &SourceMap, span: Span) -> LineRange { + let lo_char_pos = source_map.lookup_char_pos(span.lo()); + let hi_char_pos = source_map.lookup_char_pos(span.hi()); + debug_assert!(lo_char_pos.file.name == hi_char_pos.file.name); + LineRange { + file: lo_char_pos.file.clone(), + lo: lo_char_pos.line, + hi: hi_char_pos.line, + } + } } /// A range that is inclusive of both ends. diff --git a/src/missed_spans.rs b/src/missed_spans.rs index 84246e3ed51..8ac05457ec9 100644 --- a/src/missed_spans.rs +++ b/src/missed_spans.rs @@ -7,7 +7,7 @@ use crate::config::file_lines::FileLines; use crate::config::{EmitMode, FileName}; use crate::shape::{Indent, Shape}; use crate::source_map::LineRangeUtils; -use crate::utils::{count_newlines, last_line_width, mk_sp}; +use crate::utils::{count_lf_crlf, count_newlines, last_line_width, mk_sp}; use crate::visitor::FmtVisitor; struct SnippetStatus { @@ -157,7 +157,7 @@ impl<'a> FmtVisitor<'a> { fn write_snippet_inner( &mut self, big_snippet: &str, - mut big_diff: usize, + big_diff: usize, old_snippet: &str, span: Span, process_last_snippet: F, @@ -176,36 +176,26 @@ impl<'a> FmtVisitor<'a> { _ => Cow::from(old_snippet), }; - // if the snippet starts with a new line, then information about the lines needs to be - // adjusted since it is off by 1. - let snippet = if snippet.starts_with('\n') { - // this takes into account the blank_lines_* options - self.push_vertical_spaces(1); - // include the newline character into the big_diff - big_diff += 1; - status.cur_line += 1; - &snippet[1..] - } else { - snippet - }; - - let slice_within_file_lines_range = |file_lines: FileLines, cur_line, s| -> (usize, bool) { - let newline_count = count_newlines(s); - let within_file_lines_range = file_lines.contains_range( - file_name, - cur_line, - // if a newline character is at the end of the slice, then the number of newlines - // needs to be decreased by 1 so that the range checked against the file_lines is - // the visual range one would expect. - cur_line + newline_count - if s.ends_with('\n') { 1 } else { 0 }, - ); - (newline_count, within_file_lines_range) - }; + let slice_within_file_lines_range = + |file_lines: FileLines, cur_line, s| -> (usize, usize, bool) { + let (lf_count, crlf_count) = count_lf_crlf(s); + let newline_count = lf_count + crlf_count; + let within_file_lines_range = file_lines.contains_range( + file_name, + cur_line, + // if a newline character is at the end of the slice, then the number of + // newlines needs to be decreased by 1 so that the range checked against + // the file_lines is the visual range one would expect. + cur_line + newline_count - if s.ends_with('\n') { 1 } else { 0 }, + ); + (lf_count, crlf_count, within_file_lines_range) + }; for (kind, offset, subslice) in CommentCodeSlices::new(snippet) { debug!("{:?}: {:?}", kind, subslice); - let (newline_count, within_file_lines_range) = + let (lf_count, crlf_count, within_file_lines_range) = slice_within_file_lines_range(self.config.file_lines(), status.cur_line, subslice); + let newline_count = lf_count + crlf_count; if CodeCharKind::Comment == kind && within_file_lines_range { // 1: comment. self.process_comment( @@ -219,7 +209,7 @@ impl<'a> FmtVisitor<'a> { // 2: blank lines. self.push_vertical_spaces(newline_count); status.cur_line += newline_count; - status.line_start = offset + newline_count; + status.line_start = offset + lf_count + crlf_count * 2; } else { // 3: code which we failed to format or which is not within file-lines range. self.process_missing_code(&mut status, snippet, subslice, offset, file_name); @@ -227,7 +217,7 @@ impl<'a> FmtVisitor<'a> { } let last_snippet = &snippet[status.line_start..]; - let (_, within_file_lines_range) = + let (_, _, within_file_lines_range) = slice_within_file_lines_range(self.config.file_lines(), status.cur_line, last_snippet); if within_file_lines_range { process_last_snippet(self, last_snippet, snippet); diff --git a/src/source_map.rs b/src/source_map.rs index 757d5f57db3..b1ab83e7aba 100644 --- a/src/source_map.rs +++ b/src/source_map.rs @@ -5,6 +5,7 @@ use syntax::source_map::{BytePos, SourceMap, Span}; use crate::comment::FindUncommented; use crate::config::file_lines::LineRange; +use crate::utils::starts_with_newline; use crate::visitor::SnippetProvider; pub(crate) trait SpanUtils { @@ -95,7 +96,7 @@ impl LineRangeUtils for SourceMap { // in case the span starts with a newline, the line range is off by 1 without the // adjustment below - let offset = 1 + if snippet.starts_with('\n') { 1 } else { 0 }; + let offset = 1 + if starts_with_newline(&snippet) { 1 } else { 0 }; // Line numbers start at 1 LineRange { file: lo.sf.clone(), diff --git a/src/utils.rs b/src/utils.rs index d47e528d485..7a432c0f7d8 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -306,7 +306,22 @@ pub(crate) fn stmt_expr(stmt: &ast::Stmt) -> Option<&ast::Expr> { } } -#[inline] +/// Returns the number of LF and CRLF respectively. +pub(crate) fn count_lf_crlf(input: &str) -> (usize, usize) { + let mut lf = 0; + let mut crlf = 0; + let mut is_crlf = false; + for c in input.as_bytes() { + match c { + b'\r' => is_crlf = true, + b'\n' if is_crlf => crlf += 1, + b'\n' => lf += 1, + _ => is_crlf = false, + } + } + (lf, crlf) +} + pub(crate) fn count_newlines(input: &str) -> usize { // Using bytes to omit UTF-8 decoding bytecount::count(input.as_bytes(), b'\n') diff --git a/src/visitor.rs b/src/visitor.rs index 1e4abd9d139..7002d28b3e3 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -6,7 +6,7 @@ use syntax::{ast, visit}; use crate::attr::*; use crate::comment::{CodeCharKind, CommentCodeSlices}; -use crate::config::file_lines::FileName; +use crate::config::file_lines::LineRange; use crate::config::{BraceStyle, Config}; use crate::items::{ format_impl, format_trait, format_trait_alias, is_mod_decl, is_use_item, @@ -90,6 +90,10 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { Shape::indented(self.block_indent, self.config) } + fn next_span(&self, hi: BytePos) -> Span { + mk_sp(self.last_pos, hi) + } + fn visit_stmt(&mut self, stmt: &Stmt<'_>) { debug!( "visit_stmt: {:?} {:?}", @@ -133,31 +137,19 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { } } - pub(crate) fn visit_block( + /// Remove spaces between the opening brace and the first statement or the inner attribute + /// of the block. + fn trim_spaces_after_opening_brace( &mut self, b: &ast::Block, inner_attrs: Option<&[ast::Attribute]>, - has_braces: bool, ) { - debug!( - "visit_block: {:?} {:?}", - self.source_map.lookup_char_pos(b.span.lo()), - self.source_map.lookup_char_pos(b.span.hi()) - ); - - // Check if this block has braces. - let brace_compensation = BytePos(if has_braces { 1 } else { 0 }); - - self.last_pos = self.last_pos + brace_compensation; - self.block_indent = self.block_indent.block_indent(self.config); - self.push_str("{"); - if let Some(first_stmt) = b.stmts.first() { let hi = inner_attrs .and_then(|attrs| inner_attributes(attrs).first().map(|attr| attr.span.lo())) .unwrap_or_else(|| first_stmt.span().lo()); - - let snippet = self.snippet(mk_sp(self.last_pos, hi)); + let missing_span = self.next_span(hi); + let snippet = self.snippet(missing_span); let len = CommentCodeSlices::new(snippet) .nth(0) .and_then(|(kind, _, s)| { @@ -171,19 +163,57 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.last_pos = self.last_pos + BytePos::from_usize(len); } } + } - // Format inner attributes if available. - let skip_rewrite = if let Some(attrs) = inner_attrs { - self.visit_attrs(attrs, ast::AttrStyle::Inner) - } else { - false - }; + /// Returns the total length of the spaces which should be trimmed between the last statement + /// and the closing brace of the block. + fn trimmed_spaces_width_before_closing_brace( + &mut self, + b: &ast::Block, + brace_compensation: BytePos, + ) -> usize { + match b.stmts.last() { + None => 0, + Some(..) => { + let span_after_last_stmt = self.next_span(b.span.hi() - brace_compensation); + let missing_snippet = self.snippet(span_after_last_stmt); + CommentCodeSlices::new(missing_snippet) + .last() + .and_then(|(kind, _, s)| { + if kind == CodeCharKind::Normal && s.trim().is_empty() { + Some(s.len()) + } else { + None + } + }) + .unwrap_or(0) + } + } + } - if skip_rewrite { - self.push_rewrite(b.span, None); - self.close_block(false, b.span); - self.last_pos = source!(self, b.span).hi(); - return; + pub(crate) fn visit_block( + &mut self, + b: &ast::Block, + inner_attrs: Option<&[ast::Attribute]>, + has_braces: bool, + ) { + debug!( + "visit_block: {:?} {:?}", + self.source_map.lookup_char_pos(b.span.lo()), + self.source_map.lookup_char_pos(b.span.hi()) + ); + + // Check if this block has braces. + let brace_compensation = BytePos(if has_braces { 1 } else { 0 }); + + self.last_pos = self.last_pos + brace_compensation; + self.block_indent = self.block_indent.block_indent(self.config); + self.push_str("{"); + self.trim_spaces_after_opening_brace(b, inner_attrs); + + // Format inner attributes if available. + if let Some(attrs) = inner_attrs { + self.visit_attrs(attrs, ast::AttrStyle::Inner); } self.walk_block_stmts(b); @@ -196,36 +226,22 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { } } - let mut remove_len = BytePos(0); - if let Some(stmt) = b.stmts.last() { - let span_after_last_stmt = mk_sp( - stmt.span.hi(), - source!(self, b.span).hi() - brace_compensation, - ); - // if the span is outside of a file_lines range, then do not try to remove anything - if !out_of_file_lines_range!(self, span_after_last_stmt) { - let snippet = self.snippet(span_after_last_stmt); - let len = CommentCodeSlices::new(snippet) - .last() - .and_then(|(kind, _, s)| { - if kind == CodeCharKind::Normal && s.trim().is_empty() { - Some(s.len()) - } else { - None - } - }); - if let Some(len) = len { - remove_len = BytePos::from_usize(len); - } - } + let missing_span = self.next_span(b.span.hi()); + if out_of_file_lines_range!(self, missing_span) { + self.push_str(self.snippet(missing_span)); + self.block_indent = self.block_indent.block_unindent(self.config); + self.last_pos = source!(self, b.span).hi(); + return; } + let remove_len = BytePos::from_usize( + self.trimmed_spaces_width_before_closing_brace(b, brace_compensation), + ); let unindent_comment = self.is_if_else_block && !b.stmts.is_empty() && { let end_pos = source!(self, b.span).hi() - brace_compensation - remove_len; let snippet = self.snippet(mk_sp(self.last_pos, end_pos)); snippet.contains("//") || snippet.contains("/*") }; - // FIXME: we should compress any newlines here to just one if unindent_comment { self.block_indent = self.block_indent.block_unindent(self.config); } @@ -235,7 +251,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { if unindent_comment { self.block_indent = self.block_indent.block_indent(self.config); } - self.close_block(unindent_comment, b.span); + self.close_block(unindent_comment, self.next_span(b.span.hi())); self.last_pos = source!(self, b.span).hi(); } @@ -244,12 +260,13 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { // The closing brace itself, however, should be indented at a shallower // level. fn close_block(&mut self, unindent_comment: bool, span: Span) { - let file_name: FileName = self.source_map.span_to_filename(span).into(); let skip_this_line = !self .config .file_lines() - .contains_line(&file_name, self.line_number); - if !skip_this_line { + .contains(&LineRange::from_span(self.source_map, span)); + if skip_this_line { + self.push_str(self.snippet(span)); + } else { let total_len = self.buffer.len(); let chars_too_many = if unindent_comment { 0 @@ -272,8 +289,8 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.buffer.push_str("\n"); } } + self.push_str("}"); } - self.push_str("}"); self.block_indent = self.block_indent.block_unindent(self.config); } @@ -789,8 +806,9 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.block_indent = self.block_indent.block_indent(self.config); self.visit_attrs(attrs, ast::AttrStyle::Inner); self.walk_mod_items(m); - self.format_missing_with_indent(source!(self, m.inner).hi() - BytePos(1)); - self.close_block(false, m.inner); + let missing_span = mk_sp(source!(self, m.inner).hi() - BytePos(1), m.inner.hi()); + self.format_missing_with_indent(missing_span.lo()); + self.close_block(false, missing_span); } self.last_pos = source!(self, m.inner).hi(); } else { @@ -812,9 +830,9 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { pub(crate) fn skip_empty_lines(&mut self, end_pos: BytePos) { while let Some(pos) = self .snippet_provider - .opt_span_after(mk_sp(self.last_pos, end_pos), "\n") + .opt_span_after(self.next_span(end_pos), "\n") { - if let Some(snippet) = self.opt_snippet(mk_sp(self.last_pos, pos)) { + if let Some(snippet) = self.opt_snippet(self.next_span(pos)) { if snippet.trim().is_empty() { self.last_pos = pos; } else { diff --git a/tests/source/file-lines-7.rs b/tests/source/file-lines-7.rs new file mode 100644 index 00000000000..b227ac35dcc --- /dev/null +++ b/tests/source/file-lines-7.rs @@ -0,0 +1,24 @@ +// rustfmt-file_lines: [{"file":"tests/source/file-lines-7.rs","range":[8,15]}] + +struct A { + t: i64, +} + +mod foo { + fn bar() { + + + + // test + let i = 12; + // test + } + + fn baz() { + + + + /// + let j = 15; + } +} diff --git a/tests/source/issue-3494/crlf.rs b/tests/source/issue-3494/crlf.rs new file mode 100644 index 00000000000..9ce457c7b06 --- /dev/null +++ b/tests/source/issue-3494/crlf.rs @@ -0,0 +1,8 @@ +// rustfmt-file_lines: [{"file":"tests/source/issue-3494/crlf.rs","range":[4,5]}] + +pub fn main() +{ +let world1 = "world"; println!("Hello, {}!", world1); +let world2 = "world"; println!("Hello, {}!", world2); +let world3 = "world"; println!("Hello, {}!", world3); +} diff --git a/tests/source/issue-3494/lf.rs b/tests/source/issue-3494/lf.rs new file mode 100644 index 00000000000..bdbe69cef4a --- /dev/null +++ b/tests/source/issue-3494/lf.rs @@ -0,0 +1,8 @@ +// rustfmt-file_lines: [{"file":"tests/source/issue-3494/lf.rs","range":[4,5]}] + +pub fn main() +{ +let world1 = "world"; println!("Hello, {}!", world1); +let world2 = "world"; println!("Hello, {}!", world2); +let world3 = "world"; println!("Hello, {}!", world3); +} diff --git a/tests/source/issue-3636.rs b/tests/source/issue-3636.rs index 80355bbd2ef..edfa03012f2 100644 --- a/tests/source/issue-3636.rs +++ b/tests/source/issue-3636.rs @@ -1,4 +1,4 @@ -// rustfmt-file_lines: [{"file":"tests/source/issue-3636.rs","range":[4,7]}] +// rustfmt-file_lines: [{"file":"tests/source/issue-3636.rs","range":[4,7]},{"file":"tests/target/issue-3636.rs","range":[3,6]}] fn foo() { let x = @@ -7,4 +7,4 @@ fn foo() { 42; let z = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; let z = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; -} \ No newline at end of file +} diff --git a/tests/target/file-lines-7.rs b/tests/target/file-lines-7.rs new file mode 100644 index 00000000000..62d913d8823 --- /dev/null +++ b/tests/target/file-lines-7.rs @@ -0,0 +1,21 @@ +// rustfmt-file_lines: [{"file":"tests/source/file-lines-7.rs","range":[8,15]}] + +struct A { + t: i64, +} + +mod foo { + fn bar() { + // test + let i = 12; + // test + } + + fn baz() { + + + + /// + let j = 15; + } +} diff --git a/tests/target/issue-3494/crlf.rs b/tests/target/issue-3494/crlf.rs new file mode 100644 index 00000000000..cae615a0666 --- /dev/null +++ b/tests/target/issue-3494/crlf.rs @@ -0,0 +1,8 @@ +// rustfmt-file_lines: [{"file":"tests/source/issue-3494/crlf.rs","range":[4,5]}] + +pub fn main() { + let world1 = "world"; + println!("Hello, {}!", world1); +let world2 = "world"; println!("Hello, {}!", world2); +let world3 = "world"; println!("Hello, {}!", world3); +} diff --git a/tests/target/issue-3494/lf.rs b/tests/target/issue-3494/lf.rs new file mode 100644 index 00000000000..60aafe19a02 --- /dev/null +++ b/tests/target/issue-3494/lf.rs @@ -0,0 +1,8 @@ +// rustfmt-file_lines: [{"file":"tests/source/issue-3494/lf.rs","range":[4,5]}] + +pub fn main() { + let world1 = "world"; + println!("Hello, {}!", world1); +let world2 = "world"; println!("Hello, {}!", world2); +let world3 = "world"; println!("Hello, {}!", world3); +} diff --git a/tests/target/issue-3636.rs b/tests/target/issue-3636.rs index d8d78e88d0e..d467ed7389f 100644 --- a/tests/target/issue-3636.rs +++ b/tests/target/issue-3636.rs @@ -1,8 +1,8 @@ -// rustfmt-file_lines: [{"file":"tests/source/issue-3636.rs","range":[4,7]}] +// rustfmt-file_lines: [{"file":"tests/source/issue-3636.rs","range":[4,7]},{"file":"tests/target/issue-3636.rs","range":[3,6]}] fn foo() { let x = 42; let y = 42; let z = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; - let z = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + let z = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; } From ac13dbc1413c6d2680ade4edc9ee3663888da12a Mon Sep 17 00:00:00 2001 From: topecongiro Date: Mon, 15 Jul 2019 22:44:44 +0900 Subject: [PATCH 20/24] Update CHANGELOG.md --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fa24eee62c..52ef2d1d8e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ ## [Unreleased] +## [1.3.3] 2019-07-15 + +### Fixed + +- Fix `cargo fmt -- --help` printing nothing (#3620). +- Fix inserting an extra comma (#3677). +- Fix incorrect handling of CRLF with `file-lines` (#3684). +- Fix `print-config=minimal` option (#3687). + ## [1.3.2] 2019-07-06 ### Fixed From bcd05bd66c634685024474581072884cac7b53eb Mon Sep 17 00:00:00 2001 From: topecongiro Date: Mon, 15 Jul 2019 22:47:10 +0900 Subject: [PATCH 21/24] Release 1.3.3 --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/config/mod.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9568dbcfd56..03468ecf517 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -838,7 +838,7 @@ dependencies = [ [[package]] name = "rustfmt-nightly" -version = "1.3.2" +version = "1.3.3" dependencies = [ "annotate-snippets 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", "atty 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index 56815a1e5b1..0d55fb283d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "rustfmt-nightly" -version = "1.3.2" +version = "1.3.3" authors = ["Nicholas Cameron ", "The Rustfmt developers"] description = "Tool to find and fix Rust formatting issues" repository = "https://github.com/rust-lang/rustfmt" diff --git a/src/config/mod.rs b/src/config/mod.rs index 01ce1df8a9a..2f006538d48 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -519,7 +519,7 @@ use_field_init_shorthand = false force_explicit_abi = true condense_wildcard_suffixes = false color = "Auto" -required_version = "1.3.2" +required_version = "1.3.3" unstable_features = false disable_all_formatting = false skip_children = false From 7b32170915bd74f139d5fdbc3205cd7905e07a28 Mon Sep 17 00:00:00 2001 From: Michele d'Amico Date: Mon, 15 Jul 2019 17:18:28 +0200 Subject: [PATCH 22/24] #3665 Clean up: - better naming - `skip` module doc - update CHANGELOG - removed useless `*` --- CHANGELOG.md | 5 +++++ src/attr.rs | 2 +- src/formatting.rs | 2 +- src/macros.rs | 2 +- src/skip.rs | 11 ++++++++--- src/visitor.rs | 2 +- 6 files changed, 17 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52ef2d1d8e0..23ce47f816d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ ## [1.3.3] 2019-07-15 +### Added + +- Add new attribute `rustfmt::skip::attributes` to prevent rustfmt +from formatting an attribute #3665 + ### Fixed - Fix `cargo fmt -- --help` printing nothing (#3620). diff --git a/src/attr.rs b/src/attr.rs index 79426c663e8..1c9092ea629 100644 --- a/src/attr.rs +++ b/src/attr.rs @@ -321,7 +321,7 @@ impl Rewrite for ast::Attribute { } else { let should_skip = self .ident() - .map(|s| context.skip_context.attributes_skip(&*s.name.as_str())) + .map(|s| context.skip_context.skip_attribute(&s.name.as_str())) .unwrap_or(false); let prefix = attr_prefix(self); diff --git a/src/formatting.rs b/src/formatting.rs index 62b87b12a5b..879ddc61b13 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -158,7 +158,7 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> { &snippet_provider, self.report.clone(), ); - visitor.skip_context.update_by_attrs(&self.krate.attrs); + visitor.skip_context.update_with_attrs(&self.krate.attrs); // Format inner attributes if available. if !self.krate.attrs.is_empty() && is_root { diff --git a/src/macros.rs b/src/macros.rs index d4f1e880376..7e5ce9deea9 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -212,7 +212,7 @@ pub(crate) fn rewrite_macro( ) -> Option { let should_skip = context .skip_context - .macro_skip(&context.snippet(mac.node.path.span).to_owned()); + .skip_macro(&context.snippet(mac.node.path.span).to_owned()); if should_skip { None } else { diff --git a/src/skip.rs b/src/skip.rs index 436f0c48123..6b4e04a7173 100644 --- a/src/skip.rs +++ b/src/skip.rs @@ -1,5 +1,9 @@ +//! Module that contains skip related stuffs. + use syntax::ast; +/// Take care of skip name stack. You can update it by attributes slice or +/// by other context. Query this context to know if you need skip a block. #[derive(Default, Clone)] pub(crate) struct SkipContext { macros: Vec, @@ -7,7 +11,7 @@ pub(crate) struct SkipContext { } impl SkipContext { - pub(crate) fn update_by_attrs(&mut self, attrs: &[ast::Attribute]) { + pub(crate) fn update_with_attrs(&mut self, attrs: &[ast::Attribute]) { self.macros.append(&mut get_skip_names("macros", attrs)); self.attributes .append(&mut get_skip_names("attributes", attrs)); @@ -18,11 +22,11 @@ impl SkipContext { self.attributes.append(&mut other.attributes); } - pub(crate) fn macro_skip(&self, name: &str) -> bool { + pub(crate) fn skip_macro(&self, name: &str) -> bool { self.macros.iter().any(|n| n == name) } - pub(crate) fn attributes_skip(&self, name: &str) -> bool { + pub(crate) fn skip_attribute(&self, name: &str) -> bool { self.attributes.iter().any(|n| n == name) } } @@ -30,6 +34,7 @@ impl SkipContext { static RUSTFMT: &'static str = "rustfmt"; static SKIP: &'static str = "skip"; +/// Say if you're playing with `rustfmt`'s skip attribute pub(crate) fn is_skip_attr(segments: &[ast::PathSegment]) -> bool { if segments.len() < 2 || segments[0].ident.to_string() != RUSTFMT { return false; diff --git a/src/visitor.rs b/src/visitor.rs index 7002d28b3e3..35291cecfb5 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -349,7 +349,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { let filtered_attrs; let mut attrs = &item.attrs; let skip_context_saved = self.skip_context.clone(); - self.skip_context.update_by_attrs(&attrs); + self.skip_context.update_with_attrs(&attrs); let should_visit_node_again = match item.node { // For use/extern crate items, skip rewriting attributes but check for a skip attribute. From 64e82b76a659de770cc1af5e022f7cb5db3114aa Mon Sep 17 00:00:00 2001 From: Michele d'Amico Date: Mon, 15 Jul 2019 17:18:28 +0200 Subject: [PATCH 23/24] #3665 Clean up: - better naming - `skip` module doc - update CHANGELOG - removed useless `*` --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c157c6f092e..76a364cb031 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ - Add new attribute `rustfmt::skip::attributes` to prevent rustfmt from formatting an attribute #3665 - Add `--manifest-path` support to `cargo fmt` (#3683). +- Add new attribute `rustfmt::skip::attributes` to prevent rustfmt +from formatting an attribute #3665 ### Fixed From 3c5df8a4e1301225d75a87668da11612efbd4df3 Mon Sep 17 00:00:00 2001 From: michele Date: Mon, 15 Jul 2019 21:12:10 +0200 Subject: [PATCH 24/24] #3665: Clean CHANGELOG --- CHANGELOG.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76a364cb031..c157c6f092e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,6 @@ - Add new attribute `rustfmt::skip::attributes` to prevent rustfmt from formatting an attribute #3665 - Add `--manifest-path` support to `cargo fmt` (#3683). -- Add new attribute `rustfmt::skip::attributes` to prevent rustfmt -from formatting an attribute #3665 ### Fixed