From cc622608db7318b1c0fe3ccd541558436c7c6c4c Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Wed, 18 Sep 2019 08:37:41 +0200 Subject: [PATCH] new lints around `#[must_use]` fns `must_use_unit` lints unit-returning functions with a `#[must_use]` attribute, suggesting to remove it. `double_must_use` lints functions with a plain `#[must_use]` attribute, but which return a type which is already `#[must_use]`, so the attribute has no benefit. `must_use_candidate` is a pedantic lint that lints functions and methods that return some non-unit type that is not already `#[must_use]` and suggests to add the annotation. --- CHANGELOG.md | 3 + README.md | 2 +- clippy_dev/src/lib.rs | 7 + clippy_dev/src/stderr_length_check.rs | 1 + clippy_lints/src/approx_const.rs | 1 + clippy_lints/src/assign_ops.rs | 1 + clippy_lints/src/bit_mask.rs | 2 + clippy_lints/src/checked_conversions.rs | 1 + clippy_lints/src/cognitive_complexity.rs | 1 + clippy_lints/src/doc.rs | 1 + clippy_lints/src/enum_variants.rs | 4 + clippy_lints/src/excessive_precision.rs | 5 + clippy_lints/src/formatting.rs | 1 + clippy_lints/src/functions.rs | 412 +++++++++++++++++-- clippy_lints/src/infinite_iter.rs | 3 + clippy_lints/src/large_enum_variant.rs | 1 + clippy_lints/src/lib.rs | 7 +- clippy_lints/src/lifetimes.rs | 1 + clippy_lints/src/literal_representation.rs | 9 + clippy_lints/src/loops.rs | 4 + clippy_lints/src/map_unit_fn.rs | 1 + clippy_lints/src/methods/mod.rs | 2 + clippy_lints/src/misc_early.rs | 1 + clippy_lints/src/missing_const_for_fn.rs | 1 + clippy_lints/src/missing_doc.rs | 2 + clippy_lints/src/needless_continue.rs | 3 + clippy_lints/src/non_copy_const.rs | 1 + clippy_lints/src/non_expressive_names.rs | 3 + clippy_lints/src/precedence.rs | 2 + clippy_lints/src/ptr_offset_with_cast.rs | 1 + clippy_lints/src/regex.rs | 1 + clippy_lints/src/returns.rs | 1 + clippy_lints/src/types.rs | 5 + clippy_lints/src/unsafe_removed_from_name.rs | 1 + clippy_lints/src/utils/attrs.rs | 1 + clippy_lints/src/utils/author.rs | 3 + clippy_lints/src/utils/camel_case.rs | 2 + clippy_lints/src/utils/conf.rs | 2 + clippy_lints/src/utils/higher.rs | 1 + clippy_lints/src/utils/internal_lints.rs | 1 + clippy_lints/src/utils/mod.rs | 4 + clippy_lints/src/utils/sugg.rs | 1 + rustc_tools_util/src/lib.rs | 3 + src/lintlist/mod.rs | 23 +- tests/compile-test.rs | 4 + tests/ui/auxiliary/macro_rules.rs | 11 +- tests/ui/double_must_use.rs | 27 ++ tests/ui/double_must_use.stderr | 27 ++ tests/ui/functions.stderr | 8 +- tests/ui/methods.rs | 11 +- tests/ui/methods.stderr | 48 +-- tests/ui/must_use_candidates.fixed | 88 ++++ tests/ui/must_use_candidates.rs | 88 ++++ tests/ui/must_use_candidates.stderr | 40 ++ tests/ui/must_use_unit.fixed | 26 ++ tests/ui/must_use_unit.rs | 26 ++ tests/ui/must_use_unit.stderr | 28 ++ tests/ui/shadow.rs | 1 + tests/ui/shadow.stderr | 46 +-- 59 files changed, 924 insertions(+), 88 deletions(-) create mode 100644 tests/ui/double_must_use.rs create mode 100644 tests/ui/double_must_use.stderr create mode 100644 tests/ui/must_use_candidates.fixed create mode 100644 tests/ui/must_use_candidates.rs create mode 100644 tests/ui/must_use_candidates.stderr create mode 100644 tests/ui/must_use_unit.fixed create mode 100644 tests/ui/must_use_unit.rs create mode 100644 tests/ui/must_use_unit.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 860882311096..dd67bc3cdc18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -976,6 +976,7 @@ Released 2018-09-13 [`diverging_sub_expression`]: https://rust-lang.github.io/rust-clippy/master/index.html#diverging_sub_expression [`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown [`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons +[`double_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_must_use [`double_neg`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_neg [`double_parens`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_parens [`drop_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#drop_bounds @@ -1095,6 +1096,8 @@ Released 2018-09-13 [`modulo_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#modulo_one [`multiple_crate_versions`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions [`multiple_inherent_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_inherent_impl +[`must_use_candidate`]: https://rust-lang.github.io/rust-clippy/master/index.html#must_use_candidate +[`must_use_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#must_use_unit [`mut_from_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#mut_from_ref [`mut_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#mut_mut [`mut_range_bound`]: https://rust-lang.github.io/rust-clippy/master/index.html#mut_range_bound diff --git a/README.md b/README.md index 0011fb9cd303..a84ab0e8c252 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 321 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 324 lints included in this crate!](https://rust-lang.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: diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 7df7109c75f7..84b2814a7ce0 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -42,6 +42,7 @@ pub struct Lint { } impl Lint { + #[must_use] pub fn new(name: &str, group: &str, desc: &str, deprecation: Option<&str>, module: &str) -> Self { Self { name: name.to_lowercase(), @@ -58,6 +59,7 @@ impl Lint { } /// Returns the lints in a `HashMap`, grouped by the different lint groups + #[must_use] pub fn by_lint_group(lints: &[Self]) -> HashMap> { lints .iter() @@ -65,12 +67,14 @@ impl Lint { .into_group_map() } + #[must_use] pub fn is_internal(&self) -> bool { self.group.starts_with("internal") } } /// Generates the Vec items for `register_lint_group` calls in `clippy_lints/src/lib.rs`. +#[must_use] pub fn gen_lint_group_list(lints: Vec) -> Vec { lints .into_iter() @@ -86,6 +90,7 @@ pub fn gen_lint_group_list(lints: Vec) -> Vec { } /// Generates the `pub mod module_name` list in `clippy_lints/src/lib.rs`. +#[must_use] pub fn gen_modules_list(lints: Vec) -> Vec { lints .into_iter() @@ -103,6 +108,7 @@ pub fn gen_modules_list(lints: Vec) -> Vec { } /// Generates the list of lint links at the bottom of the README +#[must_use] pub fn gen_changelog_lint_list(lints: Vec) -> Vec { let mut lint_list_sorted: Vec = lints; lint_list_sorted.sort_by_key(|l| l.name.clone()); @@ -119,6 +125,7 @@ pub fn gen_changelog_lint_list(lints: Vec) -> Vec { } /// Generates the `register_removed` code in `./clippy_lints/src/lib.rs`. +#[must_use] pub fn gen_deprecated(lints: &[Lint]) -> Vec { lints .iter() diff --git a/clippy_dev/src/stderr_length_check.rs b/clippy_dev/src/stderr_length_check.rs index 3049c45ddc86..2d7d119f3ff1 100644 --- a/clippy_dev/src/stderr_length_check.rs +++ b/clippy_dev/src/stderr_length_check.rs @@ -42,6 +42,7 @@ fn stderr_files() -> impl Iterator { .filter(|f| f.path().extension() == Some(OsStr::new("stderr"))) } +#[must_use] fn count_linenumbers(filepath: &str) -> usize { if let Ok(mut file) = File::open(filepath) { let mut content = String::new(); diff --git a/clippy_lints/src/approx_const.rs b/clippy_lints/src/approx_const.rs index 7578b5fffe1a..04530542ef8d 100644 --- a/clippy_lints/src/approx_const.rs +++ b/clippy_lints/src/approx_const.rs @@ -93,6 +93,7 @@ fn check_known_consts(cx: &LateContext<'_, '_>, e: &Expr, s: symbol::Symbol, mod /// Returns `false` if the number of significant figures in `value` are /// less than `min_digits`; otherwise, returns true if `value` is equal /// to `constant`, rounded to the number of digits present in `value`. +#[must_use] fn is_approx_const(constant: f64, value: &str, min_digits: usize) -> bool { if value.len() <= min_digits { false diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs index a16c4575e703..3d12bb347aa9 100644 --- a/clippy_lints/src/assign_ops.rs +++ b/clippy_lints/src/assign_ops.rs @@ -229,6 +229,7 @@ fn lint_misrefactored_assign_op( ); } +#[must_use] fn is_commutative(op: hir::BinOpKind) -> bool { use rustc::hir::BinOpKind::*; match op { diff --git a/clippy_lints/src/bit_mask.rs b/clippy_lints/src/bit_mask.rs index e27b5269ef44..6d68c319f4c3 100644 --- a/clippy_lints/src/bit_mask.rs +++ b/clippy_lints/src/bit_mask.rs @@ -100,6 +100,7 @@ pub struct BitMask { } impl BitMask { + #[must_use] pub fn new(verbose_bit_mask_threshold: u64) -> Self { Self { verbose_bit_mask_threshold, @@ -150,6 +151,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BitMask { } } +#[must_use] fn invert_cmp(cmp: BinOpKind) -> BinOpKind { match cmp { BinOpKind::Eq => BinOpKind::Eq, diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs index d1d725678b7b..dfd497f909b7 100644 --- a/clippy_lints/src/checked_conversions.rs +++ b/clippy_lints/src/checked_conversions.rs @@ -160,6 +160,7 @@ impl<'a> Conversion<'a> { impl ConversionType { /// Creates a conversion type if the type is allowed & conversion is valid + #[must_use] fn try_new(from: &str, to: &str) -> Option { if UINTS.contains(&from) { Some(Self::FromUnsigned) diff --git a/clippy_lints/src/cognitive_complexity.rs b/clippy_lints/src/cognitive_complexity.rs index d869427467c3..370421190cb4 100644 --- a/clippy_lints/src/cognitive_complexity.rs +++ b/clippy_lints/src/cognitive_complexity.rs @@ -29,6 +29,7 @@ pub struct CognitiveComplexity { } impl CognitiveComplexity { + #[must_use] pub fn new(limit: u64) -> Self { Self { limit: LimitStack::new(limit), diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 0f95efe59f11..726a044f9ed9 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -191,6 +191,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown { /// need to keep track of /// the spans but this function is inspired from the later. #[allow(clippy::cast_possible_truncation)] +#[must_use] pub fn strip_doc_comment_decoration(comment: &str, span: Span) -> (String, Vec<(usize, Span)>) { // one-line comments lose their prefix const ONELINERS: &[&str] = &["///!", "///", "//!", "//"]; diff --git a/clippy_lints/src/enum_variants.rs b/clippy_lints/src/enum_variants.rs index e64eed383408..b64e97fba5ca 100644 --- a/clippy_lints/src/enum_variants.rs +++ b/clippy_lints/src/enum_variants.rs @@ -107,6 +107,7 @@ pub struct EnumVariantNames { } impl EnumVariantNames { + #[must_use] pub fn new(threshold: u64) -> Self { Self { modules: Vec::new(), @@ -123,6 +124,7 @@ impl_lint_pass!(EnumVariantNames => [ ]); /// Returns the number of chars that match from the start +#[must_use] fn partial_match(pre: &str, name: &str) -> usize { let mut name_iter = name.chars(); let _ = name_iter.next_back(); // make sure the name is never fully matched @@ -130,6 +132,7 @@ fn partial_match(pre: &str, name: &str) -> usize { } /// Returns the number of chars that match from the end +#[must_use] fn partial_rmatch(post: &str, name: &str) -> usize { let mut name_iter = name.chars(); let _ = name_iter.next(); // make sure the name is never fully matched @@ -211,6 +214,7 @@ fn check_variant( ); } +#[must_use] fn to_camel_case(item_name: &str) -> String { let mut s = String::new(); let mut up = true; diff --git a/clippy_lints/src/excessive_precision.rs b/clippy_lints/src/excessive_precision.rs index ae9681134ffa..763770c74efd 100644 --- a/clippy_lints/src/excessive_precision.rs +++ b/clippy_lints/src/excessive_precision.rs @@ -62,6 +62,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExcessivePrecision { impl ExcessivePrecision { // None if nothing to lint, Some(suggestion) if lint necessary + #[must_use] fn check(self, sym: Symbol, fty: FloatTy) -> Option { let max = max_digits(fty); let sym_str = sym.as_str(); @@ -97,6 +98,7 @@ impl ExcessivePrecision { /// Should we exclude the float because it has a `.0` or `.` suffix /// Ex `1_000_000_000.0` /// Ex `1_000_000_000.` +#[must_use] fn dot_zero_exclusion(s: &str) -> bool { s.split('.').nth(1).map_or(false, |after_dec| { let mut decpart = after_dec.chars().take_while(|c| *c != 'e' || *c != 'E'); @@ -109,6 +111,7 @@ fn dot_zero_exclusion(s: &str) -> bool { }) } +#[must_use] fn max_digits(fty: FloatTy) -> u32 { match fty { FloatTy::F32 => f32::DIGITS, @@ -117,6 +120,7 @@ fn max_digits(fty: FloatTy) -> u32 { } /// Counts the digits excluding leading zeros +#[must_use] fn count_digits(s: &str) -> usize { // Note that s does not contain the f32/64 suffix, and underscores have been stripped s.chars() @@ -138,6 +142,7 @@ enum FloatFormat { Normal, } impl FloatFormat { + #[must_use] fn new(s: &str) -> Self { s.chars() .find_map(|x| match x { diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs index 821e79e8c771..62606257498e 100644 --- a/clippy_lints/src/formatting.rs +++ b/clippy_lints/src/formatting.rs @@ -235,6 +235,7 @@ fn check_else(cx: &EarlyContext<'_>, expr: &Expr) { } } +#[must_use] fn has_unary_equivalent(bin_op: BinOpKind) -> bool { // &, *, - bin_op == BinOpKind::And || bin_op == BinOpKind::Mul || bin_op == BinOpKind::Sub diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index 7b6c8c7cea6d..6052f9361091 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -1,16 +1,17 @@ -use std::convert::TryFrom; - -use crate::utils::{iter_input_pats, qpath_res, snippet, snippet_opt, span_lint, type_is_unsafe_function}; +use crate::utils::{ + iter_input_pats, match_def_path, qpath_res, return_ty, snippet, snippet_opt, span_help_and_lint, span_lint, + span_lint_and_then, type_is_unsafe_function, +}; use matches::matches; -use rustc::hir; -use rustc::hir::def::Res; -use rustc::hir::intravisit; +use rustc::hir::{self, def::Res, def_id::DefId, intravisit}; use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass}; -use rustc::ty; +use rustc::ty::{self, Ty}; use rustc::{declare_tool_lint, impl_lint_pass}; use rustc_data_structures::fx::FxHashSet; +use rustc_errors::Applicability; use rustc_target::spec::abi::Abi; -use syntax::source_map::{BytePos, Span}; +use syntax::ast::Attribute; +use syntax::source_map::Span; declare_clippy_lint! { /// **What it does:** Checks for functions with too many parameters. @@ -84,6 +85,80 @@ declare_clippy_lint! { "public functions dereferencing raw pointer arguments but not marked `unsafe`" } +declare_clippy_lint! { + /// **What it does:** Checks for a [`#[must_use]`] attribute on + /// unit-returning functions and methods. + /// + /// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute + /// + /// **Why is this bad?** Unit values are useless. The attribute is likely + /// a remnant of a refactoring that removed the return type. + /// + /// **Known problems:** None. + /// + /// **Examples:** + /// ```rust + /// #[must_use] + /// fn useless() { } + /// ``` + pub MUST_USE_UNIT, + style, + "`#[must_use]` attribute on a unit-returning function / method" +} + +declare_clippy_lint! { + /// **What it does:** Checks for a [`#[must_use]`] attribute without + /// further information on functions and methods that return a type already + /// marked as `#[must_use]`. + /// + /// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute + /// + /// **Why is this bad?** The attribute isn't needed. Not using the result + /// will already be reported. Alternatively, one can add some text to the + /// attribute to improve the lint message. + /// + /// **Known problems:** None. + /// + /// **Examples:** + /// ```rust + /// #[must_use] + /// fn double_must_use() -> Result<(), ()> { + /// unimplemented!(); + /// } + /// ``` + pub DOUBLE_MUST_USE, + style, + "`#[must_use]` attribute on a `#[must_use]`-returning function / method" +} + +declare_clippy_lint! { + /// **What it does:** Checks for public functions that have no + /// [`#[must_use]`] attribute, but return something not already marked + /// must-use, have no mutable arg and mutate no statics. + /// + /// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute + /// + /// **Why is this bad?** Not bad at all, this lint just shows places where + /// you could add the attribute. + /// + /// **Known problems:** The lint only checks the arguments for mutable + /// types without looking if they are actually changed. On the other hand, + /// it also ignores a broad range of potentially interesting side effects, + /// because we cannot decide whether the programmer intends the function to + /// be called for the side effect or the result. Expect many false + /// positives. At least we don't lint if the result type is unit or already + /// `#[must_use]`. + /// + /// **Examples:** + /// ```rust + /// // this could be annotated with `#[must_use]`. + /// fn id(t: T) -> T { t } + /// ``` + pub MUST_USE_CANDIDATE, + pedantic, + "function or method that could take a `#[must_use]` attribute" +} + #[derive(Copy, Clone)] pub struct Functions { threshold: u64, @@ -96,7 +171,14 @@ impl Functions { } } -impl_lint_pass!(Functions => [TOO_MANY_ARGUMENTS, TOO_MANY_LINES, NOT_UNSAFE_PTR_ARG_DEREF]); +impl_lint_pass!(Functions => [ + TOO_MANY_ARGUMENTS, + TOO_MANY_LINES, + NOT_UNSAFE_PTR_ARG_DEREF, + MUST_USE_UNIT, + DOUBLE_MUST_USE, + MUST_USE_CANDIDATE, +]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions { fn check_fn( @@ -134,7 +216,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions { _, ) | hir::intravisit::FnKind::ItemFn(_, _, hir::FnHeader { abi: Abi::Rust, .. }, _, _) => { - self.check_arg_number(cx, decl, span) + self.check_arg_number(cx, decl, span.with_hi(decl.output.span().hi())) }, _ => {}, } @@ -144,42 +226,88 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions { self.check_line_number(cx, span, body); } + fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item) { + let attr = must_use_attr(&item.attrs); + if let hir::ItemKind::Fn(ref decl, ref _header, ref _generics, ref body_id) = item.kind { + if let Some(attr) = attr { + let fn_header_span = item.span.with_hi(decl.output.span().hi()); + check_needless_must_use(cx, decl, item.hir_id, item.span, fn_header_span, attr); + return; + } + if cx.access_levels.is_exported(item.hir_id) { + check_must_use_candidate( + cx, + decl, + cx.tcx.hir().body(*body_id), + item.span, + item.hir_id, + item.span.with_hi(decl.output.span().hi()), + "this function could have a `#[must_use]` attribute", + ); + } + } + } + + fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::ImplItem) { + if let hir::ImplItemKind::Method(ref sig, ref body_id) = item.kind { + let attr = must_use_attr(&item.attrs); + if let Some(attr) = attr { + let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); + check_needless_must_use(cx, &sig.decl, item.hir_id, item.span, fn_header_span, attr); + } else if cx.access_levels.is_exported(item.hir_id) { + check_must_use_candidate( + cx, + &sig.decl, + cx.tcx.hir().body(*body_id), + item.span, + item.hir_id, + item.span.with_hi(sig.decl.output.span().hi()), + "this method could have a `#[must_use]` attribute", + ); + } + } + } + fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem) { if let hir::TraitItemKind::Method(ref sig, ref eid) = item.kind { // don't lint extern functions decls, it's not their fault if sig.header.abi == Abi::Rust { - self.check_arg_number(cx, &sig.decl, item.span); + self.check_arg_number(cx, &sig.decl, item.span.with_hi(sig.decl.output.span().hi())); } + let attr = must_use_attr(&item.attrs); + if let Some(attr) = attr { + let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); + check_needless_must_use(cx, &sig.decl, item.hir_id, item.span, fn_header_span, attr); + } if let hir::TraitMethod::Provided(eid) = *eid { let body = cx.tcx.hir().body(eid); self.check_raw_ptr(cx, sig.header.unsafety, &sig.decl, body, item.hir_id); + + if attr.is_none() && cx.access_levels.is_exported(item.hir_id) { + check_must_use_candidate( + cx, + &sig.decl, + body, + item.span, + item.hir_id, + item.span.with_hi(sig.decl.output.span().hi()), + "this method could have a `#[must_use]` attribute", + ); + } } } } } impl<'a, 'tcx> Functions { - fn check_arg_number(self, cx: &LateContext<'_, '_>, decl: &hir::FnDecl, span: Span) { - // Remove the function body from the span. We can't use `SourceMap::def_span` because the - // argument list might span multiple lines. - let span = if let Some(snippet) = snippet_opt(cx, span) { - let snippet = snippet.split('{').nth(0).unwrap_or("").trim_end(); - if snippet.is_empty() { - span - } else { - span.with_hi(BytePos(span.lo().0 + u32::try_from(snippet.len()).unwrap())) - } - } else { - span - }; - + fn check_arg_number(self, cx: &LateContext<'_, '_>, decl: &hir::FnDecl, fn_span: Span) { let args = decl.inputs.len() as u64; if args > self.threshold { span_lint( cx, TOO_MANY_ARGUMENTS, - span, + fn_span, &format!("this function has too many arguments ({}/{})", args, self.threshold), ); } @@ -268,6 +396,164 @@ impl<'a, 'tcx> Functions { } } +fn check_needless_must_use( + cx: &LateContext<'_, '_>, + decl: &hir::FnDecl, + item_id: hir::HirId, + item_span: Span, + fn_header_span: Span, + attr: &Attribute, +) { + if in_external_macro(cx.sess(), item_span) { + return; + } + if returns_unit(decl) { + span_lint_and_then( + cx, + MUST_USE_UNIT, + fn_header_span, + "this unit-returning function has a `#[must_use]` attribute", + |db| { + db.span_suggestion( + attr.span, + "remove the attribute", + "".into(), + Applicability::MachineApplicable, + ); + }, + ); + } else if !attr.is_value_str() && is_must_use_ty(cx, return_ty(cx, item_id)) { + span_help_and_lint( + cx, + DOUBLE_MUST_USE, + fn_header_span, + "this function has an empty `#[must_use]` attribute, but returns a type already marked as `#[must_use]`", + "either add some descriptive text or remove the attribute", + ); + } +} + +fn check_must_use_candidate<'a, 'tcx>( + cx: &LateContext<'a, 'tcx>, + decl: &'tcx hir::FnDecl, + body: &'tcx hir::Body, + item_span: Span, + item_id: hir::HirId, + fn_span: Span, + msg: &str, +) { + if has_mutable_arg(cx, body) + || mutates_static(cx, body) + || in_external_macro(cx.sess(), item_span) + || returns_unit(decl) + || is_must_use_ty(cx, return_ty(cx, item_id)) + { + return; + } + span_lint_and_then(cx, MUST_USE_CANDIDATE, fn_span, msg, |db| { + if let Some(snippet) = snippet_opt(cx, fn_span) { + db.span_suggestion( + fn_span, + "add the attribute", + format!("#[must_use] {}", snippet), + Applicability::MachineApplicable, + ); + } + }); +} + +fn must_use_attr(attrs: &[Attribute]) -> Option<&Attribute> { + attrs + .iter() + .find(|attr| attr.ident().map_or(false, |ident| "must_use" == &ident.as_str())) +} + +fn returns_unit(decl: &hir::FnDecl) -> bool { + match decl.output { + hir::FunctionRetTy::DefaultReturn(_) => true, + hir::FunctionRetTy::Return(ref ty) => match ty.kind { + hir::TyKind::Tup(ref tys) => tys.is_empty(), + hir::TyKind::Never => true, + _ => false, + }, + } +} + +fn is_must_use_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool { + use ty::TyKind::*; + match ty.kind { + Adt(ref adt, _) => must_use_attr(&cx.tcx.get_attrs(adt.did)).is_some(), + Foreign(ref did) => must_use_attr(&cx.tcx.get_attrs(*did)).is_some(), + Slice(ref ty) | Array(ref ty, _) | RawPtr(ty::TypeAndMut { ref ty, .. }) | Ref(_, ref ty, _) => { + // for the Array case we don't need to care for the len == 0 case + // because we don't want to lint functions returning empty arrays + is_must_use_ty(cx, *ty) + }, + Tuple(ref substs) => substs.types().any(|ty| is_must_use_ty(cx, ty)), + Opaque(ref def_id, _) => { + for (predicate, _) in &cx.tcx.predicates_of(*def_id).predicates { + if let ty::Predicate::Trait(ref poly_trait_predicate) = predicate { + if must_use_attr(&cx.tcx.get_attrs(poly_trait_predicate.skip_binder().trait_ref.def_id)).is_some() { + return true; + } + } + } + false + }, + Dynamic(binder, _) => { + for predicate in binder.skip_binder().iter() { + if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate { + if must_use_attr(&cx.tcx.get_attrs(trait_ref.def_id)).is_some() { + return true; + } + } + } + false + }, + _ => false, + } +} + +fn has_mutable_arg(cx: &LateContext<'_, '_>, body: &hir::Body) -> bool { + let mut tys = FxHashSet::default(); + body.params.iter().any(|param| is_mutable_pat(cx, ¶m.pat, &mut tys)) +} + +fn is_mutable_pat(cx: &LateContext<'_, '_>, pat: &hir::Pat, tys: &mut FxHashSet) -> bool { + if let hir::PatKind::Wild = pat.kind { + return false; // ignore `_` patterns + } + let def_id = pat.hir_id.owner_def_id(); + if cx.tcx.has_typeck_tables(def_id) { + is_mutable_ty(cx, &cx.tcx.typeck_tables_of(def_id).pat_ty(pat), pat.span, tys) + } else { + false + } +} + +static KNOWN_WRAPPER_TYS: &[&[&str]] = &[&["alloc", "rc", "Rc"], &["std", "sync", "Arc"]]; + +fn is_mutable_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>, span: Span, tys: &mut FxHashSet) -> bool { + use ty::TyKind::*; + match ty.kind { + // primitive types are never mutable + Bool | Char | Int(_) | Uint(_) | Float(_) | Str => false, + Adt(ref adt, ref substs) => { + tys.insert(adt.did) && !ty.is_freeze(cx.tcx, cx.param_env, span) + || KNOWN_WRAPPER_TYS.iter().any(|path| match_def_path(cx, adt.did, path)) + && substs.types().any(|ty| is_mutable_ty(cx, ty, span, tys)) + }, + Tuple(ref substs) => substs.types().any(|ty| is_mutable_ty(cx, ty, span, tys)), + Array(ty, _) | Slice(ty) => is_mutable_ty(cx, ty, span, tys), + RawPtr(ty::TypeAndMut { ty, mutbl }) | Ref(_, ty, mutbl) => { + mutbl == hir::Mutability::MutMutable || is_mutable_ty(cx, ty, span, tys) + }, + // calling something constitutes a side effect, so return true on all callables + // also never calls need not be used, so return true for them, too + _ => true, + } +} + fn raw_ptr_arg(arg: &hir::Param, ty: &hir::Ty) -> Option { if let (&hir::PatKind::Binding(_, id, _, _), &hir::TyKind::Ptr(_)) = (&arg.pat.kind, &ty.kind) { Some(id) @@ -282,7 +568,7 @@ struct DerefVisitor<'a, 'tcx> { tables: &'a ty::TypeckTables<'tcx>, } -impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> { +impl<'a, 'tcx> intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx hir::Expr) { match expr.kind { hir::ExprKind::Call(ref f, ref args) => { @@ -308,8 +594,9 @@ impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> { _ => (), } - hir::intravisit::walk_expr(self, expr); + intravisit::walk_expr(self, expr); } + fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'tcx> { intravisit::NestedVisitorMap::None } @@ -331,3 +618,72 @@ impl<'a, 'tcx> DerefVisitor<'a, 'tcx> { } } } + +struct StaticMutVisitor<'a, 'tcx> { + cx: &'a LateContext<'a, 'tcx>, + mutates_static: bool, +} + +impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> { + fn visit_expr(&mut self, expr: &'tcx hir::Expr) { + use hir::ExprKind::*; + + if self.mutates_static { + return; + } + match expr.kind { + Call(_, ref args) | MethodCall(_, _, ref args) => { + let mut tys = FxHashSet::default(); + for arg in args { + let def_id = arg.hir_id.owner_def_id(); + if self.cx.tcx.has_typeck_tables(def_id) + && is_mutable_ty( + self.cx, + self.cx.tcx.typeck_tables_of(def_id).expr_ty(arg), + arg.span, + &mut tys, + ) + && is_mutated_static(self.cx, arg) + { + self.mutates_static = true; + return; + } + tys.clear(); + } + }, + Assign(ref target, _) | AssignOp(_, ref target, _) | AddrOf(hir::Mutability::MutMutable, ref target) => { + self.mutates_static |= is_mutated_static(self.cx, target) + }, + _ => {}, + } + } + + fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'tcx> { + intravisit::NestedVisitorMap::None + } +} + +fn is_mutated_static(cx: &LateContext<'_, '_>, e: &hir::Expr) -> bool { + use hir::ExprKind::*; + + match e.kind { + Path(ref qpath) => { + if let Res::Local(_) = qpath_res(cx, qpath, e.hir_id) { + false + } else { + true + } + }, + Field(ref inner, _) | Index(ref inner, _) => is_mutated_static(cx, inner), + _ => false, + } +} + +fn mutates_static<'a, 'tcx>(cx: &'a LateContext<'a, 'tcx>, body: &'tcx hir::Body) -> bool { + let mut v = StaticMutVisitor { + cx, + mutates_static: false, + }; + intravisit::walk_expr(&mut v, &body.value); + v.mutates_static +} diff --git a/clippy_lints/src/infinite_iter.rs b/clippy_lints/src/infinite_iter.rs index adda6ae0c113..e6af8e8c7fc0 100644 --- a/clippy_lints/src/infinite_iter.rs +++ b/clippy_lints/src/infinite_iter.rs @@ -67,6 +67,7 @@ enum Finiteness { use self::Finiteness::{Finite, Infinite, MaybeInfinite}; impl Finiteness { + #[must_use] fn and(self, b: Self) -> Self { match (self, b) { (Finite, _) | (_, Finite) => Finite, @@ -75,6 +76,7 @@ impl Finiteness { } } + #[must_use] fn or(self, b: Self) -> Self { match (self, b) { (Infinite, _) | (_, Infinite) => Infinite, @@ -85,6 +87,7 @@ impl Finiteness { } impl From for Finiteness { + #[must_use] fn from(b: bool) -> Self { if b { Infinite diff --git a/clippy_lints/src/large_enum_variant.rs b/clippy_lints/src/large_enum_variant.rs index d5c1318f6776..d612a4326fe4 100644 --- a/clippy_lints/src/large_enum_variant.rs +++ b/clippy_lints/src/large_enum_variant.rs @@ -35,6 +35,7 @@ pub struct LargeEnumVariant { } impl LargeEnumVariant { + #[must_use] pub fn new(maximum_size_difference_allowed: u64) -> Self { Self { maximum_size_difference_allowed, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index ec12e06b1aac..5d428221e699 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -6,7 +6,7 @@ #![feature(rustc_private)] #![feature(slice_patterns)] #![feature(stmt_expr_attributes)] -#![allow(clippy::missing_docs_in_private_items)] +#![allow(clippy::missing_docs_in_private_items, clippy::must_use_candidate)] #![recursion_limit = "512"] #![warn(rust_2018_idioms, trivial_casts, trivial_numeric_casts)] #![deny(rustc::internal)] @@ -648,6 +648,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con enum_variants::MODULE_NAME_REPETITIONS, enum_variants::PUB_ENUM_VARIANT_NAMES, eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS, + functions::MUST_USE_CANDIDATE, functions::TOO_MANY_LINES, if_not_else::IF_NOT_ELSE, infinite_iter::MAYBE_INFINITE_ITER, @@ -744,6 +745,8 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING, formatting::SUSPICIOUS_ELSE_FORMATTING, formatting::SUSPICIOUS_UNARY_OP_FORMATTING, + functions::DOUBLE_MUST_USE, + functions::MUST_USE_UNIT, functions::NOT_UNSAFE_PTR_ARG_DEREF, functions::TOO_MANY_ARGUMENTS, get_last_with_len::GET_LAST_WITH_LEN, @@ -955,6 +958,8 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING, formatting::SUSPICIOUS_ELSE_FORMATTING, formatting::SUSPICIOUS_UNARY_OP_FORMATTING, + functions::DOUBLE_MUST_USE, + functions::MUST_USE_UNIT, infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH, inherent_to_string::INHERENT_TO_STRING, len_zero::LEN_WITHOUT_IS_EMPTY, diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index 88d393c84ee6..be1e65fc17c4 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -270,6 +270,7 @@ fn lts_from_bounds<'a, T: Iterator>(mut vec: Vec, bo } /// Number of unique lifetimes in the given vector. +#[must_use] fn unique_lifetimes(lts: &[RefLt]) -> usize { lts.iter().collect::>().len() } diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index 5cdbfe1099c6..4b2bb69fa794 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -113,6 +113,7 @@ pub(super) enum Radix { impl Radix { /// Returns a reasonable digit group size for this radix. + #[must_use] crate fn suggest_grouping(&self) -> usize { match *self { Self::Binary | Self::Hexadecimal => 4, @@ -136,6 +137,7 @@ pub(super) struct DigitInfo<'a> { } impl<'a> DigitInfo<'a> { + #[must_use] crate fn new(lit: &'a str, float: bool) -> Self { // Determine delimiter for radix prefix, if present, and radix. let radix = if lit.starts_with("0x") { @@ -422,6 +424,7 @@ impl LiteralDigitGrouping { /// Given the sizes of the digit groups of both integral and fractional /// parts, and the length /// of both parts, determine if the digits have been grouped consistently. + #[must_use] fn parts_consistent(int_group_size: usize, frac_group_size: usize, int_size: usize, frac_size: usize) -> bool { match (int_group_size, frac_group_size) { // No groups on either side of decimal point - trivially consistent. @@ -499,6 +502,7 @@ impl EarlyLintPass for DecimalLiteralRepresentation { } impl DecimalLiteralRepresentation { + #[must_use] pub fn new(threshold: u64) -> Self { Self { threshold } } @@ -573,22 +577,27 @@ impl DecimalLiteralRepresentation { } } +#[must_use] fn is_mistyped_suffix(suffix: &str) -> bool { ["_8", "_16", "_32", "_64"].contains(&suffix) } +#[must_use] fn is_possible_suffix_index(lit: &str, idx: usize, len: usize) -> bool { ((len > 3 && idx == len - 3) || (len > 2 && idx == len - 2)) && is_mistyped_suffix(lit.split_at(idx).1) } +#[must_use] fn is_mistyped_float_suffix(suffix: &str) -> bool { ["_32", "_64"].contains(&suffix) } +#[must_use] fn is_possible_float_suffix_index(lit: &str, idx: usize, len: usize) -> bool { (len > 3 && idx == len - 3) && is_mistyped_float_suffix(lit.split_at(idx).1) } +#[must_use] fn has_possible_float_suffix(lit: &str) -> bool { lit.ends_with("_32") || lit.ends_with("_64") } diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index ea496a0294ab..22821e02fe29 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -610,6 +610,7 @@ enum NeverLoopResult { Otherwise, } +#[must_use] fn absorb_break(arg: &NeverLoopResult) -> NeverLoopResult { match *arg { NeverLoopResult::AlwaysBreak | NeverLoopResult::Otherwise => NeverLoopResult::Otherwise, @@ -618,6 +619,7 @@ fn absorb_break(arg: &NeverLoopResult) -> NeverLoopResult { } // Combine two results for parts that are called in order. +#[must_use] fn combine_seq(first: NeverLoopResult, second: NeverLoopResult) -> NeverLoopResult { match first { NeverLoopResult::AlwaysBreak | NeverLoopResult::MayContinueMainLoop => first, @@ -626,6 +628,7 @@ fn combine_seq(first: NeverLoopResult, second: NeverLoopResult) -> NeverLoopResu } // Combine two results where both parts are called but not necessarily in order. +#[must_use] fn combine_both(left: NeverLoopResult, right: NeverLoopResult) -> NeverLoopResult { match (left, right) { (NeverLoopResult::MayContinueMainLoop, _) | (_, NeverLoopResult::MayContinueMainLoop) => { @@ -637,6 +640,7 @@ fn combine_both(left: NeverLoopResult, right: NeverLoopResult) -> NeverLoopResul } // Combine two results where only one of the part may have been executed. +#[must_use] fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult { match (b1, b2) { (NeverLoopResult::AlwaysBreak, NeverLoopResult::AlwaysBreak) => NeverLoopResult::AlwaysBreak, diff --git a/clippy_lints/src/map_unit_fn.rs b/clippy_lints/src/map_unit_fn.rs index 70f324a5081e..8b2bcce471c0 100644 --- a/clippy_lints/src/map_unit_fn.rs +++ b/clippy_lints/src/map_unit_fn.rs @@ -195,6 +195,7 @@ fn let_binding_name(cx: &LateContext<'_, '_>, var_arg: &hir::Expr) -> String { } } +#[must_use] fn suggestion_msg(function_type: &str, map_type: &str) -> String { format!( "called `map(f)` on an {0} value where `f` is a unit {1}", diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 198f97e1f568..848e3bcb881f 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2898,6 +2898,7 @@ impl SelfKind { } } + #[must_use] fn description(self) -> &'static str { match self { Self::Value => "self by value", @@ -2909,6 +2910,7 @@ impl SelfKind { } impl Convention { + #[must_use] fn check(&self, other: &str) -> bool { match *self { Self::Eq(this) => this == other, diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index 43a5e5c1b4b1..c06eec950281 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -253,6 +253,7 @@ struct ReturnVisitor { } impl ReturnVisitor { + #[must_use] fn new() -> Self { Self { found_return: false } } diff --git a/clippy_lints/src/missing_const_for_fn.rs b/clippy_lints/src/missing_const_for_fn.rs index 2f20aa9c683c..d97e3ed88069 100644 --- a/clippy_lints/src/missing_const_for_fn.rs +++ b/clippy_lints/src/missing_const_for_fn.rs @@ -128,6 +128,7 @@ fn method_accepts_dropable(cx: &LateContext<'_, '_>, param_tys: &HirVec } // We don't have to lint on something that's already `const` +#[must_use] fn already_const(header: hir::FnHeader) -> bool { header.constness == Constness::Const } diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs index f09a864240ba..949bd4bce279 100644 --- a/clippy_lints/src/missing_doc.rs +++ b/clippy_lints/src/missing_doc.rs @@ -37,12 +37,14 @@ pub struct MissingDoc { } impl ::std::default::Default for MissingDoc { + #[must_use] fn default() -> Self { Self::new() } } impl MissingDoc { + #[must_use] pub fn new() -> Self { Self { doc_hidden_stack: vec![false], diff --git a/clippy_lints/src/needless_continue.rs b/clippy_lints/src/needless_continue.rs index bd9d1583493c..8fa30f6827ca 100644 --- a/clippy_lints/src/needless_continue.rs +++ b/clippy_lints/src/needless_continue.rs @@ -405,6 +405,7 @@ fn check_and_warn<'a>(ctx: &EarlyContext<'_>, expr: &'a ast::Expr) { /// /// NOTE: when there is no closing brace in `s`, `s` is _not_ preserved, i.e., /// an empty string will be returned in that case. +#[must_use] pub fn erode_from_back(s: &str) -> String { let mut ret = String::from(s); while ret.pop().map_or(false, |c| c != '}') {} @@ -435,6 +436,7 @@ pub fn erode_from_back(s: &str) -> String { /// inside_a_block(); /// } /// ``` +#[must_use] pub fn erode_from_front(s: &str) -> String { s.chars() .skip_while(|c| c.is_whitespace()) @@ -447,6 +449,7 @@ pub fn erode_from_front(s: &str) -> String { /// tries to get the contents of the block. If there is no closing brace /// present, /// an empty string is returned. +#[must_use] pub fn erode_block(s: &str) -> String { erode_from_back(&erode_from_front(s)) } diff --git a/clippy_lints/src/non_copy_const.rs b/clippy_lints/src/non_copy_const.rs index 33f1dde98728..9cddd812b523 100644 --- a/clippy_lints/src/non_copy_const.rs +++ b/clippy_lints/src/non_copy_const.rs @@ -92,6 +92,7 @@ enum Source { } impl Source { + #[must_use] fn lint(&self) -> (&'static Lint, &'static str, Span) { match self { Self::Item { item } | Self::Assoc { item, .. } => ( diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs index 25e8efe3cad6..08a78b784ba5 100644 --- a/clippy_lints/src/non_expressive_names.rs +++ b/clippy_lints/src/non_expressive_names.rs @@ -148,6 +148,7 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for SimilarNamesNameVisitor<'a, 'tcx, 'b> { } } +#[must_use] fn get_whitelist(interned_name: &str) -> Option<&'static [&'static str]> { for &allow in WHITELIST { if whitelisted(interned_name, allow) { @@ -157,6 +158,7 @@ fn get_whitelist(interned_name: &str) -> Option<&'static [&'static str]> { None } +#[must_use] fn whitelisted(interned_name: &str, list: &[&str]) -> bool { list.iter() .any(|&name| interned_name.starts_with(name) || interned_name.ends_with(name)) @@ -383,6 +385,7 @@ fn do_check(lint: &mut NonExpressiveNames, cx: &EarlyContext<'_>, attrs: &[Attri } /// Precondition: `a_name.chars().count() < b_name.chars().count()`. +#[must_use] fn levenstein_not_1(a_name: &str, b_name: &str) -> bool { debug_assert!(a_name.chars().count() < b_name.chars().count()); let mut a_chars = a_name.chars(); diff --git a/clippy_lints/src/precedence.rs b/clippy_lints/src/precedence.rs index b00062cd55a1..a2d054c1de42 100644 --- a/clippy_lints/src/precedence.rs +++ b/clippy_lints/src/precedence.rs @@ -121,6 +121,7 @@ fn is_arith_expr(expr: &Expr) -> bool { } } +#[must_use] fn is_bit_op(op: BinOpKind) -> bool { use syntax::ast::BinOpKind::*; match op { @@ -129,6 +130,7 @@ fn is_bit_op(op: BinOpKind) -> bool { } } +#[must_use] fn is_arith_op(op: BinOpKind) -> bool { use syntax::ast::BinOpKind::*; match op { diff --git a/clippy_lints/src/ptr_offset_with_cast.rs b/clippy_lints/src/ptr_offset_with_cast.rs index d2e48e1d7984..5c5091581280 100644 --- a/clippy_lints/src/ptr_offset_with_cast.rs +++ b/clippy_lints/src/ptr_offset_with_cast.rs @@ -131,6 +131,7 @@ enum Method { } impl Method { + #[must_use] fn suggestion(self) -> &'static str { match self { Self::Offset => "add", diff --git a/clippy_lints/src/regex.rs b/clippy_lints/src/regex.rs index 22c4b107f006..54220f1107b7 100644 --- a/clippy_lints/src/regex.rs +++ b/clippy_lints/src/regex.rs @@ -129,6 +129,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Regex { } #[allow(clippy::cast_possible_truncation)] // truncation very unlikely here +#[must_use] fn str_span(base: Span, c: regex_syntax::ast::Span, offset: u16) -> Span { let offset = u32::from(offset); let end = base.lo() + BytePos(u32::try_from(c.end.offset).expect("offset too large") + offset); diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 47fdc226e992..a53235012077 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -318,6 +318,7 @@ fn attr_is_cfg(attr: &ast::Attribute) -> bool { } // get the def site +#[must_use] fn get_def(span: Span) -> Option { if span.from_expansion() { Some(span.ctxt().outer_expn_data().def_site) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 7230c0c08f2e..be8165d51f50 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -1372,6 +1372,7 @@ pub struct TypeComplexity { } impl TypeComplexity { + #[must_use] pub fn new(threshold: u64) -> Self { Self { threshold } } @@ -1780,6 +1781,7 @@ enum FullInt { impl FullInt { #[allow(clippy::cast_sign_loss)] + #[must_use] fn cmp_s_u(s: i128, u: u128) -> Ordering { if s < 0 { Ordering::Less @@ -1792,12 +1794,14 @@ impl FullInt { } impl PartialEq for FullInt { + #[must_use] fn eq(&self, other: &Self) -> bool { self.partial_cmp(other).expect("partial_cmp only returns Some(_)") == Ordering::Equal } } impl PartialOrd for FullInt { + #[must_use] fn partial_cmp(&self, other: &Self) -> Option { Some(match (self, other) { (&Self::S(s), &Self::S(o)) => s.cmp(&o), @@ -1808,6 +1812,7 @@ impl PartialOrd for FullInt { } } impl Ord for FullInt { + #[must_use] fn cmp(&self, other: &Self) -> Ordering { self.partial_cmp(other) .expect("partial_cmp for FullInt can never return None") diff --git a/clippy_lints/src/unsafe_removed_from_name.rs b/clippy_lints/src/unsafe_removed_from_name.rs index 29c8015edcff..528a1915be84 100644 --- a/clippy_lints/src/unsafe_removed_from_name.rs +++ b/clippy_lints/src/unsafe_removed_from_name.rs @@ -72,6 +72,7 @@ fn unsafe_to_safe_check(old_name: Ident, new_name: Ident, cx: &EarlyContext<'_>, } } +#[must_use] fn contains_unsafe(name: &LocalInternedString) -> bool { name.contains("Unsafe") || name.contains("unsafe") } diff --git a/clippy_lints/src/utils/attrs.rs b/clippy_lints/src/utils/attrs.rs index bec8d53714e2..11340f69aa21 100644 --- a/clippy_lints/src/utils/attrs.rs +++ b/clippy_lints/src/utils/attrs.rs @@ -34,6 +34,7 @@ impl Drop for LimitStack { } impl LimitStack { + #[must_use] pub fn new(limit: u64) -> Self { Self { stack: vec![limit] } } diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index f36570904c0d..a424c09ef408 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -146,6 +146,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Author { } impl PrintVisitor { + #[must_use] fn new(s: &'static str) -> Self { Self { ids: FxHashMap::default(), @@ -683,6 +684,7 @@ fn has_attr(sess: &Session, attrs: &[Attribute]) -> bool { get_attr(sess, attrs, "author").count() > 0 } +#[must_use] fn desugaring_name(des: hir::MatchSource) -> String { match des { hir::MatchSource::ForLoopDesugar => "MatchSource::ForLoopDesugar".to_string(), @@ -702,6 +704,7 @@ fn desugaring_name(des: hir::MatchSource) -> String { } } +#[must_use] fn loop_desugaring_name(des: hir::LoopSource) -> &'static str { match des { hir::LoopSource::ForLoop => "LoopSource::ForLoop", diff --git a/clippy_lints/src/utils/camel_case.rs b/clippy_lints/src/utils/camel_case.rs index 5b124dd96bf2..4192a26d3c80 100644 --- a/clippy_lints/src/utils/camel_case.rs +++ b/clippy_lints/src/utils/camel_case.rs @@ -1,4 +1,5 @@ /// Returns the index of the character after the first camel-case component of `s`. +#[must_use] pub fn until(s: &str) -> usize { let mut iter = s.char_indices(); if let Some((_, first)) = iter.next() { @@ -32,6 +33,7 @@ pub fn until(s: &str) -> usize { } /// Returns index of the last camel-case component of `s`. +#[must_use] pub fn from(s: &str) -> usize { let mut iter = s.char_indices().rev(); if let Some((_, first)) = iter.next() { diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 4595819f5578..734b689ab1a6 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -90,6 +90,7 @@ macro_rules! define_Conf { } } + #[must_use] fn $rust_name() -> define_Conf!(TY $($ty)+) { define_Conf!(DEFAULT $($ty)+, $default) } @@ -153,6 +154,7 @@ define_Conf! { } impl Default for Conf { + #[must_use] fn default() -> Self { toml::from_str("").expect("we never error on empty config files") } diff --git a/clippy_lints/src/utils/higher.rs b/clippy_lints/src/utils/higher.rs index de974eb3d274..63e9a27c5459 100644 --- a/clippy_lints/src/utils/higher.rs +++ b/clippy_lints/src/utils/higher.rs @@ -10,6 +10,7 @@ use rustc::{hir, ty}; use syntax::ast; /// Converts a hir binary operator to the corresponding `ast` type. +#[must_use] pub fn binop(op: hir::BinOpKind) -> ast::BinOpKind { match op { hir::BinOpKind::Eq => ast::BinOpKind::Eq, diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 1ec89cb93f17..96b00eb154ae 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -239,6 +239,7 @@ pub struct CompilerLintFunctions { } impl CompilerLintFunctions { + #[must_use] pub fn new() -> Self { let mut map = FxHashMap::default(); map.insert("span_lint", "utils::span_lint"); diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 88553bf1d764..a5e2f3dc4b43 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -52,6 +52,7 @@ use crate::reexport::*; /// Returns `true` if the two spans come from differing expansions (i.e., one is /// from a macro and one isn't). +#[must_use] pub fn differing_macro_contexts(lhs: Span, rhs: Span) -> bool { rhs.ctxt() != lhs.ctxt() } @@ -98,6 +99,7 @@ pub fn in_constant(cx: &LateContext<'_, '_>, id: HirId) -> bool { } /// Returns `true` if this `span` was expanded by any macro. +#[must_use] pub fn in_macro(span: Span) -> bool { if span.from_expansion() { if let ExpnKind::Desugaring(..) = span.ctxt().outer_expn_data().kind { @@ -721,6 +723,7 @@ pub fn is_adjusted(cx: &LateContext<'_, '_>, e: &Expr) -> bool { /// Returns the pre-expansion span if is this comes from an expansion of the /// macro `name`. /// See also `is_direct_expn_of`. +#[must_use] pub fn is_expn_of(mut span: Span, name: &str) -> Option { loop { if span.from_expansion() { @@ -748,6 +751,7 @@ pub fn is_expn_of(mut span: Span, name: &str) -> Option { /// `42` is considered expanded from `foo!` and `bar!` by `is_expn_of` but only /// `bar!` by /// `is_direct_expn_of`. +#[must_use] pub fn is_direct_expn_of(span: Span, name: &str) -> Option { if span.from_expansion() { let data = span.ctxt().outer_expn_data(); diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index 3c31067f7f8a..0675c6033410 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -417,6 +417,7 @@ enum Associativity { /// Chained `as` and explicit `:` type coercion never need inner parenthesis so /// they are considered /// associative. +#[must_use] fn associativity(op: &AssocOp) -> Associativity { use syntax::util::parser::AssocOp::*; diff --git a/rustc_tools_util/src/lib.rs b/rustc_tools_util/src/lib.rs index 92b710614ecb..36b58a398dc4 100644 --- a/rustc_tools_util/src/lib.rs +++ b/rustc_tools_util/src/lib.rs @@ -79,6 +79,7 @@ impl std::fmt::Debug for VersionInfo { } } +#[must_use] pub fn get_commit_hash() -> Option { std::process::Command::new("git") .args(&["rev-parse", "--short", "HEAD"]) @@ -87,6 +88,7 @@ pub fn get_commit_hash() -> Option { .and_then(|r| String::from_utf8(r.stdout).ok()) } +#[must_use] pub fn get_commit_date() -> Option { std::process::Command::new("git") .args(&["log", "-1", "--date=short", "--pretty=format:%cd"]) @@ -95,6 +97,7 @@ pub fn get_commit_date() -> Option { .and_then(|r| String::from_utf8(r.stdout).ok()) } +#[must_use] pub fn get_channel() -> Option { match env::var("CFG_RELEASE_CHANNEL") { Ok(channel) => Some(channel), diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 28f212fb2b2d..66ee41402eab 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 321] = [ +pub const ALL_LINTS: [Lint; 324] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -350,6 +350,13 @@ pub const ALL_LINTS: [Lint; 321] = [ deprecation: None, module: "double_comparison", }, + Lint { + name: "double_must_use", + group: "style", + desc: "`#[must_use]` attribute on a `#[must_use]`-returning function / method", + deprecation: None, + module: "functions", + }, Lint { name: "double_neg", group: "style", @@ -1155,6 +1162,20 @@ pub const ALL_LINTS: [Lint; 321] = [ deprecation: None, module: "inherent_impl", }, + Lint { + name: "must_use_candidate", + group: "pedantic", + desc: "function or method that could take a `#[must_use]` attribute", + deprecation: None, + module: "functions", + }, + Lint { + name: "must_use_unit", + group: "style", + desc: "`#[must_use]` attribute on a unit-returning function / method", + deprecation: None, + module: "functions", + }, Lint { name: "mut_from_ref", group: "correctness", diff --git a/tests/compile-test.rs b/tests/compile-test.rs index e65a4a9a40ac..b5cd2860e811 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -9,6 +9,7 @@ use std::fs; use std::io; use std::path::{Path, PathBuf}; +#[must_use] fn clippy_driver_path() -> PathBuf { if let Some(path) = option_env!("CLIPPY_DRIVER_PATH") { PathBuf::from(path) @@ -17,6 +18,7 @@ fn clippy_driver_path() -> PathBuf { } } +#[must_use] fn host_libs() -> PathBuf { if let Some(path) = option_env!("HOST_LIBS") { PathBuf::from(path) @@ -25,10 +27,12 @@ fn host_libs() -> PathBuf { } } +#[must_use] fn rustc_test_suite() -> Option { option_env!("RUSTC_TEST_SUITE").map(PathBuf::from) } +#[must_use] fn rustc_lib_path() -> PathBuf { option_env!("RUSTC_LIB_PATH").unwrap().into() } diff --git a/tests/ui/auxiliary/macro_rules.rs b/tests/ui/auxiliary/macro_rules.rs index 486e419bee56..b717afd0b27b 100644 --- a/tests/ui/auxiliary/macro_rules.rs +++ b/tests/ui/auxiliary/macro_rules.rs @@ -1,9 +1,18 @@ #![allow(dead_code)] -/// Used to test that certain lints don't trigger in imported external macros +//! Used to test that certain lints don't trigger in imported external macros + #[macro_export] macro_rules! foofoo { () => { loop {} }; } + +#[macro_export] +macro_rules! must_use_unit { + () => { + #[must_use] + fn foo() {} + }; +} diff --git a/tests/ui/double_must_use.rs b/tests/ui/double_must_use.rs new file mode 100644 index 000000000000..a48e675e4ea2 --- /dev/null +++ b/tests/ui/double_must_use.rs @@ -0,0 +1,27 @@ +#![warn(clippy::double_must_use)] + +#[must_use] +pub fn must_use_result() -> Result<(), ()> { + unimplemented!(); +} + +#[must_use] +pub fn must_use_tuple() -> (Result<(), ()>, u8) { + unimplemented!(); +} + +#[must_use] +pub fn must_use_array() -> [Result<(), ()>; 1] { + unimplemented!(); +} + +#[must_use = "With note"] +pub fn must_use_with_note() -> Result<(), ()> { + unimplemented!(); +} + +fn main() { + must_use_result(); + must_use_tuple(); + must_use_with_note(); +} diff --git a/tests/ui/double_must_use.stderr b/tests/ui/double_must_use.stderr new file mode 100644 index 000000000000..bc37785294fc --- /dev/null +++ b/tests/ui/double_must_use.stderr @@ -0,0 +1,27 @@ +error: this function has an empty `#[must_use]` attribute, but returns a type already marked as `#[must_use]` + --> $DIR/double_must_use.rs:4:1 + | +LL | pub fn must_use_result() -> Result<(), ()> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::double-must-use` implied by `-D warnings` + = help: either add some descriptive text or remove the attribute + +error: this function has an empty `#[must_use]` attribute, but returns a type already marked as `#[must_use]` + --> $DIR/double_must_use.rs:9:1 + | +LL | pub fn must_use_tuple() -> (Result<(), ()>, u8) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: either add some descriptive text or remove the attribute + +error: this function has an empty `#[must_use]` attribute, but returns a type already marked as `#[must_use]` + --> $DIR/double_must_use.rs:14:1 + | +LL | pub fn must_use_array() -> [Result<(), ()>; 1] { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: either add some descriptive text or remove the attribute + +error: aborting due to 3 previous errors + diff --git a/tests/ui/functions.stderr b/tests/ui/functions.stderr index 10d72fb96b1d..0a86568b18de 100644 --- a/tests/ui/functions.stderr +++ b/tests/ui/functions.stderr @@ -2,7 +2,7 @@ error: this function has too many arguments (8/7) --> $DIR/functions.rs:8:1 | LL | fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::too-many-arguments` implied by `-D warnings` @@ -16,19 +16,19 @@ LL | | three: &str, ... | LL | | eight: () LL | | ) { - | |_^ + | |__^ error: this function has too many arguments (8/7) --> $DIR/functions.rs:45:5 | LL | fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this function has too many arguments (8/7) --> $DIR/functions.rs:54:5 | LL | fn bad_method(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this public function dereferences a raw pointer but is not marked `unsafe` --> $DIR/functions.rs:63:34 diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs index 8f53b8cecbd3..e5a6cba9a253 100644 --- a/tests/ui/methods.rs +++ b/tests/ui/methods.rs @@ -4,16 +4,17 @@ #![warn(clippy::all, clippy::pedantic, clippy::option_unwrap_used)] #![allow( clippy::blacklisted_name, - unused, - clippy::print_stdout, + clippy::default_trait_access, + clippy::missing_docs_in_private_items, clippy::non_ascii_literal, clippy::new_without_default, - clippy::missing_docs_in_private_items, clippy::needless_pass_by_value, - clippy::default_trait_access, + clippy::print_stdout, + clippy::must_use_candidate, clippy::use_self, clippy::useless_format, - clippy::wrong_self_convention + clippy::wrong_self_convention, + unused )] #[macro_use] diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr index b30371fa541f..28da35bff3e0 100644 --- a/tests/ui/methods.stderr +++ b/tests/ui/methods.stderr @@ -1,5 +1,5 @@ error: defining a method called `add` on this type; consider implementing the `std::ops::Add` trait or choosing a less ambiguous name - --> $DIR/methods.rs:36:5 + --> $DIR/methods.rs:37:5 | LL | / pub fn add(self, other: T) -> T { LL | | self @@ -9,7 +9,7 @@ LL | | } = note: `-D clippy::should-implement-trait` implied by `-D warnings` error: methods called `new` usually return `Self` - --> $DIR/methods.rs:152:5 + --> $DIR/methods.rs:153:5 | LL | / fn new() -> i32 { LL | | 0 @@ -19,7 +19,7 @@ LL | | } = note: `-D clippy::new-ret-no-self` implied by `-D warnings` error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:174:13 + --> $DIR/methods.rs:175:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -31,7 +31,7 @@ LL | | .unwrap_or(0); = note: replace `map(|x| x + 1).unwrap_or(0)` with `map_or(0, |x| x + 1)` error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:178:13 + --> $DIR/methods.rs:179:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -41,7 +41,7 @@ LL | | ).unwrap_or(0); | |____________________________^ error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:182:13 + --> $DIR/methods.rs:183:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -51,7 +51,7 @@ LL | | }); | |__________________^ error: called `map(f).unwrap_or(None)` on an Option value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/methods.rs:187:13 + --> $DIR/methods.rs:188:13 | LL | let _ = opt.map(|x| Some(x + 1)).unwrap_or(None); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -59,7 +59,7 @@ LL | let _ = opt.map(|x| Some(x + 1)).unwrap_or(None); = note: replace `map(|x| Some(x + 1)).unwrap_or(None)` with `and_then(|x| Some(x + 1))` error: called `map(f).unwrap_or(None)` on an Option value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/methods.rs:189:13 + --> $DIR/methods.rs:190:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -69,7 +69,7 @@ LL | | ).unwrap_or(None); | |_____________________^ error: called `map(f).unwrap_or(None)` on an Option value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/methods.rs:193:13 + --> $DIR/methods.rs:194:13 | LL | let _ = opt | _____________^ @@ -80,7 +80,7 @@ LL | | .unwrap_or(None); = note: replace `map(|x| Some(x + 1)).unwrap_or(None)` with `and_then(|x| Some(x + 1))` error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:204:13 + --> $DIR/methods.rs:205:13 | LL | let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -88,7 +88,7 @@ LL | let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id); = note: replace `map(|p| format!("{}.", p)).unwrap_or(id)` with `map_or(id, |p| format!("{}.", p))` error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/methods.rs:208:13 + --> $DIR/methods.rs:209:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -100,7 +100,7 @@ LL | | .unwrap_or_else(|| 0); = note: replace `map(|x| x + 1).unwrap_or_else(|| 0)` with `map_or_else(|| 0, |x| x + 1)` error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/methods.rs:212:13 + --> $DIR/methods.rs:213:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -110,7 +110,7 @@ LL | | ).unwrap_or_else(|| 0); | |____________________________________^ error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/methods.rs:216:13 + --> $DIR/methods.rs:217:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -120,7 +120,7 @@ LL | | ); | |_________________^ error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead. - --> $DIR/methods.rs:246:13 + --> $DIR/methods.rs:247:13 | LL | let _ = v.iter().filter(|&x| *x < 0).next(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -129,7 +129,7 @@ LL | let _ = v.iter().filter(|&x| *x < 0).next(); = note: replace `filter(|&x| *x < 0).next()` with `find(|&x| *x < 0)` error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead. - --> $DIR/methods.rs:249:13 + --> $DIR/methods.rs:250:13 | LL | let _ = v.iter().filter(|&x| { | _____________^ @@ -139,7 +139,7 @@ LL | | ).next(); | |___________________________^ error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:266:22 + --> $DIR/methods.rs:267:22 | LL | let _ = v.iter().find(|&x| *x < 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x < 0)` @@ -147,25 +147,25 @@ LL | let _ = v.iter().find(|&x| *x < 0).is_some(); = note: `-D clippy::search-is-some` implied by `-D warnings` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:267:20 + --> $DIR/methods.rs:268:20 | LL | let _ = (0..1).find(|x| **y == *x).is_some(); // one dereference less | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| **y == x)` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:268:20 + --> $DIR/methods.rs:269:20 | LL | let _ = (0..1).find(|x| *x == 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| x == 0)` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:269:22 + --> $DIR/methods.rs:270:22 | LL | let _ = v.iter().find(|x| **x == 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x == 0)` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:272:13 + --> $DIR/methods.rs:273:13 | LL | let _ = v.iter().find(|&x| { | _____________^ @@ -175,13 +175,13 @@ LL | | ).is_some(); | |______________________________^ error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:278:22 + --> $DIR/methods.rs:279:22 | LL | let _ = v.iter().position(|&x| x < 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:281:13 + --> $DIR/methods.rs:282:13 | LL | let _ = v.iter().position(|&x| { | _____________^ @@ -191,13 +191,13 @@ LL | | ).is_some(); | |______________________________^ error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:287:22 + --> $DIR/methods.rs:288:22 | LL | let _ = v.iter().rposition(|&x| x < 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:290:13 + --> $DIR/methods.rs:291:13 | LL | let _ = v.iter().rposition(|&x| { | _____________^ @@ -207,7 +207,7 @@ LL | | ).is_some(); | |______________________________^ error: used unwrap() on an Option value. If you don't want to handle the None case gracefully, consider using expect() to provide a better panic message - --> $DIR/methods.rs:305:13 + --> $DIR/methods.rs:306:13 | LL | let _ = opt.unwrap(); | ^^^^^^^^^^^^ diff --git a/tests/ui/must_use_candidates.fixed b/tests/ui/must_use_candidates.fixed new file mode 100644 index 000000000000..dded5321af83 --- /dev/null +++ b/tests/ui/must_use_candidates.fixed @@ -0,0 +1,88 @@ +// run-rustfix +#![feature(never_type)] +#![allow(unused_mut)] +#![warn(clippy::must_use_candidate)] +use std::rc::Rc; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; + +pub struct MyAtomic(AtomicBool); +pub struct MyPure; + +#[must_use] pub fn pure(i: u8) -> u8 { + i +} + +impl MyPure { + #[must_use] pub fn inherent_pure(&self) -> u8 { + 0 + } +} + +pub trait MyPureTrait { + fn trait_pure(&self, i: u32) -> u32 { + self.trait_impl_pure(i) + 1 + } + + fn trait_impl_pure(&self, i: u32) -> u32; +} + +impl MyPureTrait for MyPure { + #[must_use] fn trait_impl_pure(&self, i: u32) -> u32 { + i + } +} + +pub fn without_result() { + // OK +} + +pub fn impure_primitive(i: &mut u8) -> u8 { + *i +} + +pub fn with_callback bool>(f: &F) -> bool { + f(0) +} + +#[must_use] pub fn with_marker(_d: std::marker::PhantomData<&mut u32>) -> bool { + true +} + +pub fn quoth_the_raven(_more: !) -> u32 { + unimplemented!(); +} + +pub fn atomics(b: &AtomicBool) -> bool { + b.load(Ordering::SeqCst) +} + +#[must_use] pub fn rcd(_x: Rc) -> bool { + true +} + +pub fn rcmut(_x: Rc<&mut u32>) -> bool { + true +} + +#[must_use] pub fn arcd(_x: Arc) -> bool { + false +} + +pub fn inner_types(_m: &MyAtomic) -> bool { + true +} + +static mut COUNTER: usize = 0; + +/// # Safety +/// +/// Don't ever call this from multiple threads +pub unsafe fn mutates_static() -> usize { + COUNTER += 1; + COUNTER +} + +fn main() { + assert_eq!(1, pure(1)); +} diff --git a/tests/ui/must_use_candidates.rs b/tests/ui/must_use_candidates.rs new file mode 100644 index 000000000000..29c0752994af --- /dev/null +++ b/tests/ui/must_use_candidates.rs @@ -0,0 +1,88 @@ +// run-rustfix +#![feature(never_type)] +#![allow(unused_mut)] +#![warn(clippy::must_use_candidate)] +use std::rc::Rc; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; + +pub struct MyAtomic(AtomicBool); +pub struct MyPure; + +pub fn pure(i: u8) -> u8 { + i +} + +impl MyPure { + pub fn inherent_pure(&self) -> u8 { + 0 + } +} + +pub trait MyPureTrait { + fn trait_pure(&self, i: u32) -> u32 { + self.trait_impl_pure(i) + 1 + } + + fn trait_impl_pure(&self, i: u32) -> u32; +} + +impl MyPureTrait for MyPure { + fn trait_impl_pure(&self, i: u32) -> u32 { + i + } +} + +pub fn without_result() { + // OK +} + +pub fn impure_primitive(i: &mut u8) -> u8 { + *i +} + +pub fn with_callback bool>(f: &F) -> bool { + f(0) +} + +pub fn with_marker(_d: std::marker::PhantomData<&mut u32>) -> bool { + true +} + +pub fn quoth_the_raven(_more: !) -> u32 { + unimplemented!(); +} + +pub fn atomics(b: &AtomicBool) -> bool { + b.load(Ordering::SeqCst) +} + +pub fn rcd(_x: Rc) -> bool { + true +} + +pub fn rcmut(_x: Rc<&mut u32>) -> bool { + true +} + +pub fn arcd(_x: Arc) -> bool { + false +} + +pub fn inner_types(_m: &MyAtomic) -> bool { + true +} + +static mut COUNTER: usize = 0; + +/// # Safety +/// +/// Don't ever call this from multiple threads +pub unsafe fn mutates_static() -> usize { + COUNTER += 1; + COUNTER +} + +fn main() { + assert_eq!(1, pure(1)); +} diff --git a/tests/ui/must_use_candidates.stderr b/tests/ui/must_use_candidates.stderr new file mode 100644 index 000000000000..f3a442102373 --- /dev/null +++ b/tests/ui/must_use_candidates.stderr @@ -0,0 +1,40 @@ +error: this function could have a `#[must_use]` attribute + --> $DIR/must_use_candidates.rs:12:1 + | +LL | pub fn pure(i: u8) -> u8 { + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[must_use] pub fn pure(i: u8) -> u8` + | + = note: `-D clippy::must-use-candidate` implied by `-D warnings` + +error: this method could have a `#[must_use]` attribute + --> $DIR/must_use_candidates.rs:17:5 + | +LL | pub fn inherent_pure(&self) -> u8 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[must_use] pub fn inherent_pure(&self) -> u8` + +error: this method could have a `#[must_use]` attribute + --> $DIR/must_use_candidates.rs:31:5 + | +LL | fn trait_impl_pure(&self, i: u32) -> u32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[must_use] fn trait_impl_pure(&self, i: u32) -> u32` + +error: this function could have a `#[must_use]` attribute + --> $DIR/must_use_candidates.rs:48:1 + | +LL | pub fn with_marker(_d: std::marker::PhantomData<&mut u32>) -> bool { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[must_use] pub fn with_marker(_d: std::marker::PhantomData<&mut u32>) -> bool` + +error: this function could have a `#[must_use]` attribute + --> $DIR/must_use_candidates.rs:60:1 + | +LL | pub fn rcd(_x: Rc) -> bool { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[must_use] pub fn rcd(_x: Rc) -> bool` + +error: this function could have a `#[must_use]` attribute + --> $DIR/must_use_candidates.rs:68:1 + | +LL | pub fn arcd(_x: Arc) -> bool { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[must_use] pub fn arcd(_x: Arc) -> bool` + +error: aborting due to 6 previous errors + diff --git a/tests/ui/must_use_unit.fixed b/tests/ui/must_use_unit.fixed new file mode 100644 index 000000000000..6c9aa434ac01 --- /dev/null +++ b/tests/ui/must_use_unit.fixed @@ -0,0 +1,26 @@ +//run-rustfix +// aux-build:macro_rules.rs + +#![warn(clippy::must_use_unit)] +#![allow(clippy::unused_unit)] + +#[macro_use] +extern crate macro_rules; + + +pub fn must_use_default() {} + + +pub fn must_use_unit() -> () {} + + +pub fn must_use_with_note() {} + +fn main() { + must_use_default(); + must_use_unit(); + must_use_with_note(); + + // We should not lint in external macros + must_use_unit!(); +} diff --git a/tests/ui/must_use_unit.rs b/tests/ui/must_use_unit.rs new file mode 100644 index 000000000000..8a395dc284db --- /dev/null +++ b/tests/ui/must_use_unit.rs @@ -0,0 +1,26 @@ +//run-rustfix +// aux-build:macro_rules.rs + +#![warn(clippy::must_use_unit)] +#![allow(clippy::unused_unit)] + +#[macro_use] +extern crate macro_rules; + +#[must_use] +pub fn must_use_default() {} + +#[must_use] +pub fn must_use_unit() -> () {} + +#[must_use = "With note"] +pub fn must_use_with_note() {} + +fn main() { + must_use_default(); + must_use_unit(); + must_use_with_note(); + + // We should not lint in external macros + must_use_unit!(); +} diff --git a/tests/ui/must_use_unit.stderr b/tests/ui/must_use_unit.stderr new file mode 100644 index 000000000000..15e0906b66b5 --- /dev/null +++ b/tests/ui/must_use_unit.stderr @@ -0,0 +1,28 @@ +error: this unit-returning function has a `#[must_use]` attribute + --> $DIR/must_use_unit.rs:11:1 + | +LL | #[must_use] + | ----------- help: remove the attribute +LL | pub fn must_use_default() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::must-use-unit` implied by `-D warnings` + +error: this unit-returning function has a `#[must_use]` attribute + --> $DIR/must_use_unit.rs:14:1 + | +LL | #[must_use] + | ----------- help: remove the attribute +LL | pub fn must_use_unit() -> () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this unit-returning function has a `#[must_use]` attribute + --> $DIR/must_use_unit.rs:17:1 + | +LL | #[must_use = "With note"] + | ------------------------- help: remove the attribute +LL | pub fn must_use_with_note() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors + diff --git a/tests/ui/shadow.rs b/tests/ui/shadow.rs index 7a5da67e0266..bd91ae4e9340 100644 --- a/tests/ui/shadow.rs +++ b/tests/ui/shadow.rs @@ -16,6 +16,7 @@ fn id(x: T) -> T { x } +#[must_use] fn first(x: (isize, isize)) -> isize { x.0 } diff --git a/tests/ui/shadow.stderr b/tests/ui/shadow.stderr index 184e781ae43d..7fa58cf76499 100644 --- a/tests/ui/shadow.stderr +++ b/tests/ui/shadow.stderr @@ -1,135 +1,135 @@ error: `x` is shadowed by itself in `&mut x` - --> $DIR/shadow.rs:25:5 + --> $DIR/shadow.rs:26:5 | LL | let x = &mut x; | ^^^^^^^^^^^^^^^ | = note: `-D clippy::shadow-same` implied by `-D warnings` note: previous binding is here - --> $DIR/shadow.rs:24:13 + --> $DIR/shadow.rs:25:13 | LL | let mut x = 1; | ^ error: `x` is shadowed by itself in `{ x }` - --> $DIR/shadow.rs:26:5 + --> $DIR/shadow.rs:27:5 | LL | let x = { x }; | ^^^^^^^^^^^^^^ | note: previous binding is here - --> $DIR/shadow.rs:25:9 + --> $DIR/shadow.rs:26:9 | LL | let x = &mut x; | ^ error: `x` is shadowed by itself in `(&*x)` - --> $DIR/shadow.rs:27:5 + --> $DIR/shadow.rs:28:5 | LL | let x = (&*x); | ^^^^^^^^^^^^^^ | note: previous binding is here - --> $DIR/shadow.rs:26:9 + --> $DIR/shadow.rs:27:9 | LL | let x = { x }; | ^ error: `x` is shadowed by `{ *x + 1 }` which reuses the original value - --> $DIR/shadow.rs:28:9 + --> $DIR/shadow.rs:29:9 | LL | let x = { *x + 1 }; | ^ | = note: `-D clippy::shadow-reuse` implied by `-D warnings` note: initialization happens here - --> $DIR/shadow.rs:28:13 + --> $DIR/shadow.rs:29:13 | LL | let x = { *x + 1 }; | ^^^^^^^^^^ note: previous binding is here - --> $DIR/shadow.rs:27:9 + --> $DIR/shadow.rs:28:9 | LL | let x = (&*x); | ^ error: `x` is shadowed by `id(x)` which reuses the original value - --> $DIR/shadow.rs:29:9 + --> $DIR/shadow.rs:30:9 | LL | let x = id(x); | ^ | note: initialization happens here - --> $DIR/shadow.rs:29:13 + --> $DIR/shadow.rs:30:13 | LL | let x = id(x); | ^^^^^ note: previous binding is here - --> $DIR/shadow.rs:28:9 + --> $DIR/shadow.rs:29:9 | LL | let x = { *x + 1 }; | ^ error: `x` is shadowed by `(1, x)` which reuses the original value - --> $DIR/shadow.rs:30:9 + --> $DIR/shadow.rs:31:9 | LL | let x = (1, x); | ^ | note: initialization happens here - --> $DIR/shadow.rs:30:13 + --> $DIR/shadow.rs:31:13 | LL | let x = (1, x); | ^^^^^^ note: previous binding is here - --> $DIR/shadow.rs:29:9 + --> $DIR/shadow.rs:30:9 | LL | let x = id(x); | ^ error: `x` is shadowed by `first(x)` which reuses the original value - --> $DIR/shadow.rs:31:9 + --> $DIR/shadow.rs:32:9 | LL | let x = first(x); | ^ | note: initialization happens here - --> $DIR/shadow.rs:31:13 + --> $DIR/shadow.rs:32:13 | LL | let x = first(x); | ^^^^^^^^ note: previous binding is here - --> $DIR/shadow.rs:30:9 + --> $DIR/shadow.rs:31:9 | LL | let x = (1, x); | ^ error: `x` is shadowed by `y` - --> $DIR/shadow.rs:33:9 + --> $DIR/shadow.rs:34:9 | LL | let x = y; | ^ | = note: `-D clippy::shadow-unrelated` implied by `-D warnings` note: initialization happens here - --> $DIR/shadow.rs:33:13 + --> $DIR/shadow.rs:34:13 | LL | let x = y; | ^ note: previous binding is here - --> $DIR/shadow.rs:31:9 + --> $DIR/shadow.rs:32:9 | LL | let x = first(x); | ^ error: `x` shadows a previous declaration - --> $DIR/shadow.rs:35:5 + --> $DIR/shadow.rs:36:5 | LL | let x; | ^^^^^^ | note: previous binding is here - --> $DIR/shadow.rs:33:9 + --> $DIR/shadow.rs:34:9 | LL | let x = y; | ^