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

Lint #[cfg_attr(rustfmt, rustfmt_skip)] and suggest #[rustfmt::skip] #3123

Merged
merged 9 commits into from
Nov 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,7 @@ All notable changes to this project will be documented in this file.
[`decimal_literal_representation`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#decimal_literal_representation
[`declare_interior_mutable_const`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#declare_interior_mutable_const
[`default_trait_access`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#default_trait_access
[`deprecated_cfg_attr`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#deprecated_cfg_attr
[`deprecated_semver`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#deprecated_semver
[`deref_addrof`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#deref_addrof
[`derive_hash_xor_eq`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#derive_hash_xor_eq
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 286 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
[There are 287 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
76 changes: 74 additions & 2 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@

use crate::reexport::*;
use crate::utils::{
in_macro, last_line_of_span, match_def_path, opt_def_id, paths, snippet_opt, span_lint,
in_macro, last_line_of_span, match_def_path, opt_def_id, paths, snippet_opt, span_lint, span_lint_and_sugg,
span_lint_and_then, without_block_comments,
};
use if_chain::if_chain;
use crate::rustc::hir::*;
use crate::rustc::lint::{
CheckLintNameResult, LateContext, LateLintPass, LintArray, LintContext, LintPass,
CheckLintNameResult, EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintArray, LintContext, LintPass,
};
use crate::rustc::ty::{self, TyCtxt};
use crate::rustc::{declare_tool_lint, lint_array};
Expand Down Expand Up @@ -169,6 +169,35 @@ declare_clippy_lint! {
"unknown_lints for scoped Clippy lints"
}

/// **What it does:** Checks for `#[cfg_attr(rustfmt, rustfmt_skip)]` and suggests to replace it
/// with `#[rustfmt::skip]`.
///
/// **Why is this bad?** Since tool_attributes ([rust-lang/rust#44690](https://github.com/rust-lang/rust/issues/44690))
/// are stable now, they should be used instead of the old `cfg_attr(rustfmt)` attributes.
///
/// **Known problems:** This lint doesn't detect crate level inner attributes, because they get
/// processed before the PreExpansionPass lints get executed. See
/// [#3123](https://github.com/rust-lang-nursery/rust-clippy/pull/3123#issuecomment-422321765)
///
/// **Example:**
///
/// Bad:
/// ```rust
/// #[cfg_attr(rustfmt, rustfmt_skip)]
/// fn main() { }
/// ```
///
/// Good:
/// ```rust
/// #[rustfmt::skip]
/// fn main() { }
/// ```
declare_clippy_lint! {
pub DEPRECATED_CFG_ATTR,
complexity,
"usage of `cfg_attr(rustfmt)` instead of `tool_attributes`"
}

#[derive(Copy, Clone)]
pub struct AttrPass;

Expand Down Expand Up @@ -466,3 +495,46 @@ fn is_present_in_source(cx: &LateContext<'_, '_>, span: Span) -> bool {
}
true
}

#[derive(Copy, Clone)]
pub struct CfgAttrPass;

impl LintPass for CfgAttrPass {
fn get_lints(&self) -> LintArray {
lint_array!(
DEPRECATED_CFG_ATTR,
)
}
}

impl EarlyLintPass for CfgAttrPass {
fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
if_chain! {
// check cfg_attr
if attr.name() == "cfg_attr";
if let Some(ref items) = attr.meta_item_list();
if items.len() == 2;
// check for `rustfmt`
if let Some(feature_item) = items[0].meta_item();
if feature_item.name() == "rustfmt";
// check for `rustfmt_skip` and `rustfmt::skip`
if let Some(skip_item) = &items[1].meta_item();
if skip_item.name() == "rustfmt_skip" || skip_item.name() == "skip";
then {
let attr_style = match attr.style {
AttrStyle::Outer => "#[",
AttrStyle::Inner => "#![",
};
span_lint_and_sugg(
cx,
DEPRECATED_CFG_ATTR,
attr.span,
"`cfg_attr` is deprecated for rustfmt and got replaced by tool_attributes",
"use",
format!("{}rustfmt::skip]", attr_style),
);
}
}
}
}

3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ pub fn register_pre_expansion_lints(session: &rustc::session::Session, store: &m
store.register_pre_expansion_pass(Some(session), box non_expressive_names::NonExpressiveNames {
single_char_binding_names_threshold: conf.single_char_binding_names_threshold,
});
store.register_pre_expansion_pass(Some(session), box attrs::CfgAttrPass);
}

pub fn read_conf(reg: &rustc_plugin::Registry<'_>) -> Conf {
Expand Down Expand Up @@ -532,6 +533,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
approx_const::APPROX_CONSTANT,
assign_ops::ASSIGN_OP_PATTERN,
assign_ops::MISREFACTORED_ASSIGN_OP,
attrs::DEPRECATED_CFG_ATTR,
attrs::DEPRECATED_SEMVER,
attrs::UNKNOWN_CLIPPY_LINTS,
attrs::USELESS_ATTRIBUTE,
Expand Down Expand Up @@ -839,6 +841,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {

reg.register_lint_group("clippy::complexity", Some("clippy_complexity"), vec![
assign_ops::MISREFACTORED_ASSIGN_OP,
attrs::DEPRECATED_CFG_ATTR,
booleans::NONMINIMAL_BOOL,
cyclomatic_complexity::CYCLOMATIC_COMPLEXITY,
double_comparison::DOUBLE_COMPARISONS,
Expand Down
38 changes: 38 additions & 0 deletions tests/ui/cfg_attr_rustfmt.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(stmt_expr_attributes)]

#![warn(clippy::deprecated_cfg_attr)]

// This doesn't get linted, see known problems
#![cfg_attr(rustfmt, rustfmt_skip)]

#[rustfmt::skip]
trait Foo
{
fn foo(
);
}

fn skip_on_statements() {
#[cfg_attr(rustfmt, rustfmt::skip)]
5+3;
}

#[cfg_attr(rustfmt, rustfmt_skip)]
fn main() {
foo::f();
}

mod foo {
#![cfg_attr(rustfmt, rustfmt_skip)]

pub fn f() {}
}
22 changes: 22 additions & 0 deletions tests/ui/cfg_attr_rustfmt.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error: `cfg_attr` is deprecated for rustfmt and got replaced by tool_attributes
--> $DIR/cfg_attr_rustfmt.rs:25:5
|
25 | #[cfg_attr(rustfmt, rustfmt::skip)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `#[rustfmt::skip]`
|
= note: `-D clippy::deprecated-cfg-attr` implied by `-D warnings`

error: `cfg_attr` is deprecated for rustfmt and got replaced by tool_attributes
--> $DIR/cfg_attr_rustfmt.rs:29:1
|
29 | #[cfg_attr(rustfmt, rustfmt_skip)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `#[rustfmt::skip]`

error: `cfg_attr` is deprecated for rustfmt and got replaced by tool_attributes
--> $DIR/cfg_attr_rustfmt.rs:35:5
|
35 | #![cfg_attr(rustfmt, rustfmt_skip)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `#![rustfmt::skip]`

error: aborting due to 3 previous errors