diff options
36 files changed, 575 insertions, 253 deletions
diff --git a/.github/workflows/feature_freeze.yml b/.github/workflows/feature_freeze.yml index a5f8d4bc145..7ad58af77d4 100644 --- a/.github/workflows/feature_freeze.yml +++ b/.github/workflows/feature_freeze.yml @@ -1,7 +1,11 @@ name: Feature freeze check on: - pull_request: + pull_request_target: + types: + - opened + branches: + - master paths: - 'clippy_lints/src/declared_lints.rs' @@ -9,6 +13,12 @@ jobs: auto-comment: runs-on: ubuntu-latest + permissions: + pull-requests: write + + # Do not in any case add code that runs anything coming from the the content + # of the pull request, as malicious code would be able to access the private + # GitHub token. steps: - name: Check PR Changes id: pr-changes @@ -19,7 +29,7 @@ jobs: run: | # Use GitHub API to create a comment on the PR PR_NUMBER=${{ github.event.pull_request.number }} - COMMENT="**Seems that you are trying to add a new lint!**\nWe are currently in a [feature freeze](https://doc.rust-lang.org/nightly/clippy/development/feature_freeze.html), so we are delaying all lint-adding PRs to August 1st and focusing on bugfixes.\nThanks a lot for your contribution, and sorry for the inconvenience.\nWith ❤ from the Clippy team" + COMMENT="**Seems that you are trying to add a new lint!**\nWe are currently in a [feature freeze](https://doc.rust-lang.org/nightly/clippy/development/feature_freeze.html), so we are delaying all lint-adding PRs to September 18 and focusing on bugfixes.\nThanks a lot for your contribution, and sorry for the inconvenience.\nWith ❤ from the Clippy team\n\n@rustbot note Feature-freeze\n@rustbot blocked\n@rustbot label +A-lint\n" GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }} COMMENT_URL="https://api.github.com/repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" curl -s -H "Authorization: token ${GITHUB_TOKEN}" -X POST $COMMENT_URL -d "{\"body\":\"$COMMENT\"}" diff --git a/Cargo.toml b/Cargo.toml index 8cbdcf45693..2b6139e7e7f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,13 +28,13 @@ declare_clippy_lint = { path = "declare_clippy_lint" } rustc_tools_util = { path = "rustc_tools_util", version = "0.4.2" } clippy_lints_internal = { path = "clippy_lints_internal", optional = true } tempfile = { version = "3.20", optional = true } -termize = "0.1" +termize = "0.2" color-print = "0.3.4" anstream = "0.6.18" [dev-dependencies] cargo_metadata = "0.18.1" -ui_test = "0.29.2" +ui_test = "0.30.2" regex = "1.5.5" serde = { version = "1.0.145", features = ["derive"] } serde_json = "1.0.122" @@ -45,14 +45,6 @@ itertools = "0.12" pulldown-cmark = { version = "0.11", default-features = false, features = ["html"] } askama = { version = "0.14", default-features = false, features = ["alloc", "config", "derive"] } -# UI test dependencies -if_chain = "1.0" -quote = "1.0.25" -syn = { version = "2.0", features = ["full"] } -futures = "0.3" -parking_lot = "0.12" -tokio = { version = "1", features = ["io-util"] } - [build-dependencies] rustc_tools_util = { path = "rustc_tools_util", version = "0.4.2" } diff --git a/book/src/development/infrastructure/backport.md b/book/src/development/infrastructure/backport.md index 9526d8af1c9..47ea6a412c5 100644 --- a/book/src/development/infrastructure/backport.md +++ b/book/src/development/infrastructure/backport.md @@ -109,4 +109,4 @@ worth backporting this. When a PR is backported to Rust `beta`, label the PR with `beta-accepted`. This will then get picked up when [writing the changelog]. -[writing the changelog]: changelog_update.md#31-include-beta-accepted-prs +[writing the changelog]: changelog_update.md#4-include-beta-accepted-prs diff --git a/book/src/development/infrastructure/changelog_update.md b/book/src/development/infrastructure/changelog_update.md index eede6b78d92..c96ff228b01 100644 --- a/book/src/development/infrastructure/changelog_update.md +++ b/book/src/development/infrastructure/changelog_update.md @@ -38,7 +38,7 @@ Usually you want to write the changelog of the **upcoming stable release**. Make sure though, that `beta` was already branched in the Rust repository. To find the commit hash, issue the following command when in a `rust-lang/rust` -checkout: +checkout (most of the time on the `upstream/beta` branch): ``` git log --oneline -- src/tools/clippy/ | grep -o "Merge commit '[a-f0-9]*' into .*" | head -1 | sed -e "s/Merge commit '\([a-f0-9]*\)' into .*/\1/g" ``` @@ -48,16 +48,13 @@ git log --oneline -- src/tools/clippy/ | grep -o "Merge commit '[a-f0-9]*' into Once you've got the correct commit range, run ``` -util/fetch_prs_between.sh commit1 commit2 > changes.txt +util/fetch_prs_between.sh start_commit end_commit > changes.txt ``` -where `commit2` is the commit hash from the previous command and `commit1` -is the commit hash from the current CHANGELOG file. +where `end_commit` is the commit hash from the previous command and `start_commit` +is [the commit hash][beta_section] from the current CHANGELOG file. Open `changes.txt` file in your editor of choice. -When updating the changelog it's also a good idea to make sure that `commit1` is -already correct in the current changelog. - ### 3. Authoring the final changelog The above script should have dumped all the relevant PRs to the file you @@ -70,17 +67,7 @@ With the PRs filtered, you can start to take each PR and move the `changelog: ` content to `CHANGELOG.md`. Adapt the wording as you see fit but try to keep it somewhat coherent. -The order should roughly be: - -1. New lints -2. Moves or deprecations of lints -3. Changes that expand what code existing lints cover -4. False positive fixes -5. ICE fixes -6. Documentation improvements -7. Others - -As section headers, we use: +The sections order should roughly be: ``` ### New Lints @@ -97,10 +84,10 @@ As section headers, we use: ### Others ``` -Please also be sure to update the Beta/Unreleased sections at the top with the -relevant commit ranges. +Please also be sure to update [the `Unreleased/Beta/In Rust Nightly` section][beta_section] at the top with the +relevant commits ranges and to add the `Rust <version>` section with release date and PR ranges. -#### 3.1 Include `beta-accepted` PRs +### 4. Include `beta-accepted` PRs Look for the [`beta-accepted`] label and make sure to also include the PRs with that label in the changelog. If you can, remove the `beta-accepted` labels @@ -109,7 +96,7 @@ that label in the changelog. If you can, remove the `beta-accepted` labels > _Note:_ Some of those PRs might even get backported to the previous `beta`. > Those have to be included in the changelog of the _previous_ release. -### 4. Update `clippy::version` attributes +### 5. Update `clippy::version` attributes Next, make sure to check that the `#[clippy::version]` attributes for the added lints contain the correct version. @@ -129,3 +116,4 @@ written for. If not, update the version to the changelog version. [rust_beta_tools]: https://github.com/rust-lang/rust/tree/beta/src/tools/clippy [rust_stable_tools]: https://github.com/rust-lang/rust/releases [`beta-accepted`]: https://github.com/rust-lang/rust-clippy/issues?q=label%3Abeta-accepted+ +[beta_section]: https://github.com/rust-lang/rust-clippy/blob/master/CHANGELOG.md#unreleased--beta--in-rust-nightly diff --git a/clippy_dev/src/fmt.rs b/clippy_dev/src/fmt.rs index c1b6b370706..bd9e57c9f6d 100644 --- a/clippy_dev/src/fmt.rs +++ b/clippy_dev/src/fmt.rs @@ -223,7 +223,7 @@ fn fmt_conf(check: bool) -> Result<(), Error> { if check { return Err(Error::CheckFailed); } - fs::write(path, new_text.as_bytes())?; + fs::write(path, new_text)?; } Ok(()) } diff --git a/clippy_lints/src/coerce_container_to_any.rs b/clippy_lints/src/coerce_container_to_any.rs index 2b659253763..6217fc4c897 100644 --- a/clippy_lints/src/coerce_container_to_any.rs +++ b/clippy_lints/src/coerce_container_to_any.rs @@ -1,9 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet; +use clippy_utils::sugg::{self, Sugg}; use clippy_utils::sym; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::adjustment::{Adjust, PointerCoercion}; use rustc_middle::ty::{self, ExistentialPredicate, Ty, TyCtxt}; use rustc_session::declare_lint_pass; @@ -49,23 +50,18 @@ declare_lint_pass!(CoerceContainerToAny => [COERCE_CONTAINER_TO_ANY]); impl<'tcx> LateLintPass<'tcx> for CoerceContainerToAny { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { - // If this expression has an effective type of `&dyn Any` ... - { - let coerced_ty = cx.typeck_results().expr_ty_adjusted(e); - - let ty::Ref(_, coerced_ref_ty, _) = *coerced_ty.kind() else { - return; - }; - if !is_dyn_any(cx.tcx, coerced_ref_ty) { - return; - } + // If this expression was coerced to `&dyn Any` ... + if !cx.typeck_results().expr_adjustments(e).last().is_some_and(|adj| { + matches!(adj.kind, Adjust::Pointer(PointerCoercion::Unsize)) && is_ref_dyn_any(cx.tcx, adj.target) + }) { + return; } let expr_ty = cx.typeck_results().expr_ty(e); let ty::Ref(_, expr_ref_ty, _) = *expr_ty.kind() else { return; }; - // ... but only due to coercion ... + // ... but it's not actually `&dyn Any` ... if is_dyn_any(cx.tcx, expr_ref_ty) { return; } @@ -78,23 +74,37 @@ impl<'tcx> LateLintPass<'tcx> for CoerceContainerToAny { } // ... that's probably not intended. - let (span, deref_count) = match e.kind { + let (target_expr, deref_count) = match e.kind { // If `e` was already an `&` expression, skip `*&` in the suggestion - ExprKind::AddrOf(_, _, referent) => (referent.span, depth), - _ => (e.span, depth + 1), + ExprKind::AddrOf(_, _, referent) => (referent, depth), + _ => (e, depth + 1), }; + let ty::Ref(_, _, mutability) = *cx.typeck_results().expr_ty_adjusted(e).kind() else { + return; + }; + let sugg = sugg::make_unop( + &format!("{}{}", mutability.ref_prefix_str(), str::repeat("*", deref_count)), + Sugg::hir(cx, target_expr, ".."), + ); span_lint_and_sugg( cx, COERCE_CONTAINER_TO_ANY, e.span, - format!("coercing `{expr_ty}` to `&dyn Any`"), + format!("coercing `{expr_ty}` to `{}dyn Any`", mutability.ref_prefix_str()), "consider dereferencing", - format!("&{}{}", str::repeat("*", deref_count), snippet(cx, span, "x")), + sugg.to_string(), Applicability::MaybeIncorrect, ); } } +fn is_ref_dyn_any(tcx: TyCtxt<'_>, ty: Ty<'_>) -> bool { + let ty::Ref(_, ref_ty, _) = *ty.kind() else { + return false; + }; + is_dyn_any(tcx, ref_ty) +} + fn is_dyn_any(tcx: TyCtxt<'_>, ty: Ty<'_>) -> bool { let ty::Dynamic(traits, ..) = ty.kind() else { return false; diff --git a/clippy_lints/src/cognitive_complexity.rs b/clippy_lints/src/cognitive_complexity.rs index 5c64216dd92..d5d937d9133 100644 --- a/clippy_lints/src/cognitive_complexity.rs +++ b/clippy_lints/src/cognitive_complexity.rs @@ -14,18 +14,25 @@ use rustc_span::def_id::LocalDefId; declare_clippy_lint! { /// ### What it does - /// Checks for methods with high cognitive complexity. + /// We used to think it measured how hard a method is to understand. /// /// ### Why is this bad? - /// Methods of high cognitive complexity tend to be hard to - /// both read and maintain. Also LLVM will tend to optimize small methods better. + /// Ideally, we would like to be able to measure how hard a function is + /// to understand given its context (what we call its Cognitive Complexity). + /// But that's not what this lint does. See "Known problems" /// /// ### Known problems - /// Sometimes it's hard to find a way to reduce the - /// complexity. + /// The true Cognitive Complexity of a method is not something we can + /// calculate using modern technology. This lint has been left in the + /// `nursery` so as to not mislead users into using this lint as a + /// measurement tool. /// - /// ### Example - /// You'll see it when you get the warning. + /// For more detailed information, see [rust-clippy#3793](https://github.com/rust-lang/rust-clippy/issues/3793) + /// + /// ### Lints to consider instead of this + /// + /// * [`excessive_nesting`](https://rust-lang.github.io/rust-clippy/master/index.html#excessive_nesting) + /// * [`too_many_lines`](https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines) #[clippy::version = "1.35.0"] pub COGNITIVE_COMPLEXITY, nursery, diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index 1b6c48e53f5..49397938ca7 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -1249,7 +1249,9 @@ fn looks_like_refdef(doc: &str, range: Range<usize>) -> Option<Range<usize>> { b'[' => { start = Some(i + offset); }, - b']' if let Some(start) = start => { + b']' if let Some(start) = start + && doc.as_bytes().get(i + offset + 1) == Some(&b':') => + { return Some(start..i + offset + 1); }, _ => {}, diff --git a/clippy_lints/src/loops/empty_loop.rs b/clippy_lints/src/loops/empty_loop.rs index 823cf0f4322..e809987d75a 100644 --- a/clippy_lints/src/loops/empty_loop.rs +++ b/clippy_lints/src/loops/empty_loop.rs @@ -1,11 +1,22 @@ use super::EMPTY_LOOP; use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::{is_in_panic_handler, is_no_std_crate}; +use clippy_utils::{is_in_panic_handler, is_no_std_crate, sym}; -use rustc_hir::{Block, Expr}; +use rustc_hir::{Block, Expr, ItemKind, Node}; use rustc_lint::LateContext; pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, loop_block: &Block<'_>) { + let parent_hir_id = cx.tcx.parent_hir_id(expr.hir_id); + if let Node::Item(parent_node) = cx.tcx.hir_node(parent_hir_id) + && matches!(parent_node.kind, ItemKind::Fn { .. }) + && let attrs = cx.tcx.hir_attrs(parent_hir_id) + && attrs.iter().any(|attr| attr.has_name(sym::rustc_intrinsic)) + { + // Intrinsic functions are expanded into an empty loop when lowering the AST + // to simplify the job of later passes which might expect any function to have a body. + return; + } + if loop_block.stmts.is_empty() && loop_block.expr.is_none() && !is_in_panic_handler(cx, expr) { let msg = "empty `loop {}` wastes CPU cycles"; let help = if is_no_std_crate(cx) { diff --git a/clippy_lints/src/methods/swap_with_temporary.rs b/clippy_lints/src/methods/swap_with_temporary.rs index de729fb343a..e378cbd6ae0 100644 --- a/clippy_lints/src/methods/swap_with_temporary.rs +++ b/clippy_lints/src/methods/swap_with_temporary.rs @@ -4,6 +4,7 @@ use rustc_ast::BorrowKind; use rustc_errors::{Applicability, Diag}; use rustc_hir::{Expr, ExprKind, Node, QPath}; use rustc_lint::LateContext; +use rustc_middle::ty::adjustment::Adjust; use rustc_span::sym; use super::SWAP_WITH_TEMPORARY; @@ -11,12 +12,12 @@ use super::SWAP_WITH_TEMPORARY; const MSG_TEMPORARY: &str = "this expression returns a temporary value"; const MSG_TEMPORARY_REFMUT: &str = "this is a mutable reference to a temporary value"; -pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, func: &Expr<'_>, args: &[Expr<'_>]) { +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, func: &Expr<'_>, args: &'tcx [Expr<'_>]) { if let ExprKind::Path(QPath::Resolved(_, func_path)) = func.kind && let Some(func_def_id) = func_path.res.opt_def_id() && cx.tcx.is_diagnostic_item(sym::mem_swap, func_def_id) { - match (ArgKind::new(&args[0]), ArgKind::new(&args[1])) { + match (ArgKind::new(cx, &args[0]), ArgKind::new(cx, &args[1])) { (ArgKind::RefMutToTemp(left_temp), ArgKind::RefMutToTemp(right_temp)) => { emit_lint_useless(cx, expr, &args[0], &args[1], left_temp, right_temp); }, @@ -28,10 +29,10 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, func: &Expr<'_>, args } enum ArgKind<'tcx> { - // Mutable reference to a place, coming from a macro - RefMutToPlaceAsMacro(&'tcx Expr<'tcx>), - // Place behind a mutable reference - RefMutToPlace(&'tcx Expr<'tcx>), + // Mutable reference to a place, coming from a macro, and number of dereferences to use + RefMutToPlaceAsMacro(&'tcx Expr<'tcx>, usize), + // Place behind a mutable reference, and number of dereferences to use + RefMutToPlace(&'tcx Expr<'tcx>, usize), // Temporary value behind a mutable reference RefMutToTemp(&'tcx Expr<'tcx>), // Any other case @@ -39,13 +40,29 @@ enum ArgKind<'tcx> { } impl<'tcx> ArgKind<'tcx> { - fn new(arg: &'tcx Expr<'tcx>) -> Self { - if let ExprKind::AddrOf(BorrowKind::Ref, _, target) = arg.kind { - if target.is_syntactic_place_expr() { + /// Build a new `ArgKind` from `arg`. There must be no false positive when returning a + /// `ArgKind::RefMutToTemp` variant, as this may cause a spurious lint to be emitted. + fn new(cx: &LateContext<'tcx>, arg: &'tcx Expr<'tcx>) -> Self { + if let ExprKind::AddrOf(BorrowKind::Ref, _, target) = arg.kind + && let adjustments = cx.typeck_results().expr_adjustments(arg) + && adjustments + .first() + .is_some_and(|adj| matches!(adj.kind, Adjust::Deref(None))) + && adjustments + .last() + .is_some_and(|adj| matches!(adj.kind, Adjust::Borrow(_))) + { + let extra_derefs = adjustments[1..adjustments.len() - 1] + .iter() + .filter(|adj| matches!(adj.kind, Adjust::Deref(_))) + .count(); + // If a deref is used, `arg` might be a place expression. For example, a mutex guard + // would dereference into the mutex content which is probably not temporary. + if target.is_syntactic_place_expr() || extra_derefs > 0 { if arg.span.from_expansion() { - ArgKind::RefMutToPlaceAsMacro(arg) + ArgKind::RefMutToPlaceAsMacro(arg, extra_derefs) } else { - ArgKind::RefMutToPlace(target) + ArgKind::RefMutToPlace(target, extra_derefs) } } else { ArgKind::RefMutToTemp(target) @@ -106,10 +123,15 @@ fn emit_lint_assign(cx: &LateContext<'_>, expr: &Expr<'_>, target: &ArgKind<'_>, let mut applicability = Applicability::MachineApplicable; let ctxt = expr.span.ctxt(); let assign_target = match target { - ArgKind::Expr(target) | ArgKind::RefMutToPlaceAsMacro(target) => { - Sugg::hir_with_context(cx, target, ctxt, "_", &mut applicability).deref() - }, - ArgKind::RefMutToPlace(target) => Sugg::hir_with_context(cx, target, ctxt, "_", &mut applicability), + ArgKind::Expr(target) => Sugg::hir_with_context(cx, target, ctxt, "_", &mut applicability).deref(), + ArgKind::RefMutToPlaceAsMacro(arg, derefs) => (0..*derefs).fold( + Sugg::hir_with_context(cx, arg, ctxt, "_", &mut applicability).deref(), + |sugg, _| sugg.deref(), + ), + ArgKind::RefMutToPlace(target, derefs) => (0..*derefs).fold( + Sugg::hir_with_context(cx, target, ctxt, "_", &mut applicability), + |sugg, _| sugg.deref(), + ), ArgKind::RefMutToTemp(_) => unreachable!(), }; let assign_source = Sugg::hir_with_context(cx, temp, ctxt, "_", &mut applicability); diff --git a/clippy_lints/src/std_instead_of_core.rs b/clippy_lints/src/std_instead_of_core.rs index 442b3250d86..cf70e883bd0 100644 --- a/clippy_lints/src/std_instead_of_core.rs +++ b/clippy_lints/src/std_instead_of_core.rs @@ -1,10 +1,10 @@ use clippy_config::Conf; -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; use clippy_utils::is_from_proc_macro; use clippy_utils::msrvs::Msrv; use rustc_attr_data_structures::{StabilityLevel, StableSince}; use rustc_errors::Applicability; -use rustc_hir::def::Res; +use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::DefId; use rustc_hir::{Block, Body, HirId, Path, PathSegment}; use rustc_lint::{LateContext, LateLintPass, Lint, LintContext}; @@ -88,49 +88,52 @@ declare_clippy_lint! { } pub struct StdReexports { - lint_point: (Span, Option<LintPoint>), + lint_points: Option<(Span, Vec<LintPoint>)>, msrv: Msrv, } impl StdReexports { pub fn new(conf: &'static Conf) -> Self { Self { - lint_point: Default::default(), + lint_points: Option::default(), msrv: conf.msrv, } } - fn lint_if_finish(&mut self, cx: &LateContext<'_>, (span, item): (Span, Option<LintPoint>)) { - if span.source_equal(self.lint_point.0) { - return; + fn lint_if_finish(&mut self, cx: &LateContext<'_>, krate: Span, lint_point: LintPoint) { + match &mut self.lint_points { + Some((prev_krate, prev_lints)) if prev_krate.overlaps(krate) => { + prev_lints.push(lint_point); + }, + _ => emit_lints(cx, self.lint_points.replace((krate, vec![lint_point]))), } - - if !self.lint_point.0.is_dummy() { - emit_lints(cx, &self.lint_point); - } - - self.lint_point = (span, item); } } impl_lint_pass!(StdReexports => [STD_INSTEAD_OF_CORE, STD_INSTEAD_OF_ALLOC, ALLOC_INSTEAD_OF_CORE]); -type LintPoint = (&'static Lint, &'static str, &'static str); +#[derive(Debug)] +enum LintPoint { + Available(Span, &'static Lint, &'static str, &'static str), + Conflict, +} impl<'tcx> LateLintPass<'tcx> for StdReexports { fn check_path(&mut self, cx: &LateContext<'tcx>, path: &Path<'tcx>, _: HirId) { - if let Res::Def(_, def_id) = path.res + if let Res::Def(def_kind, def_id) = path.res && let Some(first_segment) = get_first_segment(path) && is_stable(cx, def_id, self.msrv) && !path.span.in_external_macro(cx.sess().source_map()) && !is_from_proc_macro(cx, &first_segment.ident) + && !matches!(def_kind, DefKind::Macro(_)) + && let Some(last_segment) = path.segments.last() { let (lint, used_mod, replace_with) = match first_segment.ident.name { sym::std => match cx.tcx.crate_name(def_id.krate) { sym::core => (STD_INSTEAD_OF_CORE, "std", "core"), sym::alloc => (STD_INSTEAD_OF_ALLOC, "std", "alloc"), _ => { - self.lint_if_finish(cx, (first_segment.ident.span, None)); + self.lint_if_finish(cx, first_segment.ident.span, LintPoint::Conflict); return; }, }, @@ -138,44 +141,84 @@ impl<'tcx> LateLintPass<'tcx> for StdReexports { if cx.tcx.crate_name(def_id.krate) == sym::core { (ALLOC_INSTEAD_OF_CORE, "alloc", "core") } else { - self.lint_if_finish(cx, (first_segment.ident.span, None)); + self.lint_if_finish(cx, first_segment.ident.span, LintPoint::Conflict); return; } }, - _ => return, + _ => { + self.lint_if_finish(cx, first_segment.ident.span, LintPoint::Conflict); + return; + }, }; - self.lint_if_finish(cx, (first_segment.ident.span, Some((lint, used_mod, replace_with)))); + self.lint_if_finish( + cx, + first_segment.ident.span, + LintPoint::Available(last_segment.ident.span, lint, used_mod, replace_with), + ); } } fn check_block_post(&mut self, cx: &LateContext<'tcx>, _: &Block<'tcx>) { - self.lint_if_finish(cx, Default::default()); + emit_lints(cx, self.lint_points.take()); } fn check_body_post(&mut self, cx: &LateContext<'tcx>, _: &Body<'tcx>) { - self.lint_if_finish(cx, Default::default()); + emit_lints(cx, self.lint_points.take()); } fn check_crate_post(&mut self, cx: &LateContext<'tcx>) { - self.lint_if_finish(cx, Default::default()); + emit_lints(cx, self.lint_points.take()); } } -fn emit_lints(cx: &LateContext<'_>, (span, item): &(Span, Option<LintPoint>)) { - let Some((lint, used_mod, replace_with)) = item else { +fn emit_lints(cx: &LateContext<'_>, lint_points: Option<(Span, Vec<LintPoint>)>) { + let Some((krate_span, lint_points)) = lint_points else { return; }; - span_lint_and_sugg( - cx, - lint, - *span, - format!("used import from `{used_mod}` instead of `{replace_with}`"), - format!("consider importing the item from `{replace_with}`"), - (*replace_with).to_string(), - Applicability::MachineApplicable, - ); + let mut lint: Option<(&'static Lint, &'static str, &'static str)> = None; + let mut has_conflict = false; + for lint_point in &lint_points { + match lint_point { + LintPoint::Available(_, l, used_mod, replace_with) + if lint.is_none_or(|(prev_l, ..)| l.name == prev_l.name) => + { + lint = Some((l, used_mod, replace_with)); + }, + _ => { + has_conflict = true; + break; + }, + } + } + + if !has_conflict && let Some((lint, used_mod, replace_with)) = lint { + span_lint_and_sugg( + cx, + lint, + krate_span, + format!("used import from `{used_mod}` instead of `{replace_with}`"), + format!("consider importing the item from `{replace_with}`"), + (*replace_with).to_string(), + Applicability::MachineApplicable, + ); + return; + } + + for lint_point in lint_points { + let LintPoint::Available(span, lint, used_mod, replace_with) = lint_point else { + continue; + }; + span_lint_and_help( + cx, + lint, + span, + format!("used import from `{used_mod}` instead of `{replace_with}`"), + None, + format!("consider importing the item from `{replace_with}`"), + ); + } } /// Returns the first named segment of a [`Path`]. diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index 92427473a8e..d4f8b1b4e30 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -143,7 +143,8 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks { if let Some(tail) = block.expr && !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, tail.hir_id) && !tail.span.in_external_macro(cx.tcx.sess.source_map()) - && let HasSafetyComment::Yes(pos) = stmt_has_safety_comment(cx, tail.span, tail.hir_id) + && let HasSafetyComment::Yes(pos) = + stmt_has_safety_comment(cx, tail.span, tail.hir_id, self.accept_comment_above_attributes) && let Some(help_span) = expr_has_unnecessary_safety_comment(cx, tail, pos) { span_lint_and_then( @@ -167,7 +168,8 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks { }; if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, stmt.hir_id) && !stmt.span.in_external_macro(cx.tcx.sess.source_map()) - && let HasSafetyComment::Yes(pos) = stmt_has_safety_comment(cx, stmt.span, stmt.hir_id) + && let HasSafetyComment::Yes(pos) = + stmt_has_safety_comment(cx, stmt.span, stmt.hir_id, self.accept_comment_above_attributes) && let Some(help_span) = expr_has_unnecessary_safety_comment(cx, expr, pos) { span_lint_and_then( @@ -534,7 +536,12 @@ fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> HasSaf /// Checks if the lines immediately preceding the item contain a safety comment. #[allow(clippy::collapsible_match)] -fn stmt_has_safety_comment(cx: &LateContext<'_>, span: Span, hir_id: HirId) -> HasSafetyComment { +fn stmt_has_safety_comment( + cx: &LateContext<'_>, + span: Span, + hir_id: HirId, + accept_comment_above_attributes: bool, +) -> HasSafetyComment { match span_from_macro_expansion_has_safety_comment(cx, span) { HasSafetyComment::Maybe => (), has_safety_comment => return has_safety_comment, @@ -549,6 +556,13 @@ fn stmt_has_safety_comment(cx: &LateContext<'_>, span: Span, hir_id: HirId) -> H _ => return HasSafetyComment::Maybe, }; + // if span_with_attrs_has_safety_comment(cx, span, hir_id, accept_comment_above_attrib + // } + let mut span = span; + if accept_comment_above_attributes { + span = include_attrs_in_span(cx, hir_id, span); + } + let source_map = cx.sess().source_map(); if let Some(comment_start) = comment_start && let Ok(unsafe_line) = source_map.lookup_line(span.lo()) diff --git a/clippy_test_deps/Cargo.toml b/clippy_test_deps/Cargo.toml new file mode 100644 index 00000000000..f41334f0ade --- /dev/null +++ b/clippy_test_deps/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "clippy_test_deps" +version = "0.1.0" +edition = "2021" + +# Add dependencies here to make them available in ui tests. + +[dependencies] +regex = "1.5.5" +serde = { version = "1.0.145", features = ["derive"] } +if_chain = "1.0" +quote = "1.0.25" +syn = { version = "2.0", features = ["full"] } +futures = "0.3" +parking_lot = "0.12" +tokio = { version = "1", features = ["io-util"] } +itertools = "0.12" diff --git a/clippy_test_deps/src/main.rs b/clippy_test_deps/src/main.rs new file mode 100644 index 00000000000..f328e4d9d04 --- /dev/null +++ b/clippy_test_deps/src/main.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/tests/compile-test.rs b/tests/compile-test.rs index cefe654fef6..aa786334711 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -16,8 +16,10 @@ use test_utils::IS_RUSTC_TEST_SUITE; use ui_test::custom_flags::Flag; use ui_test::custom_flags::edition::Edition; use ui_test::custom_flags::rustfix::RustfixMode; +use ui_test::dependencies::DependencyBuilder; use ui_test::spanned::Spanned; -use ui_test::{Args, CommandBuilder, Config, Match, error_on_output_conflict, status_emitter}; +use ui_test::status_emitter::StatusEmitter; +use ui_test::{Args, CommandBuilder, Config, Match, error_on_output_conflict}; use std::collections::{BTreeMap, HashMap}; use std::env::{self, set_var, var_os}; @@ -27,46 +29,26 @@ use std::path::{Path, PathBuf}; use std::sync::mpsc::{Sender, channel}; use std::{fs, iter, thread}; -// Test dependencies may need an `extern crate` here to ensure that they show up -// in the depinfo file (otherwise cargo thinks they are unused) -extern crate futures; -extern crate if_chain; -extern crate itertools; -extern crate parking_lot; -extern crate quote; -extern crate syn; -extern crate tokio; - mod test_utils; -/// All crates used in UI tests are listed here -static TEST_DEPENDENCIES: &[&str] = &[ - "clippy_config", - "clippy_lints", - "clippy_utils", - "futures", - "if_chain", - "itertools", - "parking_lot", - "quote", - "regex", - "serde_derive", - "serde", - "syn", - "tokio", -]; - -/// Produces a string with an `--extern` flag for all UI test crate -/// dependencies. +/// All crates used in internal UI tests are listed here. +/// We directly re-use these crates from their normal clippy builds, so we don't have them +/// in `clippy_test_devs`. That saves a lot of time but also means they don't work in a stage 1 +/// test in rustc bootstrap. +static INTERNAL_TEST_DEPENDENCIES: &[&str] = &["clippy_config", "clippy_lints", "clippy_utils"]; + +/// Produces a string with an `--extern` flag for all `INTERNAL_TEST_DEPENDENCIES`. /// /// The dependency files are located by parsing the depinfo file for this test /// module. This assumes the `-Z binary-dep-depinfo` flag is enabled. All test /// dependencies must be added to Cargo.toml at the project root. Test /// dependencies that are not *directly* used by this test module require an /// `extern crate` declaration. -fn extern_flags() -> Vec<String> { +fn internal_extern_flags() -> Vec<String> { + let current_exe_path = env::current_exe().unwrap(); + let deps_path = current_exe_path.parent().unwrap(); let current_exe_depinfo = { - let mut path = env::current_exe().unwrap(); + let mut path = current_exe_path.clone(); path.set_extension("d"); fs::read_to_string(path).unwrap() }; @@ -88,7 +70,7 @@ fn extern_flags() -> Vec<String> { Some((name, path_str)) }; if let Some((name, path)) = parse_name_path() - && TEST_DEPENDENCIES.contains(&name) + && INTERNAL_TEST_DEPENDENCIES.contains(&name) { // A dependency may be listed twice if it is available in sysroot, // and the sysroot dependencies are listed first. As of the writing, @@ -96,7 +78,7 @@ fn extern_flags() -> Vec<String> { crates.insert(name, path); } } - let not_found: Vec<&str> = TEST_DEPENDENCIES + let not_found: Vec<&str> = INTERNAL_TEST_DEPENDENCIES .iter() .copied() .filter(|n| !crates.contains_key(n)) @@ -111,6 +93,7 @@ fn extern_flags() -> Vec<String> { crates .into_iter() .map(|(name, path)| format!("--extern={name}={path}")) + .chain([format!("-Ldependency={}", deps_path.display())]) .collect() } @@ -119,7 +102,6 @@ const RUN_INTERNAL_TESTS: bool = cfg!(feature = "internal"); struct TestContext { args: Args, - extern_flags: Vec<String>, diagnostic_collector: Option<DiagnosticCollector>, collector_thread: Option<thread::JoinHandle<()>>, } @@ -134,7 +116,6 @@ impl TestContext { .unzip(); Self { args, - extern_flags: extern_flags(), diagnostic_collector, collector_thread, } @@ -158,6 +139,15 @@ impl TestContext { }; let defaults = config.comment_defaults.base(); defaults.set_custom("edition", Edition("2024".into())); + defaults.set_custom( + "dependencies", + DependencyBuilder { + program: CommandBuilder::cargo(), + crate_manifest_path: Path::new("clippy_test_deps").join("Cargo.toml"), + build_std: None, + bless_lockfile: self.args.bless, + }, + ); defaults.exit_status = None.into(); if mandatory_annotations { defaults.require_annotations = Some(Spanned::dummy(true)).into(); @@ -182,12 +172,10 @@ impl TestContext { "-Zui-testing", "-Zdeduplicate-diagnostics=no", "-Dwarnings", - &format!("-Ldependency={}", deps_path.display()), ] .map(OsString::from), ); - config.program.args.extend(self.extern_flags.iter().map(OsString::from)); // Prevent rustc from creating `rustc-ice-*` files the console output is enough. config.program.envs.push(("RUSTC_ICE".into(), Some("0".into()))); @@ -217,7 +205,7 @@ fn run_ui(cx: &TestContext) { vec![config], ui_test::default_file_filter, ui_test::default_per_file_config, - status_emitter::Text::from(cx.args.format), + Box::<dyn StatusEmitter>::from(cx.args.format), ) .unwrap(); } @@ -227,13 +215,17 @@ fn run_internal_tests(cx: &TestContext) { return; } let mut config = cx.base_config("ui-internal", true); + config + .program + .args + .extend(internal_extern_flags().iter().map(OsString::from)); config.bless_command = Some("cargo uitest --features internal -- -- --bless".into()); ui_test::run_tests_generic( vec![config], ui_test::default_file_filter, ui_test::default_per_file_config, - status_emitter::Text::from(cx.args.format), + Box::<dyn StatusEmitter>::from(cx.args.format), ) .unwrap(); } @@ -257,7 +249,7 @@ fn run_ui_toml(cx: &TestContext) { .envs .push(("CLIPPY_CONF_DIR".into(), Some(path.parent().unwrap().into()))); }, - status_emitter::Text::from(cx.args.format), + Box::<dyn StatusEmitter>::from(cx.args.format), ) .unwrap(); } @@ -304,7 +296,7 @@ fn run_ui_cargo(cx: &TestContext) { .then(|| ui_test::default_any_file_filter(path, config) && !ignored_32bit(path)) }, |_config, _file_contents| {}, - status_emitter::Text::from(cx.args.format), + Box::<dyn StatusEmitter>::from(cx.args.format), ) .unwrap(); } diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr index e9c5e5f9f11..453bcf1ab33 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr @@ -450,5 +450,13 @@ help: consider removing the safety comment LL | // SAFETY: unnecessary_safety_comment triggers here | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 52 previous errors +error: unsafe block missing a safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:733:12 + | +LL | return unsafe { h() }; + | ^^^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: aborting due to 53 previous errors diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs index 91a02bc3d7c..a2d7c1b6c79 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs @@ -723,4 +723,15 @@ fn issue_13039() { _ = unsafe { foo() } } +fn rfl_issue15034() -> i32 { + unsafe fn h() -> i32 { + 1i32 + } + // This shouldn't lint with accept-comment-above-attributes! Thus fixing a false positive! + // SAFETY: My safety comment! + #[allow(clippy::unnecessary_cast)] + return unsafe { h() }; + //~[disabled]^ ERROR: unsafe block missing a safety comment +} + fn main() {} diff --git a/tests/ui/coerce_container_to_any.fixed b/tests/ui/coerce_container_to_any.fixed index ae9d3ef9656..b5b3f15b4de 100644 --- a/tests/ui/coerce_container_to_any.fixed +++ b/tests/ui/coerce_container_to_any.fixed @@ -3,7 +3,7 @@ use std::any::Any; fn main() { - let x: Box<dyn Any> = Box::new(()); + let mut x: Box<dyn Any> = Box::new(()); let ref_x = &x; f(&*x); @@ -15,12 +15,23 @@ fn main() { let _: &dyn Any = &*x; //~^ coerce_container_to_any + let _: &dyn Any = &*x; + //~^ coerce_container_to_any + + let _: &mut dyn Any = &mut *x; + //~^ coerce_container_to_any + f(&42); f(&Box::new(())); f(&Box::new(Box::new(()))); + let ref_x = &x; f(&**ref_x); f(&*x); let _: &dyn Any = &*x; + + // https://github.com/rust-lang/rust-clippy/issues/15045 + #[allow(clippy::needless_borrow)] + (&x).downcast_ref::<()>().unwrap(); } fn f(_: &dyn Any) {} diff --git a/tests/ui/coerce_container_to_any.rs b/tests/ui/coerce_container_to_any.rs index 9948bd48e0d..4d6527bb552 100644 --- a/tests/ui/coerce_container_to_any.rs +++ b/tests/ui/coerce_container_to_any.rs @@ -3,7 +3,7 @@ use std::any::Any; fn main() { - let x: Box<dyn Any> = Box::new(()); + let mut x: Box<dyn Any> = Box::new(()); let ref_x = &x; f(&x); @@ -15,12 +15,23 @@ fn main() { let _: &dyn Any = &x; //~^ coerce_container_to_any + let _: &dyn Any = &mut x; + //~^ coerce_container_to_any + + let _: &mut dyn Any = &mut x; + //~^ coerce_container_to_any + f(&42); f(&Box::new(())); f(&Box::new(Box::new(()))); + let ref_x = &x; f(&**ref_x); f(&*x); let _: &dyn Any = &*x; + + // https://github.com/rust-lang/rust-clippy/issues/15045 + #[allow(clippy::needless_borrow)] + (&x).downcast_ref::<()>().unwrap(); } fn f(_: &dyn Any) {} diff --git a/tests/ui/coerce_container_to_any.stderr b/tests/ui/coerce_container_to_any.stderr index 00ab77e0ce0..26389c9186e 100644 --- a/tests/ui/coerce_container_to_any.stderr +++ b/tests/ui/coerce_container_to_any.stderr @@ -19,5 +19,17 @@ error: coercing `&std::boxed::Box<dyn std::any::Any>` to `&dyn Any` LL | let _: &dyn Any = &x; | ^^ help: consider dereferencing: `&*x` -error: aborting due to 3 previous errors +error: coercing `&mut std::boxed::Box<dyn std::any::Any>` to `&dyn Any` + --> tests/ui/coerce_container_to_any.rs:18:23 + | +LL | let _: &dyn Any = &mut x; + | ^^^^^^ help: consider dereferencing: `&*x` + +error: coercing `&mut std::boxed::Box<dyn std::any::Any>` to `&mut dyn Any` + --> tests/ui/coerce_container_to_any.rs:21:27 + | +LL | let _: &mut dyn Any = &mut x; + | ^^^^^^ help: consider dereferencing: `&mut *x` + +error: aborting due to 5 previous errors diff --git a/tests/ui/doc/doc_nested_refdef_list_item.fixed b/tests/ui/doc/doc_nested_refdef_list_item.fixed index 065f4486e39..5c57c58fbc0 100644 --- a/tests/ui/doc/doc_nested_refdef_list_item.fixed +++ b/tests/ui/doc/doc_nested_refdef_list_item.fixed @@ -72,8 +72,6 @@ pub struct NotEmptyTight; /// ## Heading /// -/// - [x][] - Done -//~^ ERROR: link reference defined in list item -/// - [ ][] - Not Done -//~^ ERROR: link reference defined in list item +/// - [x] - Done +/// - [ ] - Not Done pub struct GithubCheckboxes; diff --git a/tests/ui/doc/doc_nested_refdef_list_item.rs b/tests/ui/doc/doc_nested_refdef_list_item.rs index c7eab50c8b3..06b6ba49e19 100644 --- a/tests/ui/doc/doc_nested_refdef_list_item.rs +++ b/tests/ui/doc/doc_nested_refdef_list_item.rs @@ -73,7 +73,5 @@ pub struct NotEmptyTight; /// ## Heading /// /// - [x] - Done -//~^ ERROR: link reference defined in list item /// - [ ] - Not Done -//~^ ERROR: link reference defined in list item pub struct GithubCheckboxes; diff --git a/tests/ui/doc/doc_nested_refdef_list_item.stderr b/tests/ui/doc/doc_nested_refdef_list_item.stderr index 5a815dabf4d..27314c7e968 100644 --- a/tests/ui/doc/doc_nested_refdef_list_item.stderr +++ b/tests/ui/doc/doc_nested_refdef_list_item.stderr @@ -144,29 +144,5 @@ help: for an intra-doc link, add `[]` between the label and the colon LL | /// - [link][]: def "title" | ++ -error: link reference defined in list item - --> tests/ui/doc/doc_nested_refdef_list_item.rs:75:7 - | -LL | /// - [x] - Done - | ^^^ - | - = help: link definitions are not shown in rendered documentation -help: for an intra-doc link, add `[]` between the label and the colon - | -LL | /// - [x][] - Done - | ++ - -error: link reference defined in list item - --> tests/ui/doc/doc_nested_refdef_list_item.rs:77:7 - | -LL | /// - [ ] - Not Done - | ^^^ - | - = help: link definitions are not shown in rendered documentation -help: for an intra-doc link, add `[]` between the label and the colon - | -LL | /// - [ ][] - Not Done - | ++ - -error: aborting due to 14 previous errors +error: aborting due to 12 previous errors diff --git a/tests/ui/empty_loop_intrinsic.rs b/tests/ui/empty_loop_intrinsic.rs new file mode 100644 index 00000000000..a550e560965 --- /dev/null +++ b/tests/ui/empty_loop_intrinsic.rs @@ -0,0 +1,13 @@ +//@check-pass + +#![warn(clippy::empty_loop)] +#![feature(intrinsics)] +#![feature(rustc_attrs)] + +// From issue #15200 +#[rustc_intrinsic] +#[rustc_nounwind] +/// # Safety +pub const unsafe fn simd_insert<T, U>(x: T, idx: u32, val: U) -> T; + +fn main() {} diff --git a/tests/ui/std_instead_of_core.fixed b/tests/ui/std_instead_of_core.fixed index 1820ade422f..603ab0accb0 100644 --- a/tests/ui/std_instead_of_core.fixed +++ b/tests/ui/std_instead_of_core.fixed @@ -8,7 +8,6 @@ extern crate alloc; #[macro_use] extern crate proc_macro_derive; -#[warn(clippy::std_instead_of_core)] fn std_instead_of_core() { // Regular import use core::hash::Hasher; @@ -90,9 +89,3 @@ fn msrv_1_76(_: std::net::IpAddr) {} #[clippy::msrv = "1.77"] fn msrv_1_77(_: core::net::IpAddr) {} //~^ std_instead_of_core - -#[warn(clippy::std_instead_of_core)] -#[rustfmt::skip] -fn issue14982() { - use std::{collections::HashMap, hash::Hash}; -} diff --git a/tests/ui/std_instead_of_core.rs b/tests/ui/std_instead_of_core.rs index 32c49330981..b6d4abad9f8 100644 --- a/tests/ui/std_instead_of_core.rs +++ b/tests/ui/std_instead_of_core.rs @@ -8,7 +8,6 @@ extern crate alloc; #[macro_use] extern crate proc_macro_derive; -#[warn(clippy::std_instead_of_core)] fn std_instead_of_core() { // Regular import use std::hash::Hasher; @@ -90,9 +89,3 @@ fn msrv_1_76(_: std::net::IpAddr) {} #[clippy::msrv = "1.77"] fn msrv_1_77(_: std::net::IpAddr) {} //~^ std_instead_of_core - -#[warn(clippy::std_instead_of_core)] -#[rustfmt::skip] -fn issue14982() { - use std::{collections::HashMap, hash::Hash}; -} diff --git a/tests/ui/std_instead_of_core.stderr b/tests/ui/std_instead_of_core.stderr index 45d60d235ce..a5f8fbbe37c 100644 --- a/tests/ui/std_instead_of_core.stderr +++ b/tests/ui/std_instead_of_core.stderr @@ -1,5 +1,5 @@ error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:14:9 + --> tests/ui/std_instead_of_core.rs:13:9 | LL | use std::hash::Hasher; | ^^^ help: consider importing the item from `core`: `core` @@ -8,61 +8,61 @@ LL | use std::hash::Hasher; = help: to override `-D warnings` add `#[allow(clippy::std_instead_of_core)]` error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:17:11 + --> tests/ui/std_instead_of_core.rs:16:11 | LL | use ::std::hash::Hash; | ^^^ help: consider importing the item from `core`: `core` error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:23:9 + --> tests/ui/std_instead_of_core.rs:22:9 | LL | use std::fmt::{Debug, Result}; | ^^^ help: consider importing the item from `core`: `core` error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:28:9 + --> tests/ui/std_instead_of_core.rs:27:9 | LL | use std::{ | ^^^ help: consider importing the item from `core`: `core` error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:35:15 + --> tests/ui/std_instead_of_core.rs:34:15 | LL | let ptr = std::ptr::null::<u32>(); | ^^^ help: consider importing the item from `core`: `core` error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:37:21 + --> tests/ui/std_instead_of_core.rs:36:21 | LL | let ptr_mut = ::std::ptr::null_mut::<usize>(); | ^^^ help: consider importing the item from `core`: `core` error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:41:16 + --> tests/ui/std_instead_of_core.rs:40:16 | LL | let cell = std::cell::Cell::new(8u32); | ^^^ help: consider importing the item from `core`: `core` error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:43:27 + --> tests/ui/std_instead_of_core.rs:42:27 | LL | let cell_absolute = ::std::cell::Cell::new(8u32); | ^^^ help: consider importing the item from `core`: `core` error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:48:9 + --> tests/ui/std_instead_of_core.rs:47:9 | LL | use std::error::Error; | ^^^ help: consider importing the item from `core`: `core` error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:52:9 + --> tests/ui/std_instead_of_core.rs:51:9 | LL | use std::iter::Iterator; | ^^^ help: consider importing the item from `core`: `core` error: used import from `std` instead of `alloc` - --> tests/ui/std_instead_of_core.rs:59:9 + --> tests/ui/std_instead_of_core.rs:58:9 | LL | use std::vec; | ^^^ help: consider importing the item from `alloc`: `alloc` @@ -71,13 +71,13 @@ LL | use std::vec; = help: to override `-D warnings` add `#[allow(clippy::std_instead_of_alloc)]` error: used import from `std` instead of `alloc` - --> tests/ui/std_instead_of_core.rs:61:9 + --> tests/ui/std_instead_of_core.rs:60:9 | LL | use std::vec::Vec; | ^^^ help: consider importing the item from `alloc`: `alloc` error: used import from `alloc` instead of `core` - --> tests/ui/std_instead_of_core.rs:67:9 + --> tests/ui/std_instead_of_core.rs:66:9 | LL | use alloc::slice::from_ref; | ^^^^^ help: consider importing the item from `core`: `core` @@ -86,13 +86,13 @@ LL | use alloc::slice::from_ref; = help: to override `-D warnings` add `#[allow(clippy::alloc_instead_of_core)]` error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:82:9 + --> tests/ui/std_instead_of_core.rs:81:9 | LL | std::intrinsics::copy(a, b, 1); | ^^^ help: consider importing the item from `core`: `core` error: used import from `std` instead of `core` - --> tests/ui/std_instead_of_core.rs:91:17 + --> tests/ui/std_instead_of_core.rs:90:17 | LL | fn msrv_1_77(_: std::net::IpAddr) {} | ^^^ help: consider importing the item from `core`: `core` diff --git a/tests/ui/std_instead_of_core_unfixable.rs b/tests/ui/std_instead_of_core_unfixable.rs new file mode 100644 index 00000000000..957f472a454 --- /dev/null +++ b/tests/ui/std_instead_of_core_unfixable.rs @@ -0,0 +1,18 @@ +//@no-rustfix + +#![warn(clippy::std_instead_of_core)] +#![warn(clippy::std_instead_of_alloc)] +#![allow(unused_imports)] + +#[rustfmt::skip] +fn issue14982() { + use std::{collections::HashMap, hash::Hash}; + //~^ std_instead_of_core +} + +#[rustfmt::skip] +fn issue15143() { + use std::{error::Error, vec::Vec, fs::File}; + //~^ std_instead_of_core + //~| std_instead_of_alloc +} diff --git a/tests/ui/std_instead_of_core_unfixable.stderr b/tests/ui/std_instead_of_core_unfixable.stderr new file mode 100644 index 00000000000..0cdec56c992 --- /dev/null +++ b/tests/ui/std_instead_of_core_unfixable.stderr @@ -0,0 +1,30 @@ +error: used import from `std` instead of `core` + --> tests/ui/std_instead_of_core_unfixable.rs:9:43 + | +LL | use std::{collections::HashMap, hash::Hash}; + | ^^^^ + | + = help: consider importing the item from `core` + = note: `-D clippy::std-instead-of-core` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::std_instead_of_core)]` + +error: used import from `std` instead of `core` + --> tests/ui/std_instead_of_core_unfixable.rs:15:22 + | +LL | use std::{error::Error, vec::Vec, fs::File}; + | ^^^^^ + | + = help: consider importing the item from `core` + +error: used import from `std` instead of `alloc` + --> tests/ui/std_instead_of_core_unfixable.rs:15:34 + | +LL | use std::{error::Error, vec::Vec, fs::File}; + | ^^^ + | + = help: consider importing the item from `alloc` + = note: `-D clippy::std-instead-of-alloc` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::std_instead_of_alloc)]` + +error: aborting due to 3 previous errors + diff --git a/tests/ui/swap_with_temporary.fixed b/tests/ui/swap_with_temporary.fixed index 4007d998ba0..4b4b0d4aebd 100644 --- a/tests/ui/swap_with_temporary.fixed +++ b/tests/ui/swap_with_temporary.fixed @@ -72,3 +72,49 @@ fn dont_lint_those(s: &mut S, v: &mut [String], w: Option<&mut String>) { swap(&mut s.t, v.get_mut(0).unwrap()); swap(w.unwrap(), &mut s.t); } + +fn issue15166() { + use std::sync::Mutex; + + struct A { + thing: Mutex<Vec<u8>>, + } + + impl A { + fn a(&self) { + let mut new_vec = vec![42]; + // Do not lint here, as neither `new_vec` nor the result of `.lock().unwrap()` are temporaries + swap(&mut new_vec, &mut self.thing.lock().unwrap()); + for v in new_vec { + // Do something with v + } + // Here `vec![42]` is temporary though, and a proper dereference will have to be used in the fix + *self.thing.lock().unwrap() = vec![42]; + //~^ ERROR: swapping with a temporary value is inefficient + } + } +} + +fn multiple_deref() { + let mut v1 = &mut &mut &mut vec![42]; + ***v1 = vec![]; + //~^ ERROR: swapping with a temporary value is inefficient + + struct Wrapper<T: ?Sized>(T); + impl<T: ?Sized> std::ops::Deref for Wrapper<T> { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + impl<T: ?Sized> std::ops::DerefMut for Wrapper<T> { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + + use std::sync::Mutex; + let mut v1 = Mutex::new(Wrapper(Wrapper(vec![42]))); + ***v1.lock().unwrap() = vec![]; + //~^ ERROR: swapping with a temporary value is inefficient +} diff --git a/tests/ui/swap_with_temporary.rs b/tests/ui/swap_with_temporary.rs index d403c086c0f..8e35e6144d9 100644 --- a/tests/ui/swap_with_temporary.rs +++ b/tests/ui/swap_with_temporary.rs @@ -72,3 +72,49 @@ fn dont_lint_those(s: &mut S, v: &mut [String], w: Option<&mut String>) { swap(&mut s.t, v.get_mut(0).unwrap()); swap(w.unwrap(), &mut s.t); } + +fn issue15166() { + use std::sync::Mutex; + + struct A { + thing: Mutex<Vec<u8>>, + } + + impl A { + fn a(&self) { + let mut new_vec = vec![42]; + // Do not lint here, as neither `new_vec` nor the result of `.lock().unwrap()` are temporaries + swap(&mut new_vec, &mut self.thing.lock().unwrap()); + for v in new_vec { + // Do something with v + } + // Here `vec![42]` is temporary though, and a proper dereference will have to be used in the fix + swap(&mut vec![42], &mut self.thing.lock().unwrap()); + //~^ ERROR: swapping with a temporary value is inefficient + } + } +} + +fn multiple_deref() { + let mut v1 = &mut &mut &mut vec![42]; + swap(&mut ***v1, &mut vec![]); + //~^ ERROR: swapping with a temporary value is inefficient + + struct Wrapper<T: ?Sized>(T); + impl<T: ?Sized> std::ops::Deref for Wrapper<T> { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + impl<T: ?Sized> std::ops::DerefMut for Wrapper<T> { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + + use std::sync::Mutex; + let mut v1 = Mutex::new(Wrapper(Wrapper(vec![42]))); + swap(&mut vec![], &mut v1.lock().unwrap()); + //~^ ERROR: swapping with a temporary value is inefficient +} diff --git a/tests/ui/swap_with_temporary.stderr b/tests/ui/swap_with_temporary.stderr index 59355771a96..5ca4fccd37a 100644 --- a/tests/ui/swap_with_temporary.stderr +++ b/tests/ui/swap_with_temporary.stderr @@ -96,5 +96,41 @@ note: this expression returns a temporary value LL | swap(mac!(refmut y), &mut func()); | ^^^^^^ -error: aborting due to 8 previous errors +error: swapping with a temporary value is inefficient + --> tests/ui/swap_with_temporary.rs:92:13 + | +LL | swap(&mut vec![42], &mut self.thing.lock().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use assignment instead: `*self.thing.lock().unwrap() = vec![42]` + | +note: this expression returns a temporary value + --> tests/ui/swap_with_temporary.rs:92:23 + | +LL | swap(&mut vec![42], &mut self.thing.lock().unwrap()); + | ^^^^^^^^ + +error: swapping with a temporary value is inefficient + --> tests/ui/swap_with_temporary.rs:100:5 + | +LL | swap(&mut ***v1, &mut vec![]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use assignment instead: `***v1 = vec![]` + | +note: this expression returns a temporary value + --> tests/ui/swap_with_temporary.rs:100:27 + | +LL | swap(&mut ***v1, &mut vec![]); + | ^^^^^^ + +error: swapping with a temporary value is inefficient + --> tests/ui/swap_with_temporary.rs:118:5 + | +LL | swap(&mut vec![], &mut v1.lock().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use assignment instead: `***v1.lock().unwrap() = vec![]` + | +note: this expression returns a temporary value + --> tests/ui/swap_with_temporary.rs:118:15 + | +LL | swap(&mut vec![], &mut v1.lock().unwrap()); + | ^^^^^^ + +error: aborting due to 11 previous errors diff --git a/triagebot.toml b/triagebot.toml index 4f370758c00..81718413c38 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -43,12 +43,15 @@ reviewed_label = "S-waiting-on-author" [autolabel."S-waiting-on-review"] new_pr = true +[concern] +# These labels are set when there are unresolved concerns, removed otherwise +labels = ["S-waiting-on-concerns"] + [assign] contributing_url = "https://github.com/rust-lang/rust-clippy/blob/master/CONTRIBUTING.md" users_on_vacation = [ "matthiaskrgr", "Manishearth", - "blyxyas", ] [assign.owners] diff --git a/util/gh-pages/index_template.html b/util/gh-pages/index_template.html index 865b9523c39..6f380ec8fee 100644 --- a/util/gh-pages/index_template.html +++ b/util/gh-pages/index_template.html @@ -149,49 +149,45 @@ Otherwise, have a great day =^.^= <article class="panel panel-default" id="{{lint.id}}"> {# #} <input id="label-{{lint.id}}" type="checkbox"> {# #} <label for="label-{{lint.id}}"> {# #} - <header class="panel-heading"> {# #} - <h2 class="panel-title"> {# #} - <div class="panel-title-name" id="lint-{{lint.id}}"> {# #} - <span>{{lint.id}}</span> {#+ #} - <a href="#{{lint.id}}" class="lint-anchor anchor label label-default">¶</a> {#+ #} - <a href="" class="copy-to-clipboard anchor label label-default"> {# #} - 📋 {# #} - </a> {# #} - </div> {# #} + <h2 class="lint-title"> {# #} + <div class="panel-title-name" id="lint-{{lint.id}}"> {# #} + {{lint.id +}} + <a href="#{{lint.id}}" class="anchor label label-default">¶</a> {#+ #} + <a href="" class="copy-to-clipboard anchor label label-default"> {# #} + 📋 {# #} + </a> {# #} + </div> {# #} - <div class="panel-title-addons"> {# #} - <span class="label label-lint-group label-default label-group-{{lint.group}}">{{lint.group}}</span> {#+ #} + <span class="label label-lint-group label-default label-group-{{lint.group}}">{{lint.group}}</span> {#+ #} - <span class="label label-lint-level label-lint-level-{{lint.level}}">{{lint.level}}</span> {#+ #} + <span class="label label-lint-level label-lint-level-{{lint.level}}">{{lint.level}}</span> {#+ #} - <span class="label label-doc-folding"></span> {# #} - </div> {# #} - </h2> {# #} - </header> {# #} + <span class="label label-doc-folding"></span> {# #} + </h2> {# #} </label> {# #} <div class="list-group lint-docs"> {# #} <div class="list-group-item lint-doc-md">{{Self::markdown(lint.docs)}}</div> {# #} <div class="lint-additional-info-container"> {# Applicability #} - <div class="lint-additional-info-item"> {# #} - <span> Applicability: </span> {# #} + <div> {# #} + Applicability: {#+ #} <span class="label label-default label-applicability">{{ lint.applicability_str() }}</span> {# #} <a href="https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint_defs/enum.Applicability.html#variants">(?)</a> {# #} </div> {# Clippy version #} - <div class="lint-additional-info-item"> {# #} - <span>{% if lint.group == "deprecated" %}Deprecated{% else %} Added{% endif +%} in: </span> {# #} + <div> {# #} + {% if lint.group == "deprecated" %}Deprecated{% else %} Added{% endif +%} in: {#+ #} <span class="label label-default label-version">{{lint.version}}</span> {# #} </div> {# Open related issues #} - <div class="lint-additional-info-item"> {# #} + <div> {# #} <a href="https://github.com/rust-lang/rust-clippy/issues?q=is%3Aissue+{{lint.id}}">Related Issues</a> {# #} </div> {# Jump to source #} {% if let Some(id_location) = lint.id_location %} - <div class="lint-additional-info-item"> {# #} + <div> {# #} <a href="https://github.com/rust-lang/rust-clippy/blob/master/{{id_location}}">View Source</a> {# #} </div> {% endif %} diff --git a/util/gh-pages/script.js b/util/gh-pages/script.js index 285aa34e701..ee13f1c0cd8 100644 --- a/util/gh-pages/script.js +++ b/util/gh-pages/script.js @@ -554,10 +554,10 @@ function addListeners() { return; } - if (event.target.classList.contains("lint-anchor")) { - lintAnchor(event); - } else if (event.target.classList.contains("copy-to-clipboard")) { + if (event.target.classList.contains("copy-to-clipboard")) { copyToClipboard(event); + } else if (event.target.classList.contains("anchor")) { + lintAnchor(event); } }); diff --git a/util/gh-pages/style.css b/util/gh-pages/style.css index 3cc7a919c23..022ea875200 100644 --- a/util/gh-pages/style.css +++ b/util/gh-pages/style.css @@ -50,11 +50,25 @@ div.panel div.panel-body button.open { .panel-heading { cursor: pointer; } -.panel-title { display: flex; flex-wrap: wrap;} -.panel-title .label { display: inline-block; } +.lint-title { + cursor: pointer; + margin-top: 0; + margin-bottom: 0; + font-size: 16px; + display: flex; + flex-wrap: wrap; + background: var(--theme-hover); + color: var(--fg); + border: 1px solid var(--theme-popup-border); + padding: 10px 15px; + border-top-left-radius: 3px; + border-top-right-radius: 3px; + gap: 4px; +} + +.lint-title .label { display: inline-block; } .panel-title-name { flex: 1; min-width: 400px;} -.panel-title-name span { vertical-align: bottom; } .panel .panel-title-name .anchor { display: none; } .panel:hover .panel-title-name .anchor { display: inline;} @@ -147,7 +161,7 @@ div.panel div.panel-body button.open { display: flex; flex-flow: column; } - .lint-additional-info-item + .lint-additional-info-item { + .lint-additional-info-container > div + div { border-top: 1px solid var(--theme-popup-border); } } @@ -156,12 +170,12 @@ div.panel div.panel-body button.open { display: flex; flex-flow: row; } - .lint-additional-info-item + .lint-additional-info-item { + .lint-additional-info-container > div + div { border-left: 1px solid var(--theme-popup-border); } } -.lint-additional-info-item { +.lint-additional-info-container > div { display: inline-flex; min-width: 200px; flex-grow: 1; |