From 3d245875cbc3687d3fe7bd5e2ad1a7d37c94e3ed Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Sat, 15 Aug 2020 13:38:06 -0700 Subject: [PATCH 1/6] fixup! Correctly create artificial span for formatting closure body --- tests/source/issue-4382.rs | 4 ++++ tests/target/issue-4382.rs | 10 ++++++++++ 2 files changed, 14 insertions(+) create mode 100644 tests/source/issue-4382.rs create mode 100644 tests/target/issue-4382.rs diff --git a/tests/source/issue-4382.rs b/tests/source/issue-4382.rs new file mode 100644 index 00000000000..cbf0c4ed6d4 --- /dev/null +++ b/tests/source/issue-4382.rs @@ -0,0 +1,4 @@ +pub const NAME_MAX: usize = { + #[cfg(target_os = "linux")] { 1024 } + #[cfg(target_os = "freebsd")] { 255 } +}; diff --git a/tests/target/issue-4382.rs b/tests/target/issue-4382.rs new file mode 100644 index 00000000000..740fa9bfe0a --- /dev/null +++ b/tests/target/issue-4382.rs @@ -0,0 +1,10 @@ +pub const NAME_MAX: usize = { + #[cfg(target_os = "linux")] + { + 1024 + } + #[cfg(target_os = "freebsd")] + { + 255 + } +}; From 42eb092360474dfd5677d3d7363bbeb00aca7610 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Sat, 15 Aug 2020 13:34:06 -0700 Subject: [PATCH 2/6] Correctly create artificial span for formatting closure body This commit partially reverts #3934, opting to create a span that covers the entire body of a closure when formatting a closure body with a block-formatting strategy, rather than having the block-formatting code determine if the visitor pointer should be rewound. The problem with rewinding the visitor pointer is it may be incorrect for other (i.e. non-artificial) AST nodes, as in the case of #4382. Closes #4382 --- src/closures.rs | 7 ++++++- src/expr.rs | 12 +----------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/closures.rs b/src/closures.rs index fe632323aa4..3cfaa4513eb 100644 --- a/src/closures.rs +++ b/src/closures.rs @@ -133,6 +133,7 @@ fn veto_block(e: &ast::Expr) -> bool { } // Rewrite closure with a single expression wrapping its body with block. +// || { #[attr] foo() } -> Block { #[attr] foo() } fn rewrite_closure_with_block( body: &ast::Expr, prefix: &str, @@ -154,8 +155,12 @@ fn rewrite_closure_with_block( }], id: ast::NodeId::root(), rules: ast::BlockCheckMode::Default, - span: body.span, tokens: None, + span: body + .attrs + .first() + .map(|attr| attr.span.to(body.span)) + .unwrap_or(body.span), }; let block = crate::expr::rewrite_block_with_visitor( context, diff --git a/src/expr.rs b/src/expr.rs index 6bc54ff8601..1ba879ccc1d 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -528,17 +528,7 @@ pub(crate) fn rewrite_block_with_visitor( let open_pos = snippet.find_uncommented("{")?; visitor.last_pos = block.span.lo() + BytePos(open_pos as u32) } - (ast::BlockCheckMode::Default, None) => { - visitor.last_pos = block.span.lo(); - if let Some(attrs) = attrs { - if let Some(first) = attrs.first() { - let first_lo_span = first.span.lo(); - if first_lo_span < visitor.last_pos { - visitor.last_pos = first_lo_span; - } - } - } - } + (ast::BlockCheckMode::Default, None) => visitor.last_pos = block.span.lo(), } let inner_attrs = attrs.map(inner_attributes); From b8f08f890e57afdc3d0986edf2d71d85fe203a9b Mon Sep 17 00:00:00 2001 From: WhizSid Date: Thu, 15 Oct 2020 07:38:58 +0530 Subject: [PATCH 3/6] Fixed 'Incorrect comment indent inside if/else' issue. (#4459) * Added test cases * Fixed if condition comment issue * Fixed extern C issue * Removed previous test case * Removed tmp file * honor the authors intent * Changed the file name to its original name * Removed extra whitespace --- src/items.rs | 26 ++++++++---- src/visitor.rs | 15 +++++-- tests/source/issue-4120.rs | 85 ++++++++++++++++++++++++++++++++++++++ tests/target/issue-4120.rs | 85 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 199 insertions(+), 12 deletions(-) create mode 100644 tests/source/issue-4120.rs create mode 100644 tests/target/issue-4120.rs diff --git a/src/items.rs b/src/items.rs index 1e2eac0c74b..01063b1f3bc 100644 --- a/src/items.rs +++ b/src/items.rs @@ -161,6 +161,14 @@ enum BodyElement<'a> { ForeignItem(&'a ast::ForeignItem), } +impl BodyElement<'_> { + pub(crate) fn span(&self) -> Span { + match self { + BodyElement::ForeignItem(fi) => fi.span(), + } + } +} + /// Represents a fn's signature. pub(crate) struct FnSig<'a> { decl: &'a ast::FnDecl, @@ -268,19 +276,19 @@ impl<'a> FmtVisitor<'a> { self.last_pos = item.span.lo() + BytePos(brace_pos as u32 + 1); self.block_indent = self.block_indent.block_indent(self.config); - if item.body.is_empty() { - self.format_missing_no_indent(item.span.hi() - BytePos(1)); - self.block_indent = self.block_indent.block_unindent(self.config); - let indent_str = self.block_indent.to_string(self.config); - self.push_str(&indent_str); - } else { + if !item.body.is_empty() { + // Advance to first item (statement or inner attribute) + // within the block. + self.last_pos = item.body[0].span().lo(); for item in &item.body { self.format_body_element(item); } - - self.block_indent = self.block_indent.block_unindent(self.config); - self.format_missing_with_indent(item.span.hi() - BytePos(1)); } + + self.format_missing_no_indent(item.span.hi() - BytePos(1)); + self.block_indent = self.block_indent.block_unindent(self.config); + let indent_str = self.block_indent.to_string(self.config); + self.push_str(&indent_str); } self.push_str("}"); diff --git a/src/visitor.rs b/src/visitor.rs index 56d023d03a6..a5f6ebfb67b 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -5,7 +5,7 @@ use rustc_ast::{ast, attr::HasAttrs, token::DelimToken, visit}; use rustc_span::{symbol, BytePos, Pos, Span, DUMMY_SP}; use crate::attr::*; -use crate::comment::{rewrite_comment, CodeCharKind, CommentCodeSlices}; +use crate::comment::{contains_comment, rewrite_comment, CodeCharKind, CommentCodeSlices}; use crate::config::Version; use crate::config::{BraceStyle, Config}; use crate::coverage::transform_missing_snippet; @@ -261,14 +261,23 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { trimmed.is_empty() || trimmed.chars().all(|c| c == ';') }; - for (kind, offset, sub_slice) in CommentCodeSlices::new(self.snippet(span)) { + let comment_snippet = self.snippet(span); + + let align_to_right = if unindent_comment && contains_comment(&comment_snippet) { + let first_lines = comment_snippet.splitn(2, '/').next().unwrap_or(""); + last_line_width(first_lines) > last_line_width(&comment_snippet) + } else { + false + }; + + for (kind, offset, sub_slice) in CommentCodeSlices::new(comment_snippet) { let sub_slice = transform_missing_snippet(config, sub_slice); debug!("close_block: {:?} {:?} {:?}", kind, offset, sub_slice); match kind { CodeCharKind::Comment => { - if !unindented && unindent_comment { + if !unindented && unindent_comment && !align_to_right { unindented = true; self.block_indent = self.block_indent.block_unindent(config); } diff --git a/tests/source/issue-4120.rs b/tests/source/issue-4120.rs new file mode 100644 index 00000000000..c9ce838c51a --- /dev/null +++ b/tests/source/issue-4120.rs @@ -0,0 +1,85 @@ +fn main() { + let x = if true { + 1 + // In if + } else { + 0 + // In else + }; + + let x = if true { + 1 + /* In if */ + } else { + 0 + /* In else */ + }; + + let z = if true { + if true { + 1 + + // In if level 2 + } else { + 2 + } + } else { + 3 + }; + + let a = if true { + 1 + // In if + } else { + 0 + // In else + }; + + let a = if true { + 1 + + // In if + } else { + 0 + // In else + }; + + let b = if true { + 1 + + // In if + } else { + 0 + // In else + }; + + let c = if true { + 1 + + // In if + } else { + 0 + // In else + }; + for i in 0..2 { + println!("Something"); + // In for + } + + for i in 0..2 { + println!("Something"); + /* In for */ + } + + extern "C" { + fn first(); + + // In foreign mod + } + + extern "C" { + fn first(); + + /* In foreign mod */ + } +} diff --git a/tests/target/issue-4120.rs b/tests/target/issue-4120.rs new file mode 100644 index 00000000000..a7d461dcfdb --- /dev/null +++ b/tests/target/issue-4120.rs @@ -0,0 +1,85 @@ +fn main() { + let x = if true { + 1 + // In if + } else { + 0 + // In else + }; + + let x = if true { + 1 + /* In if */ + } else { + 0 + /* In else */ + }; + + let z = if true { + if true { + 1 + + // In if level 2 + } else { + 2 + } + } else { + 3 + }; + + let a = if true { + 1 + // In if + } else { + 0 + // In else + }; + + let a = if true { + 1 + + // In if + } else { + 0 + // In else + }; + + let b = if true { + 1 + + // In if + } else { + 0 + // In else + }; + + let c = if true { + 1 + + // In if + } else { + 0 + // In else + }; + for i in 0..2 { + println!("Something"); + // In for + } + + for i in 0..2 { + println!("Something"); + /* In for */ + } + + extern "C" { + fn first(); + + // In foreign mod + } + + extern "C" { + fn first(); + + /* In foreign mod */ + } +} From 039c8bf637fa6fda9cec5a7464eda2919d987aaf Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Fri, 13 Nov 2020 21:42:37 -0600 Subject: [PATCH 4/6] tests: attributed comment-only blocks --- tests/source/issue_4475.rs | 27 +++++++++++++++++++++++++++ tests/target/issue_4467.rs | 6 ++++++ tests/target/issue_4475.rs | 29 +++++++++++++++++++++++++++++ tests/target/issue_4522.rs | 6 ++++++ 4 files changed, 68 insertions(+) create mode 100644 tests/source/issue_4475.rs create mode 100644 tests/target/issue_4467.rs create mode 100644 tests/target/issue_4475.rs create mode 100644 tests/target/issue_4522.rs diff --git a/tests/source/issue_4475.rs b/tests/source/issue_4475.rs new file mode 100644 index 00000000000..241dc91d7ba --- /dev/null +++ b/tests/source/issue_4475.rs @@ -0,0 +1,27 @@ +fn main() { + #[cfg(debug_assertions)] + { println!("DEBUG"); } +} + +fn main() { + #[cfg(feature = "foo")] + { + /* + let foo = 0 + */ + } +} + +fn main() { + #[cfg(feature = "foo")] + { /* let foo = 0; */ } +} + +fn main() { + #[foo] + #[bar] + #[baz] + { + // let foo = 0; + } +} \ No newline at end of file diff --git a/tests/target/issue_4467.rs b/tests/target/issue_4467.rs new file mode 100644 index 00000000000..f5ee96c4c14 --- /dev/null +++ b/tests/target/issue_4467.rs @@ -0,0 +1,6 @@ +pub fn main() { + #[cfg(feature = "std")] + { + // Comment + } +} diff --git a/tests/target/issue_4475.rs b/tests/target/issue_4475.rs new file mode 100644 index 00000000000..ea6726c5a01 --- /dev/null +++ b/tests/target/issue_4475.rs @@ -0,0 +1,29 @@ +fn main() { + #[cfg(debug_assertions)] + { + println!("DEBUG"); + } +} + +fn main() { + #[cfg(feature = "foo")] + { + /* + let foo = 0 + */ + } +} + +fn main() { + #[cfg(feature = "foo")] + { /* let foo = 0; */ } +} + +fn main() { + #[foo] + #[bar] + #[baz] + { + // let foo = 0; + } +} diff --git a/tests/target/issue_4522.rs b/tests/target/issue_4522.rs new file mode 100644 index 00000000000..5ca70e1c090 --- /dev/null +++ b/tests/target/issue_4522.rs @@ -0,0 +1,6 @@ +fn main() { + #[cfg(feature = "foo")] + { + // let foo = 0; + } +} From e3c963aaa1b39df656f9d0af61a88b4fa96a45bf Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Fri, 13 Nov 2020 22:17:07 -0600 Subject: [PATCH 5/6] ci: restrict GHA workflow triggers on push --- .github/workflows/integration.yml | 6 +++++- .github/workflows/linux.yml | 6 +++++- .github/workflows/mac.yml | 6 +++++- .github/workflows/windows.yml | 6 +++++- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 6dafb483cd6..2e832b6b62d 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -1,5 +1,9 @@ name: integration -on: [push, pull_request] +on: + push: + branches: + - master + pull_request: jobs: integration-tests: diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index efdce8ce155..acbf666955b 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -1,5 +1,9 @@ name: linux -on: [push, pull_request] +on: + push: + branches: + - master + pull_request: jobs: test: diff --git a/.github/workflows/mac.yml b/.github/workflows/mac.yml index f9285100ab2..afbc8a8d740 100644 --- a/.github/workflows/mac.yml +++ b/.github/workflows/mac.yml @@ -1,5 +1,9 @@ name: mac -on: [push, pull_request] +on: + push: + branches: + - master + pull_request: jobs: test: diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index c9ee428d63b..3f6e021ce4c 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -1,5 +1,9 @@ name: windows -on: [push, pull_request] +on: + push: + branches: + - master + pull_request: jobs: test: From 92ffce5baea69fca5edf5812fadfb334179cfa87 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Fri, 13 Nov 2020 22:33:57 -0600 Subject: [PATCH 6/6] meta: bump version to v1.4.26 --- CHANGELOG.md | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++- Cargo.lock | 2 +- Cargo.toml | 2 +- 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd44e449e18..f9c7ab1944e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,55 @@ ## [Unreleased] +## [1.4.26] 2020-11-14 + +### Changed + +- Original comment indentation for trailing comments within an `if` is now taken into account when determining the indentation level to use for the trailing comment in formatted code. This does not modify any existing code formatted with rustfmt; it simply gives the programmer discretion to specify whether the comment is associated to the `else` block, or if the trailing comment is just a member of the `if` block. ([#1575](https://github.com/rust-lang/rustfmt/issues/1575), [#4120](https://github.com/rust-lang/rustfmt/issues/4120), [#4506](https://github.com/rust-lang/rustfmt/issues/4506)) + +In this example the `// else comment` refers to the `else`: +```rust +// if comment +if cond { + "if" +// else comment +} else { + "else" +} +``` + +Whereas in this case the `// continue` comments are members of their respective blocks and do not refer to the `else` below. +```rust +if toks.eat_token(Token::Word("modify"))? && toks.eat_token(Token::Word("labels"))? { + if toks.eat_token(Token::Colon)? { + // ate the token + } else if toks.eat_token(Token::Word("to"))? { + // optionally eat the colon after to, e.g.: + // @rustbot modify labels to: -S-waiting-on-author, +S-waiting-on-review + toks.eat_token(Token::Colon)?; + } else { + // It's okay if there's no to or colon, we can just eat labels + // afterwards. + } + 1 + 2; + // continue +} else if toks.eat_token(Token::Word("label"))? { + // continue +} else { + return Ok(None); +} +``` + +### Fixed +- Formatting of empty blocks with attributes which only contained comments is no longer butchered.([#4475](https://github.com/rust-lang/rustfmt/issues/4475), [#4467](https://github.com/rust-lang/rustfmt/issues/4467), [#4452](https://github.com/rust-lang/rustfmt/issues/4452#issuecomment-705886282), [#4522](https://github.com/rust-lang/rustfmt/issues/4522)) +- Indentation of trailing comments in non-empty extern blocks is now correct. ([#4120](https://github.com/rust-lang/rustfmt/issues/4120#issuecomment-696491872)) + +### Install/Download Options +- **crates.io package** - *pending* +- **rustup (nightly)** - *pending* +- **GitHub Release Binaries** - [Release v1.4.26](https://github.com/rust-lang/rustfmt/releases/tag/v1.4.26) +- **Build from source** - [Tag v1.4.26](https://github.com/rust-lang/rustfmt/tree/v1.4.26), see instructions for how to [install rustfmt from source][install-from-source] + ## [1.4.25] 2020-11-10 ### Changed @@ -10,7 +59,7 @@ ### Install/Download Options - **crates.io package** - *pending* -- **rustup (nightly)** - *pending* +- **rustup (nightly)** - Starting in `2020-11-14` - **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] diff --git a/Cargo.lock b/Cargo.lock index fa2aa1006ea..093594209f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1237,7 +1237,7 @@ dependencies = [ [[package]] name = "rustfmt-nightly" -version = "1.4.25" +version = "1.4.26" dependencies = [ "annotate-snippets 0.6.1", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index 0f5bec2dcca..636e2566285 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "rustfmt-nightly" -version = "1.4.25" +version = "1.4.26" authors = ["Nicholas Cameron ", "The Rustfmt developers"] description = "Tool to find and fix Rust formatting issues" repository = "https://github.com/rust-lang/rustfmt"