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.

Sign up for GitHub

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

Jump to bottom

warning about rustfmt::skip with nightly rustc #551

Open
martinvonz opened this issue Mar 15, 2021 · 11 comments
Open

warning about rustfmt::skip with nightly rustc #551

martinvonz opened this issue Mar 15, 2021 · 11 comments

Comments

@martinvonz
Copy link
Contributor

I updated my rust toolchain today and it seems it started failing on rust-protobuf's generated code since then. The failures look like this:

  |
9 | #![rustfmt::skip]
  |    ^^^^^^^^^^^^^
  |
  = note: `#[deny(soft_unstable)]` on by default
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #64266 <https://github.com/rust-lang/rust/issues/64266>
@chenju2k6
Copy link

I have the same problem here.

@ekexium ekexium mentioned this issue Mar 16, 2021
Merged
@ruaruagerry
Copy link

I have the same problem here.

version：nightly-x86_64-pc-windows-msvc unchanged - rustc 1.52.0-nightly (107896c32 2021-03-15)

@nrc
Copy link

nrc commented Mar 16, 2021

This is due to rust-lang/rust#82399 although I can't figure out why we didn't see a warning before.

@martinvonz
Copy link
Contributor Author

In case it helps someone: a workaround is to add #![allow(soft_unstable)] to your crate root (such as lib.rs).

apyrgio added a commit to apyrgio/tindercrypt that referenced this issue Mar 17, 2021
@apyrgio
Temporarily silence a Rust error
c11c9ed 
The generated Rust code from our proto files triggers a warning in Rust
nightly, which we treat as an error:

    error: custom inner attributes are unstable
    Error:  --> src/../proto/metadata.rs:9:4
      |
    9 | #![rustfmt::skip]
      |    ^^^^^^^^^^^^^
      |
      = note: `#[deny(soft_unstable)]` on by default
      = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
      = note: for more information, see issue #64266 <rust-lang/rust#64266>

This is already reported in the `rust-protobuf` repo [1] so until it's
fixed, we choose to silence this warning to make our builds work.

[1]: stepancheg/rust-protobuf#551
apyrgio added a commit to apyrgio/tindercrypt that referenced this issue Mar 17, 2021
@apyrgio
Temporarily silence a Rust error
3886704 
The generated Rust code from our proto files triggers a warning in Rust
nightly, which we treat as an error:

    error: custom inner attributes are unstable
    Error:  --> src/../proto/metadata.rs:9:4
      |
    9 | #![rustfmt::skip]
      |    ^^^^^^^^^^^^^
      |
      = note: `#[deny(soft_unstable)]` on by default
      = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
      = note: for more information, see issue #64266 <rust-lang/rust#64266>

This is already reported in the `rust-protobuf` repo [1] so until it's
fixed, we choose to silence this warning to make our builds work.

[1]: stepancheg/rust-protobuf#551
@stepancheg
Copy link
Owner

