diff options
| author | bors <bors@rust-lang.org> | 2021-08-13 05:30:37 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2021-08-13 05:30:37 +0000 |
| commit | 04c9901a0838d20e6ac0bcda94ea1a8c239bb0d7 (patch) | |
| tree | 0edaa187be5d38e15b7a7337a613d6803685807c | |
| parent | 13d6c5c90c84f59441a23ddfc0459b24c438589a (diff) | |
| parent | f92a0c8031339b1913e8ca89b1a9311c129c1091 (diff) | |
| download | rust-04c9901a0838d20e6ac0bcda94ea1a8c239bb0d7.tar.gz rust-04c9901a0838d20e6ac0bcda94ea1a8c239bb0d7.zip | |
Auto merge of #87954 - flip1995:clippyup, r=Manishearth
Update Clippy r? `@Manishearth`
77 files changed, 1311 insertions, 375 deletions
diff --git a/src/tools/clippy/.github/ISSUE_TEMPLATE/new_lint.md b/src/tools/clippy/.github/ISSUE_TEMPLATE/new_lint.md index e182c99ce06..2216bb9f293 100644 --- a/src/tools/clippy/.github/ISSUE_TEMPLATE/new_lint.md +++ b/src/tools/clippy/.github/ISSUE_TEMPLATE/new_lint.md @@ -15,8 +15,9 @@ labels: A-lint *What is the advantage of the recommended code over the original code* For example: -- Remove bounce checking inserted by ... -- Remove the need to duplicating/storing/typo ... +- Remove bounds check inserted by ... +- Remove the need to duplicate/store ... +- Remove typo ... ### Drawbacks diff --git a/src/tools/clippy/.github/workflows/clippy.yml b/src/tools/clippy/.github/workflows/clippy.yml index d856c55a41a..0339de77f3c 100644 --- a/src/tools/clippy/.github/workflows/clippy.yml +++ b/src/tools/clippy/.github/workflows/clippy.yml @@ -49,13 +49,13 @@ jobs: echo "LD_LIBRARY_PATH=${SYSROOT}/lib${LD_LIBRARY_PATH+:${LD_LIBRARY_PATH}}" >> $GITHUB_ENV - name: Build - run: cargo build --features deny-warnings,internal-lints + run: cargo build --features deny-warnings,internal-lints,metadata-collector-lint - name: Test - run: cargo test --features deny-warnings,internal-lints + run: cargo test --features deny-warnings,internal-lints,metadata-collector-lint - name: Test clippy_lints - run: cargo test --features deny-warnings,internal-lints + run: cargo test --features deny-warnings,internal-lints,metadata-collector-lint working-directory: clippy_lints - name: Test rustc_tools_util diff --git a/src/tools/clippy/.github/workflows/clippy_bors.yml b/src/tools/clippy/.github/workflows/clippy_bors.yml index 146b6fccd0c..1f4d666c7a9 100644 --- a/src/tools/clippy/.github/workflows/clippy_bors.yml +++ b/src/tools/clippy/.github/workflows/clippy_bors.yml @@ -112,13 +112,13 @@ jobs: echo "$SYSROOT/bin" >> $GITHUB_PATH - name: Build - run: cargo build --features deny-warnings,internal-lints + run: cargo build --features deny-warnings,internal-lints,metadata-collector-lint - name: Test - run: cargo test --features deny-warnings,internal-lints + run: cargo test --features deny-warnings,internal-lints,metadata-collector-lint - name: Test clippy_lints - run: cargo test --features deny-warnings,internal-lints + run: cargo test --features deny-warnings,internal-lints,metadata-collector-lint working-directory: clippy_lints - name: Test rustc_tools_util diff --git a/src/tools/clippy/.github/workflows/remark.yml b/src/tools/clippy/.github/workflows/remark.yml index 77efdec1e50..56c00544c93 100644 --- a/src/tools/clippy/.github/workflows/remark.yml +++ b/src/tools/clippy/.github/workflows/remark.yml @@ -20,6 +20,8 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v1.4.4 + with: + node-version: '12.x' - name: Install remark run: npm install remark-cli remark-lint remark-lint-maximum-line-length remark-preset-lint-recommended remark-gfm diff --git a/src/tools/clippy/CHANGELOG.md b/src/tools/clippy/CHANGELOG.md index acbefc8064d..2b89170073b 100644 --- a/src/tools/clippy/CHANGELOG.md +++ b/src/tools/clippy/CHANGELOG.md @@ -6,11 +6,141 @@ document. ## Unreleased / In Rust Nightly -[3ae8faf...master](https://github.com/rust-lang/rust-clippy/compare/3ae8faf...master) +[74d1561...master](https://github.com/rust-lang/rust-clippy/compare/74d1561...master) + +## Rust 1.55 + +Current beta, release 2021-09-09 + +[3ae8faf...74d1561](https://github.com/rust-lang/rust-clippy/compare/3ae8faf...74d1561) + +### Important Changes + +* Stabilized `cargo clippy --fix` :tada: + [#7405](https://github.com/rust-lang/rust-clippy/pull/7405) + +### New Lints + +* [`rc_mutex`] + [#7316](https://github.com/rust-lang/rust-clippy/pull/7316) +* [`nonstandard_macro_braces`] + [#7299](https://github.com/rust-lang/rust-clippy/pull/7299) +* [`strlen_on_c_strings`] + [#7243](https://github.com/rust-lang/rust-clippy/pull/7243) +* [`self_named_constructors`] + [#7403](https://github.com/rust-lang/rust-clippy/pull/7403) +* [`disallowed_script_idents`] + [#7400](https://github.com/rust-lang/rust-clippy/pull/7400) +* [`disallowed_type`] + [#7315](https://github.com/rust-lang/rust-clippy/pull/7315) +* [`missing_enforced_import_renames`] + [#7300](https://github.com/rust-lang/rust-clippy/pull/7300) +* [`extend_with_drain`] + [#7270](https://github.com/rust-lang/rust-clippy/pull/7270) + +### Moves and Deprecations + +* Moved [`from_iter_instead_of_collect`] to `pedantic` + [#7375](https://github.com/rust-lang/rust-clippy/pull/7375) +* Added `suspicious` as a new lint group for *code that is most likely wrong or useless* + [#7350](https://github.com/rust-lang/rust-clippy/pull/7350) + * Moved [`blanket_clippy_restriction_lints`] to `suspicious` + * Moved [`empty_loop`] to `suspicious` + * Moved [`eval_order_dependence`] to `suspicious` + * Moved [`float_equality_without_abs`] to `suspicious` + * Moved [`for_loops_over_fallibles`] to `suspicious` + * Moved [`misrefactored_assign_op`] to `suspicious` + * Moved [`mut_range_bound`] to `suspicious` + * Moved [`mutable_key_type`] to `suspicious` + * Moved [`suspicious_arithmetic_impl`] to `suspicious` + * Moved [`suspicious_assignment_formatting`] to `suspicious` + * Moved [`suspicious_else_formatting`] to `suspicious` + * Moved [`suspicious_map`] to `suspicious` + * Moved [`suspicious_op_assign_impl`] to `suspicious` + * Moved [`suspicious_unary_op_formatting`] to `suspicious` + +### Enhancements + +* [`while_let_on_iterator`]: Now suggests `&mut iter` inside closures + [#7262](https://github.com/rust-lang/rust-clippy/pull/7262) +* [`doc_markdown`]: + * Now detects unbalanced ticks + [#7357](https://github.com/rust-lang/rust-clippy/pull/7357) + * Add `FreeBSD` to the default configuration as an allowed identifier + [#7334](https://github.com/rust-lang/rust-clippy/pull/7334) +* [`wildcard_enum_match_arm`], [`match_wildcard_for_single_variants`]: Now allows wildcards for enums with unstable + or hidden variants + [#7407](https://github.com/rust-lang/rust-clippy/pull/7407) +* [`redundant_allocation`]: Now additionally supports the `Arc<>` type + [#7308](https://github.com/rust-lang/rust-clippy/pull/7308) +* [`blacklisted_name`]: Now allows blacklisted names in test code + [#7379](https://github.com/rust-lang/rust-clippy/pull/7379) +* [`redundant_closure`]: Suggests `&mut` for `FnMut` + [#7437](https://github.com/rust-lang/rust-clippy/pull/7437) +* [`disallowed_method`], [`disallowed_type`]: The configuration values `disallowed-method` and `disallowed-type` + no longer require fully qualified paths + [#7345](https://github.com/rust-lang/rust-clippy/pull/7345) +* [`zst_offset`]: Fixed lint invocation after it was accidentally suppressed + [#7396](https://github.com/rust-lang/rust-clippy/pull/7396) + +### False Positive Fixes + +* [`default_numeric_fallback`]: No longer lints on float literals as function arguments + [#7446](https://github.com/rust-lang/rust-clippy/pull/7446) +* [`use_self`]: No longer lints on type parameters + [#7288](https://github.com/rust-lang/rust-clippy/pull/7288) +* [`unimplemented`]: Now ignores the `assert` and `debug_assert` macros + [#7439](https://github.com/rust-lang/rust-clippy/pull/7439) +* [`branches_sharing_code`]: Now always checks for block expressions + [#7462](https://github.com/rust-lang/rust-clippy/pull/7462) +* [`field_reassign_with_default`]: No longer triggers in macros + [#7160](https://github.com/rust-lang/rust-clippy/pull/7160) +* [`redundant_clone`]: No longer lints on required clones for borrowed data + [#7346](https://github.com/rust-lang/rust-clippy/pull/7346) +* [`default_numeric_fallback`]: No longer triggers in external macros + [#7325](https://github.com/rust-lang/rust-clippy/pull/7325) +* [`needless_bool`]: No longer lints in macros + [#7442](https://github.com/rust-lang/rust-clippy/pull/7442) +* [`useless_format`]: No longer triggers when additional text is being appended + [#7442](https://github.com/rust-lang/rust-clippy/pull/7442) +* [`assertions_on_constants`]: `cfg!(...)` is no longer considered to be a constant + [#7319](https://github.com/rust-lang/rust-clippy/pull/7319) + +### Suggestion Fixes/Improvements + +* [`needless_collect`]: Now show correct lint messages for shadowed values + [#7289](https://github.com/rust-lang/rust-clippy/pull/7289) +* [`wrong_pub_self_convention`]: The deprecated message now suggest the correct configuration value + [#7382](https://github.com/rust-lang/rust-clippy/pull/7382) +* [`semicolon_if_nothing_returned`]: Allow missing semicolon in blocks with only one expression + [#7326](https://github.com/rust-lang/rust-clippy/pull/7326) + +### ICE Fixes + +* [`zero_sized_map_values`] + [#7470](https://github.com/rust-lang/rust-clippy/pull/7470) +* [`redundant_pattern_matching`] + [#7471](https://github.com/rust-lang/rust-clippy/pull/7471) +* [`modulo_one`] + [#7473](https://github.com/rust-lang/rust-clippy/pull/7473) +* [`use_self`] + [#7428](https://github.com/rust-lang/rust-clippy/pull/7428) + +### Documentation Improvements + +* Reworked Clippy's website: + [#7279](https://github.com/rust-lang/rust-clippy/pull/7279) + [#7172](https://github.com/rust-lang/rust-clippy/issues/7172) + * Added applicability information about lints + * Added a link to jump into the implementation + * Improved loading times + * Adapted some styling +* Clippy now uses a lint to generate its documentation + [#7298](https://github.com/rust-lang/rust-clippy/pull/7298) ## Rust 1.54 -Current beta, release 2021-07-29 +Current stable, released 2021-07-29 [7c7683c...3ae8faf](https://github.com/rust-lang/rust-clippy/compare/7c7683c...3ae8faf) @@ -29,7 +159,7 @@ Current beta, release 2021-07-29 ### Moves and Deprecations - Deprecate `pub_enum_variant_names` and `wrong_pub_self_convention` in favor of - the new `avoid_breaking_exported_api` config option (see + the new `avoid-breaking-exported-api` config option (see [Enhancements](#1-54-enhancements)) [#7187](https://github.com/rust-lang/rust-clippy/pull/7187) - Move [`inconsistent_struct_constructor`] to `pedantic` @@ -51,7 +181,7 @@ Current beta, release 2021-07-29 [#7163](https://github.com/rust-lang/rust-clippy/pull/7163) - [`if_then_some_else_none`]: Now works with the MSRV config [#7177](https://github.com/rust-lang/rust-clippy/pull/7177) -- Add `avoid_breaking_exported_api` config option for the lints +- Add `avoid-breaking-exported-api` config option for the lints [`enum_variant_names`], [`large_types_passed_by_value`], [`trivially_copy_pass_by_ref`], [`unnecessary_wraps`], [`upper_case_acronyms`], and [`wrong_self_convention`]. We recommend to set @@ -138,7 +268,7 @@ Current beta, release 2021-07-29 ## Rust 1.53 -Current stable, released 2021-06-17 +Released 2021-06-17 [6ed6f1e...7c7683c](https://github.com/rust-lang/rust-clippy/compare/6ed6f1e...7c7683c) @@ -2869,6 +2999,7 @@ Released 2018-09-13 [`unused_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit [`unusual_byte_groupings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unusual_byte_groupings [`unwrap_in_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_in_result +[`unwrap_or_else_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_or_else_default [`unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used [`upper_case_acronyms`]: https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms [`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug diff --git a/src/tools/clippy/clippy_lints/src/as_conversions.rs b/src/tools/clippy/clippy_lints/src/as_conversions.rs index 7c39a3e2ce3..0be460d67a7 100644 --- a/src/tools/clippy/clippy_lints/src/as_conversions.rs +++ b/src/tools/clippy/clippy_lints/src/as_conversions.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_help; use rustc_ast::ast::{Expr, ExprKind}; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -47,7 +47,7 @@ declare_lint_pass!(AsConversions => [AS_CONVERSIONS]); impl EarlyLintPass for AsConversions { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { - if in_external_macro(cx.sess(), expr.span) { + if in_external_macro(cx.sess, expr.span) { return; } diff --git a/src/tools/clippy/clippy_lints/src/doc.rs b/src/tools/clippy/clippy_lints/src/doc.rs index a3a3603c4c0..75561cfde36 100644 --- a/src/tools/clippy/clippy_lints/src/doc.rs +++ b/src/tools/clippy/clippy_lints/src/doc.rs @@ -230,15 +230,7 @@ impl<'tcx> LateLintPass<'tcx> for DocMarkdown { panic_span: None, }; fpu.visit_expr(&body.value); - lint_for_missing_headers( - cx, - item.def_id, - item.span, - sig, - headers, - Some(body_id), - fpu.panic_span, - ); + lint_for_missing_headers(cx, item.def_id, item.span, sig, headers, Some(body_id), fpu.panic_span); } }, hir::ItemKind::Impl(ref impl_) => { @@ -278,15 +270,7 @@ impl<'tcx> LateLintPass<'tcx> for DocMarkdown { panic_span: None, }; fpu.visit_expr(&body.value); - lint_for_missing_headers( - cx, - item.def_id, - item.span, - sig, - headers, - Some(body_id), - fpu.panic_span, - ); + lint_for_missing_headers(cx, item.def_id, item.span, sig, headers, Some(body_id), fpu.panic_span); } } } diff --git a/src/tools/clippy/clippy_lints/src/else_if_without_else.rs b/src/tools/clippy/clippy_lints/src/else_if_without_else.rs index 0541ac5eccc..b64246515f3 100644 --- a/src/tools/clippy/clippy_lints/src/else_if_without_else.rs +++ b/src/tools/clippy/clippy_lints/src/else_if_without_else.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_help; use rustc_ast::ast::{Expr, ExprKind}; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -49,7 +49,7 @@ declare_lint_pass!(ElseIfWithoutElse => [ELSE_IF_WITHOUT_ELSE]); impl EarlyLintPass for ElseIfWithoutElse { fn check_expr(&mut self, cx: &EarlyContext<'_>, mut item: &Expr) { - if in_external_macro(cx.sess(), item.span) { + if in_external_macro(cx.sess, item.span) { return; } diff --git a/src/tools/clippy/clippy_lints/src/functions/mod.rs b/src/tools/clippy/clippy_lints/src/functions/mod.rs index ce23c0ce4a0..04fc5887e8e 100644 --- a/src/tools/clippy/clippy_lints/src/functions/mod.rs +++ b/src/tools/clippy/clippy_lints/src/functions/mod.rs @@ -251,7 +251,7 @@ impl<'tcx> LateLintPass<'tcx> for Functions { hir_id: hir::HirId, ) { too_many_arguments::check_fn(cx, kind, decl, span, hir_id, self.too_many_arguments_threshold); - too_many_lines::check_fn(cx, span, body, self.too_many_lines_threshold); + too_many_lines::check_fn(cx, kind, span, body, self.too_many_lines_threshold); not_unsafe_ptr_arg_deref::check_fn(cx, kind, decl, body, hir_id); } diff --git a/src/tools/clippy/clippy_lints/src/functions/too_many_lines.rs b/src/tools/clippy/clippy_lints/src/functions/too_many_lines.rs index a666fee1a4a..008ef661b55 100644 --- a/src/tools/clippy/clippy_lints/src/functions/too_many_lines.rs +++ b/src/tools/clippy/clippy_lints/src/functions/too_many_lines.rs @@ -1,4 +1,5 @@ use rustc_hir as hir; +use rustc_hir::intravisit::FnKind; use rustc_lint::{LateContext, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_span::Span; @@ -8,8 +9,16 @@ use clippy_utils::source::snippet_opt; use super::TOO_MANY_LINES; -pub(super) fn check_fn(cx: &LateContext<'_>, span: Span, body: &'tcx hir::Body<'_>, too_many_lines_threshold: u64) { - if in_external_macro(cx.sess(), span) { +pub(super) fn check_fn( + cx: &LateContext<'_>, + kind: FnKind<'tcx>, + span: Span, + body: &'tcx hir::Body<'_>, + too_many_lines_threshold: u64, +) { + // Closures must be contained in a parent body, which will be checked for `too_many_lines`. + // Don't check closures for `too_many_lines` to avoid duplicated lints. + if matches!(kind, FnKind::Closure) || in_external_macro(cx.sess(), span) { return; } diff --git a/src/tools/clippy/clippy_lints/src/if_not_else.rs b/src/tools/clippy/clippy_lints/src/if_not_else.rs index 28db7233d70..3ce91d421ba 100644 --- a/src/tools/clippy/clippy_lints/src/if_not_else.rs +++ b/src/tools/clippy/clippy_lints/src/if_not_else.rs @@ -3,7 +3,7 @@ use clippy_utils::diagnostics::span_lint_and_help; use rustc_ast::ast::{BinOpKind, Expr, ExprKind, UnOp}; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -48,7 +48,7 @@ declare_lint_pass!(IfNotElse => [IF_NOT_ELSE]); impl EarlyLintPass for IfNotElse { fn check_expr(&mut self, cx: &EarlyContext<'_>, item: &Expr) { - if in_external_macro(cx.sess(), item.span) { + if in_external_macro(cx.sess, item.span) { return; } if let ExprKind::If(ref cond, _, Some(ref els)) = item.kind { diff --git a/src/tools/clippy/clippy_lints/src/items_after_statements.rs b/src/tools/clippy/clippy_lints/src/items_after_statements.rs index 429c6ed7d2d..3736d237642 100644 --- a/src/tools/clippy/clippy_lints/src/items_after_statements.rs +++ b/src/tools/clippy/clippy_lints/src/items_after_statements.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint; use rustc_ast::ast::{Block, ItemKind, StmtKind}; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -54,7 +54,7 @@ declare_lint_pass!(ItemsAfterStatements => [ITEMS_AFTER_STATEMENTS]); impl EarlyLintPass for ItemsAfterStatements { fn check_block(&mut self, cx: &EarlyContext<'_>, item: &Block) { - if in_external_macro(cx.sess(), item.span) { + if in_external_macro(cx.sess, item.span) { return; } @@ -68,7 +68,7 @@ impl EarlyLintPass for ItemsAfterStatements { // lint on all further items for stmt in stmts { if let StmtKind::Item(ref it) = *stmt { - if in_external_macro(cx.sess(), it.span) { + if in_external_macro(cx.sess, it.span) { return; } if let ItemKind::MacroDef(..) = it.kind { diff --git a/src/tools/clippy/clippy_lints/src/len_zero.rs b/src/tools/clippy/clippy_lints/src/len_zero.rs index a2cbfb1a05e..a519ad90df5 100644 --- a/src/tools/clippy/clippy_lints/src/len_zero.rs +++ b/src/tools/clippy/clippy_lints/src/len_zero.rs @@ -207,8 +207,7 @@ fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, trait_items } } - if cx.access_levels.is_exported(visited_trait.def_id) - && trait_items.iter().any(|i| is_named_self(cx, i, sym::len)) + if cx.access_levels.is_exported(visited_trait.def_id) && trait_items.iter().any(|i| is_named_self(cx, i, sym::len)) { let mut current_and_super_traits = DefIdSet::default(); fill_trait_set(visited_trait.def_id.to_def_id(), &mut current_and_super_traits, cx); @@ -331,17 +330,15 @@ fn check_for_is_empty( None, None, ), - Some(is_empty) if !cx.access_levels.is_exported(is_empty.def_id.expect_local()) => { - ( - format!( - "{} `{}` has a public `len` method, but a private `is_empty` method", - item_kind, - item_name.as_str(), - ), - Some(cx.tcx.def_span(is_empty.def_id)), - None, - ) - }, + Some(is_empty) if !cx.access_levels.is_exported(is_empty.def_id.expect_local()) => ( + format!( + "{} `{}` has a public `len` method, but a private `is_empty` method", + item_kind, + item_name.as_str(), + ), + Some(cx.tcx.def_span(is_empty.def_id)), + None, + ), Some(is_empty) if !(is_empty.fn_has_self_parameter && check_is_empty_sig(cx.tcx.fn_sig(is_empty.def_id).skip_binder(), self_kind, output)) => diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index f49b382c5ea..dbdb4251b3b 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -797,6 +797,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: methods::UNNECESSARY_FILTER_MAP, methods::UNNECESSARY_FOLD, methods::UNNECESSARY_LAZY_EVALUATIONS, + methods::UNWRAP_OR_ELSE_DEFAULT, methods::UNWRAP_USED, methods::USELESS_ASREF, methods::WRONG_SELF_CONVENTION, @@ -1341,6 +1342,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(methods::UNNECESSARY_FILTER_MAP), LintId::of(methods::UNNECESSARY_FOLD), LintId::of(methods::UNNECESSARY_LAZY_EVALUATIONS), + LintId::of(methods::UNWRAP_OR_ELSE_DEFAULT), LintId::of(methods::USELESS_ASREF), LintId::of(methods::WRONG_SELF_CONVENTION), LintId::of(methods::ZST_OFFSET), @@ -1535,6 +1537,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(methods::STRING_EXTEND_CHARS), LintId::of(methods::UNNECESSARY_FOLD), LintId::of(methods::UNNECESSARY_LAZY_EVALUATIONS), + LintId::of(methods::UNWRAP_OR_ELSE_DEFAULT), LintId::of(methods::WRONG_SELF_CONVENTION), LintId::of(misc::TOPLEVEL_REF_ARG), LintId::of(misc::ZERO_PTR), diff --git a/src/tools/clippy/clippy_lints/src/literal_representation.rs b/src/tools/clippy/clippy_lints/src/literal_representation.rs index 699ddce0cff..0e5121ca3d7 100644 --- a/src/tools/clippy/clippy_lints/src/literal_representation.rs +++ b/src/tools/clippy/clippy_lints/src/literal_representation.rs @@ -10,7 +10,7 @@ use clippy_utils::{ use if_chain::if_chain; use rustc_ast::ast::{Expr, ExprKind, Lit, LitKind}; use rustc_errors::Applicability; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_tool_lint, impl_lint_pass}; use std::iter; @@ -222,7 +222,7 @@ impl_lint_pass!(LiteralDigitGrouping => [ impl EarlyLintPass for LiteralDigitGrouping { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { - if in_external_macro(cx.sess(), expr.span) { + if in_external_macro(cx.sess, expr.span) { return; } @@ -415,7 +415,7 @@ impl_lint_pass!(DecimalLiteralRepresentation => [DECIMAL_LITERAL_REPRESENTATION] impl EarlyLintPass for DecimalLiteralRepresentation { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { - if in_external_macro(cx.sess(), expr.span) { + if in_external_macro(cx.sess, expr.span) { return; } diff --git a/src/tools/clippy/clippy_lints/src/loops/never_loop.rs b/src/tools/clippy/clippy_lints/src/loops/never_loop.rs index e97b7c94170..6d9f6215ed4 100644 --- a/src/tools/clippy/clippy_lints/src/loops/never_loop.rs +++ b/src/tools/clippy/clippy_lints/src/loops/never_loop.rs @@ -1,13 +1,36 @@ +use super::utils::make_iterator_snippet; use super::NEVER_LOOP; -use clippy_utils::diagnostics::span_lint; -use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, Stmt, StmtKind}; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::higher; +use clippy_utils::source::snippet; +use rustc_errors::Applicability; +use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, LoopSource, Node, Pat, Stmt, StmtKind}; use rustc_lint::LateContext; use std::iter::{once, Iterator}; pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::Loop(block, _, _, _) = expr.kind { + if let ExprKind::Loop(block, _, source, _) = expr.kind { match never_loop_block(block, expr.hir_id) { - NeverLoopResult::AlwaysBreak => span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops"), + NeverLoopResult::AlwaysBreak => { + span_lint_and_then(cx, NEVER_LOOP, expr.span, "this loop never actually loops", |diag| { + if_chain! { + if let LoopSource::ForLoop = source; + if let Some((_, Node::Expr(parent_match))) = cx.tcx.hir().parent_iter(expr.hir_id).nth(1); + if let Some((pat, iterator, _, for_span)) = higher::for_loop(parent_match); + then { + // Suggests using an `if let` instead. This is `Unspecified` because the + // loop may (probably) contain `break` statements which would be invalid + // in an `if let`. + diag.span_suggestion_verbose( + for_span.with_hi(iterator.span.hi()), + "if you need the first element of the iterator, try writing", + for_to_if_let_sugg(cx, iterator, pat), + Applicability::Unspecified, + ); + } + }; + }); + }, NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (), } } @@ -170,3 +193,14 @@ fn never_loop_expr_branch<'a, T: Iterator<Item = &'a Expr<'a>>>(e: &mut T, main_ e.map(|e| never_loop_expr(e, main_loop_id)) .fold(NeverLoopResult::AlwaysBreak, combine_branches) } + +fn for_to_if_let_sugg(cx: &LateContext<'_>, iterator: &Expr<'_>, pat: &Pat<'_>) -> String { + let pat_snippet = snippet(cx, pat.span, "_"); + let iter_snippet = make_iterator_snippet(cx, iterator, &mut Applicability::Unspecified); + + format!( + "if let Some({pat}) = {iter}.next()", + pat = pat_snippet, + iter = iter_snippet + ) +} diff --git a/src/tools/clippy/clippy_lints/src/loops/while_let_on_iterator.rs b/src/tools/clippy/clippy_lints/src/loops/while_let_on_iterator.rs index d57588716a5..ef822e0cbe5 100644 --- a/src/tools/clippy/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/src/tools/clippy/clippy_lints/src/loops/while_let_on_iterator.rs @@ -7,7 +7,7 @@ use clippy_utils::{ use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor}; -use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, MatchSource, Node, PatKind, QPath, UnOp}; +use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, MatchSource, Mutability, Node, PatKind, QPath, UnOp}; use rustc_lint::LateContext; use rustc_span::{symbol::sym, Span, Symbol}; @@ -48,7 +48,12 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // borrowed mutably. TODO: If the struct can be partially moved from and the struct isn't used // afterwards a mutable borrow of a field isn't necessary. let ref_mut = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) { - "&mut " + if cx.typeck_results().node_type(iter_expr.hir_id).ref_mutability() == Some(Mutability::Mut) { + // Reborrow for mutable references. It may not be possible to get a mutable reference here. + "&mut *" + } else { + "&mut " + } } else { "" }; @@ -69,6 +74,8 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { struct IterExpr { /// The span of the whole expression, not just the path and fields stored here. span: Span, + /// The HIR id of the whole expression, not just the path and fields stored here. + hir_id: HirId, /// The fields used, in order of child to parent. fields: Vec<Symbol>, /// The path being used. @@ -78,12 +85,14 @@ struct IterExpr { /// the expression might have side effects. fn try_parse_iter_expr(cx: &LateContext<'_>, mut e: &Expr<'_>) -> Option<IterExpr> { let span = e.span; + let hir_id = e.hir_id; let mut fields = Vec::new(); loop { match e.kind { ExprKind::Path(ref path) => { break Some(IterExpr { span, + hir_id, fields, path: cx.qpath_res(path, e.hir_id), }); @@ -137,7 +146,7 @@ fn is_expr_same_child_or_parent_field(cx: &LateContext<'_>, expr: &Expr<'_>, fie match expr.kind { ExprKind::Field(base, name) => { if let Some((head_field, tail_fields)) = fields.split_first() { - if name.name == *head_field && is_expr_same_field(cx, base, fields, path_res) { + if name.name == *head_field && is_expr_same_field(cx, base, tail_fields, path_res) { return true; } // Check if the expression is a parent field diff --git a/src/tools/clippy/clippy_lints/src/methods/extend_with_drain.rs b/src/tools/clippy/clippy_lints/src/methods/extend_with_drain.rs index 57e10ce42f8..8829b8c5f4d 100644 --- a/src/tools/clippy/clippy_lints/src/methods/extend_with_drain.rs +++ b/src/tools/clippy/clippy_lints/src/methods/extend_with_drain.rs @@ -16,7 +16,10 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, arg: //check source object if let ExprKind::MethodCall(src_method, _, [drain_vec, drain_arg], _) = &arg.kind; if src_method.ident.as_str() == "drain"; - if let src_ty = cx.typeck_results().expr_ty(drain_vec).peel_refs(); + let src_ty = cx.typeck_results().expr_ty(drain_vec); + //check if actual src type is mutable for code suggestion + let immutable = src_ty.is_mutable_ptr(); + let src_ty = src_ty.peel_refs(); if is_type_diagnostic_item(cx, src_ty, sym::vec_type); //check drain range if let src_ty_range = cx.typeck_results().expr_ty(drain_arg).peel_refs(); @@ -30,8 +33,9 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, arg: "use of `extend` instead of `append` for adding the full range of a second vector", "try this", format!( - "{}.append(&mut {})", + "{}.append({}{})", snippet_with_applicability(cx, recv.span, "..", &mut applicability), + if immutable { "" } else { "&mut " }, snippet_with_applicability(cx, drain_vec.span, "..", &mut applicability) ), applicability, diff --git a/src/tools/clippy/clippy_lints/src/methods/from_iter_instead_of_collect.rs b/src/tools/clippy/clippy_lints/src/methods/from_iter_instead_of_collect.rs index b4188d9ed30..99c03844f49 100644 --- a/src/tools/clippy/clippy_lints/src/methods/from_iter_instead_of_collect.rs +++ b/src/tools/clippy/clippy_lints/src/methods/from_iter_instead_of_collect.rs @@ -1,10 +1,11 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_opt; use clippy_utils::ty::implements_trait; use clippy_utils::{is_expr_path_def_path, paths, sugg}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; -use rustc_lint::{LateContext, LintContext}; +use rustc_lint::LateContext; use rustc_middle::ty::Ty; use rustc_span::sym; @@ -43,7 +44,7 @@ fn extract_turbofish(cx: &LateContext<'_>, expr: &hir::Expr<'_>, ty: Ty<'tcx>) - let call_site = expr.span.source_callsite(); if_chain! { - if let Ok(snippet) = cx.sess().source_map().span_to_snippet(call_site); + if let Some(snippet) = snippet_opt(cx, call_site); let snippet_split = snippet.split("::").collect::<Vec<_>>(); if let Some((_, elements)) = snippet_split.split_last(); diff --git a/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs b/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs index e8ad16bc0de..08d3a7ce92b 100644 --- a/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs +++ b/src/tools/clippy/clippy_lints/src/methods/map_flatten.rs @@ -52,18 +52,32 @@ pub(super) fn check<'tcx>( ); } - // lint if caller of `.map().flatten()` is an Option - if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::option_type) { - let func_snippet = snippet(cx, map_arg.span, ".."); - let hint = format!(".and_then({})", func_snippet); - span_lint_and_sugg( - cx, - MAP_FLATTEN, - expr.span.with_lo(recv.span.hi()), - "called `map(..).flatten()` on an `Option`", - "try using `and_then` instead", - hint, - Applicability::MachineApplicable, - ); - } + // lint if caller of `.map().flatten()` is an Option or Result + let caller_type = match cx.typeck_results().expr_ty(recv).kind() { + ty::Adt(adt, _) => { + if cx.tcx.is_diagnostic_item(sym::option_type, adt.did) { + "Option" + } else if cx.tcx.is_diagnostic_item(sym::result_type, adt.did) { + "Result" + } else { + return; + } + }, + _ => { + return; + }, + }; + + let func_snippet = snippet(cx, map_arg.span, ".."); + let hint = format!(".and_then({})", func_snippet); + let lint_info = format!("called `map(..).flatten()` on an `{}`", caller_type); + span_lint_and_sugg( + cx, + MAP_FLATTEN, + expr.span.with_lo(recv.span.hi()), + &lint_info, + "try using `and_then` instead", + hint, + Applicability::MachineApplicable, + ); } diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index 1909fabb22f..91606ed3b2b 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -56,6 +56,7 @@ mod uninit_assumed_init; mod unnecessary_filter_map; mod unnecessary_fold; mod unnecessary_lazy_eval; +mod unwrap_or_else_default; mod unwrap_used; mod useless_asref; mod utils; @@ -312,6 +313,31 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does + /// Checks for usages of `_.unwrap_or_else(Default::default)` on `Option` and + /// `Result` values. + /// + /// ### Why is this bad? + /// Readability, these can be written as `_.unwrap_or_default`, which is + /// simpler and more concise. + /// + /// ### Examples + /// ```rust + /// # let x = Some(1); + /// + /// // Bad + /// x.unwrap_or_else(Default::default); + /// x.unwrap_or_else(u32::default); + /// + /// // Good + /// x.unwrap_or_default(); + /// ``` + pub UNWRAP_OR_ELSE_DEFAULT, + style, + "using `.unwrap_or_else(Default::default)`, which is more succinctly expressed as `.unwrap_or_default()`" +} + +declare_clippy_lint! { + /// ### What it does /// Checks for usage of `option.map(_).unwrap_or(_)` or `option.map(_).unwrap_or_else(_)` or /// `result.map(_).unwrap_or_else(_)`. /// @@ -1766,6 +1792,7 @@ impl_lint_pass!(Methods => [ SHOULD_IMPLEMENT_TRAIT, WRONG_SELF_CONVENTION, OK_EXPECT, + UNWRAP_OR_ELSE_DEFAULT, MAP_UNWRAP_OR, RESULT_MAP_OR_INTO_OPTION, OPTION_MAP_OR_NONE, @@ -2172,7 +2199,10 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio }, ("unwrap_or_else", [u_arg]) => match method_call!(recv) { Some(("map", [recv, map_arg], _)) if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, msrv) => {}, - _ => unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or"), + _ => { + unwrap_or_else_default::check(cx, expr, recv, u_arg); + unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or"); + }, }, _ => {}, } diff --git a/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs b/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs index ef615b0aa40..c1d22e5d72c 100644 --- a/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs +++ b/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs @@ -1,7 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::eager_or_lazy::is_lazyness_candidate; +use clippy_utils::is_trait_item; use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_macro_callsite}; -use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type}; +use clippy_utils::ty::implements_trait; +use clippy_utils::ty::{is_type_diagnostic_item, match_type}; use clippy_utils::{contains_return, last_path_segment, paths}; use if_chain::if_chain; use rustc_errors::Applicability; @@ -34,15 +36,23 @@ pub(super) fn check<'tcx>( or_has_args: bool, span: Span, ) -> bool { + let is_default_default = || is_trait_item(cx, fun, sym::Default); + + let implements_default = |arg, default_trait_id| { + let arg_ty = cx.typeck_results().expr_ty(arg); + implements_trait(cx, arg_ty, default_trait_id, &[]) + }; + if_chain! { if !or_has_args; if name == "unwrap_or"; if let hir::ExprKind::Path(ref qpath) = fun.kind; - let path = last_path_segment(qpath).ident.name; - if matches!(path, kw::Default | sym::new); - let arg_ty = cx.typeck_results().expr_ty(arg); if let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default); - if implements_trait(cx, arg_ty, default_trait_id, &[]); + let path = last_path_segment(qpath).ident.name; + // needs to target Default::default in particular or be *::new and have a Default impl + // available + if (matches!(path, kw::Default) && is_default_default()) + || (matches!(path, sym::new) && implements_default(arg, default_trait_id)); then { let mut applicability = Applicability::MachineApplicable; diff --git a/src/tools/clippy/clippy_lints/src/methods/unwrap_or_else_default.rs b/src/tools/clippy/clippy_lints/src/methods/unwrap_or_else_default.rs new file mode 100644 index 00000000000..677aa80e1b7 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/methods/unwrap_or_else_default.rs @@ -0,0 +1,45 @@ +//! Lint for `some_result_or_option.unwrap_or_else(Default::default)` + +use super::UNWRAP_OR_ELSE_DEFAULT; +use clippy_utils::{ + diagnostics::span_lint_and_sugg, is_trait_item, source::snippet_with_applicability, ty::is_type_diagnostic_item, +}; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::LateContext; +use rustc_span::sym; + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx hir::Expr<'_>, + recv: &'tcx hir::Expr<'_>, + u_arg: &'tcx hir::Expr<'_>, +) { + // something.unwrap_or_else(Default::default) + // ^^^^^^^^^- recv ^^^^^^^^^^^^^^^^- u_arg + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- expr + let recv_ty = cx.typeck_results().expr_ty(recv); + let is_option = is_type_diagnostic_item(cx, recv_ty, sym::option_type); + let is_result = is_type_diagnostic_item(cx, recv_ty, sym::result_type); + + if_chain! { + if is_option || is_result; + if is_trait_item(cx, u_arg, sym::Default); + then { + let mut applicability = Applicability::MachineApplicable; + + span_lint_and_sugg( + cx, + UNWRAP_OR_ELSE_DEFAULT, + expr.span, + "use of `.unwrap_or_else(..)` to construct default value", + "try", + format!( + "{}.unwrap_or_default()", + snippet_with_applicability(cx, recv.span, "..", &mut applicability) + ), + applicability, + ); + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/misc_early/mod.rs b/src/tools/clippy/clippy_lints/src/misc_early/mod.rs index 06fe967dafc..b32feab4ee3 100644 --- a/src/tools/clippy/clippy_lints/src/misc_early/mod.rs +++ b/src/tools/clippy/clippy_lints/src/misc_early/mod.rs @@ -12,7 +12,7 @@ use clippy_utils::source::snippet_opt; use rustc_ast::ast::{Expr, Generics, Lit, LitFloatType, LitIntType, LitKind, NodeId, Pat, PatKind}; use rustc_ast::visit::FnKind; use rustc_data_structures::fx::FxHashMap; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; @@ -307,7 +307,7 @@ impl EarlyLintPass for MiscEarlyLints { } fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { - if in_external_macro(cx.sess(), expr.span) { + if in_external_macro(cx.sess, expr.span) { return; } double_neg::check(cx, expr); diff --git a/src/tools/clippy/clippy_lints/src/misc_early/unneeded_field_pattern.rs b/src/tools/clippy/clippy_lints/src/misc_early/unneeded_field_pattern.rs index 2201cf56d52..fff533167ed 100644 --- a/src/tools/clippy/clippy_lints/src/misc_early/unneeded_field_pattern.rs +++ b/src/tools/clippy/clippy_lints/src/misc_early/unneeded_field_pattern.rs @@ -1,6 +1,7 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_help}; +use clippy_utils::source::snippet_opt; use rustc_ast::ast::{Pat, PatKind}; -use rustc_lint::{EarlyContext, LintContext}; +use rustc_lint::EarlyContext; use super::UNNEEDED_FIELD_PATTERN; @@ -48,7 +49,7 @@ pub(super) fn check(cx: &EarlyContext<'_>, pat: &Pat) { match field.pat.kind { PatKind::Wild => {}, _ => { - if let Ok(n) = cx.sess().source_map().span_to_snippet(field.span) { + if let Some(n) = snippet_opt(cx, field.span) { normal.push(n); } }, diff --git a/src/tools/clippy/clippy_lints/src/needless_borrow.rs b/src/tools/clippy/clippy_lints/src/needless_borrow.rs index 3f0b23ee4d3..ba8f9446af8 100644 --- a/src/tools/clippy/clippy_lints/src/needless_borrow.rs +++ b/src/tools/clippy/clippy_lints/src/needless_borrow.rs @@ -27,11 +27,15 @@ declare_clippy_lint! { /// /// ### Example /// ```rust + /// fn fun(_a: &i32) {} + /// /// // Bad /// let x: &i32 = &&&&&&5; + /// fun(&x); /// /// // Good /// let x: &i32 = &5; + /// fun(x); /// ``` pub NEEDLESS_BORROW, style, diff --git a/src/tools/clippy/clippy_lints/src/needless_continue.rs b/src/tools/clippy/clippy_lints/src/needless_continue.rs index 5088b8bb0d3..5a50cc48d61 100644 --- a/src/tools/clippy/clippy_lints/src/needless_continue.rs +++ b/src/tools/clippy/clippy_lints/src/needless_continue.rs @@ -422,7 +422,7 @@ fn check_and_warn<'a>(cx: &EarlyContext<'_>, expr: &'a ast::Expr) { /// /// is transformed to /// -/// ```ignore +/// ```text /// { /// let x = 5; /// ``` diff --git a/src/tools/clippy/clippy_lints/src/needless_for_each.rs b/src/tools/clippy/clippy_lints/src/needless_for_each.rs index d9aa42fe8ee..9a6ddc72ce5 100644 --- a/src/tools/clippy/clippy_lints/src/needless_for_each.rs +++ b/src/tools/clippy/clippy_lints/src/needless_for_each.rs @@ -122,7 +122,7 @@ impl LateLintPass<'_> for NeedlessForEach { /// 2. Detect use of `return` in `Loop` in the closure body. /// /// NOTE: The functionality of this type is similar to -/// [`crate::utilts::visitors::find_all_ret_expressions`], but we can't use +/// [`clippy_utils::visitors::find_all_ret_expressions`], but we can't use /// `find_all_ret_expressions` instead of this type. The reasons are: /// 1. `find_all_ret_expressions` passes the argument of `ExprKind::Ret` to a callback, but what we /// need here is `ExprKind::Ret` itself. diff --git a/src/tools/clippy/clippy_lints/src/no_effect.rs b/src/tools/clippy/clippy_lints/src/no_effect.rs index e07518b2586..28e9e6f438e 100644 --- a/src/tools/clippy/clippy_lints/src/no_effect.rs +++ b/src/tools/clippy/clippy_lints/src/no_effect.rs @@ -3,7 +3,7 @@ use clippy_utils::source::snippet_opt; use clippy_utils::ty::has_drop; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::{BinOpKind, BlockCheckMode, Expr, ExprKind, Stmt, StmtKind, UnsafeSource}; +use rustc_hir::{is_range_literal, BinOpKind, BlockCheckMode, Expr, ExprKind, Stmt, StmtKind, UnsafeSource}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use std::ops::Deref; @@ -68,12 +68,14 @@ fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { ExprKind::Call(callee, args) => { if let ExprKind::Path(ref qpath) = callee.kind { let res = cx.qpath_res(qpath, callee.hir_id); - match res { - Res::Def(DefKind::Struct | DefKind::Variant | DefKind::Ctor(..), ..) => { - !has_drop(cx, cx.typeck_results().expr_ty(expr)) - && args.iter().all(|arg| has_no_effect(cx, arg)) - }, - _ => false, + let def_matched = matches!( + res, + Res::Def(DefKind::Struct | DefKind::Variant | DefKind::Ctor(..), ..) + ); + if def_matched || is_range_literal(expr) { + !has_drop(cx, cx.typeck_results().expr_ty(expr)) && args.iter().all(|arg| has_no_effect(cx, arg)) + } else { + false } } else { false diff --git a/src/tools/clippy/clippy_lints/src/non_expressive_names.rs b/src/tools/clippy/clippy_lints/src/non_expressive_names.rs index 06c431babc2..f6254aa715a 100644 --- a/src/tools/clippy/clippy_lints/src/non_expressive_names.rs +++ b/src/tools/clippy/clippy_lints/src/non_expressive_names.rs @@ -127,6 +127,7 @@ const ALLOWED_TO_BE_SIMILAR: &[&[&str]] = &[ &["qpath", "path"], &["lit", "lint"], &["wparam", "lparam"], + &["iter", "item"], ]; struct SimilarNamesNameVisitor<'a, 'tcx, 'b>(&'b mut SimilarNamesLocalVisitor<'a, 'tcx>); diff --git a/src/tools/clippy/clippy_lints/src/nonstandard_macro_braces.rs b/src/tools/clippy/clippy_lints/src/nonstandard_macro_braces.rs index dbe9cbe0ded..ca660a9250d 100644 --- a/src/tools/clippy/clippy_lints/src/nonstandard_macro_braces.rs +++ b/src/tools/clippy/clippy_lints/src/nonstandard_macro_braces.rs @@ -7,6 +7,7 @@ use clippy_utils::{diagnostics::span_lint_and_help, in_macro, is_direct_expn_of, use if_chain::if_chain; use rustc_ast::ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_hir::def_id::DefId; use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::Span; @@ -91,13 +92,23 @@ impl EarlyLintPass for MacroBraces { } fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, mac_braces: &'a MacroBraces) -> Option<MacroInfo<'a>> { + let unnested_or_local = || { + let nested = in_macro(span.ctxt().outer_expn_data().call_site); + !nested + || span + .macro_backtrace() + .last() + .map_or(false, |e| e.macro_def_id.map_or(false, DefId::is_local)) + }; if_chain! { + // Make sure we are only one level deep otherwise there are to many FP's if in_macro(span); if let Some((name, braces)) = find_matching_macro(span, &mac_braces.macro_braces); if let Some(snip) = snippet_opt(cx, span.ctxt().outer_expn_data().call_site); // we must check only invocation sites // https://github.com/rust-lang/rust-clippy/issues/7422 - if snip.starts_with(name); + if snip.starts_with(&format!("{}!", name)); + if unnested_or_local(); // make formatting consistent let c = snip.replace(" ", ""); if !c.starts_with(&format!("{}!{}", name, braces.0)); diff --git a/src/tools/clippy/clippy_lints/src/pass_by_ref_or_value.rs b/src/tools/clippy/clippy_lints/src/pass_by_ref_or_value.rs index 1222a95d4ea..157b18c1f6b 100644 --- a/src/tools/clippy/clippy_lints/src/pass_by_ref_or_value.rs +++ b/src/tools/clippy/clippy_lints/src/pass_by_ref_or_value.rs @@ -2,9 +2,9 @@ use std::cmp; use std::iter; use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::is_self_ty; use clippy_utils::source::snippet; use clippy_utils::ty::is_copy; +use clippy_utils::{is_self, is_self_ty}; use if_chain::if_chain; use rustc_ast::attr; use rustc_errors::Applicability; @@ -170,7 +170,7 @@ impl<'tcx> PassByRefOrValue { if size <= self.ref_min_size; if let hir::TyKind::Rptr(_, MutTy { ty: decl_ty, .. }) = input.kind; then { - let value_type = if is_self_ty(decl_ty) { + let value_type = if fn_body.and_then(|body| body.params.get(index)).map_or(false, is_self) { "self".into() } else { snippet(cx, decl_ty.span, "_").into() diff --git a/src/tools/clippy/clippy_lints/src/redundant_closure_call.rs b/src/tools/clippy/clippy_lints/src/redundant_closure_call.rs index a79b2fe76e2..7314bce83e0 100644 --- a/src/tools/clippy/clippy_lints/src/redundant_closure_call.rs +++ b/src/tools/clippy/clippy_lints/src/redundant_closure_call.rs @@ -8,7 +8,7 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::intravisit as hir_visit; use rustc_hir::intravisit::Visitor as HirVisitor; -use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass}; use rustc_middle::hir::map::Map; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -63,7 +63,7 @@ impl<'ast> ast_visit::Visitor<'ast> for ReturnVisitor { impl EarlyLintPass for RedundantClosureCall { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) { - if in_external_macro(cx.sess(), expr.span) { + if in_external_macro(cx.sess, expr.span) { return; } if_chain! { diff --git a/src/tools/clippy/clippy_lints/src/reference.rs b/src/tools/clippy/clippy_lints/src/reference.rs index e0930d69ab9..77b6e60d893 100644 --- a/src/tools/clippy/clippy_lints/src/reference.rs +++ b/src/tools/clippy/clippy_lints/src/reference.rs @@ -54,7 +54,8 @@ impl EarlyLintPass for DerefAddrOf { then { let mut applicability = Applicability::MachineApplicable; let sugg = if e.span.from_expansion() { - if let Ok(macro_source) = cx.sess.source_map().span_to_snippet(e.span) { + #[allow(clippy::option_if_let_else)] + if let Some(macro_source) = snippet_opt(cx, e.span) { // Remove leading whitespace from the given span // e.g: ` $visitor` turns into `$visitor` let trim_leading_whitespaces = |span| { diff --git a/src/tools/clippy/clippy_lints/src/swap.rs b/src/tools/clippy/clippy_lints/src/swap.rs index 4fa8e77a67b..f126908e84b 100644 --- a/src/tools/clippy/clippy_lints/src/swap.rs +++ b/src/tools/clippy/clippy_lints/src/swap.rs @@ -1,15 +1,16 @@ -use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::snippet_with_applicability; use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{differing_macro_contexts, eq_expr_value}; +use clippy_utils::{can_mut_borrow_both, differing_macro_contexts, eq_expr_value}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{Block, Expr, ExprKind, PatKind, QPath, StmtKind}; +use rustc_hir::{BinOpKind, Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::sym; +use rustc_span::source_map::Spanned; +use rustc_span::{sym, Span}; declare_clippy_lint! { /// ### What it does @@ -70,9 +71,67 @@ impl<'tcx> LateLintPass<'tcx> for Swap { fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) { check_manual_swap(cx, block); check_suspicious_swap(cx, block); + check_xor_swap(cx, block); } } +fn generate_swap_warning(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>, span: Span, is_xor_based: bool) { + let mut applicability = Applicability::MachineApplicable; + + if !can_mut_borrow_both(cx, e1, e2) { + if let ExprKind::Index(lhs1, idx1) = e1.kind { + if let ExprKind::Index(lhs2, idx2) = e2.kind { + if eq_expr_value(cx, lhs1, lhs2) { + let ty = cx.typeck_results().expr_ty(lhs1).peel_refs(); + + if matches!(ty.kind(), ty::Slice(_)) + || matches!(ty.kind(), ty::Array(_, _)) + || is_type_diagnostic_item(cx, ty, sym::vec_type) + || is_type_diagnostic_item(cx, ty, sym::vecdeque_type) + { + let slice = Sugg::hir_with_applicability(cx, lhs1, "<slice>", &mut applicability); + span_lint_and_sugg( + cx, + MANUAL_SWAP, + span, + &format!("this looks like you are swapping elements of `{}` manually", slice), + "try", + format!( + "{}.swap({}, {})", + slice.maybe_par(), + snippet_with_applicability(cx, idx1.span, "..", &mut applicability), + snippet_with_applicability(cx, idx2.span, "..", &mut applicability), + ), + applicability, + ); + } + } + } + } + return; + } + + let first = Sugg::hir_with_applicability(cx, e1, "..", &mut applicability); + let second = Sugg::hir_with_applicability(cx, e2, "..", &mut applicability); + span_lint_and_then( + cx, + MANUAL_SWAP, + span, + &format!("this looks like you are swapping `{}` and `{}` manually", first, second), + |diag| { + diag.span_suggestion( + span, + "try", + format!("std::mem::swap({}, {})", first.mut_addr(), second.mut_addr()), + applicability, + ); + if !is_xor_based { + diag.note("or maybe you should use `std::mem::replace`?"); + } + }, + ); +} + /// Implementation of the `MANUAL_SWAP` lint. fn check_manual_swap(cx: &LateContext<'_>, block: &Block<'_>) { for w in block.stmts.windows(3) { @@ -96,121 +155,11 @@ fn check_manual_swap(cx: &LateContext<'_>, block: &Block<'_>) { if eq_expr_value(cx, tmp_init, lhs1); if eq_expr_value(cx, rhs1, lhs2); then { - if let ExprKind::Field(lhs1, _) = lhs1.kind { - if let ExprKind::Field(lhs2, _) = lhs2.kind { - if lhs1.hir_id.owner == lhs2.hir_id.owner { - return; - } - } - } - - let mut applicability = Applicability::MachineApplicable; - - let slice = check_for_slice(cx, lhs1, lhs2); - let (replace, what, sugg) = if let Slice::NotSwappable = slice { - return; - } else if let Slice::Swappable(slice, idx1, idx2) = slice { - if let Some(slice) = Sugg::hir_opt(cx, slice) { - ( - false, - format!(" elements of `{}`", slice), - format!( - "{}.swap({}, {})", - slice.maybe_par(), - snippet_with_applicability(cx, idx1.span, "..", &mut applicability), - snippet_with_applicability(cx, idx2.span, "..", &mut applicability), - ), - ) - } else { - (false, String::new(), String::new()) - } - } else if let (Some(first), Some(second)) = (Sugg::hir_opt(cx, lhs1), Sugg::hir_opt(cx, rhs1)) { - ( - true, - format!(" `{}` and `{}`", first, second), - format!("std::mem::swap({}, {})", first.mut_addr(), second.mut_addr()), - ) - } else { - (true, String::new(), String::new()) - }; - let span = w[0].span.to(second.span); - - span_lint_and_then( - cx, - MANUAL_SWAP, - span, - &format!("this looks like you are swapping{} manually", what), - |diag| { - if !sugg.is_empty() { - diag.span_suggestion( - span, - "try", - sugg, - applicability, - ); - - if replace { - diag.note("or maybe you should use `std::mem::replace`?"); - } - } - } - ); - } - } - } -} - -enum Slice<'a> { - /// `slice.swap(idx1, idx2)` can be used - /// - /// ## Example - /// - /// ```rust - /// # let mut a = vec![0, 1]; - /// let t = a[1]; - /// a[1] = a[0]; - /// a[0] = t; - /// // can be written as - /// a.swap(0, 1); - /// ``` - Swappable(&'a Expr<'a>, &'a Expr<'a>, &'a Expr<'a>), - /// The `swap` function cannot be used. - /// - /// ## Example - /// - /// ```rust - /// # let mut a = [vec![1, 2], vec![3, 4]]; - /// let t = a[0][1]; - /// a[0][1] = a[1][0]; - /// a[1][0] = t; - /// ``` - NotSwappable, - /// Not a slice - None, -} - -/// Checks if both expressions are index operations into "slice-like" types. -fn check_for_slice<'a>(cx: &LateContext<'_>, lhs1: &'a Expr<'_>, lhs2: &'a Expr<'_>) -> Slice<'a> { - if let ExprKind::Index(lhs1, idx1) = lhs1.kind { - if let ExprKind::Index(lhs2, idx2) = lhs2.kind { - if eq_expr_value(cx, lhs1, lhs2) { - let ty = cx.typeck_results().expr_ty(lhs1).peel_refs(); - - if matches!(ty.kind(), ty::Slice(_)) - || matches!(ty.kind(), ty::Array(_, _)) - || is_type_diagnostic_item(cx, ty, sym::vec_type) - || is_type_diagnostic_item(cx, ty, sym::vecdeque_type) - { - return Slice::Swappable(lhs1, idx1, idx2); - } - } else { - return Slice::NotSwappable; + generate_swap_warning(cx, lhs1, lhs2, span, false); } } } - - Slice::None } /// Implementation of the `ALMOST_SWAPPED` lint. @@ -262,3 +211,40 @@ fn check_suspicious_swap(cx: &LateContext<'_>, block: &Block<'_>) { } } } + +/// Implementation of the xor case for `MANUAL_SWAP` lint. +fn check_xor_swap(cx: &LateContext<'_>, block: &Block<'_>) { + for window in block.stmts.windows(3) { + if_chain! { + if let Some((lhs0, rhs0)) = extract_sides_of_xor_assign(&window[0]); + if let Some((lhs1, rhs1)) = extract_sides_of_xor_assign(&window[1]); + if let Some((lhs2, rhs2)) = extract_sides_of_xor_assign(&window[2]); + if eq_expr_value(cx, lhs0, rhs1); + if eq_expr_value(cx, lhs2, rhs1); + if eq_expr_value(cx, lhs1, rhs0); + if eq_expr_value(cx, lhs1, rhs2); + then { + let span = window[0].span.to(window[2].span); + generate_swap_warning(cx, lhs0, rhs0, span, true); + } + }; + } +} + +/// Returns the lhs and rhs of an xor assignment statement. +fn extract_sides_of_xor_assign<'a, 'hir>(stmt: &'a Stmt<'hir>) -> Option<(&'a Expr<'hir>, &'a Expr<'hir>)> { + if let StmtKind::Semi(expr) = stmt.kind { + if let ExprKind::AssignOp( + Spanned { + node: BinOpKind::BitXor, + .. + }, + lhs, + rhs, + ) = expr.kind + { + return Some((lhs, rhs)); + } + } + None +} diff --git a/src/tools/clippy/clippy_lints/src/types/mod.rs b/src/tools/clippy/clippy_lints/src/types/mod.rs index ad7409fe3a9..371bb8b445a 100644 --- a/src/tools/clippy/clippy_lints/src/types/mod.rs +++ b/src/tools/clippy/clippy_lints/src/types/mod.rs @@ -186,7 +186,7 @@ declare_clippy_lint! { /// Checks for use of redundant allocations anywhere in the code. /// /// ### Why is this bad? - /// Expressions such as `Rc<&T>`, `Rc<Rc<T>>`, `Rc<Arc<T>>`, `Rc<Box<T>>`, Arc<&T>`, `Arc<Rc<T>>`, + /// Expressions such as `Rc<&T>`, `Rc<Rc<T>>`, `Rc<Arc<T>>`, `Rc<Box<T>>`, `Arc<&T>`, `Arc<Rc<T>>`, /// `Arc<Arc<T>>`, `Arc<Box<T>>`, `Box<&T>`, `Box<Rc<T>>`, `Box<Arc<T>>`, `Box<Box<T>>`, add an unnecessary level of indirection. /// /// ### Example diff --git a/src/tools/clippy/clippy_lints/src/unnested_or_patterns.rs b/src/tools/clippy/clippy_lints/src/unnested_or_patterns.rs index 9acfbc994b3..c8a231341b7 100644 --- a/src/tools/clippy/clippy_lints/src/unnested_or_patterns.rs +++ b/src/tools/clippy/clippy_lints/src/unnested_or_patterns.rs @@ -35,8 +35,6 @@ declare_clippy_lint! { /// ``` /// Use instead: /// ```rust - /// #![feature(or_patterns)] - /// /// fn main() { /// if let Some(0 | 2) = Some(0) {} /// } diff --git a/src/tools/clippy/clippy_lints/src/unused_unit.rs b/src/tools/clippy/clippy_lints/src/unused_unit.rs index 9ed5e585f84..1164ac4938f 100644 --- a/src/tools/clippy/clippy_lints/src/unused_unit.rs +++ b/src/tools/clippy/clippy_lints/src/unused_unit.rs @@ -1,10 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::position_before_rarrow; +use clippy_utils::source::{position_before_rarrow, snippet_opt}; use if_chain::if_chain; use rustc_ast::ast; use rustc_ast::visit::FnKind; use rustc_errors::Applicability; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; use rustc_span::BytePos; @@ -125,17 +125,16 @@ fn is_unit_expr(expr: &ast::Expr) -> bool { } fn lint_unneeded_unit_return(cx: &EarlyContext<'_>, ty: &ast::Ty, span: Span) { - let (ret_span, appl) = if let Ok(fn_source) = cx.sess().source_map().span_to_snippet(span.with_hi(ty.span.hi())) { - position_before_rarrow(&fn_source).map_or((ty.span, Applicability::MaybeIncorrect), |rpos| { - ( - #[allow(clippy::cast_possible_truncation)] - ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)), - Applicability::MachineApplicable, - ) - }) - } else { - (ty.span, Applicability::MaybeIncorrect) - }; + let (ret_span, appl) = + snippet_opt(cx, span.with_hi(ty.span.hi())).map_or((ty.span, Applicability::MaybeIncorrect), |fn_source| { + position_before_rarrow(&fn_source).map_or((ty.span, Applicability::MaybeIncorrect), |rpos| { + ( + #[allow(clippy::cast_possible_truncation)] + ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)), + Applicability::MachineApplicable, + ) + }) + }); span_lint_and_sugg( cx, UNUSED_UNIT, diff --git a/src/tools/clippy/clippy_lints/src/utils/conf.rs b/src/tools/clippy/clippy_lints/src/utils/conf.rs index 44d3d456342..a28b1d78f7d 100644 --- a/src/tools/clippy/clippy_lints/src/utils/conf.rs +++ b/src/tools/clippy/clippy_lints/src/utils/conf.rs @@ -36,13 +36,13 @@ impl TryConf { /// See (rust-clippy#7172) macro_rules! define_Conf { ($( - #[doc = $doc:literal] + $(#[doc = $doc:literal])+ $(#[conf_deprecated($dep:literal)])? ($name:ident: $ty:ty = $default:expr), )*) => { /// Clippy lint configuration pub struct Conf { - $(#[doc = $doc] pub $name: $ty,)* + $($(#[doc = $doc])+ pub $name: $ty,)* } mod defaults { @@ -119,7 +119,7 @@ macro_rules! define_Conf { stringify!($name), stringify!($ty), format!("{:?}", super::defaults::$name()), - $doc, + concat!($($doc, '\n',)*), deprecation_reason, ) }, @@ -132,18 +132,30 @@ macro_rules! define_Conf { // N.B., this macro is parsed by util/lintlib.py define_Conf! { - /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION. Suppress lints whenever the suggested change would cause breakage for other crates. + /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION. + /// + /// Suppress lints whenever the suggested change would cause breakage for other crates. (avoid_breaking_exported_api: bool = true), - /// Lint: MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE. The minimum rust version that the project supports + /// Lint: MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE. + /// + /// The minimum rust version that the project supports (msrv: Option<String> = None), - /// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about. NB: `bar` is not here since it has legitimate uses + /// Lint: BLACKLISTED_NAME. + /// + /// The list of blacklisted names to lint about. NB: `bar` is not here since it has legitimate uses (blacklisted_names: Vec<String> = ["foo", "baz", "quux"].iter().map(ToString::to_string).collect()), - /// Lint: COGNITIVE_COMPLEXITY. The maximum cognitive complexity a function can have + /// Lint: COGNITIVE_COMPLEXITY. + /// + /// The maximum cognitive complexity a function can have (cognitive_complexity_threshold: u64 = 25), - /// DEPRECATED LINT: CYCLOMATIC_COMPLEXITY. Use the Cognitive Complexity lint instead. + /// DEPRECATED LINT: CYCLOMATIC_COMPLEXITY. + /// + /// Use the Cognitive Complexity lint instead. #[conf_deprecated("Please use `cognitive-complexity-threshold` instead")] (cyclomatic_complexity_threshold: Option<u64> = None), - /// Lint: DOC_MARKDOWN. The list of words this lint should not consider as identifiers needing ticks + /// Lint: DOC_MARKDOWN. + /// + /// The list of words this lint should not consider as identifiers needing ticks (doc_valid_idents: Vec<String> = [ "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "DirectX", @@ -164,55 +176,109 @@ define_Conf! { "MinGW", "CamelCase", ].iter().map(ToString::to_string).collect()), - /// Lint: TOO_MANY_ARGUMENTS. The maximum number of argument a function or method can have + /// Lint: TOO_MANY_ARGUMENTS. + /// + /// The maximum number of argument a function or method can have (too_many_arguments_threshold: u64 = 7), - /// Lint: TYPE_COMPLEXITY. The maximum complexity a type can have + /// Lint: TYPE_COMPLEXITY. + /// + /// The maximum complexity a type can have (type_complexity_threshold: u64 = 250), - /// Lint: MANY_SINGLE_CHAR_NAMES. The maximum number of single char bindings a scope may have + /// Lint: MANY_SINGLE_CHAR_NAMES. + /// + /// The maximum number of single char bindings a scope may have (single_char_binding_names_threshold: u64 = 4), - /// Lint: BOXED_LOCAL, USELESS_VEC. The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap + /// Lint: BOXED_LOCAL, USELESS_VEC. + /// + /// The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap (too_large_for_stack: u64 = 200), - /// Lint: ENUM_VARIANT_NAMES. The minimum number of enum variants for the lints about variant names to trigger + /// Lint: ENUM_VARIANT_NAMES. + /// + /// The minimum number of enum variants for the lints about variant names to trigger (enum_variant_name_threshold: u64 = 3), - /// Lint: LARGE_ENUM_VARIANT. The maximum size of a enum's variant to avoid box suggestion + /// Lint: LARGE_ENUM_VARIANT. + /// + /// The maximum size of a enum's variant to avoid box suggestion (enum_variant_size_threshold: u64 = 200), - /// Lint: VERBOSE_BIT_MASK. The maximum allowed size of a bit mask before suggesting to use 'trailing_zeros' + /// Lint: VERBOSE_BIT_MASK. + /// + /// The maximum allowed size of a bit mask before suggesting to use 'trailing_zeros' (verbose_bit_mask_threshold: u64 = 1), - /// Lint: DECIMAL_LITERAL_REPRESENTATION. The lower bound for linting decimal literals + /// Lint: DECIMAL_LITERAL_REPRESENTATION. + /// + /// The lower bound for linting decimal literals (literal_representation_threshold: u64 = 16384), - /// Lint: TRIVIALLY_COPY_PASS_BY_REF. The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by reference. + /// Lint: TRIVIALLY_COPY_PASS_BY_REF. + /// + /// The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by reference. (trivial_copy_size_limit: Option<u64> = None), - /// Lint: LARGE_TYPE_PASS_BY_MOVE. The minimum size (in bytes) to consider a type for passing by reference instead of by value. + /// Lint: LARGE_TYPE_PASS_BY_MOVE. + /// + /// The minimum size (in bytes) to consider a type for passing by reference instead of by value. (pass_by_value_size_limit: u64 = 256), - /// Lint: TOO_MANY_LINES. The maximum number of lines a function or method can have + /// Lint: TOO_MANY_LINES. + /// + /// The maximum number of lines a function or method can have (too_many_lines_threshold: u64 = 100), - /// Lint: LARGE_STACK_ARRAYS, LARGE_CONST_ARRAYS. The maximum allowed size for arrays on the stack + /// Lint: LARGE_STACK_ARRAYS, LARGE_CONST_ARRAYS. + /// + /// The maximum allowed size for arrays on the stack (array_size_threshold: u64 = 512_000), - /// Lint: VEC_BOX. The size of the boxed type in bytes, where boxing in a `Vec` is allowed + /// Lint: VEC_BOX. + /// + /// The size of the boxed type in bytes, where boxing in a `Vec` is allowed (vec_box_size_threshold: u64 = 4096), - /// Lint: TYPE_REPETITION_IN_BOUNDS. The maximum number of bounds a trait can have to be linted + /// Lint: TYPE_REPETITION_IN_BOUNDS. + /// + /// The maximum number of bounds a trait can have to be linted (max_trait_bounds: u64 = 3), - /// Lint: STRUCT_EXCESSIVE_BOOLS. The maximum number of bool fields a struct can have + /// Lint: STRUCT_EXCESSIVE_BOOLS. + /// + /// The maximum number of bool fields a struct can have (max_struct_bools: u64 = 3), - /// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bool parameters a function can have + /// Lint: FN_PARAMS_EXCESSIVE_BOOLS. + /// + /// The maximum number of bool parameters a function can have (max_fn_params_bools: u64 = 3), - /// Lint: WILDCARD_IMPORTS. Whether to allow certain wildcard imports (prelude, super in tests). + /// Lint: WILDCARD_IMPORTS. + /// + /// Whether to allow certain wildcard imports (prelude, super in tests). (warn_on_all_wildcard_imports: bool = false), - /// Lint: DISALLOWED_METHOD. The list of disallowed methods, written as fully qualified paths. + /// Lint: DISALLOWED_METHOD. + /// + /// The list of disallowed methods, written as fully qualified paths. (disallowed_methods: Vec<String> = Vec::new()), - /// Lint: DISALLOWED_TYPE. The list of disallowed types, written as fully qualified paths. + /// Lint: DISALLOWED_TYPE. + /// + /// The list of disallowed types, written as fully qualified paths. (disallowed_types: Vec<String> = Vec::new()), - /// Lint: UNREADABLE_LITERAL. Should the fraction of a decimal be linted to include separators. + /// Lint: UNREADABLE_LITERAL. + /// + /// Should the fraction of a decimal be linted to include separators. (unreadable_literal_lint_fractions: bool = true), - /// Lint: UPPER_CASE_ACRONYMS. Enables verbose mode. Triggers if there is more than one uppercase char next to each other + /// Lint: UPPER_CASE_ACRONYMS. + /// + /// Enables verbose mode. Triggers if there is more than one uppercase char next to each other (upper_case_acronyms_aggressive: bool = false), - /// Lint: _CARGO_COMMON_METADATA. For internal testing only, ignores the current `publish` settings in the Cargo manifest. + /// Lint: _CARGO_COMMON_METADATA. + /// + /// For internal testing only, ignores the current `publish` settings in the Cargo manifest. (cargo_ignore_publish: bool = false), - /// Lint: NONSTANDARD_MACRO_BRACES. Enforce the named macros always use the braces specified. <br> A `MacroMatcher` can be added like so `{ name = "macro_name", brace = "(" }`. If the macro is could be used with a full path two `MacroMatcher`s have to be added one with the full path `crate_name::macro_name` and one with just the macro name. + /// Lint: NONSTANDARD_MACRO_BRACES. + /// + /// Enforce the named macros always use the braces specified. + /// + /// A `MacroMatcher` can be added like so `{ name = "macro_name", brace = "(" }`. If the macro + /// is could be used with a full path two `MacroMatcher`s have to be added one with the full path + /// `crate_name::macro_name` and one with just the macro name. (standard_macro_braces: Vec<crate::nonstandard_macro_braces::MacroMatcher> = Vec::new()), - /// Lint: MISSING_ENFORCED_IMPORT_RENAMES. The list of imports to always rename, a fully qualified path followed by the rename. + /// Lint: MISSING_ENFORCED_IMPORT_RENAMES. + /// + /// The list of imports to always rename, a fully qualified path followed by the rename. (enforced_import_renames: Vec<crate::utils::conf::Rename> = Vec::new()), - /// Lint: RESTRICTED_SCRIPTS. The list of unicode scripts allowed to be used in the scope. + /// Lint: RESTRICTED_SCRIPTS. + /// + /// The list of unicode scripts allowed to be used in the scope. (allowed_scripts: Vec<String> = vec!["Latin".to_string()]), } diff --git a/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs index 47336459d7d..a48a5385083 100644 --- a/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -82,7 +82,7 @@ This lint has the following configuration variables: /// `default` macro_rules! CONFIGURATION_VALUE_TEMPLATE { () => { - "* {name}: {ty}: {doc} (defaults to `{default}`)\n" + "* {name}: `{ty}`: {doc} (defaults to `{default}`)\n" }; } @@ -344,11 +344,16 @@ fn parse_config_field_doc(doc_comment: &str) -> Option<(Vec<String>, String)> { if let Some(split_pos) = doc_comment.find('.'); then { let mut doc_comment = doc_comment.to_string(); - let documentation = doc_comment.split_off(split_pos); + let mut documentation = doc_comment.split_off(split_pos); + // Extract lints doc_comment.make_ascii_lowercase(); let lints: Vec<String> = doc_comment.split_off(DOC_START.len()).split(", ").map(str::to_string).collect(); + // Format documentation correctly + // split off leading `.` from lint name list and indent for correct formatting + documentation = documentation.trim_start_matches('.').trim().replace("\n ", "\n "); + Some((lints, documentation)) } else { None diff --git a/src/tools/clippy/clippy_utils/src/diagnostics.rs b/src/tools/clippy/clippy_utils/src/diagnostics.rs index 7c94474cb35..71cfa196fc3 100644 --- a/src/tools/clippy/clippy_utils/src/diagnostics.rs +++ b/src/tools/clippy/clippy_utils/src/diagnostics.rs @@ -65,7 +65,7 @@ pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: impl Into<Mult /// /// # Example /// -/// ```ignore +/// ```text /// error: constant division of 0.0 with 0.0 will always result in NaN /// --> $DIR/zero_div_zero.rs:6:25 /// | @@ -103,7 +103,7 @@ pub fn span_lint_and_help<'a, T: LintContext>( /// /// # Example /// -/// ```ignore +/// ```text /// error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing. /// --> $DIR/drop_forget_ref.rs:10:5 /// | @@ -189,7 +189,7 @@ pub fn span_lint_hir_and_then( /// /// # Example /// -/// ```ignore +/// ```text /// error: This `.fold` can be more succinctly expressed as `.any` /// --> $DIR/methods.rs:390:13 /// | diff --git a/src/tools/clippy/clippy_utils/src/higher.rs b/src/tools/clippy/clippy_utils/src/higher.rs index f32f1109b08..884180f0586 100644 --- a/src/tools/clippy/clippy_utils/src/higher.rs +++ b/src/tools/clippy/clippy_utils/src/higher.rs @@ -195,8 +195,8 @@ pub fn vec_macro<'e>(cx: &LateContext<'_>, expr: &'e hir::Expr<'_>) -> Option<Ve /// - `assert!`, `assert_eq!` and `assert_ne!` /// - `debug_assert!`, `debug_assert_eq!` and `debug_assert_ne!` /// For example: -/// `assert!(expr)` will return Some([expr]) -/// `debug_assert_eq!(a, b)` will return Some([a, b]) +/// `assert!(expr)` will return `Some([expr])` +/// `debug_assert_eq!(a, b)` will return `Some([a, b])` pub fn extract_assert_macro_args<'tcx>(e: &'tcx Expr<'tcx>) -> Option<Vec<&'tcx Expr<'tcx>>> { /// Try to match the AST for a pattern that contains a match, for example when two args are /// compared @@ -283,7 +283,7 @@ pub struct FormatArgsExpn<'tcx> { /// String literal expressions which represent the format string split by "{}" pub format_string_parts: &'tcx [Expr<'tcx>], - /// Symbols corresponding to [`format_string_parts`] + /// Symbols corresponding to [`Self::format_string_parts`] pub format_string_symbols: Vec<Symbol>, /// Expressions like `ArgumentV1::new(arg0, Debug::fmt)` pub args: &'tcx [Expr<'tcx>], diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index 59f878f8b20..1d59d6bfea1 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -326,6 +326,25 @@ pub fn is_trait_method(cx: &LateContext<'_>, expr: &Expr<'_>, diag_item: Symbol) .map_or(false, |did| is_diag_trait_item(cx, did, diag_item)) } +/// Checks if the given expression is a path referring an item on the trait +/// that is marked with the given diagnostic item. +/// +/// For checking method call expressions instead of path expressions, use +/// [`is_trait_method`]. +/// +/// For example, this can be used to find if an expression like `u64::default` +/// refers to an item of the trait `Default`, which is associated with the +/// `diag_item` of `sym::Default`. +pub fn is_trait_item(cx: &LateContext<'_>, expr: &Expr<'_>, diag_item: Symbol) -> bool { + if let hir::ExprKind::Path(ref qpath) = expr.kind { + cx.qpath_res(qpath, expr.hir_id) + .opt_def_id() + .map_or(false, |def_id| is_diag_trait_item(cx, def_id, diag_item)) + } else { + false + } +} + pub fn last_path_segment<'tcx>(path: &QPath<'tcx>) -> &'tcx PathSegment<'tcx> { match *path { QPath::Resolved(_, path) => path.segments.last().expect("A path must have at least one segment"), @@ -558,6 +577,54 @@ pub fn trait_ref_of_method<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Optio None } +/// This method will return tuple of projection stack and root of the expression, +/// used in `can_mut_borrow_both`. +/// +/// For example, if `e` represents the `v[0].a.b[x]` +/// this method will return a tuple, composed of a `Vec` +/// containing the `Expr`s for `v[0], v[0].a, v[0].a.b, v[0].a.b[x]` +/// and a `Expr` for root of them, `v` +fn projection_stack<'a, 'hir>(mut e: &'a Expr<'hir>) -> (Vec<&'a Expr<'hir>>, &'a Expr<'hir>) { + let mut result = vec![]; + let root = loop { + match e.kind { + ExprKind::Index(ep, _) | ExprKind::Field(ep, _) => { + result.push(e); + e = ep; + }, + _ => break e, + }; + }; + result.reverse(); + (result, root) +} + +/// Checks if two expressions can be mutably borrowed simultaneously +/// and they aren't dependent on borrowing same thing twice +pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) -> bool { + let (s1, r1) = projection_stack(e1); + let (s2, r2) = projection_stack(e2); + if !eq_expr_value(cx, r1, r2) { + return true; + } + for (x1, x2) in s1.iter().zip(s2.iter()) { + match (&x1.kind, &x2.kind) { + (ExprKind::Field(_, i1), ExprKind::Field(_, i2)) => { + if i1 != i2 { + return true; + } + }, + (ExprKind::Index(_, i1), ExprKind::Index(_, i2)) => { + if !eq_expr_value(cx, i1, i2) { + return false; + } + }, + _ => return false, + } + } + false +} + /// Checks if the top level expression can be moved into a closure as is. pub fn can_move_expr_to_closure_no_visit(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, jump_targets: &[HirId]) -> bool { match expr.kind { diff --git a/src/tools/clippy/clippy_utils/src/source.rs b/src/tools/clippy/clippy_utils/src/source.rs index 4d49b43bde9..789079510c5 100644 --- a/src/tools/clippy/clippy_utils/src/source.rs +++ b/src/tools/clippy/clippy_utils/src/source.rs @@ -168,7 +168,7 @@ pub fn snippet<'a, T: LintContext>(cx: &T, span: Span, default: &'a str) -> Cow< snippet_opt(cx, span).map_or_else(|| Cow::Borrowed(default), From::from) } -/// Same as `snippet`, but it adapts the applicability level by following rules: +/// Same as [`snippet`], but it adapts the applicability level by following rules: /// /// - Applicability level `Unspecified` will never be changed. /// - If the span is inside a macro, change the applicability level to `MaybeIncorrect`. diff --git a/src/tools/clippy/clippy_utils/src/ty.rs b/src/tools/clippy/clippy_utils/src/ty.rs index e914dc1c222..4f9aaf396b8 100644 --- a/src/tools/clippy/clippy_utils/src/ty.rs +++ b/src/tools/clippy/clippy_utils/src/ty.rs @@ -114,7 +114,7 @@ pub fn has_iter_method(cx: &LateContext<'_>, probably_ref_ty: Ty<'_>) -> Option< /// Checks whether a type implements a trait. /// The function returns false in case the type contains an inference variable. -/// See also `get_trait_def_id`. +/// See also [`get_trait_def_id`](super::get_trait_def_id). pub fn implements_trait<'tcx>( cx: &LateContext<'tcx>, ty: Ty<'tcx>, diff --git a/src/tools/clippy/doc/basics.md b/src/tools/clippy/doc/basics.md index 43d3792f595..ff2e0417435 100644 --- a/src/tools/clippy/doc/basics.md +++ b/src/tools/clippy/doc/basics.md @@ -166,8 +166,8 @@ rustup component add clippy ``` > **DO NOT** install using `cargo install --path . --force` since this will overwrite rustup -[proxies](https://rust-lang.github.io/rustup/concepts/proxies.html). That is, `~/.cargo/bin/cargo-clippy` and -`~/.cargo/bin/clippy-driver` should be hard or soft links to `~/.cargo/bin/rustup`. You can repair these by running -`rustup update`. +> [proxies](https://rust-lang.github.io/rustup/concepts/proxies.html). That is, `~/.cargo/bin/cargo-clippy` and +> `~/.cargo/bin/clippy-driver` should be hard or soft links to `~/.cargo/bin/rustup`. You can repair these by running +> `rustup update`. [glossary]: https://rustc-dev-guide.rust-lang.org/appendix/glossary.html diff --git a/src/tools/clippy/rust-toolchain b/src/tools/clippy/rust-toolchain index bff657bc110..23887f17845 100644 --- a/src/tools/clippy/rust-toolchain +++ b/src/tools/clippy/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2021-07-29" +channel = "nightly-2021-08-12" components = ["llvm-tools-preview", "rustc-dev", "rust-src"] diff --git a/src/tools/clippy/tests/ui-toml/functions_maxlines/test.rs b/src/tools/clippy/tests/ui-toml/functions_maxlines/test.rs index a47677a1f3a..33a3ef75136 100644 --- a/src/tools/clippy/tests/ui-toml/functions_maxlines/test.rs +++ b/src/tools/clippy/tests/ui-toml/functions_maxlines/test.rs @@ -1,3 +1,5 @@ +// edition:2018 + #![warn(clippy::too_many_lines)] // This function should be considered one line. @@ -20,6 +22,20 @@ fn too_many_lines() { println!("This is bad."); } +// This should only fail once (#7517). +async fn async_too_many_lines() { + println!("This is bad."); + println!("This is bad."); +} + +// This should fail only once, without failing on the closure. +fn closure_too_many_lines() { + let _ = { + println!("This is bad."); + println!("This is bad."); + }; +} + // This should be considered one line. #[rustfmt::skip] fn comment_starts_after_code() { diff --git a/src/tools/clippy/tests/ui-toml/functions_maxlines/test.stderr b/src/tools/clippy/tests/ui-toml/functions_maxlines/test.stderr index a27ce945ca5..7551cac9f50 100644 --- a/src/tools/clippy/tests/ui-toml/functions_maxlines/test.stderr +++ b/src/tools/clippy/tests/ui-toml/functions_maxlines/test.stderr @@ -1,5 +1,5 @@ error: this function has too many lines (2/1) - --> $DIR/test.rs:18:1 + --> $DIR/test.rs:20:1 | LL | / fn too_many_lines() { LL | | println!("This is bad."); @@ -9,8 +9,28 @@ LL | | } | = note: `-D clippy::too-many-lines` implied by `-D warnings` +error: this function has too many lines (4/1) + --> $DIR/test.rs:26:1 + | +LL | / async fn async_too_many_lines() { +LL | | println!("This is bad."); +LL | | println!("This is bad."); +LL | | } + | |_^ + +error: this function has too many lines (4/1) + --> $DIR/test.rs:32:1 + | +LL | / fn closure_too_many_lines() { +LL | | let _ = { +LL | | println!("This is bad."); +LL | | println!("This is bad."); +LL | | }; +LL | | } + | |_^ + error: this function has too many lines (2/1) - --> $DIR/test.rs:38:1 + --> $DIR/test.rs:54:1 | LL | / fn comment_before_code() { LL | | let _ = "test"; @@ -19,5 +39,5 @@ LL | | the code but this line should still count. */ let _ = 5; LL | | } | |_^ -error: aborting due to 2 previous errors +error: aborting due to 4 previous errors diff --git a/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs b/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs index e9f042ddefc..5b4adc868df 100644 --- a/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs +++ b/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs @@ -32,13 +32,19 @@ macro_rules! type_pos { }; } +macro_rules! printlnfoo { + ($thing:expr) => { + println!("{}", $thing) + }; +} + #[rustfmt::skip] fn main() { let _ = vec! {1, 2, 3}; let _ = format!["ugh {} stop being such a good compiler", "hello"]; let _ = quote!(let x = 1;); let _ = quote::quote!(match match match); - let _ = test!(); + let _ = test!(); // trigger when macro def is inside our own crate let _ = vec![1,2,3]; let _ = quote::quote! {true || false}; @@ -49,4 +55,6 @@ fn main() { let _: type_pos!(usize) = vec![]; eprint!("test if user config overrides defaults"); + + printlnfoo!["test if printlnfoo is triggered by println"]; } diff --git a/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr b/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr index 86063a08280..87e962b9228 100644 --- a/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr +++ b/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr @@ -1,48 +1,48 @@ error: use of irregular braces for `vec!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:37:13 + --> $DIR/conf_nonstandard_macro_braces.rs:43:13 | LL | let _ = vec! {1, 2, 3}; | ^^^^^^^^^^^^^^ | = note: `-D clippy::nonstandard-macro-braces` implied by `-D warnings` help: consider writing `vec![1, 2, 3]` - --> $DIR/conf_nonstandard_macro_braces.rs:37:13 + --> $DIR/conf_nonstandard_macro_braces.rs:43:13 | LL | let _ = vec! {1, 2, 3}; | ^^^^^^^^^^^^^^ error: use of irregular braces for `format!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:38:13 + --> $DIR/conf_nonstandard_macro_braces.rs:44:13 | LL | let _ = format!["ugh {} stop being such a good compiler", "hello"]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider writing `format!("ugh () stop being such a good compiler", "hello")` - --> $DIR/conf_nonstandard_macro_braces.rs:38:13 + --> $DIR/conf_nonstandard_macro_braces.rs:44:13 | LL | let _ = format!["ugh {} stop being such a good compiler", "hello"]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: use of irregular braces for `quote!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:39:13 + --> $DIR/conf_nonstandard_macro_braces.rs:45:13 | LL | let _ = quote!(let x = 1;); | ^^^^^^^^^^^^^^^^^^ | help: consider writing `quote! {let x = 1;}` - --> $DIR/conf_nonstandard_macro_braces.rs:39:13 + --> $DIR/conf_nonstandard_macro_braces.rs:45:13 | LL | let _ = quote!(let x = 1;); | ^^^^^^^^^^^^^^^^^^ error: use of irregular braces for `quote::quote!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:40:13 + --> $DIR/conf_nonstandard_macro_braces.rs:46:13 | LL | let _ = quote::quote!(match match match); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider writing `quote::quote! {match match match}` - --> $DIR/conf_nonstandard_macro_braces.rs:40:13 + --> $DIR/conf_nonstandard_macro_braces.rs:46:13 | LL | let _ = quote::quote!(match match match); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -53,7 +53,7 @@ error: use of irregular braces for `vec!` macro LL | vec!{0, 0, 0} | ^^^^^^^^^^^^^ ... -LL | let _ = test!(); +LL | let _ = test!(); // trigger when macro def is inside our own crate | ------- in this macro invocation | help: consider writing `vec![0, 0, 0]` @@ -62,30 +62,30 @@ help: consider writing `vec![0, 0, 0]` LL | vec!{0, 0, 0} | ^^^^^^^^^^^^^ ... -LL | let _ = test!(); +LL | let _ = test!(); // trigger when macro def is inside our own crate | ------- in this macro invocation = note: this error originates in the macro `test` (in Nightly builds, run with -Z macro-backtrace for more info) error: use of irregular braces for `type_pos!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:49:12 + --> $DIR/conf_nonstandard_macro_braces.rs:55:12 | LL | let _: type_pos!(usize) = vec![]; | ^^^^^^^^^^^^^^^^ | help: consider writing `type_pos![usize]` - --> $DIR/conf_nonstandard_macro_braces.rs:49:12 + --> $DIR/conf_nonstandard_macro_braces.rs:55:12 | LL | let _: type_pos!(usize) = vec![]; | ^^^^^^^^^^^^^^^^ error: use of irregular braces for `eprint!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:51:5 + --> $DIR/conf_nonstandard_macro_braces.rs:57:5 | LL | eprint!("test if user config overrides defaults"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider writing `eprint!["test if user config overrides defaults"];` - --> $DIR/conf_nonstandard_macro_braces.rs:51:5 + --> $DIR/conf_nonstandard_macro_braces.rs:57:5 | LL | eprint!("test if user config overrides defaults"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/tools/clippy/tests/auxiliary/test_macro.rs b/src/tools/clippy/tests/ui/auxiliary/test_macro.rs index 624ca892add..624ca892add 100644 --- a/src/tools/clippy/tests/auxiliary/test_macro.rs +++ b/src/tools/clippy/tests/ui/auxiliary/test_macro.rs diff --git a/src/tools/clippy/tests/ui/extend_with_drain.fixed b/src/tools/clippy/tests/ui/extend_with_drain.fixed index 00170e649e2..e863870e7d6 100644 --- a/src/tools/clippy/tests/ui/extend_with_drain.fixed +++ b/src/tools/clippy/tests/ui/extend_with_drain.fixed @@ -41,7 +41,12 @@ fn main() { let mut heap = BinaryHeap::from(vec![1, 3]); let mut heap2 = BinaryHeap::from(vec![]); - heap2.extend(heap.drain()) + heap2.extend(heap.drain()); + + let mut x = vec![0, 1, 2, 3, 5]; + let ref_x = &mut x; + let mut y = Vec::new(); + y.append(ref_x); } fn return_vector() -> Vec<u8> { diff --git a/src/tools/clippy/tests/ui/extend_with_drain.rs b/src/tools/clippy/tests/ui/extend_with_drain.rs index d76458c3289..dcb36b5951c 100644 --- a/src/tools/clippy/tests/ui/extend_with_drain.rs +++ b/src/tools/clippy/tests/ui/extend_with_drain.rs @@ -41,7 +41,12 @@ fn main() { let mut heap = BinaryHeap::from(vec![1, 3]); let mut heap2 = BinaryHeap::from(vec![]); - heap2.extend(heap.drain()) + heap2.extend(heap.drain()); + + let mut x = vec![0, 1, 2, 3, 5]; + let ref_x = &mut x; + let mut y = Vec::new(); + y.extend(ref_x.drain(..)); } fn return_vector() -> Vec<u8> { diff --git a/src/tools/clippy/tests/ui/extend_with_drain.stderr b/src/tools/clippy/tests/ui/extend_with_drain.stderr index 57f344716a1..da14ddb25b3 100644 --- a/src/tools/clippy/tests/ui/extend_with_drain.stderr +++ b/src/tools/clippy/tests/ui/extend_with_drain.stderr @@ -18,5 +18,11 @@ error: use of `extend` instead of `append` for adding the full range of a second LL | vec11.extend(return_vector().drain(..)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec11.append(&mut return_vector())` -error: aborting due to 3 previous errors +error: use of `extend` instead of `append` for adding the full range of a second vector + --> $DIR/extend_with_drain.rs:49:5 + | +LL | y.extend(ref_x.drain(..)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `y.append(ref_x)` + +error: aborting due to 4 previous errors diff --git a/src/tools/clippy/tests/ui/implicit_hasher.rs b/src/tools/clippy/tests/ui/implicit_hasher.rs index fdcc9a33f55..97c26bc83ad 100644 --- a/src/tools/clippy/tests/ui/implicit_hasher.rs +++ b/src/tools/clippy/tests/ui/implicit_hasher.rs @@ -89,7 +89,7 @@ gen!(fn bar); // and should not cause an ICE // See #2707 #[macro_use] -#[path = "../auxiliary/test_macro.rs"] +#[path = "auxiliary/test_macro.rs"] pub mod test_macro; __implicit_hasher_test_macro!(impl<K, V> for HashMap<K, V> where V: test_macro::A); diff --git a/src/tools/clippy/tests/ui/map_flatten.fixed b/src/tools/clippy/tests/ui/map_flatten.fixed index 773b5914439..18846c898da 100644 --- a/src/tools/clippy/tests/ui/map_flatten.fixed +++ b/src/tools/clippy/tests/ui/map_flatten.fixed @@ -5,6 +5,7 @@ #![allow(clippy::missing_docs_in_private_items)] #![allow(clippy::map_identity)] #![allow(clippy::unnecessary_wraps)] +#![feature(result_flattening)] fn main() { // mapping to Option on Iterator @@ -23,4 +24,7 @@ fn main() { // mapping to Option on Option let _: Option<_> = (Some(Some(1))).and_then(|x| x); + + // mapping to Result on Result + let _: Result<_, &str> = (Ok(Ok(1))).and_then(|x| x); } diff --git a/src/tools/clippy/tests/ui/map_flatten.rs b/src/tools/clippy/tests/ui/map_flatten.rs index 578bd877267..01db27876da 100644 --- a/src/tools/clippy/tests/ui/map_flatten.rs +++ b/src/tools/clippy/tests/ui/map_flatten.rs @@ -5,6 +5,7 @@ #![allow(clippy::missing_docs_in_private_items)] #![allow(clippy::map_identity)] #![allow(clippy::unnecessary_wraps)] +#![feature(result_flattening)] fn main() { // mapping to Option on Iterator @@ -23,4 +24,7 @@ fn main() { // mapping to Option on Option let _: Option<_> = (Some(Some(1))).map(|x| x).flatten(); + + // mapping to Result on Result + let _: Result<_, &str> = (Ok(Ok(1))).map(|x| x).flatten(); } diff --git a/src/tools/clippy/tests/ui/map_flatten.stderr b/src/tools/clippy/tests/ui/map_flatten.stderr index 756e6e818ad..38457c8ea4d 100644 --- a/src/tools/clippy/tests/ui/map_flatten.stderr +++ b/src/tools/clippy/tests/ui/map_flatten.stderr @@ -1,5 +1,5 @@ error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:16:46 + --> $DIR/map_flatten.rs:17:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id)` @@ -7,34 +7,40 @@ LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().coll = note: `-D clippy::map-flatten` implied by `-D warnings` error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:17:46 + --> $DIR/map_flatten.rs:18:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_ref)` error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:18:46 + --> $DIR/map_flatten.rs:19:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_closure)` error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:19:46 + --> $DIR/map_flatten.rs:20:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(|x| x.checked_add(1))` error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:22:46 + --> $DIR/map_flatten.rs:23:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `.flat_map(|x| 0..x)` error: called `map(..).flatten()` on an `Option` - --> $DIR/map_flatten.rs:25:39 + --> $DIR/map_flatten.rs:26:39 | LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten(); | ^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `.and_then(|x| x)` -error: aborting due to 6 previous errors +error: called `map(..).flatten()` on an `Result` + --> $DIR/map_flatten.rs:29:41 + | +LL | let _: Result<_, &str> = (Ok(Ok(1))).map(|x| x).flatten(); + | ^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `.and_then(|x| x)` + +error: aborting due to 7 previous errors diff --git a/src/tools/clippy/tests/ui/never_loop.stderr b/src/tools/clippy/tests/ui/never_loop.stderr index c00b4c78cf2..f49b23924ef 100644 --- a/src/tools/clippy/tests/ui/never_loop.stderr +++ b/src/tools/clippy/tests/ui/never_loop.stderr @@ -75,6 +75,11 @@ LL | | _ => return, LL | | } LL | | } | |_____^ + | +help: if you need the first element of the iterator, try writing + | +LL | if let Some(x) = (0..10).next() { + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: this loop never actually loops --> $DIR/never_loop.rs:157:5 diff --git a/src/tools/clippy/tests/ui/no_effect.stderr b/src/tools/clippy/tests/ui/no_effect.stderr index 834b9056e31..6b24675ac2d 100644 --- a/src/tools/clippy/tests/ui/no_effect.stderr +++ b/src/tools/clippy/tests/ui/no_effect.stderr @@ -109,6 +109,12 @@ LL | 5..6; | ^^^^^ error: statement with no effect + --> $DIR/no_effect.rs:83:5 + | +LL | 5..=6; + | ^^^^^^ + +error: statement with no effect --> $DIR/no_effect.rs:84:5 | LL | [42, 55]; @@ -150,5 +156,5 @@ error: statement with no effect LL | FooString { s: s }; | ^^^^^^^^^^^^^^^^^^^ -error: aborting due to 25 previous errors +error: aborting due to 26 previous errors diff --git a/src/tools/clippy/tests/ui/or_fun_call.fixed b/src/tools/clippy/tests/ui/or_fun_call.fixed index 4390ff7dc30..c2f94d0e857 100644 --- a/src/tools/clippy/tests/ui/or_fun_call.fixed +++ b/src/tools/clippy/tests/ui/or_fun_call.fixed @@ -18,6 +18,19 @@ fn or_fun_call() { } } + struct FakeDefault; + impl FakeDefault { + fn default() -> Self { + FakeDefault + } + } + + impl Default for FakeDefault { + fn default() -> Self { + FakeDefault + } + } + enum Enum { A(i32), } @@ -53,6 +66,12 @@ fn or_fun_call() { let with_default_type = Some(1); with_default_type.unwrap_or_default(); + let self_default = None::<FakeDefault>; + self_default.unwrap_or_else(<FakeDefault>::default); + + let real_default = None::<FakeDefault>; + real_default.unwrap_or_default(); + let with_vec = Some(vec![1]); with_vec.unwrap_or_default(); diff --git a/src/tools/clippy/tests/ui/or_fun_call.rs b/src/tools/clippy/tests/ui/or_fun_call.rs index 75908c974cc..afaf92961b0 100644 --- a/src/tools/clippy/tests/ui/or_fun_call.rs +++ b/src/tools/clippy/tests/ui/or_fun_call.rs @@ -18,6 +18,19 @@ fn or_fun_call() { } } + struct FakeDefault; + impl FakeDefault { + fn default() -> Self { + FakeDefault + } + } + + impl Default for FakeDefault { + fn default() -> Self { + FakeDefault + } + } + enum Enum { A(i32), } @@ -53,6 +66,12 @@ fn or_fun_call() { let with_default_type = Some(1); with_default_type.unwrap_or(u64::default()); + let self_default = None::<FakeDefault>; + self_default.unwrap_or(<FakeDefault>::default()); + + let real_default = None::<FakeDefault>; + real_default.unwrap_or(<FakeDefault as Default>::default()); + let with_vec = Some(vec![1]); with_vec.unwrap_or(vec![]); diff --git a/src/tools/clippy/tests/ui/or_fun_call.stderr b/src/tools/clippy/tests/ui/or_fun_call.stderr index 9905029ce91..b2bcbd38c2d 100644 --- a/src/tools/clippy/tests/ui/or_fun_call.stderr +++ b/src/tools/clippy/tests/ui/or_fun_call.stderr @@ -1,5 +1,5 @@ error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:33:19 + --> $DIR/or_fun_call.rs:46:19 | LL | with_const_fn.unwrap_or(Duration::from_secs(5)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Duration::from_secs(5))` @@ -7,130 +7,142 @@ LL | with_const_fn.unwrap_or(Duration::from_secs(5)); = note: `-D clippy::or-fun-call` implied by `-D warnings` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:36:22 + --> $DIR/or_fun_call.rs:49:22 | LL | with_constructor.unwrap_or(make()); | ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(make)` error: use of `unwrap_or` followed by a call to `new` - --> $DIR/or_fun_call.rs:39:5 + --> $DIR/or_fun_call.rs:52:5 | LL | with_new.unwrap_or(Vec::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_new.unwrap_or_default()` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:42:21 + --> $DIR/or_fun_call.rs:55:21 | LL | with_const_args.unwrap_or(Vec::with_capacity(12)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Vec::with_capacity(12))` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:45:14 + --> $DIR/or_fun_call.rs:58:14 | LL | with_err.unwrap_or(make()); | ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| make())` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:48:19 + --> $DIR/or_fun_call.rs:61:19 | LL | with_err_args.unwrap_or(Vec::with_capacity(12)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| Vec::with_capacity(12))` error: use of `unwrap_or` followed by a call to `default` - --> $DIR/or_fun_call.rs:51:5 + --> $DIR/or_fun_call.rs:64:5 | LL | with_default_trait.unwrap_or(Default::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_trait.unwrap_or_default()` error: use of `unwrap_or` followed by a call to `default` - --> $DIR/or_fun_call.rs:54:5 + --> $DIR/or_fun_call.rs:67:5 | LL | with_default_type.unwrap_or(u64::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_type.unwrap_or_default()` +error: use of `unwrap_or` followed by a function call + --> $DIR/or_fun_call.rs:70:18 + | +LL | self_default.unwrap_or(<FakeDefault>::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(<FakeDefault>::default)` + +error: use of `unwrap_or` followed by a call to `default` + --> $DIR/or_fun_call.rs:73:5 + | +LL | real_default.unwrap_or(<FakeDefault as Default>::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `real_default.unwrap_or_default()` + error: use of `unwrap_or` followed by a call to `new` - --> $DIR/or_fun_call.rs:57:5 + --> $DIR/or_fun_call.rs:76:5 | LL | with_vec.unwrap_or(vec![]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_vec.unwrap_or_default()` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:60:21 + --> $DIR/or_fun_call.rs:79:21 | LL | without_default.unwrap_or(Foo::new()); | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(Foo::new)` error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:63:19 + --> $DIR/or_fun_call.rs:82:19 | LL | map.entry(42).or_insert(String::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)` error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:66:23 + --> $DIR/or_fun_call.rs:85:23 | LL | map_vec.entry(42).or_insert(vec![]); | ^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(Vec::new)` error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:69:21 + --> $DIR/or_fun_call.rs:88:21 | LL | btree.entry(42).or_insert(String::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)` error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:72:25 + --> $DIR/or_fun_call.rs:91:25 | LL | btree_vec.entry(42).or_insert(vec![]); | ^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(Vec::new)` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:75:21 + --> $DIR/or_fun_call.rs:94:21 | LL | let _ = stringy.unwrap_or("".to_owned()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:83:21 + --> $DIR/or_fun_call.rs:102:21 | LL | let _ = Some(1).unwrap_or(map[&1]); | ^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| map[&1])` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:85:21 + --> $DIR/or_fun_call.rs:104:21 | LL | let _ = Some(1).unwrap_or(map[&1]); | ^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| map[&1])` error: use of `or` followed by a function call - --> $DIR/or_fun_call.rs:109:35 + --> $DIR/or_fun_call.rs:128:35 | LL | let _ = Some("a".to_string()).or(Some("b".to_string())); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some("b".to_string()))` error: use of `or` followed by a function call - --> $DIR/or_fun_call.rs:113:10 + --> $DIR/or_fun_call.rs:132:10 | LL | .or(Some(Bar(b, Duration::from_secs(2)))); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some(Bar(b, Duration::from_secs(2))))` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:141:14 + --> $DIR/or_fun_call.rs:160:14 | LL | None.unwrap_or(s.as_mut_vec()); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| s.as_mut_vec())` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:146:14 + --> $DIR/or_fun_call.rs:165:14 | LL | None.unwrap_or(unsafe { s.as_mut_vec() }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { s.as_mut_vec() })` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:148:14 + --> $DIR/or_fun_call.rs:167:14 | LL | None.unwrap_or( unsafe { s.as_mut_vec() } ); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { s.as_mut_vec() })` -error: aborting due to 22 previous errors +error: aborting due to 24 previous errors diff --git a/src/tools/clippy/tests/ui/similar_names.rs b/src/tools/clippy/tests/ui/similar_names.rs index 2b1bc1f4859..daa07341457 100644 --- a/src/tools/clippy/tests/ui/similar_names.rs +++ b/src/tools/clippy/tests/ui/similar_names.rs @@ -76,6 +76,9 @@ fn main() { // names often used in win32 code (for example WindowProc) let wparam: i32; let lparam: i32; + + let iter: i32; + let item: i32; } fn foo() { diff --git a/src/tools/clippy/tests/ui/similar_names.stderr b/src/tools/clippy/tests/ui/similar_names.stderr index b24accd962a..f621595abae 100644 --- a/src/tools/clippy/tests/ui/similar_names.stderr +++ b/src/tools/clippy/tests/ui/similar_names.stderr @@ -72,13 +72,13 @@ LL | let parser: i32; | ^^^^^^ error: binding's name is too similar to existing binding - --> $DIR/similar_names.rs:85:16 + --> $DIR/similar_names.rs:88:16 | LL | bpple: sprang, | ^^^^^^ | note: existing binding defined here - --> $DIR/similar_names.rs:84:16 + --> $DIR/similar_names.rs:87:16 | LL | apple: spring, | ^^^^^^ diff --git a/src/tools/clippy/tests/ui/swap.fixed b/src/tools/clippy/tests/ui/swap.fixed index 0f8f839a0d5..ef518359ec5 100644 --- a/src/tools/clippy/tests/ui/swap.fixed +++ b/src/tools/clippy/tests/ui/swap.fixed @@ -6,6 +6,7 @@ clippy::no_effect, clippy::redundant_clone, redundant_semicolons, + dead_code, unused_assignments )] @@ -20,9 +21,7 @@ struct Bar { fn field() { let mut bar = Bar { a: 1, b: 2 }; - let temp = bar.a; - bar.a = bar.b; - bar.b = temp; + std::mem::swap(&mut bar.a, &mut bar.b); let mut baz = vec![bar.clone(), bar.clone()]; let temp = baz[0].a; @@ -51,6 +50,7 @@ fn unswappable_slice() { foo[1][0] = temp; // swap(foo[0][1], foo[1][0]) would fail + // this could use split_at_mut and mem::swap, but that is not much simpler. } fn vec() { @@ -60,13 +60,54 @@ fn vec() { foo.swap(0, 1); } +fn xor_swap_locals() { + // This is an xor-based swap of local variables. + let mut a = 0; + let mut b = 1; + std::mem::swap(&mut a, &mut b) +} + +fn xor_field_swap() { + // This is an xor-based swap of fields in a struct. + let mut bar = Bar { a: 0, b: 1 }; + std::mem::swap(&mut bar.a, &mut bar.b) +} + +fn xor_slice_swap() { + // This is an xor-based swap of a slice + let foo = &mut [1, 2]; + foo.swap(0, 1) +} + +fn xor_no_swap() { + // This is a sequence of xor-assignment statements that doesn't result in a swap. + let mut a = 0; + let mut b = 1; + let mut c = 2; + a ^= b; + b ^= c; + a ^= c; + c ^= a; +} + +fn xor_unswappable_slice() { + let foo = &mut [vec![1, 2], vec![3, 4]]; + foo[0][1] ^= foo[1][0]; + foo[1][0] ^= foo[0][0]; + foo[0][1] ^= foo[1][0]; + + // swap(foo[0][1], foo[1][0]) would fail + // this could use split_at_mut and mem::swap, but that is not much simpler. +} + +fn distinct_slice() { + let foo = &mut [vec![1, 2], vec![3, 4]]; + let bar = &mut [vec![1, 2], vec![3, 4]]; + std::mem::swap(&mut foo[0][1], &mut bar[1][0]); +} + #[rustfmt::skip] fn main() { - field(); - array(); - slice(); - unswappable_slice(); - vec(); let mut a = 42; let mut b = 1337; diff --git a/src/tools/clippy/tests/ui/swap.rs b/src/tools/clippy/tests/ui/swap.rs index 5763d9e82d4..8518659ccf3 100644 --- a/src/tools/clippy/tests/ui/swap.rs +++ b/src/tools/clippy/tests/ui/swap.rs @@ -6,6 +6,7 @@ clippy::no_effect, clippy::redundant_clone, redundant_semicolons, + dead_code, unused_assignments )] @@ -55,6 +56,7 @@ fn unswappable_slice() { foo[1][0] = temp; // swap(foo[0][1], foo[1][0]) would fail + // this could use split_at_mut and mem::swap, but that is not much simpler. } fn vec() { @@ -66,13 +68,62 @@ fn vec() { foo.swap(0, 1); } +fn xor_swap_locals() { + // This is an xor-based swap of local variables. + let mut a = 0; + let mut b = 1; + a ^= b; + b ^= a; + a ^= b; +} + +fn xor_field_swap() { + // This is an xor-based swap of fields in a struct. + let mut bar = Bar { a: 0, b: 1 }; + bar.a ^= bar.b; + bar.b ^= bar.a; + bar.a ^= bar.b; +} + +fn xor_slice_swap() { + // This is an xor-based swap of a slice + let foo = &mut [1, 2]; + foo[0] ^= foo[1]; + foo[1] ^= foo[0]; + foo[0] ^= foo[1]; +} + +fn xor_no_swap() { + // This is a sequence of xor-assignment statements that doesn't result in a swap. + let mut a = 0; + let mut b = 1; + let mut c = 2; + a ^= b; + b ^= c; + a ^= c; + c ^= a; +} + +fn xor_unswappable_slice() { + let foo = &mut [vec![1, 2], vec![3, 4]]; + foo[0][1] ^= foo[1][0]; + foo[1][0] ^= foo[0][0]; + foo[0][1] ^= foo[1][0]; + + // swap(foo[0][1], foo[1][0]) would fail + // this could use split_at_mut and mem::swap, but that is not much simpler. +} + +fn distinct_slice() { + let foo = &mut [vec![1, 2], vec![3, 4]]; + let bar = &mut [vec![1, 2], vec![3, 4]]; + let temp = foo[0][1]; + foo[0][1] = bar[1][0]; + bar[1][0] = temp; +} + #[rustfmt::skip] fn main() { - field(); - array(); - slice(); - unswappable_slice(); - vec(); let mut a = 42; let mut b = 1337; diff --git a/src/tools/clippy/tests/ui/swap.stderr b/src/tools/clippy/tests/ui/swap.stderr index f49bcfedf3a..614d16ced40 100644 --- a/src/tools/clippy/tests/ui/swap.stderr +++ b/src/tools/clippy/tests/ui/swap.stderr @@ -1,15 +1,24 @@ +error: this looks like you are swapping `bar.a` and `bar.b` manually + --> $DIR/swap.rs:24:5 + | +LL | / let temp = bar.a; +LL | | bar.a = bar.b; +LL | | bar.b = temp; + | |________________^ help: try: `std::mem::swap(&mut bar.a, &mut bar.b)` + | + = note: `-D clippy::manual-swap` implied by `-D warnings` + = note: or maybe you should use `std::mem::replace`? + error: this looks like you are swapping elements of `foo` manually - --> $DIR/swap.rs:35:5 + --> $DIR/swap.rs:36:5 | LL | / let temp = foo[0]; LL | | foo[0] = foo[1]; LL | | foo[1] = temp; | |_________________^ help: try: `foo.swap(0, 1)` - | - = note: `-D clippy::manual-swap` implied by `-D warnings` error: this looks like you are swapping elements of `foo` manually - --> $DIR/swap.rs:44:5 + --> $DIR/swap.rs:45:5 | LL | / let temp = foo[0]; LL | | foo[0] = foo[1]; @@ -17,7 +26,7 @@ LL | | foo[1] = temp; | |_________________^ help: try: `foo.swap(0, 1)` error: this looks like you are swapping elements of `foo` manually - --> $DIR/swap.rs:62:5 + --> $DIR/swap.rs:64:5 | LL | / let temp = foo[0]; LL | | foo[0] = foo[1]; @@ -25,7 +34,41 @@ LL | | foo[1] = temp; | |_________________^ help: try: `foo.swap(0, 1)` error: this looks like you are swapping `a` and `b` manually - --> $DIR/swap.rs:83:7 + --> $DIR/swap.rs:75:5 + | +LL | / a ^= b; +LL | | b ^= a; +LL | | a ^= b; + | |___________^ help: try: `std::mem::swap(&mut a, &mut b)` + +error: this looks like you are swapping `bar.a` and `bar.b` manually + --> $DIR/swap.rs:83:5 + | +LL | / bar.a ^= bar.b; +LL | | bar.b ^= bar.a; +LL | | bar.a ^= bar.b; + | |___________________^ help: try: `std::mem::swap(&mut bar.a, &mut bar.b)` + +error: this looks like you are swapping elements of `foo` manually + --> $DIR/swap.rs:91:5 + | +LL | / foo[0] ^= foo[1]; +LL | | foo[1] ^= foo[0]; +LL | | foo[0] ^= foo[1]; + | |_____________________^ help: try: `foo.swap(0, 1)` + +error: this looks like you are swapping `foo[0][1]` and `bar[1][0]` manually + --> $DIR/swap.rs:120:5 + | +LL | / let temp = foo[0][1]; +LL | | foo[0][1] = bar[1][0]; +LL | | bar[1][0] = temp; + | |____________________^ help: try: `std::mem::swap(&mut foo[0][1], &mut bar[1][0])` + | + = note: or maybe you should use `std::mem::replace`? + +error: this looks like you are swapping `a` and `b` manually + --> $DIR/swap.rs:134:7 | LL | ; let t = a; | _______^ @@ -36,7 +79,7 @@ LL | | b = t; = note: or maybe you should use `std::mem::replace`? error: this looks like you are swapping `c.0` and `a` manually - --> $DIR/swap.rs:92:7 + --> $DIR/swap.rs:143:7 | LL | ; let t = c.0; | _______^ @@ -47,7 +90,7 @@ LL | | a = t; = note: or maybe you should use `std::mem::replace`? error: this looks like you are trying to swap `a` and `b` - --> $DIR/swap.rs:80:5 + --> $DIR/swap.rs:131:5 | LL | / a = b; LL | | b = a; @@ -57,7 +100,7 @@ LL | | b = a; = note: or maybe you should use `std::mem::replace`? error: this looks like you are trying to swap `c.0` and `a` - --> $DIR/swap.rs:89:5 + --> $DIR/swap.rs:140:5 | LL | / c.0 = a; LL | | a = c.0; @@ -65,5 +108,5 @@ LL | | a = c.0; | = note: or maybe you should use `std::mem::replace`? -error: aborting due to 7 previous errors +error: aborting due to 12 previous errors diff --git a/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.rs b/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.rs index e7e0a31febc..1a0123803a3 100644 --- a/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.rs +++ b/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.rs @@ -58,6 +58,8 @@ impl Foo { fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} fn bad2(x: &u32, y: &Foo, z: &Baz) {} + + fn bad_issue7518(self, other: &Self) {} } impl AsRef<u32> for Foo { diff --git a/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.stderr b/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.stderr index 2b0005bbff1..9c4c49ceac4 100644 --- a/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.stderr +++ b/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.stderr @@ -65,40 +65,46 @@ LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:72:16 + --> $DIR/trivially_copy_pass_by_ref.rs:62:35 + | +LL | fn bad_issue7518(self, other: &Self) {} + | ^^^^^ help: consider passing by value instead: `Self` + +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> $DIR/trivially_copy_pass_by_ref.rs:74:16 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:72:25 + --> $DIR/trivially_copy_pass_by_ref.rs:74:25 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:72:34 + --> $DIR/trivially_copy_pass_by_ref.rs:74:34 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:76:34 + --> $DIR/trivially_copy_pass_by_ref.rs:78:34 | LL | fn trait_method(&self, _foo: &Foo); | ^^^^ help: consider passing by value instead: `Foo` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:108:21 + --> $DIR/trivially_copy_pass_by_ref.rs:110:21 | LL | fn foo_never(x: &i32) { | ^^^^ help: consider passing by value instead: `i32` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:113:15 + --> $DIR/trivially_copy_pass_by_ref.rs:115:15 | LL | fn foo(x: &i32) { | ^^^^ help: consider passing by value instead: `i32` -error: aborting due to 16 previous errors +error: aborting due to 17 previous errors diff --git a/src/tools/clippy/tests/ui/unwrap_or_else_default.fixed b/src/tools/clippy/tests/ui/unwrap_or_else_default.fixed new file mode 100644 index 00000000000..7ac3f426c97 --- /dev/null +++ b/src/tools/clippy/tests/ui/unwrap_or_else_default.fixed @@ -0,0 +1,71 @@ +// run-rustfix + +#![warn(clippy::unwrap_or_else_default)] +#![allow(dead_code)] +#![allow(clippy::unnecessary_wraps)] + +/// Checks implementation of the `UNWRAP_OR_ELSE_DEFAULT` lint. +fn unwrap_or_else_default() { + struct Foo; + + impl Foo { + fn new() -> Foo { + Foo + } + + // fake default, we should not trigger on this + fn default() -> Foo { + Foo + } + } + + struct HasDefaultAndDuplicate; + + impl HasDefaultAndDuplicate { + fn default() -> Self { + HasDefaultAndDuplicate + } + } + + impl Default for HasDefaultAndDuplicate { + fn default() -> Self { + HasDefaultAndDuplicate + } + } + + enum Enum { + A(), + } + + fn make<T, V>(_: V) -> T { + unimplemented!(); + } + + let with_enum = Some(Enum::A()); + with_enum.unwrap_or_else(Enum::A); + + let with_new = Some(vec![1]); + with_new.unwrap_or_else(Vec::new); + + let with_err: Result<_, ()> = Ok(vec![1]); + with_err.unwrap_or_else(make); + + // should not be changed + let with_fake_default = None::<Foo>; + with_fake_default.unwrap_or_else(Foo::default); + + // should not be changed + let with_fake_default2 = None::<HasDefaultAndDuplicate>; + with_fake_default2.unwrap_or_else(<HasDefaultAndDuplicate>::default); + + let with_real_default = None::<HasDefaultAndDuplicate>; + with_real_default.unwrap_or_default(); + + let with_default_trait = Some(1); + with_default_trait.unwrap_or_default(); + + let with_default_type = Some(1); + with_default_type.unwrap_or_default(); +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/unwrap_or_else_default.rs b/src/tools/clippy/tests/ui/unwrap_or_else_default.rs new file mode 100644 index 00000000000..82b727a039e --- /dev/null +++ b/src/tools/clippy/tests/ui/unwrap_or_else_default.rs @@ -0,0 +1,71 @@ +// run-rustfix + +#![warn(clippy::unwrap_or_else_default)] +#![allow(dead_code)] +#![allow(clippy::unnecessary_wraps)] + +/// Checks implementation of the `UNWRAP_OR_ELSE_DEFAULT` lint. +fn unwrap_or_else_default() { + struct Foo; + + impl Foo { + fn new() -> Foo { + Foo + } + + // fake default, we should not trigger on this + fn default() -> Foo { + Foo + } + } + + struct HasDefaultAndDuplicate; + + impl HasDefaultAndDuplicate { + fn default() -> Self { + HasDefaultAndDuplicate + } + } + + impl Default for HasDefaultAndDuplicate { + fn default() -> Self { + HasDefaultAndDuplicate + } + } + + enum Enum { + A(), + } + + fn make<T, V>(_: V) -> T { + unimplemented!(); + } + + let with_enum = Some(Enum::A()); + with_enum.unwrap_or_else(Enum::A); + + let with_new = Some(vec![1]); + with_new.unwrap_or_else(Vec::new); + + let with_err: Result<_, ()> = Ok(vec![1]); + with_err.unwrap_or_else(make); + + // should not be changed + let with_fake_default = None::<Foo>; + with_fake_default.unwrap_or_else(Foo::default); + + // should not be changed + let with_fake_default2 = None::<HasDefaultAndDuplicate>; + with_fake_default2.unwrap_or_else(<HasDefaultAndDuplicate>::default); + + let with_real_default = None::<HasDefaultAndDuplicate>; + with_real_default.unwrap_or_else(<HasDefaultAndDuplicate as Default>::default); + + let with_default_trait = Some(1); + with_default_trait.unwrap_or_else(Default::default); + + let with_default_type = Some(1); + with_default_type.unwrap_or_else(u64::default); +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/unwrap_or_else_default.stderr b/src/tools/clippy/tests/ui/unwrap_or_else_default.stderr new file mode 100644 index 00000000000..feb215b09f6 --- /dev/null +++ b/src/tools/clippy/tests/ui/unwrap_or_else_default.stderr @@ -0,0 +1,22 @@ +error: use of `.unwrap_or_else(..)` to construct default value + --> $DIR/unwrap_or_else_default.rs:62:5 + | +LL | with_real_default.unwrap_or_else(<HasDefaultAndDuplicate as Default>::default); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_real_default.unwrap_or_default()` + | + = note: `-D clippy::unwrap-or-else-default` implied by `-D warnings` + +error: use of `.unwrap_or_else(..)` to construct default value + --> $DIR/unwrap_or_else_default.rs:65:5 + | +LL | with_default_trait.unwrap_or_else(Default::default); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_default_trait.unwrap_or_default()` + +error: use of `.unwrap_or_else(..)` to construct default value + --> $DIR/unwrap_or_else_default.rs:68:5 + | +LL | with_default_type.unwrap_or_else(u64::default); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `with_default_type.unwrap_or_default()` + +error: aborting due to 3 previous errors + diff --git a/src/tools/clippy/tests/ui/while_let_on_iterator.fixed b/src/tools/clippy/tests/ui/while_let_on_iterator.fixed index 52e80ceee83..cdcdd808c94 100644 --- a/src/tools/clippy/tests/ui/while_let_on_iterator.fixed +++ b/src/tools/clippy/tests/ui/while_let_on_iterator.fixed @@ -334,6 +334,38 @@ fn issue7249() { x(); } +fn issue7510() { + let mut it = 0..10; + let it = &mut it; + // Needs to reborrow `it` as the binding isn't mutable + for x in &mut *it { + if x % 2 == 0 { + break; + } + } + println!("{}", it.next().unwrap()); + + struct S<T>(T); + let mut it = 0..10; + let it = S(&mut it); + // Needs to reborrow `it.0` as the binding isn't mutable + for x in &mut *it.0 { + if x % 2 == 0 { + break; + } + } + println!("{}", it.0.next().unwrap()); +} + +fn exact_match_with_single_field() { + struct S<T>(T); + let mut s = S(0..10); + // Don't lint. `s.0` is used inside the loop. + while let Some(_) = s.0.next() { + let _ = &mut s.0; + } +} + fn main() { let mut it = 0..20; for _ in it { diff --git a/src/tools/clippy/tests/ui/while_let_on_iterator.rs b/src/tools/clippy/tests/ui/while_let_on_iterator.rs index 5078a3c9028..72f34257d1f 100644 --- a/src/tools/clippy/tests/ui/while_let_on_iterator.rs +++ b/src/tools/clippy/tests/ui/while_let_on_iterator.rs @@ -334,6 +334,38 @@ fn issue7249() { x(); } +fn issue7510() { + let mut it = 0..10; + let it = &mut it; + // Needs to reborrow `it` as the binding isn't mutable + while let Some(x) = it.next() { + if x % 2 == 0 { + break; + } + } + println!("{}", it.next().unwrap()); + + struct S<T>(T); + let mut it = 0..10; + let it = S(&mut it); + // Needs to reborrow `it.0` as the binding isn't mutable + while let Some(x) = it.0.next() { + if x % 2 == 0 { + break; + } + } + println!("{}", it.0.next().unwrap()); +} + +fn exact_match_with_single_field() { + struct S<T>(T); + let mut s = S(0..10); + // Don't lint. `s.0` is used inside the loop. + while let Some(_) = s.0.next() { + let _ = &mut s.0; + } +} + fn main() { let mut it = 0..20; while let Some(..) = it.next() { diff --git a/src/tools/clippy/tests/ui/while_let_on_iterator.stderr b/src/tools/clippy/tests/ui/while_let_on_iterator.stderr index cb0afeae15e..ff9b08996da 100644 --- a/src/tools/clippy/tests/ui/while_let_on_iterator.stderr +++ b/src/tools/clippy/tests/ui/while_let_on_iterator.stderr @@ -111,10 +111,22 @@ LL | while let Some(x) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:339:5 + --> $DIR/while_let_on_iterator.rs:341:5 + | +LL | while let Some(x) = it.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut *it` + +error: this loop could be written as a `for` loop + --> $DIR/while_let_on_iterator.rs:352:5 + | +LL | while let Some(x) = it.0.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut *it.0` + +error: this loop could be written as a `for` loop + --> $DIR/while_let_on_iterator.rs:371:5 | LL | while let Some(..) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in it` -error: aborting due to 19 previous errors +error: aborting due to 21 previous errors |