Auto merge of #5883 - flip1995:rollup-x9mftxe, r=flip1995

Rollup of 5 pull requests

Successful merges:

 - #5825 (Add the new lint `same_item_push`)
 - #5869 (New lint against `Self` as an arbitrary self type)
 - #5870 (enable #[allow(clippy::unsafe_derive_deserialize)])
 - #5871 (Lint .min(x).max(y) with x < y)
 - #5874 (Make the docs clearer for new contributors)

Failed merges:

r? @ghost

changelog: rollup
This commit is contained in:
bors 2020-08-10 13:02:53 +00:00
commit c576bedc41
28 changed files with 795 additions and 89 deletions

View file

@ -1616,6 +1616,7 @@ Released 2018-09-13
[`mutex_atomic`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutex_atomic
[`mutex_integer`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutex_integer
[`naive_bytecount`]: https://rust-lang.github.io/rust-clippy/master/index.html#naive_bytecount
[`needless_arbitrary_self_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_arbitrary_self_type
[`needless_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_bool
[`needless_borrow`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
[`needless_borrowed_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrowed_reference
@ -1687,6 +1688,7 @@ Released 2018-09-13
[`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn
[`reversed_empty_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges
[`same_functions_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_functions_in_if_condition
[`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push
[`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some
[`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse
[`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse

View file

@ -28,11 +28,14 @@ All contributors are expected to follow the [Rust Code of Conduct].
## Getting started
High level approach:
**Note: If this is your first time contributing to Clippy, you should
first read the [Basics docs](doc/basics.md).**
### High level approach
1. Find something to fix/improve
2. Change code (likely some file in `clippy_lints/src/`)
3. Follow the instructions in the [Basics docs](doc/basics.md) such as running the `setup-toolchain.sh` script
3. Follow the instructions in the [Basics docs](doc/basics.md) to get set up
4. Run `cargo test` in the root directory and wiggle code until it passes
5. Open a PR (also can be done after 2. if you run into problems)

View file

@ -1,7 +1,7 @@
use crate::utils::paths;
use crate::utils::{
get_trait_def_id, is_automatically_derived, is_copy, match_path, span_lint_and_help, span_lint_and_note,
span_lint_and_then,
get_trait_def_id, is_allowed, is_automatically_derived, is_copy, match_path, span_lint_and_help,
span_lint_and_note, span_lint_and_then,
};
use if_chain::if_chain;
use rustc_hir::def_id::DefId;
@ -354,7 +354,9 @@ fn check_unsafe_derive_deserialize<'tcx>(
if_chain! {
if match_path(&trait_ref.path, &paths::SERDE_DESERIALIZE);
if let ty::Adt(def, _) = ty.kind;
if def.did.is_local();
if let Some(local_def_id) = def.did.as_local();
let adt_hir_id = cx.tcx.hir().as_local_hir_id(local_def_id);
if !is_allowed(cx, UNSAFE_DERIVE_DESERIALIZE, adt_hir_id);
if cx.tcx.inherent_impls(def.did)
.iter()
.map(|imp_did| item_from_def_id(cx, *imp_did))

View file

@ -250,6 +250,7 @@ mod mut_mut;
mod mut_reference;
mod mutable_debug_assertion;
mod mutex_atomic;
mod needless_arbitrary_self_type;
mod needless_bool;
mod needless_borrow;
mod needless_borrowed_ref;
@ -608,6 +609,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&loops::NEEDLESS_COLLECT,
&loops::NEEDLESS_RANGE_LOOP,
&loops::NEVER_LOOP,
&loops::SAME_ITEM_PUSH,
&loops::WHILE_IMMUTABLE_CONDITION,
&loops::WHILE_LET_LOOP,
&loops::WHILE_LET_ON_ITERATOR,
@ -717,6 +719,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL,
&mutex_atomic::MUTEX_ATOMIC,
&mutex_atomic::MUTEX_INTEGER,
&needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE,
&needless_bool::BOOL_COMPARISON,
&needless_bool::NEEDLESS_BOOL,
&needless_borrow::NEEDLESS_BORROW,
@ -1027,6 +1030,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| box items_after_statements::ItemsAfterStatements);
store.register_early_pass(|| box precedence::Precedence);
store.register_early_pass(|| box needless_continue::NeedlessContinue);
store.register_early_pass(|| box needless_arbitrary_self_type::NeedlessArbitrarySelfType);
store.register_early_pass(|| box redundant_static_lifetimes::RedundantStaticLifetimes);
store.register_late_pass(|| box cargo_common_metadata::CargoCommonMetadata);
store.register_late_pass(|| box multiple_crate_versions::MultipleCrateVersions);
@ -1295,6 +1299,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::NEEDLESS_COLLECT),
LintId::of(&loops::NEEDLESS_RANGE_LOOP),
LintId::of(&loops::NEVER_LOOP),
LintId::of(&loops::SAME_ITEM_PUSH),
LintId::of(&loops::WHILE_IMMUTABLE_CONDITION),
LintId::of(&loops::WHILE_LET_LOOP),
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
@ -1371,6 +1376,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&mut_key::MUTABLE_KEY_TYPE),
LintId::of(&mut_reference::UNNECESSARY_MUT_PASSED),
LintId::of(&mutex_atomic::MUTEX_ATOMIC),
LintId::of(&needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE),
LintId::of(&needless_bool::BOOL_COMPARISON),
LintId::of(&needless_bool::NEEDLESS_BOOL),
LintId::of(&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
@ -1497,6 +1503,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::EMPTY_LOOP),
LintId::of(&loops::FOR_KV_MAP),
LintId::of(&loops::NEEDLESS_RANGE_LOOP),
LintId::of(&loops::SAME_ITEM_PUSH),
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
LintId::of(&main_recursion::MAIN_RECURSION),
LintId::of(&manual_async_fn::MANUAL_ASYNC_FN),
@ -1602,6 +1609,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&misc::SHORT_CIRCUIT_STATEMENT),
LintId::of(&misc_early::UNNEEDED_WILDCARD_PATTERN),
LintId::of(&misc_early::ZERO_PREFIXED_LITERAL),
LintId::of(&needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE),
LintId::of(&needless_bool::BOOL_COMPARISON),
LintId::of(&needless_bool::NEEDLESS_BOOL),
LintId::of(&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),

View file

@ -5,8 +5,9 @@ use crate::utils::usage::{is_unused, mutated_variables};
use crate::utils::{
get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item, last_path_segment, match_trait_method,
match_type, match_var, multispan_sugg, qpath_res, snippet, snippet_opt, snippet_with_applicability, span_lint,
span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg, SpanlessEq,
match_type, match_var, multispan_sugg, qpath_res, snippet, snippet_opt, snippet_with_applicability,
snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg,
SpanlessEq,
};
use if_chain::if_chain;
use rustc_ast::ast;
@ -419,6 +420,39 @@ declare_clippy_lint! {
"variables used within while expression are not mutated in the body"
}
declare_clippy_lint! {
/// **What it does:** Checks whether a for loop is being used to push a constant
/// value into a Vec.
///
/// **Why is this bad?** This kind of operation can be expressed more succinctly with
/// `vec![item;SIZE]` or `vec.resize(NEW_SIZE, item)` and using these alternatives may also
/// have better performance.
/// **Known problems:** None
///
/// **Example:**
/// ```rust
/// let item1 = 2;
/// let item2 = 3;
/// let mut vec: Vec<u8> = Vec::new();
/// for _ in 0..20 {
/// vec.push(item1);
/// }
/// for _ in 0..30 {
/// vec.push(item2);
/// }
/// ```
/// could be written as
/// ```rust
/// let item1 = 2;
/// let item2 = 3;
/// let mut vec: Vec<u8> = vec![item1; 20];
/// vec.resize(20 + 30, item2);
/// ```
pub SAME_ITEM_PUSH,
style,
"the same item is pushed inside of a for loop"
}
declare_lint_pass!(Loops => [
MANUAL_MEMCPY,
NEEDLESS_RANGE_LOOP,
@ -435,6 +469,7 @@ declare_lint_pass!(Loops => [
NEVER_LOOP,
MUT_RANGE_BOUND,
WHILE_IMMUTABLE_CONDITION,
SAME_ITEM_PUSH,
]);
impl<'tcx> LateLintPass<'tcx> for Loops {
@ -740,6 +775,7 @@ fn check_for_loop<'tcx>(
check_for_loop_over_map_kv(cx, pat, arg, body, expr);
check_for_mut_range_bound(cx, arg, body);
detect_manual_memcpy(cx, pat, arg, body, expr);
detect_same_item_push(cx, pat, arg, body, expr);
}
fn same_var<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, var: HirId) -> bool {
@ -1016,6 +1052,117 @@ fn detect_manual_memcpy<'tcx>(
}
}
// Scans the body of the for loop and determines whether lint should be given
struct SameItemPushVisitor<'a, 'tcx> {
should_lint: bool,
// this field holds the last vec push operation visited, which should be the only push seen
vec_push: Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>,
cx: &'a LateContext<'tcx>,
}
impl<'a, 'tcx> Visitor<'tcx> for SameItemPushVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
match &expr.kind {
// Non-determinism may occur ... don't give a lint
ExprKind::Loop(_, _, _) | ExprKind::Match(_, _, _) => self.should_lint = false,
ExprKind::Block(block, _) => self.visit_block(block),
_ => {},
}
}
fn visit_block(&mut self, b: &'tcx Block<'_>) {
for stmt in b.stmts.iter() {
self.visit_stmt(stmt);
}
}
fn visit_stmt(&mut self, s: &'tcx Stmt<'_>) {
let vec_push_option = get_vec_push(self.cx, s);
if vec_push_option.is_none() {
// Current statement is not a push so visit inside
match &s.kind {
StmtKind::Expr(expr) | StmtKind::Semi(expr) => self.visit_expr(&expr),
_ => {},
}
} else {
// Current statement is a push ...check whether another
// push had been previously done
if self.vec_push.is_none() {
self.vec_push = vec_push_option;
} else {
// There are multiple pushes ... don't lint
self.should_lint = false;
}
}
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}
// Given some statement, determine if that statement is a push on a Vec. If it is, return
// the Vec being pushed into and the item being pushed
fn get_vec_push<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> {
if_chain! {
// Extract method being called
if let StmtKind::Semi(semi_stmt) = &stmt.kind;
if let ExprKind::MethodCall(path, _, args, _) = &semi_stmt.kind;
// Figure out the parameters for the method call
if let Some(self_expr) = args.get(0);
if let Some(pushed_item) = args.get(1);
// Check that the method being called is push() on a Vec
if match_type(cx, cx.typeck_results().expr_ty(self_expr), &paths::VEC);
if path.ident.name.as_str() == "push";
then {
return Some((self_expr, pushed_item))
}
}
None
}
/// Detects for loop pushing the same item into a Vec
fn detect_same_item_push<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
_: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
_: &'tcx Expr<'_>,
) {
// Determine whether it is safe to lint the body
let mut same_item_push_visitor = SameItemPushVisitor {
should_lint: true,
vec_push: None,
cx,
};
walk_expr(&mut same_item_push_visitor, body);
if same_item_push_visitor.should_lint {
if let Some((vec, pushed_item)) = same_item_push_visitor.vec_push {
// Make sure that the push does not involve possibly mutating values
if mutated_variables(pushed_item, cx).map_or(false, |mutvars| mutvars.is_empty()) {
if let PatKind::Wild = pat.kind {
let vec_str = snippet_with_macro_callsite(cx, vec.span, "");
let item_str = snippet_with_macro_callsite(cx, pushed_item.span, "");
span_lint_and_help(
cx,
SAME_ITEM_PUSH,
vec.span,
"it looks like the same item is being pushed into this Vec",
None,
&format!(
"try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})",
item_str, vec_str, item_str
),
)
}
}
}
}
}
/// Checks for looping over a range and then indexing a sequence with it.
/// The iteratee must be a range literal.
#[allow(clippy::too_many_lines)]

View file

@ -1,5 +1,6 @@
use crate::consts::{constant_simple, Constant};
use crate::utils::{match_def_path, paths, span_lint};
use crate::utils::{match_def_path, match_trait_method, paths, span_lint};
use if_chain::if_chain;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
@ -18,6 +19,10 @@ declare_clippy_lint! {
/// ```ignore
/// min(0, max(100, x))
/// ```
/// or
/// ```ignore
/// x.max(100).min(0)
/// ```
/// It will always be equal to `0`. Probably the author meant to clamp the value
/// between 0 and 100, but has erroneously swapped `min` and `max`.
pub MIN_MAX,
@ -60,25 +65,43 @@ enum MinMax {
}
fn min_max<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<(MinMax, Constant, &'a Expr<'a>)> {
if let ExprKind::Call(ref path, ref args) = expr.kind {
if let ExprKind::Path(ref qpath) = path.kind {
cx.typeck_results()
.qpath_res(qpath, path.hir_id)
.opt_def_id()
.and_then(|def_id| {
if match_def_path(cx, def_id, &paths::CMP_MIN) {
fetch_const(cx, args, MinMax::Min)
} else if match_def_path(cx, def_id, &paths::CMP_MAX) {
match expr.kind {
ExprKind::Call(ref path, ref args) => {
if let ExprKind::Path(ref qpath) = path.kind {
cx.typeck_results()
.qpath_res(qpath, path.hir_id)
.opt_def_id()
.and_then(|def_id| {
if match_def_path(cx, def_id, &paths::CMP_MIN) {
fetch_const(cx, args, MinMax::Min)
} else if match_def_path(cx, def_id, &paths::CMP_MAX) {
fetch_const(cx, args, MinMax::Max)
} else {
None
}
})
} else {
None
}
},
ExprKind::MethodCall(ref path, _, ref args, _) => {
if_chain! {
if let [obj, _] = args;
if cx.typeck_results().expr_ty(obj).is_floating_point() || match_trait_method(cx, expr, &paths::ORD);
then {
if path.ident.as_str() == sym!(max).as_str() {
fetch_const(cx, args, MinMax::Max)
} else if path.ident.as_str() == sym!(min).as_str() {
fetch_const(cx, args, MinMax::Min)
} else {
None
}
})
} else {
None
}
} else {
None
} else {
None
}
}
},
_ => None,
}
}

View file

@ -0,0 +1,118 @@
use crate::utils::span_lint_and_sugg;
use if_chain::if_chain;
use rustc_ast::ast::{BindingMode, Lifetime, Mutability, Param, PatKind, Path, TyKind};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::kw;
use rustc_span::Span;
declare_clippy_lint! {
/// **What it does:** The lint checks for `self` in fn parameters that
/// specify the `Self`-type explicitly
/// **Why is this bad?** Increases the amount and decreases the readability of code
///
/// **Known problems:** None
///
/// **Example:**
/// ```rust
/// enum ValType {
/// I32,
/// I64,
/// F32,
/// F64,
/// }
///
/// impl ValType {
/// pub fn bytes(self: Self) -> usize {
/// match self {
/// Self::I32 | Self::F32 => 4,
/// Self::I64 | Self::F64 => 8,
/// }
/// }
/// }
/// ```
///
/// Could be rewritten as
///
/// ```rust
/// enum ValType {
/// I32,
/// I64,
/// F32,
/// F64,
/// }
///
/// impl ValType {
/// pub fn bytes(self) -> usize {
/// match self {
/// Self::I32 | Self::F32 => 4,
/// Self::I64 | Self::F64 => 8,
/// }
/// }
/// }
/// ```
pub NEEDLESS_ARBITRARY_SELF_TYPE,
complexity,
"type of `self` parameter is already by default `Self`"
}
declare_lint_pass!(NeedlessArbitrarySelfType => [NEEDLESS_ARBITRARY_SELF_TYPE]);
enum Mode {
Ref(Option<Lifetime>),
Value,
}
fn check_param_inner(cx: &EarlyContext<'_>, path: &Path, span: Span, binding_mode: &Mode, mutbl: Mutability) {
if_chain! {
if let [segment] = &path.segments[..];
if segment.ident.name == kw::SelfUpper;
then {
let self_param = match (binding_mode, mutbl) {
(Mode::Ref(None), Mutability::Mut) => "&mut self".to_string(),
(Mode::Ref(Some(lifetime)), Mutability::Mut) => format!("&{} mut self", &lifetime.ident.name),
(Mode::Ref(None), Mutability::Not) => "&self".to_string(),
(Mode::Ref(Some(lifetime)), Mutability::Not) => format!("&{} self", &lifetime.ident.name),
(Mode::Value, Mutability::Mut) => "mut self".to_string(),
(Mode::Value, Mutability::Not) => "self".to_string(),
};
span_lint_and_sugg(
cx,
NEEDLESS_ARBITRARY_SELF_TYPE,
span,
"the type of the `self` parameter does not need to be arbitrary",
"consider to change this parameter to",
self_param,
Applicability::MachineApplicable,
)
}
}
}
impl EarlyLintPass for NeedlessArbitrarySelfType {
fn check_param(&mut self, cx: &EarlyContext<'_>, p: &Param) {
if !p.is_self() {
return;
}
match &p.ty.kind {
TyKind::Path(None, path) => {
if let PatKind::Ident(BindingMode::ByValue(mutbl), _, _) = p.pat.kind {
check_param_inner(cx, path, p.span.to(p.ty.span), &Mode::Value, mutbl)
}
},
TyKind::Rptr(lifetime, mut_ty) => {
if_chain! {
if let TyKind::Path(None, path) = &mut_ty.ty.kind;
if let PatKind::Ident(BindingMode::ByValue(Mutability::Not), _, _) = p.pat.kind;
then {
check_param_inner(cx, path, p.span.to(p.ty.span), &Mode::Ref(*lifetime), mut_ty.mutbl)
}
}
},
_ => {},
}
}
}

View file

@ -50,7 +50,7 @@ declare_clippy_lint! {
/// fn func<T: Clone + Default>(arg: T) {}
/// ```
/// or
/// ///
///
/// ```rust
/// fn func<T>(arg: T) where T: Clone + Default {}
/// ```

View file

@ -53,6 +53,9 @@ rustup-toolchain-install-master -f -n master -c rustc-dev -c llvm-tools
rustup override set master
```
_Note:_ Sometimes you may get compiler errors when building Clippy, even if you
didn't change anything. Normally those will be fixed by a maintainer in a few hours.
## Building and Testing
Once the `master` toolchain is installed, you can build and test Clippy like

View file

@ -1459,6 +1459,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "bytecount",
},
Lint {
name: "needless_arbitrary_self_type",
group: "complexity",
desc: "type of `self` parameter is already by default `Self`",
deprecation: None,
module: "needless_arbitrary_self_type",
},
Lint {
name: "needless_bool",
group: "complexity",
@ -1935,6 +1942,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "copies",
},
Lint {
name: "same_item_push",
group: "style",
desc: "the same item is pushed inside of a for loop",
deprecation: None,
module: "loops",
},
Lint {
name: "search_is_some",
group: "complexity",

View file

@ -1,4 +1,10 @@
#![allow(unused, dead_code, clippy::needless_lifetimes, clippy::needless_pass_by_value)]
#![allow(
unused,
dead_code,
clippy::needless_lifetimes,
clippy::needless_pass_by_value,
clippy::needless_arbitrary_self_type
)]
#![warn(clippy::extra_unused_lifetimes)]
fn empty() {}

View file

@ -1,5 +1,5 @@
error: this lifetime isn't used in the function definition
--> $DIR/extra_unused_lifetimes.rs:8:14
--> $DIR/extra_unused_lifetimes.rs:14:14
|
LL | fn unused_lt<'a>(x: u8) {}
| ^^
@ -7,19 +7,19 @@ LL | fn unused_lt<'a>(x: u8) {}
= note: `-D clippy::extra-unused-lifetimes` implied by `-D warnings`
error: this lifetime isn't used in the function definition
--> $DIR/extra_unused_lifetimes.rs:10:25
--> $DIR/extra_unused_lifetimes.rs:16:25
|
LL | fn unused_lt_transitive<'a, 'b: 'a>(x: &'b u8) {
| ^^
error: this lifetime isn't used in the function definition
--> $DIR/extra_unused_lifetimes.rs:35:10
--> $DIR/extra_unused_lifetimes.rs:41:10
|
LL | fn x<'a>(&self) {}
| ^^
error: this lifetime isn't used in the function definition
--> $DIR/extra_unused_lifetimes.rs:61:22
--> $DIR/extra_unused_lifetimes.rs:67:22
|
LL | fn unused_lt<'a>(x: u8) {}
| ^^

View file

@ -4,14 +4,14 @@
pub struct PubOne;
impl PubOne {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
}
impl PubOne {
// A second impl for this struct -- the error span shouldn't mention this.
pub fn irrelevant(self: &Self) -> bool {
pub fn irrelevant(&self) -> bool {
false
}
}
@ -21,7 +21,7 @@ pub struct PubAllowed;
#[allow(clippy::len_without_is_empty)]
impl PubAllowed {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
}
@ -29,17 +29,17 @@ impl PubAllowed {
// No `allow` attribute on this impl block, but that doesn't matter -- we only require one on the
// impl containing `len`.
impl PubAllowed {
pub fn irrelevant(self: &Self) -> bool {
pub fn irrelevant(&self) -> bool {
false
}
}
pub trait PubTraitsToo {
fn len(self: &Self) -> isize;
fn len(&self) -> isize;
}
impl PubTraitsToo for One {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
0
}
}
@ -47,11 +47,11 @@ impl PubTraitsToo for One {
pub struct HasIsEmpty;
impl HasIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}
@ -59,11 +59,11 @@ impl HasIsEmpty {
pub struct HasWrongIsEmpty;
impl HasWrongIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
pub fn is_empty(self: &Self, x: u32) -> bool {
pub fn is_empty(&self, x: u32) -> bool {
false
}
}
@ -71,7 +71,7 @@ impl HasWrongIsEmpty {
struct NotPubOne;
impl NotPubOne {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
// No error; `len` is pub but `NotPubOne` is not exported anyway.
1
}
@ -80,19 +80,19 @@ impl NotPubOne {
struct One;
impl One {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
// No error; `len` is private; see issue #1085.
1
}
}
trait TraitsToo {
fn len(self: &Self) -> isize;
fn len(&self) -> isize;
// No error; `len` is private; see issue #1085.
}
impl TraitsToo for One {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
0
}
}
@ -100,11 +100,11 @@ impl TraitsToo for One {
struct HasPrivateIsEmpty;
impl HasPrivateIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}
@ -112,16 +112,16 @@ impl HasPrivateIsEmpty {
struct Wither;
pub trait WithIsEmpty {
fn len(self: &Self) -> isize;
fn is_empty(self: &Self) -> bool;
fn len(&self) -> isize;
fn is_empty(&self) -> bool;
}
impl WithIsEmpty for Wither {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}

View file

@ -2,7 +2,7 @@ error: item `PubOne` has a public `len` method but no corresponding `is_empty` m
--> $DIR/len_without_is_empty.rs:6:1
|
LL | / impl PubOne {
LL | | pub fn len(self: &Self) -> isize {
LL | | pub fn len(&self) -> isize {
LL | | 1
LL | | }
LL | | }
@ -14,7 +14,7 @@ error: trait `PubTraitsToo` has a `len` method but no (possibly inherited) `is_e
--> $DIR/len_without_is_empty.rs:37:1
|
LL | / pub trait PubTraitsToo {
LL | | fn len(self: &Self) -> isize;
LL | | fn len(&self) -> isize;
LL | | }
| |_^
@ -22,7 +22,7 @@ error: item `HasIsEmpty` has a public `len` method but a private `is_empty` meth
--> $DIR/len_without_is_empty.rs:49:1
|
LL | / impl HasIsEmpty {
LL | | pub fn len(self: &Self) -> isize {
LL | | pub fn len(&self) -> isize {
LL | | 1
LL | | }
... |
@ -34,7 +34,7 @@ error: item `HasWrongIsEmpty` has a public `len` method but no corresponding `is
--> $DIR/len_without_is_empty.rs:61:1
|
LL | / impl HasWrongIsEmpty {
LL | | pub fn len(self: &Self) -> isize {
LL | | pub fn len(&self) -> isize {
LL | | 1
LL | | }
... |

View file

@ -7,12 +7,12 @@ pub struct One;
struct Wither;
trait TraitsToo {
fn len(self: &Self) -> isize;
fn len(&self) -> isize;
// No error; `len` is private; see issue #1085.
}
impl TraitsToo for One {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
0
}
}
@ -20,11 +20,11 @@ impl TraitsToo for One {
pub struct HasIsEmpty;
impl HasIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}
@ -32,26 +32,26 @@ impl HasIsEmpty {
pub struct HasWrongIsEmpty;
impl HasWrongIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
pub fn is_empty(self: &Self, x: u32) -> bool {
pub fn is_empty(&self, x: u32) -> bool {
false
}
}
pub trait WithIsEmpty {
fn len(self: &Self) -> isize;
fn is_empty(self: &Self) -> bool;
fn len(&self) -> isize;
fn is_empty(&self) -> bool;
}
impl WithIsEmpty for Wither {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}

View file

@ -7,12 +7,12 @@ pub struct One;
struct Wither;
trait TraitsToo {
fn len(self: &Self) -> isize;
fn len(&self) -> isize;
// No error; `len` is private; see issue #1085.
}
impl TraitsToo for One {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
0
}
}
@ -20,11 +20,11 @@ impl TraitsToo for One {
pub struct HasIsEmpty;
impl HasIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}
@ -32,26 +32,26 @@ impl HasIsEmpty {
pub struct HasWrongIsEmpty;
impl HasWrongIsEmpty {
pub fn len(self: &Self) -> isize {
pub fn len(&self) -> isize {
1
}
pub fn is_empty(self: &Self, x: u32) -> bool {
pub fn is_empty(&self, x: u32) -> bool {
false
}
}
pub trait WithIsEmpty {
fn len(self: &Self) -> isize;
fn is_empty(self: &Self) -> bool;
fn len(&self) -> isize;
fn is_empty(&self) -> bool;
}
impl WithIsEmpty for Wither {
fn len(self: &Self) -> isize {
fn len(&self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
fn is_empty(&self) -> bool {
false
}
}

View file

@ -6,6 +6,18 @@ use std::cmp::{max, min};
const LARGE: usize = 3;
struct NotOrd(u64);
impl NotOrd {
fn min(self, x: u64) -> NotOrd {
NotOrd(x)
}
fn max(self, x: u64) -> NotOrd {
NotOrd(x)
}
}
fn main() {
let x;
x = 2usize;
@ -30,4 +42,24 @@ fn main() {
max(min(s, "Apple"), "Zoo");
max("Apple", min(s, "Zoo")); // ok
let f = 3f32;
x.min(1).max(3);
x.max(3).min(1);
f.max(3f32).min(1f32);
x.max(1).min(3); // ok
x.min(3).max(1); // ok
f.min(3f32).max(1f32); // ok
max(x.min(1), 3);
min(x.max(1), 3); // ok
s.max("Zoo").min("Apple");
s.min("Apple").max("Zoo");
s.min("Zoo").max("Apple"); // ok
let not_ord = NotOrd(1);
not_ord.min(1).max(3); // ok
}

View file

@ -1,5 +1,5 @@
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:12:5
--> $DIR/min_max.rs:24:5
|
LL | min(1, max(3, x));
| ^^^^^^^^^^^^^^^^^
@ -7,40 +7,76 @@ LL | min(1, max(3, x));
= note: `-D clippy::min-max` implied by `-D warnings`
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:13:5
--> $DIR/min_max.rs:25:5
|
LL | min(max(3, x), 1);
| ^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:14:5
--> $DIR/min_max.rs:26:5
|
LL | max(min(x, 1), 3);
| ^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:15:5
--> $DIR/min_max.rs:27:5
|
LL | max(3, min(x, 1));
| ^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:17:5
--> $DIR/min_max.rs:29:5
|
LL | my_max(3, my_min(x, 1));
| ^^^^^^^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:29:5
--> $DIR/min_max.rs:41:5
|
LL | min("Apple", max("Zoo", s));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:30:5
--> $DIR/min_max.rs:42:5
|
LL | max(min(s, "Apple"), "Zoo");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 7 previous errors
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:47:5
|
LL | x.min(1).max(3);
| ^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:48:5
|
LL | x.max(3).min(1);
| ^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:49:5
|
LL | f.max(3f32).min(1f32);
| ^^^^^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:55:5
|
LL | max(x.min(1), 3);
| ^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:58:5
|
LL | s.max("Zoo").min("Apple");
| ^^^^^^^^^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:59:5
|
LL | s.min("Apple").max("Zoo");
| ^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 13 previous errors

View file

@ -0,0 +1,69 @@
// run-rustfix
#![warn(clippy::needless_arbitrary_self_type)]
#![allow(unused_mut, clippy::needless_lifetimes)]
pub enum ValType {
A,
B,
}
impl ValType {
pub fn bad(self) {
unimplemented!();
}
pub fn good(self) {
unimplemented!();
}
pub fn mut_bad(mut self) {
unimplemented!();
}
pub fn mut_good(mut self) {
unimplemented!();
}
pub fn ref_bad(&self) {
unimplemented!();
}
pub fn ref_good(&self) {
unimplemented!();
}
pub fn ref_bad_with_lifetime<'a>(&'a self) {
unimplemented!();
}
pub fn ref_good_with_lifetime<'a>(&'a self) {
unimplemented!();
}
pub fn mut_ref_bad(&mut self) {
unimplemented!();
}
pub fn mut_ref_good(&mut self) {
unimplemented!();
}
pub fn mut_ref_bad_with_lifetime<'a>(&'a mut self) {
unimplemented!();
}
pub fn mut_ref_good_with_lifetime<'a>(&'a mut self) {
unimplemented!();
}
pub fn mut_ref_mut_good(mut self: &mut Self) {
unimplemented!();
}
pub fn mut_ref_mut_ref_good(self: &&mut &mut Self) {
unimplemented!();
}
}
fn main() {}

View file

@ -0,0 +1,69 @@
// run-rustfix
#![warn(clippy::needless_arbitrary_self_type)]
#![allow(unused_mut, clippy::needless_lifetimes)]
pub enum ValType {
A,
B,
}
impl ValType {
pub fn bad(self: Self) {
unimplemented!();
}
pub fn good(self) {
unimplemented!();
}
pub fn mut_bad(mut self: Self) {
unimplemented!();
}
pub fn mut_good(mut self) {
unimplemented!();
}
pub fn ref_bad(self: &Self) {
unimplemented!();
}
pub fn ref_good(&self) {
unimplemented!();
}
pub fn ref_bad_with_lifetime<'a>(self: &'a Self) {
unimplemented!();
}
pub fn ref_good_with_lifetime<'a>(&'a self) {
unimplemented!();
}
pub fn mut_ref_bad(self: &mut Self) {
unimplemented!();
}
pub fn mut_ref_good(&mut self) {
unimplemented!();
}
pub fn mut_ref_bad_with_lifetime<'a>(self: &'a mut Self) {
unimplemented!();
}
pub fn mut_ref_good_with_lifetime<'a>(&'a mut self) {
unimplemented!();
}
pub fn mut_ref_mut_good(mut self: &mut Self) {
unimplemented!();
}
pub fn mut_ref_mut_ref_good(self: &&mut &mut Self) {
unimplemented!();
}
}
fn main() {}

View file

@ -0,0 +1,40 @@
error: the type of the `self` parameter does not need to be arbitrary
--> $DIR/needless_arbitrary_self_type.rs:12:16
|
LL | pub fn bad(self: Self) {
| ^^^^^^^^^^ help: consider to change this parameter to: `self`
|
= note: `-D clippy::needless-arbitrary-self-type` implied by `-D warnings`
error: the type of the `self` parameter does not need to be arbitrary
--> $DIR/needless_arbitrary_self_type.rs:20:20
|
LL | pub fn mut_bad(mut self: Self) {
| ^^^^^^^^^^^^^^ help: consider to change this parameter to: `mut self`
error: the type of the `self` parameter does not need to be arbitrary
--> $DIR/needless_arbitrary_self_type.rs:28:20
|
LL | pub fn ref_bad(self: &Self) {
| ^^^^^^^^^^^ help: consider to change this parameter to: `&self`
error: the type of the `self` parameter does not need to be arbitrary
--> $DIR/needless_arbitrary_self_type.rs:36:38
|
LL | pub fn ref_bad_with_lifetime<'a>(self: &'a Self) {
| ^^^^^^^^^^^^^^ help: consider to change this parameter to: `&'a self`
error: the type of the `self` parameter does not need to be arbitrary
--> $DIR/needless_arbitrary_self_type.rs:44:24
|
LL | pub fn mut_ref_bad(self: &mut Self) {
| ^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&mut self`
error: the type of the `self` parameter does not need to be arbitrary
--> $DIR/needless_arbitrary_self_type.rs:52:42
|
LL | pub fn mut_ref_bad_with_lifetime<'a>(self: &'a mut Self) {
| ^^^^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&'a mut self`
error: aborting due to 6 previous errors

View file

@ -22,9 +22,9 @@ struct HasOption {
}
impl HasOption {
fn do_option_nothing(self: &Self, value: usize) {}
fn do_option_nothing(&self, value: usize) {}
fn do_option_plus_one(self: &Self, value: usize) -> usize {
fn do_option_plus_one(&self, value: usize) -> usize {
value + 1
}
}

View file

@ -22,9 +22,9 @@ struct HasOption {
}
impl HasOption {
fn do_option_nothing(self: &Self, value: usize) {}
fn do_option_nothing(&self, value: usize) {}
fn do_option_plus_one(self: &Self, value: usize) -> usize {
fn do_option_plus_one(&self, value: usize) -> usize {
value + 1
}
}

View file

@ -18,9 +18,9 @@ struct HasResult {
}
impl HasResult {
fn do_result_nothing(self: &Self, value: usize) {}
fn do_result_nothing(&self, value: usize) {}
fn do_result_plus_one(self: &Self, value: usize) -> usize {
fn do_result_plus_one(&self, value: usize) -> usize {
value + 1
}
}

View file

@ -18,9 +18,9 @@ struct HasResult {
}
impl HasResult {
fn do_result_nothing(self: &Self, value: usize) {}
fn do_result_nothing(&self, value: usize) {}
fn do_result_plus_one(self: &Self, value: usize) -> usize {
fn do_result_plus_one(&self, value: usize) -> usize {
value + 1
}
}

View file

@ -0,0 +1,89 @@
#![warn(clippy::same_item_push)]
fn mutate_increment(x: &mut u8) -> u8 {
*x += 1;
*x
}
fn increment(x: u8) -> u8 {
x + 1
}
fn main() {
// Test for basic case
let mut spaces = Vec::with_capacity(10);
for _ in 0..10 {
spaces.push(vec![b' ']);
}
let mut vec2: Vec<u8> = Vec::new();
let item = 2;
for _ in 5..=20 {
vec2.push(item);
}
let mut vec3: Vec<u8> = Vec::new();
for _ in 0..15 {
let item = 2;
vec3.push(item);
}
let mut vec4: Vec<u8> = Vec::new();
for _ in 0..15 {
vec4.push(13);
}
// Suggestion should not be given as pushed variable can mutate
let mut vec5: Vec<u8> = Vec::new();
let mut item: u8 = 2;
for _ in 0..30 {
vec5.push(mutate_increment(&mut item));
}
let mut vec6: Vec<u8> = Vec::new();
let mut item: u8 = 2;
let mut item2 = &mut mutate_increment(&mut item);
for _ in 0..30 {
vec6.push(mutate_increment(item2));
}
let mut vec7: Vec<usize> = Vec::new();
for (a, b) in [0, 1, 4, 9, 16].iter().enumerate() {
vec7.push(a);
}
let mut vec8: Vec<u8> = Vec::new();
for i in 0..30 {
vec8.push(increment(i));
}
let mut vec9: Vec<u8> = Vec::new();
for i in 0..30 {
vec9.push(i + i * i);
}
// Suggestion should not be given as there are multiple pushes that are not the same
let mut vec10: Vec<u8> = Vec::new();
let item: u8 = 2;
for _ in 0..30 {
vec10.push(item);
vec10.push(item * 2);
}
// Suggestion should not be given as Vec is not involved
for _ in 0..5 {
println!("Same Item Push");
}
struct A {
kind: u32,
}
let mut vec_a: Vec<A> = Vec::new();
for i in 0..30 {
vec_a.push(A { kind: i });
}
let mut vec12: Vec<u8> = Vec::new();
for a in vec_a {
vec12.push(2u8.pow(a.kind));
}
}

View file

@ -0,0 +1,35 @@
error: it looks like the same item is being pushed into this Vec
--> $DIR/same_item_push.rs:16:9
|
LL | spaces.push(vec![b' ']);
| ^^^^^^
|
= note: `-D clippy::same-item-push` implied by `-D warnings`
= help: try using vec![vec![b' '];SIZE] or spaces.resize(NEW_SIZE, vec![b' '])
error: it looks like the same item is being pushed into this Vec
--> $DIR/same_item_push.rs:22:9
|
LL | vec2.push(item);
| ^^^^
|
= help: try using vec![item;SIZE] or vec2.resize(NEW_SIZE, item)
error: it looks like the same item is being pushed into this Vec
--> $DIR/same_item_push.rs:28:9
|
LL | vec3.push(item);
| ^^^^
|
= help: try using vec![item;SIZE] or vec3.resize(NEW_SIZE, item)
error: it looks like the same item is being pushed into this Vec
--> $DIR/same_item_push.rs:33:9
|
LL | vec4.push(13);
| ^^^^
|
= help: try using vec![13;SIZE] or vec4.resize(NEW_SIZE, 13)
error: aborting due to 4 previous errors

View file

@ -57,4 +57,14 @@ impl E {
#[derive(Deserialize)]
pub struct F {}
// Check that we honor the `allow` attribute on the ADT
#[allow(clippy::unsafe_derive_deserialize)]
#[derive(Deserialize)]
pub struct G {}
impl G {
pub fn unsafe_block(&self) {
unsafe {}
}
}
fn main() {}