If I understand it correctly, this would also produce the same warnings as in #3159, right? Should we wait until tool_lints are stable to merge this?

No this is a tool_attribute, which is already stable. But I realized yesterday that #![rustfmt::skip] does make sense. So I want to look into this problem first, before merging this.

Ok, I looked into this a little more:

cfg_attrs already get processed here:

830  let (mut krate, features) = syntax::config::features(
831      krate,
832      &sess.parse_sess,
833      sess.edition(),
834  );

while PreExpansionPass lints are executed here:

942  time(sess, "pre ast expansion lint checks", || {
943      lint::check_ast_crate(sess, &krate, true)
944  });

The reason only inner/crate attributes are effected by this, is that the compiler only looks at Crate-level attributes and not on Item-level attributes in the config::features() function. That would mean, that this should get linted. Let's add a test for it!

Since having #![rustfmt::skip] as a crate level attribute really doesn't make any sense, I think this is ready to merge. tool_attributes should be stable in 1.30.

tool_attributes are stable on 1.30.beta.1: Playground

tool_attributes are stable now! The issue that cfg_attr lints can't handle crate level attributes, can only be resolved by a non trivial change in the compiler. But since crate level rustfmt attributes shouldn't be a thing (IMO), this shouldn't be a blocker for this lint.

I added a few tests from rustfmt. Now also #[cfg_attr(rustfmt, rustfmt::skip)] gets linted.

I just realized, that we don't have a check for the copyright statement in test and clippy source files. We should add such a check (for easier reviews). Since it is hard to write lints for comments, we should probably add this check in clippy_dev?

bors try

try

Build failed

• continuous-integration/appveyor/branch

LGTM!

bors r+

Build succeeded

• continuous-integration/travis-ci/push
• continuous-integration/appveyor/branch