use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg};
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::{contains_ty, implements_trait, is_copy, is_type_diagnostic_item};
+use clippy_utils::{
+ contains_return, get_trait_def_id, in_macro, iter_input_pats, match_def_path, match_qpath, method_calls,
+ method_chain_args, paths, return_ty, single_segment_path, SpanlessEq,
+};
use if_chain::if_chain;
use rustc_ast::ast;
use rustc_errors::Applicability;
use rustc_span::symbol::{sym, SymbolStr};
use rustc_typeck::hir_ty_to_ty;
-use crate::utils::{
- contains_return, get_trait_def_id, in_macro, iter_input_pats, match_def_path, match_qpath, method_calls,
- method_chain_args, paths, return_ty, single_segment_path, SpanlessEq,
-};
-
declare_clippy_lint! {
/// **What it does:** Checks for `.unwrap()` calls on `Option`s and on `Result`s.
///
/// **What it does:** Checks for methods with certain name prefixes and which
/// doesn't match how self is taken. The actual rules are:
///
- /// |Prefix |Postfix |`self` taken |
- /// |-------|------------|----------------------|
- /// |`as_` | none |`&self` or `&mut self`|
- /// |`from_`| none | none |
- /// |`into_`| none |`self` |
- /// |`is_` | none |`&self` or none |
- /// |`to_` | `_mut` |`&mut &self` |
- /// |`to_` | not `_mut` |`&self` |
+ /// |Prefix |Postfix |`self` taken | `self` type |
+ /// |-------|------------|-----------------------|--------------|
+ /// |`as_` | none |`&self` or `&mut self` | any |
+ /// |`from_`| none | none | any |
+ /// |`into_`| none |`self` | any |
+ /// |`is_` | none |`&self` or none | any |
+ /// |`to_` | `_mut` |`&mut self` | any |
+ /// |`to_` | not `_mut` |`self` | `Copy` |
+ /// |`to_` | not `_mut` |`&self` | not `Copy` |
+ ///
+ /// Please find more info here:
+ /// https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv
///
/// **Why is this bad?** Consistency breeds readability. If you follow the
/// conventions, your users won't be surprised that they, e.g., need to supply a
declare_clippy_lint! {
/// **What it does:** Checks for an iterator or string search (such as `find()`,
- /// `position()`, or `rposition()`) followed by a call to `is_some()`.
+ /// `position()`, or `rposition()`) followed by a call to `is_some()` or `is_none()`.
///
- /// **Why is this bad?** Readability, this can be written more concisely as
- /// `_.any(_)` or `_.contains(_)`.
+ /// **Why is this bad?** Readability, this can be written more concisely as:
+ /// * `_.any(_)`, or `_.contains(_)` for `is_some()`,
+ /// * `!_.any(_)`, or `!_.contains(_)` for `is_none()`.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
- /// # let vec = vec![1];
+ /// let vec = vec![1];
/// vec.iter().find(|x| **x == 0).is_some();
+ ///
+ /// let _ = "hello world".find("world").is_none();
/// ```
/// Could be written as
/// ```rust
- /// # let vec = vec![1];
+ /// let vec = vec![1];
/// vec.iter().any(|x| *x == 0);
+ ///
+ /// let _ = !"hello world".contains("world");
/// ```
pub SEARCH_IS_SOME,
complexity,
- "using an iterator or string search followed by `is_some()`, which is more succinctly expressed as a call to `any()` or `contains()`"
+ "using an iterator or string search followed by `is_some()` or `is_none()`, which is more succinctly expressed as a call to `any()` or `contains()` (with negation in case of `is_none()`)"
}
declare_clippy_lint! {
["flat_map", "filter_map"] => filter_map_flat_map::check(cx, expr, arg_lists[1], arg_lists[0]),
["flat_map", ..] => flat_map_identity::check(cx, expr, arg_lists[0], method_spans[0]),
["flatten", "map"] => map_flatten::check(cx, expr, arg_lists[1]),
- ["is_some", "find"] => search_is_some::check(cx, expr, "find", arg_lists[1], arg_lists[0], method_spans[1]),
- ["is_some", "position"] => {
- search_is_some::check(cx, expr, "position", arg_lists[1], arg_lists[0], method_spans[1])
+ [option_check_method, "find"] if "is_some" == *option_check_method || "is_none" == *option_check_method => {
+ search_is_some::check(
+ cx,
+ expr,
+ "find",
+ option_check_method,
+ arg_lists[1],
+ arg_lists[0],
+ method_spans[1],
+ )
},
- ["is_some", "rposition"] => {
- search_is_some::check(cx, expr, "rposition", arg_lists[1], arg_lists[0], method_spans[1])
+ [option_check_method, "position"]
+ if "is_some" == *option_check_method || "is_none" == *option_check_method =>
+ {
+ search_is_some::check(
+ cx,
+ expr,
+ "position",
+ option_check_method,
+ arg_lists[1],
+ arg_lists[0],
+ method_spans[1],
+ )
+ },
+ [option_check_method, "rposition"]
+ if "is_some" == *option_check_method || "is_none" == *option_check_method =>
+ {
+ search_is_some::check(
+ cx,
+ expr,
+ "rposition",
+ option_check_method,
+ arg_lists[1],
+ arg_lists[0],
+ method_spans[1],
+ )
},
["extend", ..] => string_extend_chars::check(cx, expr, arg_lists[0]),
["count", "into_iter"] => iter_count::check(cx, expr, &arg_lists[1], "into_iter"),
let item = cx.tcx.hir().expect_item(parent);
let self_ty = cx.tcx.type_of(item.def_id);
- // if this impl block implements a trait, lint in trait definition instead
- if let hir::ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }) = item.kind {
- return;
- }
+ let implements_trait = matches!(item.kind, hir::ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }));
if_chain! {
if let hir::ImplItemKind::Fn(ref sig, id) = impl_item.kind;
if let Some(first_arg_ty) = first_arg_ty;
then {
- if cx.access_levels.is_exported(impl_item.hir_id()) {
+ // if this impl block implements a trait, lint in trait definition instead
+ if !implements_trait && cx.access_levels.is_exported(impl_item.hir_id()) {
// check missing trait implementations
for method_config in &TRAIT_METHODS {
if name == method_config.method_name &&
item.vis.node.is_pub(),
self_ty,
first_arg_ty,
- first_arg.pat.span
+ first_arg.pat.span,
+ false
);
}
}
+ // if this impl block implements a trait, lint in trait definition instead
+ if implements_trait {
+ return;
+ }
+
if let hir::ImplItemKind::Fn(_, _) = impl_item.kind {
let ret_ty = return_ty(cx, impl_item.hir_id());
false,
self_ty,
first_arg_ty,
- first_arg_span
+ first_arg_span,
+ true
);
}
}
#[must_use]
fn description(self) -> &'static str {
match self {
- Self::Value => "self by value",
- Self::Ref => "self by reference",
- Self::RefMut => "self by mutable reference",
- Self::No => "no self",
+ Self::Value => "`self` by value",
+ Self::Ref => "`self` by reference",
+ Self::RefMut => "`self` by mutable reference",
+ Self::No => "no `self`",
}
}
}