From 164d5668200671a6e24cd55e927100d8bf21bd56 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Tue, 10 Nov 2020 20:07:42 -0600 Subject: [PATCH 1/2] feat: don't insert semi in macro_rules arm body --- src/comment.rs | 2 +- src/formatting.rs | 20 +++++++++++++++----- src/lib.rs | 24 ++++++++++++++---------- src/macros.rs | 4 ++-- src/rewrite.rs | 1 + src/utils.rs | 7 +++++++ src/visitor.rs | 3 +++ tests/target/macro_rules_semi.rs | 22 ++++++++++++++++++++++ 8 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 tests/target/macro_rules_semi.rs diff --git a/src/comment.rs b/src/comment.rs index 1da62d17681..9959ef7fd9e 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -659,7 +659,7 @@ impl<'a> CommentRewrite<'a> { config.set().wrap_comments(false); if config.format_code_in_doc_comments() { if let Some(s) = - crate::format_code_block(&self.code_block_buffer, &config) + crate::format_code_block(&self.code_block_buffer, &config, false) { trim_custom_comment_prefix(&s.snippet) } else { diff --git a/src/formatting.rs b/src/formatting.rs index 4da89e760ad..26ae494227d 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -25,7 +25,11 @@ pub(crate) type SourceFile = Vec; pub(crate) type FileRecord = (FileName, String); impl<'b, T: Write + 'b> Session<'b, T> { - pub(crate) fn format_input_inner(&mut self, input: Input) -> Result { + pub(crate) fn format_input_inner( + &mut self, + input: Input, + is_macro_def: bool, + ) -> Result { if !self.config.version_meets_requirement() { return Err(ErrorKind::VersionMismatch); } @@ -42,7 +46,7 @@ impl<'b, T: Write + 'b> Session<'b, T> { } let config = &self.config.clone(); - let format_result = format_project(input, config, self); + let format_result = format_project(input, config, self, is_macro_def); format_result.map(|report| { self.errors.add(&report.internal.borrow().1); @@ -57,6 +61,7 @@ fn format_project( input: Input, config: &Config, handler: &mut T, + is_macro_def: bool, ) -> Result { let mut timer = Timer::start(); @@ -103,7 +108,7 @@ fn format_project( continue; } should_emit_verbose(input_is_stdin, config, || println!("Formatting {}", path)); - context.format_file(path, &module)?; + context.format_file(path, &module, is_macro_def)?; } timer = timer.done_formatting(); @@ -134,7 +139,12 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> { } // Formats a single file/module. - fn format_file(&mut self, path: FileName, module: &Module<'_>) -> Result<(), ErrorKind> { + fn format_file( + &mut self, + path: FileName, + module: &Module<'_>, + is_macro_def: bool, + ) -> Result<(), ErrorKind> { let snippet_provider = self.parse_session.snippet_provider(module.as_ref().inner); let mut visitor = FmtVisitor::from_parse_sess( &self.parse_session, @@ -143,7 +153,7 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> { self.report.clone(), ); visitor.skip_context.update_with_attrs(&self.krate.attrs); - + visitor.is_macro_def = is_macro_def; visitor.last_pos = snippet_provider.start_pos(); visitor.skip_empty_lines(snippet_provider.end_pos()); visitor.format_separate_mod(module, snippet_provider.end_pos()); diff --git a/src/lib.rs b/src/lib.rs index c6a9654d4d1..753840e065c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -286,7 +286,7 @@ impl fmt::Display for FormatReport { /// Format the given snippet. The snippet is expected to be *complete* code. /// When we cannot parse the given snippet, this function returns `None`. -fn format_snippet(snippet: &str, config: &Config) -> Option { +fn format_snippet(snippet: &str, config: &Config, is_macro_def: bool) -> Option { let mut config = config.clone(); panic::catch_unwind(|| { let mut out: Vec = Vec::with_capacity(snippet.len() * 2); @@ -297,7 +297,7 @@ fn format_snippet(snippet: &str, config: &Config) -> Option { let (formatting_error, result) = { let input = Input::Text(snippet.into()); let mut session = Session::new(config, Some(&mut out)); - let result = session.format(input); + let result = session.format_input_inner(input, is_macro_def); ( session.errors.has_macro_format_failure || session.out.as_ref().unwrap().is_empty() && !snippet.is_empty() @@ -323,7 +323,11 @@ fn format_snippet(snippet: &str, config: &Config) -> Option { /// The code block may be incomplete (i.e., parser may be unable to parse it). /// To avoid panic in parser, we wrap the code block with a dummy function. /// The returned code block does **not** end with newline. -fn format_code_block(code_snippet: &str, config: &Config) -> Option { +fn format_code_block( + code_snippet: &str, + config: &Config, + is_macro_def: bool, +) -> Option { const FN_MAIN_PREFIX: &str = "fn main() {\n"; fn enclose_in_main_block(s: &str, config: &Config) -> String { @@ -356,7 +360,7 @@ fn format_code_block(code_snippet: &str, config: &Config) -> Option Session<'b, T> { /// The main entry point for Rustfmt. Formats the given input according to the /// given config. `out` is only necessary if required by the configuration. pub fn format(&mut self, input: Input) -> Result { - self.format_input_inner(input) + self.format_input_inner(input, false) } pub fn override_config(&mut self, mut config: Config, f: F) -> U @@ -550,15 +554,15 @@ mod unit_tests { // `format_snippet()` and `format_code_block()` should not panic // even when we cannot parse the given snippet. let snippet = "let"; - assert!(format_snippet(snippet, &Config::default()).is_none()); - assert!(format_code_block(snippet, &Config::default()).is_none()); + assert!(format_snippet(snippet, &Config::default(), false).is_none()); + assert!(format_code_block(snippet, &Config::default(), false).is_none()); } fn test_format_inner(formatter: F, input: &str, expected: &str) -> bool where - F: Fn(&str, &Config) -> Option, + F: Fn(&str, &Config, bool) -> Option, { - let output = formatter(input, &Config::default()); + let output = formatter(input, &Config::default(), false); output.is_some() && output.unwrap().snippet == expected } @@ -580,7 +584,7 @@ mod unit_tests { fn test_format_code_block_fail() { #[rustfmt::skip] let code_block = "this_line_is_100_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(x, y, z);"; - assert!(format_code_block(code_block, &Config::default()).is_none()); + assert!(format_code_block(code_block, &Config::default(), false).is_none()); } #[test] diff --git a/src/macros.rs b/src/macros.rs index c4e2b6ac748..3bff059984f 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -1363,12 +1363,12 @@ impl MacroBranch { config.set().max_width(new_width); // First try to format as items, then as statements. - let new_body_snippet = match crate::format_snippet(&body_str, &config) { + let new_body_snippet = match crate::format_snippet(&body_str, &config, true) { Some(new_body) => new_body, None => { let new_width = new_width + config.tab_spaces(); config.set().max_width(new_width); - match crate::format_code_block(&body_str, &config) { + match crate::format_code_block(&body_str, &config, true) { Some(new_body) => new_body, None => return None, } diff --git a/src/rewrite.rs b/src/rewrite.rs index 86b2e9ded1c..c8abe70141b 100644 --- a/src/rewrite.rs +++ b/src/rewrite.rs @@ -39,6 +39,7 @@ pub(crate) struct RewriteContext<'a> { pub(crate) snippet_provider: &'a SnippetProvider, // Used for `format_snippet` pub(crate) macro_rewrite_failure: Cell, + pub(crate) is_macro_def: bool, pub(crate) report: FormatReport, pub(crate) skip_context: SkipContext, pub(crate) skipped_range: Rc>>, diff --git a/src/utils.rs b/src/utils.rs index f8867914937..96d465608fa 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -279,6 +279,13 @@ pub(crate) fn contains_skip(attrs: &[Attribute]) -> bool { #[inline] pub(crate) fn semicolon_for_expr(context: &RewriteContext<'_>, expr: &ast::Expr) -> bool { + // Never try to insert semicolons on expressions when we're inside + // a macro definition - this can prevent the macro from compiling + // when used in expression position + if context.is_macro_def { + return false; + } + match expr.kind { ast::ExprKind::Ret(..) | ast::ExprKind::Continue(..) | ast::ExprKind::Break(..) => { context.config.trailing_semicolon() diff --git a/src/visitor.rs b/src/visitor.rs index e7fa27aae0c..56d023d03a6 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -86,6 +86,7 @@ pub(crate) struct FmtVisitor<'a> { pub(crate) macro_rewrite_failure: bool, pub(crate) report: FormatReport, pub(crate) skip_context: SkipContext, + pub(crate) is_macro_def: bool, } impl<'a> Drop for FmtVisitor<'a> { @@ -811,6 +812,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { snippet_provider, line_number: 0, skipped_range: Rc::new(RefCell::new(vec![])), + is_macro_def: false, macro_rewrite_failure: false, report, skip_context: Default::default(), @@ -1003,6 +1005,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { force_one_line_chain: Cell::new(false), snippet_provider: self.snippet_provider, macro_rewrite_failure: Cell::new(false), + is_macro_def: self.is_macro_def, report: self.report.clone(), skip_context: self.skip_context.clone(), skipped_range: self.skipped_range.clone(), diff --git a/tests/target/macro_rules_semi.rs b/tests/target/macro_rules_semi.rs new file mode 100644 index 00000000000..84e12d16e6e --- /dev/null +++ b/tests/target/macro_rules_semi.rs @@ -0,0 +1,22 @@ +macro_rules! expr { + (no_semi) => { + return true + }; + (semi) => { + return true; + }; +} + +fn foo() -> bool { + match true { + true => expr!(no_semi), + false if false => { + expr!(semi) + } + false => { + expr!(semi); + } + } +} + +fn main() {} From e88875015c950d4840920a5715d63ac4fb3b68f7 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Tue, 10 Nov 2020 20:18:22 -0600 Subject: [PATCH 2/2] meta: release v1.4.25 --- CHANGELOG.md | 28 ++++++++++++++++++++++++++++ Cargo.lock | 2 +- Cargo.toml | 2 +- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fcaf454dc55..fd44e449e18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ ## [Unreleased] +## [1.4.25] 2020-11-10 + +### Changed + +- Semicolons are no longer automatically inserted on trailing expressions in macro definition arms ([#4507](https://github.com/rust-lang/rustfmt/pull/4507)). This gives the programmer control and discretion over whether there should be semicolons in these scenarios so that potential expansion issues can be avoided. + +### Install/Download Options +- **crates.io package** - *pending* +- **rustup (nightly)** - *pending* +- **GitHub Release Binaries** - [Release v1.4.25](https://github.com/rust-lang/rustfmt/releases/tag/v1.4.25) +- **Build from source** - [Tag v1.4.25](https://github.com/rust-lang/rustfmt/tree/v1.4.25), see instructions for how to [install rustfmt from source][install-from-source] + ## [1.4.24] 2020-11-05 ### Changed @@ -11,6 +23,12 @@ ### Fixed - Remove useless `deprecated` attribute on a trait impl block in the rustfmt lib, as these now trigger errors ([rust-lang/rust/#78626](https://github.com/rust-lang/rust/pull/78626)) +### Install/Download Options +- **crates.io package** - *pending* +- **rustup (nightly)** - Starting in `2020-11-09` +- **GitHub Release Binaries** - [Release v1.4.24](https://github.com/rust-lang/rustfmt/releases/tag/v1.4.24) +- **Build from source** - [Tag v1.4.24](https://github.com/rust-lang/rustfmt/tree/v1.4.24), see instructions for how to [install rustfmt from source][install-from-source] + ## [1.4.23] 2020-10-30 ### Changed @@ -28,6 +46,13 @@ - Preserve comments in empty statements [#4018](https://github.com/rust-lang/rustfmt/issues/4018)) - Indentation on skipped code [#4398](https://github.com/rust-lang/rustfmt/issues/4398)) +### Install/Download Options +- **crates.io package** - *pending* +- **rustup (nightly)** - n/a (superseded by [v1.4.24](#1424-2020-11-05)) +- **GitHub Release Binaries** - [Release v1.4.23](https://github.com/rust-lang/rustfmt/releases/tag/v1.4.23) +- **Build from source** - [Tag v1.4.23](https://github.com/rust-lang/rustfmt/tree/v1.4.23), see instructions for how to [install rustfmt from source][install-from-source] + + ## [1.4.22] 2020-10-04 @@ -919,3 +944,6 @@ from formatting an attribute #3665 - Handle tabs properly inside macro with braces (#1918). - Fix a typo in `compute_budgets_for_args()` (#1924). - Recover comment between keyword (`impl` and `trait`) and `{` which used to get removed (#1925). + + +[install-from-source]: https://github.com/rust-lang/rustfmt#installing-from-source diff --git a/Cargo.lock b/Cargo.lock index d7a586aacee..fa2aa1006ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1237,7 +1237,7 @@ dependencies = [ [[package]] name = "rustfmt-nightly" -version = "1.4.24" +version = "1.4.25" dependencies = [ "annotate-snippets 0.6.1", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index 23df8f28c16..0f5bec2dcca 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "rustfmt-nightly" -version = "1.4.24" +version = "1.4.25" authors = ["Nicholas Cameron ", "The Rustfmt developers"] description = "Tool to find and fix Rust formatting issues" repository = "https://github.com/rust-lang/rustfmt"