Does anyone know how to fix the issue? (I don't have time right now to investigate, will return in a couple days, but right now I can commit a fix if the fix is known).

@BusyJay
Copy link
Contributor

BusyJay commented Mar 18, 2021

You can use #![cfg_attr(rustfmt, rustfmt_skip)] instead.

@stepancheg
Copy link
Owner

About a year ago we migrated from rustfmt_skip to rustfmt::skip because rustfmt_skip became deprecated. Did they undeprecate it back?

@stepancheg
Copy link
Owner

stepancheg commented Mar 18, 2021 •
edited
Loading

Moreover, rustfmt officially recommends using rustfmt::skip and does not even mention rustfmt_skip.

@nrc
Copy link

nrc commented Mar 18, 2021

I think the problem is using it as an inner attribute (#![...]) rather than an outer attribute (#[...]). I'm not up to speed with why this is a problem now though. rustfmt_skip in cfg_attr is a desugaring, I think.

stepancheg added a commit that referenced this issue Mar 18, 2021
@stepancheg
Emit allow soft_unstable as a temporary workaround for rustfmt::skip
1b21803 
Issue: #551
stepancheg added a commit that referenced this issue Mar 18, 2021
@stepancheg
Replace rustfmt::skip with cfg_attr(rustfmt, rustfmt::skip)
6069c64 
As temporary work around for #551
@stepancheg
Copy link
Owner

stepancheg commented Mar 18, 2021 •
edited
Loading

OK, I did this as temporary workaround until we find the proper fix: 6069c64.

Any reason not to release a new stable version with this change?

stepancheg added a commit that referenced this issue Mar 18, 2021
@stepancheg
Replace rustfmt::skip with cfg_attr(rustfmt, rustfmt::skip)
e441696 
As temporary work around for #551
@stepancheg
Copy link
Owner

Published version 2.22.1 with the workaround. Let me know if similar fix is needed in older 2.x versions.

Keeping this issue open because we need to find a proper fix.

@Farenjihn Farenjihn mentioned this issue Mar 23, 2021
Closed
stepancheg added a commit that referenced this issue Mar 28, 2021
@stepancheg
Replace rustfmt::skip with cfg_attr(rustfmt, rustfmt::skip)
81565e8 
As temporary work around for #551
stepancheg added a commit that referenced this issue Mar 28, 2021
@stepancheg
Replace rustfmt::skip with cfg_attr(rustfmt, rustfmt::skip)
a4b6346 
As temporary work around for #551
stepancheg added a commit that referenced this issue Mar 28, 2021
@stepancheg
Replace rustfmt::skip with cfg_attr(rustfmt, rustfmt::skip)
f39159e 
As temporary work around for #551
stepancheg added a commit that referenced this issue Mar 28, 2021
@stepancheg
Replace rustfmt::skip with cfg_attr(rustfmt, rustfmt::skip)
b06075c 
As temporary work around for #551
@eranrund eranrund mentioned this issue Apr 14, 2021
Merged
apyrgio added a commit to apyrgio/tindercrypt that referenced this issue Apr 20, 2021
@apyrgio
Remove temporary workaround for rust-protobuf
2f26b60 
Remove #![allow(soft_unstable)], which was previously added to
workaround an issue in rust-protobuf [1]. This issue has been
semi-resolved upstream [2], so we can deny this warning once more.

[1]: stepancheg/rust-protobuf#551
[2]: stepancheg/rust-protobuf#551 (comment)
apyrgio added a commit to apyrgio/tindercrypt that referenced this issue Apr 20, 2021
@apyrgio
proto: Update generated Rust code
a1de8c0 
Update the Rust code that is generated from the `.proto` files, since
we've updated the `rust-protobuf` library in the previous commit. This
update semi-resolves a `rustfmt` warning that the generated code was
triggering [1].

[1]: stepancheg/rust-protobuf#551
apyrgio added a commit to apyrgio/tindercrypt that referenced this issue Apr 20, 2021
@apyrgio
Remove temporary workaround for rust-protobuf
f24498b 
Remove #![allow(soft_unstable)], which was previously added to
workaround an issue in rust-protobuf [1]. This issue has been
semi-resolved upstream [2], so we can deny this warning once more.

[1]: stepancheg/rust-protobuf#551
[2]: stepancheg/rust-protobuf#551 (comment)
@LER0ever LER0ever mentioned this issue Apr 25, 2021
Open
stepancheg added a commit to stepancheg/rustfmt that referenced this issue Jun 20, 2021
@stepancheg
Support @generated marker to skip code formatting
a0597f2 
`@generated` marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by human:
* linters should not be invoked on these files,
* diffs in these files are less important,
* and these files should not be reformatted.

This PR proposes builtin support for `@generated` marker.

I have not found a standard for a generated file marker, but:
* Facebook [uses `@generated` marker](https://tinyurl.com/fb-generated)
* In particular, Phabricator tool which was spawned from Facebook
  internal tool [also understands `@generated` marker](https://git.io/JnVHa)
* Cargo inserts `@generated` marker into [generated Cargo.lock files](https://git.io/JnVHP)

My personal story is that rust-protobuf project which I maintain
was broken twice because of incompatibilities/bugs in rustfmt marker handling:
[one](stepancheg/rust-protobuf#493),
[two](stepancheg/rust-protobuf#551).

While rustfmt AST markers are useful to apply to a certain AST
elements, disable all-at-once at text level marker might be easier
to use and more reliable for generated code.
stepancheg added a commit to stepancheg/rustfmt that referenced this issue Jun 20, 2021
@stepancheg
Support @generated marker to skip code formatting
889d96f 
`@generated` marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by human:
* linters should not be invoked on these files,
* diffs in these files are less important,
* and these files should not be reformatted.

This PR proposes builtin support for `@generated` marker.

I have not found a standard for a generated file marker, but:
* Facebook [uses `@generated` marker](https://tinyurl.com/fb-generated)
* Phabricator tool which was spawned from Facebook internal tool
  [also understands `@generated` marker](https://git.io/JnVHa)
* Cargo inserts `@generated` marker into [generated Cargo.lock files](https://git.io/JnVHP)

My personal story is that rust-protobuf project which I maintain
was broken twice because of incompatibilities/bugs in rustfmt marker handling:
[one](stepancheg/rust-protobuf#493),
[two](stepancheg/rust-protobuf#551).

While rustfmt AST markers are useful to apply to a certain AST
elements, disable all-at-once at text level marker might be easier
to use and more reliable for generated code.
stepancheg added a commit to stepancheg/rustfmt that referenced this issue Jun 20, 2021
@stepancheg
Support @generated marker to skip code formatting
4dfa846 
`@generated` marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by human:
* linters should not be invoked on these files,
* diffs in these files are less important,
* and these files should not be reformatted.

This PR proposes builtin support for `@generated` marker.

I have not found a standard for a generated file marker, but:
* Facebook [uses `@generated` marker](https://tinyurl.com/fb-generated)
* Phabricator tool which was spawned from Facebook internal tool
  [also understands `@generated` marker](https://git.io/JnVHa)
* Cargo inserts `@generated` marker into [generated Cargo.lock files](https://git.io/JnVHP)

My personal story is that rust-protobuf project which I maintain
was broken twice because of incompatibilities/bugs in rustfmt marker handling:
[one](stepancheg/rust-protobuf#493),
[two](stepancheg/rust-protobuf#551).

While rustfmt AST markers are useful to apply to a certain AST
elements, disable all-at-once at text level marker might be easier
to use and more reliable for generated code.
stepancheg added a commit to stepancheg/rustfmt that referenced this issue Jun 20, 2021
@stepancheg
Support @generated marker to skip code formatting
64103ef 
`@generated` marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by a human:
* linters should not be invoked on these files,
* diffs in these files are less important,
* and these files should not be reformatted.

This PR proposes builtin support for `@generated` marker.

I have not found a standard for a generated file marker, but:
* Facebook [uses `@generated` marker](https://tinyurl.com/fb-generated)
* Phabricator tool which was spawned from Facebook internal tool
  [also understands `@generated` marker](https://git.io/JnVHa)
* Cargo inserts `@generated` marker into [generated Cargo.lock files](https://git.io/JnVHP)

My personal story is that rust-protobuf project which I maintain
was broken twice because of incompatibilities/bugs in rustfmt marker handling:
[one](stepancheg/rust-protobuf#493),
[two](stepancheg/rust-protobuf#551).

While rustfmt AST markers are useful to apply to a certain AST
elements, disable all-at-once at text level marker might be easier
to use and more reliable for generated code.
stepancheg added a commit to stepancheg/rustfmt that referenced this issue Jun 20, 2021
@stepancheg
Support @generated marker to skip code formatting
28a0205 
`@generated` marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by a human:
* linters should not be invoked on these files,
* diffs in these files are less important,
* and these files should not be reformatted.

This PR proposes builtin support for `@generated` marker.

I have not found a standard for a generated file marker, but:
* Facebook [uses `@generated` marker](https://tinyurl.com/fb-generated)
* Phabricator tool which was spawned from Facebook internal tool
  [also understands `@generated` marker](https://git.io/JnVHa)
* Cargo inserts `@generated` marker into [generated Cargo.lock files](https://git.io/JnVHP)

My personal story is that rust-protobuf project which I maintain
was broken twice because of incompatibilities/bugs in rustfmt marker handling:
[one](stepancheg/rust-protobuf#493),
[two](stepancheg/rust-protobuf#551).

While rustfmt AST markers are useful to apply to a certain AST
elements, disable whole-file-at-once all-tools-at-once at text level
marker might be easier to use and more reliable for generated code.
stepancheg added a commit to stepancheg/rustfmt that referenced this issue Jun 20, 2021
@stepancheg
Support @generated marker to skip code formatting
f78adda 
`@generated` marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by a human:
* linters should not be invoked on these files,
* diffs in these files are less important,
* and these files should not be reformatted.

This PR proposes builtin support for `@generated` marker.

I have not found a standard for a generated file marker, but:
* Facebook [uses `@generated` marker](https://tinyurl.com/fb-generated)
* Phabricator tool which was spawned from Facebook internal tool
  [also understands `@generated` marker](https://git.io/JnVHa)
* Cargo inserts `@generated` marker into [generated Cargo.lock files](https://git.io/JnVHP)

My personal story is that rust-protobuf project which I maintain
was broken twice because of incompatibilities/bugs in rustfmt marker handling:
[one](stepancheg/rust-protobuf#493),
[two](stepancheg/rust-protobuf#551).

While rustfmt AST markers are useful to apply to a certain AST
elements, disable whole-file-at-once all-tools-at-once text level
marker might be easier to use and more reliable for generated code.
stepancheg added a commit to stepancheg/rustfmt that referenced this issue Jun 20, 2021
@stepancheg
Support @generated marker to skip code formatting
7ddc5ff 
`@generated` marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by a human:
* linters should not be invoked on these files,
* diffs in these files are less important,
* and these files should not be reformatted.

This PR proposes builtin support for `@generated` marker.

I have not found a standard for a generated file marker, but:
* Facebook [uses `@generated` marker](https://tinyurl.com/fb-generated)
* Phabricator tool which was spawned from Facebook internal tool
  [also understands `@generated` marker](https://git.io/JnVHa)
* Cargo inserts `@generated` marker into [generated Cargo.lock files](https://git.io/JnVHP)

My personal story is that rust-protobuf project which I maintain
was broken twice because of incompatibilities/bugs in rustfmt marker
handling: [one](stepancheg/rust-protobuf#493),
[two](stepancheg/rust-protobuf#551).
(Also, rust-protobuf started generating `@generated` marker
[6 years ago](https://git.io/JnV5h)).

While rustfmt AST markers are useful to apply to a certain AST
elements, disable whole-file-at-once all-tools-at-once text level
marker might be easier to use and more reliable for generated code.
@stepancheg stepancheg mentioned this issue Jun 20, 2021
Merged
stepancheg pushed a commit to stepancheg/rustfmt that referenced this issue Jun 22, 2021
@topecongiro @stepancheg
Support @generated marker to skip code formatting
b1d0548 
This is a copy of rust-lang#4296 with these changes:
* file is not reopened again to find if the file is generated
* first five lines are scanned for `@generated` marker instead of one
* no attempt is made to only search for marker in comments

`@generated` marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by a human:
* linters should not be invoked on these files,
* diffs in these files are less important,
* and these files should not be reformatted.

This PR proposes builtin support for `@generated` marker.

I have not found a standard for a generated file marker, but:
* Facebook [uses `@generated` marker](https://tinyurl.com/fb-generated)
* Phabricator tool which was spawned from Facebook internal tool
  [also understands `@generated` marker](https://git.io/JnVHa)
* Cargo inserts `@generated` marker into [generated Cargo.lock files](https://git.io/JnVHP)

My personal story is that rust-protobuf project which I maintain
was broken twice because of incompatibilities/bugs in rustfmt marker
handling: [one](stepancheg/rust-protobuf#493),
[two](stepancheg/rust-protobuf#551).
(Also, rust-protobuf started generating `@generated` marker
[6 years ago](https://git.io/JnV5h)).

While rustfmt AST markers are useful to apply to a certain AST
elements, disable whole-file-at-once all-tools-at-once text level
marker might be easier to use and more reliable for generated code.
stepancheg pushed a commit to stepancheg/rustfmt that referenced this issue Jun 22, 2021
@topecongiro @stepancheg
Support @generated marker to skip code formatting
2aacad8 
This is a copy of rust-lang#4296 with these changes:
* file is not reopened again to find if the file is generated
* first five lines are scanned for `@generated` marker instead of one
* no attempt is made to only search for marker in comments

`@generated` marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by a human:
* linters should not be invoked on these files,
* diffs in these files are less important,
* and these files should not be reformatted.

This PR proposes builtin support for `@generated` marker.

I have not found a standard for a generated file marker, but:
* Facebook [uses `@generated` marker](https://tinyurl.com/fb-generated)
* Phabricator tool which was spawned from Facebook internal tool
  [also understands `@generated` marker](https://git.io/JnVHa)
* Cargo inserts `@generated` marker into [generated Cargo.lock files](https://git.io/JnVHP)

My personal story is that rust-protobuf project which I maintain
was broken twice because of incompatibilities/bugs in rustfmt marker
handling: [one](stepancheg/rust-protobuf#493),
[two](stepancheg/rust-protobuf#551).
(Also, rust-protobuf started generating `@generated` marker
[6 years ago](https://git.io/JnV5h)).

While rustfmt AST markers are useful to apply to a certain AST
elements, disable whole-file-at-once all-tools-at-once text level
marker might be easier to use and more reliable for generated code.
calebcartwright pushed a commit to rust-lang/rustfmt that referenced this issue Sep 8, 2021
@topecongiro @calebcartwright
Support @generated marker to skip code formatting
1b87a8b 
This is a copy of #4296 with these changes:
* file is not reopened again to find if the file is generated
* first five lines are scanned for `@generated` marker instead of one
* no attempt is made to only search for marker in comments

`@generated` marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by a human:
* linters should not be invoked on these files,
* diffs in these files are less important,
* and these files should not be reformatted.

This PR proposes builtin support for `@generated` marker.

I have not found a standard for a generated file marker, but:
* Facebook [uses `@generated` marker](https://tinyurl.com/fb-generated)
* Phabricator tool which was spawned from Facebook internal tool
  [also understands `@generated` marker](https://git.io/JnVHa)
* Cargo inserts `@generated` marker into [generated Cargo.lock files](https://git.io/JnVHP)

My personal story is that rust-protobuf project which I maintain
was broken twice because of incompatibilities/bugs in rustfmt marker
handling: [one](stepancheg/rust-protobuf#493),
[two](stepancheg/rust-protobuf#551).
(Also, rust-protobuf started generating `@generated` marker
[6 years ago](https://git.io/JnV5h)).

While rustfmt AST markers are useful to apply to a certain AST
elements, disable whole-file-at-once all-tools-at-once text level
marker might be easier to use and more reliable for generated code.
calebcartwright pushed a commit to calebcartwright/rustfmt that referenced this issue Sep 15, 2021
@topecongiro @calebcartwright
Support @generated marker to skip code formatting
0a89312 
This is a copy of rust-lang#4296 with these changes:
* file is not reopened again to find if the file is generated
* first five lines are scanned for `@generated` marker instead of one
* no attempt is made to only search for marker in comments

`@generated` marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by a human:
* linters should not be invoked on these files,
* diffs in these files are less important,
* and these files should not be reformatted.

This PR proposes builtin support for `@generated` marker.

I have not found a standard for a generated file marker, but:
* Facebook [uses `@generated` marker](https://tinyurl.com/fb-generated)
* Phabricator tool which was spawned from Facebook internal tool
  [also understands `@generated` marker](https://git.io/JnVHa)
* Cargo inserts `@generated` marker into [generated Cargo.lock files](https://git.io/JnVHP)

My personal story is that rust-protobuf project which I maintain
was broken twice because of incompatibilities/bugs in rustfmt marker
handling: [one](stepancheg/rust-protobuf#493),
[two](stepancheg/rust-protobuf#551).
(Also, rust-protobuf started generating `@generated` marker
[6 years ago](https://git.io/JnV5h)).

While rustfmt AST markers are useful to apply to a certain AST
elements, disable whole-file-at-once all-tools-at-once text level
marker might be easier to use and more reliable for generated code.
calebcartwright pushed a commit to rust-lang/rustfmt that referenced this issue Sep 15, 2021
@topecongiro @calebcartwright
Support @generated marker to skip code formatting
d4ffd1e 
This is a copy of #4296 with these changes:
* file is not reopened again to find if the file is generated
* first five lines are scanned for `@generated` marker instead of one
* no attempt is made to only search for marker in comments

`@generated` marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by a human:
* linters should not be invoked on these files,
* diffs in these files are less important,
* and these files should not be reformatted.

This PR proposes builtin support for `@generated` marker.

I have not found a standard for a generated file marker, but:
* Facebook [uses `@generated` marker](https://tinyurl.com/fb-generated)
* Phabricator tool which was spawned from Facebook internal tool
  [also understands `@generated` marker](https://git.io/JnVHa)
* Cargo inserts `@generated` marker into [generated Cargo.lock files](https://git.io/JnVHP)

My personal story is that rust-protobuf project which I maintain
was broken twice because of incompatibilities/bugs in rustfmt marker
handling: [one](stepancheg/rust-protobuf#493),
[two](stepancheg/rust-protobuf#551).
(Also, rust-protobuf started generating `@generated` marker
[6 years ago](https://git.io/JnV5h)).

While rustfmt AST markers are useful to apply to a certain AST
elements, disable whole-file-at-once all-tools-at-once text level
marker might be easier to use and more reliable for generated code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@stepancheg @nrc @martinvonz @BusyJay @chenju2k6 @ruaruagerry