Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comma deleted if followed by empty line #4791

Closed
ente76 opened this issue Apr 9, 2021 · 4 comments · Fixed by #5159
Closed

Comma deleted if followed by empty line #4791

ente76 opened this issue Apr 9, 2021 · 4 comments · Fixed by #5159
Labels
1x-backport:completed bug Panic, non-idempotency, invalid code, etc. help wanted needs-test only-with-option requires a non-default option value to reproduce

Comments

@ente76
Copy link

ente76 commented Apr 9, 2021

Describe the bug

Define a struct with empty lines, as shown below:

pub struct Segment {
    header: SegmentHeader,
    
    end:    SegmentEnd
}

format the document (using IDE functionality which relies on rustfmt or by calling cargo fmt). Result looks like:

pub struct Segment {
    header: SegmentHeader

    end: SegmentEnd
}

To Reproduce

Start on a brand new project. Have all your standard project template setup and ready to start (incl. rustfmt.toml). Feel totally overwhelmed by all the new information you just gathered. Setup some basic structs. Put an empty line between two struct elements and format the document.

Expected behavior

rustfmt reformats the document. While doing so, some artificial intelligence hidden deep inside rustfmt realizes that Segment actually is a wrong point to start at. For such reason rustfmt deletes the comma and all the struct just before it magically implements some perfectly working UN/edifact serialization code.

Meta

  • rustfmt version: rustfmt 1.4.36-nightly (7de6968 2021-02-07)
  • From where did you install rustfmt?: I guess rustup, but system is running stable for years now... i.e., I am not sure
  • How do you run rustfmt: IDE (vscode with rust-analyzer) calls rustfmt directly, cargo fmt was executed for test
  • my rustfmt.toml:
edition = "2018" 
enum_discrim_align_threshold = 30
format_macro_matchers = true 
imports_indent = "Visual"
license_template_path = ".license.template"
imports_granularity = "Crate"
newline_style = "Unix"
normalize_comments = true
normalize_doc_attributes = true
reorder_impl_items = true
struct_field_align_threshold = 30
trailing_comma = "Never"
unstable_features = true
where_single_line = true
max_width = 200
@ente76 ente76 added the bug Panic, non-idempotency, invalid code, etc. label Apr 9, 2021
@calebcartwright calebcartwright added 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release needs-test only-with-option requires a non-default option value to reproduce labels Apr 10, 2021
@calebcartwright
Copy link
Member

Thanks for the report!

Minimal repro config seems to just be setting struct_field_align_threshold. This appears to be fixed in source on the master branch so quite possibly just a bug fix that needs to be backported and released

@karyon
Copy link
Contributor

karyon commented Oct 28, 2021

Could repro with 1.4.38, but only with this config:

struct_field_align_threshold = 30
trailing_comma = "Never"

cassaundra added a commit to cassaundra/rustfmt that referenced this issue Dec 29, 2021
When struct_field_align_threshold is non-zero and trailing_comma is set to
"Never," struct field separators are omitted between field groups. This issue is
resolved by forcing separators between groups.

Fixes rust-lang#4791.

A test is included with a minimal reproducible example.
cassaundra added a commit to cassaundra/rustfmt that referenced this issue Mar 5, 2022
calebcartwright pushed a commit that referenced this issue Mar 6, 2022
When struct_field_align_threshold is non-zero and trailing_comma is set to
"Never," struct field separators are omitted between field groups. This issue is
resolved by forcing separators between groups.

Fixes #4791.

A test is included with a minimal reproducible example.
calebcartwright pushed a commit that referenced this issue Mar 6, 2022
@ytmimi
Copy link
Contributor

ytmimi commented Mar 29, 2022

@calebcartwright since you recently merged a fix for this can we update the label to backport:complete?

@calebcartwright
Copy link
Member

Sure can!

@calebcartwright calebcartwright added 1x-backport:completed and removed 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release labels Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1x-backport:completed bug Panic, non-idempotency, invalid code, etc. help wanted needs-test only-with-option requires a non-default option value to reproduce
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants