diff options
115 files changed, 3683 insertions, 1041 deletions
diff --git a/.github/workflows/lintcheck.yml b/.github/workflows/lintcheck.yml new file mode 100644 index 00000000000..91c98b3a256 --- /dev/null +++ b/.github/workflows/lintcheck.yml @@ -0,0 +1,118 @@ +name: Lintcheck + +on: pull_request + +env: + RUST_BACKTRACE: 1 + CARGO_INCREMENTAL: 0 + +concurrency: + # For a given workflow, if we push to the same PR, cancel all previous builds on that PR. + group: "${{ github.workflow }}-${{ github.event.pull_request.number}}" + cancel-in-progress: true + +jobs: + # Runs lintcheck on the PR's target branch and stores the results as an artifact + base: + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 2 + + # HEAD is the generated merge commit `refs/pull/N/merge` between the PR and `master`, `HEAD^` + # being the commit from `master` that is the base of the merge + - name: Checkout base + run: git checkout HEAD^ + + # Use the lintcheck from the PR to generate the JSON in case the PR modifies lintcheck in some + # way + - name: Checkout current lintcheck + run: | + rm -rf lintcheck + git checkout ${{ github.sha }} -- lintcheck + + - name: Cache lintcheck bin + id: cache-lintcheck-bin + uses: actions/cache@v4 + with: + path: target/debug/lintcheck + key: lintcheck-bin-${{ hashfiles('lintcheck/**') }} + + - name: Build lintcheck + if: steps.cache-lintcheck-bin.outputs.cache-hit != 'true' + run: cargo build --manifest-path=lintcheck/Cargo.toml + + - name: Create cache key + id: key + run: echo "key=lintcheck-base-${{ hashfiles('lintcheck/**') }}-$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" + + - name: Cache results JSON + id: cache-json + uses: actions/cache@v4 + with: + path: lintcheck-logs/lintcheck_crates_logs.json + key: ${{ steps.key.outputs.key }} + + - name: Run lintcheck + if: steps.cache-json.outputs.cache-hit != 'true' + run: ./target/debug/lintcheck --format json + + - name: Upload base JSON + uses: actions/upload-artifact@v4 + with: + name: base + path: lintcheck-logs/lintcheck_crates_logs.json + + # Runs lintcheck on the PR and stores the results as an artifact + head: + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Cache lintcheck bin + id: cache-lintcheck-bin + uses: actions/cache@v4 + with: + path: target/debug/lintcheck + key: lintcheck-bin-${{ hashfiles('lintcheck/**') }} + + - name: Build lintcheck + if: steps.cache-lintcheck-bin.outputs.cache-hit != 'true' + run: cargo build --manifest-path=lintcheck/Cargo.toml + + - name: Run lintcheck + run: ./target/debug/lintcheck --format json + + - name: Upload head JSON + uses: actions/upload-artifact@v4 + with: + name: head + path: lintcheck-logs/lintcheck_crates_logs.json + + # Retrieves the head and base JSON results and prints the diff to the GH actions step summary + diff: + runs-on: ubuntu-latest + + needs: [base, head] + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Restore lintcheck bin + uses: actions/cache/restore@v4 + with: + path: target/debug/lintcheck + key: lintcheck-bin-${{ hashfiles('lintcheck/**') }} + fail-on-cache-miss: true + + - name: Download JSON + uses: actions/download-artifact@v4 + + - name: Diff results + run: ./target/debug/lintcheck diff {base,head}/lintcheck_crates_logs.json >> $GITHUB_STEP_SUMMARY diff --git a/CHANGELOG.md b/CHANGELOG.md index d1cd87473ce..70ef2c79364 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5362,6 +5362,7 @@ Released 2018-09-13 [`extra_unused_type_parameters`]: https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_type_parameters [`fallible_impl_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#fallible_impl_from [`field_reassign_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default +[`field_scoped_visibility_modifiers`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_scoped_visibility_modifiers [`filetype_is_file`]: https://rust-lang.github.io/rust-clippy/master/index.html#filetype_is_file [`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map [`filter_map_bool_then`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_bool_then @@ -5520,6 +5521,7 @@ Released 2018-09-13 [`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map [`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten [`manual_hash_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one +[`manual_inspect`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect [`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed [`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check [`manual_is_finite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_finite @@ -5915,6 +5917,7 @@ Released 2018-09-13 [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations [`unnecessary_literal_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_literal_unwrap [`unnecessary_map_on_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_map_on_constructor +[`unnecessary_min_or_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_min_or_max [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation [`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 6dad3463aa4..cfd34c7d2a7 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -454,7 +454,7 @@ default configuration of Clippy. By default, any configuration will replace the * `doc-valid-idents = ["ClipPy"]` would replace the default list with `["ClipPy"]`. * `doc-valid-idents = ["ClipPy", ".."]` would append `ClipPy` to the default list. -**Default Value:** `["KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "DirectX", "ECMAScript", "GPLv2", "GPLv3", "GitHub", "GitLab", "IPv4", "IPv6", "ClojureScript", "CoffeeScript", "JavaScript", "PureScript", "TypeScript", "WebAssembly", "NaN", "NaNs", "OAuth", "GraphQL", "OCaml", "OpenDNS", "OpenGL", "OpenMP", "OpenSSH", "OpenSSL", "OpenStreetMap", "OpenTelemetry", "WebGL", "WebGL2", "WebGPU", "TensorFlow", "TrueType", "iOS", "macOS", "FreeBSD", "TeX", "LaTeX", "BibTeX", "BibLaTeX", "MinGW", "CamelCase"]` +**Default Value:** `["KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "DevOps", "DirectX", "ECMAScript", "GPLv2", "GPLv3", "GitHub", "GitLab", "IPv4", "IPv6", "ClojureScript", "CoffeeScript", "JavaScript", "PureScript", "TypeScript", "WebAssembly", "NaN", "NaNs", "OAuth", "GraphQL", "OCaml", "OpenDNS", "OpenGL", "OpenMP", "OpenSSH", "OpenSSL", "OpenStreetMap", "OpenTelemetry", "WebGL", "WebGL2", "WebGPU", "TensorFlow", "TrueType", "iOS", "macOS", "FreeBSD", "TeX", "LaTeX", "BibTeX", "BibLaTeX", "MinGW", "CamelCase"]` --- **Affected lints:** @@ -695,6 +695,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio * [`manual_is_ascii_check`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check) * [`manual_let_else`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else) * [`manual_non_exhaustive`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive) +* [`manual_pattern_char_comparison`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison) * [`manual_range_contains`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains) * [`manual_rem_euclid`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid) * [`manual_retain`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain) diff --git a/book/src/usage.md b/book/src/usage.md index 36448e4cccf..7a0be6994fe 100644 --- a/book/src/usage.md +++ b/book/src/usage.md @@ -36,7 +36,7 @@ You can configure lint levels on the command line by adding cargo clippy -- -Aclippy::style -Wclippy::double_neg -Dclippy::perf ``` -For [CI] all warnings can be elevated to errors which will inturn fail +For [CI] all warnings can be elevated to errors which will in turn fail the build and cause Clippy to exit with a code other than `0`. ``` diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 279193e516e..dbab3b106a8 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -18,6 +18,7 @@ use std::{cmp, env, fmt, fs, io}; #[rustfmt::skip] const DEFAULT_DOC_VALID_IDENTS: &[&str] = &[ "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", + "DevOps", "DirectX", "ECMAScript", "GPLv2", "GPLv3", @@ -265,7 +266,7 @@ define_Conf! { /// /// Suppress lints whenever the suggested change would cause breakage for other crates. (avoid_breaking_exported_api: bool = true), - /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, 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, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP, MANUAL_C_STR_LITERALS, ASSIGNING_CLONES, LEGACY_NUMERIC_CONSTANTS, ALLOW_ATTRIBUTES, ALLOW_ATTRIBUTES_WITHOUT_REASON. + /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, 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, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP, MANUAL_C_STR_LITERALS, ASSIGNING_CLONES, LEGACY_NUMERIC_CONSTANTS, MANUAL_PATTERN_CHAR_COMPARISON, ALLOW_ATTRIBUTES, ALLOW_ATTRIBUTES_WITHOUT_REASON. /// /// The minimum rust version that the project supports. Defaults to the `rust-version` field in `Cargo.toml` #[default_text = ""] diff --git a/clippy_config/src/msrvs.rs b/clippy_config/src/msrvs.rs index 8bdb5b317e5..a5761d3270c 100644 --- a/clippy_config/src/msrvs.rs +++ b/clippy_config/src/msrvs.rs @@ -19,7 +19,7 @@ macro_rules! msrv_aliases { msrv_aliases! { 1,81,0 { LINT_REASONS_STABILIZATION } 1,77,0 { C_STR_LITERALS } - 1,76,0 { PTR_FROM_REF } + 1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT } 1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE } 1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN } 1,68,0 { PATH_MAIN_SEPARATOR_STR } diff --git a/clippy_lints/src/arc_with_non_send_sync.rs b/clippy_lints/src/arc_with_non_send_sync.rs index 38933897389..d57ab539fff 100644 --- a/clippy_lints/src/arc_with_non_send_sync.rs +++ b/clippy_lints/src/arc_with_non_send_sync.rs @@ -17,7 +17,7 @@ declare_clippy_lint! { /// `Arc<T>` is a thread-safe `Rc<T>` and guarantees that updates to the reference counter /// use atomic operations. To send an `Arc<T>` across thread boundaries and /// share ownership between multiple threads, `T` must be [both `Send` and `Sync`](https://doc.rust-lang.org/std/sync/struct.Arc.html#thread-safety), - /// so either `T` should be made `Send + Sync` or an `Rc` should be used instead of an `Arc` + /// so either `T` should be made `Send + Sync` or an `Rc` should be used instead of an `Arc`. /// /// ### Example /// ```no_run diff --git a/clippy_lints/src/assigning_clones.rs b/clippy_lints/src/assigning_clones.rs index e94a6f3e3fc..05ea74b0d53 100644 --- a/clippy_lints/src/assigning_clones.rs +++ b/clippy_lints/src/assigning_clones.rs @@ -1,16 +1,18 @@ use clippy_config::msrvs::{self, Msrv}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::macros::HirNode; +use clippy_utils::mir::{enclosing_mir, PossibleBorrowerMap}; use clippy_utils::sugg::Sugg; use clippy_utils::{is_trait_method, local_is_initialized, path_to_local}; use rustc_errors::Applicability; use rustc_hir::{self as hir, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::mir; use rustc_middle::ty::{self, Instance, Mutability}; use rustc_session::impl_lint_pass; use rustc_span::def_id::DefId; use rustc_span::symbol::sym; -use rustc_span::{ExpnKind, SyntaxContext}; +use rustc_span::{ExpnKind, Span, SyntaxContext}; declare_clippy_lint! { /// ### What it does @@ -144,6 +146,7 @@ fn extract_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option< }; Some(CallCandidate { + span: expr.span, target, kind, method_def_id: resolved_method.def_id(), @@ -215,6 +218,10 @@ fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallC return false; }; + if clone_source_borrows_from_dest(cx, lhs, call.span) { + return false; + } + // Now take a look if the impl block defines an implementation for the method that we're interested // in. If not, then we're using a default implementation, which is not interesting, so we will // not suggest the lint. @@ -222,6 +229,74 @@ fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallC implemented_fns.contains_key(&provided_fn.def_id) } +/// Checks if the data being cloned borrows from the place that is being assigned to: +/// +/// ``` +/// let mut s = String::new(); +/// let s2 = &s; +/// s = s2.to_owned(); +/// ``` +/// +/// This cannot be written `s2.clone_into(&mut s)` because it has conflicting borrows. +fn clone_source_borrows_from_dest(cx: &LateContext<'_>, lhs: &Expr<'_>, call_span: Span) -> bool { + /// If this basic block only exists to drop a local as part of an assignment, returns its + /// successor. Otherwise returns the basic block that was passed in. + fn skip_drop_block(mir: &mir::Body<'_>, bb: mir::BasicBlock) -> mir::BasicBlock { + if let mir::TerminatorKind::Drop { target, .. } = mir.basic_blocks[bb].terminator().kind { + target + } else { + bb + } + } + + let Some(mir) = enclosing_mir(cx.tcx, lhs.hir_id) else { + return false; + }; + let PossibleBorrowerMap { map: borrow_map, .. } = PossibleBorrowerMap::new(cx, mir); + + // The operation `dest = src.to_owned()` in MIR is split up across 3 blocks *if* the type has `Drop` + // code. For types that don't, the second basic block is simply skipped. + // For the doc example above that would be roughly: + // + // bb0: + // s2 = &s + // s_temp = ToOwned::to_owned(move s2) -> bb1 + // + // bb1: + // drop(s) -> bb2 // drop the old string + // + // bb2: + // s = s_temp + for bb in mir.basic_blocks.iter() { + let terminator = bb.terminator(); + + // Look for the to_owned/clone call. + if terminator.source_info.span != call_span { + continue; + } + + if let mir::TerminatorKind::Call { ref args, target: Some(assign_bb), .. } = terminator.kind + && let [source] = &**args + && let mir::Operand::Move(source) = &source.node + && let assign_bb = skip_drop_block(mir, assign_bb) + // Skip any storage statements as they are just noise + && let Some(assignment) = mir.basic_blocks[assign_bb].statements + .iter() + .find(|stmt| { + !matches!(stmt.kind, mir::StatementKind::StorageDead(_) | mir::StatementKind::StorageLive(_)) + }) + && let mir::StatementKind::Assign(box (borrowed, _)) = &assignment.kind + && let Some(borrowers) = borrow_map.get(&borrowed.local) + && borrowers.contains(source.local) + { + return true; + } + + return false; + } + false +} + fn suggest<'tcx>( cx: &LateContext<'tcx>, ctxt: SyntaxContext, @@ -255,6 +330,7 @@ enum TargetTrait { #[derive(Debug)] struct CallCandidate<'tcx> { + span: Span, target: TargetTrait, kind: CallKind<'tcx>, // DefId of the called method from an impl block that implements the target trait diff --git a/clippy_lints/src/attrs/allow_attributes.rs b/clippy_lints/src/attrs/allow_attributes.rs index c5b6980b0b9..df9994086cd 100644 --- a/clippy_lints/src/attrs/allow_attributes.rs +++ b/clippy_lints/src/attrs/allow_attributes.rs @@ -1,17 +1,17 @@ +use super::ALLOW_ATTRIBUTES; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::is_from_proc_macro; use rustc_ast::{AttrStyle, Attribute}; use rustc_errors::Applicability; use rustc_lint::{LateContext, LintContext}; use rustc_middle::lint::in_external_macro; -use super::ALLOW_ATTRIBUTES; // Separate each crate's features. pub fn check<'cx>(cx: &LateContext<'cx>, attr: &'cx Attribute) { if !in_external_macro(cx.sess(), attr.span) && let AttrStyle::Outer = attr.style && let Some(ident) = attr.ident() - && !is_from_proc_macro(cx, &attr) + && !is_from_proc_macro(cx, attr) { span_lint_and_sugg( cx, diff --git a/clippy_lints/src/attrs/allow_attributes_without_reason.rs b/clippy_lints/src/attrs/allow_attributes_without_reason.rs index 8bf985a36c7..4b42616a636 100644 --- a/clippy_lints/src/attrs/allow_attributes_without_reason.rs +++ b/clippy_lints/src/attrs/allow_attributes_without_reason.rs @@ -17,7 +17,7 @@ pub(super) fn check<'cx>(cx: &LateContext<'cx>, name: Symbol, items: &[NestedMet } // Check if the attribute is in an external macro and therefore out of the developer's control - if in_external_macro(cx.sess(), attr.span) || is_from_proc_macro(cx, &attr) { + if in_external_macro(cx.sess(), attr.span) || is_from_proc_macro(cx, attr) { return; } diff --git a/clippy_lints/src/attrs/mod.rs b/clippy_lints/src/attrs/mod.rs index da19f17998a..8ec60314cc9 100644 --- a/clippy_lints/src/attrs/mod.rs +++ b/clippy_lints/src/attrs/mod.rs @@ -1,7 +1,7 @@ //! checks for attributes -mod allow_attributes_without_reason; mod allow_attributes; +mod allow_attributes_without_reason; mod blanket_clippy_restriction_lints; mod deprecated_cfg_attr; mod deprecated_semver; @@ -505,6 +505,7 @@ pub struct Attributes { } impl_lint_pass!(Attributes => [ + ALLOW_ATTRIBUTES, ALLOW_ATTRIBUTES_WITHOUT_REASON, INLINE_ALWAYS, DEPRECATED_SEMVER, @@ -534,15 +535,12 @@ impl<'tcx> LateLintPass<'tcx> for Attributes { if is_lint_level(ident.name, attr.id) { blanket_clippy_restriction_lints::check(cx, ident.name, items); } - if matches!(ident.name, sym::allow) { - if self.msrv.meets(msrvs::LINT_REASONS_STABILIZATION) { - allow_attributes::check(cx, attr); - } + if matches!(ident.name, sym::allow) && self.msrv.meets(msrvs::LINT_REASONS_STABILIZATION) { + allow_attributes::check(cx, attr); } - if matches!(ident.name, sym::allow | sym::expect) { - if self.msrv.meets(msrvs::LINT_REASONS_STABILIZATION) { - allow_attributes_without_reason::check(cx, ident.name, items, attr); - } + if matches!(ident.name, sym::allow | sym::expect) && self.msrv.meets(msrvs::LINT_REASONS_STABILIZATION) + { + allow_attributes_without_reason::check(cx, ident.name, items, attr); } if items.is_empty() || !attr.has_name(sym::deprecated) { return; diff --git a/clippy_lints/src/casts/ref_as_ptr.rs b/clippy_lints/src/casts/ref_as_ptr.rs index f42bafce4dd..5f48b8bd206 100644 --- a/clippy_lints/src/casts/ref_as_ptr.rs +++ b/clippy_lints/src/casts/ref_as_ptr.rs @@ -22,9 +22,9 @@ pub(super) fn check<'tcx>( if matches!(cast_from.kind(), ty::Ref(..)) && let ty::RawPtr(_, to_mutbl) = cast_to.kind() - && let Some(use_cx) = expr_use_ctxt(cx, expr) + && let use_cx = expr_use_ctxt(cx, expr) // TODO: only block the lint if `cast_expr` is a temporary - && !matches!(use_cx.node, ExprUseNode::LetStmt(_) | ExprUseNode::ConstStatic(_)) + && !matches!(use_cx.use_node(cx), ExprUseNode::LetStmt(_) | ExprUseNode::ConstStatic(_)) { let core_or_std = if is_no_std_crate(cx) { "core" } else { "std" }; let fn_name = match to_mutbl { diff --git a/clippy_lints/src/cognitive_complexity.rs b/clippy_lints/src/cognitive_complexity.rs index e41abf42234..60815f4f2af 100644 --- a/clippy_lints/src/cognitive_complexity.rs +++ b/clippy_lints/src/cognitive_complexity.rs @@ -1,7 +1,7 @@ //! calculate cognitive complexity and warn about overly complex functions use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::source::snippet_opt; +use clippy_utils::source::{IntoSpan, SpanRangeExt}; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::visitors::for_each_expr_without_closures; use clippy_utils::{get_async_fn_body, is_async_fn, LimitStack}; @@ -12,7 +12,7 @@ use rustc_hir::{Body, Expr, ExprKind, FnDecl}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::impl_lint_pass; use rustc_span::def_id::LocalDefId; -use rustc_span::{sym, BytePos, Span}; +use rustc_span::{sym, Span}; declare_clippy_lint! { /// ### What it does @@ -50,7 +50,6 @@ impl CognitiveComplexity { impl_lint_pass!(CognitiveComplexity => [COGNITIVE_COMPLEXITY]); impl CognitiveComplexity { - #[expect(clippy::cast_possible_truncation)] fn check<'tcx>( &mut self, cx: &LateContext<'tcx>, @@ -100,17 +99,12 @@ impl CognitiveComplexity { FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => ident.span, FnKind::Closure => { let header_span = body_span.with_hi(decl.output.span().lo()); - let pos = snippet_opt(cx, header_span).and_then(|snip| { - let low_offset = snip.find('|')?; - let high_offset = 1 + snip.get(low_offset + 1..)?.find('|')?; - let low = header_span.lo() + BytePos(low_offset as u32); - let high = low + BytePos(high_offset as u32 + 1); - - Some((low, high)) - }); - - if let Some((low, high)) = pos { - Span::new(low, high, header_span.ctxt(), header_span.parent()) + #[expect(clippy::range_plus_one)] + if let Some(range) = header_span.map_range(cx, |src, range| { + let mut idxs = src.get(range.clone())?.match_indices('|'); + Some(range.start + idxs.next()?.0..range.start + idxs.next()?.0 + 1) + }) { + range.with_ctxt(header_span.ctxt()) } else { return; } diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index 480df675d75..d896452be92 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then}; -use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, snippet, snippet_opt}; +use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, snippet, IntoSpan, SpanRangeExt}; use clippy_utils::ty::{needs_ordered_drop, InteriorMut}; use clippy_utils::visitors::for_each_expr_without_closures; use clippy_utils::{ @@ -14,7 +14,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; use rustc_span::hygiene::walk_chain; use rustc_span::source_map::SourceMap; -use rustc_span::{BytePos, Span, Symbol}; +use rustc_span::{Span, Symbol}; use std::borrow::Cow; declare_clippy_lint! { @@ -266,12 +266,12 @@ fn lint_branches_sharing_code<'tcx>( let span = span.with_hi(last_block.span.hi()); // Improve formatting if the inner block has indention (i.e. normal Rust formatting) - let test_span = Span::new(span.lo() - BytePos(4), span.lo(), span.ctxt(), span.parent()); - let span = if snippet_opt(cx, test_span).map_or(false, |snip| snip == " ") { - span.with_lo(test_span.lo()) - } else { - span - }; + let span = span + .map_range(cx, |src, range| { + (range.start > 4 && src.get(range.start - 4..range.start)? == " ") + .then_some(range.start - 4..range.end) + }) + .map_or(span, |range| range.with_ctxt(span.ctxt())); (span, suggestion.to_string()) }); diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 76a0e450e59..638de5e818c 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -178,6 +178,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::explicit_write::EXPLICIT_WRITE_INFO, crate::extra_unused_type_parameters::EXTRA_UNUSED_TYPE_PARAMETERS_INFO, crate::fallible_impl_from::FALLIBLE_IMPL_FROM_INFO, + crate::field_scoped_visibility_modifiers::FIELD_SCOPED_VISIBILITY_MODIFIERS_INFO, crate::float_literal::EXCESSIVE_PRECISION_INFO, crate::float_literal::LOSSY_FLOAT_LITERAL_INFO, crate::floating_point_arithmetic::IMPRECISE_FLOPS_INFO, @@ -402,6 +403,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::MANUAL_C_STR_LITERALS_INFO, crate::methods::MANUAL_FILTER_MAP_INFO, crate::methods::MANUAL_FIND_MAP_INFO, + crate::methods::MANUAL_INSPECT_INFO, crate::methods::MANUAL_IS_VARIANT_AND_INFO, crate::methods::MANUAL_NEXT_BACK_INFO, crate::methods::MANUAL_OK_OR_INFO, @@ -469,6 +471,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::UNNECESSARY_JOIN_INFO, crate::methods::UNNECESSARY_LAZY_EVALUATIONS_INFO, crate::methods::UNNECESSARY_LITERAL_UNWRAP_INFO, + crate::methods::UNNECESSARY_MIN_OR_MAX_INFO, crate::methods::UNNECESSARY_RESULT_MAP_OR_ELSE_INFO, crate::methods::UNNECESSARY_SORT_BY_INFO, crate::methods::UNNECESSARY_TO_OWNED_INFO, diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index f451758c335..a115f8d0631 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -6,7 +6,7 @@ use clippy_utils::{ expr_use_ctxt, get_parent_expr, is_block_like, is_lint_allowed, path_to_local, DefinedTy, ExprUseNode, }; use core::mem; -use rustc_ast::util::parser::{PREC_UNAMBIGUOUS, PREC_PREFIX}; +use rustc_ast::util::parser::{PREC_PREFIX, PREC_UNAMBIGUOUS}; use rustc_data_structures::fx::FxIndexMap; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_ty, Visitor}; @@ -260,18 +260,13 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { (None, kind) => { let expr_ty = typeck.expr_ty(expr); let use_cx = expr_use_ctxt(cx, expr); - let adjusted_ty = match &use_cx { - Some(use_cx) => match use_cx.adjustments { - [.., a] => a.target, - _ => expr_ty, - }, - _ => typeck.expr_ty_adjusted(expr), - }; + let adjusted_ty = use_cx.adjustments.last().map_or(expr_ty, |a| a.target); - match (use_cx, kind) { - (Some(use_cx), RefOp::Deref) => { + match kind { + RefOp::Deref if use_cx.same_ctxt => { + let use_node = use_cx.use_node(cx); let sub_ty = typeck.expr_ty(sub_expr); - if let ExprUseNode::FieldAccess(name) = use_cx.node + if let ExprUseNode::FieldAccess(name) = use_node && !use_cx.moved_before_use && !ty_contains_field(sub_ty, name.name) { @@ -288,9 +283,9 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { } else if sub_ty.is_ref() // Linting method receivers would require verifying that name lookup // would resolve the same way. This is complicated by trait methods. - && !use_cx.node.is_recv() - && let Some(ty) = use_cx.node.defined_ty(cx) - && TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return()).is_deref_stable() + && !use_node.is_recv() + && let Some(ty) = use_node.defined_ty(cx) + && TyCoercionStability::for_defined_ty(cx, ty, use_node.is_return()).is_deref_stable() { self.state = Some(( State::ExplicitDeref { mutability: None }, @@ -301,7 +296,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { )); } }, - (_, RefOp::Method { mutbl, is_ufcs }) + RefOp::Method { mutbl, is_ufcs } if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id) // Allow explicit deref in method chains. e.g. `foo.deref().bar()` && (is_ufcs || !in_postfix_position(cx, expr)) => @@ -319,7 +314,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { }, )); }, - (Some(use_cx), RefOp::AddrOf(mutability)) => { + RefOp::AddrOf(mutability) if use_cx.same_ctxt => { // Find the number of times the borrow is auto-derefed. let mut iter = use_cx.adjustments.iter(); let mut deref_count = 0usize; @@ -338,10 +333,11 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { }; }; - let stability = use_cx.node.defined_ty(cx).map_or(TyCoercionStability::None, |ty| { - TyCoercionStability::for_defined_ty(cx, ty, use_cx.node.is_return()) + let use_node = use_cx.use_node(cx); + let stability = use_node.defined_ty(cx).map_or(TyCoercionStability::None, |ty| { + TyCoercionStability::for_defined_ty(cx, ty, use_node.is_return()) }); - let can_auto_borrow = match use_cx.node { + let can_auto_borrow = match use_node { ExprUseNode::FieldAccess(_) if !use_cx.moved_before_use && matches!(sub_expr.kind, ExprKind::Field(..)) => { @@ -353,7 +349,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { // deref through `ManuallyDrop<_>` will not compile. !adjust_derefs_manually_drop(use_cx.adjustments, expr_ty) }, - ExprUseNode::Callee | ExprUseNode::FieldAccess(_) => true, + ExprUseNode::Callee | ExprUseNode::FieldAccess(_) if !use_cx.moved_before_use => true, ExprUseNode::MethodArg(hir_id, _, 0) if !use_cx.moved_before_use => { // Check for calls to trait methods where the trait is implemented // on a reference. @@ -363,9 +359,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { // priority. if let Some(fn_id) = typeck.type_dependent_def_id(hir_id) && let Some(trait_id) = cx.tcx.trait_of_item(fn_id) - && let arg_ty = cx - .tcx - .erase_regions(use_cx.adjustments.last().map_or(expr_ty, |a| a.target)) + && let arg_ty = cx.tcx.erase_regions(adjusted_ty) && let ty::Ref(_, sub_ty, _) = *arg_ty.kind() && let args = typeck.node_args_opt(hir_id).map(|args| &args[1..]).unwrap_or_default() @@ -443,7 +437,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { count: deref_count - required_refs, msg, stability, - for_field_access: if let ExprUseNode::FieldAccess(name) = use_cx.node + for_field_access: if let ExprUseNode::FieldAccess(name) = use_node && !use_cx.moved_before_use { Some(name.name) @@ -453,7 +447,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { }), StateData { first_expr: expr, - adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target), + adjusted_ty, }, )); } else if stability.is_deref_stable() @@ -465,12 +459,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { State::Borrow { mutability }, StateData { first_expr: expr, - adjusted_ty: use_cx.adjustments.last().map_or(expr_ty, |a| a.target), + adjusted_ty, }, )); } }, - (None, _) | (_, RefOp::Method { .. }) => (), + _ => {}, } }, ( diff --git a/clippy_lints/src/field_scoped_visibility_modifiers.rs b/clippy_lints/src/field_scoped_visibility_modifiers.rs new file mode 100644 index 00000000000..bb74e345703 --- /dev/null +++ b/clippy_lints/src/field_scoped_visibility_modifiers.rs @@ -0,0 +1,75 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use rustc_ast::ast::{Item, ItemKind, VisibilityKind}; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_session::declare_lint_pass; + +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of scoped visibility modifiers, like `pub(crate)`, on fields. These + /// make a field visible within a scope between public and private. + /// + /// ### Why restrict this? + /// Scoped visibility modifiers cause a field to be accessible within some scope between + /// public and private, potentially within an entire crate. This allows for fields to be + /// non-private while upholding internal invariants, but can be a code smell. Scoped visibility + /// requires checking a greater area, potentially an entire crate, to verify that an invariant + /// is upheld, and global analysis requires a lot of effort. + /// + /// ### Example + /// ```no_run + /// pub mod public_module { + /// struct MyStruct { + /// pub(crate) first_field: bool, + /// pub(super) second_field: bool + /// } + /// } + /// ``` + /// Use instead: + /// ```no_run + /// pub mod public_module { + /// struct MyStruct { + /// first_field: bool, + /// second_field: bool + /// } + /// impl MyStruct { + /// pub(crate) fn get_first_field(&self) -> bool { + /// self.first_field + /// } + /// pub(super) fn get_second_field(&self) -> bool { + /// self.second_field + /// } + /// } + /// } + /// ``` + #[clippy::version = "1.78.0"] + pub FIELD_SCOPED_VISIBILITY_MODIFIERS, + restriction, + "checks for usage of a scoped visibility modifier, like `pub(crate)`, on fields" +} + +declare_lint_pass!(FieldScopedVisibilityModifiers => [FIELD_SCOPED_VISIBILITY_MODIFIERS]); + +impl EarlyLintPass for FieldScopedVisibilityModifiers { + fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { + let ItemKind::Struct(ref st, _) = item.kind else { + return; + }; + for field in st.fields() { + let VisibilityKind::Restricted { path, .. } = &field.vis.kind else { + continue; + }; + if !path.segments.is_empty() && path.segments[0].ident.name == rustc_span::symbol::kw::SelfLower { + // pub(self) is equivalent to not using pub at all, so we ignore it + continue; + } + span_lint_and_help( + cx, + FIELD_SCOPED_VISIBILITY_MODIFIERS, + field.vis.span, + "scoped visibility modifier on a field", + None, + "consider making the field private and adding a scoped visibility method for it", + ); + } + } +} diff --git a/clippy_lints/src/from_over_into.rs b/clippy_lints/src/from_over_into.rs index 1933a00891b..93527bcdf5c 100644 --- a/clippy_lints/src/from_over_into.rs +++ b/clippy_lints/src/from_over_into.rs @@ -181,6 +181,9 @@ fn convert_to_from( let from = snippet_opt(cx, self_ty.span)?; let into = snippet_opt(cx, target_ty.span)?; + let return_type = matches!(sig.decl.output, FnRetTy::Return(_)) + .then_some(String::from("Self")) + .unwrap_or_default(); let mut suggestions = vec![ // impl Into<T> for U -> impl From<T> for U // ~~~~ ~~~~ @@ -197,13 +200,10 @@ fn convert_to_from( // fn into([mut] self) -> T -> fn into([mut] v: T) -> T // ~~~~ ~~~~ (self_ident.span, format!("val: {from}")), - ]; - - if let FnRetTy::Return(_) = sig.decl.output { // fn into(self) -> T -> fn into(self) -> Self // ~ ~~~~ - suggestions.push((sig.decl.output.span(), String::from("Self"))); - } + (sig.decl.output.span(), return_type), + ]; let mut finder = SelfFinder { cx, diff --git a/clippy_lints/src/implicit_hasher.rs b/clippy_lints/src/implicit_hasher.rs index ca830af3b2f..344a04e6e7e 100644 --- a/clippy_lints/src/implicit_hasher.rs +++ b/clippy_lints/src/implicit_hasher.rs @@ -14,7 +14,7 @@ use rustc_span::symbol::sym; use rustc_span::Span; use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; -use clippy_utils::source::{snippet, snippet_opt}; +use clippy_utils::source::{snippet, IntoSpan, SpanRangeExt}; use clippy_utils::ty::is_type_diagnostic_item; declare_clippy_lint! { @@ -59,10 +59,8 @@ declare_clippy_lint! { declare_lint_pass!(ImplicitHasher => [IMPLICIT_HASHER]); impl<'tcx> LateLintPass<'tcx> for ImplicitHasher { - #[expect(clippy::cast_possible_truncation, clippy::too_many_lines)] + #[expect(clippy::too_many_lines)] fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { - use rustc_span::BytePos; - fn suggestion( cx: &LateContext<'_>, diag: &mut Diag<'_, ()>, @@ -123,10 +121,11 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitHasher { } let generics_suggestion_span = impl_.generics.span.substitute_dummy({ - let pos = snippet_opt(cx, item.span.until(target.span())) - .and_then(|snip| Some(item.span.lo() + BytePos(snip.find("impl")? as u32 + 4))); - if let Some(pos) = pos { - Span::new(pos, pos, item.span.ctxt(), item.span.parent()) + let range = (item.span.lo()..target.span().lo()).map_range(cx, |src, range| { + Some(src.get(range.clone())?.find("impl")? + 4..range.end) + }); + if let Some(range) = range { + range.with_ctxt(item.span.ctxt()) } else { return; } @@ -163,21 +162,16 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitHasher { continue; } let generics_suggestion_span = generics.span.substitute_dummy({ - let pos = snippet_opt( - cx, - Span::new( - item.span.lo(), - body.params[0].pat.span.lo(), - item.span.ctxt(), - item.span.parent(), - ), - ) - .and_then(|snip| { - let i = snip.find("fn")?; - Some(item.span.lo() + BytePos((i + snip[i..].find('(')?) as u32)) - }) - .expect("failed to create span for type parameters"); - Span::new(pos, pos, item.span.ctxt(), item.span.parent()) + let range = (item.span.lo()..body.params[0].pat.span.lo()).map_range(cx, |src, range| { + let (pre, post) = src.get(range.clone())?.split_once("fn")?; + let pos = post.find('(')? + pre.len() + 2; + Some(pos..pos) + }); + if let Some(range) = range { + range.with_ctxt(item.span.ctxt()) + } else { + return; + } }); let mut ctr_vis = ImplicitHasherConstructorVisitor::new(cx, target); diff --git a/clippy_lints/src/implicit_return.rs b/clippy_lints/src/implicit_return.rs index 2f543781c44..a102b434cfa 100644 --- a/clippy_lints/src/implicit_return.rs +++ b/clippy_lints/src/implicit_return.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_hir_and_then; use clippy_utils::source::{snippet_with_applicability, snippet_with_context, walk_span_to_context}; use clippy_utils::visitors::for_each_expr_without_closures; -use clippy_utils::{get_async_fn_body, is_async_fn}; +use clippy_utils::{get_async_fn_body, is_async_fn, is_from_proc_macro}; use core::ops::ControlFlow; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; @@ -245,6 +245,10 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitReturn { } else { body.value }; + + if is_from_proc_macro(cx, expr) { + return; + } lint_implicit_returns(cx, expr, expr.span.ctxt(), None); } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 63461c14d77..ed0ff51fbf6 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -6,7 +6,6 @@ #![feature(if_let_guard)] #![feature(iter_intersperse)] #![feature(let_chains)] -#![cfg_attr(bootstrap, feature(lint_reasons))] #![feature(never_type)] #![feature(rustc_private)] #![feature(stmt_expr_attributes)] @@ -137,6 +136,7 @@ mod exit; mod explicit_write; mod extra_unused_type_parameters; mod fallible_impl_from; +mod field_scoped_visibility_modifiers; mod float_literal; mod floating_point_arithmetic; mod format; @@ -1168,7 +1168,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { ..Default::default() }) }); - store.register_late_pass(|_| Box::new(string_patterns::StringPatterns)); + store.register_late_pass(move |_| Box::new(string_patterns::StringPatterns::new(msrv()))); + store.register_early_pass(|| Box::new(field_scoped_visibility_modifiers::FieldScopedVisibilityModifiers)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs index 08682942153..64ea591993d 100644 --- a/clippy_lints/src/loops/mod.rs +++ b/clippy_lints/src/loops/mod.rs @@ -356,10 +356,10 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for loops which have a range bound that is a mutable variable + /// Checks for loops with a range bound that is a mutable variable. /// /// ### Why is this bad? - /// One might think that modifying the mutable variable changes the loop bounds + /// One might think that modifying the mutable variable changes the loop bounds. It doesn't. /// /// ### Known problems /// False positive when mutation is followed by a `break`, but the `break` is not immediately @@ -381,7 +381,7 @@ declare_clippy_lint! { /// let mut foo = 42; /// for i in 0..foo { /// foo -= 1; - /// println!("{}", i); // prints numbers from 0 to 42, not 0 to 21 + /// println!("{i}"); // prints numbers from 0 to 41, not 0 to 21 /// } /// ``` #[clippy::version = "pre 1.29.0"] diff --git a/clippy_lints/src/manual_unwrap_or_default.rs b/clippy_lints/src/manual_unwrap_or_default.rs index 17399fb2cc2..58b2ebebbf0 100644 --- a/clippy_lints/src/manual_unwrap_or_default.rs +++ b/clippy_lints/src/manual_unwrap_or_default.rs @@ -53,6 +53,7 @@ declare_lint_pass!(ManualUnwrapOrDefault => [MANUAL_UNWRAP_OR_DEFAULT]); fn get_some<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<HirId> { if let PatKind::TupleStruct(QPath::Resolved(_, path), &[pat], _) = pat.kind + && let PatKind::Binding(_, pat_id, _, _) = pat.kind && let Some(def_id) = path.res.opt_def_id() // Since it comes from a pattern binding, we need to get the parent to actually match // against it. @@ -60,13 +61,7 @@ fn get_some<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<HirId> { && (cx.tcx.lang_items().get(LangItem::OptionSome) == Some(def_id) || cx.tcx.lang_items().get(LangItem::ResultOk) == Some(def_id)) { - let mut bindings = Vec::new(); - pat.each_binding(|_, id, _, _| bindings.push(id)); - if let &[id] = bindings.as_slice() { - Some(id) - } else { - None - } + Some(pat_id) } else { None } diff --git a/clippy_lints/src/matches/manual_unwrap_or.rs b/clippy_lints/src/matches/manual_unwrap_or.rs index 9edd6c95404..0940fc3219b 100644 --- a/clippy_lints/src/matches/manual_unwrap_or.rs +++ b/clippy_lints/src/matches/manual_unwrap_or.rs @@ -3,46 +3,78 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{indent_of, reindent_multiline, snippet_opt}; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::usage::contains_return_break_continue_macro; -use clippy_utils::{is_res_lang_ctor, path_to_local_id, sugg}; +use clippy_utils::{is_res_lang_ctor, path_to_local_id, peel_blocks, sugg}; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::LangItem::{OptionNone, ResultErr}; -use rustc_hir::{Arm, Expr, PatKind}; +use rustc_hir::{Arm, Expr, Pat, PatKind}; use rustc_lint::LateContext; +use rustc_middle::ty::Ty; use rustc_span::sym; use super::MANUAL_UNWRAP_OR; -pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, scrutinee: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>]) { +pub(super) fn check_match<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + scrutinee: &'tcx Expr<'_>, + arms: &'tcx [Arm<'_>], +) { let ty = cx.typeck_results().expr_ty(scrutinee); - if let Some(ty_name) = if is_type_diagnostic_item(cx, ty, sym::Option) { + if let Some((or_arm, unwrap_arm)) = applicable_or_arm(cx, arms) { + check_and_lint(cx, expr, unwrap_arm.pat, scrutinee, unwrap_arm.body, or_arm.body, ty); + } +} + +pub(super) fn check_if_let<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + let_pat: &'tcx Pat<'_>, + let_expr: &'tcx Expr<'_>, + then_expr: &'tcx Expr<'_>, + else_expr: &'tcx Expr<'_>, +) { + let ty = cx.typeck_results().expr_ty(let_expr); + check_and_lint(cx, expr, let_pat, let_expr, then_expr, peel_blocks(else_expr), ty); +} + +fn check_and_lint<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + let_pat: &'tcx Pat<'_>, + let_expr: &'tcx Expr<'_>, + then_expr: &'tcx Expr<'_>, + else_expr: &'tcx Expr<'_>, + ty: Ty<'tcx>, +) { + if let PatKind::TupleStruct(ref qpath, [unwrap_pat], _) = let_pat.kind + && let Res::Def(DefKind::Ctor(..), ctor_id) = cx.qpath_res(qpath, let_pat.hir_id) + && let Some(variant_id) = cx.tcx.opt_parent(ctor_id) + && (cx.tcx.lang_items().option_some_variant() == Some(variant_id) + || cx.tcx.lang_items().result_ok_variant() == Some(variant_id)) + && let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind + && path_to_local_id(peel_blocks(then_expr), binding_hir_id) + && cx.typeck_results().expr_adjustments(then_expr).is_empty() + && let Some(ty_name) = find_type_name(cx, ty) + && let Some(or_body_snippet) = snippet_opt(cx, else_expr.span) + && let Some(indent) = indent_of(cx, expr.span) + && constant_simple(cx, cx.typeck_results(), else_expr).is_some() + { + lint(cx, expr, let_expr, ty_name, or_body_snippet, indent); + } +} + +fn find_type_name<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<&'static str> { + if is_type_diagnostic_item(cx, ty, sym::Option) { Some("Option") } else if is_type_diagnostic_item(cx, ty, sym::Result) { Some("Result") } else { None - } && let Some(or_arm) = applicable_or_arm(cx, arms) - && let Some(or_body_snippet) = snippet_opt(cx, or_arm.body.span) - && let Some(indent) = indent_of(cx, expr.span) - && constant_simple(cx, cx.typeck_results(), or_arm.body).is_some() - { - let reindented_or_body = reindent_multiline(or_body_snippet.into(), true, Some(indent)); - - let mut app = Applicability::MachineApplicable; - let suggestion = sugg::Sugg::hir_with_context(cx, scrutinee, expr.span.ctxt(), "..", &mut app).maybe_par(); - span_lint_and_sugg( - cx, - MANUAL_UNWRAP_OR, - expr.span, - format!("this pattern reimplements `{ty_name}::unwrap_or`"), - "replace with", - format!("{suggestion}.unwrap_or({reindented_or_body})",), - app, - ); } } -fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<&'a Arm<'a>> { +fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<(&'a Arm<'a>, &'a Arm<'a>)> { if arms.len() == 2 && arms.iter().all(|arm| arm.guard.is_none()) && let Some((idx, or_arm)) = arms.iter().enumerate().find(|(_, arm)| match arm.pat.kind { @@ -54,18 +86,33 @@ fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<&' _ => false, }) && let unwrap_arm = &arms[1 - idx] - && let PatKind::TupleStruct(ref qpath, [unwrap_pat], _) = unwrap_arm.pat.kind - && let Res::Def(DefKind::Ctor(..), ctor_id) = cx.qpath_res(qpath, unwrap_arm.pat.hir_id) - && let Some(variant_id) = cx.tcx.opt_parent(ctor_id) - && (cx.tcx.lang_items().option_some_variant() == Some(variant_id) - || cx.tcx.lang_items().result_ok_variant() == Some(variant_id)) - && let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind - && path_to_local_id(unwrap_arm.body, binding_hir_id) - && cx.typeck_results().expr_adjustments(unwrap_arm.body).is_empty() && !contains_return_break_continue_macro(or_arm.body) { - Some(or_arm) + Some((or_arm, unwrap_arm)) } else { None } } + +fn lint<'tcx>( + cx: &LateContext<'tcx>, + expr: &Expr<'tcx>, + scrutinee: &'tcx Expr<'_>, + ty_name: &str, + or_body_snippet: String, + indent: usize, +) { + let reindented_or_body = reindent_multiline(or_body_snippet.into(), true, Some(indent)); + + let mut app = Applicability::MachineApplicable; + let suggestion = sugg::Sugg::hir_with_context(cx, scrutinee, expr.span.ctxt(), "..", &mut app).maybe_par(); + span_lint_and_sugg( + cx, + MANUAL_UNWRAP_OR, + expr.span, + format!("this pattern reimplements `{ty_name}::unwrap_or`"), + "replace with", + format!("{suggestion}.unwrap_or({reindented_or_body})",), + app, + ); +} diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index 691ecd57535..bf7156cc53e 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -1069,7 +1069,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { redundant_guards::check(cx, arms, &self.msrv); if !in_constant(cx, expr.hir_id) { - manual_unwrap_or::check(cx, expr, ex, arms); + manual_unwrap_or::check_match(cx, expr, ex, arms); manual_map::check_match(cx, expr, ex, arms); manual_filter::check_match(cx, ex, arms, expr); } @@ -1097,6 +1097,14 @@ impl<'tcx> LateLintPass<'tcx> for Matches { ); } if !in_constant(cx, expr.hir_id) { + manual_unwrap_or::check_if_let( + cx, + expr, + if_let.let_pat, + if_let.let_expr, + if_let.if_then, + else_expr, + ); manual_map::check_if_let(cx, expr, if_let.let_pat, if_let.let_expr, if_let.if_then, else_expr); manual_filter::check_if_let( cx, diff --git a/clippy_lints/src/matches/single_match.rs b/clippy_lints/src/matches/single_match.rs index 69791414f72..99fdbcff890 100644 --- a/clippy_lints/src/matches/single_match.rs +++ b/clippy_lints/src/matches/single_match.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::{expr_block, get_source_text, snippet}; +use clippy_utils::source::{expr_block, snippet, SpanRangeExt}; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, peel_mid_ty_refs}; use clippy_utils::{is_lint_allowed, is_unit_expr, is_wild, peel_blocks, peel_hir_pat_refs, peel_n_hir_expr_refs}; use core::cmp::max; @@ -17,7 +17,7 @@ use super::{MATCH_BOOL, SINGLE_MATCH, SINGLE_MATCH_ELSE}; /// span, e.g. a string literal `"//"`, but we know that this isn't the case for empty /// match arms. fn empty_arm_has_comment(cx: &LateContext<'_>, span: Span) -> bool { - if let Some(ff) = get_source_text(cx, span) + if let Some(ff) = span.get_source_text(cx) && let Some(text) = ff.as_str() { text.as_bytes().windows(2).any(|w| w == b"//" || w == b"/*") diff --git a/clippy_lints/src/methods/manual_inspect.rs b/clippy_lints/src/methods/manual_inspect.rs new file mode 100644 index 00000000000..e3ce64c246a --- /dev/null +++ b/clippy_lints/src/methods/manual_inspect.rs @@ -0,0 +1,238 @@ +use clippy_config::msrvs::{self, Msrv}; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::{IntoSpan, SpanRangeExt}; +use clippy_utils::ty::get_field_by_name; +use clippy_utils::visitors::{for_each_expr, for_each_expr_without_closures}; +use clippy_utils::{expr_use_ctxt, is_diag_item_method, is_diag_trait_item, path_to_local_id, ExprUseNode}; +use core::ops::ControlFlow; +use rustc_errors::Applicability; +use rustc_hir::{BindingMode, BorrowKind, ByRef, ClosureKind, Expr, ExprKind, Mutability, Node, PatKind}; +use rustc_lint::LateContext; +use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability}; +use rustc_span::{sym, Span, Symbol, DUMMY_SP}; + +use super::MANUAL_INSPECT; + +#[expect(clippy::too_many_lines)] +pub(crate) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &Expr<'_>, name: &str, name_span: Span, msrv: &Msrv) { + if let ExprKind::Closure(c) = arg.kind + && matches!(c.kind, ClosureKind::Closure) + && let typeck = cx.typeck_results() + && let Some(fn_id) = typeck.type_dependent_def_id(expr.hir_id) + && (is_diag_trait_item(cx, fn_id, sym::Iterator) + || (msrv.meets(msrvs::OPTION_RESULT_INSPECT) + && (is_diag_item_method(cx, fn_id, sym::Option) || is_diag_item_method(cx, fn_id, sym::Result)))) + && let body = cx.tcx.hir().body(c.body) + && let [param] = body.params + && let PatKind::Binding(BindingMode(ByRef::No, Mutability::Not), arg_id, _, None) = param.pat.kind + && let arg_ty = typeck.node_type(arg_id) + && let ExprKind::Block(block, _) = body.value.kind + && let Some(final_expr) = block.expr + && !block.stmts.is_empty() + && path_to_local_id(final_expr, arg_id) + && typeck.expr_adjustments(final_expr).is_empty() + { + let mut requires_copy = false; + let mut requires_deref = false; + + // The number of unprocessed return expressions. + let mut ret_count = 0u32; + + // The uses for which processing is delayed until after the visitor. + let mut delayed = vec![]; + + let ctxt = arg.span.ctxt(); + let can_lint = for_each_expr_without_closures(block.stmts, |e| { + if let ExprKind::Closure(c) = e.kind { + // Nested closures don't need to treat returns specially. + let _: Option<!> = for_each_expr(cx, cx.tcx.hir().body(c.body).value, |e| { + if path_to_local_id(e, arg_id) { + let (kind, same_ctxt) = check_use(cx, e); + match (kind, same_ctxt && e.span.ctxt() == ctxt) { + (_, false) | (UseKind::Deref | UseKind::Return(..), true) => { + requires_copy = true; + requires_deref = true; + }, + (UseKind::AutoBorrowed, true) => {}, + (UseKind::WillAutoDeref, true) => { + requires_copy = true; + }, + (kind, true) => delayed.push(kind), + } + } + ControlFlow::Continue(()) + }); + } else if matches!(e.kind, ExprKind::Ret(_)) { + ret_count += 1; + } else if path_to_local_id(e, arg_id) { + let (kind, same_ctxt) = check_use(cx, e); + match (kind, same_ctxt && e.span.ctxt() == ctxt) { + (UseKind::Return(..), false) => { + return ControlFlow::Break(()); + }, + (_, false) | (UseKind::Deref, true) => { + requires_copy = true; + requires_deref = true; + }, + (UseKind::AutoBorrowed, true) => {}, + (UseKind::WillAutoDeref, true) => { + requires_copy = true; + }, + (kind @ UseKind::Return(_), true) => { + ret_count -= 1; + delayed.push(kind); + }, + (kind, true) => delayed.push(kind), + } + } + ControlFlow::Continue(()) + }) + .is_none(); + + if ret_count != 0 { + // A return expression that didn't return the original value was found. + return; + } + + let mut edits = Vec::with_capacity(delayed.len() + 3); + let mut addr_of_edits = Vec::with_capacity(delayed.len()); + for x in delayed { + match x { + UseKind::Return(s) => edits.push((s.with_leading_whitespace(cx).with_ctxt(s.ctxt()), String::new())), + UseKind::Borrowed(s) => { + #[expect(clippy::range_plus_one)] + let range = s.map_range(cx, |src, range| { + let src = src.get(range.clone())?; + let trimmed = src.trim_start_matches([' ', '\t', '\n', '\r', '(']); + trimmed.starts_with('&').then(|| { + let pos = range.start + src.len() - trimmed.len(); + pos..pos + 1 + }) + }); + if let Some(range) = range { + addr_of_edits.push((range.with_ctxt(s.ctxt()), String::new())); + } else { + requires_copy = true; + requires_deref = true; + } + }, + UseKind::FieldAccess(name, e) => { + let Some(mut ty) = get_field_by_name(cx.tcx, arg_ty.peel_refs(), name) else { + requires_copy = true; + continue; + }; + let mut prev_expr = e; + + for (_, parent) in cx.tcx.hir().parent_iter(e.hir_id) { + if let Node::Expr(e) = parent { + match e.kind { + ExprKind::Field(_, name) + if let Some(fty) = get_field_by_name(cx.tcx, ty.peel_refs(), name.name) => + { + ty = fty; + prev_expr = e; + continue; + }, + ExprKind::AddrOf(BorrowKind::Ref, ..) => break, + _ if matches!( + typeck.expr_adjustments(prev_expr).first(), + Some(Adjustment { + kind: Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Not)) + | Adjust::Deref(_), + .. + }) + ) => + { + break; + }, + _ => {}, + } + } + requires_copy |= !ty.is_copy_modulo_regions(cx.tcx, cx.param_env); + break; + } + }, + // Already processed uses. + UseKind::AutoBorrowed | UseKind::WillAutoDeref | UseKind::Deref => {}, + } + } + + if can_lint + && (!requires_copy || arg_ty.is_copy_modulo_regions(cx.tcx, cx.param_env)) + // This case could be handled, but a fair bit of care would need to be taken. + && (!requires_deref || arg_ty.is_freeze(cx.tcx, cx.param_env)) + { + if requires_deref { + edits.push((param.span.shrink_to_lo(), "&".into())); + } else { + edits.extend(addr_of_edits); + } + edits.push(( + name_span, + String::from(match name { + "map" => "inspect", + "map_err" => "inspect_err", + _ => return, + }), + )); + edits.push(( + final_expr + .span + .with_leading_whitespace(cx) + .with_ctxt(final_expr.span.ctxt()), + String::new(), + )); + let app = if edits.iter().any(|(s, _)| s.from_expansion()) { + Applicability::MaybeIncorrect + } else { + Applicability::MachineApplicable + }; + span_lint_and_then(cx, MANUAL_INSPECT, name_span, "", |diag| { + diag.multipart_suggestion("try", edits, app); + }); + } + } +} + +enum UseKind<'tcx> { + AutoBorrowed, + WillAutoDeref, + Deref, + Return(Span), + Borrowed(Span), + FieldAccess(Symbol, &'tcx Expr<'tcx>), +} + +/// Checks how the value is used, and whether it was used in the same `SyntaxContext`. +fn check_use<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (UseKind<'tcx>, bool) { + let use_cx = expr_use_ctxt(cx, e); + if use_cx + .adjustments + .first() + .is_some_and(|a| matches!(a.kind, Adjust::Deref(_))) + { + return (UseKind::AutoBorrowed, use_cx.same_ctxt); + } + let res = match use_cx.use_node(cx) { + ExprUseNode::Return(_) => { + if let ExprKind::Ret(Some(e)) = use_cx.node.expect_expr().kind { + UseKind::Return(e.span) + } else { + return (UseKind::Return(DUMMY_SP), false); + } + }, + ExprUseNode::FieldAccess(name) => UseKind::FieldAccess(name.name, use_cx.node.expect_expr()), + ExprUseNode::Callee | ExprUseNode::MethodArg(_, _, 0) + if use_cx + .adjustments + .first() + .is_some_and(|a| matches!(a.kind, Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Not)))) => + { + UseKind::AutoBorrowed + }, + ExprUseNode::Callee | ExprUseNode::MethodArg(_, _, 0) => UseKind::WillAutoDeref, + ExprUseNode::AddrOf(BorrowKind::Ref, _) => UseKind::Borrowed(use_cx.node.expect_expr().span), + _ => UseKind::Deref, + }; + (res, use_cx.same_ctxt) +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 6200716afbe..1408f454820 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -53,6 +53,7 @@ mod iter_with_drain; mod iterator_step_by_zero; mod join_absolute_paths; mod manual_c_str_literals; +mod manual_inspect; mod manual_is_variant_and; mod manual_next_back; mod manual_ok_or; @@ -116,6 +117,7 @@ mod unnecessary_iter_cloned; mod unnecessary_join; mod unnecessary_lazy_eval; mod unnecessary_literal_unwrap; +mod unnecessary_min_or_max; mod unnecessary_result_map_or_else; mod unnecessary_sort_by; mod unnecessary_to_owned; @@ -3946,6 +3948,31 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does + /// Checks for unnecessary calls to `min()` or `max()` in the following cases + /// - Either both side is constant + /// - One side is clearly larger than the other, like i32::MIN and an i32 variable + /// + /// ### Why is this bad? + /// + /// In the aformentioned cases it is not necessary to call `min()` or `max()` + /// to compare values, it may even cause confusion. + /// + /// ### Example + /// ```no_run + /// let _ = 0.min(7_u32); + /// ``` + /// Use instead: + /// ```no_run + /// let _ = 0; + /// ``` + #[clippy::version = "1.78.0"] + pub UNNECESSARY_MIN_OR_MAX, + complexity, + "using 'min()/max()' when there is no need for it" +} + +declare_clippy_lint! { + /// ### What it does /// Checks for usage of `.map_or_else()` "map closure" for `Result` type. /// /// ### Why is this bad? @@ -4079,6 +4106,27 @@ declare_clippy_lint! { "is_ascii() called on a char iterator" } +declare_clippy_lint! { + /// ### What it does + /// Checks for uses of `map` which return the original item. + /// + /// ### Why is this bad? + /// `inspect` is both clearer in intent and shorter. + /// + /// ### Example + /// ```no_run + /// let x = Some(0).map(|x| { println!("{x}"); x }); + /// ``` + /// Use instead: + /// ```no_run + /// let x = Some(0).inspect(|x| println!("{x}")); + /// ``` + #[clippy::version = "1.78.0"] + pub MANUAL_INSPECT, + complexity, + "use of `map` returning the original item" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Msrv, @@ -4244,6 +4292,8 @@ impl_lint_pass!(Methods => [ MANUAL_C_STR_LITERALS, UNNECESSARY_GET_THEN_CHECK, NEEDLESS_CHARACTER_ITERATION, + MANUAL_INSPECT, + UNNECESSARY_MIN_OR_MAX, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -4543,6 +4593,9 @@ impl Methods { Some(("bytes", recv2, [], _, _)) => bytes_count_to_len::check(cx, expr, recv, recv2), _ => {}, }, + ("min" | "max", [arg]) => { + unnecessary_min_or_max::check(cx, expr, name, recv, arg); + }, ("drain", ..) => { if let Node::Stmt(Stmt { hir_id: _, kind, .. }) = cx.tcx.parent_hir_node(expr.hir_id) && matches!(kind, StmtKind::Semi(_)) @@ -4747,6 +4800,7 @@ impl Methods { } } map_identity::check(cx, expr, recv, m_arg, name, span); + manual_inspect::check(cx, expr, m_arg, name, span, &self.msrv); }, ("map_or", [def, map]) => { option_map_or_none::check(cx, expr, recv, def, map); diff --git a/clippy_lints/src/methods/unnecessary_min_or_max.rs b/clippy_lints/src/methods/unnecessary_min_or_max.rs new file mode 100644 index 00000000000..78851d4122f --- /dev/null +++ b/clippy_lints/src/methods/unnecessary_min_or_max.rs @@ -0,0 +1,90 @@ +use std::cmp::Ordering; + +use super::UNNECESSARY_MIN_OR_MAX; +use clippy_utils::diagnostics::span_lint_and_sugg; + +use clippy_utils::consts::{constant, constant_with_source, Constant, ConstantSource, FullInt}; +use clippy_utils::source::snippet; + +use rustc_errors::Applicability; +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_middle::ty; +use rustc_span::Span; + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + name: &str, + recv: &'tcx Expr<'_>, + arg: &'tcx Expr<'_>, +) { + let typeck_results = cx.typeck_results(); + if let Some((left, ConstantSource::Local | ConstantSource::CoreConstant)) = + constant_with_source(cx, typeck_results, recv) + && let Some((right, ConstantSource::Local | ConstantSource::CoreConstant)) = + constant_with_source(cx, typeck_results, arg) + { + let Some(ord) = Constant::partial_cmp(cx.tcx, typeck_results.expr_ty(recv), &left, &right) else { + return; + }; + + lint(cx, expr, name, recv.span, arg.span, ord); + } else if let Some(extrema) = detect_extrema(cx, recv) { + let ord = match extrema { + Extrema::Minimum => Ordering::Less, + Extrema::Maximum => Ordering::Greater, + }; + lint(cx, expr, name, recv.span, arg.span, ord); + } else if let Some(extrema) = detect_extrema(cx, arg) { + let ord = match extrema { + Extrema::Minimum => Ordering::Greater, + Extrema::Maximum => Ordering::Less, + }; + lint(cx, expr, name, recv.span, arg.span, ord); + } +} + +fn lint(cx: &LateContext<'_>, expr: &Expr<'_>, name: &str, lhs: Span, rhs: Span, order: Ordering) { + let cmp_str = if order.is_ge() { "smaller" } else { "greater" }; + + let suggested_value = if (name == "min" && order.is_ge()) || (name == "max" && order.is_le()) { + snippet(cx, rhs, "..") + } else { + snippet(cx, lhs, "..") + }; + + span_lint_and_sugg( + cx, + UNNECESSARY_MIN_OR_MAX, + expr.span, + format!( + "`{}` is never {} than `{}` and has therefore no effect", + snippet(cx, lhs, ".."), + cmp_str, + snippet(cx, rhs, "..") + ), + "try", + suggested_value.to_string(), + Applicability::MachineApplicable, + ); +} + +#[derive(Debug)] +enum Extrema { + Minimum, + Maximum, +} +fn detect_extrema<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Extrema> { + let ty = cx.typeck_results().expr_ty(expr); + + let cv = constant(cx, cx.typeck_results(), expr)?; + + match (cv.int_value(cx, ty)?, ty.kind()) { + (FullInt::S(i), &ty::Int(ity)) if i == i128::MIN >> (128 - ity.bit_width()?) => Some(Extrema::Minimum), + (FullInt::S(i), &ty::Int(ity)) if i == i128::MAX >> (128 - ity.bit_width()?) => Some(Extrema::Maximum), + (FullInt::U(i), &ty::Uint(uty)) if i == u128::MAX >> (128 - uty.bit_width()?) => Some(Extrema::Maximum), + (FullInt::U(0), &ty::Uint(_)) => Some(Extrema::Minimum), + _ => None, + } +} diff --git a/clippy_lints/src/methods/unnecessary_sort_by.rs b/clippy_lints/src/methods/unnecessary_sort_by.rs index 6911da69b94..a92fe9e55a4 100644 --- a/clippy_lints/src/methods/unnecessary_sort_by.rs +++ b/clippy_lints/src/methods/unnecessary_sort_by.rs @@ -5,7 +5,8 @@ use clippy_utils::ty::implements_trait; use rustc_errors::Applicability; use rustc_hir::{Closure, Expr, ExprKind, Mutability, Param, Pat, PatKind, Path, PathSegment, QPath}; use rustc_lint::LateContext; -use rustc_middle::ty::{self, GenericArgKind}; +use rustc_middle::ty; +use rustc_middle::ty::GenericArgKind; use rustc_span::sym; use rustc_span::symbol::Ident; use std::iter; diff --git a/clippy_lints/src/missing_const_for_fn.rs b/clippy_lints/src/missing_const_for_fn.rs index 4592324809f..bb0d714a31f 100644 --- a/clippy_lints/src/missing_const_for_fn.rs +++ b/clippy_lints/src/missing_const_for_fn.rs @@ -1,11 +1,11 @@ use clippy_config::msrvs::{self, Msrv}; -use clippy_utils::diagnostics::span_lint; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::qualify_min_const_fn::is_min_const_fn; use clippy_utils::{fn_has_unsatisfiable_preds, is_entrypoint_fn, is_from_proc_macro, trait_ref_of_method}; -use rustc_hir as hir; +use rustc_errors::Applicability; use rustc_hir::def_id::CRATE_DEF_ID; use rustc_hir::intravisit::FnKind; -use rustc_hir::{Body, Constness, FnDecl, GenericParamKind}; +use rustc_hir::{self as hir, Body, Constness, FnDecl, GenericParamKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::impl_lint_pass; @@ -120,7 +120,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn { } }, FnKind::Method(_, sig, ..) => { - if trait_ref_of_method(cx, def_id).is_some() || already_const(sig.header) { + if already_const(sig.header) || trait_ref_of_method(cx, def_id).is_some() { return; } }, @@ -147,10 +147,22 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn { let mir = cx.tcx.optimized_mir(def_id); - if let Ok(()) = is_min_const_fn(cx.tcx, mir, &self.msrv) { - span_lint(cx, MISSING_CONST_FOR_FN, span, "this could be a `const fn`"); + if let Ok(()) = is_min_const_fn(cx.tcx, mir, &self.msrv) + && let hir::Node::Item(hir::Item { vis_span, .. }) | hir::Node::ImplItem(hir::ImplItem { vis_span, .. }) = + cx.tcx.hir_node_by_def_id(def_id) + { + let suggestion = if vis_span.is_empty() { "const " } else { " const" }; + span_lint_and_then(cx, MISSING_CONST_FOR_FN, span, "this could be a `const fn`", |diag| { + diag.span_suggestion_verbose( + vis_span.shrink_to_hi(), + "make the function `const`", + suggestion, + Applicability::MachineApplicable, + ); + }); } } + extract_msrv_attr!(LateContext); } diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs index ca344dc5c81..250fd5cbd48 100644 --- a/clippy_lints/src/missing_doc.rs +++ b/clippy_lints/src/missing_doc.rs @@ -8,7 +8,7 @@ use clippy_utils::attrs::is_doc_hidden; use clippy_utils::diagnostics::span_lint; use clippy_utils::is_from_proc_macro; -use clippy_utils::source::snippet_opt; +use clippy_utils::source::SpanRangeExt; use rustc_ast::ast::{self, MetaItem, MetaItemKind}; use rustc_hir as hir; use rustc_hir::def_id::LocalDefId; @@ -266,8 +266,5 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc { } fn span_to_snippet_contains_docs(cx: &LateContext<'_>, search_span: Span) -> bool { - let Some(snippet) = snippet_opt(cx, search_span) else { - return false; - }; - snippet.lines().rev().any(|line| line.trim().starts_with("///")) + search_span.check_source_text(cx, |src| src.lines().rev().any(|line| line.trim().starts_with("///"))) } diff --git a/clippy_lints/src/multiple_bound_locations.rs b/clippy_lints/src/multiple_bound_locations.rs index d608f3bf7b4..d276e29bace 100644 --- a/clippy_lints/src/multiple_bound_locations.rs +++ b/clippy_lints/src/multiple_bound_locations.rs @@ -6,7 +6,7 @@ use rustc_session::declare_lint_pass; use rustc_span::Span; use clippy_utils::diagnostics::span_lint; -use clippy_utils::source::snippet_opt; +use clippy_utils::source::SpanRangeExt; declare_clippy_lint! { /// ### What it does @@ -54,8 +54,10 @@ impl EarlyLintPass for MultipleBoundLocations { match clause { WherePredicate::BoundPredicate(pred) => { if (!pred.bound_generic_params.is_empty() || !pred.bounds.is_empty()) - && let Some(name) = snippet_opt(cx, pred.bounded_ty.span) - && let Some(bound_span) = generic_params_with_bounds.get(name.as_str()) + && let Some(Some(bound_span)) = pred + .bounded_ty + .span + .with_source_text(cx, |src| generic_params_with_bounds.get(src)) { emit_lint(cx, *bound_span, pred.bounded_ty.span); } diff --git a/clippy_lints/src/mutable_debug_assertion.rs b/clippy_lints/src/mutable_debug_assertion.rs index e92ba93942e..563ce2d82ea 100644 --- a/clippy_lints/src/mutable_debug_assertion.rs +++ b/clippy_lints/src/mutable_debug_assertion.rs @@ -96,10 +96,6 @@ impl<'a, 'tcx> Visitor<'tcx> for MutArgVisitor<'a, 'tcx> { self.found = true; return; }, - ExprKind::If(..) => { - self.found = true; - return; - }, ExprKind::Path(_) => { if let Some(adj) = self.cx.typeck_results().adjustments().get(expr.hir_id) { if adj diff --git a/clippy_lints/src/needless_borrows_for_generic_args.rs b/clippy_lints/src/needless_borrows_for_generic_args.rs index f4846a1753f..064ce59c234 100644 --- a/clippy_lints/src/needless_borrows_for_generic_args.rs +++ b/clippy_lints/src/needless_borrows_for_generic_args.rs @@ -80,11 +80,13 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowsForGenericArgs<'tcx> { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if matches!(expr.kind, ExprKind::AddrOf(..)) && !expr.span.from_expansion() - && let Some(use_cx) = expr_use_ctxt(cx, expr) + && let use_cx = expr_use_ctxt(cx, expr) + && use_cx.same_ctxt && !use_cx.is_ty_unified - && let Some(DefinedTy::Mir(ty)) = use_cx.node.defined_ty(cx) + && let use_node = use_cx.use_node(cx) + && let Some(DefinedTy::Mir(ty)) = use_node.defined_ty(cx) && let ty::Param(ty) = *ty.value.skip_binder().kind() - && let Some((hir_id, fn_id, i)) = match use_cx.node { + && let Some((hir_id, fn_id, i)) = match use_node { ExprUseNode::MethodArg(_, _, 0) => None, ExprUseNode::MethodArg(hir_id, None, i) => cx .typeck_results() diff --git a/clippy_lints/src/needless_else.rs b/clippy_lints/src/needless_else.rs index b6aad69d166..f8bb72a16db 100644 --- a/clippy_lints/src/needless_else.rs +++ b/clippy_lints/src/needless_else.rs @@ -1,8 +1,8 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::{snippet_opt, trim_span}; +use clippy_utils::source::{IntoSpan, SpanRangeExt}; use rustc_ast::ast::{Expr, ExprKind}; use rustc_errors::Applicability; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_session::declare_lint_pass; declare_clippy_lint! { @@ -41,16 +41,16 @@ impl EarlyLintPass for NeedlessElse { && !expr.span.from_expansion() && !else_clause.span.from_expansion() && block.stmts.is_empty() - && let Some(trimmed) = expr.span.trim_start(then_block.span) - && let span = trim_span(cx.sess().source_map(), trimmed) - && let Some(else_snippet) = snippet_opt(cx, span) - // Ignore else blocks that contain comments or #[cfg]s - && !else_snippet.contains(['/', '#']) + && let range = (then_block.span.hi()..expr.span.hi()).trim_start(cx) + && range.clone().check_source_text(cx, |src| { + // Ignore else blocks that contain comments or #[cfg]s + !src.contains(['/', '#']) + }) { span_lint_and_sugg( cx, NEEDLESS_ELSE, - span, + range.with_ctxt(expr.span.ctxt()), "this `else` branch is empty", "you can remove it", String::new(), diff --git a/clippy_lints/src/needless_for_each.rs b/clippy_lints/src/needless_for_each.rs index 630018238f4..143acc2b1cb 100644 --- a/clippy_lints/src/needless_for_each.rs +++ b/clippy_lints/src/needless_for_each.rs @@ -25,14 +25,14 @@ declare_clippy_lint! { /// ```no_run /// let v = vec![0, 1, 2]; /// v.iter().for_each(|elem| { - /// println!("{}", elem); + /// println!("{elem}"); /// }) /// ``` /// Use instead: /// ```no_run /// let v = vec![0, 1, 2]; - /// for elem in v.iter() { - /// println!("{}", elem); + /// for elem in &v { + /// println!("{elem}"); /// } /// ``` /// diff --git a/clippy_lints/src/needless_if.rs b/clippy_lints/src/needless_if.rs index 51bee4b51f6..1d6233d432a 100644 --- a/clippy_lints/src/needless_if.rs +++ b/clippy_lints/src/needless_if.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::higher::If; use clippy_utils::is_from_proc_macro; -use clippy_utils::source::snippet_opt; +use clippy_utils::source::{snippet_opt, SpanRangeExt}; use rustc_errors::Applicability; use rustc_hir::{ExprKind, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -39,18 +39,24 @@ declare_lint_pass!(NeedlessIf => [NEEDLESS_IF]); impl LateLintPass<'_> for NeedlessIf { fn check_stmt<'tcx>(&mut self, cx: &LateContext<'tcx>, stmt: &Stmt<'tcx>) { if let StmtKind::Expr(expr) = stmt.kind - && let Some(If {cond, then, r#else: None }) = If::hir(expr) + && let Some(If { + cond, + then, + r#else: None, + }) = If::hir(expr) && let ExprKind::Block(block, ..) = then.kind && block.stmts.is_empty() && block.expr.is_none() && !in_external_macro(cx.sess(), expr.span) - && let Some(then_snippet) = snippet_opt(cx, then.span) - // Ignore - // - empty macro expansions - // - empty reptitions in macro expansions - // - comments - // - #[cfg]'d out code - && then_snippet.chars().all(|ch| matches!(ch, '{' | '}') || ch.is_ascii_whitespace()) + && then.span.check_source_text(cx, |src| { + // Ignore + // - empty macro expansions + // - empty reptitions in macro expansions + // - comments + // - #[cfg]'d out code + src.bytes() + .all(|ch| matches!(ch, b'{' | b'}') || ch.is_ascii_whitespace()) + }) && let Some(cond_snippet) = snippet_opt(cx, cond.span) && !is_from_proc_macro(cx, expr) { diff --git a/clippy_lints/src/non_octal_unix_permissions.rs b/clippy_lints/src/non_octal_unix_permissions.rs index 2701d6bdca3..b915df52762 100644 --- a/clippy_lints/src/non_octal_unix_permissions.rs +++ b/clippy_lints/src/non_octal_unix_permissions.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::{snippet_opt, snippet_with_applicability}; +use clippy_utils::source::{snippet_with_applicability, SpanRangeExt}; use clippy_utils::{match_def_path, paths}; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; @@ -53,8 +53,9 @@ impl<'tcx> LateLintPass<'tcx> for NonOctalUnixPermissions { && cx.tcx.is_diagnostic_item(sym::FsPermissions, adt.did()))) && let ExprKind::Lit(_) = param.kind && param.span.eq_ctxt(expr.span) - && let Some(snip) = snippet_opt(cx, param.span) - && !(snip.starts_with("0o") || snip.starts_with("0b")) + && param + .span + .check_source_text(cx, |src| !matches!(src.as_bytes(), [b'0', b'o' | b'b', ..])) { show_error(cx, param); } @@ -65,8 +66,9 @@ impl<'tcx> LateLintPass<'tcx> for NonOctalUnixPermissions { && match_def_path(cx, def_id, &paths::PERMISSIONS_FROM_MODE) && let ExprKind::Lit(_) = param.kind && param.span.eq_ctxt(expr.span) - && let Some(snip) = snippet_opt(cx, param.span) - && !(snip.starts_with("0o") || snip.starts_with("0b")) + && param + .span + .check_source_text(cx, |src| !matches!(src.as_bytes(), [b'0', b'o' | b'b', ..])) { show_error(cx, param); } diff --git a/clippy_lints/src/octal_escapes.rs b/clippy_lints/src/octal_escapes.rs index 2fc039ae886..2eae9b23746 100644 --- a/clippy_lints/src/octal_escapes.rs +++ b/clippy_lints/src/octal_escapes.rs @@ -1,12 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_then; -use rustc_ast::ast::{Expr, ExprKind}; -use rustc_ast::token::{Lit, LitKind}; +use clippy_utils::source::SpanRangeExt; +use rustc_ast::token::LitKind; +use rustc_ast::{Expr, ExprKind}; use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::declare_lint_pass; -use rustc_span::Span; -use std::fmt::Write; +use rustc_span::{BytePos, Pos, SpanData}; declare_clippy_lint! { /// ### What it does @@ -52,104 +52,66 @@ declare_lint_pass!(OctalEscapes => [OCTAL_ESCAPES]); impl EarlyLintPass for OctalEscapes { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { - if in_external_macro(cx.sess(), expr.span) { - return; - } - - if let ExprKind::Lit(token_lit) = &expr.kind { - if matches!(token_lit.kind, LitKind::Str) { - check_lit(cx, token_lit, expr.span, true); - } else if matches!(token_lit.kind, LitKind::ByteStr) { - check_lit(cx, token_lit, expr.span, false); - } - } - } -} - -fn check_lit(cx: &EarlyContext<'_>, lit: &Lit, span: Span, is_string: bool) { - let contents = lit.symbol.as_str(); - let mut iter = contents.char_indices().peekable(); - let mut found = vec![]; + if let ExprKind::Lit(lit) = &expr.kind + // The number of bytes from the start of the token to the start of literal's text. + && let start_offset = BytePos::from_u32(match lit.kind { + LitKind::Str => 1, + LitKind::ByteStr | LitKind::CStr => 2, + _ => return, + }) + && !in_external_macro(cx.sess(), expr.span) + { + let s = lit.symbol.as_str(); + let mut iter = s.as_bytes().iter(); + while let Some(&c) = iter.next() { + if c == b'\\' + // Always move the iterator to read the escape char. + && let Some(b'0') = iter.next() + { + // C-style octal escapes read from one to three characters. + // The first character (`0`) has already been read. + let (tail, len, c_hi, c_lo) = match *iter.as_slice() { + [c_hi @ b'0'..=b'7', c_lo @ b'0'..=b'7', ref tail @ ..] => (tail, 4, c_hi, c_lo), + [c_lo @ b'0'..=b'7', ref tail @ ..] => (tail, 3, b'0', c_lo), + _ => continue, + }; + iter = tail.iter(); + let offset = start_offset + BytePos::from_usize(s.len() - tail.len()); + let data = expr.span.data(); + let span = SpanData { + lo: data.lo + offset - BytePos::from_u32(len), + hi: data.lo + offset, + ..data + } + .span(); - // go through the string, looking for \0[0-7][0-7]? - while let Some((from, ch)) = iter.next() { - if ch == '\\' { - if let Some((_, '0')) = iter.next() { - // collect up to two further octal digits - if let Some((mut to, _)) = iter.next_if(|(_, ch)| matches!(ch, '0'..='7')) { - if iter.next_if(|(_, ch)| matches!(ch, '0'..='7')).is_some() { - to += 1; + // Last check to make sure the source text matches what we read from the string. + // Macros are involved somehow if this doesn't match. + if span.check_source_text(cx, |src| match *src.as_bytes() { + [b'\\', b'0', lo] => lo == c_lo, + [b'\\', b'0', hi, lo] => hi == c_hi && lo == c_lo, + _ => false, + }) { + span_lint_and_then(cx, OCTAL_ESCAPES, span, "octal-looking escape in a literal", |diag| { + diag.help_once("octal escapes are not supported, `\\0` is always null") + .span_suggestion( + span, + "if an octal escape is intended, use a hex escape instead", + format!("\\x{:02x}", (((c_hi - b'0') << 3) | (c_lo - b'0'))), + Applicability::MaybeIncorrect, + ) + .span_suggestion( + span, + "if a null escape is intended, disambiguate using", + format!("\\x00{}{}", c_hi as char, c_lo as char), + Applicability::MaybeIncorrect, + ); + }); + } else { + break; } - found.push((from, to + 1)); } } } } - - if found.is_empty() { - return; - } - - span_lint_and_then( - cx, - OCTAL_ESCAPES, - span, - format!( - "octal-looking escape in {} literal", - if is_string { "string" } else { "byte string" } - ), - |diag| { - diag.help(format!( - "octal escapes are not supported, `\\0` is always a null {}", - if is_string { "character" } else { "byte" } - )); - - // Generate suggestions if the string is not too long (~ 5 lines) - if contents.len() < 400 { - // construct two suggestion strings, one with \x escapes with octal meaning - // as in C, and one with \x00 for null bytes. - let mut suggest_1 = if is_string { "\"" } else { "b\"" }.to_string(); - let mut suggest_2 = suggest_1.clone(); - let mut index = 0; - for (from, to) in found { - suggest_1.push_str(&contents[index..from]); - suggest_2.push_str(&contents[index..from]); - - // construct a replacement escape - // the maximum value is \077, or \x3f, so u8 is sufficient here - if let Ok(n) = u8::from_str_radix(&contents[from + 1..to], 8) { - write!(suggest_1, "\\x{n:02x}").unwrap(); - } - - // append the null byte as \x00 and the following digits literally - suggest_2.push_str("\\x00"); - suggest_2.push_str(&contents[from + 2..to]); - - index = to; - } - suggest_1.push_str(&contents[index..]); - suggest_2.push_str(&contents[index..]); - - suggest_1.push('"'); - suggest_2.push('"'); - // suggestion 1: equivalent hex escape - diag.span_suggestion( - span, - "if an octal escape was intended, use the hexadecimal representation instead", - suggest_1, - Applicability::MaybeIncorrect, - ); - // suggestion 2: unambiguous null byte - diag.span_suggestion( - span, - format!( - "if the null {} is intended, disambiguate using", - if is_string { "character" } else { "byte" } - ), - suggest_2, - Applicability::MaybeIncorrect, - ); - } - }, - ); } diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index 186e548d373..4fdaa9f00a1 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -1,7 +1,7 @@ use clippy_config::msrvs::{self, Msrv}; use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then}; -use clippy_utils::source::{snippet, snippet_opt, snippet_with_applicability}; +use clippy_utils::source::{snippet, snippet_with_applicability, SpanRangeExt}; use clippy_utils::sugg::Sugg; use clippy_utils::{get_parent_expr, higher, in_constant, is_integer_const, path_to_local}; use rustc_ast::ast::RangeLimits; @@ -285,9 +285,10 @@ fn check_possible_range_contains( if let ExprKind::Binary(ref lhs_op, _left, new_lhs) = left.kind && op == lhs_op.node && let new_span = Span::new(new_lhs.span.lo(), right.span.hi(), expr.span.ctxt(), expr.span.parent()) - && let Some(snip) = &snippet_opt(cx, new_span) - // Do not continue if we have mismatched number of parens, otherwise the suggestion is wrong - && snip.matches('(').count() == snip.matches(')').count() + && new_span.check_source_text(cx, |src| { + // Do not continue if we have mismatched number of parens, otherwise the suggestion is wrong + src.matches('(').count() == src.matches(')').count() + }) { check_possible_range_contains(cx, op, new_lhs, right, expr, new_span); } @@ -363,17 +364,19 @@ fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) { |diag| { let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").maybe_par().to_string()); let end = Sugg::hir(cx, y, "y").maybe_par(); - if let Some(is_wrapped) = &snippet_opt(cx, span) { - if is_wrapped.starts_with('(') && is_wrapped.ends_with(')') { + match span.with_source_text(cx, |src| src.starts_with('(') && src.ends_with(')')) { + Some(true) => { diag.span_suggestion(span, "use", format!("({start}..={end})"), Applicability::MaybeIncorrect); - } else { + }, + Some(false) => { diag.span_suggestion( span, "use", format!("{start}..={end}"), Applicability::MachineApplicable, // snippet ); - } + }, + None => {}, } }, ); diff --git a/clippy_lints/src/string_patterns.rs b/clippy_lints/src/string_patterns.rs index 64b5b8f9f27..7ba58942a17 100644 --- a/clippy_lints/src/string_patterns.rs +++ b/clippy_lints/src/string_patterns.rs @@ -1,5 +1,6 @@ use std::ops::ControlFlow; +use clippy_config::msrvs::{self, Msrv}; use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::eager_or_lazy::switch_to_eager_eval; use clippy_utils::macros::matching_root_macro_call; @@ -12,7 +13,7 @@ use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, PatKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; -use rustc_session::declare_lint_pass; +use rustc_session::impl_lint_pass; use rustc_span::{sym, Span}; declare_clippy_lint! { @@ -69,7 +70,18 @@ declare_clippy_lint! { "using a single-character str where a char could be used, e.g., `_.split(\"x\")`" } -declare_lint_pass!(StringPatterns => [MANUAL_PATTERN_CHAR_COMPARISON, SINGLE_CHAR_PATTERN]); +pub struct StringPatterns { + msrv: Msrv, +} + +impl StringPatterns { + #[must_use] + pub fn new(msrv: Msrv) -> Self { + Self { msrv } + } +} + +impl_lint_pass!(StringPatterns => [MANUAL_PATTERN_CHAR_COMPARISON, SINGLE_CHAR_PATTERN]); const PATTERN_METHODS: [(&str, usize); 22] = [ ("contains", 0), @@ -122,7 +134,7 @@ fn get_char_span<'tcx>(cx: &'_ LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Optio } } -fn check_manual_pattern_char_comparison(cx: &LateContext<'_>, method_arg: &Expr<'_>) { +fn check_manual_pattern_char_comparison(cx: &LateContext<'_>, method_arg: &Expr<'_>, msrv: &Msrv) { if let ExprKind::Closure(closure) = method_arg.kind && let body = cx.tcx.hir().body(closure.body) && let Some(PatKind::Binding(_, binding, ..)) = body.params.first().map(|p| p.pat.kind) @@ -178,6 +190,9 @@ fn check_manual_pattern_char_comparison(cx: &LateContext<'_>, method_arg: &Expr< { return; } + if set_char_spans.len() > 1 && !msrv.meets(msrvs::PATTERN_TRAIT_CHAR_ARRAY) { + return; + } span_lint_and_then( cx, MANUAL_PATTERN_CHAR_COMPARISON, @@ -221,7 +236,9 @@ impl<'tcx> LateLintPass<'tcx> for StringPatterns { { check_single_char_pattern_lint(cx, arg); - check_manual_pattern_char_comparison(cx, arg); + check_manual_pattern_char_comparison(cx, arg, &self.msrv); } } + + extract_msrv_attr!(LateContext); } diff --git a/clippy_lints/src/upper_case_acronyms.rs b/clippy_lints/src/upper_case_acronyms.rs index f376d349646..72392f8e1f7 100644 --- a/clippy_lints/src/upper_case_acronyms.rs +++ b/clippy_lints/src/upper_case_acronyms.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_hir_and_then; -use itertools::Itertools; +use core::mem::replace; use rustc_errors::Applicability; use rustc_hir::{HirId, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -56,55 +56,71 @@ impl UpperCaseAcronyms { impl_lint_pass!(UpperCaseAcronyms => [UPPER_CASE_ACRONYMS]); -fn correct_ident(ident: &str) -> String { - let ident = ident.chars().rev().collect::<String>(); - let fragments = ident - .split_inclusive(|x: char| !x.is_ascii_lowercase()) - .rev() - .map(|x| x.chars().rev().collect::<String>()); - - let mut ident = fragments.clone().next().unwrap(); - for (ref prev, ref curr) in fragments.tuple_windows() { - if <[&String; 2]>::from((prev, curr)) - .iter() - .all(|s| s.len() == 1 && s.chars().next().unwrap().is_ascii_uppercase()) - { - ident.push_str(&curr.to_ascii_lowercase()); +fn contains_acronym(s: &str) -> bool { + let mut count = 0; + for c in s.chars() { + if c.is_ascii_uppercase() { + count += 1; + if count == 3 { + return true; + } } else { - ident.push_str(curr); + count = 0; } } - ident + count == 2 } fn check_ident(cx: &LateContext<'_>, ident: &Ident, hir_id: HirId, be_aggressive: bool) { - let span = ident.span; - let ident = ident.as_str(); - let corrected = correct_ident(ident); - // warn if we have pure-uppercase idents - // assume that two-letter words are some kind of valid abbreviation like FP for false positive - // (and don't warn) - if (ident.chars().all(|c| c.is_ascii_uppercase()) && ident.len() > 2) - // otherwise, warn if we have SOmeTHING lIKE THIs but only warn with the aggressive - // upper-case-acronyms-aggressive config option enabled - || (be_aggressive && ident != corrected) + let s = ident.as_str(); + + // By default, only warn for upper case identifiers with at least 3 characters. + let replacement = if s.len() > 2 && s.bytes().all(|c| c.is_ascii_uppercase()) { + let mut r = String::with_capacity(s.len()); + let mut s = s.chars(); + r.push(s.next().unwrap()); + r.extend(s.map(|c| c.to_ascii_lowercase())); + r + } else if be_aggressive + // Only lint if the ident starts with an upper case character. + && let unprefixed = s.trim_start_matches('_') + && unprefixed.starts_with(|c: char| c.is_ascii_uppercase()) + && contains_acronym(unprefixed) { - span_lint_hir_and_then( - cx, - UPPER_CASE_ACRONYMS, - hir_id, - span, - format!("name `{ident}` contains a capitalized acronym"), - |diag| { - diag.span_suggestion( - span, - "consider making the acronym lowercase, except the initial letter", - corrected, - Applicability::MaybeIncorrect, - ); - }, - ); - } + let mut r = String::with_capacity(s.len()); + let mut s = s.chars(); + let mut prev_upper = false; + while let Some(c) = s.next() { + r.push( + if replace(&mut prev_upper, c.is_ascii_uppercase()) + && s.clone().next().map_or(true, |c| c.is_ascii_uppercase()) + { + c.to_ascii_lowercase() + } else { + c + }, + ); + } + r + } else { + return; + }; + + span_lint_hir_and_then( + cx, + UPPER_CASE_ACRONYMS, + hir_id, + ident.span, + format!("name `{ident}` contains a capitalized acronym"), + |diag| { + diag.span_suggestion( + ident.span, + "consider making the acronym lowercase, except the initial letter", + replacement, + Applicability::MaybeIncorrect, + ); + }, + ); } impl LateLintPass<'_> for UpperCaseAcronyms { diff --git a/clippy_lints/src/utils/internal_lints/invalid_paths.rs b/clippy_lints/src/utils/internal_lints/invalid_paths.rs index c62ae8d718d..0beb0bb8ed4 100644 --- a/clippy_lints/src/utils/internal_lints/invalid_paths.rs +++ b/clippy_lints/src/utils/internal_lints/invalid_paths.rs @@ -69,6 +69,7 @@ pub fn check_path(cx: &LateContext<'_>, path: &[&str]) -> bool { SimplifiedType::Float(FloatTy::F64), SimplifiedType::Slice, SimplifiedType::Str, + SimplifiedType::Bool, ] .iter() .flat_map(|&ty| cx.tcx.incoherent_impls(ty).into_iter()) diff --git a/clippy_utils/src/ast_utils.rs b/clippy_utils/src/ast_utils.rs index 785d5ed5dbe..13256c5557d 100644 --- a/clippy_utils/src/ast_utils.rs +++ b/clippy_utils/src/ast_utils.rs @@ -724,9 +724,7 @@ pub fn eq_ty(l: &Ty, r: &Ty) -> bool { (Tup(l), Tup(r)) => over(l, r, |l, r| eq_ty(l, r)), (Path(lq, lp), Path(rq, rp)) => both(lq, rq, eq_qself) && eq_path(lp, rp), (TraitObject(lg, ls), TraitObject(rg, rs)) => ls == rs && over(lg, rg, eq_generic_bound), - (ImplTrait(_, lg), ImplTrait(_, rg)) => { - over(lg, rg, eq_generic_bound) - }, + (ImplTrait(_, lg), ImplTrait(_, rg)) => over(lg, rg, eq_generic_bound), (Typeof(l), Typeof(r)) => eq_expr(&l.value, &r.value), (MacCall(l), MacCall(r)) => eq_mac_call(l, r), _ => false, diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 553e8999975..7f2234b310b 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -18,8 +18,8 @@ use rustc_ast::AttrStyle; use rustc_hir::intravisit::FnKind; use rustc_hir::{ Block, BlockCheckMode, Body, Closure, Destination, Expr, ExprKind, FieldDef, FnHeader, FnRetTy, HirId, Impl, - ImplItem, ImplItemKind, IsAuto, Item, ItemKind, LoopSource, MatchSource, MutTy, Node, QPath, Safety, TraitItem, - TraitItemKind, Ty, TyKind, UnOp, UnsafeSource, Variant, VariantData, YieldSource, + ImplItem, ImplItemKind, IsAuto, Item, ItemKind, Lit, LoopSource, MatchSource, MutTy, Node, Path, QPath, Safety, + TraitItem, TraitItemKind, Ty, TyKind, UnOp, UnsafeSource, Variant, VariantData, YieldSource, }; use rustc_lint::{LateContext, LintContext}; use rustc_middle::ty::TyCtxt; @@ -121,6 +121,26 @@ fn qpath_search_pat(path: &QPath<'_>) -> (Pat, Pat) { } } +fn path_search_pat(path: &Path<'_>) -> (Pat, Pat) { + let (head, tail) = match path.segments { + [head, .., tail] => (head, tail), + [p] => (p, p), + [] => return (Pat::Str(""), Pat::Str("")), + }; + ( + if head.ident.name == kw::PathRoot { + Pat::Str("::") + } else { + Pat::Sym(head.ident.name) + }, + if tail.args.is_some() { + Pat::Str(">") + } else { + Pat::Sym(tail.ident.name) + }, + ) +} + /// Get the search patterns to use for the given expression fn expr_search_pat(tcx: TyCtxt<'_>, e: &Expr<'_>) -> (Pat, Pat) { match e.kind { @@ -355,19 +375,21 @@ fn ty_search_pat(ty: &Ty<'_>) -> (Pat, Pat) { } } +fn ident_search_pat(ident: Ident) -> (Pat, Pat) { + (Pat::Sym(ident.name), Pat::Sym(ident.name)) +} + pub trait WithSearchPat<'cx> { type Context: LintContext; fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat); fn span(&self) -> Span; } macro_rules! impl_with_search_pat { - ($cx:ident: $ty:ident with $fn:ident $(($tcx:ident))?) => { - impl<'cx> WithSearchPat<'cx> for $ty<'cx> { - type Context = $cx<'cx>; - #[allow(unused_variables)] - fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat) { - $(let $tcx = cx.tcx;)? - $fn($($tcx,)? self) + (($cx_ident:ident: $cx_ty:ident<$cx_lt:lifetime>, $self:tt: $ty:ty) => $fn:ident($($args:tt)*)) => { + impl<$cx_lt> WithSearchPat<$cx_lt> for $ty { + type Context = $cx_ty<$cx_lt>; + fn search_pat(&$self, $cx_ident: &Self::Context) -> (Pat, Pat) { + $fn($($args)*) } fn span(&self) -> Span { self.span @@ -375,13 +397,17 @@ macro_rules! impl_with_search_pat { } }; } -impl_with_search_pat!(LateContext: Expr with expr_search_pat(tcx)); -impl_with_search_pat!(LateContext: Item with item_search_pat); -impl_with_search_pat!(LateContext: TraitItem with trait_item_search_pat); -impl_with_search_pat!(LateContext: ImplItem with impl_item_search_pat); -impl_with_search_pat!(LateContext: FieldDef with field_def_search_pat); -impl_with_search_pat!(LateContext: Variant with variant_search_pat); -impl_with_search_pat!(LateContext: Ty with ty_search_pat); +impl_with_search_pat!((cx: LateContext<'tcx>, self: Expr<'tcx>) => expr_search_pat(cx.tcx, self)); +impl_with_search_pat!((_cx: LateContext<'tcx>, self: Item<'_>) => item_search_pat(self)); +impl_with_search_pat!((_cx: LateContext<'tcx>, self: TraitItem<'_>) => trait_item_search_pat(self)); +impl_with_search_pat!((_cx: LateContext<'tcx>, self: ImplItem<'_>) => impl_item_search_pat(self)); +impl_with_search_pat!((_cx: LateContext<'tcx>, self: FieldDef<'_>) => field_def_search_pat(self)); +impl_with_search_pat!((_cx: LateContext<'tcx>, self: Variant<'_>) => variant_search_pat(self)); +impl_with_search_pat!((_cx: LateContext<'tcx>, self: Ty<'_>) => ty_search_pat(self)); +impl_with_search_pat!((_cx: LateContext<'tcx>, self: Attribute) => attr_search_pat(self)); +impl_with_search_pat!((_cx: LateContext<'tcx>, self: Ident) => ident_search_pat(*self)); +impl_with_search_pat!((_cx: LateContext<'tcx>, self: Lit) => lit_search_pat(&self.node)); +impl_with_search_pat!((_cx: LateContext<'tcx>, self: Path<'_>) => path_search_pat(self)); impl<'cx> WithSearchPat<'cx> for (&FnKind<'cx>, &Body<'cx>, HirId, Span) { type Context = LateContext<'cx>; @@ -395,32 +421,6 @@ impl<'cx> WithSearchPat<'cx> for (&FnKind<'cx>, &Body<'cx>, HirId, Span) { } } -// `Attribute` does not have the `hir` associated lifetime, so we cannot use the macro -impl<'cx> WithSearchPat<'cx> for &'cx Attribute { - type Context = LateContext<'cx>; - - fn search_pat(&self, _cx: &Self::Context) -> (Pat, Pat) { - attr_search_pat(self) - } - - fn span(&self) -> Span { - self.span - } -} - -// `Ident` does not have the `hir` associated lifetime, so we cannot use the macro -impl<'cx> WithSearchPat<'cx> for Ident { - type Context = LateContext<'cx>; - - fn search_pat(&self, _cx: &Self::Context) -> (Pat, Pat) { - (Pat::Sym(self.name), Pat::Sym(self.name)) - } - - fn span(&self) -> Span { - self.span - } -} - /// Checks if the item likely came from a proc-macro. /// /// This should be called after `in_external_macro` and the initial pattern matching of the ast as diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 681c86f76d0..de6ccfe476f 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -1,6 +1,7 @@ #![allow(clippy::float_cmp)] -use crate::source::{get_source_text, walk_span_to_context}; +use crate::macros::HirNode; +use crate::source::{walk_span_to_context, SpanRangeExt}; use crate::{clip, is_direct_expn_of, sext, unsext}; use rustc_apfloat::ieee::{Half, Quad}; @@ -17,7 +18,7 @@ use rustc_middle::ty::{self, EarlyBinder, FloatTy, GenericArgsRef, IntTy, List, use rustc_middle::{bug, mir, span_bug}; use rustc_span::def_id::DefId; use rustc_span::symbol::{Ident, Symbol}; -use rustc_span::SyntaxContext; +use rustc_span::{sym, SyntaxContext}; use rustc_target::abi::Size; use std::cmp::Ordering; use std::hash::{Hash, Hasher}; @@ -326,6 +327,8 @@ pub enum ConstantSource { Local, /// The value is dependent on a defined constant. Constant, + /// The value is dependent on a constant defined in `core` crate. + CoreConstant, } impl ConstantSource { pub fn is_local(&self) -> bool { @@ -439,9 +442,19 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { ExprKind::ConstBlock(ConstBlock { body, .. }) => self.expr(self.lcx.tcx.hir().body(body).value), ExprKind::DropTemps(e) => self.expr(e), ExprKind::Path(ref qpath) => { + let is_core_crate = if let Some(def_id) = self.lcx.qpath_res(qpath, e.hir_id()).opt_def_id() { + self.lcx.tcx.crate_name(def_id.krate) == sym::core + } else { + false + }; self.fetch_path_and_apply(qpath, e.hir_id, self.typeck_results.expr_ty(e), |this, result| { let result = mir_to_const(this.lcx, result)?; - this.source = ConstantSource::Constant; + // If source is already Constant we wouldn't want to override it with CoreConstant + this.source = if is_core_crate && !matches!(this.source, ConstantSource::Constant) { + ConstantSource::CoreConstant + } else { + ConstantSource::Constant + }; Some(result) }) }, @@ -683,7 +696,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { if let Some(expr_span) = walk_span_to_context(expr.span, span.ctxt) && let expr_lo = expr_span.lo() && expr_lo >= span.lo - && let Some(src) = get_source_text(self.lcx, span.lo..expr_lo) + && let Some(src) = (span.lo..expr_lo).get_source_text(self.lcx) && let Some(src) = src.as_str() { use rustc_lexer::TokenKind::{BlockComment, LineComment, OpenBrace, Semi, Whitespace}; diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index 801a9852159..277ba8427e0 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -218,51 +218,56 @@ pub struct Range<'a> { impl<'a> Range<'a> { /// Higher a `hir` range to something similar to `ast::ExprKind::Range`. + #[allow(clippy::similar_names)] pub fn hir(expr: &'a Expr<'_>) -> Option<Range<'a>> { - /// Finds the field named `name` in the field. Always return `Some` for - /// convenience. - fn get_field<'c>(name: &str, fields: &'c [hir::ExprField<'_>]) -> Option<&'c Expr<'c>> { - let expr = &fields.iter().find(|field| field.ident.name.as_str() == name)?.expr; - Some(expr) - } - match expr.kind { - ExprKind::Call(path, args) + ExprKind::Call(path, [arg1, arg2]) if matches!( path.kind, ExprKind::Path(QPath::LangItem(hir::LangItem::RangeInclusiveNew, ..)) ) => { Some(Range { - start: Some(&args[0]), - end: Some(&args[1]), + start: Some(arg1), + end: Some(arg2), limits: ast::RangeLimits::Closed, }) }, - ExprKind::Struct(path, fields, None) => match &path { - QPath::LangItem(hir::LangItem::RangeFull, ..) => Some(Range { + ExprKind::Struct(path, fields, None) => match (path, fields) { + (QPath::LangItem(hir::LangItem::RangeFull, ..), []) => Some(Range { start: None, end: None, limits: ast::RangeLimits::HalfOpen, }), - QPath::LangItem(hir::LangItem::RangeFrom, ..) => Some(Range { - start: Some(get_field("start", fields)?), - end: None, - limits: ast::RangeLimits::HalfOpen, - }), - QPath::LangItem(hir::LangItem::Range, ..) => Some(Range { - start: Some(get_field("start", fields)?), - end: Some(get_field("end", fields)?), - limits: ast::RangeLimits::HalfOpen, - }), - QPath::LangItem(hir::LangItem::RangeToInclusive, ..) => Some(Range { - start: None, - end: Some(get_field("end", fields)?), - limits: ast::RangeLimits::Closed, - }), - QPath::LangItem(hir::LangItem::RangeTo, ..) => Some(Range { + (QPath::LangItem(hir::LangItem::RangeFrom, ..), [field]) if field.ident.name == sym::start => { + Some(Range { + start: Some(field.expr), + end: None, + limits: ast::RangeLimits::HalfOpen, + }) + }, + (QPath::LangItem(hir::LangItem::Range, ..), [field1, field2]) => { + let (start, end) = match (field1.ident.name, field2.ident.name) { + (sym::start, sym::end) => (field1.expr, field2.expr), + (sym::end, sym::start) => (field2.expr, field1.expr), + _ => return None, + }; + Some(Range { + start: Some(start), + end: Some(end), + limits: ast::RangeLimits::HalfOpen, + }) + }, + (QPath::LangItem(hir::LangItem::RangeToInclusive, ..), [field]) if field.ident.name == sym::end => { + Some(Range { + start: None, + end: Some(field.expr), + limits: ast::RangeLimits::Closed, + }) + }, + (QPath::LangItem(hir::LangItem::RangeTo, ..), [field]) if field.ident.name == sym::end => Some(Range { start: None, - end: Some(get_field("end", fields)?), + end: Some(field.expr), limits: ast::RangeLimits::HalfOpen, }), _ => None, diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 50dd8430ac0..8706cec5d38 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -1,6 +1,6 @@ use crate::consts::constant_simple; use crate::macros::macro_backtrace; -use crate::source::{get_source_text, snippet_opt, walk_span_to_context, SpanRange}; +use crate::source::{snippet_opt, walk_span_to_context, SpanRange, SpanRangeExt}; use crate::tokenize_with_text; use rustc_ast::ast::InlineAsmTemplatePiece; use rustc_data_structures::fx::FxHasher; @@ -1173,9 +1173,9 @@ fn eq_span_tokens( pred: impl Fn(TokenKind) -> bool, ) -> bool { fn f(cx: &LateContext<'_>, left: Range<BytePos>, right: Range<BytePos>, pred: impl Fn(TokenKind) -> bool) -> bool { - if let Some(lsrc) = get_source_text(cx, left) + if let Some(lsrc) = left.get_source_text(cx) && let Some(lsrc) = lsrc.as_str() - && let Some(rsrc) = get_source_text(cx, right) + && let Some(rsrc) = right.get_source_text(cx) && let Some(rsrc) = rsrc.as_str() { let pred = |t: &(_, _)| pred(t.0); diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index ee43d95272a..73de8aed35a 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -5,7 +5,6 @@ #![feature(f16)] #![feature(if_let_guard)] #![feature(let_chains)] -#![cfg_attr(bootstrap, feature(lint_reasons))] #![feature(never_type)] #![feature(rustc_private)] #![feature(assert_matches)] @@ -2666,13 +2665,80 @@ pub enum DefinedTy<'tcx> { /// The context an expressions value is used in. pub struct ExprUseCtxt<'tcx> { /// The parent node which consumes the value. - pub node: ExprUseNode<'tcx>, + pub node: Node<'tcx>, + /// The child id of the node the value came from. + pub child_id: HirId, /// Any adjustments applied to the type. pub adjustments: &'tcx [Adjustment<'tcx>], - /// Whether or not the type must unify with another code path. + /// Whether the type must unify with another code path. pub is_ty_unified: bool, - /// Whether or not the value will be moved before it's used. + /// Whether the value will be moved before it's used. pub moved_before_use: bool, + /// Whether the use site has the same `SyntaxContext` as the value. + pub same_ctxt: bool, +} +impl<'tcx> ExprUseCtxt<'tcx> { + pub fn use_node(&self, cx: &LateContext<'tcx>) -> ExprUseNode<'tcx> { + match self.node { + Node::LetStmt(l) => ExprUseNode::LetStmt(l), + Node::ExprField(field) => ExprUseNode::Field(field), + + Node::Item(&Item { + kind: ItemKind::Static(..) | ItemKind::Const(..), + owner_id, + .. + }) + | Node::TraitItem(&TraitItem { + kind: TraitItemKind::Const(..), + owner_id, + .. + }) + | Node::ImplItem(&ImplItem { + kind: ImplItemKind::Const(..), + owner_id, + .. + }) => ExprUseNode::ConstStatic(owner_id), + + Node::Item(&Item { + kind: ItemKind::Fn(..), + owner_id, + .. + }) + | Node::TraitItem(&TraitItem { + kind: TraitItemKind::Fn(..), + owner_id, + .. + }) + | Node::ImplItem(&ImplItem { + kind: ImplItemKind::Fn(..), + owner_id, + .. + }) => ExprUseNode::Return(owner_id), + + Node::Expr(use_expr) => match use_expr.kind { + ExprKind::Ret(_) => ExprUseNode::Return(OwnerId { + def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()), + }), + + ExprKind::Closure(closure) => ExprUseNode::Return(OwnerId { def_id: closure.def_id }), + ExprKind::Call(func, args) => match args.iter().position(|arg| arg.hir_id == self.child_id) { + Some(i) => ExprUseNode::FnArg(func, i), + None => ExprUseNode::Callee, + }, + ExprKind::MethodCall(name, _, args, _) => ExprUseNode::MethodArg( + use_expr.hir_id, + name.args, + args.iter() + .position(|arg| arg.hir_id == self.child_id) + .map_or(0, |i| i + 1), + ), + ExprKind::Field(_, name) => ExprUseNode::FieldAccess(name), + ExprKind::AddrOf(kind, mutbl, _) => ExprUseNode::AddrOf(kind, mutbl), + _ => ExprUseNode::Other, + }, + _ => ExprUseNode::Other, + } + } } /// The node which consumes a value. @@ -2693,7 +2759,8 @@ pub enum ExprUseNode<'tcx> { Callee, /// Access of a field. FieldAccess(Ident), - Expr, + /// Borrow expression. + AddrOf(ast::BorrowKind, Mutability), Other, } impl<'tcx> ExprUseNode<'tcx> { @@ -2770,26 +2837,25 @@ impl<'tcx> ExprUseNode<'tcx> { let sig = cx.tcx.fn_sig(id).skip_binder(); Some(DefinedTy::Mir(cx.tcx.param_env(id).and(sig.input(i)))) }, - Self::LetStmt(_) | Self::FieldAccess(..) | Self::Callee | Self::Expr | Self::Other => None, + Self::LetStmt(_) | Self::FieldAccess(..) | Self::Callee | Self::Other | Self::AddrOf(..) => None, } } } /// Gets the context an expression's value is used in. -pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Option<ExprUseCtxt<'tcx>> { +pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> ExprUseCtxt<'tcx> { let mut adjustments = [].as_slice(); let mut is_ty_unified = false; let mut moved_before_use = false; + let mut same_ctxt = true; let ctxt = e.span.ctxt(); - walk_to_expr_usage(cx, e, &mut |parent_id, parent, child_id| { + let node = walk_to_expr_usage(cx, e, &mut |parent_id, parent, child_id| -> ControlFlow<!> { if adjustments.is_empty() && let Node::Expr(e) = cx.tcx.hir_node(child_id) { adjustments = cx.typeck_results().expr_adjustments(e); } - if cx.tcx.hir().span(parent_id).ctxt() != ctxt { - return ControlFlow::Break(()); - } + same_ctxt &= cx.tcx.hir().span(parent_id).ctxt() == ctxt; if let Node::Expr(e) = parent { match e.kind { ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id != child_id => { @@ -2805,71 +2871,26 @@ pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Optio } } ControlFlow::Continue(()) - })? - .continue_value() - .map(|(use_node, child_id)| { - let node = match use_node { - Node::LetStmt(l) => ExprUseNode::LetStmt(l), - Node::ExprField(field) => ExprUseNode::Field(field), - - Node::Item(&Item { - kind: ItemKind::Static(..) | ItemKind::Const(..), - owner_id, - .. - }) - | Node::TraitItem(&TraitItem { - kind: TraitItemKind::Const(..), - owner_id, - .. - }) - | Node::ImplItem(&ImplItem { - kind: ImplItemKind::Const(..), - owner_id, - .. - }) => ExprUseNode::ConstStatic(owner_id), - - Node::Item(&Item { - kind: ItemKind::Fn(..), - owner_id, - .. - }) - | Node::TraitItem(&TraitItem { - kind: TraitItemKind::Fn(..), - owner_id, - .. - }) - | Node::ImplItem(&ImplItem { - kind: ImplItemKind::Fn(..), - owner_id, - .. - }) => ExprUseNode::Return(owner_id), - - Node::Expr(use_expr) => match use_expr.kind { - ExprKind::Ret(_) => ExprUseNode::Return(OwnerId { - def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()), - }), - ExprKind::Closure(closure) => ExprUseNode::Return(OwnerId { def_id: closure.def_id }), - ExprKind::Call(func, args) => match args.iter().position(|arg| arg.hir_id == child_id) { - Some(i) => ExprUseNode::FnArg(func, i), - None => ExprUseNode::Callee, - }, - ExprKind::MethodCall(name, _, args, _) => ExprUseNode::MethodArg( - use_expr.hir_id, - name.args, - args.iter().position(|arg| arg.hir_id == child_id).map_or(0, |i| i + 1), - ), - ExprKind::Field(child, name) if child.hir_id == e.hir_id => ExprUseNode::FieldAccess(name), - _ => ExprUseNode::Expr, - }, - _ => ExprUseNode::Other, - }; - ExprUseCtxt { + }); + match node { + Some(ControlFlow::Continue((node, child_id))) => ExprUseCtxt { node, + child_id, adjustments, is_ty_unified, moved_before_use, - } - }) + same_ctxt, + }, + Some(ControlFlow::Break(_)) => unreachable!("type of node is ControlFlow<!>"), + None => ExprUseCtxt { + node: Node::Crate(cx.tcx.hir().root_module()), + child_id: HirId::INVALID, + adjustments: &[], + is_ty_unified: true, + moved_before_use: true, + same_ctxt: false, + }, + } } /// Tokenizes the input while keeping the text associated with each token. diff --git a/clippy_utils/src/mir/possible_borrower.rs b/clippy_utils/src/mir/possible_borrower.rs index 7b4fd8a210e..07e6705cd3d 100644 --- a/clippy_utils/src/mir/possible_borrower.rs +++ b/clippy_utils/src/mir/possible_borrower.rs @@ -69,7 +69,7 @@ impl<'a, 'b, 'tcx> mir::visit::Visitor<'tcx> for PossibleBorrowerVisitor<'a, 'b, fn visit_assign(&mut self, place: &mir::Place<'tcx>, rvalue: &mir::Rvalue<'_>, _location: mir::Location) { let lhs = place.local; match rvalue { - mir::Rvalue::Ref(_, _, borrowed) => { + mir::Rvalue::Ref(_, _, borrowed) | mir::Rvalue::CopyForDeref(borrowed) => { self.possible_borrower.add(borrowed.local, lhs); }, other => { diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 9bf068ee3cd..3f66813801d 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -110,5 +110,4 @@ pub const VEC_POP: [&str; 4] = ["alloc", "vec", "Vec", "pop"]; pub const WAKER: [&str; 4] = ["core", "task", "wake", "Waker"]; pub const OPTION_UNWRAP: [&str; 4] = ["core", "option", "Option", "unwrap"]; pub const OPTION_EXPECT: [&str; 4] = ["core", "option", "Option", "expect"]; -#[expect(clippy::invalid_paths)] // not sure why it thinks this, it works so pub const BOOL_THEN: [&str; 4] = ["core", "bool", "<impl bool>", "then"]; diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index e2e5cde947d..496c8f5b553 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -9,12 +9,16 @@ use rustc_hir::{BlockCheckMode, Expr, ExprKind, UnsafeSource}; use rustc_lint::{LateContext, LintContext}; use rustc_session::Session; use rustc_span::source_map::{original_sp, SourceMap}; -use rustc_span::{hygiene, BytePos, SourceFileAndLine, Pos, SourceFile, Span, SpanData, SyntaxContext, DUMMY_SP}; +use rustc_span::{ + hygiene, BytePos, FileNameDisplayPreference, Pos, SourceFile, SourceFileAndLine, Span, SpanData, SyntaxContext, + DUMMY_SP, +}; use std::borrow::Cow; +use std::fmt; use std::ops::Range; -/// A type which can be converted to the range portion of a `Span`. -pub trait SpanRange { +/// Conversion of a value into the range portion of a `Span`. +pub trait SpanRange: Sized { fn into_range(self) -> Range<BytePos>; } impl SpanRange for Span { @@ -34,6 +38,182 @@ impl SpanRange for Range<BytePos> { } } +/// Conversion of a value into a `Span` +pub trait IntoSpan: Sized { + fn into_span(self) -> Span; + fn with_ctxt(self, ctxt: SyntaxContext) -> Span; +} +impl IntoSpan for Span { + fn into_span(self) -> Span { + self + } + fn with_ctxt(self, ctxt: SyntaxContext) -> Span { + self.with_ctxt(ctxt) + } +} +impl IntoSpan for SpanData { + fn into_span(self) -> Span { + self.span() + } + fn with_ctxt(self, ctxt: SyntaxContext) -> Span { + Span::new(self.lo, self.hi, ctxt, self.parent) + } +} +impl IntoSpan for Range<BytePos> { + fn into_span(self) -> Span { + Span::with_root_ctxt(self.start, self.end) + } + fn with_ctxt(self, ctxt: SyntaxContext) -> Span { + Span::new(self.start, self.end, ctxt, None) + } +} + +pub trait SpanRangeExt: SpanRange { + /// Gets the source file, and range in the file, of the given span. Returns `None` if the span + /// extends through multiple files, or is malformed. + fn get_source_text(self, cx: &impl LintContext) -> Option<SourceFileRange> { + get_source_text(cx.sess().source_map(), self.into_range()) + } + + /// Calls the given function with the source text referenced and returns the value. Returns + /// `None` if the source text cannot be retrieved. + fn with_source_text<T>(self, cx: &impl LintContext, f: impl for<'a> FnOnce(&'a str) -> T) -> Option<T> { + with_source_text(cx.sess().source_map(), self.into_range(), f) + } + + /// Checks if the referenced source text satisfies the given predicate. Returns `false` if the + /// source text cannot be retrieved. + fn check_source_text(self, cx: &impl LintContext, pred: impl for<'a> FnOnce(&'a str) -> bool) -> bool { + self.with_source_text(cx, pred).unwrap_or(false) + } + + /// Calls the given function with the both the text of the source file and the referenced range, + /// and returns the value. Returns `None` if the source text cannot be retrieved. + fn with_source_text_and_range<T>( + self, + cx: &impl LintContext, + f: impl for<'a> FnOnce(&'a str, Range<usize>) -> T, + ) -> Option<T> { + with_source_text_and_range(cx.sess().source_map(), self.into_range(), f) + } + + /// Calls the given function with the both the text of the source file and the referenced range, + /// and creates a new span with the returned range. Returns `None` if the source text cannot be + /// retrieved, or no result is returned. + /// + /// The new range must reside within the same source file. + fn map_range( + self, + cx: &impl LintContext, + f: impl for<'a> FnOnce(&'a str, Range<usize>) -> Option<Range<usize>>, + ) -> Option<Range<BytePos>> { + map_range(cx.sess().source_map(), self.into_range(), f) + } + + /// Extends the range to include all preceding whitespace characters. + fn with_leading_whitespace(self, cx: &impl LintContext) -> Range<BytePos> { + with_leading_whitespace(cx.sess().source_map(), self.into_range()) + } + + /// Trims the leading whitespace from the range. + fn trim_start(self, cx: &impl LintContext) -> Range<BytePos> { + trim_start(cx.sess().source_map(), self.into_range()) + } + + /// Writes the referenced source text to the given writer. Will return `Err` if the source text + /// could not be retrieved. + fn write_source_text_to(self, cx: &impl LintContext, dst: &mut impl fmt::Write) -> fmt::Result { + write_source_text_to(cx.sess().source_map(), self.into_range(), dst) + } + + /// Extracts the referenced source text as an owned string. + fn source_text_to_string(self, cx: &impl LintContext) -> Option<String> { + self.with_source_text(cx, ToOwned::to_owned) + } +} +impl<T: SpanRange> SpanRangeExt for T {} + +fn get_source_text(sm: &SourceMap, sp: Range<BytePos>) -> Option<SourceFileRange> { + let start = sm.lookup_byte_offset(sp.start); + let end = sm.lookup_byte_offset(sp.end); + if !Lrc::ptr_eq(&start.sf, &end.sf) || start.pos > end.pos { + return None; + } + let range = start.pos.to_usize()..end.pos.to_usize(); + Some(SourceFileRange { sf: start.sf, range }) +} + +fn with_source_text<T>(sm: &SourceMap, sp: Range<BytePos>, f: impl for<'a> FnOnce(&'a str) -> T) -> Option<T> { + if let Some(src) = get_source_text(sm, sp) + && let Some(src) = src.as_str() + { + Some(f(src)) + } else { + None + } +} + +fn with_source_text_and_range<T>( + sm: &SourceMap, + sp: Range<BytePos>, + f: impl for<'a> FnOnce(&'a str, Range<usize>) -> T, +) -> Option<T> { + if let Some(src) = get_source_text(sm, sp) + && let Some(text) = &src.sf.src + { + Some(f(text, src.range)) + } else { + None + } +} + +#[expect(clippy::cast_possible_truncation)] +fn map_range( + sm: &SourceMap, + sp: Range<BytePos>, + f: impl for<'a> FnOnce(&'a str, Range<usize>) -> Option<Range<usize>>, +) -> Option<Range<BytePos>> { + if let Some(src) = get_source_text(sm, sp.clone()) + && let Some(text) = &src.sf.src + && let Some(range) = f(text, src.range.clone()) + { + debug_assert!( + range.start <= text.len() && range.end <= text.len(), + "Range `{range:?}` is outside the source file (file `{}`, length `{}`)", + src.sf.name.display(FileNameDisplayPreference::Local), + text.len(), + ); + debug_assert!(range.start <= range.end, "Range `{range:?}` has overlapping bounds"); + let dstart = (range.start as u32).wrapping_sub(src.range.start as u32); + let dend = (range.end as u32).wrapping_sub(src.range.start as u32); + Some(BytePos(sp.start.0.wrapping_add(dstart))..BytePos(sp.start.0.wrapping_add(dend))) + } else { + None + } +} + +fn with_leading_whitespace(sm: &SourceMap, sp: Range<BytePos>) -> Range<BytePos> { + map_range(sm, sp.clone(), |src, range| { + Some(src.get(..range.start)?.trim_end().len()..range.end) + }) + .unwrap_or(sp) +} + +fn trim_start(sm: &SourceMap, sp: Range<BytePos>) -> Range<BytePos> { + map_range(sm, sp.clone(), |src, range| { + let src = src.get(range.clone())?; + Some(range.start + (src.len() - src.trim_start().len())..range.end) + }) + .unwrap_or(sp) +} + +fn write_source_text_to(sm: &SourceMap, sp: Range<BytePos>, dst: &mut impl fmt::Write) -> fmt::Result { + match with_source_text(sm, sp, |src| dst.write_str(src)) { + Some(x) => x, + None => Err(fmt::Error), + } +} + pub struct SourceFileRange { pub sf: Lrc<SourceFile>, pub range: Range<usize>, @@ -46,21 +226,6 @@ impl SourceFileRange { } } -/// Gets the source file, and range in the file, of the given span. Returns `None` if the span -/// extends through multiple files, or is malformed. -pub fn get_source_text(cx: &impl LintContext, sp: impl SpanRange) -> Option<SourceFileRange> { - fn f(sm: &SourceMap, sp: Range<BytePos>) -> Option<SourceFileRange> { - let start = sm.lookup_byte_offset(sp.start); - let end = sm.lookup_byte_offset(sp.end); - if !Lrc::ptr_eq(&start.sf, &end.sf) || start.pos > end.pos { - return None; - } - let range = start.pos.to_usize()..end.pos.to_usize(); - Some(SourceFileRange { sf: start.sf, range }) - } - f(cx.sess().source_map(), sp.into_range()) -} - /// Like `snippet_block`, but add braces if the expr is not an `ExprKind::Block`. pub fn expr_block<T: LintContext>( cx: &T, diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index 3790a852f7e..e5d20564196 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -1349,3 +1349,17 @@ pub fn get_adt_inherent_method<'a>(cx: &'a LateContext<'_>, ty: Ty<'_>, method_n None } } + +/// Get's the type of a field by name. +pub fn get_field_by_name<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, name: Symbol) -> Option<Ty<'tcx>> { + match *ty.kind() { + ty::Adt(def, args) if def.is_union() || def.is_struct() => def + .non_enum_variant() + .fields + .iter() + .find(|f| f.name == name) + .map(|f| f.ty(tcx, args)), + ty::Tuple(args) => name.as_str().parse::<usize>().ok().and_then(|i| args.get(i).copied()), + _ => None, + } +} diff --git a/lintcheck/Cargo.toml b/lintcheck/Cargo.toml index 8c5a409e25b..e0e94d7fec3 100644 --- a/lintcheck/Cargo.toml +++ b/lintcheck/Cargo.toml @@ -11,28 +11,19 @@ publish = false default-run = "lintcheck" [dependencies] -anyhow = "1.0.69" cargo_metadata = "0.15.3" clap = { version = "4.4", features = ["derive", "env"] } -crates_io_api = "0.8.1" crossbeam-channel = "0.5.6" +diff = "0.1.13" flate2 = "1.0" -indicatif = "0.17.3" rayon = "1.5.1" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0.85" +strip-ansi-escapes = "0.1.1" tar = "0.4" toml = "0.7.3" -ureq = "2.2" +ureq = { version = "2.2", features = ["json"] } walkdir = "2.3" [features] deny-warnings = [] - -[[bin]] -name = "lintcheck" -path = "src/main.rs" - -[[bin]] -name = "popular-crates" -path = "src/popular-crates.rs" diff --git a/lintcheck/README.md b/lintcheck/README.md index 61b581ba0fa..2d6039caeef 100644 --- a/lintcheck/README.md +++ b/lintcheck/README.md @@ -26,11 +26,11 @@ the repo root. The results will then be saved to `lintcheck-logs/custom_logs.toml`. The `custom.toml` file may be built using <https://crates.io> recently most -downloaded crates by using the `popular-crates` binary from the `lintcheck` -directory. For example, to retrieve the 100 recently most downloaded crates: +downloaded crates by using `cargo lintcheck popular`. For example, to retrieve +the 200 recently most downloaded crates: ``` -cargo run --release --bin popular-crates -- -n 100 custom.toml +cargo lintcheck popular -n 200 custom.toml ``` diff --git a/lintcheck/src/config.rs b/lintcheck/src/config.rs index 3f712f453fa..e6cd7c9fdc2 100644 --- a/lintcheck/src/config.rs +++ b/lintcheck/src/config.rs @@ -1,8 +1,9 @@ -use clap::Parser; +use clap::{Parser, Subcommand, ValueEnum}; use std::num::NonZero; use std::path::PathBuf; -#[derive(Clone, Debug, Parser)] +#[derive(Parser, Clone, Debug)] +#[command(args_conflicts_with_subcommands = true)] pub(crate) struct LintcheckConfig { /// Number of threads to use (default: all unless --fix or --recursive) #[clap( @@ -35,12 +36,45 @@ pub(crate) struct LintcheckConfig { /// Apply a filter to only collect specified lints, this also overrides `allow` attributes #[clap(long = "filter", value_name = "clippy_lint_name", use_value_delimiter = true)] pub lint_filter: Vec<String>, - /// Change the reports table to use markdown links - #[clap(long)] - pub markdown: bool, + /// Set the output format of the log file + #[clap(long, short, default_value = "text")] + pub format: OutputFormat, /// Run clippy on the dependencies of crates specified in crates-toml #[clap(long, conflicts_with("max_jobs"))] pub recursive: bool, + #[command(subcommand)] + pub subcommand: Option<Commands>, +} + +#[derive(Subcommand, Clone, Debug)] +pub(crate) enum Commands { + /// Display a markdown diff between two lintcheck log files in JSON format + Diff { old: PathBuf, new: PathBuf }, + /// Create a lintcheck crates TOML file containing the top N popular crates + Popular { + /// Output TOML file name + output: PathBuf, + /// Number of crate names to download + #[clap(short, long, default_value_t = 100)] + number: usize, + }, +} + +#[derive(ValueEnum, Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum OutputFormat { + Text, + Markdown, + Json, +} + +impl OutputFormat { + fn file_extension(self) -> &'static str { + match self { + OutputFormat::Text => "txt", + OutputFormat::Markdown => "md", + OutputFormat::Json => "json", + } + } } impl LintcheckConfig { @@ -53,7 +87,7 @@ impl LintcheckConfig { config.lintcheck_results_path = PathBuf::from(format!( "lintcheck-logs/{}_logs.{}", filename.display(), - if config.markdown { "md" } else { "txt" } + config.format.file_extension(), )); // look at the --threads arg, if 0 is passed, use the threads count diff --git a/lintcheck/src/json.rs b/lintcheck/src/json.rs new file mode 100644 index 00000000000..43d0413c7ce --- /dev/null +++ b/lintcheck/src/json.rs @@ -0,0 +1,122 @@ +use std::collections::HashMap; +use std::fmt::Write; +use std::fs; +use std::hash::Hash; +use std::path::Path; + +use crate::ClippyWarning; + +/// Creates the log file output for [`crate::config::OutputFormat::Json`] +pub(crate) fn output(clippy_warnings: &[ClippyWarning]) -> String { + serde_json::to_string(&clippy_warnings).unwrap() +} + +fn load_warnings(path: &Path) -> Vec<ClippyWarning> { + let file = fs::read(path).unwrap_or_else(|e| panic!("failed to read {}: {e}", path.display())); + + serde_json::from_slice(&file).unwrap_or_else(|e| panic!("failed to deserialize {}: {e}", path.display())) +} + +/// Group warnings by their primary span location + lint name +fn create_map(warnings: &[ClippyWarning]) -> HashMap<impl Eq + Hash + '_, Vec<&ClippyWarning>> { + let mut map = HashMap::<_, Vec<_>>::with_capacity(warnings.len()); + + for warning in warnings { + let span = warning.span(); + let key = (&warning.lint_type, &span.file_name, span.byte_start, span.byte_end); + + map.entry(key).or_default().push(warning); + } + + map +} + +fn print_warnings(title: &str, warnings: &[&ClippyWarning]) { + if warnings.is_empty() { + return; + } + + println!("### {title}"); + println!("```"); + for warning in warnings { + print!("{}", warning.diag); + } + println!("```"); +} + +fn print_changed_diff(changed: &[(&[&ClippyWarning], &[&ClippyWarning])]) { + fn render(warnings: &[&ClippyWarning]) -> String { + let mut rendered = String::new(); + for warning in warnings { + write!(&mut rendered, "{}", warning.diag).unwrap(); + } + rendered + } + + if changed.is_empty() { + return; + } + + println!("### Changed"); + println!("```diff"); + for &(old, new) in changed { + let old_rendered = render(old); + let new_rendered = render(new); + + for change in diff::lines(&old_rendered, &new_rendered) { + use diff::Result::{Both, Left, Right}; + + match change { + Both(unchanged, _) => { + println!(" {unchanged}"); + }, + Left(removed) => { + println!("-{removed}"); + }, + Right(added) => { + println!("+{added}"); + }, + } + } + } + println!("```"); +} + +pub(crate) fn diff(old_path: &Path, new_path: &Path) { + let old_warnings = load_warnings(old_path); + let new_warnings = load_warnings(new_path); + + let old_map = create_map(&old_warnings); + let new_map = create_map(&new_warnings); + + let mut added = Vec::new(); + let mut removed = Vec::new(); + let mut changed = Vec::new(); + + for (key, new) in &new_map { + if let Some(old) = old_map.get(key) { + if old != new { + changed.push((old.as_slice(), new.as_slice())); + } + } else { + added.extend(new); + } + } + + for (key, old) in &old_map { + if !new_map.contains_key(key) { + removed.extend(old); + } + } + + print!( + "{} added, {} removed, {} changed\n\n", + added.len(), + removed.len(), + changed.len() + ); + + print_warnings("Added", &added); + print_warnings("Removed", &removed); + print_changed_diff(&changed); +} diff --git a/lintcheck/src/main.rs b/lintcheck/src/main.rs index deb06094d83..8dd6e95bb7d 100644 --- a/lintcheck/src/main.rs +++ b/lintcheck/src/main.rs @@ -12,26 +12,29 @@ unused_lifetimes, unused_qualifications )] -#![allow(clippy::collapsible_else_if)] +#![allow(clippy::collapsible_else_if, clippy::needless_borrows_for_generic_args)] mod config; mod driver; +mod json; +mod popular_crates; mod recursive; -use crate::config::LintcheckConfig; +use crate::config::{Commands, LintcheckConfig, OutputFormat}; use crate::recursive::LintcheckServer; use std::collections::{HashMap, HashSet}; use std::env::consts::EXE_SUFFIX; -use std::fmt::{self, Write as _}; +use std::fmt::{self, Display, Write as _}; +use std::hash::Hash; use std::io::{self, ErrorKind}; use std::path::{Path, PathBuf}; -use std::process::{Command, ExitStatus}; +use std::process::{Command, ExitStatus, Stdio}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::Duration; use std::{env, fs, thread}; -use cargo_metadata::diagnostic::Diagnostic; +use cargo_metadata::diagnostic::{Diagnostic, DiagnosticSpan}; use cargo_metadata::Message; use rayon::prelude::*; use serde::{Deserialize, Serialize}; @@ -41,21 +44,21 @@ const LINTCHECK_DOWNLOADS: &str = "target/lintcheck/downloads"; const LINTCHECK_SOURCES: &str = "target/lintcheck/sources"; /// List of sources to check, loaded from a .toml file -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Deserialize)] struct SourceList { crates: HashMap<String, TomlCrate>, #[serde(default)] recursive: RecursiveOptions, } -#[derive(Debug, Serialize, Deserialize, Default)] +#[derive(Debug, Deserialize, Default)] struct RecursiveOptions { ignore: HashSet<String>, } /// A crate source stored inside the .toml /// will be translated into on one of the `CrateSource` variants -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Deserialize)] struct TomlCrate { name: String, versions: Option<Vec<String>>, @@ -67,7 +70,7 @@ struct TomlCrate { /// Represents an archive we download from crates.io, or a git repo, or a local repo/folder /// Once processed (downloaded/extracted/cloned/copied...), this will be translated into a `Crate` -#[derive(Debug, Serialize, Deserialize, Eq, Hash, PartialEq, Ord, PartialOrd)] +#[derive(Debug, Deserialize, Eq, Hash, PartialEq, Ord, PartialOrd)] enum CrateSource { CratesIo { name: String, @@ -111,6 +114,17 @@ struct RustcIce { pub crate_name: String, pub ice_content: String, } + +impl Display for RustcIce { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "{}:\n{}\n========================================\n", + self.crate_name, self.ice_content + ) + } +} + impl RustcIce { pub fn from_stderr_and_status(crate_name: &str, status: ExitStatus, stderr: &str) -> Option<Self> { if status.code().unwrap_or(0) == 101 @@ -127,60 +141,58 @@ impl RustcIce { } /// A single warning that clippy issued while checking a `Crate` -#[derive(Debug)] +#[derive(Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] struct ClippyWarning { - file: String, - line: usize, - column: usize, + crate_name: String, + crate_version: String, lint_type: String, - message: String, + diag: Diagnostic, } #[allow(unused)] impl ClippyWarning { - fn new(diag: Diagnostic, crate_name: &str, crate_version: &str) -> Option<Self> { - let lint_type = diag.code?.code; + fn new(mut diag: Diagnostic, crate_name: &str, crate_version: &str) -> Option<Self> { + let lint_type = diag.code.clone()?.code; if !(lint_type.contains("clippy") || diag.message.contains("clippy")) || diag.message.contains("could not read cargo metadata") { return None; } - let span = diag.spans.into_iter().find(|span| span.is_primary)?; - - let file = if let Ok(stripped) = Path::new(&span.file_name).strip_prefix(env!("CARGO_HOME")) { - format!("$CARGO_HOME/{}", stripped.display()) - } else { - format!( - "target/lintcheck/sources/{crate_name}-{crate_version}/{}", - span.file_name - ) - }; + // --recursive bypasses cargo so we have to strip the rendered output ourselves + let rendered = diag.rendered.as_mut().unwrap(); + *rendered = String::from_utf8(strip_ansi_escapes::strip(&rendered).unwrap()).unwrap(); Some(Self { - file, - line: span.line_start, - column: span.column_start, + crate_name: crate_name.to_owned(), + crate_version: crate_version.to_owned(), lint_type, - message: diag.message, + diag, }) } - fn to_output(&self, markdown: bool) -> String { - let file_with_pos = format!("{}:{}:{}", &self.file, &self.line, &self.column); - if markdown { - let mut file = self.file.clone(); - if !file.starts_with('$') { - file.insert_str(0, "../"); - } + fn span(&self) -> &DiagnosticSpan { + self.diag.spans.iter().find(|span| span.is_primary).unwrap() + } - let mut output = String::from("| "); - let _: fmt::Result = write!(output, "[`{file_with_pos}`]({file}#L{})", self.line); - let _: fmt::Result = write!(output, r#" | `{:<50}` | "{}" |"#, self.lint_type, self.message); - output.push('\n'); - output - } else { - format!("{file_with_pos} {} \"{}\"\n", self.lint_type, self.message) + fn to_output(&self, format: OutputFormat) -> String { + let span = self.span(); + let mut file = span.file_name.clone(); + let file_with_pos = format!("{file}:{}:{}", span.line_start, span.line_end); + match format { + OutputFormat::Text => format!("{file_with_pos} {} \"{}\"\n", self.lint_type, self.diag.message), + OutputFormat::Markdown => { + if file.starts_with("target") { + file.insert_str(0, "../"); + } + + let mut output = String::from("| "); + write!(output, "[`{file_with_pos}`]({file}#L{})", span.line_start).unwrap(); + write!(output, r#" | `{:<50}` | "{}" |"#, self.lint_type, self.diag.message).unwrap(); + output.push('\n'); + output + }, + OutputFormat::Json => unreachable!("JSON output is handled via serde"), } } } @@ -333,10 +345,9 @@ impl CrateSource { impl Crate { /// Run `cargo clippy` on the `Crate` and collect and return all the lint warnings that clippy /// issued - #[allow(clippy::too_many_arguments)] + #[allow(clippy::too_many_arguments, clippy::too_many_lines)] fn run_clippy_lints( &self, - cargo_clippy_path: &Path, clippy_driver_path: &Path, target_dir_index: &AtomicUsize, total_crates_to_lint: usize, @@ -362,17 +373,27 @@ impl Crate { ); } - let cargo_clippy_path = fs::canonicalize(cargo_clippy_path).unwrap(); - let shared_target_dir = clippy_project_root().join("target/lintcheck/shared_target_dir"); - let mut cargo_clippy_args = if config.fix { - vec!["--quiet", "--fix", "--"] - } else { - vec!["--quiet", "--message-format=json", "--"] - }; + let cargo_home = env!("CARGO_HOME"); + + // `src/lib.rs` -> `target/lintcheck/sources/crate-1.2.3/src/lib.rs` + let remap_relative = format!("={}", self.path.display()); + // Fallback for other sources, `~/.cargo/...` -> `$CARGO_HOME/...` + let remap_cargo_home = format!("{cargo_home}=$CARGO_HOME"); + // `~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crate-2.3.4/src/lib.rs` + // -> `crate-2.3.4/src/lib.rs` + let remap_crates_io = format!("{cargo_home}/registry/src/index.crates.io-6f17d22bba15001f/="); + + let mut clippy_args = vec![ + "--remap-path-prefix", + &remap_relative, + "--remap-path-prefix", + &remap_cargo_home, + "--remap-path-prefix", + &remap_crates_io, + ]; - let mut clippy_args = Vec::<&str>::new(); if let Some(options) = &self.options { for opt in options { clippy_args.push(opt); @@ -388,23 +409,23 @@ impl Crate { clippy_args.extend(lint_filter.iter().map(String::as_str)); } - if let Some(server) = server { - let target = shared_target_dir.join("recursive"); + let mut cmd = Command::new("cargo"); + cmd.arg(if config.fix { "fix" } else { "check" }) + .arg("--quiet") + .current_dir(&self.path) + .env("CLIPPY_ARGS", clippy_args.join("__CLIPPY_HACKERY__")); + if let Some(server) = server { // `cargo clippy` is a wrapper around `cargo check` that mainly sets `RUSTC_WORKSPACE_WRAPPER` to // `clippy-driver`. We do the same thing here with a couple changes: // // `RUSTC_WRAPPER` is used instead of `RUSTC_WORKSPACE_WRAPPER` so that we can lint all crate // dependencies rather than only workspace members // - // The wrapper is set to the `lintcheck` so we can force enable linting and ignore certain crates + // The wrapper is set to `lintcheck` itself so we can force enable linting and ignore certain crates // (see `crate::driver`) - let status = Command::new(env::var("CARGO").unwrap_or("cargo".into())) - .arg("check") - .arg("--quiet") - .current_dir(&self.path) - .env("CLIPPY_ARGS", clippy_args.join("__CLIPPY_HACKERY__")) - .env("CARGO_TARGET_DIR", target) + let status = cmd + .env("CARGO_TARGET_DIR", shared_target_dir.join("recursive")) .env("RUSTC_WRAPPER", env::current_exe().unwrap()) // Pass the absolute path so `crate::driver` can find `clippy-driver`, as it's executed in various // different working directories @@ -416,23 +437,19 @@ impl Crate { assert_eq!(status.code(), Some(0)); return Vec::new(); - } + }; - cargo_clippy_args.extend(clippy_args); + if !config.fix { + cmd.arg("--message-format=json"); + } - let all_output = Command::new(&cargo_clippy_path) + let all_output = cmd // use the looping index to create individual target dirs .env("CARGO_TARGET_DIR", shared_target_dir.join(format!("_{thread_index:?}"))) - .args(&cargo_clippy_args) - .current_dir(&self.path) + // Roughly equivalent to `cargo clippy`/`cargo clippy --fix` + .env("RUSTC_WORKSPACE_WRAPPER", clippy_driver_path) .output() - .unwrap_or_else(|error| { - panic!( - "Encountered error:\n{error:?}\ncargo_clippy_path: {}\ncrate path:{}\n", - &cargo_clippy_path.display(), - &self.path.display() - ); - }); + .unwrap(); let stdout = String::from_utf8_lossy(&all_output.stdout); let stderr = String::from_utf8_lossy(&all_output.stderr); let status = &all_output.status; @@ -479,15 +496,17 @@ impl Crate { } /// Builds clippy inside the repo to make sure we have a clippy executable we can use. -fn build_clippy() { - let status = Command::new(env::var("CARGO").unwrap_or("cargo".into())) - .arg("build") - .status() - .expect("Failed to build clippy!"); - if !status.success() { +fn build_clippy() -> String { + let output = Command::new("cargo") + .args(["run", "--bin=clippy-driver", "--", "--version"]) + .stderr(Stdio::inherit()) + .output() + .unwrap(); + if !output.status.success() { eprintln!("Error: Failed to compile Clippy!"); std::process::exit(1); } + String::from_utf8_lossy(&output.stdout).into_owned() } /// Read a `lintcheck_crates.toml` file @@ -554,10 +573,10 @@ fn read_crates(toml_path: &Path) -> (Vec<CrateSource>, RecursiveOptions) { } /// Generate a short list of occurring lints-types and their count -fn gather_stats(clippy_warnings: &[ClippyWarning]) -> (String, HashMap<&String, usize>) { +fn gather_stats(warnings: &[ClippyWarning]) -> (String, HashMap<&String, usize>) { // count lint type occurrences let mut counter: HashMap<&String, usize> = HashMap::new(); - clippy_warnings + warnings .iter() .for_each(|wrn| *counter.entry(&wrn.lint_type).or_insert(0) += 1); @@ -580,7 +599,6 @@ fn gather_stats(clippy_warnings: &[ClippyWarning]) -> (String, HashMap<&String, (stats_string, counter) } -#[allow(clippy::too_many_lines)] fn main() { // We're being executed as a `RUSTC_WRAPPER` as part of `--recursive` if let Ok(addr) = env::var("LINTCHECK_SERVER") { @@ -595,31 +613,29 @@ fn main() { let config = LintcheckConfig::new(); - println!("Compiling clippy..."); - build_clippy(); - println!("Done compiling"); + match config.subcommand { + Some(Commands::Diff { old, new }) => json::diff(&old, &new), + Some(Commands::Popular { output, number }) => popular_crates::fetch(output, number).unwrap(), + None => lintcheck(config), + } +} - let cargo_clippy_path = fs::canonicalize(format!("target/debug/cargo-clippy{EXE_SUFFIX}")).unwrap(); +#[allow(clippy::too_many_lines)] +fn lintcheck(config: LintcheckConfig) { + let clippy_ver = build_clippy(); let clippy_driver_path = fs::canonicalize(format!("target/debug/clippy-driver{EXE_SUFFIX}")).unwrap(); // assert that clippy is found assert!( - cargo_clippy_path.is_file(), - "target/debug/cargo-clippy binary not found! {}", - cargo_clippy_path.display() + clippy_driver_path.is_file(), + "target/debug/clippy-driver binary not found! {}", + clippy_driver_path.display() ); - let clippy_ver = Command::new(&cargo_clippy_path) - .arg("--version") - .output() - .map(|o| String::from_utf8_lossy(&o.stdout).into_owned()) - .expect("could not get clippy version!"); - // download and extract the crates, then run clippy on them and collect clippy's warnings // flatten into one big list of warnings let (crates, recursive_options) = read_crates(&config.sources_toml_path); - let old_stats = read_stats_from_file(&config.lintcheck_results_path); let counter = AtomicUsize::new(1); let lint_filter: Vec<String> = config @@ -678,7 +694,6 @@ fn main() { .par_iter() .flat_map(|krate| { krate.run_clippy_lints( - &cargo_clippy_path, &clippy_driver_path, &counter, crates.len(), @@ -711,39 +726,51 @@ fn main() { } } + let text = match config.format { + OutputFormat::Text | OutputFormat::Markdown => output(&warnings, &raw_ices, clippy_ver, &config), + OutputFormat::Json => { + if !raw_ices.is_empty() { + for ice in raw_ices { + println!("{ice}"); + } + panic!("Some crates ICEd"); + } + + json::output(&warnings) + }, + }; + + println!("Writing logs to {}", config.lintcheck_results_path.display()); + fs::create_dir_all(config.lintcheck_results_path.parent().unwrap()).unwrap(); + fs::write(&config.lintcheck_results_path, text).unwrap(); +} + +/// Creates the log file output for [`OutputFormat::Text`] and [`OutputFormat::Markdown`] +fn output(warnings: &[ClippyWarning], ices: &[RustcIce], clippy_ver: String, config: &LintcheckConfig) -> String { // generate some stats - let (stats_formatted, new_stats) = gather_stats(&warnings); + let (stats_formatted, new_stats) = gather_stats(warnings); + let old_stats = read_stats_from_file(&config.lintcheck_results_path); - let mut all_msgs: Vec<String> = warnings.iter().map(|warn| warn.to_output(config.markdown)).collect(); + let mut all_msgs: Vec<String> = warnings.iter().map(|warn| warn.to_output(config.format)).collect(); all_msgs.sort(); all_msgs.push("\n\n### Stats:\n\n".into()); all_msgs.push(stats_formatted); - // save the text into lintcheck-logs/logs.txt let mut text = clippy_ver; // clippy version number on top text.push_str("\n### Reports\n\n"); - if config.markdown { + if config.format == OutputFormat::Markdown { text.push_str("| file | lint | message |\n"); text.push_str("| --- | --- | --- |\n"); } write!(text, "{}", all_msgs.join("")).unwrap(); text.push_str("\n\n### ICEs:\n"); - for ice in &raw_ices { - let _: fmt::Result = write!( - text, - "{}:\n{}\n========================================\n\n", - ice.crate_name, ice.ice_content - ); + for ice in ices { + writeln!(text, "{ice}").unwrap(); } - println!("Writing logs to {}", config.lintcheck_results_path.display()); - if !raw_ices.is_empty() { - println!("WARNING: at least one ICE reported, check log file"); - } - fs::create_dir_all(config.lintcheck_results_path.parent().unwrap()).unwrap(); - fs::write(&config.lintcheck_results_path, text).unwrap(); - print_stats(old_stats, new_stats, &config.lint_filter); + + text } /// read the previous stats from the lintcheck-log file @@ -865,7 +892,7 @@ fn lintcheck_test() { "--crates-toml", "lintcheck/test_sources.toml", ]; - let status = Command::new(env::var("CARGO").unwrap_or("cargo".into())) + let status = Command::new("cargo") .args(args) .current_dir("..") // repo root .status(); diff --git a/lintcheck/src/popular-crates.rs b/lintcheck/src/popular-crates.rs deleted file mode 100644 index fdab984ad86..00000000000 --- a/lintcheck/src/popular-crates.rs +++ /dev/null @@ -1,65 +0,0 @@ -#![deny(clippy::pedantic)] - -use clap::Parser; -use crates_io_api::{CratesQueryBuilder, Sort, SyncClient}; -use indicatif::ProgressBar; -use std::collections::HashSet; -use std::fs::File; -use std::io::{BufWriter, Write}; -use std::path::PathBuf; -use std::time::Duration; - -#[derive(Parser)] -struct Opts { - /// Output TOML file name - output: PathBuf, - /// Number of crate names to download - #[clap(short, long, default_value_t = 100)] - number: usize, - /// Do not output progress - #[clap(short, long)] - quiet: bool, -} - -fn main() -> anyhow::Result<()> { - let opts = Opts::parse(); - let mut output = BufWriter::new(File::create(opts.output)?); - output.write_all(b"[crates]\n")?; - let client = SyncClient::new( - "clippy/lintcheck (github.com/rust-lang/rust-clippy/)", - Duration::from_secs(1), - )?; - let mut seen_crates = HashSet::new(); - let pb = if opts.quiet { - None - } else { - Some(ProgressBar::new(opts.number as u64)) - }; - let mut query = CratesQueryBuilder::new() - .sort(Sort::RecentDownloads) - .page_size(100) - .build(); - while seen_crates.len() < opts.number { - let retrieved = client.crates(query.clone())?.crates; - if retrieved.is_empty() { - eprintln!("No more than {} crates available from API", seen_crates.len()); - break; - } - for c in retrieved { - if seen_crates.insert(c.name.clone()) { - output.write_all( - format!( - "{} = {{ name = '{}', versions = ['{}'] }}\n", - c.name, c.name, c.max_version - ) - .as_bytes(), - )?; - if let Some(pb) = &pb { - pb.inc(1); - } - } - } - query.set_page(query.page() + 1); - } - Ok(()) -} diff --git a/lintcheck/src/popular_crates.rs b/lintcheck/src/popular_crates.rs new file mode 100644 index 00000000000..880a8bd81f0 --- /dev/null +++ b/lintcheck/src/popular_crates.rs @@ -0,0 +1,52 @@ +use serde::Deserialize; +use std::error::Error; +use std::fmt::Write; +use std::fs; +use std::path::PathBuf; + +#[derive(Deserialize, Debug)] +struct Page { + crates: Vec<Crate>, + meta: Meta, +} + +#[derive(Deserialize, Debug)] +struct Crate { + name: String, + max_version: String, +} + +#[derive(Deserialize, Debug)] +struct Meta { + next_page: String, +} + +pub(crate) fn fetch(output: PathBuf, number: usize) -> Result<(), Box<dyn Error>> { + let agent = ureq::builder() + .user_agent("clippy/lintcheck (github.com/rust-lang/rust-clippy/)") + .build(); + + let mut crates = Vec::with_capacity(number); + let mut query = "?sort=recent-downloads&per_page=100".to_string(); + while crates.len() < number { + let page: Page = agent + .get(&format!("https://crates.io/api/v1/crates{query}")) + .call()? + .into_json()?; + + query = page.meta.next_page; + crates.extend(page.crates); + crates.truncate(number); + + let width = number.ilog10() as usize + 1; + println!("Fetched {:>width$}/{number} crates", crates.len()); + } + + let mut out = "[crates]\n".to_string(); + for Crate { name, max_version } in crates { + writeln!(out, "{name} = {{ name = '{name}', versions = ['{max_version}'] }}").unwrap(); + } + fs::write(output, out)?; + + Ok(()) +} diff --git a/src/driver.rs b/src/driver.rs index 9b1577f24b8..3fafe2427a2 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -2,7 +2,6 @@ #![allow(rustc::untranslatable_diagnostic)] #![feature(rustc_private)] #![feature(let_chains)] -#![cfg_attr(bootstrap, feature(lint_reasons))] #![cfg_attr(feature = "deny-warnings", deny(warnings))] // warn on lints, that are included in `rust-lang/rust`s bootstrap #![warn(rust_2018_idioms, unused_lifetimes)] diff --git a/tests/integration.rs b/tests/integration.rs index 7f4500826ff..19c5f3a4133 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -55,6 +55,7 @@ fn integration_test() { "clippy", "--all-targets", "--all-features", + "--message-format=short", "--", "--cap-lints", "warn", diff --git a/tests/ui-cargo/duplicate_mod/fail/Cargo.stderr b/tests/ui-cargo/duplicate_mod/fail/Cargo.stderr index 912ea9bb4d1..b626551e35b 100644 --- a/tests/ui-cargo/duplicate_mod/fail/Cargo.stderr +++ b/tests/ui-cargo/duplicate_mod/fail/Cargo.stderr @@ -1,10 +1,10 @@ error: file is loaded as a module multiple times: `src/b.rs` - --> src/main.rs:5:1 + --> src/main.rs:3:1 | -5 | mod b; +3 | mod b; | ^^^^^^ first loaded here -6 | / #[path = "b.rs"] -7 | | mod b2; +4 | / #[path = "b.rs"] +5 | | mod b2; | |_______^ loaded again here | = help: replace all but one `mod` item with `use` items @@ -12,34 +12,34 @@ error: file is loaded as a module multiple times: `src/b.rs` = help: to override `-D warnings` add `#[allow(clippy::duplicate_mod)]` error: file is loaded as a module multiple times: `src/c.rs` - --> src/main.rs:9:1 + --> src/main.rs:7:1 | -9 | mod c; +7 | mod c; | ^^^^^^ first loaded here -10 | / #[path = "c.rs"] -11 | | mod c2; +8 | / #[path = "c.rs"] +9 | | mod c2; | |_______^ loaded again here -12 | / #[path = "c.rs"] -13 | | mod c3; +10 | / #[path = "c.rs"] +11 | | mod c3; | |_______^ loaded again here | = help: replace all but one `mod` item with `use` items error: file is loaded as a module multiple times: `src/d.rs` - --> src/main.rs:18:1 + --> src/main.rs:16:1 | -18 | mod d; +16 | mod d; | ^^^^^^ first loaded here -19 | / #[path = "d.rs"] -20 | | mod d2; +17 | / #[path = "d.rs"] +18 | | mod d2; | |_______^ loaded again here | = help: replace all but one `mod` item with `use` items error: file is loaded as a module multiple times: `src/from_other_module.rs` - --> src/main.rs:15:1 + --> src/main.rs:13:1 | -15 | mod from_other_module; +13 | mod from_other_module; | ^^^^^^^^^^^^^^^^^^^^^^ first loaded here | ::: src/other_module/mod.rs:1:1 diff --git a/tests/ui-cargo/duplicate_mod/fail/src/main.rs b/tests/ui-cargo/duplicate_mod/fail/src/main.rs index a99fe2e12bb..9cbe4b83b25 100644 --- a/tests/ui-cargo/duplicate_mod/fail/src/main.rs +++ b/tests/ui-cargo/duplicate_mod/fail/src/main.rs @@ -1,4 +1,3 @@ - mod a; mod b; diff --git a/tests/ui-internal/unnecessary_def_path_hardcoded_path.stderr b/tests/ui-internal/unnecessary_def_path_hardcoded_path.stderr index 9d7c00088fe..e4575d99d03 100644 --- a/tests/ui-internal/unnecessary_def_path_hardcoded_path.stderr +++ b/tests/ui-internal/unnecessary_def_path_hardcoded_path.stderr @@ -19,8 +19,8 @@ LL | const DEREF_MUT_TRAIT: [&str; 4] = ["core", "ops", "deref", "DerefMut"] error: hardcoded path to a diagnostic item --> tests/ui-internal/unnecessary_def_path_hardcoded_path.rs:12:43 | -LL | const OPS_MOD: [&str; 5] = ["core", "ops"]; - | ^^^^^^^^^^^^^^^ +LL | const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: convert all references to use `sym::deref_method` diff --git a/tests/ui/assigning_clones.fixed b/tests/ui/assigning_clones.fixed index 70ab43b49b3..60f6a385fc5 100644 --- a/tests/ui/assigning_clones.fixed +++ b/tests/ui/assigning_clones.fixed @@ -272,3 +272,59 @@ impl<'a> Add for &'a mut HasCloneFrom { self } } + +mod borrowck_conflicts { + //! Cases where clone_into and friends cannot be used because the src/dest have conflicting + //! borrows. + use std::path::PathBuf; + + fn issue12444(mut name: String) { + let parts = name.split(", ").collect::<Vec<_>>(); + let first = *parts.first().unwrap(); + name = first.to_owned(); + } + + fn issue12444_simple() { + let mut s = String::new(); + let s2 = &s; + s = s2.to_owned(); + } + + fn issue12444_nodrop_projections() { + struct NoDrop; + + impl Clone for NoDrop { + fn clone(&self) -> Self { + todo!() + } + fn clone_from(&mut self, other: &Self) { + todo!() + } + } + + let mut s = NoDrop; + let s2 = &s; + s = s2.clone(); + + let mut s = (NoDrop, NoDrop); + let s2 = &s.0; + s.0 = s2.clone(); + + // This *could* emit a warning, but PossibleBorrowerMap only works with locals so it + // considers `s` fully borrowed + let mut s = (NoDrop, NoDrop); + let s2 = &s.1; + s.0 = s2.clone(); + } + + fn issue12460(mut name: String) { + if let Some(stripped_name) = name.strip_prefix("baz-") { + name = stripped_name.to_owned(); + } + } + + fn issue12749() { + let mut path = PathBuf::from("/a/b/c"); + path = path.components().as_path().to_owned(); + } +} diff --git a/tests/ui/assigning_clones.rs b/tests/ui/assigning_clones.rs index 9699fed100c..6eb85be511a 100644 --- a/tests/ui/assigning_clones.rs +++ b/tests/ui/assigning_clones.rs @@ -272,3 +272,59 @@ impl<'a> Add for &'a mut HasCloneFrom { self } } + +mod borrowck_conflicts { + //! Cases where clone_into and friends cannot be used because the src/dest have conflicting + //! borrows. + use std::path::PathBuf; + + fn issue12444(mut name: String) { + let parts = name.split(", ").collect::<Vec<_>>(); + let first = *parts.first().unwrap(); + name = first.to_owned(); + } + + fn issue12444_simple() { + let mut s = String::new(); + let s2 = &s; + s = s2.to_owned(); + } + + fn issue12444_nodrop_projections() { + struct NoDrop; + + impl Clone for NoDrop { + fn clone(&self) -> Self { + todo!() + } + fn clone_from(&mut self, other: &Self) { + todo!() + } + } + + let mut s = NoDrop; + let s2 = &s; + s = s2.clone(); + + let mut s = (NoDrop, NoDrop); + let s2 = &s.0; + s.0 = s2.clone(); + + // This *could* emit a warning, but PossibleBorrowerMap only works with locals so it + // considers `s` fully borrowed + let mut s = (NoDrop, NoDrop); + let s2 = &s.1; + s.0 = s2.clone(); + } + + fn issue12460(mut name: String) { + if let Some(stripped_name) = name.strip_prefix("baz-") { + name = stripped_name.to_owned(); + } + } + + fn issue12749() { + let mut path = PathBuf::from("/a/b/c"); + path = path.components().as_path().to_owned(); + } +} diff --git a/tests/ui/auxiliary/external_consts.rs b/tests/ui/auxiliary/external_consts.rs new file mode 100644 index 00000000000..1885ba341f5 --- /dev/null +++ b/tests/ui/auxiliary/external_consts.rs @@ -0,0 +1 @@ +pub const MAGIC_NUMBER: i32 = 1; diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index c39f65a43e3..146b04ab813 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -12,6 +12,7 @@ #![allow( clippy::cast_abs_to_unsigned, clippy::no_effect, + clippy::unnecessary_min_or_max, clippy::unnecessary_operation, clippy::unnecessary_literal_unwrap, clippy::identity_op diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index 452482fc88e..7824bdfac25 100644 --- a/tests/ui/cast.stderr +++ b/tests/ui/cast.stderr @@ -1,5 +1,5 @@ error: casting `i32` to `f32` causes a loss of precision (`i32` is 32 bits wide, but `f32`'s mantissa is only 23 bits wide) - --> tests/ui/cast.rs:25:5 + --> tests/ui/cast.rs:26:5 | LL | x0 as f32; | ^^^^^^^^^ @@ -8,37 +8,37 @@ LL | x0 as f32; = help: to override `-D warnings` add `#[allow(clippy::cast_precision_loss)]` error: casting `i64` to `f32` causes a loss of precision (`i64` is 64 bits wide, but `f32`'s mantissa is only 23 bits wide) - --> tests/ui/cast.rs:29:5 + --> tests/ui/cast.rs:30:5 | LL | x1 as f32; | ^^^^^^^^^ error: casting `i64` to `f64` causes a loss of precision (`i64` is 64 bits wide, but `f64`'s mantissa is only 52 bits wide) - --> tests/ui/cast.rs:31:5 + --> tests/ui/cast.rs:32:5 | LL | x1 as f64; | ^^^^^^^^^ error: casting `u32` to `f32` causes a loss of precision (`u32` is 32 bits wide, but `f32`'s mantissa is only 23 bits wide) - --> tests/ui/cast.rs:34:5 + --> tests/ui/cast.rs:35:5 | LL | x2 as f32; | ^^^^^^^^^ error: casting `u64` to `f32` causes a loss of precision (`u64` is 64 bits wide, but `f32`'s mantissa is only 23 bits wide) - --> tests/ui/cast.rs:37:5 + --> tests/ui/cast.rs:38:5 | LL | x3 as f32; | ^^^^^^^^^ error: casting `u64` to `f64` causes a loss of precision (`u64` is 64 bits wide, but `f64`'s mantissa is only 52 bits wide) - --> tests/ui/cast.rs:39:5 + --> tests/ui/cast.rs:40:5 | LL | x3 as f64; | ^^^^^^^^^ error: casting `f32` to `i32` may truncate the value - --> tests/ui/cast.rs:42:5 + --> tests/ui/cast.rs:43:5 | LL | 1f32 as i32; | ^^^^^^^^^^^ @@ -48,7 +48,7 @@ LL | 1f32 as i32; = help: to override `-D warnings` add `#[allow(clippy::cast_possible_truncation)]` error: casting `f32` to `u32` may truncate the value - --> tests/ui/cast.rs:44:5 + --> tests/ui/cast.rs:45:5 | LL | 1f32 as u32; | ^^^^^^^^^^^ @@ -56,7 +56,7 @@ LL | 1f32 as u32; = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... error: casting `f32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:44:5 + --> tests/ui/cast.rs:45:5 | LL | 1f32 as u32; | ^^^^^^^^^^^ @@ -65,7 +65,7 @@ LL | 1f32 as u32; = help: to override `-D warnings` add `#[allow(clippy::cast_sign_loss)]` error: casting `f64` to `f32` may truncate the value - --> tests/ui/cast.rs:48:5 + --> tests/ui/cast.rs:49:5 | LL | 1f64 as f32; | ^^^^^^^^^^^ @@ -73,7 +73,7 @@ LL | 1f64 as f32; = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... error: casting `i32` to `i8` may truncate the value - --> tests/ui/cast.rs:50:5 + --> tests/ui/cast.rs:51:5 | LL | 1i32 as i8; | ^^^^^^^^^^ @@ -85,7 +85,7 @@ LL | i8::try_from(1i32); | ~~~~~~~~~~~~~~~~~~ error: casting `i32` to `u8` may truncate the value - --> tests/ui/cast.rs:52:5 + --> tests/ui/cast.rs:53:5 | LL | 1i32 as u8; | ^^^^^^^^^^ @@ -97,7 +97,7 @@ LL | u8::try_from(1i32); | ~~~~~~~~~~~~~~~~~~ error: casting `f64` to `isize` may truncate the value - --> tests/ui/cast.rs:54:5 + --> tests/ui/cast.rs:55:5 | LL | 1f64 as isize; | ^^^^^^^^^^^^^ @@ -105,7 +105,7 @@ LL | 1f64 as isize; = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... error: casting `f64` to `usize` may truncate the value - --> tests/ui/cast.rs:56:5 + --> tests/ui/cast.rs:57:5 | LL | 1f64 as usize; | ^^^^^^^^^^^^^ @@ -113,13 +113,13 @@ LL | 1f64 as usize; = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... error: casting `f64` to `usize` may lose the sign of the value - --> tests/ui/cast.rs:56:5 + --> tests/ui/cast.rs:57:5 | LL | 1f64 as usize; | ^^^^^^^^^^^^^ error: casting `u32` to `u16` may truncate the value - --> tests/ui/cast.rs:59:5 + --> tests/ui/cast.rs:60:5 | LL | 1f32 as u32 as u16; | ^^^^^^^^^^^^^^^^^^ @@ -131,7 +131,7 @@ LL | u16::try_from(1f32 as u32); | ~~~~~~~~~~~~~~~~~~~~~~~~~~ error: casting `f32` to `u32` may truncate the value - --> tests/ui/cast.rs:59:5 + --> tests/ui/cast.rs:60:5 | LL | 1f32 as u32 as u16; | ^^^^^^^^^^^ @@ -139,13 +139,13 @@ LL | 1f32 as u32 as u16; = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... error: casting `f32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:59:5 + --> tests/ui/cast.rs:60:5 | LL | 1f32 as u32 as u16; | ^^^^^^^^^^^ error: casting `i32` to `i8` may truncate the value - --> tests/ui/cast.rs:64:22 + --> tests/ui/cast.rs:65:22 | LL | let _x: i8 = 1i32 as _; | ^^^^^^^^^ @@ -157,7 +157,7 @@ LL | let _x: i8 = 1i32.try_into(); | ~~~~~~~~~~~~~~~ error: casting `f32` to `i32` may truncate the value - --> tests/ui/cast.rs:66:9 + --> tests/ui/cast.rs:67:9 | LL | 1f32 as i32; | ^^^^^^^^^^^ @@ -165,7 +165,7 @@ LL | 1f32 as i32; = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... error: casting `f64` to `i32` may truncate the value - --> tests/ui/cast.rs:68:9 + --> tests/ui/cast.rs:69:9 | LL | 1f64 as i32; | ^^^^^^^^^^^ @@ -173,7 +173,7 @@ LL | 1f64 as i32; = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... error: casting `f32` to `u8` may truncate the value - --> tests/ui/cast.rs:70:9 + --> tests/ui/cast.rs:71:9 | LL | 1f32 as u8; | ^^^^^^^^^^ @@ -181,13 +181,13 @@ LL | 1f32 as u8; = help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ... error: casting `f32` to `u8` may lose the sign of the value - --> tests/ui/cast.rs:70:9 + --> tests/ui/cast.rs:71:9 | LL | 1f32 as u8; | ^^^^^^^^^^ error: casting `u8` to `i8` may wrap around the value - --> tests/ui/cast.rs:75:5 + --> tests/ui/cast.rs:76:5 | LL | 1u8 as i8; | ^^^^^^^^^ @@ -196,31 +196,31 @@ LL | 1u8 as i8; = help: to override `-D warnings` add `#[allow(clippy::cast_possible_wrap)]` error: casting `u16` to `i16` may wrap around the value - --> tests/ui/cast.rs:78:5 + --> tests/ui/cast.rs:79:5 | LL | 1u16 as i16; | ^^^^^^^^^^^ error: casting `u32` to `i32` may wrap around the value - --> tests/ui/cast.rs:80:5 + --> tests/ui/cast.rs:81:5 | LL | 1u32 as i32; | ^^^^^^^^^^^ error: casting `u64` to `i64` may wrap around the value - --> tests/ui/cast.rs:82:5 + --> tests/ui/cast.rs:83:5 | LL | 1u64 as i64; | ^^^^^^^^^^^ error: casting `usize` to `isize` may wrap around the value - --> tests/ui/cast.rs:84:5 + --> tests/ui/cast.rs:85:5 | LL | 1usize as isize; | ^^^^^^^^^^^^^^^ error: casting `usize` to `i8` may truncate the value - --> tests/ui/cast.rs:87:5 + --> tests/ui/cast.rs:88:5 | LL | 1usize as i8; | ^^^^^^^^^^^^ @@ -232,7 +232,7 @@ LL | i8::try_from(1usize); | ~~~~~~~~~~~~~~~~~~~~ error: casting `usize` to `i16` may truncate the value - --> tests/ui/cast.rs:90:5 + --> tests/ui/cast.rs:91:5 | LL | 1usize as i16; | ^^^^^^^^^^^^^ @@ -244,7 +244,7 @@ LL | i16::try_from(1usize); | ~~~~~~~~~~~~~~~~~~~~~ error: casting `usize` to `i16` may wrap around the value on targets with 16-bit wide pointers - --> tests/ui/cast.rs:90:5 + --> tests/ui/cast.rs:91:5 | LL | 1usize as i16; | ^^^^^^^^^^^^^ @@ -253,7 +253,7 @@ LL | 1usize as i16; = note: for more information see https://doc.rust-lang.org/reference/types/numeric.html#machine-dependent-integer-types error: casting `usize` to `i32` may truncate the value on targets with 64-bit wide pointers - --> tests/ui/cast.rs:95:5 + --> tests/ui/cast.rs:96:5 | LL | 1usize as i32; | ^^^^^^^^^^^^^ @@ -265,19 +265,19 @@ LL | i32::try_from(1usize); | ~~~~~~~~~~~~~~~~~~~~~ error: casting `usize` to `i32` may wrap around the value on targets with 32-bit wide pointers - --> tests/ui/cast.rs:95:5 + --> tests/ui/cast.rs:96:5 | LL | 1usize as i32; | ^^^^^^^^^^^^^ error: casting `usize` to `i64` may wrap around the value on targets with 64-bit wide pointers - --> tests/ui/cast.rs:99:5 + --> tests/ui/cast.rs:100:5 | LL | 1usize as i64; | ^^^^^^^^^^^^^ error: casting `u16` to `isize` may wrap around the value on targets with 16-bit wide pointers - --> tests/ui/cast.rs:104:5 + --> tests/ui/cast.rs:105:5 | LL | 1u16 as isize; | ^^^^^^^^^^^^^ @@ -286,13 +286,13 @@ LL | 1u16 as isize; = note: for more information see https://doc.rust-lang.org/reference/types/numeric.html#machine-dependent-integer-types error: casting `u32` to `isize` may wrap around the value on targets with 32-bit wide pointers - --> tests/ui/cast.rs:108:5 + --> tests/ui/cast.rs:109:5 | LL | 1u32 as isize; | ^^^^^^^^^^^^^ error: casting `u64` to `isize` may truncate the value on targets with 32-bit wide pointers - --> tests/ui/cast.rs:111:5 + --> tests/ui/cast.rs:112:5 | LL | 1u64 as isize; | ^^^^^^^^^^^^^ @@ -304,55 +304,55 @@ LL | isize::try_from(1u64); | ~~~~~~~~~~~~~~~~~~~~~ error: casting `u64` to `isize` may wrap around the value on targets with 64-bit wide pointers - --> tests/ui/cast.rs:111:5 + --> tests/ui/cast.rs:112:5 | LL | 1u64 as isize; | ^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:116:5 + --> tests/ui/cast.rs:117:5 | LL | -1i32 as u32; | ^^^^^^^^^^^^ error: casting `isize` to `usize` may lose the sign of the value - --> tests/ui/cast.rs:119:5 + --> tests/ui/cast.rs:120:5 | LL | -1isize as usize; | ^^^^^^^^^^^^^^^^ error: casting `i8` to `u8` may lose the sign of the value - --> tests/ui/cast.rs:130:5 + --> tests/ui/cast.rs:131:5 | LL | (i8::MIN).abs() as u8; | ^^^^^^^^^^^^^^^^^^^^^ error: casting `i64` to `u64` may lose the sign of the value - --> tests/ui/cast.rs:134:5 + --> tests/ui/cast.rs:135:5 | LL | (-1i64).abs() as u64; | ^^^^^^^^^^^^^^^^^^^^ error: casting `isize` to `usize` may lose the sign of the value - --> tests/ui/cast.rs:135:5 + --> tests/ui/cast.rs:136:5 | LL | (-1isize).abs() as usize; | ^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `i64` to `u64` may lose the sign of the value - --> tests/ui/cast.rs:142:5 + --> tests/ui/cast.rs:143:5 | LL | (unsafe { (-1i64).checked_abs().unwrap_unchecked() }) as u64; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `i64` to `u64` may lose the sign of the value - --> tests/ui/cast.rs:157:5 + --> tests/ui/cast.rs:158:5 | LL | (unsafe { (-1i64).checked_isqrt().unwrap_unchecked() }) as u64; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `i64` to `i8` may truncate the value - --> tests/ui/cast.rs:208:5 + --> tests/ui/cast.rs:209:5 | LL | (-99999999999i64).min(1) as i8; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -364,7 +364,7 @@ LL | i8::try_from((-99999999999i64).min(1)); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: casting `u64` to `u8` may truncate the value - --> tests/ui/cast.rs:222:5 + --> tests/ui/cast.rs:223:5 | LL | 999999u64.clamp(0, 256) as u8; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -376,7 +376,7 @@ LL | u8::try_from(999999u64.clamp(0, 256)); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: casting `main::E2` to `u8` may truncate the value - --> tests/ui/cast.rs:245:21 + --> tests/ui/cast.rs:246:21 | LL | let _ = self as u8; | ^^^^^^^^^^ @@ -388,7 +388,7 @@ LL | let _ = u8::try_from(self); | ~~~~~~~~~~~~~~~~~~ error: casting `main::E2::B` to `u8` will truncate the value - --> tests/ui/cast.rs:247:21 + --> tests/ui/cast.rs:248:21 | LL | let _ = Self::B as u8; | ^^^^^^^^^^^^^ @@ -397,7 +397,7 @@ LL | let _ = Self::B as u8; = help: to override `-D warnings` add `#[allow(clippy::cast_enum_truncation)]` error: casting `main::E5` to `i8` may truncate the value - --> tests/ui/cast.rs:289:21 + --> tests/ui/cast.rs:290:21 | LL | let _ = self as i8; | ^^^^^^^^^^ @@ -409,13 +409,13 @@ LL | let _ = i8::try_from(self); | ~~~~~~~~~~~~~~~~~~ error: casting `main::E5::A` to `i8` will truncate the value - --> tests/ui/cast.rs:291:21 + --> tests/ui/cast.rs:292:21 | LL | let _ = Self::A as i8; | ^^^^^^^^^^^^^ error: casting `main::E6` to `i16` may truncate the value - --> tests/ui/cast.rs:308:21 + --> tests/ui/cast.rs:309:21 | LL | let _ = self as i16; | ^^^^^^^^^^^ @@ -427,7 +427,7 @@ LL | let _ = i16::try_from(self); | ~~~~~~~~~~~~~~~~~~~ error: casting `main::E7` to `usize` may truncate the value on targets with 32-bit wide pointers - --> tests/ui/cast.rs:327:21 + --> tests/ui/cast.rs:328:21 | LL | let _ = self as usize; | ^^^^^^^^^^^^^ @@ -439,7 +439,7 @@ LL | let _ = usize::try_from(self); | ~~~~~~~~~~~~~~~~~~~~~ error: casting `main::E10` to `u16` may truncate the value - --> tests/ui/cast.rs:374:21 + --> tests/ui/cast.rs:375:21 | LL | let _ = self as u16; | ^^^^^^^^^^^ @@ -451,7 +451,7 @@ LL | let _ = u16::try_from(self); | ~~~~~~~~~~~~~~~~~~~ error: casting `u32` to `u8` may truncate the value - --> tests/ui/cast.rs:385:13 + --> tests/ui/cast.rs:386:13 | LL | let c = (q >> 16) as u8; | ^^^^^^^^^^^^^^^ @@ -463,7 +463,7 @@ LL | let c = u8::try_from(q >> 16); | ~~~~~~~~~~~~~~~~~~~~~ error: casting `u32` to `u8` may truncate the value - --> tests/ui/cast.rs:389:13 + --> tests/ui/cast.rs:390:13 | LL | let c = (q / 1000) as u8; | ^^^^^^^^^^^^^^^^ @@ -475,85 +475,85 @@ LL | let c = u8::try_from(q / 1000); | ~~~~~~~~~~~~~~~~~~~~~~ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:401:9 + --> tests/ui/cast.rs:402:9 | LL | (x * x) as u32; | ^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:406:32 + --> tests/ui/cast.rs:407:32 | LL | let _a = |x: i32| -> u32 { (x * x * x * x) as u32 }; | ^^^^^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:408:5 + --> tests/ui/cast.rs:409:5 | LL | (2_i32).checked_pow(3).unwrap() as u32; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:409:5 + --> tests/ui/cast.rs:410:5 | LL | (-2_i32).pow(3) as u32; | ^^^^^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:414:5 + --> tests/ui/cast.rs:415:5 | LL | (-5_i32 % 2) as u32; | ^^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:416:5 + --> tests/ui/cast.rs:417:5 | LL | (-5_i32 % -2) as u32; | ^^^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:419:5 + --> tests/ui/cast.rs:420:5 | LL | (-2_i32 >> 1) as u32; | ^^^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:423:5 + --> tests/ui/cast.rs:424:5 | LL | (x * x) as u32; | ^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:424:5 + --> tests/ui/cast.rs:425:5 | LL | (x * x * x) as u32; | ^^^^^^^^^^^^^^^^^^ error: casting `i16` to `u16` may lose the sign of the value - --> tests/ui/cast.rs:428:5 + --> tests/ui/cast.rs:429:5 | LL | (y * y * y * y * -2) as u16; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `i16` to `u16` may lose the sign of the value - --> tests/ui/cast.rs:430:5 + --> tests/ui/cast.rs:431:5 | LL | (y * y * y / y * 2) as u16; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `i16` to `u16` may lose the sign of the value - --> tests/ui/cast.rs:431:5 + --> tests/ui/cast.rs:432:5 | LL | (y * y / y * 2) as u16; | ^^^^^^^^^^^^^^^^^^^^^^ error: casting `i16` to `u16` may lose the sign of the value - --> tests/ui/cast.rs:433:5 + --> tests/ui/cast.rs:434:5 | LL | (y / y * y * -2) as u16; | ^^^^^^^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `/` - --> tests/ui/cast.rs:433:6 + --> tests/ui/cast.rs:434:6 | LL | (y / y * y * -2) as u16; | ^^^^^ @@ -561,97 +561,97 @@ LL | (y / y * y * -2) as u16; = note: `#[deny(clippy::eq_op)]` on by default error: casting `i16` to `u16` may lose the sign of the value - --> tests/ui/cast.rs:436:5 + --> tests/ui/cast.rs:437:5 | LL | (y + y + y + -2) as u16; | ^^^^^^^^^^^^^^^^^^^^^^^ error: casting `i16` to `u16` may lose the sign of the value - --> tests/ui/cast.rs:438:5 + --> tests/ui/cast.rs:439:5 | LL | (y + y + y + 2) as u16; | ^^^^^^^^^^^^^^^^^^^^^^ error: casting `i16` to `u16` may lose the sign of the value - --> tests/ui/cast.rs:442:5 + --> tests/ui/cast.rs:443:5 | LL | (z + -2) as u16; | ^^^^^^^^^^^^^^^ error: casting `i16` to `u16` may lose the sign of the value - --> tests/ui/cast.rs:444:5 + --> tests/ui/cast.rs:445:5 | LL | (z + z + 2) as u16; | ^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:447:9 + --> tests/ui/cast.rs:448:9 | LL | (a * a * b * b * c * c) as u32; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:448:9 + --> tests/ui/cast.rs:449:9 | LL | (a * b * c) as u32; | ^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:450:9 + --> tests/ui/cast.rs:451:9 | LL | (a * -b * c) as u32; | ^^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:452:9 + --> tests/ui/cast.rs:453:9 | LL | (a * b * c * c) as u32; | ^^^^^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:453:9 + --> tests/ui/cast.rs:454:9 | LL | (a * -2) as u32; | ^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:455:9 + --> tests/ui/cast.rs:456:9 | LL | (a * b * c * -2) as u32; | ^^^^^^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:457:9 + --> tests/ui/cast.rs:458:9 | LL | (a / b) as u32; | ^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:458:9 + --> tests/ui/cast.rs:459:9 | LL | (a / b * c) as u32; | ^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:460:9 + --> tests/ui/cast.rs:461:9 | LL | (a / b + b * c) as u32; | ^^^^^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:462:9 + --> tests/ui/cast.rs:463:9 | LL | a.saturating_pow(3) as u32; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:464:9 + --> tests/ui/cast.rs:465:9 | LL | (a.abs() * b.pow(2) / c.abs()) as u32 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> tests/ui/cast.rs:472:21 + --> tests/ui/cast.rs:473:21 | LL | let _ = i32::MIN as u32; // cast_sign_loss | ^^^^^^^^^^^^^^^ @@ -662,7 +662,7 @@ LL | m!(); = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info) error: casting `u32` to `u8` may truncate the value - --> tests/ui/cast.rs:473:21 + --> tests/ui/cast.rs:474:21 | LL | let _ = u32::MAX as u8; // cast_possible_truncation | ^^^^^^^^^^^^^^ @@ -678,7 +678,7 @@ LL | let _ = u8::try_from(u32::MAX); // cast_possible_truncation | ~~~~~~~~~~~~~~~~~~~~~~ error: casting `f64` to `f32` may truncate the value - --> tests/ui/cast.rs:474:21 + --> tests/ui/cast.rs:475:21 | LL | let _ = std::f64::consts::PI as f32; // cast_possible_truncation | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -690,7 +690,7 @@ LL | m!(); = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info) error: casting `i64` to `usize` may truncate the value on targets with 32-bit wide pointers - --> tests/ui/cast.rs:483:5 + --> tests/ui/cast.rs:484:5 | LL | bar.unwrap().unwrap() as usize | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -702,13 +702,13 @@ LL | usize::try_from(bar.unwrap().unwrap()) | error: casting `i64` to `usize` may lose the sign of the value - --> tests/ui/cast.rs:483:5 + --> tests/ui/cast.rs:484:5 | LL | bar.unwrap().unwrap() as usize | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `u64` to `u8` may truncate the value - --> tests/ui/cast.rs:498:5 + --> tests/ui/cast.rs:499:5 | LL | (256 & 999999u64) as u8; | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -720,7 +720,7 @@ LL | u8::try_from(256 & 999999u64); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: casting `u64` to `u8` may truncate the value - --> tests/ui/cast.rs:500:5 + --> tests/ui/cast.rs:501:5 | LL | (255 % 999999u64) as u8; | ^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/cast_size.32bit.stderr b/tests/ui/cast_size.32bit.stderr index eb6c59deabb..8ff3464676c 100644 --- a/tests/ui/cast_size.32bit.stderr +++ b/tests/ui/cast_size.32bit.stderr @@ -12,35 +12,35 @@ help: ... or use `try_from` and handle the error accordingly LL | i8::try_from(1isize); | ~~~~~~~~~~~~~~~~~~~~ -error: casting `isize` to `f64` causes a loss of precision on targets with 64-bit wide pointers (`isize` is 64 bits wide, but `f64`'s mantissa is only 52 bits wide) - --> tests/ui/cast_size.rs:18:5 +error: casting `isize` to `f32` causes a loss of precision (`isize` is 32 or 64 bits wide, but `f32`'s mantissa is only 23 bits wide) + --> tests/ui/cast_size.rs:21:5 | -LL | x0 as f64; +LL | x0 as f32; | ^^^^^^^^^ | = note: `-D clippy::cast-precision-loss` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::cast_precision_loss)]` -error: casting `usize` to `f64` causes a loss of precision on targets with 64-bit wide pointers (`usize` is 64 bits wide, but `f64`'s mantissa is only 52 bits wide) - --> tests/ui/cast_size.rs:19:5 +error: casting `usize` to `f32` causes a loss of precision (`usize` is 32 or 64 bits wide, but `f32`'s mantissa is only 23 bits wide) + --> tests/ui/cast_size.rs:22:5 | -LL | x1 as f64; +LL | x1 as f32; | ^^^^^^^^^ -error: casting `isize` to `f32` causes a loss of precision (`isize` is 32 or 64 bits wide, but `f32`'s mantissa is only 23 bits wide) - --> tests/ui/cast_size.rs:20:5 +error: casting `isize` to `f64` causes a loss of precision on targets with 64-bit wide pointers (`isize` is 64 bits wide, but `f64`'s mantissa is only 52 bits wide) + --> tests/ui/cast_size.rs:23:5 | -LL | x0 as f32; +LL | x0 as f64; | ^^^^^^^^^ -error: casting `usize` to `f32` causes a loss of precision (`usize` is 32 or 64 bits wide, but `f32`'s mantissa is only 23 bits wide) - --> tests/ui/cast_size.rs:21:5 +error: casting `usize` to `f64` causes a loss of precision on targets with 64-bit wide pointers (`usize` is 64 bits wide, but `f64`'s mantissa is only 52 bits wide) + --> tests/ui/cast_size.rs:24:5 | -LL | x1 as f32; +LL | x1 as f64; | ^^^^^^^^^ error: casting `isize` to `i32` may truncate the value on targets with 64-bit wide pointers - --> tests/ui/cast_size.rs:22:5 + --> tests/ui/cast_size.rs:28:5 | LL | 1isize as i32; | ^^^^^^^^^^^^^ @@ -52,7 +52,7 @@ LL | i32::try_from(1isize); | ~~~~~~~~~~~~~~~~~~~~~ error: casting `isize` to `u32` may truncate the value on targets with 64-bit wide pointers - --> tests/ui/cast_size.rs:23:5 + --> tests/ui/cast_size.rs:29:5 | LL | 1isize as u32; | ^^^^^^^^^^^^^ @@ -64,7 +64,7 @@ LL | u32::try_from(1isize); | ~~~~~~~~~~~~~~~~~~~~~ error: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers - --> tests/ui/cast_size.rs:24:5 + --> tests/ui/cast_size.rs:30:5 | LL | 1usize as u32; | ^^^^^^^^^^^^^ @@ -76,7 +76,7 @@ LL | u32::try_from(1usize); | ~~~~~~~~~~~~~~~~~~~~~ error: casting `usize` to `i32` may truncate the value on targets with 64-bit wide pointers - --> tests/ui/cast_size.rs:25:5 + --> tests/ui/cast_size.rs:31:5 | LL | 1usize as i32; | ^^^^^^^^^^^^^ @@ -88,7 +88,7 @@ LL | i32::try_from(1usize); | ~~~~~~~~~~~~~~~~~~~~~ error: casting `usize` to `i32` may wrap around the value on targets with 32-bit wide pointers - --> tests/ui/cast_size.rs:25:5 + --> tests/ui/cast_size.rs:31:5 | LL | 1usize as i32; | ^^^^^^^^^^^^^ @@ -97,7 +97,7 @@ LL | 1usize as i32; = help: to override `-D warnings` add `#[allow(clippy::cast_possible_wrap)]` error: casting `i64` to `isize` may truncate the value on targets with 32-bit wide pointers - --> tests/ui/cast_size.rs:26:5 + --> tests/ui/cast_size.rs:32:5 | LL | 1i64 as isize; | ^^^^^^^^^^^^^ @@ -109,7 +109,7 @@ LL | isize::try_from(1i64); | ~~~~~~~~~~~~~~~~~~~~~ error: casting `i64` to `usize` may truncate the value on targets with 32-bit wide pointers - --> tests/ui/cast_size.rs:27:5 + --> tests/ui/cast_size.rs:33:5 | LL | 1i64 as usize; | ^^^^^^^^^^^^^ @@ -121,7 +121,7 @@ LL | usize::try_from(1i64); | ~~~~~~~~~~~~~~~~~~~~~ error: casting `u64` to `isize` may truncate the value on targets with 32-bit wide pointers - --> tests/ui/cast_size.rs:28:5 + --> tests/ui/cast_size.rs:34:5 | LL | 1u64 as isize; | ^^^^^^^^^^^^^ @@ -133,13 +133,13 @@ LL | isize::try_from(1u64); | ~~~~~~~~~~~~~~~~~~~~~ error: casting `u64` to `isize` may wrap around the value on targets with 64-bit wide pointers - --> tests/ui/cast_size.rs:28:5 + --> tests/ui/cast_size.rs:34:5 | LL | 1u64 as isize; | ^^^^^^^^^^^^^ error: casting `u64` to `usize` may truncate the value on targets with 32-bit wide pointers - --> tests/ui/cast_size.rs:29:5 + --> tests/ui/cast_size.rs:35:5 | LL | 1u64 as usize; | ^^^^^^^^^^^^^ @@ -151,25 +151,25 @@ LL | usize::try_from(1u64); | ~~~~~~~~~~~~~~~~~~~~~ error: casting `u32` to `isize` may wrap around the value on targets with 32-bit wide pointers - --> tests/ui/cast_size.rs:30:5 + --> tests/ui/cast_size.rs:36:5 | LL | 1u32 as isize; | ^^^^^^^^^^^^^ error: casting `i32` to `f32` causes a loss of precision (`i32` is 32 bits wide, but `f32`'s mantissa is only 23 bits wide) - --> tests/ui/cast_size.rs:35:5 + --> tests/ui/cast_size.rs:43:5 | LL | 999_999_999 as f32; | ^^^^^^^^^^^^^^^^^^ error: casting `usize` to `f64` causes a loss of precision on targets with 64-bit wide pointers (`usize` is 64 bits wide, but `f64`'s mantissa is only 52 bits wide) - --> tests/ui/cast_size.rs:36:5 + --> tests/ui/cast_size.rs:44:5 | LL | 9_999_999_999_999_999usize as f64; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: literal out of range for `usize` - --> tests/ui/cast_size.rs:36:5 + --> tests/ui/cast_size.rs:44:5 | LL | 9_999_999_999_999_999usize as f64; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/copy_iterator.rs b/tests/ui/copy_iterator.rs index c0e5fc3e446..ea3a4face93 100644 --- a/tests/ui/copy_iterator.rs +++ b/tests/ui/copy_iterator.rs @@ -1,4 +1,5 @@ #![warn(clippy::copy_iterator)] +#![allow(clippy::manual_inspect)] #[derive(Copy, Clone)] struct Countdown(u8); diff --git a/tests/ui/copy_iterator.stderr b/tests/ui/copy_iterator.stderr index 533bddaadb5..990b1ce628d 100644 --- a/tests/ui/copy_iterator.stderr +++ b/tests/ui/copy_iterator.stderr @@ -1,5 +1,5 @@ error: you are implementing `Iterator` on a `Copy` type - --> tests/ui/copy_iterator.rs:6:1 + --> tests/ui/copy_iterator.rs:7:1 | LL | / impl Iterator for Countdown { LL | | diff --git a/tests/ui/crashes/ice-12284.rs b/tests/ui/crashes/ice-12284.rs new file mode 100644 index 00000000000..8d1dbface8e --- /dev/null +++ b/tests/ui/crashes/ice-12284.rs @@ -0,0 +1,10 @@ +#![allow(incomplete_features)] +#![feature(unnamed_fields)] + +#[repr(C)] +struct Foo { + _: struct { + }, +} + +fn main() {} diff --git a/tests/ui/derive.rs b/tests/ui/derive.rs index 20ac8a6e6be..4ac21f2cb4b 100644 --- a/tests/ui/derive.rs +++ b/tests/ui/derive.rs @@ -1,7 +1,6 @@ #![allow(clippy::non_canonical_clone_impl, clippy::non_canonical_partial_ord_impl, dead_code)] #![warn(clippy::expl_impl_clone_on_copy)] - #[derive(Copy)] struct Qux; diff --git a/tests/ui/derive.stderr b/tests/ui/derive.stderr index d5f9d7cf2a2..486e6dc1b6b 100644 --- a/tests/ui/derive.stderr +++ b/tests/ui/derive.stderr @@ -1,5 +1,5 @@ error: you are implementing `Clone` explicitly on a `Copy` type - --> tests/ui/derive.rs:8:1 + --> tests/ui/derive.rs:7:1 | LL | / impl Clone for Qux { LL | | @@ -10,7 +10,7 @@ LL | | } | |_^ | note: consider deriving `Clone` or removing `Copy` - --> tests/ui/derive.rs:8:1 + --> tests/ui/derive.rs:7:1 | LL | / impl Clone for Qux { LL | | @@ -23,7 +23,7 @@ LL | | } = help: to override `-D warnings` add `#[allow(clippy::expl_impl_clone_on_copy)]` error: you are implementing `Clone` explicitly on a `Copy` type - --> tests/ui/derive.rs:33:1 + --> tests/ui/derive.rs:32:1 | LL | / impl<'a> Clone for Lt<'a> { LL | | @@ -34,7 +34,7 @@ LL | | } | |_^ | note: consider deriving `Clone` or removing `Copy` - --> tests/ui/derive.rs:33:1 + --> tests/ui/derive.rs:32:1 | LL | / impl<'a> Clone for Lt<'a> { LL | | @@ -45,7 +45,7 @@ LL | | } | |_^ error: you are implementing `Clone` explicitly on a `Copy` type - --> tests/ui/derive.rs:45:1 + --> tests/ui/derive.rs:44:1 | LL | / impl Clone for BigArray { LL | | @@ -56,7 +56,7 @@ LL | | } | |_^ | note: consider deriving `Clone` or removing `Copy` - --> tests/ui/derive.rs:45:1 + --> tests/ui/derive.rs:44:1 | LL | / impl Clone for BigArray { LL | | @@ -67,7 +67,7 @@ LL | | } | |_^ error: you are implementing `Clone` explicitly on a `Copy` type - --> tests/ui/derive.rs:57:1 + --> tests/ui/derive.rs:56:1 | LL | / impl Clone for FnPtr { LL | | @@ -78,7 +78,7 @@ LL | | } | |_^ | note: consider deriving `Clone` or removing `Copy` - --> tests/ui/derive.rs:57:1 + --> tests/ui/derive.rs:56:1 | LL | / impl Clone for FnPtr { LL | | @@ -89,7 +89,7 @@ LL | | } | |_^ error: you are implementing `Clone` explicitly on a `Copy` type - --> tests/ui/derive.rs:78:1 + --> tests/ui/derive.rs:77:1 | LL | / impl<T: Clone> Clone for Generic2<T> { LL | | @@ -100,7 +100,7 @@ LL | | } | |_^ | note: consider deriving `Clone` or removing `Copy` - --> tests/ui/derive.rs:78:1 + --> tests/ui/derive.rs:77:1 | LL | / impl<T: Clone> Clone for Generic2<T> { LL | | diff --git a/tests/ui/field_scoped_visibility_modifiers.rs b/tests/ui/field_scoped_visibility_modifiers.rs new file mode 100644 index 00000000000..5789dbf9b1d --- /dev/null +++ b/tests/ui/field_scoped_visibility_modifiers.rs @@ -0,0 +1,21 @@ +#![warn(clippy::field_scoped_visibility_modifiers)] + +pub mod pub_module { + pub(in crate::pub_module) mod pub_in_path_module {} + pub(super) mod pub_super_module {} + struct MyStruct { + private_field: bool, + pub pub_field: bool, + pub(crate) pub_crate_field: bool, + pub(in crate::pub_module) pub_in_path_field: bool, + pub(super) pub_super_field: bool, + #[allow(clippy::needless_pub_self)] + pub(self) pub_self_field: bool, + } +} +pub(crate) mod pub_crate_module {} + +#[allow(clippy::needless_pub_self)] +pub(self) mod pub_self_module {} + +fn main() {} diff --git a/tests/ui/field_scoped_visibility_modifiers.stderr b/tests/ui/field_scoped_visibility_modifiers.stderr new file mode 100644 index 00000000000..beea6c92107 --- /dev/null +++ b/tests/ui/field_scoped_visibility_modifiers.stderr @@ -0,0 +1,28 @@ +error: scoped visibility modifier on a field + --> tests/ui/field_scoped_visibility_modifiers.rs:9:9 + | +LL | pub(crate) pub_crate_field: bool, + | ^^^^^^^^^^ + | + = help: consider making the field private and adding a scoped visibility method for it + = note: `-D clippy::field-scoped-visibility-modifiers` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::field_scoped_visibility_modifiers)]` + +error: scoped visibility modifier on a field + --> tests/ui/field_scoped_visibility_modifiers.rs:10:9 + | +LL | pub(in crate::pub_module) pub_in_path_field: bool, + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider making the field private and adding a scoped visibility method for it + +error: scoped visibility modifier on a field + --> tests/ui/field_scoped_visibility_modifiers.rs:11:9 + | +LL | pub(super) pub_super_field: bool, + | ^^^^^^^^^^ + | + = help: consider making the field private and adding a scoped visibility method for it + +error: aborting due to 3 previous errors + diff --git a/tests/ui/implicit_return.fixed b/tests/ui/implicit_return.fixed index ba73e64f186..56fe29b4e67 100644 --- a/tests/ui/implicit_return.fixed +++ b/tests/ui/implicit_return.fixed @@ -1,6 +1,11 @@ +//@aux-build: proc_macros.rs + #![warn(clippy::implicit_return)] #![allow(clippy::needless_return, clippy::needless_bool, unused, clippy::never_loop)] +extern crate proc_macros; +use proc_macros::with_span; + fn test_end_of_fn() -> bool { if true { // no error! @@ -136,3 +141,11 @@ fn check_expect() -> bool { #[expect(clippy::implicit_return)] true } + +with_span!( + span + + fn dont_lint_proc_macro(x: usize) -> usize{ + x + } +); diff --git a/tests/ui/implicit_return.rs b/tests/ui/implicit_return.rs index 522fc6a0a44..f066ce20cfd 100644 --- a/tests/ui/implicit_return.rs +++ b/tests/ui/implicit_return.rs @@ -1,6 +1,11 @@ +//@aux-build: proc_macros.rs + #![warn(clippy::implicit_return)] #![allow(clippy::needless_return, clippy::needless_bool, unused, clippy::never_loop)] +extern crate proc_macros; +use proc_macros::with_span; + fn test_end_of_fn() -> bool { if true { // no error! @@ -136,3 +141,11 @@ fn check_expect() -> bool { #[expect(clippy::implicit_return)] true } + +with_span!( + span + + fn dont_lint_proc_macro(x: usize) -> usize{ + x + } +); diff --git a/tests/ui/implicit_return.stderr b/tests/ui/implicit_return.stderr index b2f7bc69439..f06d4e983c5 100644 --- a/tests/ui/implicit_return.stderr +++ b/tests/ui/implicit_return.stderr @@ -1,5 +1,5 @@ error: missing `return` statement - --> tests/ui/implicit_return.rs:10:5 + --> tests/ui/implicit_return.rs:15:5 | LL | true | ^^^^ help: add `return` as shown: `return true` @@ -8,85 +8,85 @@ LL | true = help: to override `-D warnings` add `#[allow(clippy::implicit_return)]` error: missing `return` statement - --> tests/ui/implicit_return.rs:14:15 + --> tests/ui/implicit_return.rs:19:15 | LL | if true { true } else { false } | ^^^^ help: add `return` as shown: `return true` error: missing `return` statement - --> tests/ui/implicit_return.rs:14:29 + --> tests/ui/implicit_return.rs:19:29 | LL | if true { true } else { false } | ^^^^^ help: add `return` as shown: `return false` error: missing `return` statement - --> tests/ui/implicit_return.rs:20:17 + --> tests/ui/implicit_return.rs:25:17 | LL | true => false, | ^^^^^ help: add `return` as shown: `return false` error: missing `return` statement - --> tests/ui/implicit_return.rs:21:20 + --> tests/ui/implicit_return.rs:26:20 | LL | false => { true }, | ^^^^ help: add `return` as shown: `return true` error: missing `return` statement - --> tests/ui/implicit_return.rs:34:9 + --> tests/ui/implicit_return.rs:39:9 | LL | break true; | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing `return` statement - --> tests/ui/implicit_return.rs:41:13 + --> tests/ui/implicit_return.rs:46:13 | LL | break true; | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing `return` statement - --> tests/ui/implicit_return.rs:49:13 + --> tests/ui/implicit_return.rs:54:13 | LL | break true; | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing `return` statement - --> tests/ui/implicit_return.rs:67:18 + --> tests/ui/implicit_return.rs:72:18 | LL | let _ = || { true }; | ^^^^ help: add `return` as shown: `return true` error: missing `return` statement - --> tests/ui/implicit_return.rs:68:16 + --> tests/ui/implicit_return.rs:73:16 | LL | let _ = || true; | ^^^^ help: add `return` as shown: `return true` error: missing `return` statement - --> tests/ui/implicit_return.rs:76:5 + --> tests/ui/implicit_return.rs:81:5 | LL | format!("test {}", "test") | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `return` as shown: `return format!("test {}", "test")` error: missing `return` statement - --> tests/ui/implicit_return.rs:85:5 + --> tests/ui/implicit_return.rs:90:5 | LL | m!(true, false) | ^^^^^^^^^^^^^^^ help: add `return` as shown: `return m!(true, false)` error: missing `return` statement - --> tests/ui/implicit_return.rs:91:13 + --> tests/ui/implicit_return.rs:96:13 | LL | break true; | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing `return` statement - --> tests/ui/implicit_return.rs:96:17 + --> tests/ui/implicit_return.rs:101:17 | LL | break 'outer false; | ^^^^^^^^^^^^^^^^^^ help: change `break` to `return` as shown: `return false` error: missing `return` statement - --> tests/ui/implicit_return.rs:111:5 + --> tests/ui/implicit_return.rs:116:5 | LL | / loop { LL | | m!(true); @@ -101,7 +101,7 @@ LL + } | error: missing `return` statement - --> tests/ui/implicit_return.rs:125:5 + --> tests/ui/implicit_return.rs:130:5 | LL | true | ^^^^ help: add `return` as shown: `return true` diff --git a/tests/ui/manual_inspect.fixed b/tests/ui/manual_inspect.fixed new file mode 100644 index 00000000000..0e1b8fe3edb --- /dev/null +++ b/tests/ui/manual_inspect.fixed @@ -0,0 +1,174 @@ +#![warn(clippy::manual_inspect)] +#![allow(clippy::no_effect, clippy::op_ref)] + +fn main() { + let _ = Some(0).inspect(|&x| { + println!("{}", x); + }); + + let _ = Some(0).inspect(|&x| { + println!("{x}"); + }); + + let _ = Some(0).inspect(|&x| { + println!("{}", x * 5 + 1); + }); + + let _ = Some(0).inspect(|&x| { + if x == 0 { + panic!(); + } + }); + + let _ = Some(0).inspect(|&x| { + if &x == &0 { + let _y = x; + panic!(); + } + }); + + let _ = Some(0).map(|x| { + let y = x + 1; + if y > 5 { + return y; + } + x + }); + + { + #[derive(PartialEq)] + struct Foo(i32); + + let _ = Some(Foo(0)).map(|x| { + if x == Foo(0) { + panic!(); + } + x + }); + + let _ = Some(Foo(0)).map(|x| { + if &x == &Foo(0) { + let _y = x; + panic!(); + } + x + }); + } + + { + macro_rules! maybe_ret { + ($e:expr) => { + if $e == 0 { + return $e; + } + }; + } + + let _ = Some(0).map(|x| { + maybe_ret!(x); + x + }); + } + + let _ = Some((String::new(), 0u32)).inspect(|x| { + if x.1 == 0 { + let _x = x.1; + panic!(); + } + }); + + let _ = Some((String::new(), 0u32)).map(|x| { + if x.1 == 0 { + let _x = x.0; + panic!(); + } + x + }); + + let _ = Some(String::new()).map(|x| { + if x.is_empty() { + let _ = || { + let _x = x; + }; + panic!(); + } + x + }); + + let _ = Some(String::new()).inspect(|x| { + if x.is_empty() { + let _ = || { + let _x = x; + }; + return; + } + println!("test"); + }); + + let _ = Some(0).inspect(|&x| { + if x == 0 { + let _ = || { + let _x = x; + }; + panic!(); + } + }); + + { + use core::cell::Cell; + #[derive(Debug)] + struct Cell2(core::cell::Cell<u32>); + + let _ = Some(Cell2(Cell::new(0u32))).inspect(|x| { + x.0.set(1); + }); + + let _ = Some(Cell2(Cell::new(0u32))).map(|x| { + let y = &x; + if x.0.get() == 0 { + y.0.set(1) + } else { + println!("{x:?}"); + } + x + }); + } + + let _: Result<_, ()> = Ok(0).inspect(|&x| { + println!("{}", x); + }); + + let _: Result<(), _> = Err(0).inspect_err(|&x| { + println!("{}", x); + }); + + let _ = [0] + .into_iter() + .inspect(|&x| { + println!("{}", x); + }) + .count(); + + { + struct S<T>(T); + impl<T> S<T> { + fn map<U>(self, f: impl FnOnce(T) -> U) -> S<U> { + S(f(self.0)) + } + + fn map_err<U>(self, f: impl FnOnce(T) -> U) -> S<U> { + S(f(self.0)) + } + } + + let _ = S(0).map(|x| { + println!("{}", x); + x + }); + + let _ = S(0).map_err(|x| { + println!("{}", x); + x + }); + } +} diff --git a/tests/ui/manual_inspect.rs b/tests/ui/manual_inspect.rs new file mode 100644 index 00000000000..94cdfe39140 --- /dev/null +++ b/tests/ui/manual_inspect.rs @@ -0,0 +1,186 @@ +#![warn(clippy::manual_inspect)] +#![allow(clippy::no_effect, clippy::op_ref)] + +fn main() { + let _ = Some(0).map(|x| { + println!("{}", x); + x + }); + + let _ = Some(0).map(|x| { + println!("{x}"); + x + }); + + let _ = Some(0).map(|x| { + println!("{}", x * 5 + 1); + x + }); + + let _ = Some(0).map(|x| { + if x == 0 { + panic!(); + } + x + }); + + let _ = Some(0).map(|x| { + if &x == &0 { + let _y = x; + panic!(); + } + x + }); + + let _ = Some(0).map(|x| { + let y = x + 1; + if y > 5 { + return y; + } + x + }); + + { + #[derive(PartialEq)] + struct Foo(i32); + + let _ = Some(Foo(0)).map(|x| { + if x == Foo(0) { + panic!(); + } + x + }); + + let _ = Some(Foo(0)).map(|x| { + if &x == &Foo(0) { + let _y = x; + panic!(); + } + x + }); + } + + { + macro_rules! maybe_ret { + ($e:expr) => { + if $e == 0 { + return $e; + } + }; + } + + let _ = Some(0).map(|x| { + maybe_ret!(x); + x + }); + } + + let _ = Some((String::new(), 0u32)).map(|x| { + if x.1 == 0 { + let _x = x.1; + panic!(); + } + x + }); + + let _ = Some((String::new(), 0u32)).map(|x| { + if x.1 == 0 { + let _x = x.0; + panic!(); + } + x + }); + + let _ = Some(String::new()).map(|x| { + if x.is_empty() { + let _ = || { + let _x = x; + }; + panic!(); + } + x + }); + + let _ = Some(String::new()).map(|x| { + if x.is_empty() { + let _ = || { + let _x = &x; + }; + return x; + } + println!("test"); + x + }); + + let _ = Some(0).map(|x| { + if x == 0 { + let _ = || { + let _x = x; + }; + panic!(); + } + x + }); + + { + use core::cell::Cell; + #[derive(Debug)] + struct Cell2(core::cell::Cell<u32>); + + let _ = Some(Cell2(Cell::new(0u32))).map(|x| { + x.0.set(1); + x + }); + + let _ = Some(Cell2(Cell::new(0u32))).map(|x| { + let y = &x; + if x.0.get() == 0 { + y.0.set(1) + } else { + println!("{x:?}"); + } + x + }); + } + + let _: Result<_, ()> = Ok(0).map(|x| { + println!("{}", x); + x + }); + + let _: Result<(), _> = Err(0).map_err(|x| { + println!("{}", x); + x + }); + + let _ = [0] + .into_iter() + .map(|x| { + println!("{}", x); + x + }) + .count(); + + { + struct S<T>(T); + impl<T> S<T> { + fn map<U>(self, f: impl FnOnce(T) -> U) -> S<U> { + S(f(self.0)) + } + + fn map_err<U>(self, f: impl FnOnce(T) -> U) -> S<U> { + S(f(self.0)) + } + } + + let _ = S(0).map(|x| { + println!("{}", x); + x + }); + + let _ = S(0).map_err(|x| { + println!("{}", x); + x + }); + } +} diff --git a/tests/ui/manual_inspect.stderr b/tests/ui/manual_inspect.stderr new file mode 100644 index 00000000000..8548c0cd294 --- /dev/null +++ b/tests/ui/manual_inspect.stderr @@ -0,0 +1,182 @@ +error: + --> tests/ui/manual_inspect.rs:5:21 + | +LL | let _ = Some(0).map(|x| { + | ^^^ + | + = note: `-D clippy::manual-inspect` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::manual_inspect)]` +help: try + | +LL ~ let _ = Some(0).inspect(|&x| { +LL ~ println!("{}", x); + | + +error: + --> tests/ui/manual_inspect.rs:10:21 + | +LL | let _ = Some(0).map(|x| { + | ^^^ + | +help: try + | +LL ~ let _ = Some(0).inspect(|&x| { +LL ~ println!("{x}"); + | + +error: + --> tests/ui/manual_inspect.rs:15:21 + | +LL | let _ = Some(0).map(|x| { + | ^^^ + | +help: try + | +LL ~ let _ = Some(0).inspect(|&x| { +LL ~ println!("{}", x * 5 + 1); + | + +error: + --> tests/ui/manual_inspect.rs:20:21 + | +LL | let _ = Some(0).map(|x| { + | ^^^ + | +help: try + | +LL ~ let _ = Some(0).inspect(|&x| { +LL | if x == 0 { +LL | panic!(); +LL ~ } + | + +error: + --> tests/ui/manual_inspect.rs:27:21 + | +LL | let _ = Some(0).map(|x| { + | ^^^ + | +help: try + | +LL ~ let _ = Some(0).inspect(|&x| { +LL | if &x == &0 { +LL | let _y = x; +LL | panic!(); +LL ~ } + | + +error: + --> tests/ui/manual_inspect.rs:78:41 + | +LL | let _ = Some((String::new(), 0u32)).map(|x| { + | ^^^ + | +help: try + | +LL ~ let _ = Some((String::new(), 0u32)).inspect(|x| { +LL | if x.1 == 0 { +LL | let _x = x.1; +LL | panic!(); +LL ~ } + | + +error: + --> tests/ui/manual_inspect.rs:104:33 + | +LL | let _ = Some(String::new()).map(|x| { + | ^^^ + | +help: try + | +LL ~ let _ = Some(String::new()).inspect(|x| { +LL | if x.is_empty() { +LL | let _ = || { +LL ~ let _x = x; +LL | }; +LL ~ return; +LL | } +LL ~ println!("test"); + | + +error: + --> tests/ui/manual_inspect.rs:115:21 + | +LL | let _ = Some(0).map(|x| { + | ^^^ + | +help: try + | +LL ~ let _ = Some(0).inspect(|&x| { +LL | if x == 0 { +... +LL | panic!(); +LL ~ } + | + +error: + --> tests/ui/manual_inspect.rs:130:46 + | +LL | let _ = Some(Cell2(Cell::new(0u32))).map(|x| { + | ^^^ + | +help: try + | +LL ~ let _ = Some(Cell2(Cell::new(0u32))).inspect(|x| { +LL ~ x.0.set(1); + | + +error: + --> tests/ui/manual_inspect.rs:146:34 + | +LL | let _: Result<_, ()> = Ok(0).map(|x| { + | ^^^ + | +help: try + | +LL ~ let _: Result<_, ()> = Ok(0).inspect(|&x| { +LL ~ println!("{}", x); + | + +error: + --> tests/ui/manual_inspect.rs:151:35 + | +LL | let _: Result<(), _> = Err(0).map_err(|x| { + | ^^^^^^^ + | +help: try + | +LL ~ let _: Result<(), _> = Err(0).inspect_err(|&x| { +LL ~ println!("{}", x); + | + +error: this call to `map()` won't have an effect on the call to `count()` + --> tests/ui/manual_inspect.rs:156:13 + | +LL | let _ = [0] + | _____________^ +LL | | .into_iter() +LL | | .map(|x| { +LL | | println!("{}", x); +LL | | x +LL | | }) +LL | | .count(); + | |________________^ + | + = help: make sure you did not confuse `map` with `filter`, `for_each` or `inspect` + = note: `-D clippy::suspicious-map` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::suspicious_map)]` + +error: + --> tests/ui/manual_inspect.rs:158:10 + | +LL | .map(|x| { + | ^^^ + | +help: try + | +LL ~ .inspect(|&x| { +LL ~ println!("{}", x); + | + +error: aborting due to 13 previous errors + diff --git a/tests/ui/manual_pattern_char_comparison.fixed b/tests/ui/manual_pattern_char_comparison.fixed index 588226b87e8..03e621d95ba 100644 --- a/tests/ui/manual_pattern_char_comparison.fixed +++ b/tests/ui/manual_pattern_char_comparison.fixed @@ -47,3 +47,15 @@ fn main() { } "".find(|c| m!(c)); } + +#[clippy::msrv = "1.57"] +fn msrv_1_57() { + let sentence = "Hello, world!"; + sentence.trim_end_matches(|c: char| c == '.' || c == ',' || c == '!' || c == '?'); +} + +#[clippy::msrv = "1.58"] +fn msrv_1_58() { + let sentence = "Hello, world!"; + sentence.trim_end_matches(['.', ',', '!', '?']); +} diff --git a/tests/ui/manual_pattern_char_comparison.rs b/tests/ui/manual_pattern_char_comparison.rs index 5078f3ee27f..43e883cd325 100644 --- a/tests/ui/manual_pattern_char_comparison.rs +++ b/tests/ui/manual_pattern_char_comparison.rs @@ -47,3 +47,15 @@ fn main() { } "".find(|c| m!(c)); } + +#[clippy::msrv = "1.57"] +fn msrv_1_57() { + let sentence = "Hello, world!"; + sentence.trim_end_matches(|c: char| c == '.' || c == ',' || c == '!' || c == '?'); +} + +#[clippy::msrv = "1.58"] +fn msrv_1_58() { + let sentence = "Hello, world!"; + sentence.trim_end_matches(|c: char| c == '.' || c == ',' || c == '!' || c == '?'); +} diff --git a/tests/ui/manual_pattern_char_comparison.stderr b/tests/ui/manual_pattern_char_comparison.stderr index b6b51794a11..f185d7c8f67 100644 --- a/tests/ui/manual_pattern_char_comparison.stderr +++ b/tests/ui/manual_pattern_char_comparison.stderr @@ -55,5 +55,11 @@ error: this manual char comparison can be written more succinctly LL | sentence.find(|c| c == '🎈'); | ^^^^^^^^^^^^^ help: consider using a `char`: `'🎈'` -error: aborting due to 9 previous errors +error: this manual char comparison can be written more succinctly + --> tests/ui/manual_pattern_char_comparison.rs:60:31 + | +LL | sentence.trim_end_matches(|c: char| c == '.' || c == ',' || c == '!' || c == '?'); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using an array of `char`: `['.', ',', '!', '?']` + +error: aborting due to 10 previous errors diff --git a/tests/ui/manual_unwrap_or.fixed b/tests/ui/manual_unwrap_or.fixed index dffd44b6a7c..74afa00e12f 100644 --- a/tests/ui/manual_unwrap_or.fixed +++ b/tests/ui/manual_unwrap_or.fixed @@ -68,6 +68,32 @@ fn option_unwrap_or() { Some(s) => s, None => &format!("{} {}!", "hello", "world"), }; + + Some(1).unwrap_or(42); + + //don't lint + if let Some(x) = Some(1) { + x + 1 + } else { + 42 + }; + if let Some(x) = Some(1) { + x + } else { + return; + }; + for j in 0..4 { + if let Some(x) = Some(j) { + x + } else { + continue; + }; + if let Some(x) = Some(j) { + x + } else { + break; + }; + } } fn result_unwrap_or() { @@ -138,6 +164,32 @@ fn result_unwrap_or() { Ok(s) => s, Err(s) => "Bob", }; + + Ok::<i32, i32>(1).unwrap_or(42); + + //don't lint + if let Ok(x) = Ok::<i32, i32>(1) { + x + 1 + } else { + 42 + }; + if let Ok(x) = Ok::<i32, i32>(1) { + x + } else { + return; + }; + for j in 0..4 { + if let Ok(x) = Ok::<i32, i32>(j) { + x + } else { + continue; + }; + if let Ok(x) = Ok::<i32, i32>(j) { + x + } else { + break; + }; + } } // don't lint in const fn diff --git a/tests/ui/manual_unwrap_or.rs b/tests/ui/manual_unwrap_or.rs index 67427132c1a..2d01b8ceaaa 100644 --- a/tests/ui/manual_unwrap_or.rs +++ b/tests/ui/manual_unwrap_or.rs @@ -83,6 +83,36 @@ fn option_unwrap_or() { Some(s) => s, None => &format!("{} {}!", "hello", "world"), }; + + if let Some(x) = Some(1) { + x + } else { + 42 + }; + + //don't lint + if let Some(x) = Some(1) { + x + 1 + } else { + 42 + }; + if let Some(x) = Some(1) { + x + } else { + return; + }; + for j in 0..4 { + if let Some(x) = Some(j) { + x + } else { + continue; + }; + if let Some(x) = Some(j) { + x + } else { + break; + }; + } } fn result_unwrap_or() { @@ -177,6 +207,36 @@ fn result_unwrap_or() { Ok(s) => s, Err(s) => "Bob", }; + + if let Ok(x) = Ok::<i32, i32>(1) { + x + } else { + 42 + }; + + //don't lint + if let Ok(x) = Ok::<i32, i32>(1) { + x + 1 + } else { + 42 + }; + if let Ok(x) = Ok::<i32, i32>(1) { + x + } else { + return; + }; + for j in 0..4 { + if let Ok(x) = Ok::<i32, i32>(j) { + x + } else { + continue; + }; + if let Ok(x) = Ok::<i32, i32>(j) { + x + } else { + break; + }; + } } // don't lint in const fn diff --git a/tests/ui/manual_unwrap_or.stderr b/tests/ui/manual_unwrap_or.stderr index 33a099680ce..c93a8952a08 100644 --- a/tests/ui/manual_unwrap_or.stderr +++ b/tests/ui/manual_unwrap_or.stderr @@ -58,8 +58,18 @@ LL | | None => "Alice", LL | | }; | |_____^ help: replace with: `Some("Bob").unwrap_or("Alice")` +error: this pattern reimplements `Option::unwrap_or` + --> tests/ui/manual_unwrap_or.rs:87:5 + | +LL | / if let Some(x) = Some(1) { +LL | | x +LL | | } else { +LL | | 42 +LL | | }; + | |_____^ help: replace with: `Some(1).unwrap_or(42)` + error: this pattern reimplements `Result::unwrap_or` - --> tests/ui/manual_unwrap_or.rs:90:5 + --> tests/ui/manual_unwrap_or.rs:120:5 | LL | / match Ok::<i32, &str>(1) { LL | | Ok(i) => i, @@ -68,7 +78,7 @@ LL | | }; | |_____^ help: replace with: `Ok::<i32, &str>(1).unwrap_or(42)` error: this pattern reimplements `Result::unwrap_or` - --> tests/ui/manual_unwrap_or.rs:97:5 + --> tests/ui/manual_unwrap_or.rs:127:5 | LL | / match a { LL | | Ok(i) => i, @@ -77,7 +87,7 @@ LL | | }; | |_____^ help: replace with: `a.unwrap_or(42)` error: this pattern reimplements `Result::unwrap_or` - --> tests/ui/manual_unwrap_or.rs:103:5 + --> tests/ui/manual_unwrap_or.rs:133:5 | LL | / match Ok(1) as Result<i32, &str> { LL | | Ok(i) => i, @@ -86,7 +96,7 @@ LL | | }; | |_____^ help: replace with: `(Ok(1) as Result<i32, &str>).unwrap_or(42)` error: this pattern reimplements `Option::unwrap_or` - --> tests/ui/manual_unwrap_or.rs:116:5 + --> tests/ui/manual_unwrap_or.rs:146:5 | LL | / match s.method() { LL | | Some(i) => i, @@ -95,7 +105,7 @@ LL | | }; | |_____^ help: replace with: `s.method().unwrap_or(42)` error: this pattern reimplements `Result::unwrap_or` - --> tests/ui/manual_unwrap_or.rs:122:5 + --> tests/ui/manual_unwrap_or.rs:152:5 | LL | / match Ok::<i32, &str>(1) { LL | | Err(_) => 42, @@ -104,7 +114,7 @@ LL | | }; | |_____^ help: replace with: `Ok::<i32, &str>(1).unwrap_or(42)` error: this pattern reimplements `Result::unwrap_or` - --> tests/ui/manual_unwrap_or.rs:128:5 + --> tests/ui/manual_unwrap_or.rs:158:5 | LL | / match Ok::<i32, &str>(1) { LL | | Ok(i) => i, @@ -113,7 +123,7 @@ LL | | }; | |_____^ help: replace with: `Ok::<i32, &str>(1).unwrap_or(1 + 42)` error: this pattern reimplements `Result::unwrap_or` - --> tests/ui/manual_unwrap_or.rs:135:5 + --> tests/ui/manual_unwrap_or.rs:165:5 | LL | / match Ok::<i32, &str>(1) { LL | | Ok(i) => i, @@ -134,7 +144,7 @@ LL ~ }); | error: this pattern reimplements `Result::unwrap_or` - --> tests/ui/manual_unwrap_or.rs:145:5 + --> tests/ui/manual_unwrap_or.rs:175:5 | LL | / match Ok::<&str, &str>("Bob") { LL | | Ok(i) => i, @@ -142,8 +152,18 @@ LL | | Err(_) => "Alice", LL | | }; | |_____^ help: replace with: `Ok::<&str, &str>("Bob").unwrap_or("Alice")` +error: this pattern reimplements `Result::unwrap_or` + --> tests/ui/manual_unwrap_or.rs:211:5 + | +LL | / if let Ok(x) = Ok::<i32, i32>(1) { +LL | | x +LL | | } else { +LL | | 42 +LL | | }; + | |_____^ help: replace with: `Ok::<i32, i32>(1).unwrap_or(42)` + error: this pattern reimplements `Option::unwrap_or` - --> tests/ui/manual_unwrap_or.rs:205:17 + --> tests/ui/manual_unwrap_or.rs:265:17 | LL | let _ = match some_macro!() { | _________________^ @@ -152,5 +172,5 @@ LL | | None => 0, LL | | }; | |_________^ help: replace with: `some_macro!().unwrap_or(0)` -error: aborting due to 14 previous errors +error: aborting due to 16 previous errors diff --git a/tests/ui/manual_unwrap_or_default.fixed b/tests/ui/manual_unwrap_or_default.fixed index 663de1a5f06..832376fa5af 100644 --- a/tests/ui/manual_unwrap_or_default.fixed +++ b/tests/ui/manual_unwrap_or_default.fixed @@ -1,5 +1,5 @@ #![warn(clippy::manual_unwrap_or_default)] -#![allow(clippy::unnecessary_literal_unwrap)] +#![allow(clippy::unnecessary_literal_unwrap, clippy::manual_unwrap_or)] fn main() { let x: Option<Vec<String>> = None; @@ -78,3 +78,24 @@ fn issue_12569() { 0 }; } + +// Should not warn! +fn issue_12928() { + let x = Some((1, 2)); + let y = if let Some((a, _)) = x { a } else { 0 }; + let y = if let Some((a, ..)) = x { a } else { 0 }; + let x = Some([1, 2]); + let y = if let Some([a, _]) = x { a } else { 0 }; + let y = if let Some([a, ..]) = x { a } else { 0 }; + + struct X { + a: u8, + b: u8, + } + let x = Some(X { a: 0, b: 0 }); + let y = if let Some(X { a, .. }) = x { a } else { 0 }; + struct Y(u8, u8); + let x = Some(Y(0, 0)); + let y = if let Some(Y(a, _)) = x { a } else { 0 }; + let y = if let Some(Y(a, ..)) = x { a } else { 0 }; +} diff --git a/tests/ui/manual_unwrap_or_default.rs b/tests/ui/manual_unwrap_or_default.rs index 75ffe09be9d..649f65c89fb 100644 --- a/tests/ui/manual_unwrap_or_default.rs +++ b/tests/ui/manual_unwrap_or_default.rs @@ -1,5 +1,5 @@ #![warn(clippy::manual_unwrap_or_default)] -#![allow(clippy::unnecessary_literal_unwrap)] +#![allow(clippy::unnecessary_literal_unwrap, clippy::manual_unwrap_or)] fn main() { let x: Option<Vec<String>> = None; @@ -111,3 +111,24 @@ fn issue_12569() { 0 }; } + +// Should not warn! +fn issue_12928() { + let x = Some((1, 2)); + let y = if let Some((a, _)) = x { a } else { 0 }; + let y = if let Some((a, ..)) = x { a } else { 0 }; + let x = Some([1, 2]); + let y = if let Some([a, _]) = x { a } else { 0 }; + let y = if let Some([a, ..]) = x { a } else { 0 }; + + struct X { + a: u8, + b: u8, + } + let x = Some(X { a: 0, b: 0 }); + let y = if let Some(X { a, .. }) = x { a } else { 0 }; + struct Y(u8, u8); + let x = Some(Y(0, 0)); + let y = if let Some(Y(a, _)) = x { a } else { 0 }; + let y = if let Some(Y(a, ..)) = x { a } else { 0 }; +} diff --git a/tests/ui/match_result_ok.fixed b/tests/ui/match_result_ok.fixed index 76b26f5e438..117e0bc68cc 100644 --- a/tests/ui/match_result_ok.fixed +++ b/tests/ui/match_result_ok.fixed @@ -1,6 +1,11 @@ #![warn(clippy::match_result_ok)] #![allow(dead_code)] -#![allow(clippy::boxed_local, clippy::uninlined_format_args, clippy::manual_unwrap_or_default)] +#![allow( + clippy::boxed_local, + clippy::uninlined_format_args, + clippy::manual_unwrap_or_default, + clippy::manual_unwrap_or +)] // Checking `if` cases diff --git a/tests/ui/match_result_ok.rs b/tests/ui/match_result_ok.rs index d6f2475ba79..f8a5269024d 100644 --- a/tests/ui/match_result_ok.rs +++ b/tests/ui/match_result_ok.rs @@ -1,6 +1,11 @@ #![warn(clippy::match_result_ok)] #![allow(dead_code)] -#![allow(clippy::boxed_local, clippy::uninlined_format_args, clippy::manual_unwrap_or_default)] +#![allow( + clippy::boxed_local, + clippy::uninlined_format_args, + clippy::manual_unwrap_or_default, + clippy::manual_unwrap_or +)] // Checking `if` cases diff --git a/tests/ui/match_result_ok.stderr b/tests/ui/match_result_ok.stderr index 0d42d59dc01..b5b91cbe553 100644 --- a/tests/ui/match_result_ok.stderr +++ b/tests/ui/match_result_ok.stderr @@ -1,5 +1,5 @@ error: matching on `Some` with `ok()` is redundant - --> tests/ui/match_result_ok.rs:8:5 + --> tests/ui/match_result_ok.rs:13:5 | LL | if let Some(y) = x.parse().ok() { y } else { 0 } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -12,7 +12,7 @@ LL | if let Ok(y) = x.parse() { y } else { 0 } | ~~~~~~~~~~~~~~~~~~~~~~~~ error: matching on `Some` with `ok()` is redundant - --> tests/ui/match_result_ok.rs:18:9 + --> tests/ui/match_result_ok.rs:23:9 | LL | if let Some(y) = x . parse() . ok () { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -23,7 +23,7 @@ LL | if let Ok(y) = x . parse() { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: matching on `Some` with `ok()` is redundant - --> tests/ui/match_result_ok.rs:44:5 + --> tests/ui/match_result_ok.rs:49:5 | LL | while let Some(a) = wat.next().ok() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/missing_const_for_fn/could_be_const.fixed b/tests/ui/missing_const_for_fn/could_be_const.fixed new file mode 100644 index 00000000000..921dcf0b162 --- /dev/null +++ b/tests/ui/missing_const_for_fn/could_be_const.fixed @@ -0,0 +1,173 @@ +#![warn(clippy::missing_const_for_fn)] +#![allow(incomplete_features, clippy::let_and_return, clippy::missing_transmute_annotations)] +#![feature(const_mut_refs)] +#![feature(const_trait_impl)] + +use std::mem::transmute; + +struct Game { + guess: i32, +} + +impl Game { + // Could be const + pub const fn new() -> Self { + //~^ ERROR: this could be a `const fn` + //~| NOTE: `-D clippy::missing-const-for-fn` implied by `-D warnings` + Self { guess: 42 } + } + + const fn const_generic_params<'a, T, const N: usize>(&self, b: &'a [T; N]) -> &'a [T; N] { + //~^ ERROR: this could be a `const fn` + b + } +} + +// Could be const +const fn one() -> i32 { + //~^ ERROR: this could be a `const fn` + 1 +} + +// Could also be const +const fn two() -> i32 { + //~^ ERROR: this could be a `const fn` + let abc = 2; + abc +} + +// Could be const (since Rust 1.39) +const fn string() -> String { + //~^ ERROR: this could be a `const fn` + String::new() +} + +// Could be const +const unsafe fn four() -> i32 { + //~^ ERROR: this could be a `const fn` + 4 +} + +// Could also be const +const fn generic<T>(t: T) -> T { + //~^ ERROR: this could be a `const fn` + t +} + +fn sub(x: u32) -> usize { + unsafe { transmute(&x) } +} + +const fn generic_arr<T: Copy>(t: [T; 1]) -> T { + //~^ ERROR: this could be a `const fn` + t[0] +} + +mod with_drop { + pub struct A; + pub struct B; + impl Drop for A { + fn drop(&mut self) {} + } + + impl B { + // This can be const, because `a` is passed by reference + pub const fn b(self, a: &A) -> B { + //~^ ERROR: this could be a `const fn` + B + } + } +} + +#[clippy::msrv = "1.47.0"] +mod const_fn_stabilized_before_msrv { + // This could be const because `u8::is_ascii_digit` is a stable const function in 1.47. + const fn const_fn_stabilized_before_msrv(byte: u8) { + //~^ ERROR: this could be a `const fn` + byte.is_ascii_digit(); + } +} + +#[clippy::msrv = "1.45"] +fn msrv_1_45() -> i32 { + 45 +} + +#[clippy::msrv = "1.46"] +const fn msrv_1_46() -> i32 { + //~^ ERROR: this could be a `const fn` + 46 +} + +// Should not be const +fn main() {} + +struct D; + +impl const Drop for D { + fn drop(&mut self) { + todo!(); + } +} + +// Lint this, since it can be dropped in const contexts +// FIXME(effects) +fn d(this: D) {} + +mod msrv { + struct Foo(*const u8, &'static u8); + + impl Foo { + #[clippy::msrv = "1.58"] + const fn deref_ptr_can_be_const(self) -> usize { + //~^ ERROR: this could be a `const fn` + unsafe { *self.0 as usize } + } + + const fn deref_copied_val(self) -> usize { + //~^ ERROR: this could be a `const fn` + *self.1 as usize + } + } + + union Bar { + val: u8, + } + + #[clippy::msrv = "1.56"] + const fn union_access_can_be_const() { + //~^ ERROR: this could be a `const fn` + let bar = Bar { val: 1 }; + let _ = unsafe { bar.val }; + } +} + +mod issue12677 { + pub struct Wrapper { + pub strings: Vec<String>, + } + + impl Wrapper { + #[must_use] + pub const fn new(strings: Vec<String>) -> Self { + Self { strings } + } + + #[must_use] + pub const fn empty() -> Self { + Self { strings: Vec::new() } + } + } + + pub struct Other { + pub text: String, + pub vec: Vec<String>, + } + + impl Other { + pub const fn new(text: String) -> Self { + let vec = Vec::new(); + Self { text, vec } + } + } +} diff --git a/tests/ui/missing_const_for_fn/could_be_const.stderr b/tests/ui/missing_const_for_fn/could_be_const.stderr index 1c61c3e8713..8999af761e3 100644 --- a/tests/ui/missing_const_for_fn/could_be_const.stderr +++ b/tests/ui/missing_const_for_fn/could_be_const.stderr @@ -10,6 +10,10 @@ LL | | } | = note: `-D clippy::missing-const-for-fn` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::missing_const_for_fn)]` +help: make the function `const` + | +LL | pub const fn new() -> Self { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:20:5 @@ -19,6 +23,11 @@ LL | | LL | | b LL | | } | |_____^ + | +help: make the function `const` + | +LL | const fn const_generic_params<'a, T, const N: usize>(&self, b: &'a [T; N]) -> &'a [T; N] { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:27:1 @@ -28,6 +37,11 @@ LL | | LL | | 1 LL | | } | |_^ + | +help: make the function `const` + | +LL | const fn one() -> i32 { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:33:1 @@ -38,6 +52,11 @@ LL | | let abc = 2; LL | | abc LL | | } | |_^ + | +help: make the function `const` + | +LL | const fn two() -> i32 { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:40:1 @@ -47,6 +66,11 @@ LL | | LL | | String::new() LL | | } | |_^ + | +help: make the function `const` + | +LL | const fn string() -> String { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:46:1 @@ -56,6 +80,11 @@ LL | | LL | | 4 LL | | } | |_^ + | +help: make the function `const` + | +LL | const unsafe fn four() -> i32 { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:52:1 @@ -65,6 +94,11 @@ LL | | LL | | t LL | | } | |_^ + | +help: make the function `const` + | +LL | const fn generic<T>(t: T) -> T { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:61:1 @@ -74,6 +108,11 @@ LL | | LL | | t[0] LL | | } | |_^ + | +help: make the function `const` + | +LL | const fn generic_arr<T: Copy>(t: [T; 1]) -> T { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:75:9 @@ -83,6 +122,11 @@ LL | | LL | | B LL | | } | |_________^ + | +help: make the function `const` + | +LL | pub const fn b(self, a: &A) -> B { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:85:5 @@ -92,6 +136,11 @@ LL | | LL | | byte.is_ascii_digit(); LL | | } | |_____^ + | +help: make the function `const` + | +LL | const fn const_fn_stabilized_before_msrv(byte: u8) { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:97:1 @@ -101,6 +150,11 @@ LL | | LL | | 46 LL | | } | |_^ + | +help: make the function `const` + | +LL | const fn msrv_1_46() -> i32 { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:122:9 @@ -110,6 +164,11 @@ LL | | LL | | unsafe { *self.0 as usize } LL | | } | |_________^ + | +help: make the function `const` + | +LL | const fn deref_ptr_can_be_const(self) -> usize { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:127:9 @@ -119,6 +178,11 @@ LL | | LL | | *self.1 as usize LL | | } | |_________^ + | +help: make the function `const` + | +LL | const fn deref_copied_val(self) -> usize { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:138:5 @@ -129,6 +193,11 @@ LL | | let bar = Bar { val: 1 }; LL | | let _ = unsafe { bar.val }; LL | | } | |_____^ + | +help: make the function `const` + | +LL | const fn union_access_can_be_const() { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:152:9 @@ -137,6 +206,11 @@ LL | / pub fn new(strings: Vec<String>) -> Self { LL | | Self { strings } LL | | } | |_________^ + | +help: make the function `const` + | +LL | pub const fn new(strings: Vec<String>) -> Self { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:157:9 @@ -145,6 +219,11 @@ LL | / pub fn empty() -> Self { LL | | Self { strings: Vec::new() } LL | | } | |_________^ + | +help: make the function `const` + | +LL | pub const fn empty() -> Self { + | +++++ error: this could be a `const fn` --> tests/ui/missing_const_for_fn/could_be_const.rs:168:9 @@ -154,6 +233,11 @@ LL | | let vec = Vec::new(); LL | | Self { text, vec } LL | | } | |_________^ + | +help: make the function `const` + | +LL | pub const fn new(text: String) -> Self { + | +++++ error: aborting due to 17 previous errors diff --git a/tests/ui/needless_doc_main.rs b/tests/ui/needless_doc_main.rs index fee05926ce4..18adfff8beb 100644 --- a/tests/ui/needless_doc_main.rs +++ b/tests/ui/needless_doc_main.rs @@ -10,7 +10,7 @@ /// unimplemented!(); /// } /// ``` -/// +/// /// With an explicit return type it should lint too /// ```edition2015 /// fn main() -> () { @@ -18,7 +18,7 @@ /// unimplemented!(); /// } /// ``` -/// +/// /// This should, too. /// ```rust /// fn main() { @@ -26,7 +26,7 @@ /// unimplemented!(); /// } /// ``` -/// +/// /// This one too. /// ```no_run /// // the fn is not always the first line diff --git a/tests/ui/octal_escapes.rs b/tests/ui/octal_escapes.rs index 3915dfdb841..f2664e2fa67 100644 --- a/tests/ui/octal_escapes.rs +++ b/tests/ui/octal_escapes.rs @@ -2,25 +2,20 @@ #![warn(clippy::octal_escapes)] fn main() { - let _bad1 = "\033[0m"; - //~^ ERROR: octal-looking escape in string literal - let _bad2 = b"\033[0m"; - //~^ ERROR: octal-looking escape in byte string literal - let _bad3 = "\\\033[0m"; - //~^ ERROR: octal-looking escape in string literal + let _bad1 = "\033[0m"; //~ octal_escapes + let _bad2 = b"\033[0m"; //~ octal_escapes + let _bad3 = "\\\033[0m"; //~ octal_escapes // maximum 3 digits (\012 is the escape) - let _bad4 = "\01234567"; - //~^ ERROR: octal-looking escape in string literal - let _bad5 = "\0\03"; - //~^ ERROR: octal-looking escape in string literal + let _bad4 = "\01234567"; //~ octal_escapes + let _bad5 = "\0\03"; //~ octal_escapes let _bad6 = "Text-\055\077-MoreText"; - //~^ ERROR: octal-looking escape in string literal + //~^ octal_escapes + //~| octal_escapes let _bad7 = "EvenMoreText-\01\02-ShortEscapes"; - //~^ ERROR: octal-looking escape in string literal - let _bad8 = "锈\01锈"; - //~^ ERROR: octal-looking escape in string literal - let _bad9 = "锈\011锈"; - //~^ ERROR: octal-looking escape in string literal + //~^ octal_escapes + //~| octal_escapes + let _bad8 = "锈\01锈"; //~ octal_escapes + let _bad9 = "锈\011锈"; //~ octal_escapes let _good1 = "\\033[0m"; let _good2 = "\0\\0"; diff --git a/tests/ui/octal_escapes.stderr b/tests/ui/octal_escapes.stderr index 7ed9ee3ae2f..9343ba64a30 100644 --- a/tests/ui/octal_escapes.stderr +++ b/tests/ui/octal_escapes.stderr @@ -1,148 +1,170 @@ -error: octal-looking escape in string literal - --> tests/ui/octal_escapes.rs:5:17 +error: octal-looking escape in a literal + --> tests/ui/octal_escapes.rs:5:18 | LL | let _bad1 = "\033[0m"; - | ^^^^^^^^^ + | ^^^^ | - = help: octal escapes are not supported, `\0` is always a null character + = help: octal escapes are not supported, `\0` is always null = note: `-D clippy::octal-escapes` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::octal_escapes)]` -help: if an octal escape was intended, use the hexadecimal representation instead +help: if an octal escape is intended, use a hex escape instead | LL | let _bad1 = "\x1b[0m"; - | ~~~~~~~~~ -help: if the null character is intended, disambiguate using + | ~~~~ +help: if a null escape is intended, disambiguate using | LL | let _bad1 = "\x0033[0m"; - | ~~~~~~~~~~~ + | ~~~~~~ -error: octal-looking escape in byte string literal - --> tests/ui/octal_escapes.rs:7:17 +error: octal-looking escape in a literal + --> tests/ui/octal_escapes.rs:6:19 | LL | let _bad2 = b"\033[0m"; - | ^^^^^^^^^^ + | ^^^^ | - = help: octal escapes are not supported, `\0` is always a null byte -help: if an octal escape was intended, use the hexadecimal representation instead +help: if an octal escape is intended, use a hex escape instead | LL | let _bad2 = b"\x1b[0m"; - | ~~~~~~~~~~ -help: if the null byte is intended, disambiguate using + | ~~~~ +help: if a null escape is intended, disambiguate using | LL | let _bad2 = b"\x0033[0m"; - | ~~~~~~~~~~~~ + | ~~~~~~ -error: octal-looking escape in string literal - --> tests/ui/octal_escapes.rs:9:17 +error: octal-looking escape in a literal + --> tests/ui/octal_escapes.rs:7:20 | LL | let _bad3 = "\\\033[0m"; - | ^^^^^^^^^^^ + | ^^^^ | - = help: octal escapes are not supported, `\0` is always a null character -help: if an octal escape was intended, use the hexadecimal representation instead +help: if an octal escape is intended, use a hex escape instead | LL | let _bad3 = "\\\x1b[0m"; - | ~~~~~~~~~~~ -help: if the null character is intended, disambiguate using + | ~~~~ +help: if a null escape is intended, disambiguate using | LL | let _bad3 = "\\\x0033[0m"; - | ~~~~~~~~~~~~~ + | ~~~~~~ -error: octal-looking escape in string literal - --> tests/ui/octal_escapes.rs:12:17 +error: octal-looking escape in a literal + --> tests/ui/octal_escapes.rs:9:18 | LL | let _bad4 = "\01234567"; - | ^^^^^^^^^^^ + | ^^^^ | - = help: octal escapes are not supported, `\0` is always a null character -help: if an octal escape was intended, use the hexadecimal representation instead +help: if an octal escape is intended, use a hex escape instead | LL | let _bad4 = "\x0a34567"; - | ~~~~~~~~~~~ -help: if the null character is intended, disambiguate using + | ~~~~ +help: if a null escape is intended, disambiguate using | LL | let _bad4 = "\x001234567"; - | ~~~~~~~~~~~~~ + | ~~~~~~ -error: octal-looking escape in string literal - --> tests/ui/octal_escapes.rs:14:17 +error: octal-looking escape in a literal + --> tests/ui/octal_escapes.rs:10:20 | LL | let _bad5 = "\0\03"; - | ^^^^^^^ + | ^^^ | - = help: octal escapes are not supported, `\0` is always a null character -help: if an octal escape was intended, use the hexadecimal representation instead +help: if an octal escape is intended, use a hex escape instead | LL | let _bad5 = "\0\x03"; - | ~~~~~~~~ -help: if the null character is intended, disambiguate using + | ~~~~ +help: if a null escape is intended, disambiguate using | -LL | let _bad5 = "\0\x003"; - | ~~~~~~~~~ +LL | let _bad5 = "\0\x0003"; + | ~~~~~~ -error: octal-looking escape in string literal - --> tests/ui/octal_escapes.rs:16:17 +error: octal-looking escape in a literal + --> tests/ui/octal_escapes.rs:11:23 | LL | let _bad6 = "Text-\055\077-MoreText"; - | ^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^ | - = help: octal escapes are not supported, `\0` is always a null character -help: if an octal escape was intended, use the hexadecimal representation instead +help: if an octal escape is intended, use a hex escape instead | -LL | let _bad6 = "Text-\x2d\x3f-MoreText"; - | ~~~~~~~~~~~~~~~~~~~~~~~~ -help: if the null character is intended, disambiguate using +LL | let _bad6 = "Text-\x2d\077-MoreText"; + | ~~~~ +help: if a null escape is intended, disambiguate using | -LL | let _bad6 = "Text-\x0055\x0077-MoreText"; - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +LL | let _bad6 = "Text-\x0055\077-MoreText"; + | ~~~~~~ -error: octal-looking escape in string literal - --> tests/ui/octal_escapes.rs:18:17 +error: octal-looking escape in a literal + --> tests/ui/octal_escapes.rs:11:27 + | +LL | let _bad6 = "Text-\055\077-MoreText"; + | ^^^^ + | +help: if an octal escape is intended, use a hex escape instead + | +LL | let _bad6 = "Text-\055\x3f-MoreText"; + | ~~~~ +help: if a null escape is intended, disambiguate using + | +LL | let _bad6 = "Text-\055\x0077-MoreText"; + | ~~~~~~ + +error: octal-looking escape in a literal + --> tests/ui/octal_escapes.rs:14:31 + | +LL | let _bad7 = "EvenMoreText-\01\02-ShortEscapes"; + | ^^^ + | +help: if an octal escape is intended, use a hex escape instead + | +LL | let _bad7 = "EvenMoreText-\x01\02-ShortEscapes"; + | ~~~~ +help: if a null escape is intended, disambiguate using + | +LL | let _bad7 = "EvenMoreText-\x0001\02-ShortEscapes"; + | ~~~~~~ + +error: octal-looking escape in a literal + --> tests/ui/octal_escapes.rs:14:34 | LL | let _bad7 = "EvenMoreText-\01\02-ShortEscapes"; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^ | - = help: octal escapes are not supported, `\0` is always a null character -help: if an octal escape was intended, use the hexadecimal representation instead +help: if an octal escape is intended, use a hex escape instead | -LL | let _bad7 = "EvenMoreText-\x01\x02-ShortEscapes"; - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -help: if the null character is intended, disambiguate using +LL | let _bad7 = "EvenMoreText-\01\x02-ShortEscapes"; + | ~~~~ +help: if a null escape is intended, disambiguate using | -LL | let _bad7 = "EvenMoreText-\x001\x002-ShortEscapes"; - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +LL | let _bad7 = "EvenMoreText-\01\x0002-ShortEscapes"; + | ~~~~~~ -error: octal-looking escape in string literal - --> tests/ui/octal_escapes.rs:20:17 +error: octal-looking escape in a literal + --> tests/ui/octal_escapes.rs:17:19 | LL | let _bad8 = "锈\01锈"; - | ^^^^^^^^^ + | ^^^ | - = help: octal escapes are not supported, `\0` is always a null character -help: if an octal escape was intended, use the hexadecimal representation instead +help: if an octal escape is intended, use a hex escape instead | LL | let _bad8 = "锈\x01锈"; - | ~~~~~~~~~~ -help: if the null character is intended, disambiguate using + | ~~~~ +help: if a null escape is intended, disambiguate using | -LL | let _bad8 = "锈\x001锈"; - | ~~~~~~~~~~~ +LL | let _bad8 = "锈\x0001锈"; + | ~~~~~~ -error: octal-looking escape in string literal - --> tests/ui/octal_escapes.rs:22:17 +error: octal-looking escape in a literal + --> tests/ui/octal_escapes.rs:18:19 | LL | let _bad9 = "锈\011锈"; - | ^^^^^^^^^^ + | ^^^^ | - = help: octal escapes are not supported, `\0` is always a null character -help: if an octal escape was intended, use the hexadecimal representation instead +help: if an octal escape is intended, use a hex escape instead | LL | let _bad9 = "锈\x09锈"; - | ~~~~~~~~~~ -help: if the null character is intended, disambiguate using + | ~~~~ +help: if a null escape is intended, disambiguate using | LL | let _bad9 = "锈\x0011锈"; - | ~~~~~~~~~~~~ + | ~~~~~~ -error: aborting due to 9 previous errors +error: aborting due to 11 previous errors diff --git a/tests/ui/option_if_let_else.fixed b/tests/ui/option_if_let_else.fixed index eeab801b7da..0ac89bf0d8e 100644 --- a/tests/ui/option_if_let_else.fixed +++ b/tests/ui/option_if_let_else.fixed @@ -4,7 +4,8 @@ clippy::equatable_if_let, clippy::let_unit_value, clippy::redundant_locals, - clippy::manual_unwrap_or_default + clippy::manual_unwrap_or_default, + clippy::manual_unwrap_or )] fn bad1(string: Option<&str>) -> (bool, &str) { diff --git a/tests/ui/option_if_let_else.rs b/tests/ui/option_if_let_else.rs index 3e5b96d7c31..b4f1b2cd1f7 100644 --- a/tests/ui/option_if_let_else.rs +++ b/tests/ui/option_if_let_else.rs @@ -4,7 +4,8 @@ clippy::equatable_if_let, clippy::let_unit_value, clippy::redundant_locals, - clippy::manual_unwrap_or_default + clippy::manual_unwrap_or_default, + clippy::manual_unwrap_or )] fn bad1(string: Option<&str>) -> (bool, &str) { diff --git a/tests/ui/option_if_let_else.stderr b/tests/ui/option_if_let_else.stderr index f5359a0c34f..37ef791edb0 100644 --- a/tests/ui/option_if_let_else.stderr +++ b/tests/ui/option_if_let_else.stderr @@ -1,5 +1,5 @@ error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:11:5 + --> tests/ui/option_if_let_else.rs:12:5 | LL | / if let Some(x) = string { LL | | (true, x) @@ -12,19 +12,19 @@ LL | | } = help: to override `-D warnings` add `#[allow(clippy::option_if_let_else)]` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:29:13 + --> tests/ui/option_if_let_else.rs:30:13 | LL | let _ = if let Some(s) = *string { s.len() } else { 0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `string.map_or(0, |s| s.len())` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:30:13 + --> tests/ui/option_if_let_else.rs:31:13 | LL | let _ = if let Some(s) = &num { s } else { &0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:31:13 + --> tests/ui/option_if_let_else.rs:32:13 | LL | let _ = if let Some(s) = &mut num { | _____________^ @@ -44,13 +44,13 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:37:13 + --> tests/ui/option_if_let_else.rs:38:13 | LL | let _ = if let Some(ref s) = num { s } else { &0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:38:13 + --> tests/ui/option_if_let_else.rs:39:13 | LL | let _ = if let Some(mut s) = num { | _____________^ @@ -70,7 +70,7 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:44:13 + --> tests/ui/option_if_let_else.rs:45:13 | LL | let _ = if let Some(ref mut s) = num { | _____________^ @@ -90,7 +90,7 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:53:5 + --> tests/ui/option_if_let_else.rs:54:5 | LL | / if let Some(x) = arg { LL | | let y = x * x; @@ -109,7 +109,7 @@ LL + }) | error: use Option::map_or_else instead of an if let/else - --> tests/ui/option_if_let_else.rs:66:13 + --> tests/ui/option_if_let_else.rs:67:13 | LL | let _ = if let Some(x) = arg { | _____________^ @@ -121,7 +121,7 @@ LL | | }; | |_____^ help: try: `arg.map_or_else(side_effect, |x| x)` error: use Option::map_or_else instead of an if let/else - --> tests/ui/option_if_let_else.rs:75:13 + --> tests/ui/option_if_let_else.rs:76:13 | LL | let _ = if let Some(x) = arg { | _____________^ @@ -144,7 +144,7 @@ LL ~ }, |x| x * x * x * x); | error: use Option::map_or_else instead of an if let/else - --> tests/ui/option_if_let_else.rs:108:13 + --> tests/ui/option_if_let_else.rs:109:13 | LL | / if let Some(idx) = s.find('.') { LL | | vec![s[..idx].to_string(), s[idx..].to_string()] @@ -154,7 +154,7 @@ LL | | } | |_____________^ help: try: `s.find('.').map_or_else(|| vec![s.to_string()], |idx| vec![s[..idx].to_string(), s[idx..].to_string()])` error: use Option::map_or_else instead of an if let/else - --> tests/ui/option_if_let_else.rs:119:5 + --> tests/ui/option_if_let_else.rs:120:5 | LL | / if let Ok(binding) = variable { LL | | println!("Ok {binding}"); @@ -177,13 +177,13 @@ LL + }) | error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:143:13 + --> tests/ui/option_if_let_else.rs:144:13 | LL | let _ = if let Some(x) = optional { x + 2 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:153:13 + --> tests/ui/option_if_let_else.rs:154:13 | LL | let _ = if let Some(x) = Some(0) { | _____________^ @@ -205,13 +205,13 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:181:13 + --> tests/ui/option_if_let_else.rs:182:13 | LL | let _ = if let Some(x) = Some(0) { s.len() + x } else { s.len() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or(s.len(), |x| s.len() + x)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:185:13 + --> tests/ui/option_if_let_else.rs:186:13 | LL | let _ = if let Some(x) = Some(0) { | _____________^ @@ -231,7 +231,7 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:224:13 + --> tests/ui/option_if_let_else.rs:225:13 | LL | let _ = match s { | _____________^ @@ -241,7 +241,7 @@ LL | | }; | |_____^ help: try: `s.map_or(1, |string| string.len())` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:228:13 + --> tests/ui/option_if_let_else.rs:229:13 | LL | let _ = match Some(10) { | _____________^ @@ -251,7 +251,7 @@ LL | | }; | |_____^ help: try: `Some(10).map_or(5, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:234:13 + --> tests/ui/option_if_let_else.rs:235:13 | LL | let _ = match res { | _____________^ @@ -261,7 +261,7 @@ LL | | }; | |_____^ help: try: `res.map_or(1, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:238:13 + --> tests/ui/option_if_let_else.rs:239:13 | LL | let _ = match res { | _____________^ @@ -271,13 +271,13 @@ LL | | }; | |_____^ help: try: `res.map_or(1, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:242:13 + --> tests/ui/option_if_let_else.rs:243:13 | LL | let _ = if let Ok(a) = res { a + 1 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `res.map_or(5, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:259:17 + --> tests/ui/option_if_let_else.rs:260:17 | LL | let _ = match initial { | _________________^ @@ -287,7 +287,7 @@ LL | | }; | |_________^ help: try: `initial.as_ref().map_or(42, |value| do_something(value))` error: use Option::map_or instead of an if let/else - --> tests/ui/option_if_let_else.rs:266:17 + --> tests/ui/option_if_let_else.rs:267:17 | LL | let _ = match initial { | _________________^ @@ -297,7 +297,7 @@ LL | | }; | |_________^ help: try: `initial.as_mut().map_or(42, |value| do_something2(value))` error: use Option::map_or_else instead of an if let/else - --> tests/ui/option_if_let_else.rs:289:24 + --> tests/ui/option_if_let_else.rs:290:24 | LL | let mut _hashmap = if let Some(hm) = &opt { | ________________________^ @@ -308,7 +308,7 @@ LL | | }; | |_____^ help: try: `opt.as_ref().map_or_else(HashMap::new, |hm| hm.clone())` error: use Option::map_or_else instead of an if let/else - --> tests/ui/option_if_let_else.rs:295:19 + --> tests/ui/option_if_let_else.rs:296:19 | LL | let mut _hm = if let Some(hm) = &opt { hm.clone() } else { new_map!() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.as_ref().map_or_else(|| new_map!(), |hm| hm.clone())` diff --git a/tests/ui/temporary_assignment.rs b/tests/ui/temporary_assignment.rs index 383e70be925..e2b982aeaf5 100644 --- a/tests/ui/temporary_assignment.rs +++ b/tests/ui/temporary_assignment.rs @@ -1,5 +1,4 @@ #![warn(clippy::temporary_assignment)] -#![allow(const_item_mutation)] use std::ops::{Deref, DerefMut}; diff --git a/tests/ui/temporary_assignment.stderr b/tests/ui/temporary_assignment.stderr index 8c284594075..1b6007f0b98 100644 --- a/tests/ui/temporary_assignment.stderr +++ b/tests/ui/temporary_assignment.stderr @@ -1,5 +1,5 @@ error: assignment to temporary - --> tests/ui/temporary_assignment.rs:48:5 + --> tests/ui/temporary_assignment.rs:47:5 | LL | Struct { field: 0 }.field = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,7 +8,7 @@ LL | Struct { field: 0 }.field = 1; = help: to override `-D warnings` add `#[allow(clippy::temporary_assignment)]` error: assignment to temporary - --> tests/ui/temporary_assignment.rs:51:5 + --> tests/ui/temporary_assignment.rs:50:5 | LL | / MultiStruct { LL | | @@ -19,13 +19,13 @@ LL | | .field = 1; | |______________^ error: assignment to temporary - --> tests/ui/temporary_assignment.rs:57:5 + --> tests/ui/temporary_assignment.rs:56:5 | LL | ArrayStruct { array: [0] }.array[0] = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: assignment to temporary - --> tests/ui/temporary_assignment.rs:59:5 + --> tests/ui/temporary_assignment.rs:58:5 | LL | (0, 0).0 = 1; | ^^^^^^^^^^^^ diff --git a/tests/ui/to_string_in_format_args_incremental.fixed b/tests/ui/to_string_in_format_args_incremental.fixed new file mode 100644 index 00000000000..1f789579636 --- /dev/null +++ b/tests/ui/to_string_in_format_args_incremental.fixed @@ -0,0 +1,8 @@ +//@compile-flags: -C incremental=target/debug/test/incr + +// see https://github.com/rust-lang/rust-clippy/issues/10969 + +fn main() { + let s = "Hello, world!"; + println!("{}", s); +} diff --git a/tests/ui/to_string_in_format_args_incremental.rs b/tests/ui/to_string_in_format_args_incremental.rs new file mode 100644 index 00000000000..514febe8c92 --- /dev/null +++ b/tests/ui/to_string_in_format_args_incremental.rs @@ -0,0 +1,8 @@ +//@compile-flags: -C incremental=target/debug/test/incr + +// see https://github.com/rust-lang/rust-clippy/issues/10969 + +fn main() { + let s = "Hello, world!"; + println!("{}", s.to_string()); +} diff --git a/tests/ui/to_string_in_format_args_incremental.stderr b/tests/ui/to_string_in_format_args_incremental.stderr new file mode 100644 index 00000000000..535dd21ea58 --- /dev/null +++ b/tests/ui/to_string_in_format_args_incremental.stderr @@ -0,0 +1,11 @@ +error: `to_string` applied to a type that implements `Display` in `println!` args + --> tests/ui/to_string_in_format_args_incremental.rs:7:21 + | +LL | println!("{}", s.to_string()); + | ^^^^^^^^^^^^ help: remove this + | + = note: `-D clippy::to-string-in-format-args` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::to_string_in_format_args)]` + +error: aborting due to 1 previous error + diff --git a/tests/ui/unnecessary_min_or_max.fixed b/tests/ui/unnecessary_min_or_max.fixed new file mode 100644 index 00000000000..392f6dd1fee --- /dev/null +++ b/tests/ui/unnecessary_min_or_max.fixed @@ -0,0 +1,67 @@ +//@aux-build:external_consts.rs + +#![allow(unused)] +#![warn(clippy::unnecessary_min_or_max)] +#![allow(clippy::identity_op)] + +extern crate external_consts; + +const X: i32 = 1; + +fn main() { + // Both are Literals + let _ = (-6_i32); + let _ = 9; + let _ = 6; + let _ = 9_u32; + let _ = 6; + let _ = 7_u8; + + let x: u32 = 42; + // unsigned with zero + let _ = 0; + let _ = x; + let _ = 0_u32; + let _ = x; + + let x: i32 = 42; + // signed MIN + let _ = i32::MIN; + let _ = x; + let _ = i32::MIN; + let _ = x; + + let _ = i32::MIN - 0; + let _ = x; + + let _ = i32::MIN - 0; + + // The below cases shouldn't be lint + let mut min = u32::MAX; + for _ in 0..1000 { + min = min.min(random_u32()); + } + + let _ = 2.min(external_consts::MAGIC_NUMBER); + let _ = 2.max(external_consts::MAGIC_NUMBER); + let _ = external_consts::MAGIC_NUMBER.min(2); + let _ = external_consts::MAGIC_NUMBER.max(2); + + let _ = X.min(external_consts::MAGIC_NUMBER); + let _ = X.max(external_consts::MAGIC_NUMBER); + let _ = external_consts::MAGIC_NUMBER.min(X); + let _ = external_consts::MAGIC_NUMBER.max(X); + + let _ = X.max(12); + let _ = X.min(12); + let _ = 12.min(X); + let _ = 12.max(X); + let _ = (X + 1).max(12); + let _ = (X + 1).min(12); + let _ = 12.min(X - 1); + let _ = 12.max(X - 1); +} +fn random_u32() -> u32 { + // random number generator + 0 +} diff --git a/tests/ui/unnecessary_min_or_max.rs b/tests/ui/unnecessary_min_or_max.rs new file mode 100644 index 00000000000..b03755e6d23 --- /dev/null +++ b/tests/ui/unnecessary_min_or_max.rs @@ -0,0 +1,67 @@ +//@aux-build:external_consts.rs + +#![allow(unused)] +#![warn(clippy::unnecessary_min_or_max)] +#![allow(clippy::identity_op)] + +extern crate external_consts; + +const X: i32 = 1; + +fn main() { + // Both are Literals + let _ = (-6_i32).min(9); + let _ = (-6_i32).max(9); + let _ = 9_u32.min(6); + let _ = 9_u32.max(6); + let _ = 6.min(7_u8); + let _ = 6.max(7_u8); + + let x: u32 = 42; + // unsigned with zero + let _ = 0.min(x); + let _ = 0.max(x); + let _ = x.min(0_u32); + let _ = x.max(0_u32); + + let x: i32 = 42; + // signed MIN + let _ = i32::MIN.min(x); + let _ = i32::MIN.max(x); + let _ = x.min(i32::MIN); + let _ = x.max(i32::MIN); + + let _ = x.min(i32::MIN - 0); + let _ = x.max(i32::MIN); + + let _ = x.min(i32::MIN - 0); + + // The below cases shouldn't be lint + let mut min = u32::MAX; + for _ in 0..1000 { + min = min.min(random_u32()); + } + + let _ = 2.min(external_consts::MAGIC_NUMBER); + let _ = 2.max(external_consts::MAGIC_NUMBER); + let _ = external_consts::MAGIC_NUMBER.min(2); + let _ = external_consts::MAGIC_NUMBER.max(2); + + let _ = X.min(external_consts::MAGIC_NUMBER); + let _ = X.max(external_consts::MAGIC_NUMBER); + let _ = external_consts::MAGIC_NUMBER.min(X); + let _ = external_consts::MAGIC_NUMBER.max(X); + + let _ = X.max(12); + let _ = X.min(12); + let _ = 12.min(X); + let _ = 12.max(X); + let _ = (X + 1).max(12); + let _ = (X + 1).min(12); + let _ = 12.min(X - 1); + let _ = 12.max(X - 1); +} +fn random_u32() -> u32 { + // random number generator + 0 +} diff --git a/tests/ui/unnecessary_min_or_max.stderr b/tests/ui/unnecessary_min_or_max.stderr new file mode 100644 index 00000000000..f5cd31fbaf2 --- /dev/null +++ b/tests/ui/unnecessary_min_or_max.stderr @@ -0,0 +1,107 @@ +error: `(-6_i32)` is never greater than `9` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:13:13 + | +LL | let _ = (-6_i32).min(9); + | ^^^^^^^^^^^^^^^ help: try: `(-6_i32)` + | + = note: `-D clippy::unnecessary-min-or-max` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unnecessary_min_or_max)]` + +error: `(-6_i32)` is never greater than `9` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:14:13 + | +LL | let _ = (-6_i32).max(9); + | ^^^^^^^^^^^^^^^ help: try: `9` + +error: `9_u32` is never smaller than `6` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:15:13 + | +LL | let _ = 9_u32.min(6); + | ^^^^^^^^^^^^ help: try: `6` + +error: `9_u32` is never smaller than `6` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:16:13 + | +LL | let _ = 9_u32.max(6); + | ^^^^^^^^^^^^ help: try: `9_u32` + +error: `6` is never greater than `7_u8` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:17:13 + | +LL | let _ = 6.min(7_u8); + | ^^^^^^^^^^^ help: try: `6` + +error: `6` is never greater than `7_u8` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:18:13 + | +LL | let _ = 6.max(7_u8); + | ^^^^^^^^^^^ help: try: `7_u8` + +error: `0` is never greater than `x` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:22:13 + | +LL | let _ = 0.min(x); + | ^^^^^^^^ help: try: `0` + +error: `0` is never greater than `x` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:23:13 + | +LL | let _ = 0.max(x); + | ^^^^^^^^ help: try: `x` + +error: `x` is never smaller than `0_u32` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:24:13 + | +LL | let _ = x.min(0_u32); + | ^^^^^^^^^^^^ help: try: `0_u32` + +error: `x` is never smaller than `0_u32` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:25:13 + | +LL | let _ = x.max(0_u32); + | ^^^^^^^^^^^^ help: try: `x` + +error: `i32::MIN` is never greater than `x` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:29:13 + | +LL | let _ = i32::MIN.min(x); + | ^^^^^^^^^^^^^^^ help: try: `i32::MIN` + +error: `i32::MIN` is never greater than `x` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:30:13 + | +LL | let _ = i32::MIN.max(x); + | ^^^^^^^^^^^^^^^ help: try: `x` + +error: `x` is never smaller than `i32::MIN` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:31:13 + | +LL | let _ = x.min(i32::MIN); + | ^^^^^^^^^^^^^^^ help: try: `i32::MIN` + +error: `x` is never smaller than `i32::MIN` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:32:13 + | +LL | let _ = x.max(i32::MIN); + | ^^^^^^^^^^^^^^^ help: try: `x` + +error: `x` is never smaller than `i32::MIN - 0` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:34:13 + | +LL | let _ = x.min(i32::MIN - 0); + | ^^^^^^^^^^^^^^^^^^^ help: try: `i32::MIN - 0` + +error: `x` is never smaller than `i32::MIN` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:35:13 + | +LL | let _ = x.max(i32::MIN); + | ^^^^^^^^^^^^^^^ help: try: `x` + +error: `x` is never smaller than `i32::MIN - 0` and has therefore no effect + --> tests/ui/unnecessary_min_or_max.rs:37:13 + | +LL | let _ = x.min(i32::MIN - 0); + | ^^^^^^^^^^^^^^^^^^^ help: try: `i32::MIN - 0` + +error: aborting due to 17 previous errors + diff --git a/triagebot.toml b/triagebot.toml index 4d66b728b76..0f0e62670ff 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -22,7 +22,6 @@ contributing_url = "https://github.com/rust-lang/rust-clippy/blob/master/CONTRIB users_on_vacation = [ "matthiaskrgr", "giraffate", - "Centri3", ] [assign.owners] @@ -32,10 +31,10 @@ users_on_vacation = [ "*" = [ "@Manishearth", "@llogiq", - "@xFrednet", "@Alexendoo", "@dswij", "@Jarcho", "@blyxyas", "@y21", + "@Centri3", ] diff --git a/util/gh-pages/index.html b/util/gh-pages/index.html index c88c298d5d7..8de36fc4005 100644 --- a/util/gh-pages/index.html +++ b/util/gh-pages/index.html @@ -103,6 +103,7 @@ Otherwise, have a great day =^.^= @media (min-width: 405px) { #upper-filters { display: flex; + flex-wrap: wrap; } } @@ -404,7 +405,7 @@ Otherwise, have a great day =^.^= <div class="panel panel-default" ng-show="data"> <div class="panel-body row"> - <div id="upper-filters" class="col-12 col-md-4"> + <div id="upper-filters" class="col-12 col-md-6"> <div class="btn-group" filter-dropdown> <button type="button" class="btn btn-default dropdown-toggle"> Lint levels <span class="badge">{{selectedValuesCount(levels)}}</span> <span class="caret"></span> @@ -496,9 +497,34 @@ Otherwise, have a great day =^.^= </ul> </div> </div> - + <div class="btn-group" filter-dropdown> + <button type="button" class="btn btn-default dropdown-toggle"> + Applicability <span class="badge">{{selectedValuesCount(applicabilities)}}</span> <span class="caret"></span> + </button> + <ul class="dropdown-menu"> + <li class="checkbox"> + <label ng-click="toggleApplicabilities(true)"> + <input type="checkbox" class="invisible" /> + All + </label> + </li> + <li class="checkbox"> + <label ng-click="toggleApplicabilities(false)"> + <input type="checkbox" class="invisible" /> + None + </label> + </li> + <li role="separator" class="divider"></li> + <li class="checkbox" ng-repeat="(applicability, enabled) in applicabilities"> + <label class="text-capitalize"> + <input type="checkbox" ng-model="applicabilities[applicability]" /> + {{applicability}} + </label> + </li> + </ul> + </div> </div> - <div class="col-12 col-md-7 search-control"> + <div class="col-12 col-md-6 search-control"> <div class="input-group"> <label class="input-group-addon" id="filter-label" for="search-input">Filter:</label> <input type="text" class="form-control filter-input" placeholder="Keywords or search string" id="search-input" @@ -514,7 +540,7 @@ Otherwise, have a great day =^.^= </div> </div> <!-- The order of the filters should be from most likely to remove a lint to least likely to improve performance. --> - <article class="panel panel-default" id="{{lint.id}}" ng-repeat="lint in data | filter:bySearch | filter:byGroups | filter:byLevels | filter:byVersion"> + <article class="panel panel-default" id="{{lint.id}}" ng-repeat="lint in data | filter:bySearch | filter:byGroups | filter:byLevels | filter:byVersion | filter:byApplicabilities"> <header class="panel-heading" ng-click="open[lint.id] = !open[lint.id]"> <h2 class="panel-title"> <div class="panel-title-name"> diff --git a/util/gh-pages/script.js b/util/gh-pages/script.js index 7cca298df8e..921bb0376f6 100644 --- a/util/gh-pages/script.js +++ b/util/gh-pages/script.js @@ -156,6 +156,18 @@ Object.entries(versionFilterKeyMap).map(([key, value]) => [value, key]) ); + const APPLICABILITIES_FILTER_DEFAULT = { + Unspecified: true, + Unresolved: true, + MachineApplicable: true, + MaybeIncorrect: true, + HasPlaceholders: true + }; + + $scope.applicabilities = { + ...APPLICABILITIES_FILTER_DEFAULT + } + // loadFromURLParameters retrieves filter settings from the URL parameters and assigns them // to corresponding $scope variables. function loadFromURLParameters() { @@ -182,6 +194,7 @@ handleParameter('levels', $scope.levels, LEVEL_FILTERS_DEFAULT); handleParameter('groups', $scope.groups, GROUPS_FILTER_DEFAULT); + handleParameter('applicabilities', $scope.applicabilities, APPLICABILITIES_FILTER_DEFAULT); // Handle 'versions' parameter separately because it needs additional processing if (urlParameters.versions) { @@ -249,6 +262,7 @@ updateURLParameter($scope.levels, 'levels', LEVEL_FILTERS_DEFAULT); updateURLParameter($scope.groups, 'groups', GROUPS_FILTER_DEFAULT); updateVersionURLParameter($scope.versionFilters); + updateURLParameter($scope.applicabilities, 'applicabilities', APPLICABILITIES_FILTER_DEFAULT); } // Add $watches to automatically update URL parameters when the data changes @@ -270,6 +284,12 @@ } }, true); + $scope.$watch('applicabilities', function (newVal, oldVal) { + if (newVal !== oldVal) { + updateURLParameter(newVal, 'applicabilities', APPLICABILITIES_FILTER_DEFAULT) + } + }, true); + // Watch for changes in the URL path and update the search and lint display $scope.$watch(function () { return $location.path(); }, function (newPath) { const searchParameter = newPath.substring(1); @@ -327,6 +347,15 @@ } }; + $scope.toggleApplicabilities = function (value) { + const applicabilities = $scope.applicabilities; + for (const key in applicabilities) { + if (applicabilities.hasOwnProperty(key)) { + applicabilities[key] = value; + } + } + } + $scope.resetGroupsToDefault = function () { $scope.groups = { ...GROUPS_FILTER_DEFAULT @@ -430,6 +459,10 @@ return true; } + $scope.byApplicabilities = function (lint) { + return $scope.applicabilities[lint.applicability.applicability]; + }; + // Show details for one lint $scope.openLint = function (lint) { $scope.open[lint.id] = true; |