diff --git a/CHANGELOG.md b/CHANGELOG.md index 1aaf617f5b15..b2e9f6d1dd3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7052,6 +7052,7 @@ Released 2018-09-13 [`msrv`]: https://doc.rust-lang.org/clippy/lint_configuration.html#msrv [`pass-by-value-size-limit`]: https://doc.rust-lang.org/clippy/lint_configuration.html#pass-by-value-size-limit [`pub-underscore-fields-behavior`]: https://doc.rust-lang.org/clippy/lint_configuration.html#pub-underscore-fields-behavior +[`recursive-self-in-type-definitions`]: https://doc.rust-lang.org/clippy/lint_configuration.html#recursive-self-in-type-definitions [`semicolon-inside-block-ignore-singleline`]: https://doc.rust-lang.org/clippy/lint_configuration.html#semicolon-inside-block-ignore-singleline [`semicolon-outside-block-ignore-multiline`]: https://doc.rust-lang.org/clippy/lint_configuration.html#semicolon-outside-block-ignore-multiline [`single-char-binding-names-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#single-char-binding-names-threshold diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index b2ba19631f13..f0b2feca6a01 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -927,6 +927,16 @@ exported visibility, or whether they are marked as "pub". * [`pub_underscore_fields`](https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields) +## `recursive-self-in-type-definitions` +Whether the type itself in a struct or enum should be replaced with `Self` when encountering recursive types. + +**Default Value:** `true` + +--- +**Affected lints:** +* [`use_self`](https://rust-lang.github.io/rust-clippy/master/index.html#use_self) + + ## `semicolon-inside-block-ignore-singleline` Whether to lint only if it's multiline. diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 843aa6a40f09..9902065422fb 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -809,6 +809,9 @@ define_Conf! { /// exported visibility, or whether they are marked as "pub". #[lints(pub_underscore_fields)] pub_underscore_fields_behavior: PubUnderscoreFieldsBehaviour = PubUnderscoreFieldsBehaviour::PubliclyExported, + /// Whether the type itself in a struct or enum should be replaced with `Self` when encountering recursive types. + #[lints(use_self)] + recursive_self_in_type_definitions: bool = true, /// Whether to lint only if it's multiline. #[lints(semicolon_inside_block)] semicolon_inside_block_ignore_singleline: bool = false, diff --git a/clippy_lints/src/matches/single_match.rs b/clippy_lints/src/matches/single_match.rs index 02f87512966b..44c4d7a31ff3 100644 --- a/clippy_lints/src/matches/single_match.rs +++ b/clippy_lints/src/matches/single_match.rs @@ -224,7 +224,7 @@ enum PatState<'a> { /// A std enum we know won't be extended. Tracks the states of each variant separately. /// /// This is not used for `Option` since it uses the current pattern to track its state. - StdEnum(&'a mut [PatState<'a>]), + StdEnum(&'a mut [Self]), /// Either the initial state for a pattern or a non-std enum. There is currently no need to /// distinguish these cases. /// diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index 9d5be922f43f..f46dedf1261e 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -58,6 +58,7 @@ declare_clippy_lint! { pub struct UseSelf { msrv: Msrv, stack: Vec, + recursive_self_in_type_definitions: bool, } impl UseSelf { @@ -65,6 +66,7 @@ impl UseSelf { Self { msrv: conf.msrv, stack: Vec::new(), + recursive_self_in_type_definitions: conf.recursive_self_in_type_definitions, } } } @@ -84,10 +86,10 @@ const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element"; impl<'tcx> LateLintPass<'tcx> for UseSelf { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &Item<'tcx>) { - // We push the self types of `impl`s on a stack here. Only the top type on the stack is - // relevant for linting, since this is the self type of the `impl` we're currently in. To - // avoid linting on nested items, we push `StackItem::NoCheck` on the stack to signal, that - // we're in an `impl` or nested item, that we don't want to lint + // We push the self types of items on a stack here. Only the top type on the stack is + // relevant for linting, since this is the self type of the item we're currently in. To + // avoid linting on nested items, we push `StackItem::NoCheck` on the stack to signal that + // we're in an item or nested item that we don't want to lint let stack_item = if let ItemKind::Impl(Impl { self_ty, generics, .. }) = item.kind && let TyKind::Path(QPath::Resolved(_, item_path)) = self_ty.kind && let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args @@ -112,6 +114,15 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { impl_id: item.owner_id.def_id, types_to_skip, } + } else if let ItemKind::Struct(..) | ItemKind::Enum(..) = item.kind + && self.recursive_self_in_type_definitions + && !item.span.from_expansion() + && !is_from_proc_macro(cx, item) + { + StackItem::Check { + impl_id: item.owner_id.def_id, + types_to_skip: FxHashSet::default(), + } } else { StackItem::NoCheck }; diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index e03ba2f73b43..6e2cabd5e61b 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -50,15 +50,15 @@ pub enum Constant { /// `true` or `false`. Bool(bool), /// An array of constants. - Vec(Vec), + Vec(Vec), /// Also an array, but with only one constant, repeated N times. - Repeat(Box, u64), + Repeat(Box, u64), /// A tuple of constants. - Tuple(Vec), + Tuple(Vec), /// A raw pointer. RawPtr(u128), /// A reference - Ref(Box), + Ref(Box), /// A literal with syntax error. Err, } diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs index a63333c9b48f..581c2b02839d 100644 --- a/clippy_utils/src/sugg.rs +++ b/clippy_utils/src/sugg.rs @@ -33,7 +33,7 @@ pub enum Sugg<'a> { /// or `-`, but only if the type with and without the operator is kept identical. /// It means that doubling the operator can be used to remove it instead, in /// order to provide better suggestions. - UnOp(UnOp, Box>), + UnOp(UnOp, Box), } /// Literal constant `0`, for convenience. diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 20aeb4bb8498..b96a5486c72b 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -66,6 +66,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect msrv pass-by-value-size-limit pub-underscore-fields-behavior + recursive-self-in-type-definitions semicolon-inside-block-ignore-singleline semicolon-outside-block-ignore-multiline single-char-binding-names-threshold @@ -161,6 +162,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect msrv pass-by-value-size-limit pub-underscore-fields-behavior + recursive-self-in-type-definitions semicolon-inside-block-ignore-singleline semicolon-outside-block-ignore-multiline single-char-binding-names-threshold @@ -256,6 +258,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni msrv pass-by-value-size-limit pub-underscore-fields-behavior + recursive-self-in-type-definitions semicolon-inside-block-ignore-singleline semicolon-outside-block-ignore-multiline single-char-binding-names-threshold diff --git a/tests/ui-toml/use_self/default/clippy.toml b/tests/ui-toml/use_self/default/clippy.toml new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/ui-toml/use_self/disabled/clippy.toml b/tests/ui-toml/use_self/disabled/clippy.toml new file mode 100644 index 000000000000..866cc5c624b5 --- /dev/null +++ b/tests/ui-toml/use_self/disabled/clippy.toml @@ -0,0 +1 @@ +recursive-self-in-type-definitions = false diff --git a/tests/ui-toml/use_self/use_self.default.fixed b/tests/ui-toml/use_self/use_self.default.fixed new file mode 100644 index 000000000000..288e304c60a8 --- /dev/null +++ b/tests/ui-toml/use_self/use_self.default.fixed @@ -0,0 +1,17 @@ +//@revisions: default disabled +//@[disabled] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/use_self/disabled +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/use_self/default + +#![warn(clippy::use_self)] + +fn main() {} + +struct Basic { + flag: Option>, + //~[default]^ use_self +} + +impl Basic { + fn x(_: Self) {} + //~[default,disabled]^ use_self +} diff --git a/tests/ui-toml/use_self/use_self.default.stderr b/tests/ui-toml/use_self/use_self.default.stderr new file mode 100644 index 000000000000..34cfdfd938aa --- /dev/null +++ b/tests/ui-toml/use_self/use_self.default.stderr @@ -0,0 +1,17 @@ +error: unnecessary structure name repetition + --> tests/ui-toml/use_self/use_self.rs:10:22 + | +LL | flag: Option>, + | ^^^^^ help: use the applicable keyword: `Self` + | + = note: `-D clippy::use-self` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::use_self)]` + +error: unnecessary structure name repetition + --> tests/ui-toml/use_self/use_self.rs:15:13 + | +LL | fn x(_: Basic) {} + | ^^^^^ help: use the applicable keyword: `Self` + +error: aborting due to 2 previous errors + diff --git a/tests/ui-toml/use_self/use_self.disabled.fixed b/tests/ui-toml/use_self/use_self.disabled.fixed new file mode 100644 index 000000000000..227606e69005 --- /dev/null +++ b/tests/ui-toml/use_self/use_self.disabled.fixed @@ -0,0 +1,17 @@ +//@revisions: default disabled +//@[disabled] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/use_self/disabled +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/use_self/default + +#![warn(clippy::use_self)] + +fn main() {} + +struct Basic { + flag: Option>, + //~[default]^ use_self +} + +impl Basic { + fn x(_: Self) {} + //~[default,disabled]^ use_self +} diff --git a/tests/ui-toml/use_self/use_self.disabled.stderr b/tests/ui-toml/use_self/use_self.disabled.stderr new file mode 100644 index 000000000000..1801744f0d41 --- /dev/null +++ b/tests/ui-toml/use_self/use_self.disabled.stderr @@ -0,0 +1,11 @@ +error: unnecessary structure name repetition + --> tests/ui-toml/use_self/use_self.rs:15:13 + | +LL | fn x(_: Basic) {} + | ^^^^^ help: use the applicable keyword: `Self` + | + = note: `-D clippy::use-self` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::use_self)]` + +error: aborting due to 1 previous error + diff --git a/tests/ui-toml/use_self/use_self.rs b/tests/ui-toml/use_self/use_self.rs new file mode 100644 index 000000000000..d20006d4df1a --- /dev/null +++ b/tests/ui-toml/use_self/use_self.rs @@ -0,0 +1,17 @@ +//@revisions: default disabled +//@[disabled] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/use_self/disabled +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/use_self/default + +#![warn(clippy::use_self)] + +fn main() {} + +struct Basic { + flag: Option>, + //~[default]^ use_self +} + +impl Basic { + fn x(_: Basic) {} + //~[default,disabled]^ use_self +} diff --git a/tests/ui/use_self_structs.fixed b/tests/ui/use_self_structs.fixed new file mode 100644 index 000000000000..bd7bc3e0c1f6 --- /dev/null +++ b/tests/ui/use_self_structs.fixed @@ -0,0 +1,134 @@ +#![warn(clippy::use_self)] +#![allow(clippy::type_complexity)] + +fn main() {} + +struct Basic { + flag: Option>, + //~^ use_self +} + +struct BasicSelf { + okay: Option>, +} + +struct Generic<'q, T: From> { + t: &'q T, + flag: Option>, + //~^ use_self +} + +struct GenericSelf<'q, T: From> { + t: &'q T, + okay: Option>, +} + +struct MixedLifetimes<'q, T: From + 'static> { + t: &'q T, + okay: Option>>, +} + +struct ConcreteType<'q, T: From> { + t: &'q T, + okay: Option>>, +} + +struct ConcreteAndGeneric<'q, T: From> { + t: &'q T, + flag: Option>, + //~^ use_self + okay: Option>>, +} + +struct ConcreteAndGenericSelf<'q, T: From> { + t: &'q T, + okay_1: Option>, + okay_2: Option>>, +} + +macro_rules! recursive_struct { + ($name:ident) => { + struct $name { + okay: Option>, + } + }; +} + +recursive_struct!(X); +recursive_struct!(Y); +recursive_struct!(Z); + +struct Tree { + left: Option>, + //~^ use_self + right: Option>, + //~^ use_self +} + +struct TreeSelf { + left: Option>, + right: Option>, +} + +struct TreeMixed { + left: Option>, + right: Option>, + //~^ use_self +} + +struct Nested { + flag: Option>>>, + //~^ use_self +} + +struct NestedSelf { + okay: Option>>>, +} + +struct Tuple(Option>); +//~^ use_self + +struct TupleSelf(Option>); + +use std::cell::RefCell; +use std::rc::{Rc, Weak}; + +struct Containers { + flag: Vec>>>>>>, + //~^ use_self +} + +struct ContainersSelf { + okay: Vec>>>>>>, +} + +type Wrappers = Vec>>>>>>; + +struct Alias { + flag: Wrappers, + //~^ use_self +} + +struct AliasSelf { + okay: Wrappers, +} + +struct Array { + flag: [Option>; N], + //~^ use_self +} + +struct ArraySelf { + okay: [Option>; N], +} + +enum Enum { + Nil, + Cons(Box), + //~^ use_self +} + +enum EnumSelf { + Nil, + Cons(Box), +} diff --git a/tests/ui/use_self_structs.rs b/tests/ui/use_self_structs.rs new file mode 100644 index 000000000000..624f158164ab --- /dev/null +++ b/tests/ui/use_self_structs.rs @@ -0,0 +1,134 @@ +#![warn(clippy::use_self)] +#![allow(clippy::type_complexity)] + +fn main() {} + +struct Basic { + flag: Option>, + //~^ use_self +} + +struct BasicSelf { + okay: Option>, +} + +struct Generic<'q, T: From> { + t: &'q T, + flag: Option>>, + //~^ use_self +} + +struct GenericSelf<'q, T: From> { + t: &'q T, + okay: Option>, +} + +struct MixedLifetimes<'q, T: From + 'static> { + t: &'q T, + okay: Option>>, +} + +struct ConcreteType<'q, T: From> { + t: &'q T, + okay: Option>>, +} + +struct ConcreteAndGeneric<'q, T: From> { + t: &'q T, + flag: Option>>, + //~^ use_self + okay: Option>>, +} + +struct ConcreteAndGenericSelf<'q, T: From> { + t: &'q T, + okay_1: Option>, + okay_2: Option>>, +} + +macro_rules! recursive_struct { + ($name:ident) => { + struct $name { + okay: Option>, + } + }; +} + +recursive_struct!(X); +recursive_struct!(Y); +recursive_struct!(Z); + +struct Tree { + left: Option>, + //~^ use_self + right: Option>, + //~^ use_self +} + +struct TreeSelf { + left: Option>, + right: Option>, +} + +struct TreeMixed { + left: Option>, + right: Option>, + //~^ use_self +} + +struct Nested { + flag: Option>>>, + //~^ use_self +} + +struct NestedSelf { + okay: Option>>>, +} + +struct Tuple(Option>); +//~^ use_self + +struct TupleSelf(Option>); + +use std::cell::RefCell; +use std::rc::{Rc, Weak}; + +struct Containers { + flag: Vec>>>>>>, + //~^ use_self +} + +struct ContainersSelf { + okay: Vec>>>>>>, +} + +type Wrappers = Vec>>>>>>; + +struct Alias { + flag: Wrappers, + //~^ use_self +} + +struct AliasSelf { + okay: Wrappers, +} + +struct Array { + flag: [Option>>; N], + //~^ use_self +} + +struct ArraySelf { + okay: [Option>; N], +} + +enum Enum { + Nil, + Cons(Box), + //~^ use_self +} + +enum EnumSelf { + Nil, + Cons(Box), +} diff --git a/tests/ui/use_self_structs.stderr b/tests/ui/use_self_structs.stderr new file mode 100644 index 000000000000..766d1d4fd2c0 --- /dev/null +++ b/tests/ui/use_self_structs.stderr @@ -0,0 +1,77 @@ +error: unnecessary structure name repetition + --> tests/ui/use_self_structs.rs:7:22 + | +LL | flag: Option>, + | ^^^^^ help: use the applicable keyword: `Self` + | + = note: `-D clippy::use-self` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::use_self)]` + +error: unnecessary structure name repetition + --> tests/ui/use_self_structs.rs:17:22 + | +LL | flag: Option>>, + | ^^^^^^^^^^^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> tests/ui/use_self_structs.rs:38:22 + | +LL | flag: Option>>, + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> tests/ui/use_self_structs.rs:62:22 + | +LL | left: Option>, + | ^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> tests/ui/use_self_structs.rs:64:23 + | +LL | right: Option>, + | ^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> tests/ui/use_self_structs.rs:75:23 + | +LL | right: Option>, + | ^^^^^^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> tests/ui/use_self_structs.rs:80:33 + | +LL | flag: Option>>>, + | ^^^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> tests/ui/use_self_structs.rs:88:25 + | +LL | struct Tuple(Option>); + | ^^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> tests/ui/use_self_structs.rs:97:46 + | +LL | flag: Vec>>>>>>, + | ^^^^^^^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> tests/ui/use_self_structs.rs:108:20 + | +LL | flag: Wrappers, + | ^^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> tests/ui/use_self_structs.rs:117:23 + | +LL | flag: [Option>>; N], + | ^^^^^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> tests/ui/use_self_structs.rs:127:14 + | +LL | Cons(Box), + | ^^^^ help: use the applicable keyword: `Self` + +error: aborting due to 12 previous errors +