diff --git a/CHANGELOG.md b/CHANGELOG.md index c9adf77c0..1b52a6fcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1438,7 +1438,7 @@ Released 2020-11-19 * [`manual_strip`] [#6038](https://github.com/rust-lang/rust-clippy/pull/6038) * [`map_err_ignore`] [#5998](https://github.com/rust-lang/rust-clippy/pull/5998) * [`rc_buffer`] [#6044](https://github.com/rust-lang/rust-clippy/pull/6044) -* [`to_string_in_display`] [#5831](https://github.com/rust-lang/rust-clippy/pull/5831) +* `to_string_in_display` [#5831](https://github.com/rust-lang/rust-clippy/pull/5831) * `single_char_push_str` [#5881](https://github.com/rust-lang/rust-clippy/pull/5881) ### Moves and Deprecations @@ -1481,7 +1481,7 @@ Released 2020-11-19 [#5949](https://github.com/rust-lang/rust-clippy/pull/5949) * [`doc_markdown`]: allow using "GraphQL" without backticks [#5996](https://github.com/rust-lang/rust-clippy/pull/5996) -* [`to_string_in_display`]: avoid linting when calling `to_string()` on anything that is not `self` +* `to_string_in_display`: avoid linting when calling `to_string()` on anything that is not `self` [#5971](https://github.com/rust-lang/rust-clippy/pull/5971) * [`indexing_slicing`] and [`out_of_bounds_indexing`] treat references to arrays as arrays [#6034](https://github.com/rust-lang/rust-clippy/pull/6034) @@ -3068,6 +3068,7 @@ Released 2018-09-13 [`bytes_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#bytes_nth [`cargo_common_metadata`]: https://rust-lang.github.io/rust-clippy/master/index.html#cargo_common_metadata [`case_sensitive_file_extension_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#case_sensitive_file_extension_comparisons +[`cast_enum_truncation`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_enum_truncation [`cast_lossless`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless [`cast_possible_truncation`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation [`cast_possible_wrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_wrap @@ -3105,6 +3106,7 @@ Released 2018-09-13 [`deprecated_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_cfg_attr [`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver [`deref_addrof`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_addrof +[`deref_by_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_by_slicing [`derivable_impls`]: https://rust-lang.github.io/rust-clippy/master/index.html#derivable_impls [`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq [`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord @@ -3385,6 +3387,7 @@ Released 2018-09-13 [`range_zip_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_zip_with_len [`rc_buffer`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer [`rc_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex +[`recursive_format_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#recursive_format_impl [`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation [`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone [`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure @@ -3459,7 +3462,6 @@ Released 2018-09-13 [`tabs_in_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#tabs_in_doc_comments [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment [`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some -[`to_string_in_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_display [`to_string_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_format_args [`todo`]: https://rust-lang.github.io/rust-clippy/master/index.html#todo [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments diff --git a/Cargo.toml b/Cargo.toml index e445889a5..ef5b0a0cd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ syn = { version = "1.0", features = ["full"] } futures = "0.3" parking_lot = "0.11.2" tokio = { version = "1", features = ["io-util"] } +num_cpus = "1.13" [build-dependencies] rustc_tools_util = { version = "0.2", path = "rustc_tools_util" } diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index 1cc3418d4..f0979840f 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::span_lint_and_note; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::{match_def_path, paths}; use rustc_hir::def_id::DefId; use rustc_hir::{AsyncGeneratorKind, Body, BodyId, GeneratorKind}; @@ -9,8 +9,7 @@ use rustc_span::Span; declare_clippy_lint! { /// ### What it does - /// Checks for calls to await while holding a - /// non-async-aware MutexGuard. + /// Checks for calls to await while holding a non-async-aware MutexGuard. /// /// ### Why is this bad? /// The Mutex types found in std::sync and parking_lot @@ -22,41 +21,57 @@ declare_clippy_lint! { /// either by introducing a scope or an explicit call to Drop::drop. /// /// ### Known problems - /// Will report false positive for explicitly dropped guards ([#6446](https://github.com/rust-lang/rust-clippy/issues/6446)). + /// Will report false positive for explicitly dropped guards + /// ([#6446](https://github.com/rust-lang/rust-clippy/issues/6446)). A workaround for this is + /// to wrap the `.lock()` call in a block instead of explicitly dropping the guard. /// /// ### Example - /// ```rust,ignore - /// use std::sync::Mutex; - /// + /// ```rust + /// # use std::sync::Mutex; + /// # async fn baz() {} /// async fn foo(x: &Mutex) { - /// let guard = x.lock().unwrap(); + /// let mut guard = x.lock().unwrap(); /// *guard += 1; - /// bar.await; + /// baz().await; + /// } + /// + /// async fn bar(x: &Mutex) { + /// let mut guard = x.lock().unwrap(); + /// *guard += 1; + /// drop(guard); // explicit drop + /// baz().await; /// } /// ``` /// /// Use instead: - /// ```rust,ignore - /// use std::sync::Mutex; - /// + /// ```rust + /// # use std::sync::Mutex; + /// # async fn baz() {} /// async fn foo(x: &Mutex) { /// { - /// let guard = x.lock().unwrap(); + /// let mut guard = x.lock().unwrap(); /// *guard += 1; /// } - /// bar.await; + /// baz().await; + /// } + /// + /// async fn bar(x: &Mutex) { + /// { + /// let mut guard = x.lock().unwrap(); + /// *guard += 1; + /// } // guard dropped here at end of scope + /// baz().await; /// } /// ``` #[clippy::version = "1.45.0"] pub AWAIT_HOLDING_LOCK, - pedantic, - "Inside an async function, holding a MutexGuard while calling await" + suspicious, + "inside an async function, holding a `MutexGuard` while calling `await`" } declare_clippy_lint! { /// ### What it does - /// Checks for calls to await while holding a - /// `RefCell` `Ref` or `RefMut`. + /// Checks for calls to await while holding a `RefCell` `Ref` or `RefMut`. /// /// ### Why is this bad? /// `RefCell` refs only check for exclusive mutable access @@ -64,35 +79,52 @@ declare_clippy_lint! { /// risks panics from a mutable ref shared while other refs are outstanding. /// /// ### Known problems - /// Will report false positive for explicitly dropped refs ([#6353](https://github.com/rust-lang/rust-clippy/issues/6353)). + /// Will report false positive for explicitly dropped refs + /// ([#6353](https://github.com/rust-lang/rust-clippy/issues/6353)). A workaround for this is + /// to wrap the `.borrow[_mut]()` call in a block instead of explicitly dropping the ref. /// /// ### Example - /// ```rust,ignore - /// use std::cell::RefCell; - /// + /// ```rust + /// # use std::cell::RefCell; + /// # async fn baz() {} /// async fn foo(x: &RefCell) { /// let mut y = x.borrow_mut(); /// *y += 1; - /// bar.await; + /// baz().await; + /// } + /// + /// async fn bar(x: &RefCell) { + /// let mut y = x.borrow_mut(); + /// *y += 1; + /// drop(y); // explicit drop + /// baz().await; /// } /// ``` /// /// Use instead: - /// ```rust,ignore - /// use std::cell::RefCell; - /// + /// ```rust + /// # use std::cell::RefCell; + /// # async fn baz() {} /// async fn foo(x: &RefCell) { /// { /// let mut y = x.borrow_mut(); /// *y += 1; /// } - /// bar.await; + /// baz().await; + /// } + /// + /// async fn bar(x: &RefCell) { + /// { + /// let mut y = x.borrow_mut(); + /// *y += 1; + /// } // y dropped here at end of scope + /// baz().await; /// } /// ``` #[clippy::version = "1.49.0"] pub AWAIT_HOLDING_REFCELL_REF, - pedantic, - "Inside an async function, holding a RefCell ref while calling await" + suspicious, + "inside an async function, holding a `RefCell` ref while calling `await`" } declare_lint_pass!(AwaitHolding => [AWAIT_HOLDING_LOCK, AWAIT_HOLDING_REFCELL_REF]); @@ -118,23 +150,36 @@ fn check_interior_types(cx: &LateContext<'_>, ty_causes: &[GeneratorInteriorType for ty_cause in ty_causes { if let rustc_middle::ty::Adt(adt, _) = ty_cause.ty.kind() { if is_mutex_guard(cx, adt.did) { - span_lint_and_note( + span_lint_and_then( cx, AWAIT_HOLDING_LOCK, ty_cause.span, - "this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await", - ty_cause.scope_span.or(Some(span)), - "these are all the await points this lock is held through", + "this `MutexGuard` is held across an `await` point", + |diag| { + diag.help( + "consider using an async-aware `Mutex` type or ensuring the \ + `MutexGuard` is dropped before calling await", + ); + diag.span_note( + ty_cause.scope_span.unwrap_or(span), + "these are all the `await` points this lock is held through", + ); + }, ); } if is_refcell_ref(cx, adt.did) { - span_lint_and_note( + span_lint_and_then( cx, AWAIT_HOLDING_REFCELL_REF, ty_cause.span, - "this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await", - ty_cause.scope_span.or(Some(span)), - "these are all the await points this ref is held through", + "this `RefCell` reference is held across an `await` point", + |diag| { + diag.help("ensure the reference is dropped before calling `await`"); + diag.span_note( + ty_cause.scope_span.unwrap_or(span), + "these are all the `await` points this reference is held through", + ); + }, ); } } diff --git a/clippy_lints/src/cargo/common_metadata.rs b/clippy_lints/src/cargo/common_metadata.rs new file mode 100644 index 000000000..e0442dda4 --- /dev/null +++ b/clippy_lints/src/cargo/common_metadata.rs @@ -0,0 +1,54 @@ +//! lint on missing cargo common metadata + +use cargo_metadata::Metadata; +use clippy_utils::diagnostics::span_lint; +use rustc_lint::LateContext; +use rustc_span::source_map::DUMMY_SP; + +use super::CARGO_COMMON_METADATA; + +pub(super) fn check(cx: &LateContext<'_>, metadata: &Metadata, ignore_publish: bool) { + for package in &metadata.packages { + // only run the lint if publish is `None` (`publish = true` or skipped entirely) + // or if the vector isn't empty (`publish = ["something"]`) + if package.publish.as_ref().filter(|publish| publish.is_empty()).is_none() || ignore_publish { + if is_empty_str(&package.description) { + missing_warning(cx, package, "package.description"); + } + + if is_empty_str(&package.license) && is_empty_str(&package.license_file) { + missing_warning(cx, package, "either package.license or package.license_file"); + } + + if is_empty_str(&package.repository) { + missing_warning(cx, package, "package.repository"); + } + + if is_empty_str(&package.readme) { + missing_warning(cx, package, "package.readme"); + } + + if is_empty_vec(&package.keywords) { + missing_warning(cx, package, "package.keywords"); + } + + if is_empty_vec(&package.categories) { + missing_warning(cx, package, "package.categories"); + } + } + } +} + +fn missing_warning(cx: &LateContext<'_>, package: &cargo_metadata::Package, field: &str) { + let message = format!("package `{}` is missing `{}` metadata", package.name, field); + span_lint(cx, CARGO_COMMON_METADATA, DUMMY_SP, &message); +} + +fn is_empty_str>(value: &Option) -> bool { + value.as_ref().map_or(true, |s| s.as_ref().is_empty()) +} + +fn is_empty_vec(value: &[String]) -> bool { + // This works because empty iterators return true + value.iter().all(String::is_empty) +} diff --git a/clippy_lints/src/cargo/feature_name.rs b/clippy_lints/src/cargo/feature_name.rs new file mode 100644 index 000000000..79a469a42 --- /dev/null +++ b/clippy_lints/src/cargo/feature_name.rs @@ -0,0 +1,92 @@ +use cargo_metadata::Metadata; +use clippy_utils::diagnostics::span_lint_and_help; +use rustc_lint::LateContext; +use rustc_span::source_map::DUMMY_SP; + +use super::{NEGATIVE_FEATURE_NAMES, REDUNDANT_FEATURE_NAMES}; + +static PREFIXES: [&str; 8] = ["no-", "no_", "not-", "not_", "use-", "use_", "with-", "with_"]; +static SUFFIXES: [&str; 2] = ["-support", "_support"]; + +pub(super) fn check(cx: &LateContext<'_>, metadata: &Metadata) { + for package in &metadata.packages { + let mut features: Vec<&String> = package.features.keys().collect(); + features.sort(); + for feature in features { + let prefix_opt = { + let i = PREFIXES.partition_point(|prefix| prefix < &feature.as_str()); + if i > 0 && feature.starts_with(PREFIXES[i - 1]) { + Some(PREFIXES[i - 1]) + } else { + None + } + }; + if let Some(prefix) = prefix_opt { + lint(cx, feature, prefix, true); + } + + let suffix_opt: Option<&str> = { + let i = SUFFIXES.partition_point(|suffix| { + suffix.bytes().rev().cmp(feature.bytes().rev()) == std::cmp::Ordering::Less + }); + if i > 0 && feature.ends_with(SUFFIXES[i - 1]) { + Some(SUFFIXES[i - 1]) + } else { + None + } + }; + if let Some(suffix) = suffix_opt { + lint(cx, feature, suffix, false); + } + } + } +} + +fn is_negative_prefix(s: &str) -> bool { + s.starts_with("no") +} + +fn lint(cx: &LateContext<'_>, feature: &str, substring: &str, is_prefix: bool) { + let is_negative = is_prefix && is_negative_prefix(substring); + span_lint_and_help( + cx, + if is_negative { + NEGATIVE_FEATURE_NAMES + } else { + REDUNDANT_FEATURE_NAMES + }, + DUMMY_SP, + &format!( + "the \"{}\" {} in the feature name \"{}\" is {}", + substring, + if is_prefix { "prefix" } else { "suffix" }, + feature, + if is_negative { "negative" } else { "redundant" } + ), + None, + &format!( + "consider renaming the feature to \"{}\"{}", + if is_prefix { + feature.strip_prefix(substring) + } else { + feature.strip_suffix(substring) + } + .unwrap(), + if is_negative { + ", but make sure the feature adds functionality" + } else { + "" + } + ), + ); +} + +#[test] +fn test_prefixes_sorted() { + let mut sorted_prefixes = PREFIXES; + sorted_prefixes.sort_unstable(); + assert_eq!(PREFIXES, sorted_prefixes); + let mut sorted_suffixes = SUFFIXES; + sorted_suffixes.sort_by(|a, b| a.bytes().rev().cmp(b.bytes().rev())); + assert_eq!(SUFFIXES, sorted_suffixes); +} diff --git a/clippy_lints/src/cargo/mod.rs b/clippy_lints/src/cargo/mod.rs new file mode 100644 index 000000000..abe95c666 --- /dev/null +++ b/clippy_lints/src/cargo/mod.rs @@ -0,0 +1,221 @@ +use cargo_metadata::MetadataCommand; +use clippy_utils::diagnostics::span_lint; +use clippy_utils::is_lint_allowed; +use rustc_hir::hir_id::CRATE_HIR_ID; +use rustc_lint::{LateContext, LateLintPass, Lint}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::DUMMY_SP; + +mod common_metadata; +mod feature_name; +mod multiple_crate_versions; +mod wildcard_dependencies; + +declare_clippy_lint! { + /// ### What it does + /// Checks to see if all common metadata is defined in + /// `Cargo.toml`. See: https://rust-lang-nursery.github.io/api-guidelines/documentation.html#cargotoml-includes-all-common-metadata-c-metadata + /// + /// ### Why is this bad? + /// It will be more difficult for users to discover the + /// purpose of the crate, and key information related to it. + /// + /// ### Example + /// ```toml + /// # This `Cargo.toml` is missing a description field: + /// [package] + /// name = "clippy" + /// version = "0.0.212" + /// repository = "https://github.com/rust-lang/rust-clippy" + /// readme = "README.md" + /// license = "MIT OR Apache-2.0" + /// keywords = ["clippy", "lint", "plugin"] + /// categories = ["development-tools", "development-tools::cargo-plugins"] + /// ``` + /// + /// Should include a description field like: + /// + /// ```toml + /// # This `Cargo.toml` includes all common metadata + /// [package] + /// name = "clippy" + /// version = "0.0.212" + /// description = "A bunch of helpful lints to avoid common pitfalls in Rust" + /// repository = "https://github.com/rust-lang/rust-clippy" + /// readme = "README.md" + /// license = "MIT OR Apache-2.0" + /// keywords = ["clippy", "lint", "plugin"] + /// categories = ["development-tools", "development-tools::cargo-plugins"] + /// ``` + #[clippy::version = "1.32.0"] + pub CARGO_COMMON_METADATA, + cargo, + "common metadata is defined in `Cargo.toml`" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for feature names with prefix `use-`, `with-` or suffix `-support` + /// + /// ### Why is this bad? + /// These prefixes and suffixes have no significant meaning. + /// + /// ### Example + /// ```toml + /// # The `Cargo.toml` with feature name redundancy + /// [features] + /// default = ["use-abc", "with-def", "ghi-support"] + /// use-abc = [] // redundant + /// with-def = [] // redundant + /// ghi-support = [] // redundant + /// ``` + /// + /// Use instead: + /// ```toml + /// [features] + /// default = ["abc", "def", "ghi"] + /// abc = [] + /// def = [] + /// ghi = [] + /// ``` + /// + #[clippy::version = "1.57.0"] + pub REDUNDANT_FEATURE_NAMES, + cargo, + "usage of a redundant feature name" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for negative feature names with prefix `no-` or `not-` + /// + /// ### Why is this bad? + /// Features are supposed to be additive, and negatively-named features violate it. + /// + /// ### Example + /// ```toml + /// # The `Cargo.toml` with negative feature names + /// [features] + /// default = [] + /// no-abc = [] + /// not-def = [] + /// + /// ``` + /// Use instead: + /// ```toml + /// [features] + /// default = ["abc", "def"] + /// abc = [] + /// def = [] + /// + /// ``` + #[clippy::version = "1.57.0"] + pub NEGATIVE_FEATURE_NAMES, + cargo, + "usage of a negative feature name" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks to see if multiple versions of a crate are being + /// used. + /// + /// ### Why is this bad? + /// This bloats the size of targets, and can lead to + /// confusing error messages when structs or traits are used interchangeably + /// between different versions of a crate. + /// + /// ### Known problems + /// Because this can be caused purely by the dependencies + /// themselves, it's not always possible to fix this issue. + /// + /// ### Example + /// ```toml + /// # This will pull in both winapi v0.3.x and v0.2.x, triggering a warning. + /// [dependencies] + /// ctrlc = "=3.1.0" + /// ansi_term = "=0.11.0" + /// ``` + #[clippy::version = "pre 1.29.0"] + pub MULTIPLE_CRATE_VERSIONS, + cargo, + "multiple versions of the same crate being used" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for wildcard dependencies in the `Cargo.toml`. + /// + /// ### Why is this bad? + /// [As the edition guide says](https://rust-lang-nursery.github.io/edition-guide/rust-2018/cargo-and-crates-io/crates-io-disallows-wildcard-dependencies.html), + /// it is highly unlikely that you work with any possible version of your dependency, + /// and wildcard dependencies would cause unnecessary breakage in the ecosystem. + /// + /// ### Example + /// ```toml + /// [dependencies] + /// regex = "*" + /// ``` + #[clippy::version = "1.32.0"] + pub WILDCARD_DEPENDENCIES, + cargo, + "wildcard dependencies being used" +} + +pub struct Cargo { + pub ignore_publish: bool, +} + +impl_lint_pass!(Cargo => [ + CARGO_COMMON_METADATA, + REDUNDANT_FEATURE_NAMES, + NEGATIVE_FEATURE_NAMES, + MULTIPLE_CRATE_VERSIONS, + WILDCARD_DEPENDENCIES +]); + +impl LateLintPass<'_> for Cargo { + fn check_crate(&mut self, cx: &LateContext<'_>) { + static NO_DEPS_LINTS: &[&Lint] = &[ + CARGO_COMMON_METADATA, + REDUNDANT_FEATURE_NAMES, + NEGATIVE_FEATURE_NAMES, + WILDCARD_DEPENDENCIES, + ]; + static WITH_DEPS_LINTS: &[&Lint] = &[MULTIPLE_CRATE_VERSIONS]; + + if !NO_DEPS_LINTS + .iter() + .all(|&lint| is_lint_allowed(cx, lint, CRATE_HIR_ID)) + { + match MetadataCommand::new().no_deps().exec() { + Ok(metadata) => { + common_metadata::check(cx, &metadata, self.ignore_publish); + feature_name::check(cx, &metadata); + wildcard_dependencies::check(cx, &metadata); + }, + Err(e) => { + for lint in NO_DEPS_LINTS { + span_lint(cx, lint, DUMMY_SP, &format!("could not read cargo metadata: {}", e)); + } + }, + } + } + + if !WITH_DEPS_LINTS + .iter() + .all(|&lint| is_lint_allowed(cx, lint, CRATE_HIR_ID)) + { + match MetadataCommand::new().exec() { + Ok(metadata) => { + multiple_crate_versions::check(cx, &metadata); + }, + Err(e) => { + for lint in WITH_DEPS_LINTS { + span_lint(cx, lint, DUMMY_SP, &format!("could not read cargo metadata: {}", e)); + } + }, + } + } + } +} diff --git a/clippy_lints/src/cargo/multiple_crate_versions.rs b/clippy_lints/src/cargo/multiple_crate_versions.rs new file mode 100644 index 000000000..76fd0819a --- /dev/null +++ b/clippy_lints/src/cargo/multiple_crate_versions.rs @@ -0,0 +1,63 @@ +//! lint on multiple versions of a crate being used + +use cargo_metadata::{DependencyKind, Metadata, Node, Package, PackageId}; +use clippy_utils::diagnostics::span_lint; +use if_chain::if_chain; +use itertools::Itertools; +use rustc_hir::def_id::LOCAL_CRATE; +use rustc_lint::LateContext; +use rustc_span::source_map::DUMMY_SP; + +use super::MULTIPLE_CRATE_VERSIONS; + +pub(super) fn check(cx: &LateContext<'_>, metadata: &Metadata) { + let local_name = cx.tcx.crate_name(LOCAL_CRATE); + let mut packages = metadata.packages.clone(); + packages.sort_by(|a, b| a.name.cmp(&b.name)); + + if_chain! { + if let Some(resolve) = &metadata.resolve; + if let Some(local_id) = packages + .iter() + .find_map(|p| if p.name == local_name.as_str() { Some(&p.id) } else { None }); + then { + for (name, group) in &packages.iter().group_by(|p| p.name.clone()) { + let group: Vec<&Package> = group.collect(); + + if group.len() <= 1 { + continue; + } + + if group.iter().all(|p| is_normal_dep(&resolve.nodes, local_id, &p.id)) { + let mut versions: Vec<_> = group.into_iter().map(|p| &p.version).collect(); + versions.sort(); + let versions = versions.iter().join(", "); + + span_lint( + cx, + MULTIPLE_CRATE_VERSIONS, + DUMMY_SP, + &format!("multiple versions for dependency `{}`: {}", name, versions), + ); + } + } + } + } +} + +fn is_normal_dep(nodes: &[Node], local_id: &PackageId, dep_id: &PackageId) -> bool { + fn depends_on(node: &Node, dep_id: &PackageId) -> bool { + node.deps.iter().any(|dep| { + dep.pkg == *dep_id + && dep + .dep_kinds + .iter() + .any(|info| matches!(info.kind, DependencyKind::Normal)) + }) + } + + nodes + .iter() + .filter(|node| depends_on(node, dep_id)) + .any(|node| node.id == *local_id || is_normal_dep(nodes, local_id, &node.id)) +} diff --git a/clippy_lints/src/cargo/wildcard_dependencies.rs b/clippy_lints/src/cargo/wildcard_dependencies.rs new file mode 100644 index 000000000..7fa6acbf5 --- /dev/null +++ b/clippy_lints/src/cargo/wildcard_dependencies.rs @@ -0,0 +1,27 @@ +use cargo_metadata::Metadata; +use clippy_utils::diagnostics::span_lint; +use if_chain::if_chain; +use rustc_lint::LateContext; +use rustc_span::source_map::DUMMY_SP; + +use super::WILDCARD_DEPENDENCIES; + +pub(super) fn check(cx: &LateContext<'_>, metadata: &Metadata) { + for dep in &metadata.packages[0].dependencies { + // VersionReq::any() does not work + if_chain! { + if let Ok(wildcard_ver) = semver::VersionReq::parse("*"); + if let Some(ref source) = dep.source; + if !source.starts_with("git"); + if dep.req == wildcard_ver; + then { + span_lint( + cx, + WILDCARD_DEPENDENCIES, + DUMMY_SP, + &format!("wildcard dependency for `{}`", dep.name), + ); + } + } + } +} diff --git a/clippy_lints/src/cargo_common_metadata.rs b/clippy_lints/src/cargo_common_metadata.rs deleted file mode 100644 index 23f79fdc6..000000000 --- a/clippy_lints/src/cargo_common_metadata.rs +++ /dev/null @@ -1,118 +0,0 @@ -//! lint on missing cargo common metadata - -use clippy_utils::{diagnostics::span_lint, is_lint_allowed}; -use rustc_hir::hir_id::CRATE_HIR_ID; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::source_map::DUMMY_SP; - -declare_clippy_lint! { - /// ### What it does - /// Checks to see if all common metadata is defined in - /// `Cargo.toml`. See: https://rust-lang-nursery.github.io/api-guidelines/documentation.html#cargotoml-includes-all-common-metadata-c-metadata - /// - /// ### Why is this bad? - /// It will be more difficult for users to discover the - /// purpose of the crate, and key information related to it. - /// - /// ### Example - /// ```toml - /// # This `Cargo.toml` is missing a description field: - /// [package] - /// name = "clippy" - /// version = "0.0.212" - /// repository = "https://github.com/rust-lang/rust-clippy" - /// readme = "README.md" - /// license = "MIT OR Apache-2.0" - /// keywords = ["clippy", "lint", "plugin"] - /// categories = ["development-tools", "development-tools::cargo-plugins"] - /// ``` - /// - /// Should include a description field like: - /// - /// ```toml - /// # This `Cargo.toml` includes all common metadata - /// [package] - /// name = "clippy" - /// version = "0.0.212" - /// description = "A bunch of helpful lints to avoid common pitfalls in Rust" - /// repository = "https://github.com/rust-lang/rust-clippy" - /// readme = "README.md" - /// license = "MIT OR Apache-2.0" - /// keywords = ["clippy", "lint", "plugin"] - /// categories = ["development-tools", "development-tools::cargo-plugins"] - /// ``` - #[clippy::version = "1.32.0"] - pub CARGO_COMMON_METADATA, - cargo, - "common metadata is defined in `Cargo.toml`" -} - -#[derive(Copy, Clone, Debug)] -pub struct CargoCommonMetadata { - ignore_publish: bool, -} - -impl CargoCommonMetadata { - pub fn new(ignore_publish: bool) -> Self { - Self { ignore_publish } - } -} - -impl_lint_pass!(CargoCommonMetadata => [ - CARGO_COMMON_METADATA -]); - -fn missing_warning(cx: &LateContext<'_>, package: &cargo_metadata::Package, field: &str) { - let message = format!("package `{}` is missing `{}` metadata", package.name, field); - span_lint(cx, CARGO_COMMON_METADATA, DUMMY_SP, &message); -} - -fn is_empty_str>(value: &Option) -> bool { - value.as_ref().map_or(true, |s| s.as_ref().is_empty()) -} - -fn is_empty_vec(value: &[String]) -> bool { - // This works because empty iterators return true - value.iter().all(String::is_empty) -} - -impl LateLintPass<'_> for CargoCommonMetadata { - fn check_crate(&mut self, cx: &LateContext<'_>) { - if is_lint_allowed(cx, CARGO_COMMON_METADATA, CRATE_HIR_ID) { - return; - } - - let metadata = unwrap_cargo_metadata!(cx, CARGO_COMMON_METADATA, false); - - for package in metadata.packages { - // only run the lint if publish is `None` (`publish = true` or skipped entirely) - // or if the vector isn't empty (`publish = ["something"]`) - if package.publish.as_ref().filter(|publish| publish.is_empty()).is_none() || self.ignore_publish { - if is_empty_str(&package.description) { - missing_warning(cx, &package, "package.description"); - } - - if is_empty_str(&package.license) && is_empty_str(&package.license_file) { - missing_warning(cx, &package, "either package.license or package.license_file"); - } - - if is_empty_str(&package.repository) { - missing_warning(cx, &package, "package.repository"); - } - - if is_empty_str(&package.readme) { - missing_warning(cx, &package, "package.readme"); - } - - if is_empty_vec(&package.keywords) { - missing_warning(cx, &package, "package.keywords"); - } - - if is_empty_vec(&package.categories) { - missing_warning(cx, &package, "package.categories"); - } - } - } - } -} diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs index ea74d5acb..9b189ea1e 100644 --- a/clippy_lints/src/casts/cast_possible_truncation.rs +++ b/clippy_lints/src/casts/cast_possible_truncation.rs @@ -1,12 +1,15 @@ use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::span_lint; use clippy_utils::expr_or_init; -use clippy_utils::ty::is_isize_or_usize; +use clippy_utils::ty::{get_discriminant_value, is_isize_or_usize}; +use rustc_ast::ast; +use rustc_attr::IntType; +use rustc_hir::def::{DefKind, Res}; use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::LateContext; use rustc_middle::ty::{self, FloatTy, Ty}; -use super::{utils, CAST_POSSIBLE_TRUNCATION}; +use super::{utils, CAST_ENUM_TRUNCATION, CAST_POSSIBLE_TRUNCATION}; fn constant_int(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { if let Some((Constant::Int(c), _)) = constant(cx, cx.typeck_results(), expr) { @@ -75,8 +78,8 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b } pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { - let msg = match (cast_from.is_integral(), cast_to.is_integral()) { - (true, true) => { + let msg = match (cast_from.kind(), cast_to.is_integral()) { + (ty::Int(_) | ty::Uint(_), true) => { let from_nbits = apply_reductions( cx, utils::int_ty_to_nbits(cast_from, cx.tcx), @@ -108,19 +111,60 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, ) }, - (false, true) => { + (ty::Adt(def, _), true) if def.is_enum() => { + let (from_nbits, variant) = if let ExprKind::Path(p) = &cast_expr.kind + && let Res::Def(DefKind::Ctor(..), id) = cx.qpath_res(p, cast_expr.hir_id) + { + let i = def.variant_index_with_ctor_id(id); + let variant = &def.variants[i]; + let nbits = utils::enum_value_nbits(get_discriminant_value(cx.tcx, def, i)); + (nbits, Some(variant)) + } else { + (utils::enum_ty_to_nbits(def, cx.tcx), None) + }; + let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx); + + let cast_from_ptr_size = def.repr.int.map_or(true, |ty| { + matches!( + ty, + IntType::SignedInt(ast::IntTy::Isize) | IntType::UnsignedInt(ast::UintTy::Usize) + ) + }); + let suffix = match (cast_from_ptr_size, is_isize_or_usize(cast_to)) { + (false, false) if from_nbits > to_nbits => "", + (true, false) if from_nbits > to_nbits => "", + (false, true) if from_nbits > 64 => "", + (false, true) if from_nbits > 32 => " on targets with 32-bit wide pointers", + _ => return, + }; + + if let Some(variant) = variant { + span_lint( + cx, + CAST_ENUM_TRUNCATION, + expr.span, + &format!( + "casting `{}::{}` to `{}` will truncate the value{}", + cast_from, variant.name, cast_to, suffix, + ), + ); + return; + } + format!( + "casting `{}` to `{}` may truncate the value{}", + cast_from, cast_to, suffix, + ) + }, + + (ty::Float(_), true) => { format!("casting `{}` to `{}` may truncate the value", cast_from, cast_to) }, - (_, _) => { - if matches!(cast_from.kind(), &ty::Float(FloatTy::F64)) - && matches!(cast_to.kind(), &ty::Float(FloatTy::F32)) - { - "casting `f64` to `f32` may truncate the value".to_string() - } else { - return; - } + (ty::Float(FloatTy::F64), false) if matches!(cast_to.kind(), &ty::Float(FloatTy::F32)) => { + "casting `f64` to `f32` may truncate the value".to_string() }, + + _ => return, }; span_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span, &msg); diff --git a/clippy_lints/src/casts/cast_ptr_alignment.rs b/clippy_lints/src/casts/cast_ptr_alignment.rs index 079b7ff06..a4ef1344a 100644 --- a/clippy_lints/src/casts/cast_ptr_alignment.rs +++ b/clippy_lints/src/casts/cast_ptr_alignment.rs @@ -1,11 +1,11 @@ use clippy_utils::diagnostics::span_lint; use clippy_utils::is_hir_ty_cfg_dependant; +use clippy_utils::ty::is_c_void; use if_chain::if_chain; use rustc_hir::{Expr, ExprKind, GenericArg}; use rustc_lint::LateContext; use rustc_middle::ty::layout::LayoutOf; use rustc_middle::ty::{self, Ty}; -use rustc_span::symbol::sym; use super::CAST_PTR_ALIGNMENT; @@ -62,19 +62,3 @@ fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_f } } } - -/// Check if the given type is either `core::ffi::c_void` or -/// one of the platform specific `libc::::c_void` of libc. -fn is_c_void(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { - if let ty::Adt(adt, _) = ty.kind() { - let names = cx.get_def_path(adt.did); - - if names.is_empty() { - return false; - } - if names[0] == sym::libc || names[0] == sym::core && *names.last().unwrap() == sym!(c_void) { - return true; - } - } - false -} diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index aee1e50b9..f2077c569 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -390,6 +390,25 @@ declare_clippy_lint! { "casting using `as` from and to raw pointers that doesn't change its mutability, where `pointer::cast` could take the place of `as`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for casts from an enum type to an integral type which will definitely truncate the + /// value. + /// + /// ### Why is this bad? + /// The resulting integral value will not match the value of the variant it came from. + /// + /// ### Example + /// ```rust + /// enum E { X = 256 }; + /// let _ = E::X as u8; + /// ``` + #[clippy::version = "1.60.0"] + pub CAST_ENUM_TRUNCATION, + suspicious, + "casts from an enum type to an integral type which will truncate the value" +} + pub struct Casts { msrv: Option, } @@ -415,10 +434,15 @@ impl_lint_pass!(Casts => [ FN_TO_NUMERIC_CAST_WITH_TRUNCATION, CHAR_LIT_AS_U8, PTR_AS_PTR, + CAST_ENUM_TRUNCATION, ]); impl<'tcx> LateLintPass<'tcx> for Casts { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if !in_external_macro(cx.sess(), expr.span) { + ptr_as_ptr::check(cx, expr, &self.msrv); + } + if expr.span.from_expansion() { return; } @@ -441,13 +465,12 @@ impl<'tcx> LateLintPass<'tcx> for Casts { fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to); if cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { + cast_possible_truncation::check(cx, expr, cast_expr, cast_from, cast_to); if cast_from.is_numeric() { - cast_possible_truncation::check(cx, expr, cast_expr, cast_from, cast_to); cast_possible_wrap::check(cx, expr, cast_from, cast_to); cast_precision_loss::check(cx, expr, cast_from, cast_to); cast_sign_loss::check(cx, expr, cast_expr, cast_from, cast_to); } - cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to, &self.msrv); } } @@ -455,7 +478,6 @@ impl<'tcx> LateLintPass<'tcx> for Casts { cast_ref_to_mut::check(cx, expr); cast_ptr_alignment::check(cx, expr); char_lit_as_u8::check(cx, expr); - ptr_as_ptr::check(cx, expr, &self.msrv); } extract_msrv_attr!(LateContext); diff --git a/clippy_lints/src/casts/utils.rs b/clippy_lints/src/casts/utils.rs index 00fd0b347..bbed766c4 100644 --- a/clippy_lints/src/casts/utils.rs +++ b/clippy_lints/src/casts/utils.rs @@ -1,4 +1,5 @@ -use rustc_middle::ty::{self, IntTy, Ty, TyCtxt, UintTy}; +use clippy_utils::ty::{read_explicit_enum_value, EnumValue}; +use rustc_middle::ty::{self, AdtDef, IntTy, Ty, TyCtxt, UintTy, VariantDiscr}; /// Returns the size in bits of an integral type. /// Will return 0 if the type is not an int or uint variant @@ -23,3 +24,52 @@ pub(super) fn int_ty_to_nbits(typ: Ty<'_>, tcx: TyCtxt<'_>) -> u64 { _ => 0, } } + +pub(super) fn enum_value_nbits(value: EnumValue) -> u64 { + match value { + EnumValue::Unsigned(x) => 128 - x.leading_zeros(), + EnumValue::Signed(x) if x < 0 => 128 - (-(x + 1)).leading_zeros() + 1, + EnumValue::Signed(x) => 128 - x.leading_zeros(), + } + .into() +} + +pub(super) fn enum_ty_to_nbits(adt: &AdtDef, tcx: TyCtxt<'_>) -> u64 { + let mut explicit = 0i128; + let (start, end) = adt + .variants + .iter() + .fold((0, i128::MIN), |(start, end), variant| match variant.discr { + VariantDiscr::Relative(x) => match explicit.checked_add(i128::from(x)) { + Some(x) => (start, end.max(x)), + None => (i128::MIN, end), + }, + VariantDiscr::Explicit(id) => match read_explicit_enum_value(tcx, id) { + Some(EnumValue::Signed(x)) => { + explicit = x; + (start.min(x), end.max(x)) + }, + Some(EnumValue::Unsigned(x)) => match i128::try_from(x) { + Ok(x) => { + explicit = x; + (start, end.max(x)) + }, + Err(_) => (i128::MIN, end), + }, + None => (start, end), + }, + }); + + if start > end { + // No variants. + 0 + } else { + let neg_bits = if start < 0 { + 128 - (-(start + 1)).leading_zeros() + 1 + } else { + 0 + }; + let pos_bits = if end > 0 { 128 - end.leading_zeros() } else { 0 }; + neg_bits.max(pos_bits).into() + } +} diff --git a/clippy_lints/src/dbg_macro.rs b/clippy_lints/src/dbg_macro.rs index 5a0b60fdf..df1a4128a 100644 --- a/clippy_lints/src/dbg_macro.rs +++ b/clippy_lints/src/dbg_macro.rs @@ -1,11 +1,11 @@ -use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; -use clippy_utils::source::snippet_opt; -use rustc_ast::ast; -use rustc_ast::tokenstream::TokenStream; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::macros::root_macro_call_first_node; +use clippy_utils::source::snippet_with_applicability; use rustc_errors::Applicability; -use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::source_map::Span; +use rustc_span::sym; declare_clippy_lint! { /// ### What it does @@ -15,14 +15,6 @@ declare_clippy_lint! { /// `dbg!` macro is intended as a debugging tool. It /// should not be in version control. /// - /// ### Known problems - /// * The lint level is unaffected by crate attributes. The level can still - /// be set for functions, modules and other items. To change the level for - /// the entire crate, please use command line flags. More information and a - /// configuration example can be found in [clippy#6610]. - /// - /// [clippy#6610]: https://github.com/rust-lang/rust-clippy/issues/6610#issuecomment-977120558 - /// /// ### Example /// ```rust,ignore /// // Bad @@ -39,37 +31,52 @@ declare_clippy_lint! { declare_lint_pass!(DbgMacro => [DBG_MACRO]); -impl EarlyLintPass for DbgMacro { - fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &ast::MacCall) { - if mac.path == sym!(dbg) { - if let Some(sugg) = tts_span(mac.args.inner_tokens()).and_then(|span| snippet_opt(cx, span)) { - span_lint_and_sugg( - cx, - DBG_MACRO, - mac.span(), - "`dbg!` macro is intended as a debugging tool", - "ensure to avoid having uses of it in version control", - sugg, - Applicability::MaybeIncorrect, - ); - } else { - span_lint_and_help( - cx, - DBG_MACRO, - mac.span(), - "`dbg!` macro is intended as a debugging tool", - None, - "ensure to avoid having uses of it in version control", - ); - } +impl LateLintPass<'_> for DbgMacro { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return }; + if cx.tcx.is_diagnostic_item(sym::dbg_macro, macro_call.def_id) { + let mut applicability = Applicability::MachineApplicable; + let suggestion = match expr.peel_drop_temps().kind { + // dbg!() + ExprKind::Block(_, _) => String::new(), + // dbg!(1) + ExprKind::Match(val, ..) => { + snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability).to_string() + }, + // dbg!(2, 3) + ExprKind::Tup( + [ + Expr { + kind: ExprKind::Match(first, ..), + .. + }, + .., + Expr { + kind: ExprKind::Match(last, ..), + .. + }, + ], + ) => { + let snippet = snippet_with_applicability( + cx, + first.span.source_callsite().to(last.span.source_callsite()), + "..", + &mut applicability, + ); + format!("({snippet})") + }, + _ => return, + }; + + span_lint_and_sugg( + cx, + DBG_MACRO, + macro_call.span, + "`dbg!` macro is intended as a debugging tool", + "ensure to avoid having uses of it in version control", + suggestion, + applicability, + ); } } } - -// Get span enclosing entire the token stream. -fn tts_span(tts: TokenStream) -> Option { - let mut cursor = tts.into_trees(); - let first = cursor.next()?.span(); - let span = cursor.last().map_or(first, |tree| first.to(tree.span())); - Some(span) -} diff --git a/clippy_lints/src/default.rs b/clippy_lints/src/default.rs index 307058848..06e6bf986 100644 --- a/clippy_lints/src/default.rs +++ b/clippy_lints/src/default.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_sugg}; use clippy_utils::source::snippet_with_macro_callsite; use clippy_utils::ty::{has_drop, is_copy}; -use clippy_utils::{any_parent_is_automatically_derived, contains_name, match_def_path, paths}; +use clippy_utils::{any_parent_is_automatically_derived, contains_name, get_parent_expr, match_def_path, paths}; use if_chain::if_chain; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; @@ -88,6 +88,7 @@ impl<'tcx> LateLintPass<'tcx> for Default { if let ExprKind::Path(ref qpath) = path.kind; if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id(); if match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD); + if !is_update_syntax_base(cx, expr); // Detect and ignore ::default() because these calls do explicitly name the type. if let QPath::Resolved(None, _path) = qpath; let expr_ty = cx.typeck_results().expr_ty(expr); @@ -290,3 +291,16 @@ fn field_reassigned_by_stmt<'tcx>(this: &Stmt<'tcx>, binding_name: Symbol) -> Op } } } + +/// Returns whether `expr` is the update syntax base: `Foo { a: 1, .. base }` +fn is_update_syntax_base<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { + if_chain! { + if let Some(parent) = get_parent_expr(cx, expr); + if let ExprKind::Struct(_, _, Some(base)) = parent.kind; + then { + base.hir_id == expr.hir_id + } else { + false + } + } +} diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index 6d3df260c..7277e4080 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note, span_lint_and_then}; use clippy_utils::paths; use clippy_utils::ty::{implements_trait, is_copy}; -use clippy_utils::{get_trait_def_id, is_automatically_derived, is_lint_allowed, match_def_path}; +use clippy_utils::{is_automatically_derived, is_lint_allowed, match_def_path}; use if_chain::if_chain; use rustc_hir::intravisit::{walk_expr, walk_fn, walk_item, FnKind, Visitor}; use rustc_hir::{ @@ -12,6 +12,7 @@ use rustc_middle::hir::nested_filter; use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; +use rustc_span::sym; declare_clippy_lint! { /// ### What it does @@ -196,7 +197,7 @@ fn check_hash_peq<'tcx>( if_chain! { if let Some(peq_trait_def_id) = cx.tcx.lang_items().eq_trait(); if let Some(def_id) = trait_ref.trait_def_id(); - if match_def_path(cx, def_id, &paths::HASH); + if cx.tcx.is_diagnostic_item(sym::Hash, def_id); then { // Look for the PartialEq implementations for `ty` cx.tcx.for_each_relevant_impl(peq_trait_def_id, ty, |impl_id| { @@ -247,7 +248,7 @@ fn check_ord_partial_ord<'tcx>( ord_is_automatically_derived: bool, ) { if_chain! { - if let Some(ord_trait_def_id) = get_trait_def_id(cx, &paths::ORD); + if let Some(ord_trait_def_id) = cx.tcx.get_diagnostic_item(sym::Ord); if let Some(partial_ord_trait_def_id) = cx.tcx.lang_items().partial_ord_trait(); if let Some(def_id) = &trait_ref.trait_def_id(); if *def_id == ord_trait_def_id; diff --git a/clippy_lints/src/duration_subsec.rs b/clippy_lints/src/duration_subsec.rs index 24e32c09f..09318f745 100644 --- a/clippy_lints/src/duration_subsec.rs +++ b/clippy_lints/src/duration_subsec.rs @@ -1,15 +1,14 @@ +use clippy_utils::consts::{constant, Constant}; +use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::ty::match_type; +use clippy_utils::ty::is_type_diagnostic_item; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Spanned; - -use clippy_utils::consts::{constant, Constant}; -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::paths; +use rustc_span::sym; declare_clippy_lint! { /// ### What it does @@ -46,7 +45,7 @@ impl<'tcx> LateLintPass<'tcx> for DurationSubsec { if_chain! { if let ExprKind::Binary(Spanned { node: BinOpKind::Div, .. }, left, right) = expr.kind; if let ExprKind::MethodCall(method_path, args, _) = left.kind; - if match_type(cx, cx.typeck_results().expr_ty(&args[0]).peel_refs(), &paths::DURATION); + if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&args[0]).peel_refs(), sym::Duration); if let Some((Constant::Int(divisor), _)) = constant(cx, cx.typeck_results(), right); then { let suggested_fn = match (method_path.ident.as_str(), divisor) { diff --git a/clippy_lints/src/eq_op.rs b/clippy_lints/src/eq_op.rs index ea547793b..6490231fe 100644 --- a/clippy_lints/src/eq_op.rs +++ b/clippy_lints/src/eq_op.rs @@ -6,9 +6,7 @@ use clippy_utils::ty::{implements_trait, is_copy}; use clippy_utils::{ast_utils::is_useless_with_eq_exprs, eq_expr_value, is_in_test_function}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{ - def::Res, def_id::DefId, BinOpKind, BorrowKind, Expr, ExprKind, GenericArg, ItemKind, QPath, TyKind, -}; +use rustc_hir::{def::Res, def_id::DefId, BinOpKind, BorrowKind, Expr, ExprKind, GenericArg, ItemKind, QPath, TyKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -279,7 +277,11 @@ impl<'tcx> LateLintPass<'tcx> for EqOp { } } -fn in_impl<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, bin_op: DefId) -> Option<(&'tcx rustc_hir::Ty<'tcx>, &'tcx rustc_hir::Ty<'tcx>)> { +fn in_impl<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx Expr<'_>, + bin_op: DefId, +) -> Option<(&'tcx rustc_hir::Ty<'tcx>, &'tcx rustc_hir::Ty<'tcx>)> { if_chain! { if let Some(block) = get_enclosing_block(cx, e.hir_id); if let Some(impl_def_id) = cx.tcx.impl_of_method(block.hir_id.owner.to_def_id()); diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs index 263bff487..d23c0c225 100644 --- a/clippy_lints/src/eta_reduction.rs +++ b/clippy_lints/src/eta_reduction.rs @@ -10,6 +10,7 @@ use rustc_hir::def_id::DefId; use rustc_hir::{Expr, ExprKind, Param, PatKind, Unsafety}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow}; +use rustc_middle::ty::binding::BindingMode; use rustc_middle::ty::subst::Subst; use rustc_middle::ty::{self, ClosureKind, Ty, TypeFoldable}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -169,11 +170,17 @@ fn check_inputs(cx: &LateContext<'_>, params: &[Param<'_>], call_args: &[Expr<'_ if params.len() != call_args.len() { return false; } + let binding_modes = cx.typeck_results().pat_binding_modes(); std::iter::zip(params, call_args).all(|(param, arg)| { match param.pat.kind { PatKind::Binding(_, id, ..) if path_to_local_id(arg, id) => {}, _ => return false, } + // checks that parameters are not bound as `ref` or `ref mut` + if let Some(BindingMode::BindByReference(_)) = binding_modes.get(param.pat.hir_id) { + return false; + } + match *cx.typeck_results().expr_adjustments(arg) { [] => true, [ diff --git a/clippy_lints/src/feature_name.rs b/clippy_lints/src/feature_name.rs deleted file mode 100644 index dc6bef52d..000000000 --- a/clippy_lints/src/feature_name.rs +++ /dev/null @@ -1,166 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::{diagnostics::span_lint, is_lint_allowed}; -use rustc_hir::CRATE_HIR_ID; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::source_map::DUMMY_SP; - -declare_clippy_lint! { - /// ### What it does - /// Checks for feature names with prefix `use-`, `with-` or suffix `-support` - /// - /// ### Why is this bad? - /// These prefixes and suffixes have no significant meaning. - /// - /// ### Example - /// ```toml - /// # The `Cargo.toml` with feature name redundancy - /// [features] - /// default = ["use-abc", "with-def", "ghi-support"] - /// use-abc = [] // redundant - /// with-def = [] // redundant - /// ghi-support = [] // redundant - /// ``` - /// - /// Use instead: - /// ```toml - /// [features] - /// default = ["abc", "def", "ghi"] - /// abc = [] - /// def = [] - /// ghi = [] - /// ``` - /// - #[clippy::version = "1.57.0"] - pub REDUNDANT_FEATURE_NAMES, - cargo, - "usage of a redundant feature name" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for negative feature names with prefix `no-` or `not-` - /// - /// ### Why is this bad? - /// Features are supposed to be additive, and negatively-named features violate it. - /// - /// ### Example - /// ```toml - /// # The `Cargo.toml` with negative feature names - /// [features] - /// default = [] - /// no-abc = [] - /// not-def = [] - /// - /// ``` - /// Use instead: - /// ```toml - /// [features] - /// default = ["abc", "def"] - /// abc = [] - /// def = [] - /// - /// ``` - #[clippy::version = "1.57.0"] - pub NEGATIVE_FEATURE_NAMES, - cargo, - "usage of a negative feature name" -} - -declare_lint_pass!(FeatureName => [REDUNDANT_FEATURE_NAMES, NEGATIVE_FEATURE_NAMES]); - -static PREFIXES: [&str; 8] = ["no-", "no_", "not-", "not_", "use-", "use_", "with-", "with_"]; -static SUFFIXES: [&str; 2] = ["-support", "_support"]; - -fn is_negative_prefix(s: &str) -> bool { - s.starts_with("no") -} - -fn lint(cx: &LateContext<'_>, feature: &str, substring: &str, is_prefix: bool) { - let is_negative = is_prefix && is_negative_prefix(substring); - span_lint_and_help( - cx, - if is_negative { - NEGATIVE_FEATURE_NAMES - } else { - REDUNDANT_FEATURE_NAMES - }, - DUMMY_SP, - &format!( - "the \"{}\" {} in the feature name \"{}\" is {}", - substring, - if is_prefix { "prefix" } else { "suffix" }, - feature, - if is_negative { "negative" } else { "redundant" } - ), - None, - &format!( - "consider renaming the feature to \"{}\"{}", - if is_prefix { - feature.strip_prefix(substring) - } else { - feature.strip_suffix(substring) - } - .unwrap(), - if is_negative { - ", but make sure the feature adds functionality" - } else { - "" - } - ), - ); -} - -impl LateLintPass<'_> for FeatureName { - fn check_crate(&mut self, cx: &LateContext<'_>) { - if is_lint_allowed(cx, REDUNDANT_FEATURE_NAMES, CRATE_HIR_ID) - && is_lint_allowed(cx, NEGATIVE_FEATURE_NAMES, CRATE_HIR_ID) - { - return; - } - - let metadata = unwrap_cargo_metadata!(cx, REDUNDANT_FEATURE_NAMES, false); - - for package in metadata.packages { - let mut features: Vec<&String> = package.features.keys().collect(); - features.sort(); - for feature in features { - let prefix_opt = { - let i = PREFIXES.partition_point(|prefix| prefix < &feature.as_str()); - if i > 0 && feature.starts_with(PREFIXES[i - 1]) { - Some(PREFIXES[i - 1]) - } else { - None - } - }; - if let Some(prefix) = prefix_opt { - lint(cx, feature, prefix, true); - } - - let suffix_opt: Option<&str> = { - let i = SUFFIXES.partition_point(|suffix| { - suffix.bytes().rev().cmp(feature.bytes().rev()) == std::cmp::Ordering::Less - }); - if i > 0 && feature.ends_with(SUFFIXES[i - 1]) { - Some(SUFFIXES[i - 1]) - } else { - None - } - }; - if let Some(suffix) = suffix_opt { - lint(cx, feature, suffix, false); - } - } - } - } -} - -#[test] -fn test_prefixes_sorted() { - let mut sorted_prefixes = PREFIXES; - sorted_prefixes.sort_unstable(); - assert_eq!(PREFIXES, sorted_prefixes); - let mut sorted_suffixes = SUFFIXES; - sorted_suffixes.sort_by(|a, b| a.bytes().rev().cmp(b.bytes().rev())); - assert_eq!(SUFFIXES, sorted_suffixes); -} diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index 503aac8cc..1e6feaac2 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -1,8 +1,8 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; -use clippy_utils::macros::{FormatArgsArg, FormatArgsExpn}; +use clippy_utils::is_diag_trait_item; +use clippy_utils::macros::{is_format_macro, FormatArgsArg, FormatArgsExpn}; use clippy_utils::source::snippet_opt; use clippy_utils::ty::implements_trait; -use clippy_utils::{is_diag_trait_item, match_def_path, paths}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; @@ -65,21 +65,6 @@ declare_clippy_lint! { declare_lint_pass!(FormatArgs => [FORMAT_IN_FORMAT_ARGS, TO_STRING_IN_FORMAT_ARGS]); -const FORMAT_MACRO_PATHS: &[&[&str]] = &[ - &paths::FORMAT_ARGS_MACRO, - &paths::ASSERT_EQ_MACRO, - &paths::ASSERT_MACRO, - &paths::ASSERT_NE_MACRO, - &paths::EPRINT_MACRO, - &paths::EPRINTLN_MACRO, - &paths::PRINT_MACRO, - &paths::PRINTLN_MACRO, - &paths::WRITE_MACRO, - &paths::WRITELN_MACRO, -]; - -const FORMAT_MACRO_DIAG_ITEMS: &[Symbol] = &[sym::format_macro, sym::std_panic_macro]; - impl<'tcx> LateLintPass<'tcx> for FormatArgs { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if_chain! { @@ -87,12 +72,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs { let expr_expn_data = expr.span.ctxt().outer_expn_data(); let outermost_expn_data = outermost_expn_data(expr_expn_data); if let Some(macro_def_id) = outermost_expn_data.macro_def_id; - if FORMAT_MACRO_PATHS - .iter() - .any(|path| match_def_path(cx, macro_def_id, path)) - || FORMAT_MACRO_DIAG_ITEMS - .iter() - .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, macro_def_id)); + if is_format_macro(cx, macro_def_id); if let ExpnKind::Macro(_, name) = outermost_expn_data.kind; if let Some(args) = format_args.args(); then { diff --git a/clippy_lints/src/infinite_iter.rs b/clippy_lints/src/infinite_iter.rs index b6badef02..b2b9889f5 100644 --- a/clippy_lints/src/infinite_iter.rs +++ b/clippy_lints/src/infinite_iter.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; -use clippy_utils::{get_trait_def_id, higher, match_def_path, path_def_id, paths}; +use clippy_utils::{higher, match_def_path, path_def_id, paths}; use rustc_hir::{BorrowKind, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -229,9 +229,12 @@ fn complete_infinite_iter(cx: &LateContext<'_>, expr: &Expr<'_>) -> Finiteness { } } if method.ident.name == sym!(last) && args.len() == 1 { - let not_double_ended = get_trait_def_id(cx, &paths::DOUBLE_ENDED_ITERATOR).map_or(false, |id| { - !implements_trait(cx, cx.typeck_results().expr_ty(&args[0]), id, &[]) - }); + let not_double_ended = cx + .tcx + .get_diagnostic_item(sym::DoubleEndedIterator) + .map_or(false, |id| { + !implements_trait(cx, cx.typeck_results().expr_ty(&args[0]), id, &[]) + }); if not_double_ended { return is_infinite(cx, &args[0]); } diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 4721b7f2b..c6f8470cd 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -14,6 +14,8 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(attrs::DEPRECATED_SEMVER), LintId::of(attrs::MISMATCHED_TARGET_OS), LintId::of(attrs::USELESS_ATTRIBUTE), + LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK), + LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF), LintId::of(bit_mask::BAD_BIT_MASK), LintId::of(bit_mask::INEFFECTIVE_BIT_MASK), LintId::of(blacklisted_name::BLACKLISTED_NAME), @@ -21,6 +23,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON), LintId::of(booleans::LOGIC_BUG), LintId::of(booleans::NONMINIMAL_BOOL), + LintId::of(casts::CAST_ENUM_TRUNCATION), LintId::of(casts::CAST_REF_TO_MUT), LintId::of(casts::CHAR_LIT_AS_U8), LintId::of(casts::FN_TO_NUMERIC_CAST), @@ -241,6 +244,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(ranges::MANUAL_RANGE_CONTAINS), LintId::of(ranges::RANGE_ZIP_WITH_LEN), LintId::of(ranges::REVERSED_EMPTY_RANGES), + LintId::of(recursive_format_impl::RECURSIVE_FORMAT_IMPL), LintId::of(redundant_clone::REDUNDANT_CLONE), LintId::of(redundant_closure_call::REDUNDANT_CLOSURE_CALL), LintId::of(redundant_field_names::REDUNDANT_FIELD_NAMES), @@ -267,7 +271,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(tabs_in_doc_comments::TABS_IN_DOC_COMMENTS), LintId::of(temporary_assignment::TEMPORARY_ASSIGNMENT), LintId::of(to_digit_is_some::TO_DIGIT_IS_SOME), - LintId::of(to_string_in_display::TO_STRING_IN_DISPLAY), LintId::of(transmute::CROSSPOINTER_TRANSMUTE), LintId::of(transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS), LintId::of(transmute::TRANSMUTE_BYTES_TO_STR), @@ -277,6 +280,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(transmute::TRANSMUTE_INT_TO_FLOAT), LintId::of(transmute::TRANSMUTE_NUM_TO_BYTES), LintId::of(transmute::TRANSMUTE_PTR_TO_REF), + LintId::of(transmute::TRANSMUTE_UNDEFINED_REPR), LintId::of(transmute::UNSOUND_COLLECTION_TRANSMUTE), LintId::of(transmute::WRONG_TRANSMUTE), LintId::of(transmuting_null::TRANSMUTING_NULL), diff --git a/clippy_lints/src/lib.register_cargo.rs b/clippy_lints/src/lib.register_cargo.rs index 1809f2cc7..c890523fe 100644 --- a/clippy_lints/src/lib.register_cargo.rs +++ b/clippy_lints/src/lib.register_cargo.rs @@ -3,9 +3,9 @@ // Manual edits will be overwritten. store.register_group(true, "clippy::cargo", Some("clippy_cargo"), vec![ - LintId::of(cargo_common_metadata::CARGO_COMMON_METADATA), - LintId::of(feature_name::NEGATIVE_FEATURE_NAMES), - LintId::of(feature_name::REDUNDANT_FEATURE_NAMES), - LintId::of(multiple_crate_versions::MULTIPLE_CRATE_VERSIONS), - LintId::of(wildcard_dependencies::WILDCARD_DEPENDENCIES), + LintId::of(cargo::CARGO_COMMON_METADATA), + LintId::of(cargo::MULTIPLE_CRATE_VERSIONS), + LintId::of(cargo::NEGATIVE_FEATURE_NAMES), + LintId::of(cargo::REDUNDANT_FEATURE_NAMES), + LintId::of(cargo::WILDCARD_DEPENDENCIES), ]) diff --git a/clippy_lints/src/lib.register_correctness.rs b/clippy_lints/src/lib.register_correctness.rs index 4217fd3a3..18fe44282 100644 --- a/clippy_lints/src/lib.register_correctness.rs +++ b/clippy_lints/src/lib.register_correctness.rs @@ -52,12 +52,13 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve LintId::of(ptr::INVALID_NULL_PTR_USAGE), LintId::of(ptr::MUT_FROM_REF), LintId::of(ranges::REVERSED_EMPTY_RANGES), + LintId::of(recursive_format_impl::RECURSIVE_FORMAT_IMPL), LintId::of(regex::INVALID_REGEX), LintId::of(self_assignment::SELF_ASSIGNMENT), LintId::of(serde_api::SERDE_API_MISUSE), LintId::of(size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT), LintId::of(swap::ALMOST_SWAPPED), - LintId::of(to_string_in_display::TO_STRING_IN_DISPLAY), + LintId::of(transmute::TRANSMUTE_UNDEFINED_REPR), LintId::of(transmute::UNSOUND_COLLECTION_TRANSMUTE), LintId::of(transmute::WRONG_TRANSMUTE), LintId::of(transmuting_null::TRANSMUTING_NULL), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index a80320a57..75ef1b0a9 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -61,8 +61,13 @@ store.register_lints(&[ booleans::NONMINIMAL_BOOL, borrow_as_ptr::BORROW_AS_PTR, bytecount::NAIVE_BYTECOUNT, - cargo_common_metadata::CARGO_COMMON_METADATA, + cargo::CARGO_COMMON_METADATA, + cargo::MULTIPLE_CRATE_VERSIONS, + cargo::NEGATIVE_FEATURE_NAMES, + cargo::REDUNDANT_FEATURE_NAMES, + cargo::WILDCARD_DEPENDENCIES, case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS, + casts::CAST_ENUM_TRUNCATION, casts::CAST_LOSSLESS, casts::CAST_POSSIBLE_TRUNCATION, casts::CAST_POSSIBLE_WRAP, @@ -139,8 +144,6 @@ store.register_lints(&[ exit::EXIT, explicit_write::EXPLICIT_WRITE, fallible_impl_from::FALLIBLE_IMPL_FROM, - feature_name::NEGATIVE_FEATURE_NAMES, - feature_name::REDUNDANT_FEATURE_NAMES, float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS, float_literal::EXCESSIVE_PRECISION, float_literal::LOSSY_FLOAT_LITERAL, @@ -353,7 +356,6 @@ store.register_lints(&[ module_style::MOD_MODULE_FILES, module_style::SELF_NAMED_MODULE_FILES, modulo_arithmetic::MODULO_ARITHMETIC, - multiple_crate_versions::MULTIPLE_CRATE_VERSIONS, mut_key::MUTABLE_KEY_TYPE, mut_mut::MUT_MUT, mut_mutex_lock::MUT_MUTEX_LOCK, @@ -415,11 +417,13 @@ store.register_lints(&[ ranges::RANGE_PLUS_ONE, ranges::RANGE_ZIP_WITH_LEN, ranges::REVERSED_EMPTY_RANGES, + recursive_format_impl::RECURSIVE_FORMAT_IMPL, redundant_clone::REDUNDANT_CLONE, redundant_closure_call::REDUNDANT_CLOSURE_CALL, redundant_else::REDUNDANT_ELSE, redundant_field_names::REDUNDANT_FIELD_NAMES, redundant_pub_crate::REDUNDANT_PUB_CRATE, + redundant_slicing::DEREF_BY_SLICING, redundant_slicing::REDUNDANT_SLICING, redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES, ref_option_ref::REF_OPTION_REF, @@ -459,7 +463,6 @@ store.register_lints(&[ tabs_in_doc_comments::TABS_IN_DOC_COMMENTS, temporary_assignment::TEMPORARY_ASSIGNMENT, to_digit_is_some::TO_DIGIT_IS_SOME, - to_string_in_display::TO_STRING_IN_DISPLAY, trailing_empty_array::TRAILING_EMPTY_ARRAY, trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS, trait_bounds::TYPE_REPETITION_IN_BOUNDS, @@ -520,7 +523,6 @@ store.register_lints(&[ vec_init_then_push::VEC_INIT_THEN_PUSH, vec_resize_to_zero::VEC_RESIZE_TO_ZERO, verbose_file_reads::VERBOSE_FILE_READS, - wildcard_dependencies::WILDCARD_DEPENDENCIES, wildcard_imports::ENUM_GLOB_USE, wildcard_imports::WILDCARD_IMPORTS, write::PRINTLN_EMPTY_STRING, diff --git a/clippy_lints/src/lib.register_nursery.rs b/clippy_lints/src/lib.register_nursery.rs index 8d4dde42b..a73537901 100644 --- a/clippy_lints/src/lib.register_nursery.rs +++ b/clippy_lints/src/lib.register_nursery.rs @@ -26,7 +26,6 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![ LintId::of(strings::STRING_LIT_AS_BYTES), LintId::of(suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS), LintId::of(trailing_empty_array::TRAILING_EMPTY_ARRAY), - LintId::of(transmute::TRANSMUTE_UNDEFINED_REPR), LintId::of(transmute::USELESS_TRANSMUTE), LintId::of(use_self::USE_SELF), ]) diff --git a/clippy_lints/src/lib.register_pedantic.rs b/clippy_lints/src/lib.register_pedantic.rs index 1292675f4..00d305131 100644 --- a/clippy_lints/src/lib.register_pedantic.rs +++ b/clippy_lints/src/lib.register_pedantic.rs @@ -4,8 +4,6 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![ LintId::of(attrs::INLINE_ALWAYS), - LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK), - LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF), LintId::of(bit_mask::VERBOSE_BIT_MASK), LintId::of(borrow_as_ptr::BORROW_AS_PTR), LintId::of(bytecount::NAIVE_BYTECOUNT), diff --git a/clippy_lints/src/lib.register_restriction.rs b/clippy_lints/src/lib.register_restriction.rs index 5a89fdb05..f89f35b88 100644 --- a/clippy_lints/src/lib.register_restriction.rs +++ b/clippy_lints/src/lib.register_restriction.rs @@ -51,6 +51,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve LintId::of(panic_unimplemented::UNIMPLEMENTED), LintId::of(panic_unimplemented::UNREACHABLE), LintId::of(pattern_type_mismatch::PATTERN_TYPE_MISMATCH), + LintId::of(redundant_slicing::DEREF_BY_SLICING), LintId::of(same_name_method::SAME_NAME_METHOD), LintId::of(shadow::SHADOW_REUSE), LintId::of(shadow::SHADOW_SAME), diff --git a/clippy_lints/src/lib.register_suspicious.rs b/clippy_lints/src/lib.register_suspicious.rs index 10f8ae4b7..6a8859e19 100644 --- a/clippy_lints/src/lib.register_suspicious.rs +++ b/clippy_lints/src/lib.register_suspicious.rs @@ -5,6 +5,9 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec![ LintId::of(assign_ops::MISREFACTORED_ASSIGN_OP), LintId::of(attrs::BLANKET_CLIPPY_RESTRICTION_LINTS), + LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK), + LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF), + LintId::of(casts::CAST_ENUM_TRUNCATION), LintId::of(eval_order_dependence::EVAL_ORDER_DEPENDENCE), LintId::of(float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS), LintId::of(formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5c45012ef..a21a87899 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -5,6 +5,7 @@ #![feature(control_flow_enum)] #![feature(drain_filter)] #![feature(iter_intersperse)] +#![feature(let_chains)] #![feature(let_else)] #![feature(once_cell)] #![feature(rustc_private)] @@ -18,12 +19,13 @@ // warn on rustc internal lints #![warn(rustc::internal)] // Disable this rustc lint for now, as it was also done in rustc -#![cfg_attr(not(bootstrap), allow(rustc::potential_query_instability))] +#![allow(rustc::potential_query_instability)] // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) extern crate rustc_ast; extern crate rustc_ast_pretty; +extern crate rustc_attr; extern crate rustc_data_structures; extern crate rustc_driver; extern crate rustc_errors; @@ -177,7 +179,7 @@ mod bool_assert_comparison; mod booleans; mod borrow_as_ptr; mod bytecount; -mod cargo_common_metadata; +mod cargo; mod case_sensitive_file_extension_comparisons; mod casts; mod checked_conversions; @@ -219,7 +221,6 @@ mod exhaustive_items; mod exit; mod explicit_write; mod fallible_impl_from; -mod feature_name; mod float_equality_without_abs; mod float_literal; mod floating_point_arithmetic; @@ -289,7 +290,6 @@ mod missing_enforced_import_rename; mod missing_inline; mod module_style; mod modulo_arithmetic; -mod multiple_crate_versions; mod mut_key; mod mut_mut; mod mut_mutex_lock; @@ -333,6 +333,7 @@ mod ptr_eq; mod ptr_offset_with_cast; mod question_mark; mod ranges; +mod recursive_format_impl; mod redundant_clone; mod redundant_closure_call; mod redundant_else; @@ -365,7 +366,6 @@ mod swap; mod tabs_in_doc_comments; mod temporary_assignment; mod to_digit_is_some; -mod to_string_in_display; mod trailing_empty_array; mod trait_bounds; mod transmute; @@ -398,7 +398,6 @@ mod vec; mod vec_init_then_push; mod vec_resize_to_zero; mod verbose_file_reads; -mod wildcard_dependencies; mod wildcard_imports; mod write; mod zero_div_zero; @@ -431,7 +430,6 @@ pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore, sess: &Se store.register_pre_expansion_pass(|| Box::new(write::Write::default())); store.register_pre_expansion_pass(move || Box::new(attrs::EarlyAttributes { msrv })); - store.register_pre_expansion_pass(|| Box::new(dbg_macro::DbgMacro)); } #[doc(hidden)] @@ -707,7 +705,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(modulo_arithmetic::ModuloArithmetic)); store.register_early_pass(|| Box::new(reference::DerefAddrOf)); store.register_early_pass(|| Box::new(double_parens::DoubleParens)); - store.register_late_pass(|| Box::new(to_string_in_display::ToStringInDisplay::new())); + store.register_late_pass(|| Box::new(recursive_format_impl::RecursiveFormatImpl::new())); store.register_early_pass(|| Box::new(unsafe_removed_from_name::UnsafeNameRemoval)); store.register_early_pass(|| Box::new(else_if_without_else::ElseIfWithoutElse)); store.register_early_pass(|| Box::new(int_plus_one::IntPlusOne)); @@ -724,10 +722,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| Box::new(redundant_else::RedundantElse)); store.register_late_pass(|| Box::new(create_dir::CreateDir)); store.register_early_pass(|| Box::new(needless_arbitrary_self_type::NeedlessArbitrarySelfType)); - let cargo_ignore_publish = conf.cargo_ignore_publish; - store.register_late_pass(move || Box::new(cargo_common_metadata::CargoCommonMetadata::new(cargo_ignore_publish))); - store.register_late_pass(|| Box::new(multiple_crate_versions::MultipleCrateVersions)); - store.register_late_pass(|| Box::new(wildcard_dependencies::WildcardDependencies)); let literal_representation_lint_fraction_readability = conf.unreadable_literal_lint_fractions; store.register_early_pass(move || { Box::new(literal_representation::LiteralDigitGrouping::new( @@ -842,7 +836,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(move || Box::new(disallowed_script_idents::DisallowedScriptIdents::new(&scripts))); store.register_late_pass(|| Box::new(strlen_on_c_strings::StrlenOnCStrings)); store.register_late_pass(move || Box::new(self_named_constructors::SelfNamedConstructors)); - store.register_late_pass(move || Box::new(feature_name::FeatureName)); store.register_late_pass(move || Box::new(iter_not_returning_iterator::IterNotReturningIterator)); store.register_late_pass(move || Box::new(manual_assert::ManualAssert)); let enable_raw_pointer_heuristic_for_send = conf.enable_raw_pointer_heuristic_for_send; @@ -863,6 +856,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || Box::new(borrow_as_ptr::BorrowAsPtr::new(msrv))); store.register_late_pass(move || Box::new(manual_bits::ManualBits::new(msrv))); store.register_late_pass(|| Box::new(default_union_representation::DefaultUnionRepresentation)); + store.register_late_pass(|| Box::new(dbg_macro::DbgMacro)); + let cargo_ignore_publish = conf.cargo_ignore_publish; + store.register_late_pass(move || { + Box::new(cargo::Cargo { + ignore_publish: cargo_ignore_publish, + }) + }); // add lints here, do not remove this comment, it's used in `new_lint` } @@ -939,6 +939,7 @@ pub fn register_renamed(ls: &mut rustc_lint::LintStore) { ls.register_renamed("clippy::disallowed_type", "clippy::disallowed_types"); ls.register_renamed("clippy::disallowed_method", "clippy::disallowed_methods"); ls.register_renamed("clippy::ref_in_deref", "clippy::needless_borrow"); + ls.register_renamed("clippy::to_string_in_display", "clippy::recursive_format_impl"); // uplifted lints ls.register_renamed("clippy::invalid_ref", "invalid_value"); diff --git a/clippy_lints/src/macro_use.rs b/clippy_lints/src/macro_use.rs index ca1ccb93b..76c5cfadc 100644 --- a/clippy_lints/src/macro_use.rs +++ b/clippy_lints/src/macro_use.rs @@ -35,7 +35,8 @@ struct PathAndSpan { span: Span, } -/// `MacroRefData` includes the name of the macro. +/// `MacroRefData` includes the name of the macro +/// and the path from `SourceMap::span_to_filename`. #[derive(Debug, Clone)] pub struct MacroRefData { name: String, diff --git a/clippy_lints/src/matches/match_like_matches.rs b/clippy_lints/src/matches/match_like_matches.rs index d605b6d73..2e1f7646e 100644 --- a/clippy_lints/src/matches/match_like_matches.rs +++ b/clippy_lints/src/matches/match_like_matches.rs @@ -3,7 +3,7 @@ use clippy_utils::source::snippet_with_applicability; use clippy_utils::{higher, is_wild}; use rustc_ast::{Attribute, LitKind}; use rustc_errors::Applicability; -use rustc_hir::{BorrowKind, Expr, ExprKind, Guard, MatchSource, Pat}; +use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Guard, Pat}; use rustc_lint::LateContext; use rustc_middle::ty; use rustc_span::source_map::Spanned; @@ -11,7 +11,7 @@ use rustc_span::source_map::Spanned; use super::MATCH_LIKE_MATCHES_MACRO; /// Lint a `match` or `if let .. { .. } else { .. }` expr that could be replaced by `matches!` -pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { +pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if let Some(higher::IfLet { let_pat, let_expr, @@ -19,7 +19,7 @@ pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool if_else: Some(if_else), }) = higher::IfLet::hir(cx, expr) { - return find_matches_sugg( + find_matches_sugg( cx, let_expr, IntoIterator::into_iter([(&[][..], Some(let_pat), if_then, None), (&[][..], None, if_else, None)]), @@ -27,25 +27,28 @@ pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool true, ); } +} - if let ExprKind::Match(scrut, arms, MatchSource::Normal) = expr.kind { - return find_matches_sugg( - cx, - scrut, - arms.iter().map(|arm| { - ( - cx.tcx.hir().attrs(arm.hir_id), - Some(arm.pat), - arm.body, - arm.guard.as_ref(), - ) - }), - expr, - false, - ); - } - - false +pub(super) fn check_match<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx Expr<'_>, + scrutinee: &'tcx Expr<'_>, + arms: &'tcx [Arm<'tcx>], +) -> bool { + find_matches_sugg( + cx, + scrutinee, + arms.iter().map(|arm| { + ( + cx.tcx.hir().attrs(arm.hir_id), + Some(arm.pat), + arm.body, + arm.guard.as_ref(), + ) + }), + e, + false, + ) } /// Lint a `match` or `if let` for replacement by `matches!` diff --git a/clippy_lints/src/matches/match_same_arms.rs b/clippy_lints/src/matches/match_same_arms.rs index 271a38685..d11dda57e 100644 --- a/clippy_lints/src/matches/match_same_arms.rs +++ b/clippy_lints/src/matches/match_same_arms.rs @@ -1,96 +1,94 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; use clippy_utils::{path_to_local, search_same, SpanlessEq, SpanlessHash}; -use rustc_hir::{Arm, Expr, ExprKind, HirId, HirIdMap, HirIdSet, MatchSource, Pat, PatKind}; +use rustc_hir::{Arm, Expr, HirId, HirIdMap, HirIdSet, Pat, PatKind}; use rustc_lint::LateContext; use std::collections::hash_map::Entry; use super::MATCH_SAME_ARMS; -pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) { - if let ExprKind::Match(_, arms, MatchSource::Normal) = expr.kind { - let hash = |&(_, arm): &(usize, &Arm<'_>)| -> u64 { - let mut h = SpanlessHash::new(cx); - h.hash_expr(arm.body); - h.finish() - }; +pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) { + let hash = |&(_, arm): &(usize, &Arm<'_>)| -> u64 { + let mut h = SpanlessHash::new(cx); + h.hash_expr(arm.body); + h.finish() + }; - let eq = |&(lindex, lhs): &(usize, &Arm<'_>), &(rindex, rhs): &(usize, &Arm<'_>)| -> bool { - let min_index = usize::min(lindex, rindex); - let max_index = usize::max(lindex, rindex); + let eq = |&(lindex, lhs): &(usize, &Arm<'_>), &(rindex, rhs): &(usize, &Arm<'_>)| -> bool { + let min_index = usize::min(lindex, rindex); + let max_index = usize::max(lindex, rindex); - let mut local_map: HirIdMap = HirIdMap::default(); - let eq_fallback = |a: &Expr<'_>, b: &Expr<'_>| { - if_chain! { - if let Some(a_id) = path_to_local(a); - if let Some(b_id) = path_to_local(b); - let entry = match local_map.entry(a_id) { - Entry::Vacant(entry) => entry, - // check if using the same bindings as before - Entry::Occupied(entry) => return *entry.get() == b_id, - }; - // the names technically don't have to match; this makes the lint more conservative - if cx.tcx.hir().name(a_id) == cx.tcx.hir().name(b_id); - if cx.typeck_results().expr_ty(a) == cx.typeck_results().expr_ty(b); - if pat_contains_local(lhs.pat, a_id); - if pat_contains_local(rhs.pat, b_id); - then { - entry.insert(b_id); - true - } else { - false - } + let mut local_map: HirIdMap = HirIdMap::default(); + let eq_fallback = |a: &Expr<'_>, b: &Expr<'_>| { + if_chain! { + if let Some(a_id) = path_to_local(a); + if let Some(b_id) = path_to_local(b); + let entry = match local_map.entry(a_id) { + Entry::Vacant(entry) => entry, + // check if using the same bindings as before + Entry::Occupied(entry) => return *entry.get() == b_id, + }; + // the names technically don't have to match; this makes the lint more conservative + if cx.tcx.hir().name(a_id) == cx.tcx.hir().name(b_id); + if cx.typeck_results().expr_ty(a) == cx.typeck_results().expr_ty(b); + if pat_contains_local(lhs.pat, a_id); + if pat_contains_local(rhs.pat, b_id); + then { + entry.insert(b_id); + true + } else { + false } - }; - // Arms with a guard are ignored, those can’t always be merged together - // This is also the case for arms in-between each there is an arm with a guard - (min_index..=max_index).all(|index| arms[index].guard.is_none()) - && SpanlessEq::new(cx) - .expr_fallback(eq_fallback) - .eq_expr(lhs.body, rhs.body) - // these checks could be removed to allow unused bindings - && bindings_eq(lhs.pat, local_map.keys().copied().collect()) - && bindings_eq(rhs.pat, local_map.values().copied().collect()) + } }; + // Arms with a guard are ignored, those can’t always be merged together + // This is also the case for arms in-between each there is an arm with a guard + (min_index..=max_index).all(|index| arms[index].guard.is_none()) + && SpanlessEq::new(cx) + .expr_fallback(eq_fallback) + .eq_expr(lhs.body, rhs.body) + // these checks could be removed to allow unused bindings + && bindings_eq(lhs.pat, local_map.keys().copied().collect()) + && bindings_eq(rhs.pat, local_map.values().copied().collect()) + }; - let indexed_arms: Vec<(usize, &Arm<'_>)> = arms.iter().enumerate().collect(); - for (&(_, i), &(_, j)) in search_same(&indexed_arms, hash, eq) { - span_lint_and_then( - cx, - MATCH_SAME_ARMS, - j.body.span, - "this `match` has identical arm bodies", - |diag| { - diag.span_note(i.body.span, "same as this"); + let indexed_arms: Vec<(usize, &Arm<'_>)> = arms.iter().enumerate().collect(); + for (&(_, i), &(_, j)) in search_same(&indexed_arms, hash, eq) { + span_lint_and_then( + cx, + MATCH_SAME_ARMS, + j.body.span, + "this `match` has identical arm bodies", + |diag| { + diag.span_note(i.body.span, "same as this"); - // Note: this does not use `span_suggestion` on purpose: - // there is no clean way - // to remove the other arm. Building a span and suggest to replace it to "" - // makes an even more confusing error message. Also in order not to make up a - // span for the whole pattern, the suggestion is only shown when there is only - // one pattern. The user should know about `|` if they are already using it… + // Note: this does not use `span_suggestion` on purpose: + // there is no clean way + // to remove the other arm. Building a span and suggest to replace it to "" + // makes an even more confusing error message. Also in order not to make up a + // span for the whole pattern, the suggestion is only shown when there is only + // one pattern. The user should know about `|` if they are already using it… - let lhs = snippet(cx, i.pat.span, ""); - let rhs = snippet(cx, j.pat.span, ""); + let lhs = snippet(cx, i.pat.span, ""); + let rhs = snippet(cx, j.pat.span, ""); - if let PatKind::Wild = j.pat.kind { - // if the last arm is _, then i could be integrated into _ - // note that i.pat cannot be _, because that would mean that we're - // hiding all the subsequent arms, and rust won't compile - diag.span_note( - i.body.span, - &format!( - "`{}` has the same arm body as the `_` wildcard, consider removing it", - lhs - ), - ); - } else { - diag.span_help(i.pat.span, &format!("consider refactoring into `{} | {}`", lhs, rhs,)) - .help("...or consider changing the match arm bodies"); - } - }, - ); - } + if let PatKind::Wild = j.pat.kind { + // if the last arm is _, then i could be integrated into _ + // note that i.pat cannot be _, because that would mean that we're + // hiding all the subsequent arms, and rust won't compile + diag.span_note( + i.body.span, + &format!( + "`{}` has the same arm body as the `_` wildcard, consider removing it", + lhs + ), + ); + } else { + diag.span_help(i.pat.span, &format!("consider refactoring into `{} | {}`", lhs, rhs,)) + .help("...or consider changing the match arm bodies"); + } + }, + ); } } diff --git a/clippy_lints/src/matches/match_single_binding.rs b/clippy_lints/src/matches/match_single_binding.rs index 8ae19e03f..39fe54648 100644 --- a/clippy_lints/src/matches/match_single_binding.rs +++ b/clippy_lints/src/matches/match_single_binding.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::{indent_of, snippet_block, snippet_opt, snippet_with_applicability}; +use clippy_utils::source::{indent_of, snippet_block, snippet_with_applicability}; use clippy_utils::sugg::Sugg; use clippy_utils::{get_parent_expr, is_refutable, peel_blocks}; use rustc_errors::Applicability; @@ -14,23 +14,6 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e return; } - // HACK: - // This is a hack to deal with arms that are excluded by macros like `#[cfg]`. It is only used here - // to prevent false positives as there is currently no better way to detect if code was excluded by - // a macro. See PR #6435 - if_chain! { - if let Some(match_snippet) = snippet_opt(cx, expr.span); - if let Some(arm_snippet) = snippet_opt(cx, arms[0].span); - if let Some(ex_snippet) = snippet_opt(cx, ex.span); - let rest_snippet = match_snippet.replace(&arm_snippet, "").replace(&ex_snippet, ""); - if rest_snippet.contains("=>"); - then { - // The code it self contains another thick arrow "=>" - // -> Either another arm or a comment - return; - } - } - let matched_vars = ex.span; let bind_names = arms[0].pat.span; let match_body = peel_blocks(arms[0].body); diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index b5ee4561f..92179eb6f 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -1,8 +1,11 @@ +use clippy_utils::source::{snippet_opt, walk_span_to_context}; use clippy_utils::{meets_msrv, msrvs}; -use rustc_hir::{Expr, ExprKind, Local, MatchSource, Pat}; +use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat}; +use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::{Span, SpanData, SyntaxContext}; mod infalliable_detructuring_match; mod match_as_ref; @@ -604,33 +607,39 @@ impl<'tcx> LateLintPass<'tcx> for Matches { return; } - redundant_pattern_match::check(cx, expr); + if let ExprKind::Match(ex, arms, source) = expr.kind { + if !contains_cfg_arm(cx, expr, ex, arms) { + if source == MatchSource::Normal { + if !(meets_msrv(self.msrv.as_ref(), &msrvs::MATCHES_MACRO) + && match_like_matches::check_match(cx, expr, ex, arms)) + { + match_same_arms::check(cx, arms); + } - if meets_msrv(self.msrv.as_ref(), &msrvs::MATCHES_MACRO) { - if !match_like_matches::check(cx, expr) { - match_same_arms::check(cx, expr); + redundant_pattern_match::check_match(cx, expr, ex, arms); + single_match::check(cx, ex, arms, expr); + match_bool::check(cx, ex, arms, expr); + overlapping_arms::check(cx, ex, arms); + match_wild_enum::check(cx, ex, arms); + match_as_ref::check(cx, ex, arms, expr); + + if self.infallible_destructuring_match_linted { + self.infallible_destructuring_match_linted = false; + } else { + match_single_binding::check(cx, ex, arms, expr); + } + } + match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr); } - } else { - match_same_arms::check(cx, expr); - } - if let ExprKind::Match(ex, arms, MatchSource::Normal) = expr.kind { - single_match::check(cx, ex, arms, expr); - match_bool::check(cx, ex, arms, expr); - overlapping_arms::check(cx, ex, arms); + // These don't depend on a relationship between multiple arms match_wild_err_arm::check(cx, ex, arms); - match_wild_enum::check(cx, ex, arms); - match_as_ref::check(cx, ex, arms, expr); wild_in_or_pats::check(cx, arms); - - if self.infallible_destructuring_match_linted { - self.infallible_destructuring_match_linted = false; - } else { - match_single_binding::check(cx, ex, arms, expr); + } else { + if meets_msrv(self.msrv.as_ref(), &msrvs::MATCHES_MACRO) { + match_like_matches::check(cx, expr); } - } - if let ExprKind::Match(ex, arms, _) = expr.kind { - match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr); + redundant_pattern_match::check(cx, expr); } } @@ -644,3 +653,94 @@ impl<'tcx> LateLintPass<'tcx> for Matches { extract_msrv_attr!(LateContext); } + +/// Checks if there are any arms with a `#[cfg(..)]` attribute. +fn contains_cfg_arm(cx: &LateContext<'_>, e: &Expr<'_>, scrutinee: &Expr<'_>, arms: &[Arm<'_>]) -> bool { + let Some(scrutinee_span) = walk_span_to_context(scrutinee.span, SyntaxContext::root()) else { + // Shouldn't happen, but treat this as though a `cfg` attribute were found + return true; + }; + + let start = scrutinee_span.hi(); + let mut arm_spans = arms.iter().map(|arm| { + let data = arm.span.data(); + (data.ctxt == SyntaxContext::root()).then(|| (data.lo, data.hi)) + }); + let end = e.span.hi(); + + // Walk through all the non-code space before each match arm. The space trailing the final arm is + // handled after the `try_fold` e.g. + // + // match foo { + // _________^- everything between the scrutinee and arm1 + //| arm1 => (), + //|---^___________^ everything before arm2 + //| #[cfg(feature = "enabled")] + //| arm2 => some_code(), + //|---^____________________^ everything before arm3 + //| // some comment about arm3 + //| arm3 => some_code(), + //|---^____________________^ everything after arm3 + //| #[cfg(feature = "disabled")] + //| arm4 = some_code(), + //|}; + //|^ + let found = arm_spans.try_fold(start, |start, range| { + let Some((end, next_start)) = range else { + // Shouldn't happen as macros can't expand to match arms, but treat this as though a `cfg` attribute were + // found. + return Err(()); + }; + let span = SpanData { + lo: start, + hi: end, + ctxt: SyntaxContext::root(), + parent: None, + } + .span(); + (!span_contains_cfg(cx, span)).then(|| next_start).ok_or(()) + }); + match found { + Ok(start) => { + let span = SpanData { + lo: start, + hi: end, + ctxt: SyntaxContext::root(), + parent: None, + } + .span(); + span_contains_cfg(cx, span) + }, + Err(()) => true, + } +} + +/// Checks if the given span contains a `#[cfg(..)]` attribute +fn span_contains_cfg(cx: &LateContext<'_>, s: Span) -> bool { + let Some(snip) = snippet_opt(cx, s) else { + // Assume true. This would require either an invalid span, or one which crosses file boundaries. + return true; + }; + let mut pos = 0usize; + let mut iter = tokenize(&snip).map(|t| { + let start = pos; + pos += t.len; + (t.kind, start..pos) + }); + + // Search for the token sequence [`#`, `[`, `cfg`] + while iter.any(|(t, _)| matches!(t, TokenKind::Pound)) { + let mut iter = iter.by_ref().skip_while(|(t, _)| { + matches!( + t, + TokenKind::Whitespace | TokenKind::LineComment { .. } | TokenKind::BlockComment { .. } + ) + }); + if matches!(iter.next(), Some((TokenKind::OpenBracket, _))) + && matches!(iter.next(), Some((TokenKind::Ident, range)) if &snip[range.clone()] == "cfg") + { + return true; + } + } + false +} diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs index 677b8cdf2..777ec9b75 100644 --- a/clippy_lints/src/matches/redundant_pattern_match.rs +++ b/clippy_lints/src/matches/redundant_pattern_match.rs @@ -12,13 +12,13 @@ use rustc_errors::Applicability; use rustc_hir::LangItem::{OptionNone, PollPending}; use rustc_hir::{ intravisit::{walk_expr, Visitor}, - Arm, Block, Expr, ExprKind, LangItem, MatchSource, Node, Pat, PatKind, QPath, UnOp, + Arm, Block, Expr, ExprKind, LangItem, Node, Pat, PatKind, QPath, UnOp, }; use rustc_lint::LateContext; use rustc_middle::ty::{self, subst::GenericArgKind, DefIdTree, Ty}; use rustc_span::sym; -pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if let Some(higher::IfLet { if_else, let_pat, @@ -27,11 +27,7 @@ pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { }) = higher::IfLet::hir(cx, expr) { find_sugg_for_if_let(cx, expr, let_pat, let_expr, "if", if_else.is_some()); - } - if let ExprKind::Match(op, arms, MatchSource::Normal) = &expr.kind { - find_sugg_for_match(cx, expr, op, arms); - } - if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) { + } else if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) { find_sugg_for_if_let(cx, expr, let_pat, let_expr, "while", false); } } @@ -59,7 +55,7 @@ fn type_needs_ordered_drop_inner<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, see // Check if any component type has any. match ty.kind() { ty::Tuple(fields) => fields.iter().any(|ty| type_needs_ordered_drop_inner(cx, ty, seen)), - &ty::Array(ty, _) => type_needs_ordered_drop_inner(cx, ty, seen), + ty::Array(ty, _) => type_needs_ordered_drop_inner(cx, *ty, seen), ty::Adt(adt, subs) => adt .all_fields() .map(|f| f.ty(cx.tcx, subs)) @@ -304,7 +300,7 @@ fn find_sugg_for_if_let<'tcx>( ); } -fn find_sugg_for_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op: &Expr<'_>, arms: &[Arm<'_>]) { +pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op: &Expr<'_>, arms: &[Arm<'_>]) { if arms.len() == 2 { let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind); diff --git a/clippy_lints/src/minmax.rs b/clippy_lints/src/minmax.rs index cf9770f5c..65d1f440b 100644 --- a/clippy_lints/src/minmax.rs +++ b/clippy_lints/src/minmax.rs @@ -1,10 +1,11 @@ use clippy_utils::consts::{constant_simple, Constant}; use clippy_utils::diagnostics::span_lint; -use clippy_utils::{match_def_path, match_trait_method, paths}; +use clippy_utils::{match_trait_method, paths}; use if_chain::if_chain; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; use std::cmp::Ordering; declare_clippy_lint! { @@ -73,14 +74,10 @@ fn min_max<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<(MinMax, Cons cx.typeck_results() .qpath_res(qpath, path.hir_id) .opt_def_id() - .and_then(|def_id| { - if match_def_path(cx, def_id, &paths::CMP_MIN) { - fetch_const(cx, args, MinMax::Min) - } else if match_def_path(cx, def_id, &paths::CMP_MAX) { - fetch_const(cx, args, MinMax::Max) - } else { - None - } + .and_then(|def_id| match cx.tcx.get_diagnostic_name(def_id) { + Some(sym::cmp_min) => fetch_const(cx, args, MinMax::Min), + Some(sym::cmp_max) => fetch_const(cx, args, MinMax::Max), + _ => None, }) } else { None diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs index fc0483a92..db6aab067 100644 --- a/clippy_lints/src/missing_doc.rs +++ b/clippy_lints/src/missing_doc.rs @@ -7,7 +7,8 @@ use clippy_utils::attrs::is_doc_hidden; use clippy_utils::diagnostics::span_lint; -use rustc_ast::ast; +use if_chain::if_chain; +use rustc_ast::ast::{self, MetaItem, MetaItemKind}; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::ty; @@ -57,6 +58,20 @@ impl MissingDoc { *self.doc_hidden_stack.last().expect("empty doc_hidden_stack") } + fn has_include(meta: Option) -> bool { + if_chain! { + if let Some(meta) = meta; + if let MetaItemKind::List(list) = meta.kind; + if let Some(meta) = list.get(0); + if let Some(name) = meta.ident(); + then { + name.name == sym::include + } else { + false + } + } + } + fn check_missing_docs_attrs( &self, cx: &LateContext<'_>, @@ -80,7 +95,9 @@ impl MissingDoc { return; } - let has_doc = attrs.iter().any(|a| a.doc_str().is_some()); + let has_doc = attrs + .iter() + .any(|a| a.doc_str().is_some() || Self::has_include(a.meta())); if !has_doc { span_lint( cx, diff --git a/clippy_lints/src/multiple_crate_versions.rs b/clippy_lints/src/multiple_crate_versions.rs deleted file mode 100644 index 1f9db39cf..000000000 --- a/clippy_lints/src/multiple_crate_versions.rs +++ /dev/null @@ -1,101 +0,0 @@ -//! lint on multiple versions of a crate being used - -use clippy_utils::diagnostics::span_lint; -use clippy_utils::is_lint_allowed; -use rustc_hir::def_id::LOCAL_CRATE; -use rustc_hir::CRATE_HIR_ID; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::source_map::DUMMY_SP; - -use cargo_metadata::{DependencyKind, Node, Package, PackageId}; -use if_chain::if_chain; -use itertools::Itertools; - -declare_clippy_lint! { - /// ### What it does - /// Checks to see if multiple versions of a crate are being - /// used. - /// - /// ### Why is this bad? - /// This bloats the size of targets, and can lead to - /// confusing error messages when structs or traits are used interchangeably - /// between different versions of a crate. - /// - /// ### Known problems - /// Because this can be caused purely by the dependencies - /// themselves, it's not always possible to fix this issue. - /// - /// ### Example - /// ```toml - /// # This will pull in both winapi v0.3.x and v0.2.x, triggering a warning. - /// [dependencies] - /// ctrlc = "=3.1.0" - /// ansi_term = "=0.11.0" - /// ``` - #[clippy::version = "pre 1.29.0"] - pub MULTIPLE_CRATE_VERSIONS, - cargo, - "multiple versions of the same crate being used" -} - -declare_lint_pass!(MultipleCrateVersions => [MULTIPLE_CRATE_VERSIONS]); - -impl LateLintPass<'_> for MultipleCrateVersions { - fn check_crate(&mut self, cx: &LateContext<'_>) { - if is_lint_allowed(cx, MULTIPLE_CRATE_VERSIONS, CRATE_HIR_ID) { - return; - } - - let metadata = unwrap_cargo_metadata!(cx, MULTIPLE_CRATE_VERSIONS, true); - let local_name = cx.tcx.crate_name(LOCAL_CRATE); - let mut packages = metadata.packages; - packages.sort_by(|a, b| a.name.cmp(&b.name)); - - if_chain! { - if let Some(resolve) = &metadata.resolve; - if let Some(local_id) = packages - .iter() - .find_map(|p| if p.name == local_name.as_str() { Some(&p.id) } else { None }); - then { - for (name, group) in &packages.iter().group_by(|p| p.name.clone()) { - let group: Vec<&Package> = group.collect(); - - if group.len() <= 1 { - continue; - } - - if group.iter().all(|p| is_normal_dep(&resolve.nodes, local_id, &p.id)) { - let mut versions: Vec<_> = group.into_iter().map(|p| &p.version).collect(); - versions.sort(); - let versions = versions.iter().join(", "); - - span_lint( - cx, - MULTIPLE_CRATE_VERSIONS, - DUMMY_SP, - &format!("multiple versions for dependency `{}`: {}", name, versions), - ); - } - } - } - } - } -} - -fn is_normal_dep(nodes: &[Node], local_id: &PackageId, dep_id: &PackageId) -> bool { - fn depends_on(node: &Node, dep_id: &PackageId) -> bool { - node.deps.iter().any(|dep| { - dep.pkg == *dep_id - && dep - .dep_kinds - .iter() - .any(|info| matches!(info.kind, DependencyKind::Normal)) - }) - } - - nodes - .iter() - .filter(|node| depends_on(node, dep_id)) - .any(|node| node.id == *local_id || is_normal_dep(nodes, local_id, &node.id)) -} diff --git a/clippy_lints/src/mutable_debug_assertion.rs b/clippy_lints/src/mutable_debug_assertion.rs index cd1bc2023..4ba68c8ea 100644 --- a/clippy_lints/src/mutable_debug_assertion.rs +++ b/clippy_lints/src/mutable_debug_assertion.rs @@ -92,10 +92,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/new_without_default.rs b/clippy_lints/src/new_without_default.rs index f86af7a7b..47121ad84 100644 --- a/clippy_lints/src/new_without_default.rs +++ b/clippy_lints/src/new_without_default.rs @@ -13,7 +13,7 @@ use rustc_span::sym; declare_clippy_lint! { /// ### What it does - /// Checks for types with a `fn new() -> Self` method and no + /// Checks for public types with a `pub fn new() -> Self` method and no /// implementation of /// [`Default`](https://doc.rust-lang.org/std/default/trait.Default.html). /// @@ -24,10 +24,10 @@ declare_clippy_lint! { /// /// ### Example /// ```ignore - /// struct Foo(Bar); + /// pub struct Foo(Bar); /// /// impl Foo { - /// fn new() -> Self { + /// pub fn new() -> Self { /// Foo(Bar::new()) /// } /// } @@ -36,7 +36,7 @@ declare_clippy_lint! { /// To fix the lint, add a `Default` implementation that delegates to `new`: /// /// ```ignore - /// struct Foo(Bar); + /// pub struct Foo(Bar); /// /// impl Default for Foo { /// fn default() -> Self { @@ -47,7 +47,7 @@ declare_clippy_lint! { #[clippy::version = "pre 1.29.0"] pub NEW_WITHOUT_DEFAULT, style, - "`fn new() -> Self` method without `Default` implementation" + "`pub fn new() -> Self` method without `Default` implementation" } #[derive(Clone, Default)] diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index be7610f36..e213c2087 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -330,7 +330,7 @@ fn check_range_zip_with_len(cx: &LateContext<'_>, path: &PathSegment<'_>, args: // `.iter()` and `.len()` called on same `Path` if let ExprKind::Path(QPath::Resolved(_, iter_path)) = iter_args[0].kind; if let ExprKind::Path(QPath::Resolved(_, len_path)) = len_args[0].kind; - if SpanlessEq::new(cx).eq_path_segments(&iter_path.segments, &len_path.segments); + if SpanlessEq::new(cx).eq_path_segments(iter_path.segments, len_path.segments); then { span_lint(cx, RANGE_ZIP_WITH_LEN, diff --git a/clippy_lints/src/recursive_format_impl.rs b/clippy_lints/src/recursive_format_impl.rs new file mode 100644 index 000000000..5bb974074 --- /dev/null +++ b/clippy_lints/src/recursive_format_impl.rs @@ -0,0 +1,190 @@ +use clippy_utils::diagnostics::span_lint; +use clippy_utils::macros::{is_format_macro, root_macro_call_first_node, FormatArgsArg, FormatArgsExpn}; +use clippy_utils::{is_diag_trait_item, path_to_local, peel_ref_operators}; +use if_chain::if_chain; +use rustc_hir::{Expr, ExprKind, Impl, Item, ItemKind, QPath}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::{sym, symbol::kw, Symbol}; + +#[derive(Clone, Copy)] +enum FormatTrait { + Debug, + Display, +} + +impl FormatTrait { + fn name(self) -> Symbol { + match self { + FormatTrait::Debug => sym::Debug, + FormatTrait::Display => sym::Display, + } + } +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for format trait implementations (e.g. `Display`) with a recursive call to itself + /// which uses `self` as a parameter. + /// This is typically done indirectly with the `write!` macro or with `to_string()`. + /// + /// ### Why is this bad? + /// This will lead to infinite recursion and a stack overflow. + /// + /// ### Example + /// + /// ```rust + /// use std::fmt; + /// + /// struct Structure(i32); + /// impl fmt::Display for Structure { + /// fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + /// write!(f, "{}", self.to_string()) + /// } + /// } + /// + /// ``` + /// Use instead: + /// ```rust + /// use std::fmt; + /// + /// struct Structure(i32); + /// impl fmt::Display for Structure { + /// fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + /// write!(f, "{}", self.0) + /// } + /// } + /// ``` + #[clippy::version = "1.48.0"] + pub RECURSIVE_FORMAT_IMPL, + correctness, + "Format trait method called while implementing the same Format trait" +} + +#[derive(Default)] +pub struct RecursiveFormatImpl { + // Whether we are inside Display or Debug trait impl - None for neither + format_trait_impl: Option, +} + +impl RecursiveFormatImpl { + pub fn new() -> Self { + Self { + format_trait_impl: None, + } + } +} + +impl_lint_pass!(RecursiveFormatImpl => [RECURSIVE_FORMAT_IMPL]); + +impl<'tcx> LateLintPass<'tcx> for RecursiveFormatImpl { + fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + if let Some(format_trait_impl) = is_format_trait_impl(cx, item) { + self.format_trait_impl = Some(format_trait_impl); + } + } + + fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + // Assume no nested Impl of Debug and Display within eachother + if is_format_trait_impl(cx, item).is_some() { + self.format_trait_impl = None; + } + } + + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + match self.format_trait_impl { + Some(FormatTrait::Display) => { + check_to_string_in_display(cx, expr); + check_self_in_format_args(cx, expr, FormatTrait::Display); + }, + Some(FormatTrait::Debug) => { + check_self_in_format_args(cx, expr, FormatTrait::Debug); + }, + None => {}, + } + } +} + +fn check_to_string_in_display(cx: &LateContext<'_>, expr: &Expr<'_>) { + if_chain! { + // Get the hir_id of the object we are calling the method on + if let ExprKind::MethodCall(path, [ref self_arg, ..], _) = expr.kind; + // Is the method to_string() ? + if path.ident.name == sym!(to_string); + // Is the method a part of the ToString trait? (i.e. not to_string() implemented + // separately) + if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); + if is_diag_trait_item(cx, expr_def_id, sym::ToString); + // Is the method is called on self + if let ExprKind::Path(QPath::Resolved(_, path)) = self_arg.kind; + if let [segment] = path.segments; + if segment.ident.name == kw::SelfLower; + then { + span_lint( + cx, + RECURSIVE_FORMAT_IMPL, + expr.span, + "using `self.to_string` in `fmt::Display` implementation will cause infinite recursion", + ); + } + } +} + +fn check_self_in_format_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, impl_trait: FormatTrait) { + // Check each arg in format calls - do we ever use Display on self (directly or via deref)? + if_chain! { + if let Some(outer_macro) = root_macro_call_first_node(cx, expr); + if let macro_def_id = outer_macro.def_id; + if let Some(format_args) = FormatArgsExpn::find_nested(cx, expr, outer_macro.expn); + if is_format_macro(cx, macro_def_id); + if let Some(args) = format_args.args(); + then { + for arg in args { + if arg.format_trait != impl_trait.name() { + continue; + } + check_format_arg_self(cx, expr, &arg, impl_trait); + } + } + } +} + +fn check_format_arg_self(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &FormatArgsArg<'_>, impl_trait: FormatTrait) { + // Handle multiple dereferencing of references e.g. &&self + // Handle dereference of &self -> self that is equivalent (i.e. via *self in fmt() impl) + // Since the argument to fmt is itself a reference: &self + let reference = peel_ref_operators(cx, arg.value); + let map = cx.tcx.hir(); + // Is the reference self? + let symbol_ident = impl_trait.name().to_ident_string(); + if path_to_local(reference).map(|x| map.name(x)) == Some(kw::SelfLower) { + span_lint( + cx, + RECURSIVE_FORMAT_IMPL, + expr.span, + &format!( + "using `self` as `{}` in `impl {}` will cause infinite recursion", + &symbol_ident, &symbol_ident + ), + ); + } +} + +fn is_format_trait_impl(cx: &LateContext<'_>, item: &Item<'_>) -> Option { + if_chain! { + // Are we at an Impl? + if let ItemKind::Impl(Impl { of_trait: Some(trait_ref), .. }) = &item.kind; + if let Some(did) = trait_ref.trait_def_id(); + if let Some(name) = cx.tcx.get_diagnostic_name(did); + then { + // Is Impl for Debug or Display? + match name { + sym::Debug => Some(FormatTrait::Debug), + sym::Display => Some(FormatTrait::Display), + _ => None, + } + } else { + None + } + } +} diff --git a/clippy_lints/src/redundant_slicing.rs b/clippy_lints/src/redundant_slicing.rs index cd3aee556..25a9072ef 100644 --- a/clippy_lints/src/redundant_slicing.rs +++ b/clippy_lints/src/redundant_slicing.rs @@ -1,11 +1,14 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::get_parent_expr; use clippy_utils::source::snippet_with_context; -use clippy_utils::ty::is_type_lang_item; +use clippy_utils::ty::{is_type_lang_item, peel_mid_ty_refs}; use if_chain::if_chain; +use rustc_ast::util::parser::PREC_PREFIX; use rustc_errors::Applicability; use rustc_hir::{BorrowKind, Expr, ExprKind, LangItem, Mutability}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, Lint}; +use rustc_middle::ty::adjustment::{Adjust, AutoBorrow, AutoBorrowMutability}; +use rustc_middle::ty::subst::GenericArg; use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { @@ -39,7 +42,34 @@ declare_clippy_lint! { "redundant slicing of the whole range of a type" } -declare_lint_pass!(RedundantSlicing => [REDUNDANT_SLICING]); +declare_clippy_lint! { + /// ### What it does + /// Checks for slicing expressions which are equivalent to dereferencing the + /// value. + /// + /// ### Why is this bad? + /// Some people may prefer to dereference rather than slice. + /// + /// ### Example + /// ```rust + /// let vec = vec![1, 2, 3]; + /// let slice = &vec[..]; + /// ``` + /// Use instead: + /// ```rust + /// let vec = vec![1, 2, 3]; + /// let slice = &*vec; + /// ``` + #[clippy::version = "1.60.0"] + pub DEREF_BY_SLICING, + restriction, + "slicing instead of dereferencing" +} + +declare_lint_pass!(RedundantSlicing => [REDUNDANT_SLICING, DEREF_BY_SLICING]); + +static REDUNDANT_SLICING_LINT: (&Lint, &str) = (REDUNDANT_SLICING, "redundant slicing of the whole range"); +static DEREF_BY_SLICING_LINT: (&Lint, &str) = (DEREF_BY_SLICING, "slicing when dereferencing would work"); impl<'tcx> LateLintPass<'tcx> for RedundantSlicing { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { @@ -53,34 +83,84 @@ impl<'tcx> LateLintPass<'tcx> for RedundantSlicing { if addressee.span.ctxt() == ctxt; if let ExprKind::Index(indexed, range) = addressee.kind; if is_type_lang_item(cx, cx.typeck_results().expr_ty_adjusted(range), LangItem::RangeFull); - if cx.typeck_results().expr_ty(expr) == cx.typeck_results().expr_ty(indexed); then { + let (expr_ty, expr_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(expr)); + let (indexed_ty, indexed_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(indexed)); + let parent_expr = get_parent_expr(cx, expr); + let needs_parens_for_prefix = parent_expr.map_or(false, |parent| { + parent.precedence().order() > PREC_PREFIX + }); let mut app = Applicability::MachineApplicable; - let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0; - let (reborrow_str, help_str) = if mutability == Mutability::Mut { - // The slice was used to reborrow the mutable reference. - ("&mut *", "reborrow the original value instead") - } else if matches!( - get_parent_expr(cx, expr), - Some(Expr { - kind: ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _), - .. - }) - ) { - // The slice was used to make a temporary reference. - ("&*", "reborrow the original value instead") + let ((lint, msg), help, sugg) = if expr_ty == indexed_ty { + if expr_ref_count > indexed_ref_count { + // Indexing takes self by reference and can't return a reference to that + // reference as it's a local variable. The only way this could happen is if + // `self` contains a reference to the `Self` type. If this occurs then the + // lint no longer applies as it's essentially a field access, which is not + // redundant. + return; + } + let deref_count = indexed_ref_count - expr_ref_count; + + let (lint, reborrow_str, help_str) = if mutability == Mutability::Mut { + // The slice was used to reborrow the mutable reference. + (DEREF_BY_SLICING_LINT, "&mut *", "reborrow the original value instead") + } else if matches!( + parent_expr, + Some(Expr { + kind: ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _), + .. + }) + ) || cx.typeck_results().expr_adjustments(expr).first().map_or(false, |a| { + matches!(a.kind, Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Mut { .. }))) + }) { + // The slice was used to make a temporary reference. + (DEREF_BY_SLICING_LINT, "&*", "reborrow the original value instead") + } else if deref_count != 0 { + (DEREF_BY_SLICING_LINT, "", "dereference the original value instead") + } else { + (REDUNDANT_SLICING_LINT, "", "use the original value instead") + }; + + let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0; + let sugg = if (deref_count != 0 || !reborrow_str.is_empty()) && needs_parens_for_prefix { + format!("({}{}{})", reborrow_str, "*".repeat(deref_count), snip) + } else { + format!("{}{}{}", reborrow_str, "*".repeat(deref_count), snip) + }; + + (lint, help_str, sugg) + } else if let Some(target_id) = cx.tcx.lang_items().deref_target() { + if let Ok(deref_ty) = cx.tcx.try_normalize_erasing_regions( + cx.param_env, + cx.tcx.mk_projection(target_id, cx.tcx.mk_substs([GenericArg::from(indexed_ty)].into_iter())), + ) { + if deref_ty == expr_ty { + let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0; + let sugg = if needs_parens_for_prefix { + format!("(&{}{}*{})", mutability.prefix_str(), "*".repeat(indexed_ref_count), snip) + } else { + format!("&{}{}*{}", mutability.prefix_str(), "*".repeat(indexed_ref_count), snip) + }; + (DEREF_BY_SLICING_LINT, "dereference the original value instead", sugg) + } else { + return; + } + } else { + return; + } } else { - ("", "use the original value instead") + return; }; span_lint_and_sugg( cx, - REDUNDANT_SLICING, + lint, expr.span, - "redundant slicing of the whole range", - help_str, - format!("{}{}", reborrow_str, snip), + msg, + help, + sugg, app, ); } diff --git a/clippy_lints/src/to_string_in_display.rs b/clippy_lints/src/to_string_in_display.rs deleted file mode 100644 index 03060d78f..000000000 --- a/clippy_lints/src/to_string_in_display.rs +++ /dev/null @@ -1,123 +0,0 @@ -use clippy_utils::diagnostics::span_lint; -use clippy_utils::{is_diag_trait_item, match_def_path, path_to_local_id, paths}; -use if_chain::if_chain; -use rustc_hir::{Expr, ExprKind, HirId, Impl, ImplItem, ImplItemKind, Item, ItemKind}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::symbol::sym; - -declare_clippy_lint! { - /// ### What it does - /// Checks for uses of `to_string()` in `Display` traits. - /// - /// ### Why is this bad? - /// Usually `to_string` is implemented indirectly - /// via `Display`. Hence using it while implementing `Display` would - /// lead to infinite recursion. - /// - /// ### Example - /// - /// ```rust - /// use std::fmt; - /// - /// struct Structure(i32); - /// impl fmt::Display for Structure { - /// fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - /// write!(f, "{}", self.to_string()) - /// } - /// } - /// - /// ``` - /// Use instead: - /// ```rust - /// use std::fmt; - /// - /// struct Structure(i32); - /// impl fmt::Display for Structure { - /// fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - /// write!(f, "{}", self.0) - /// } - /// } - /// ``` - #[clippy::version = "1.48.0"] - pub TO_STRING_IN_DISPLAY, - correctness, - "`to_string` method used while implementing `Display` trait" -} - -#[derive(Default)] -pub struct ToStringInDisplay { - in_display_impl: bool, - self_hir_id: Option, -} - -impl ToStringInDisplay { - pub fn new() -> Self { - Self { - in_display_impl: false, - self_hir_id: None, - } - } -} - -impl_lint_pass!(ToStringInDisplay => [TO_STRING_IN_DISPLAY]); - -impl LateLintPass<'_> for ToStringInDisplay { - fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { - if is_display_impl(cx, item) { - self.in_display_impl = true; - } - } - - fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { - if is_display_impl(cx, item) { - self.in_display_impl = false; - self.self_hir_id = None; - } - } - - fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) { - if_chain! { - if self.in_display_impl; - if let ImplItemKind::Fn(.., body_id) = &impl_item.kind; - let body = cx.tcx.hir().body(*body_id); - if !body.params.is_empty(); - then { - let self_param = &body.params[0]; - self.self_hir_id = Some(self_param.pat.hir_id); - } - } - } - - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { - if_chain! { - if self.in_display_impl; - if let Some(self_hir_id) = self.self_hir_id; - if let ExprKind::MethodCall(path, [ref self_arg, ..], _) = expr.kind; - if path.ident.name == sym!(to_string); - if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); - if is_diag_trait_item(cx, expr_def_id, sym::ToString); - if path_to_local_id(self_arg, self_hir_id); - then { - span_lint( - cx, - TO_STRING_IN_DISPLAY, - expr.span, - "using `to_string` in `fmt::Display` implementation might lead to infinite recursion", - ); - } - } - } -} - -fn is_display_impl(cx: &LateContext<'_>, item: &Item<'_>) -> bool { - if_chain! { - if let ItemKind::Impl(Impl { of_trait: Some(trait_ref), .. }) = &item.kind; - if let Some(did) = trait_ref.trait_def_id(); - then { - match_def_path(cx, did, &paths::DISPLAY_TRAIT) - } else { - false - } - } -} diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index bca95b7f2..be9d538c3 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -98,8 +98,9 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds { if let WherePredicate::BoundPredicate(ref bound_predicate) = predicate; if !bound_predicate.span.from_expansion(); if let TyKind::Path(QPath::Resolved(_, Path { segments, .. })) = bound_predicate.bounded_ty.kind; - if let Some(PathSegment { res: Some(Res::SelfTy{ trait_: Some(def_id), alias_to: _ }), .. }) = segments.first(); - + if let Some(PathSegment { + res: Some(Res::SelfTy{ trait_: Some(def_id), alias_to: _ }), .. + }) = segments.first(); if let Some( Node::Item( Item { diff --git a/clippy_lints/src/transmute/mod.rs b/clippy_lints/src/transmute/mod.rs index 5e94ab6d0..22a8c53a5 100644 --- a/clippy_lints/src/transmute/mod.rs +++ b/clippy_lints/src/transmute/mod.rs @@ -358,7 +358,8 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for transmutes either to or from a type which does not have a defined representation. + /// Checks for transmutes between types which do not have a representation defined relative to + /// each other. /// /// ### Why is this bad? /// The results of such a transmute are not defined. @@ -376,7 +377,7 @@ declare_clippy_lint! { /// ``` #[clippy::version = "1.60.0"] pub TRANSMUTE_UNDEFINED_REPR, - nursery, + correctness, "transmute to or from a type with an undefined representation" } diff --git a/clippy_lints/src/transmute/transmute_undefined_repr.rs b/clippy_lints/src/transmute/transmute_undefined_repr.rs index a57c819cb..05eadab3e 100644 --- a/clippy_lints/src/transmute/transmute_undefined_repr.rs +++ b/clippy_lints/src/transmute/transmute_undefined_repr.rs @@ -1,5 +1,6 @@ use super::TRANSMUTE_UNDEFINED_REPR; use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::ty::is_c_void; use rustc_hir::Expr; use rustc_lint::LateContext; use rustc_middle::ty::subst::Subst; @@ -18,33 +19,55 @@ pub(super) fn check<'tcx>( while from_ty != to_ty { match reduce_refs(cx, e.span, from_ty, to_ty) { - ReducedTys::FromFatPtr { unsized_ty, .. } => { - span_lint_and_then( - cx, - TRANSMUTE_UNDEFINED_REPR, - e.span, - &format!("transmute from `{}` which has an undefined layout", from_ty_orig), - |diag| { - if from_ty_orig.peel_refs() != unsized_ty { - diag.note(&format!("the contained type `&{}` has an undefined layout", unsized_ty)); - } - }, - ); - return true; + ReducedTys::FromFatPtr { + unsized_ty, + to_ty: to_sub_ty, + } => match reduce_ty(cx, to_sub_ty) { + ReducedTy::IntArray | ReducedTy::TypeErasure => break, + ReducedTy::Ref(to_sub_ty) => { + from_ty = unsized_ty; + to_ty = to_sub_ty; + continue; + }, + _ => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute from `{}` which has an undefined layout", from_ty_orig), + |diag| { + if from_ty_orig.peel_refs() != unsized_ty { + diag.note(&format!("the contained type `&{}` has an undefined layout", unsized_ty)); + } + }, + ); + return true; + }, }, - ReducedTys::ToFatPtr { unsized_ty, .. } => { - span_lint_and_then( - cx, - TRANSMUTE_UNDEFINED_REPR, - e.span, - &format!("transmute to `{}` which has an undefined layout", to_ty_orig), - |diag| { - if to_ty_orig.peel_refs() != unsized_ty { - diag.note(&format!("the contained type `&{}` has an undefined layout", unsized_ty)); - } - }, - ); - return true; + ReducedTys::ToFatPtr { + unsized_ty, + from_ty: from_sub_ty, + } => match reduce_ty(cx, from_sub_ty) { + ReducedTy::IntArray | ReducedTy::TypeErasure => break, + ReducedTy::Ref(from_sub_ty) => { + from_ty = from_sub_ty; + to_ty = unsized_ty; + continue; + }, + _ => { + span_lint_and_then( + cx, + TRANSMUTE_UNDEFINED_REPR, + e.span, + &format!("transmute to `{}` which has an undefined layout", to_ty_orig), + |diag| { + if to_ty_orig.peel_refs() != unsized_ty { + diag.note(&format!("the contained type `&{}` has an undefined layout", unsized_ty)); + } + }, + ); + return true; + }, }, ReducedTys::ToPtr { from_ty: from_sub_ty, @@ -100,7 +123,8 @@ pub(super) fn check<'tcx>( from_ty: from_sub_ty, to_ty: to_sub_ty, } => match (reduce_ty(cx, from_sub_ty), reduce_ty(cx, to_sub_ty)) { - (ReducedTy::IntArray, _) | (_, ReducedTy::IntArray) => return false, + (ReducedTy::IntArray | ReducedTy::TypeErasure, _) + | (_, ReducedTy::IntArray | ReducedTy::TypeErasure) => return false, (ReducedTy::UnorderedFields(from_ty), ReducedTy::UnorderedFields(to_ty)) if from_ty != to_ty => { span_lint_and_then( cx, @@ -184,13 +208,14 @@ pub(super) fn check<'tcx>( } enum ReducedTys<'tcx> { - FromFatPtr { unsized_ty: Ty<'tcx> }, - ToFatPtr { unsized_ty: Ty<'tcx> }, + FromFatPtr { unsized_ty: Ty<'tcx>, to_ty: Ty<'tcx> }, + ToFatPtr { unsized_ty: Ty<'tcx>, from_ty: Ty<'tcx> }, ToPtr { from_ty: Ty<'tcx>, to_ty: Ty<'tcx> }, FromPtr { from_ty: Ty<'tcx>, to_ty: Ty<'tcx> }, Other { from_ty: Ty<'tcx>, to_ty: Ty<'tcx> }, } +/// Remove references so long as both types are references. fn reduce_refs<'tcx>( cx: &LateContext<'tcx>, span: Span, @@ -200,27 +225,27 @@ fn reduce_refs<'tcx>( loop { return match (from_ty.kind(), to_ty.kind()) { ( - &ty::Ref(_, from_sub_ty, _) | &ty::RawPtr(TypeAndMut { ty: from_sub_ty, .. }), - &ty::Ref(_, to_sub_ty, _) | &ty::RawPtr(TypeAndMut { ty: to_sub_ty, .. }), + &(ty::Ref(_, from_sub_ty, _) | ty::RawPtr(TypeAndMut { ty: from_sub_ty, .. })), + &(ty::Ref(_, to_sub_ty, _) | ty::RawPtr(TypeAndMut { ty: to_sub_ty, .. })), ) => { from_ty = from_sub_ty; to_ty = to_sub_ty; continue; }, - (&ty::Ref(_, unsized_ty, _) | &ty::RawPtr(TypeAndMut { ty: unsized_ty, .. }), _) + (&(ty::Ref(_, unsized_ty, _) | ty::RawPtr(TypeAndMut { ty: unsized_ty, .. })), _) if !unsized_ty.is_sized(cx.tcx.at(span), cx.param_env) => { - ReducedTys::FromFatPtr { unsized_ty } + ReducedTys::FromFatPtr { unsized_ty, to_ty } }, - (_, &ty::Ref(_, unsized_ty, _) | &ty::RawPtr(TypeAndMut { ty: unsized_ty, .. })) + (_, &(ty::Ref(_, unsized_ty, _) | ty::RawPtr(TypeAndMut { ty: unsized_ty, .. }))) if !unsized_ty.is_sized(cx.tcx.at(span), cx.param_env) => { - ReducedTys::ToFatPtr { unsized_ty } + ReducedTys::ToFatPtr { unsized_ty, from_ty } }, - (&ty::Ref(_, from_ty, _) | &ty::RawPtr(TypeAndMut { ty: from_ty, .. }), _) => { + (&(ty::Ref(_, from_ty, _) | ty::RawPtr(TypeAndMut { ty: from_ty, .. })), _) => { ReducedTys::FromPtr { from_ty, to_ty } }, - (_, &ty::Ref(_, to_ty, _) | &ty::RawPtr(TypeAndMut { ty: to_ty, .. })) => { + (_, &(ty::Ref(_, to_ty, _) | ty::RawPtr(TypeAndMut { ty: to_ty, .. }))) => { ReducedTys::ToPtr { from_ty, to_ty } }, _ => ReducedTys::Other { from_ty, to_ty }, @@ -229,13 +254,23 @@ fn reduce_refs<'tcx>( } enum ReducedTy<'tcx> { + /// The type can be used for type erasure. + TypeErasure, + /// The type is a struct containing either zero non-zero sized fields, or multiple non-zero + /// sized fields with a defined order. OrderedFields(Ty<'tcx>), + /// The type is a struct containing multiple non-zero sized fields with no defined order. UnorderedFields(Ty<'tcx>), + /// The type is a reference to the contained type. Ref(Ty<'tcx>), - Other(Ty<'tcx>), + /// The type is an array of a primitive integer type. These can be used as storage for a value + /// of another type. IntArray, + /// Any other type. + Other(Ty<'tcx>), } +/// Reduce structs containing a single non-zero sized field to it's contained type. fn reduce_ty<'tcx>(cx: &LateContext<'tcx>, mut ty: Ty<'tcx>) -> ReducedTy<'tcx> { loop { ty = cx.tcx.try_normalize_erasing_regions(cx.param_env, ty).unwrap_or(ty); @@ -245,8 +280,9 @@ fn reduce_ty<'tcx>(cx: &LateContext<'tcx>, mut ty: Ty<'tcx>) -> ReducedTy<'tcx> ty = sub_ty; continue; }, + ty::Tuple(args) if args.is_empty() => ReducedTy::TypeErasure, ty::Tuple(args) => { - let Some(sized_ty) = args.iter().find(|&ty| !is_zero_sized_ty(cx, ty)) else { + let Some(sized_ty) = args.iter().find(|&ty| !is_zero_sized_ty(cx, ty)) else { return ReducedTy::OrderedFields(ty); }; if args.iter().all(|ty| is_zero_sized_ty(cx, ty)) { @@ -256,24 +292,30 @@ fn reduce_ty<'tcx>(cx: &LateContext<'tcx>, mut ty: Ty<'tcx>) -> ReducedTy<'tcx> ReducedTy::UnorderedFields(ty) }, ty::Adt(def, substs) if def.is_struct() => { - if def.repr.inhibit_struct_field_reordering_opt() { - return ReducedTy::OrderedFields(ty); - } let mut iter = def .non_enum_variant() .fields .iter() .map(|f| cx.tcx.type_of(f.did).subst(cx.tcx, substs)); - let Some(sized_ty) = iter.find(|ty| !is_zero_sized_ty(cx, *ty)) else { - return ReducedTy::OrderedFields(ty); + let Some(sized_ty) = iter.find(|&ty| !is_zero_sized_ty(cx, ty)) else { + return ReducedTy::TypeErasure; }; if iter.all(|ty| is_zero_sized_ty(cx, ty)) { ty = sized_ty; continue; } - ReducedTy::UnorderedFields(ty) + if def.repr.inhibit_struct_field_reordering_opt() { + ReducedTy::OrderedFields(ty) + } else { + ReducedTy::UnorderedFields(ty) + } }, - ty::Ref(..) | ty::RawPtr(_) => ReducedTy::Ref(ty), + ty::Adt(def, _) if def.is_enum() && (def.variants.is_empty() || is_c_void(cx, ty)) => { + ReducedTy::TypeErasure + }, + ty::Foreign(_) => ReducedTy::TypeErasure, + ty::Ref(_, ty, _) => ReducedTy::Ref(ty), + ty::RawPtr(ty) => ReducedTy::Ref(ty.ty), _ => ReducedTy::Other(ty), }; } diff --git a/clippy_lints/src/types/borrowed_box.rs b/clippy_lints/src/types/borrowed_box.rs index 63ad65b8a..7c0690629 100644 --- a/clippy_lints/src/types/borrowed_box.rs +++ b/clippy_lints/src/types/borrowed_box.rs @@ -1,11 +1,11 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet; -use clippy_utils::{match_def_path, paths}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{self as hir, GenericArg, GenericBounds, GenericParamKind}; use rustc_hir::{HirId, Lifetime, MutTy, Mutability, Node, QPath, TyKind}; use rustc_lint::LateContext; +use rustc_span::sym; use super::BORROWED_BOX; @@ -89,7 +89,7 @@ fn is_any_trait(cx: &LateContext<'_>, t: &hir::Ty<'_>) -> bool { if let Some(trait_did) = traits[0].trait_ref.trait_def_id(); // Only Send/Sync can be used as additional traits, so it is enough to // check only the first trait. - if match_def_path(cx, trait_did, &paths::ANY_TRAIT); + if cx.tcx.is_diagnostic_item(sym::Any, trait_did); then { return true; } diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index dc0f515bf..4433d5f5b 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -1305,7 +1305,7 @@ fn if_chain_local_span(cx: &LateContext<'_>, local: &Local<'_>, if_chain_span: S } span.adjust(if_chain_span.ctxt().outer_expn()); let sm = cx.sess().source_map(); - let span = sm.span_extend_to_prev_str(span, "let", false); + let span = sm.span_extend_to_prev_str(span, "let", false, true).unwrap_or(span); let span = sm.span_extend_to_next_char(span, ';', false); Span::new( span.lo() - BytePos(3), diff --git a/clippy_lints/src/wildcard_dependencies.rs b/clippy_lints/src/wildcard_dependencies.rs deleted file mode 100644 index 80d7b8a1b..000000000 --- a/clippy_lints/src/wildcard_dependencies.rs +++ /dev/null @@ -1,57 +0,0 @@ -use clippy_utils::{diagnostics::span_lint, is_lint_allowed}; -use rustc_hir::hir_id::CRATE_HIR_ID; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::source_map::DUMMY_SP; - -use if_chain::if_chain; - -declare_clippy_lint! { - /// ### What it does - /// Checks for wildcard dependencies in the `Cargo.toml`. - /// - /// ### Why is this bad? - /// [As the edition guide says](https://rust-lang-nursery.github.io/edition-guide/rust-2018/cargo-and-crates-io/crates-io-disallows-wildcard-dependencies.html), - /// it is highly unlikely that you work with any possible version of your dependency, - /// and wildcard dependencies would cause unnecessary breakage in the ecosystem. - /// - /// ### Example - /// ```toml - /// [dependencies] - /// regex = "*" - /// ``` - #[clippy::version = "1.32.0"] - pub WILDCARD_DEPENDENCIES, - cargo, - "wildcard dependencies being used" -} - -declare_lint_pass!(WildcardDependencies => [WILDCARD_DEPENDENCIES]); - -impl LateLintPass<'_> for WildcardDependencies { - fn check_crate(&mut self, cx: &LateContext<'_>) { - if is_lint_allowed(cx, WILDCARD_DEPENDENCIES, CRATE_HIR_ID) { - return; - } - - let metadata = unwrap_cargo_metadata!(cx, WILDCARD_DEPENDENCIES, false); - - for dep in &metadata.packages[0].dependencies { - // VersionReq::any() does not work - if_chain! { - if let Ok(wildcard_ver) = semver::VersionReq::parse("*"); - if let Some(ref source) = dep.source; - if !source.starts_with("git"); - if dep.req == wildcard_ver; - then { - span_lint( - cx, - WILDCARD_DEPENDENCIES, - DUMMY_SP, - &format!("wildcard dependency for `{}`", dep.name), - ); - } - } - } - } -} diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 4bb401273..397783e30 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1,6 +1,7 @@ #![feature(box_patterns)] #![feature(control_flow_enum)] #![feature(let_else)] +#![feature(let_chains)] #![feature(once_cell)] #![feature(rustc_private)] #![recursion_limit = "512"] @@ -2042,24 +2043,6 @@ pub fn peel_ref_operators<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir> expr } -#[macro_export] -macro_rules! unwrap_cargo_metadata { - ($cx: ident, $lint: ident, $deps: expr) => {{ - let mut command = cargo_metadata::MetadataCommand::new(); - if !$deps { - command.no_deps(); - } - - match command.exec() { - Ok(metadata) => metadata, - Err(err) => { - span_lint($cx, $lint, DUMMY_SP, &format!("could not read cargo metadata: {}", err)); - return; - }, - } - }}; -} - pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool { if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind { if let Res::Def(_, def_id) = path.res { diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs index 5a76ac233..256f884ae 100644 --- a/clippy_utils/src/macros.rs +++ b/clippy_utils/src/macros.rs @@ -1,6 +1,7 @@ #![allow(clippy::similar_names)] // `expr` and `expn` use crate::visitors::expr_visitor_no_bodies; +use crate::{match_def_path, paths}; use arrayvec::ArrayVec; use if_chain::if_chain; @@ -13,6 +14,31 @@ use rustc_span::hygiene::{self, MacroKind, SyntaxContext}; use rustc_span::{sym, ExpnData, ExpnId, ExpnKind, Span, Symbol}; use std::ops::ControlFlow; +const FORMAT_MACRO_PATHS: &[&[&str]] = &[ + &paths::FORMAT_ARGS_MACRO, + &paths::ASSERT_EQ_MACRO, + &paths::ASSERT_MACRO, + &paths::ASSERT_NE_MACRO, + &paths::EPRINT_MACRO, + &paths::EPRINTLN_MACRO, + &paths::PRINT_MACRO, + &paths::PRINTLN_MACRO, + &paths::WRITE_MACRO, + &paths::WRITELN_MACRO, +]; + +const FORMAT_MACRO_DIAG_ITEMS: &[Symbol] = &[sym::format_macro, sym::std_panic_macro]; + +/// Returns true if a given Macro `DefId` is a format macro (e.g. `println!`) +pub fn is_format_macro(cx: &LateContext<'_>, macro_def_id: DefId) -> bool { + FORMAT_MACRO_PATHS + .iter() + .any(|path| match_def_path(cx, macro_def_id, path)) + || FORMAT_MACRO_DIAG_ITEMS + .iter() + .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, macro_def_id)) +} + /// A macro call, like `vec![1, 2, 3]`. /// /// Use `tcx.item_name(macro_call.def_id)` to get the macro name. diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 288c56e9f..b54bd3a4f 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -4,7 +4,6 @@ //! Whenever possible, please consider diagnostic items over hardcoded paths. //! See for more information. -pub const ANY_TRAIT: [&str; 3] = ["core", "any", "Any"]; #[cfg(feature = "internal")] pub const APPLICABILITY: [&str; 2] = ["rustc_lint_defs", "Applicability"]; #[cfg(feature = "internal")] @@ -32,19 +31,16 @@ pub const BTREEMAP_CONTAINS_KEY: [&str; 6] = ["alloc", "collections", "btree", " pub const BTREEMAP_ENTRY: [&str; 6] = ["alloc", "collections", "btree", "map", "entry", "Entry"]; pub const BTREEMAP_INSERT: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "insert"]; pub const CLONE_TRAIT_METHOD: [&str; 4] = ["core", "clone", "Clone", "clone"]; -pub const CMP_MAX: [&str; 3] = ["core", "cmp", "max"]; -pub const CMP_MIN: [&str; 3] = ["core", "cmp", "min"]; pub const COW: [&str; 3] = ["alloc", "borrow", "Cow"]; pub const CSTRING_AS_C_STR: [&str; 5] = ["std", "ffi", "c_str", "CString", "as_c_str"]; +pub const DEBUG_TRAIT: [&str; 3] = ["core", "fmt", "Debug"]; pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"]; pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"]; /// Preferably use the diagnostic item `sym::deref_method` where possible pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"]; pub const DIR_BUILDER: [&str; 3] = ["std", "fs", "DirBuilder"]; pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"]; -pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"]; pub const DROP: [&str; 3] = ["core", "mem", "drop"]; -pub const DURATION: [&str; 3] = ["core", "time", "Duration"]; #[cfg(feature = "internal")] pub const EARLY_CONTEXT: [&str; 2] = ["rustc_lint", "EarlyContext"]; #[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros @@ -59,7 +55,6 @@ pub const FILE_TYPE: [&str; 3] = ["std", "fs", "FileType"]; #[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros pub const FORMAT_ARGS_MACRO: [&str; 4] = ["core", "macros", "builtin", "format_args"]; pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"]; -pub const FROM_ITERATOR: [&str; 5] = ["core", "iter", "traits", "collect", "FromIterator"]; pub const FROM_ITERATOR_METHOD: [&str; 6] = ["core", "iter", "traits", "collect", "FromIterator", "from_iter"]; pub const FROM_STR_METHOD: [&str; 5] = ["core", "str", "traits", "FromStr", "from_str"]; pub const FUTURE_FROM_GENERATOR: [&str; 3] = ["core", "future", "from_generator"]; @@ -67,7 +62,6 @@ pub const FUTURE_FROM_GENERATOR: [&str; 3] = ["core", "future", "from_generator" pub const FUTURES_IO_ASYNCREADEXT: [&str; 3] = ["futures_util", "io", "AsyncReadExt"]; #[allow(clippy::invalid_paths)] // internal lints do not know about all external crates pub const FUTURES_IO_ASYNCWRITEEXT: [&str; 3] = ["futures_util", "io", "AsyncWriteExt"]; -pub const HASH: [&str; 3] = ["core", "hash", "Hash"]; pub const HASHMAP_CONTAINS_KEY: [&str; 6] = ["std", "collections", "hash", "map", "HashMap", "contains_key"]; pub const HASHMAP_ENTRY: [&str; 5] = ["std", "collections", "hash", "map", "Entry"]; pub const HASHMAP_INSERT: [&str; 6] = ["std", "collections", "hash", "map", "HashMap", "insert"]; @@ -111,9 +105,9 @@ pub const OS_STRING_AS_OS_STR: [&str; 5] = ["std", "ffi", "os_str", "OsString", pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to_os_string"]; pub const PARKING_LOT_RAWMUTEX: [&str; 3] = ["parking_lot", "raw_mutex", "RawMutex"]; pub const PARKING_LOT_RAWRWLOCK: [&str; 3] = ["parking_lot", "raw_rwlock", "RawRwLock"]; -pub const PARKING_LOT_MUTEX_GUARD: [&str; 2] = ["parking_lot", "MutexGuard"]; -pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 2] = ["parking_lot", "RwLockReadGuard"]; -pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 2] = ["parking_lot", "RwLockWriteGuard"]; +pub const PARKING_LOT_MUTEX_GUARD: [&str; 3] = ["lock_api", "mutex", "MutexGuard"]; +pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 3] = ["lock_api", "rwlock", "RwLockReadGuard"]; +pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 3] = ["lock_api", "rwlock", "RwLockWriteGuard"]; pub const PATH_BUF_AS_PATH: [&str; 4] = ["std", "path", "PathBuf", "as_path"]; pub const PATH_TO_PATH_BUF: [&str; 4] = ["std", "path", "Path", "to_path_buf"]; pub const PERMISSIONS: [&str; 3] = ["std", "fs", "Permissions"]; diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index 0d39226d9..0646d1524 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -10,12 +10,14 @@ use rustc_hir::def_id::DefId; use rustc_hir::{Expr, TyKind, Unsafety}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::LateContext; +use rustc_middle::mir::interpret::{ConstValue, Scalar}; use rustc_middle::ty::subst::{GenericArg, GenericArgKind, Subst}; use rustc_middle::ty::{ - self, AdtDef, Binder, FnSig, IntTy, Predicate, PredicateKind, Ty, TyCtxt, TypeFoldable, UintTy, + self, AdtDef, Binder, FnSig, IntTy, Predicate, PredicateKind, Ty, TyCtxt, TypeFoldable, UintTy, VariantDiscr, }; use rustc_span::symbol::Ident; use rustc_span::{sym, Span, Symbol, DUMMY_SP}; +use rustc_target::abi::{Size, VariantIdx}; use rustc_trait_selection::infer::InferCtxtExt; use rustc_trait_selection::traits::query::normalize::AtExt; use std::iter; @@ -515,3 +517,72 @@ pub fn expr_sig<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option for EnumValue { + type Output = Self; + fn add(self, n: u32) -> Self::Output { + match self { + Self::Unsigned(x) => Self::Unsigned(x + u128::from(n)), + Self::Signed(x) => Self::Signed(x + i128::from(n)), + } + } +} + +/// Attempts to read the given constant as though it were an an enum value. +#[allow(clippy::cast_possible_truncation, clippy::cast_possible_wrap)] +pub fn read_explicit_enum_value(tcx: TyCtxt<'_>, id: DefId) -> Option { + if let Ok(ConstValue::Scalar(Scalar::Int(value))) = tcx.const_eval_poly(id) { + match tcx.type_of(id).kind() { + ty::Int(_) => Some(EnumValue::Signed(match value.size().bytes() { + 1 => i128::from(value.assert_bits(Size::from_bytes(1)) as u8 as i8), + 2 => i128::from(value.assert_bits(Size::from_bytes(2)) as u16 as i16), + 4 => i128::from(value.assert_bits(Size::from_bytes(4)) as u32 as i32), + 8 => i128::from(value.assert_bits(Size::from_bytes(8)) as u64 as i64), + 16 => value.assert_bits(Size::from_bytes(16)) as i128, + _ => return None, + })), + ty::Uint(_) => Some(EnumValue::Unsigned(match value.size().bytes() { + 1 => value.assert_bits(Size::from_bytes(1)), + 2 => value.assert_bits(Size::from_bytes(2)), + 4 => value.assert_bits(Size::from_bytes(4)), + 8 => value.assert_bits(Size::from_bytes(8)), + 16 => value.assert_bits(Size::from_bytes(16)), + _ => return None, + })), + _ => None, + } + } else { + None + } +} + +/// Gets the value of the given variant. +pub fn get_discriminant_value(tcx: TyCtxt<'_>, adt: &'_ AdtDef, i: VariantIdx) -> EnumValue { + let variant = &adt.variants[i]; + match variant.discr { + VariantDiscr::Explicit(id) => read_explicit_enum_value(tcx, id).unwrap(), + VariantDiscr::Relative(x) => match adt.variants[(i.as_usize() - x as usize).into()].discr { + VariantDiscr::Explicit(id) => read_explicit_enum_value(tcx, id).unwrap() + x, + VariantDiscr::Relative(_) => EnumValue::Unsigned(x.into()), + }, + } +} + +/// Check if the given type is either `core::ffi::c_void`, `std::os::raw::c_void`, or one of the +/// platform specific `libc::::c_void` types in libc. +pub fn is_c_void(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + if let ty::Adt(adt, _) = ty.kind() + && let &[krate, .., name] = &*cx.get_def_path(adt.did) + && let sym::libc | sym::core | sym::std = krate + && name.as_str() == "c_void" + { + true + } else { + false + } +} diff --git a/tests/compile-test.rs b/tests/compile-test.rs index a82ff1828..6bc74bc1e 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -162,6 +162,11 @@ fn run_ui() { let config = base_config("ui"); // use tests/clippy.toml let _g = VarGuard::set("CARGO_MANIFEST_DIR", fs::canonicalize("tests").unwrap()); + let _threads = VarGuard::set( + "RUST_TEST_THREADS", + // if RUST_TEST_THREADS is set, adhere to it, otherwise override it + env::var("RUST_TEST_THREADS").unwrap_or_else(|_| num_cpus::get().to_string()), + ); compiletest::run_tests(&config); } diff --git a/tests/ui/auxiliary/macro_rules.rs b/tests/ui/auxiliary/macro_rules.rs index 0251fada9..9f283337c 100644 --- a/tests/ui/auxiliary/macro_rules.rs +++ b/tests/ui/auxiliary/macro_rules.rs @@ -120,3 +120,10 @@ macro_rules! mut_mut { let mut_mut_ty: &mut &mut u32 = &mut &mut 1u32; }; } + +#[macro_export] +macro_rules! ptr_as_ptr_cast { + ($ptr: ident) => { + $ptr as *const i32 + }; +} diff --git a/tests/ui/await_holding_lock.rs b/tests/ui/await_holding_lock.rs index dd6640a38..57e5b5504 100644 --- a/tests/ui/await_holding_lock.rs +++ b/tests/ui/await_holding_lock.rs @@ -1,64 +1,192 @@ #![warn(clippy::await_holding_lock)] -use std::sync::Mutex; +// When adding or modifying a test, please do the same for parking_lot::Mutex. +mod std_mutex { + use super::baz; + use std::sync::{Mutex, RwLock}; -async fn bad(x: &Mutex) -> u32 { - let guard = x.lock().unwrap(); - baz().await + pub async fn bad(x: &Mutex) -> u32 { + let guard = x.lock().unwrap(); + baz().await + } + + pub async fn good(x: &Mutex) -> u32 { + { + let guard = x.lock().unwrap(); + let y = *guard + 1; + } + baz().await; + let guard = x.lock().unwrap(); + 47 + } + + pub async fn bad_rw(x: &RwLock) -> u32 { + let guard = x.read().unwrap(); + baz().await + } + + pub async fn bad_rw_write(x: &RwLock) -> u32 { + let mut guard = x.write().unwrap(); + baz().await + } + + pub async fn good_rw(x: &RwLock) -> u32 { + { + let guard = x.read().unwrap(); + let y = *guard + 1; + } + { + let mut guard = x.write().unwrap(); + *guard += 1; + } + baz().await; + let guard = x.read().unwrap(); + 47 + } + + pub async fn also_bad(x: &Mutex) -> u32 { + let first = baz().await; + + let guard = x.lock().unwrap(); + + let second = baz().await; + + let third = baz().await; + + first + second + third + } + + pub async fn not_good(x: &Mutex) -> u32 { + let first = baz().await; + + let second = { + let guard = x.lock().unwrap(); + baz().await + }; + + let third = baz().await; + + first + second + third + } + + #[allow(clippy::manual_async_fn)] + pub fn block_bad(x: &Mutex) -> impl std::future::Future + '_ { + async move { + let guard = x.lock().unwrap(); + baz().await + } + } } -async fn good(x: &Mutex) -> u32 { - { - let guard = x.lock().unwrap(); - let y = *guard + 1; +// When adding or modifying a test, please do the same for std::Mutex. +mod parking_lot_mutex { + use super::baz; + use parking_lot::{Mutex, RwLock}; + + pub async fn bad(x: &Mutex) -> u32 { + let guard = x.lock(); + baz().await + } + + pub async fn good(x: &Mutex) -> u32 { + { + let guard = x.lock(); + let y = *guard + 1; + } + baz().await; + let guard = x.lock(); + 47 + } + + pub async fn bad_rw(x: &RwLock) -> u32 { + let guard = x.read(); + baz().await + } + + pub async fn bad_rw_write(x: &RwLock) -> u32 { + let mut guard = x.write(); + baz().await + } + + pub async fn good_rw(x: &RwLock) -> u32 { + { + let guard = x.read(); + let y = *guard + 1; + } + { + let mut guard = x.write(); + *guard += 1; + } + baz().await; + let guard = x.read(); + 47 + } + + pub async fn also_bad(x: &Mutex) -> u32 { + let first = baz().await; + + let guard = x.lock(); + + let second = baz().await; + + let third = baz().await; + + first + second + third + } + + pub async fn not_good(x: &Mutex) -> u32 { + let first = baz().await; + + let second = { + let guard = x.lock(); + baz().await + }; + + let third = baz().await; + + first + second + third + } + + #[allow(clippy::manual_async_fn)] + pub fn block_bad(x: &Mutex) -> impl std::future::Future + '_ { + async move { + let guard = x.lock(); + baz().await + } } - baz().await; - let guard = x.lock().unwrap(); - 47 } async fn baz() -> u32 { 42 } -async fn also_bad(x: &Mutex) -> u32 { - let first = baz().await; - - let guard = x.lock().unwrap(); - - let second = baz().await; - - let third = baz().await; - - first + second + third +async fn no_await(x: std::sync::Mutex) { + let mut guard = x.lock().unwrap(); + *guard += 1; } -async fn not_good(x: &Mutex) -> u32 { - let first = baz().await; - - let second = { - let guard = x.lock().unwrap(); - baz().await - }; - - let third = baz().await; - - first + second + third -} - -#[allow(clippy::manual_async_fn)] -fn block_bad(x: &Mutex) -> impl std::future::Future + '_ { - async move { - let guard = x.lock().unwrap(); - baz().await - } +// FIXME: FP, because the `MutexGuard` is dropped before crossing the await point. This is +// something the needs to be fixed in rustc. There's already drop-tracking, but this is currently +// disabled, see rust-lang/rust#93751. This case isn't picked up by drop-tracking though. If the +// `*guard += 1` is removed it is picked up. +async fn dropped_before_await(x: std::sync::Mutex) { + let mut guard = x.lock().unwrap(); + *guard += 1; + drop(guard); + baz().await; } fn main() { - let m = Mutex::new(100); - good(&m); - bad(&m); - also_bad(&m); - not_good(&m); - block_bad(&m); + let m = std::sync::Mutex::new(100); + std_mutex::good(&m); + std_mutex::bad(&m); + std_mutex::also_bad(&m); + std_mutex::not_good(&m); + std_mutex::block_bad(&m); + + let m = parking_lot::Mutex::new(100); + parking_lot_mutex::good(&m); + parking_lot_mutex::bad(&m); + parking_lot_mutex::also_bad(&m); + parking_lot_mutex::not_good(&m); } diff --git a/tests/ui/await_holding_lock.stderr b/tests/ui/await_holding_lock.stderr index ddfb104cd..976da8d92 100644 --- a/tests/ui/await_holding_lock.stderr +++ b/tests/ui/await_holding_lock.stderr @@ -1,63 +1,208 @@ -error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await - --> $DIR/await_holding_lock.rs:6:9 +error: this `MutexGuard` is held across an `await` point + --> $DIR/await_holding_lock.rs:9:13 | -LL | let guard = x.lock().unwrap(); - | ^^^^^ +LL | let guard = x.lock().unwrap(); + | ^^^^^ | = note: `-D clippy::await-holding-lock` implied by `-D warnings` -note: these are all the await points this lock is held through - --> $DIR/await_holding_lock.rs:6:5 - | -LL | / let guard = x.lock().unwrap(); -LL | | baz().await -LL | | } - | |_^ - -error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await - --> $DIR/await_holding_lock.rs:27:9 - | -LL | let guard = x.lock().unwrap(); - | ^^^^^ - | -note: these are all the await points this lock is held through - --> $DIR/await_holding_lock.rs:27:5 - | -LL | / let guard = x.lock().unwrap(); -LL | | -LL | | let second = baz().await; -LL | | -... | -LL | | first + second + third -LL | | } - | |_^ - -error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await - --> $DIR/await_holding_lock.rs:40:13 - | -LL | let guard = x.lock().unwrap(); - | ^^^^^ - | -note: these are all the await points this lock is held through - --> $DIR/await_holding_lock.rs:40:9 - | -LL | / let guard = x.lock().unwrap(); -LL | | baz().await -LL | | }; - | |_____^ - -error: this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await - --> $DIR/await_holding_lock.rs:52:13 - | -LL | let guard = x.lock().unwrap(); - | ^^^^^ - | -note: these are all the await points this lock is held through - --> $DIR/await_holding_lock.rs:52:9 + = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await +note: these are all the `await` points this lock is held through + --> $DIR/await_holding_lock.rs:9:9 | LL | / let guard = x.lock().unwrap(); LL | | baz().await LL | | } | |_____^ -error: aborting due to 4 previous errors +error: this `MutexGuard` is held across an `await` point + --> $DIR/await_holding_lock.rs:24:13 + | +LL | let guard = x.read().unwrap(); + | ^^^^^ + | + = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await +note: these are all the `await` points this lock is held through + --> $DIR/await_holding_lock.rs:24:9 + | +LL | / let guard = x.read().unwrap(); +LL | | baz().await +LL | | } + | |_____^ + +error: this `MutexGuard` is held across an `await` point + --> $DIR/await_holding_lock.rs:29:13 + | +LL | let mut guard = x.write().unwrap(); + | ^^^^^^^^^ + | + = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await +note: these are all the `await` points this lock is held through + --> $DIR/await_holding_lock.rs:29:9 + | +LL | / let mut guard = x.write().unwrap(); +LL | | baz().await +LL | | } + | |_____^ + +error: this `MutexGuard` is held across an `await` point + --> $DIR/await_holding_lock.rs:50:13 + | +LL | let guard = x.lock().unwrap(); + | ^^^^^ + | + = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await +note: these are all the `await` points this lock is held through + --> $DIR/await_holding_lock.rs:50:9 + | +LL | / let guard = x.lock().unwrap(); +LL | | +LL | | let second = baz().await; +LL | | +... | +LL | | first + second + third +LL | | } + | |_____^ + +error: this `MutexGuard` is held across an `await` point + --> $DIR/await_holding_lock.rs:63:17 + | +LL | let guard = x.lock().unwrap(); + | ^^^^^ + | + = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await +note: these are all the `await` points this lock is held through + --> $DIR/await_holding_lock.rs:63:13 + | +LL | / let guard = x.lock().unwrap(); +LL | | baz().await +LL | | }; + | |_________^ + +error: this `MutexGuard` is held across an `await` point + --> $DIR/await_holding_lock.rs:75:17 + | +LL | let guard = x.lock().unwrap(); + | ^^^^^ + | + = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await +note: these are all the `await` points this lock is held through + --> $DIR/await_holding_lock.rs:75:13 + | +LL | / let guard = x.lock().unwrap(); +LL | | baz().await +LL | | } + | |_________^ + +error: this `MutexGuard` is held across an `await` point + --> $DIR/await_holding_lock.rs:87:13 + | +LL | let guard = x.lock(); + | ^^^^^ + | + = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await +note: these are all the `await` points this lock is held through + --> $DIR/await_holding_lock.rs:87:9 + | +LL | / let guard = x.lock(); +LL | | baz().await +LL | | } + | |_____^ + +error: this `MutexGuard` is held across an `await` point + --> $DIR/await_holding_lock.rs:102:13 + | +LL | let guard = x.read(); + | ^^^^^ + | + = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await +note: these are all the `await` points this lock is held through + --> $DIR/await_holding_lock.rs:102:9 + | +LL | / let guard = x.read(); +LL | | baz().await +LL | | } + | |_____^ + +error: this `MutexGuard` is held across an `await` point + --> $DIR/await_holding_lock.rs:107:13 + | +LL | let mut guard = x.write(); + | ^^^^^^^^^ + | + = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await +note: these are all the `await` points this lock is held through + --> $DIR/await_holding_lock.rs:107:9 + | +LL | / let mut guard = x.write(); +LL | | baz().await +LL | | } + | |_____^ + +error: this `MutexGuard` is held across an `await` point + --> $DIR/await_holding_lock.rs:128:13 + | +LL | let guard = x.lock(); + | ^^^^^ + | + = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await +note: these are all the `await` points this lock is held through + --> $DIR/await_holding_lock.rs:128:9 + | +LL | / let guard = x.lock(); +LL | | +LL | | let second = baz().await; +LL | | +... | +LL | | first + second + third +LL | | } + | |_____^ + +error: this `MutexGuard` is held across an `await` point + --> $DIR/await_holding_lock.rs:141:17 + | +LL | let guard = x.lock(); + | ^^^^^ + | + = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await +note: these are all the `await` points this lock is held through + --> $DIR/await_holding_lock.rs:141:13 + | +LL | / let guard = x.lock(); +LL | | baz().await +LL | | }; + | |_________^ + +error: this `MutexGuard` is held across an `await` point + --> $DIR/await_holding_lock.rs:153:17 + | +LL | let guard = x.lock(); + | ^^^^^ + | + = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await +note: these are all the `await` points this lock is held through + --> $DIR/await_holding_lock.rs:153:13 + | +LL | / let guard = x.lock(); +LL | | baz().await +LL | | } + | |_________^ + +error: this `MutexGuard` is held across an `await` point + --> $DIR/await_holding_lock.rs:173:9 + | +LL | let mut guard = x.lock().unwrap(); + | ^^^^^^^^^ + | + = help: consider using an async-aware `Mutex` type or ensuring the `MutexGuard` is dropped before calling await +note: these are all the `await` points this lock is held through + --> $DIR/await_holding_lock.rs:173:5 + | +LL | / let mut guard = x.lock().unwrap(); +LL | | *guard += 1; +LL | | drop(guard); +LL | | baz().await; +LL | | } + | |_^ + +error: aborting due to 13 previous errors diff --git a/tests/ui/await_holding_refcell_ref.stderr b/tests/ui/await_holding_refcell_ref.stderr index 67cc0032b..4339fca73 100644 --- a/tests/ui/await_holding_refcell_ref.stderr +++ b/tests/ui/await_holding_refcell_ref.stderr @@ -1,11 +1,12 @@ -error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await +error: this `RefCell` reference is held across an `await` point --> $DIR/await_holding_refcell_ref.rs:6:9 | LL | let b = x.borrow(); | ^ | = note: `-D clippy::await-holding-refcell-ref` implied by `-D warnings` -note: these are all the await points this ref is held through + = help: ensure the reference is dropped before calling `await` +note: these are all the `await` points this reference is held through --> $DIR/await_holding_refcell_ref.rs:6:5 | LL | / let b = x.borrow(); @@ -13,13 +14,14 @@ LL | | baz().await LL | | } | |_^ -error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await +error: this `RefCell` reference is held across an `await` point --> $DIR/await_holding_refcell_ref.rs:11:9 | LL | let b = x.borrow_mut(); | ^ | -note: these are all the await points this ref is held through + = help: ensure the reference is dropped before calling `await` +note: these are all the `await` points this reference is held through --> $DIR/await_holding_refcell_ref.rs:11:5 | LL | / let b = x.borrow_mut(); @@ -27,13 +29,14 @@ LL | | baz().await LL | | } | |_^ -error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await +error: this `RefCell` reference is held across an `await` point --> $DIR/await_holding_refcell_ref.rs:32:9 | LL | let b = x.borrow_mut(); | ^ | -note: these are all the await points this ref is held through + = help: ensure the reference is dropped before calling `await` +note: these are all the `await` points this reference is held through --> $DIR/await_holding_refcell_ref.rs:32:5 | LL | / let b = x.borrow_mut(); @@ -45,13 +48,14 @@ LL | | first + second + third LL | | } | |_^ -error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await +error: this `RefCell` reference is held across an `await` point --> $DIR/await_holding_refcell_ref.rs:44:9 | LL | let b = x.borrow_mut(); | ^ | -note: these are all the await points this ref is held through + = help: ensure the reference is dropped before calling `await` +note: these are all the `await` points this reference is held through --> $DIR/await_holding_refcell_ref.rs:44:5 | LL | / let b = x.borrow_mut(); @@ -63,13 +67,14 @@ LL | | first + second + third LL | | } | |_^ -error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await +error: this `RefCell` reference is held across an `await` point --> $DIR/await_holding_refcell_ref.rs:59:13 | LL | let b = x.borrow_mut(); | ^ | -note: these are all the await points this ref is held through + = help: ensure the reference is dropped before calling `await` +note: these are all the `await` points this reference is held through --> $DIR/await_holding_refcell_ref.rs:59:9 | LL | / let b = x.borrow_mut(); @@ -77,13 +82,14 @@ LL | | baz().await LL | | }; | |_____^ -error: this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await +error: this `RefCell` reference is held across an `await` point --> $DIR/await_holding_refcell_ref.rs:71:13 | LL | let b = x.borrow_mut(); | ^ | -note: these are all the await points this ref is held through + = help: ensure the reference is dropped before calling `await` +note: these are all the `await` points this reference is held through --> $DIR/await_holding_refcell_ref.rs:71:9 | LL | / let b = x.borrow_mut(); diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index ebc1ed558..2e31ad317 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -1,3 +1,6 @@ +#![feature(repr128)] +#![allow(incomplete_features)] + #[warn( clippy::cast_precision_loss, clippy::cast_possible_truncation, @@ -115,4 +118,137 @@ fn main() { }) as u8; 999999u64.clamp(0, 255) as u8; 999999u64.clamp(0, 256) as u8; // should still be linted + + #[derive(Clone, Copy)] + enum E1 { + A, + B, + C, + } + impl E1 { + fn test(self) { + let _ = self as u8; // Don't lint. `0..=2` fits in u8 + } + } + + #[derive(Clone, Copy)] + enum E2 { + A = 255, + B, + } + impl E2 { + fn test(self) { + let _ = self as u8; + let _ = Self::B as u8; + let _ = self as i16; // Don't lint. `255..=256` fits in i16 + let _ = Self::A as u8; // Don't lint. + } + } + + #[derive(Clone, Copy)] + enum E3 { + A = -1, + B, + C = 50, + } + impl E3 { + fn test(self) { + let _ = self as i8; // Don't lint. `-1..=50` fits in i8 + } + } + + #[derive(Clone, Copy)] + enum E4 { + A = -128, + B, + } + impl E4 { + fn test(self) { + let _ = self as i8; // Don't lint. `-128..=-127` fits in i8 + } + } + + #[derive(Clone, Copy)] + enum E5 { + A = -129, + B = 127, + } + impl E5 { + fn test(self) { + let _ = self as i8; + let _ = Self::A as i8; + let _ = self as i16; // Don't lint. `-129..=127` fits in i16 + let _ = Self::B as u8; // Don't lint. + } + } + + #[derive(Clone, Copy)] + #[repr(u32)] + enum E6 { + A = u16::MAX as u32, + B, + } + impl E6 { + fn test(self) { + let _ = self as i16; + let _ = Self::A as u16; // Don't lint. `2^16-1` fits in u16 + let _ = self as u32; // Don't lint. `2^16-1..=2^16` fits in u32 + let _ = Self::A as u16; // Don't lint. + } + } + + #[derive(Clone, Copy)] + #[repr(u64)] + enum E7 { + A = u32::MAX as u64, + B, + } + impl E7 { + fn test(self) { + let _ = self as usize; + let _ = Self::A as usize; // Don't lint. + let _ = self as u64; // Don't lint. `2^32-1..=2^32` fits in u64 + } + } + + #[derive(Clone, Copy)] + #[repr(i128)] + enum E8 { + A = i128::MIN, + B, + C = 0, + D = i128::MAX, + } + impl E8 { + fn test(self) { + let _ = self as i128; // Don't lint. `-(2^127)..=2^127-1` fits it i128 + } + } + + #[derive(Clone, Copy)] + #[repr(u128)] + enum E9 { + A, + B = u128::MAX, + } + impl E9 { + fn test(self) { + let _ = Self::A as u8; // Don't lint. + let _ = self as u128; // Don't lint. `0..=2^128-1` fits in u128 + } + } + + #[derive(Clone, Copy)] + #[repr(usize)] + enum E10 { + A, + B = u32::MAX as usize, + } + impl E10 { + fn test(self) { + let _ = self as u16; + let _ = Self::B as u32; // Don't lint. + let _ = self as u64; // Don't lint. + } + } } diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index edf8790cf..7a68c0984 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) - --> $DIR/cast.rs:11:5 + --> $DIR/cast.rs:14:5 | LL | x0 as f32; | ^^^^^^^^^ @@ -7,37 +7,37 @@ LL | x0 as f32; = note: `-D clippy::cast-precision-loss` implied by `-D warnings` error: casting `i64` to `f32` causes a loss of precision (`i64` is 64 bits wide, but `f32`'s mantissa is only 23 bits wide) - --> $DIR/cast.rs:13:5 + --> $DIR/cast.rs:16: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) - --> $DIR/cast.rs:14:5 + --> $DIR/cast.rs:17: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) - --> $DIR/cast.rs:16:5 + --> $DIR/cast.rs:19: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) - --> $DIR/cast.rs:18:5 + --> $DIR/cast.rs:21: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) - --> $DIR/cast.rs:19:5 + --> $DIR/cast.rs:22:5 | LL | x3 as f64; | ^^^^^^^^^ error: casting `f32` to `i32` may truncate the value - --> $DIR/cast.rs:21:5 + --> $DIR/cast.rs:24:5 | LL | 1f32 as i32; | ^^^^^^^^^^^ @@ -45,13 +45,13 @@ LL | 1f32 as i32; = note: `-D clippy::cast-possible-truncation` implied by `-D warnings` error: casting `f32` to `u32` may truncate the value - --> $DIR/cast.rs:22:5 + --> $DIR/cast.rs:25:5 | LL | 1f32 as u32; | ^^^^^^^^^^^ error: casting `f32` to `u32` may lose the sign of the value - --> $DIR/cast.rs:22:5 + --> $DIR/cast.rs:25:5 | LL | 1f32 as u32; | ^^^^^^^^^^^ @@ -59,43 +59,43 @@ LL | 1f32 as u32; = note: `-D clippy::cast-sign-loss` implied by `-D warnings` error: casting `f64` to `f32` may truncate the value - --> $DIR/cast.rs:23:5 + --> $DIR/cast.rs:26:5 | LL | 1f64 as f32; | ^^^^^^^^^^^ error: casting `i32` to `i8` may truncate the value - --> $DIR/cast.rs:24:5 + --> $DIR/cast.rs:27:5 | LL | 1i32 as i8; | ^^^^^^^^^^ error: casting `i32` to `u8` may truncate the value - --> $DIR/cast.rs:25:5 + --> $DIR/cast.rs:28:5 | LL | 1i32 as u8; | ^^^^^^^^^^ error: casting `f64` to `isize` may truncate the value - --> $DIR/cast.rs:26:5 + --> $DIR/cast.rs:29:5 | LL | 1f64 as isize; | ^^^^^^^^^^^^^ error: casting `f64` to `usize` may truncate the value - --> $DIR/cast.rs:27:5 + --> $DIR/cast.rs:30:5 | LL | 1f64 as usize; | ^^^^^^^^^^^^^ error: casting `f64` to `usize` may lose the sign of the value - --> $DIR/cast.rs:27:5 + --> $DIR/cast.rs:30:5 | LL | 1f64 as usize; | ^^^^^^^^^^^^^ error: casting `u8` to `i8` may wrap around the value - --> $DIR/cast.rs:29:5 + --> $DIR/cast.rs:32:5 | LL | 1u8 as i8; | ^^^^^^^^^ @@ -103,52 +103,96 @@ LL | 1u8 as i8; = note: `-D clippy::cast-possible-wrap` implied by `-D warnings` error: casting `u16` to `i16` may wrap around the value - --> $DIR/cast.rs:30:5 + --> $DIR/cast.rs:33:5 | LL | 1u16 as i16; | ^^^^^^^^^^^ error: casting `u32` to `i32` may wrap around the value - --> $DIR/cast.rs:31:5 + --> $DIR/cast.rs:34:5 | LL | 1u32 as i32; | ^^^^^^^^^^^ error: casting `u64` to `i64` may wrap around the value - --> $DIR/cast.rs:32:5 + --> $DIR/cast.rs:35:5 | LL | 1u64 as i64; | ^^^^^^^^^^^ error: casting `usize` to `isize` may wrap around the value - --> $DIR/cast.rs:33:5 + --> $DIR/cast.rs:36:5 | LL | 1usize as isize; | ^^^^^^^^^^^^^^^ error: casting `i32` to `u32` may lose the sign of the value - --> $DIR/cast.rs:36:5 + --> $DIR/cast.rs:39:5 | LL | -1i32 as u32; | ^^^^^^^^^^^^ error: casting `isize` to `usize` may lose the sign of the value - --> $DIR/cast.rs:38:5 + --> $DIR/cast.rs:41:5 | LL | -1isize as usize; | ^^^^^^^^^^^^^^^^ error: casting `i64` to `i8` may truncate the value - --> $DIR/cast.rs:105:5 + --> $DIR/cast.rs:108:5 | LL | (-99999999999i64).min(1) as i8; // should be linted because signed | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `u64` to `u8` may truncate the value - --> $DIR/cast.rs:117:5 + --> $DIR/cast.rs:120:5 | LL | 999999u64.clamp(0, 256) as u8; // should still be linted | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 24 previous errors +error: casting `main::E2` to `u8` may truncate the value + --> $DIR/cast.rs:141:21 + | +LL | let _ = self as u8; + | ^^^^^^^^^^ + +error: casting `main::E2::B` to `u8` will truncate the value + --> $DIR/cast.rs:142:21 + | +LL | let _ = Self::B as u8; + | ^^^^^^^^^^^^^ + | + = note: `-D clippy::cast-enum-truncation` implied by `-D warnings` + +error: casting `main::E5` to `i8` may truncate the value + --> $DIR/cast.rs:178:21 + | +LL | let _ = self as i8; + | ^^^^^^^^^^ + +error: casting `main::E5::A` to `i8` will truncate the value + --> $DIR/cast.rs:179:21 + | +LL | let _ = Self::A as i8; + | ^^^^^^^^^^^^^ + +error: casting `main::E6` to `i16` may truncate the value + --> $DIR/cast.rs:193:21 + | +LL | let _ = self as i16; + | ^^^^^^^^^^^ + +error: casting `main::E7` to `usize` may truncate the value on targets with 32-bit wide pointers + --> $DIR/cast.rs:208:21 + | +LL | let _ = self as usize; + | ^^^^^^^^^^^^^ + +error: casting `main::E10` to `u16` may truncate the value + --> $DIR/cast.rs:249:21 + | +LL | let _ = self as u16; + | ^^^^^^^^^^^ + +error: aborting due to 31 previous errors diff --git a/tests/ui/dbg_macro.rs b/tests/ui/dbg_macro.rs index d74e2611e..9b03c9b47 100644 --- a/tests/ui/dbg_macro.rs +++ b/tests/ui/dbg_macro.rs @@ -16,4 +16,27 @@ fn main() { dbg!(42); dbg!(dbg!(dbg!(42))); foo(3) + dbg!(factorial(4)); + dbg!(1, 2, dbg!(3, 4)); + dbg!(1, 2, 3, 4, 5); +} + +mod issue7274 { + trait Thing<'b> { + fn foo(&self); + } + + macro_rules! define_thing { + ($thing:ident, $body:expr) => { + impl<'a> Thing<'a> for $thing { + fn foo<'b>(&self) { + $body + } + } + }; + } + + struct MyThing; + define_thing!(MyThing, { + dbg!(2); + }); } diff --git a/tests/ui/dbg_macro.stderr b/tests/ui/dbg_macro.stderr index 0abe953af..8ee1b3287 100644 --- a/tests/ui/dbg_macro.stderr +++ b/tests/ui/dbg_macro.stderr @@ -76,5 +76,38 @@ help: ensure to avoid having uses of it in version control LL | foo(3) + factorial(4); | ~~~~~~~~~~~~ -error: aborting due to 7 previous errors +error: `dbg!` macro is intended as a debugging tool + --> $DIR/dbg_macro.rs:19:5 + | +LL | dbg!(1, 2, dbg!(3, 4)); + | ^^^^^^^^^^^^^^^^^^^^^^ + | +help: ensure to avoid having uses of it in version control + | +LL | (1, 2, dbg!(3, 4)); + | ~~~~~~~~~~~~~~~~~~ + +error: `dbg!` macro is intended as a debugging tool + --> $DIR/dbg_macro.rs:20:5 + | +LL | dbg!(1, 2, 3, 4, 5); + | ^^^^^^^^^^^^^^^^^^^ + | +help: ensure to avoid having uses of it in version control + | +LL | (1, 2, 3, 4, 5); + | ~~~~~~~~~~~~~~~ + +error: `dbg!` macro is intended as a debugging tool + --> $DIR/dbg_macro.rs:40:9 + | +LL | dbg!(2); + | ^^^^^^^ + | +help: ensure to avoid having uses of it in version control + | +LL | 2; + | ~ + +error: aborting due to 10 previous errors diff --git a/tests/ui/default_trait_access.fixed b/tests/ui/default_trait_access.fixed index 9114d8754..264dd4efa 100644 --- a/tests/ui/default_trait_access.fixed +++ b/tests/ui/default_trait_access.fixed @@ -46,9 +46,14 @@ fn main() { let s19 = ::default(); + let s20 = UpdateSyntax { + s: "foo", + ..Default::default() + }; + println!( - "[{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}], [{:?}]", - s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, s11, s12, s13, s14, s15, s16, s17, s18, s19, + "[{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}]", + s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, s11, s12, s13, s14, s15, s16, s17, s18, s19, s20, ); } @@ -86,3 +91,9 @@ struct ArrayDerivedDefault { #[derive(Debug, Default)] struct TupleStructDerivedDefault(String); + +#[derive(Debug, Default)] +struct UpdateSyntax { + pub s: &'static str, + pub u: u64, +} diff --git a/tests/ui/default_trait_access.rs b/tests/ui/default_trait_access.rs index 8a5f0d6a7..a0930fab8 100644 --- a/tests/ui/default_trait_access.rs +++ b/tests/ui/default_trait_access.rs @@ -46,9 +46,14 @@ fn main() { let s19 = ::default(); + let s20 = UpdateSyntax { + s: "foo", + ..Default::default() + }; + println!( - "[{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}], [{:?}]", - s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, s11, s12, s13, s14, s15, s16, s17, s18, s19, + "[{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}]", + s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, s11, s12, s13, s14, s15, s16, s17, s18, s19, s20, ); } @@ -86,3 +91,9 @@ struct ArrayDerivedDefault { #[derive(Debug, Default)] struct TupleStructDerivedDefault(String); + +#[derive(Debug, Default)] +struct UpdateSyntax { + pub s: &'static str, + pub u: u64, +} diff --git a/tests/ui/deref_by_slicing.fixed b/tests/ui/deref_by_slicing.fixed new file mode 100644 index 000000000..b26276218 --- /dev/null +++ b/tests/ui/deref_by_slicing.fixed @@ -0,0 +1,29 @@ +// run-rustfix + +#![warn(clippy::deref_by_slicing)] + +use std::io::Read; + +fn main() { + let mut vec = vec![0]; + let _ = &*vec; + let _ = &mut *vec; + + let ref_vec = &mut vec; + let _ = &**ref_vec; + let mut_slice = &mut **ref_vec; + let _ = &mut *mut_slice; // Err, re-borrows slice + + let s = String::new(); + let _ = &*s; + + static S: &[u8] = &[0, 1, 2]; + let _ = &mut &*S; // Err, re-borrows slice + + let slice: &[u32] = &[0u32, 1u32]; + let slice_ref = &slice; + let _ = *slice_ref; // Err, derefs slice + + let bytes: &[u8] = &[]; + let _ = (&*bytes).read_to_end(&mut vec![]).unwrap(); // Err, re-borrows slice +} diff --git a/tests/ui/deref_by_slicing.rs b/tests/ui/deref_by_slicing.rs new file mode 100644 index 000000000..6aa1408ba --- /dev/null +++ b/tests/ui/deref_by_slicing.rs @@ -0,0 +1,29 @@ +// run-rustfix + +#![warn(clippy::deref_by_slicing)] + +use std::io::Read; + +fn main() { + let mut vec = vec![0]; + let _ = &vec[..]; + let _ = &mut vec[..]; + + let ref_vec = &mut vec; + let _ = &ref_vec[..]; + let mut_slice = &mut ref_vec[..]; + let _ = &mut mut_slice[..]; // Err, re-borrows slice + + let s = String::new(); + let _ = &s[..]; + + static S: &[u8] = &[0, 1, 2]; + let _ = &mut &S[..]; // Err, re-borrows slice + + let slice: &[u32] = &[0u32, 1u32]; + let slice_ref = &slice; + let _ = &slice_ref[..]; // Err, derefs slice + + let bytes: &[u8] = &[]; + let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap(); // Err, re-borrows slice +} diff --git a/tests/ui/deref_by_slicing.stderr b/tests/ui/deref_by_slicing.stderr new file mode 100644 index 000000000..ffd76de37 --- /dev/null +++ b/tests/ui/deref_by_slicing.stderr @@ -0,0 +1,58 @@ +error: slicing when dereferencing would work + --> $DIR/deref_by_slicing.rs:9:13 + | +LL | let _ = &vec[..]; + | ^^^^^^^^ help: dereference the original value instead: `&*vec` + | + = note: `-D clippy::deref-by-slicing` implied by `-D warnings` + +error: slicing when dereferencing would work + --> $DIR/deref_by_slicing.rs:10:13 + | +LL | let _ = &mut vec[..]; + | ^^^^^^^^^^^^ help: dereference the original value instead: `&mut *vec` + +error: slicing when dereferencing would work + --> $DIR/deref_by_slicing.rs:13:13 + | +LL | let _ = &ref_vec[..]; + | ^^^^^^^^^^^^ help: dereference the original value instead: `&**ref_vec` + +error: slicing when dereferencing would work + --> $DIR/deref_by_slicing.rs:14:21 + | +LL | let mut_slice = &mut ref_vec[..]; + | ^^^^^^^^^^^^^^^^ help: dereference the original value instead: `&mut **ref_vec` + +error: slicing when dereferencing would work + --> $DIR/deref_by_slicing.rs:15:13 + | +LL | let _ = &mut mut_slice[..]; // Err, re-borrows slice + | ^^^^^^^^^^^^^^^^^^ help: reborrow the original value instead: `&mut *mut_slice` + +error: slicing when dereferencing would work + --> $DIR/deref_by_slicing.rs:18:13 + | +LL | let _ = &s[..]; + | ^^^^^^ help: dereference the original value instead: `&*s` + +error: slicing when dereferencing would work + --> $DIR/deref_by_slicing.rs:21:18 + | +LL | let _ = &mut &S[..]; // Err, re-borrows slice + | ^^^^^^ help: reborrow the original value instead: `&*S` + +error: slicing when dereferencing would work + --> $DIR/deref_by_slicing.rs:25:13 + | +LL | let _ = &slice_ref[..]; // Err, derefs slice + | ^^^^^^^^^^^^^^ help: dereference the original value instead: `*slice_ref` + +error: slicing when dereferencing would work + --> $DIR/deref_by_slicing.rs:28:13 + | +LL | let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap(); // Err, re-borrows slice + | ^^^^^^^^^^^^ help: reborrow the original value instead: `(&*bytes)` + +error: aborting due to 9 previous errors + diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed index 618f80cdc..5aedbea38 100644 --- a/tests/ui/eta.fixed +++ b/tests/ui/eta.fixed @@ -256,3 +256,22 @@ fn arc_fp() { (0..5).map(|n| arc(n)); Some(4).map(|n| ref_arc(n)); } + +// #8460 Don't replace closures with params bounded as `ref` +mod bind_by_ref { + struct A; + struct B; + + impl From<&A> for B { + fn from(A: &A) -> Self { + B + } + } + + fn test() { + // should not lint + Some(A).map(|a| B::from(&a)); + // should not lint + Some(A).map(|ref a| B::from(a)); + } +} diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs index a759e6eb5..5fdf7fb97 100644 --- a/tests/ui/eta.rs +++ b/tests/ui/eta.rs @@ -256,3 +256,22 @@ fn arc_fp() { (0..5).map(|n| arc(n)); Some(4).map(|n| ref_arc(n)); } + +// #8460 Don't replace closures with params bounded as `ref` +mod bind_by_ref { + struct A; + struct B; + + impl From<&A> for B { + fn from(A: &A) -> Self { + B + } + } + + fn test() { + // should not lint + Some(A).map(|a| B::from(&a)); + // should not lint + Some(A).map(|ref a| B::from(a)); + } +} diff --git a/tests/ui/match_as_ref.fixed b/tests/ui/match_as_ref.fixed index c61eb9216..ddfa1e741 100644 --- a/tests/ui/match_as_ref.fixed +++ b/tests/ui/match_as_ref.fixed @@ -32,4 +32,12 @@ mod issue4437 { } } -fn main() {} +fn main() { + // Don't lint + let _ = match Some(0) { + #[cfg(feature = "foo")] + Some(ref x) if *x > 50 => None, + Some(ref x) => Some(x), + None => None, + }; +} diff --git a/tests/ui/match_as_ref.rs b/tests/ui/match_as_ref.rs index 2fbd0b255..025d475ae 100644 --- a/tests/ui/match_as_ref.rs +++ b/tests/ui/match_as_ref.rs @@ -41,4 +41,12 @@ mod issue4437 { } } -fn main() {} +fn main() { + // Don't lint + let _ = match Some(0) { + #[cfg(feature = "foo")] + Some(ref x) if *x > 50 => None, + Some(ref x) => Some(x), + None => None, + }; +} diff --git a/tests/ui/match_bool.rs b/tests/ui/match_bool.rs index 9ed55ca7a..bcc999a49 100644 --- a/tests/ui/match_bool.rs +++ b/tests/ui/match_bool.rs @@ -50,6 +50,14 @@ fn match_bool() { 11..=20 => 2, _ => 3, }; + + // Don't lint + let _ = match test { + #[cfg(feature = "foo")] + true if option == 5 => 10, + true => 0, + false => 1, + }; } fn main() {} diff --git a/tests/ui/match_expr_like_matches_macro.fixed b/tests/ui/match_expr_like_matches_macro.fixed index c611f76bf..36f233f33 100644 --- a/tests/ui/match_expr_like_matches_macro.fixed +++ b/tests/ui/match_expr_like_matches_macro.fixed @@ -146,4 +146,19 @@ fn main() { let _res = matches!(&val, &Some(ref _a)); fun(val); } + + { + enum E { + A, + B, + C, + } + + let _ = match E::A { + E::B => true, + #[cfg(feature = "foo")] + E::A => true, + _ => false, + }; + } } diff --git a/tests/ui/match_expr_like_matches_macro.rs b/tests/ui/match_expr_like_matches_macro.rs index 2deeb84e7..750f69fa5 100644 --- a/tests/ui/match_expr_like_matches_macro.rs +++ b/tests/ui/match_expr_like_matches_macro.rs @@ -181,4 +181,19 @@ fn main() { }; fun(val); } + + { + enum E { + A, + B, + C, + } + + let _ = match E::A { + E::B => true, + #[cfg(feature = "foo")] + E::A => true, + _ => false, + }; + } } diff --git a/tests/ui/match_same_arms2.rs b/tests/ui/match_same_arms2.rs index da4e3020d..67e1d5184 100644 --- a/tests/ui/match_same_arms2.rs +++ b/tests/ui/match_same_arms2.rs @@ -166,4 +166,12 @@ fn match_expr_like_matches_macro_priority() { }; } -fn main() {} +fn main() { + let _ = match Some(0) { + Some(0) => 0, + Some(1) => 1, + #[cfg(feature = "foo")] + Some(2) => 2, + _ => 1, + }; +} diff --git a/tests/ui/match_single_binding.fixed b/tests/ui/match_single_binding.fixed index b4ec525ad..b8dc8179f 100644 --- a/tests/ui/match_single_binding.fixed +++ b/tests/ui/match_single_binding.fixed @@ -106,10 +106,8 @@ fn main() { 0 => println!("Array index start"), _ => println!("Not an array index start"), } - // False negative + + // Lint let x = 1; - match x { - // => - _ => println!("Not an array index start"), - } + println!("Not an array index start"); } diff --git a/tests/ui/match_single_binding.rs b/tests/ui/match_single_binding.rs index e04c4018b..fe63dcd63 100644 --- a/tests/ui/match_single_binding.rs +++ b/tests/ui/match_single_binding.rs @@ -118,7 +118,8 @@ fn main() { 0 => println!("Array index start"), _ => println!("Not an array index start"), } - // False negative + + // Lint let x = 1; match x { // => diff --git a/tests/ui/match_single_binding.stderr b/tests/ui/match_single_binding.stderr index 291fa77dc..d939291f5 100644 --- a/tests/ui/match_single_binding.stderr +++ b/tests/ui/match_single_binding.stderr @@ -167,5 +167,14 @@ LL + unwrapped LL ~ }) | -error: aborting due to 11 previous errors +error: this match could be replaced by its body itself + --> $DIR/match_single_binding.rs:124:5 + | +LL | / match x { +LL | | // => +LL | | _ => println!("Not an array index start"), +LL | | } + | |_____^ help: consider using the match body instead: `println!("Not an array index start");` + +error: aborting due to 12 previous errors diff --git a/tests/ui/new_without_default.rs b/tests/ui/new_without_default.rs index 4b2e7444d..c76e3b424 100644 --- a/tests/ui/new_without_default.rs +++ b/tests/ui/new_without_default.rs @@ -90,6 +90,22 @@ impl Private { } // We don't lint private items } +struct PrivateStruct; + +impl PrivateStruct { + pub fn new() -> PrivateStruct { + unimplemented!() + } // We don't lint public items on private structs +} + +pub struct PrivateItem; + +impl PrivateItem { + fn new() -> PrivateItem { + unimplemented!() + } // We don't lint private items on public structs +} + struct Const; impl Const { diff --git a/tests/ui/new_without_default.stderr b/tests/ui/new_without_default.stderr index 14ddb66f2..19572dfe8 100644 --- a/tests/ui/new_without_default.stderr +++ b/tests/ui/new_without_default.stderr @@ -51,7 +51,7 @@ LL + } | error: you should consider adding a `Default` implementation for `NewNotEqualToDerive` - --> $DIR/new_without_default.rs:156:5 + --> $DIR/new_without_default.rs:172:5 | LL | / pub fn new() -> Self { LL | | NewNotEqualToDerive { foo: 1 } @@ -68,7 +68,7 @@ LL + } | error: you should consider adding a `Default` implementation for `FooGenerics` - --> $DIR/new_without_default.rs:164:5 + --> $DIR/new_without_default.rs:180:5 | LL | / pub fn new() -> Self { LL | | Self(Default::default()) @@ -85,7 +85,7 @@ LL + } | error: you should consider adding a `Default` implementation for `BarGenerics` - --> $DIR/new_without_default.rs:171:5 + --> $DIR/new_without_default.rs:187:5 | LL | / pub fn new() -> Self { LL | | Self(Default::default()) @@ -102,7 +102,7 @@ LL + } | error: you should consider adding a `Default` implementation for `Foo` - --> $DIR/new_without_default.rs:182:9 + --> $DIR/new_without_default.rs:198:9 | LL | / pub fn new() -> Self { LL | | todo!() diff --git a/tests/ui/ptr_as_ptr.fixed b/tests/ui/ptr_as_ptr.fixed index 8346a9454..bea6be66a 100644 --- a/tests/ui/ptr_as_ptr.fixed +++ b/tests/ui/ptr_as_ptr.fixed @@ -1,8 +1,17 @@ // run-rustfix +// aux-build:macro_rules.rs #![warn(clippy::ptr_as_ptr)] #![feature(custom_inner_attributes)] +extern crate macro_rules; + +macro_rules! cast_it { + ($ptr: ident) => { + $ptr.cast::() + }; +} + fn main() { let ptr: *const u32 = &42_u32; let mut_ptr: *mut u32 = &mut 42_u32; @@ -28,6 +37,12 @@ fn main() { // Ensure the lint doesn't produce unnecessary turbofish for inferred types. let _: *const i32 = ptr.cast(); let _: *mut i32 = mut_ptr.cast(); + + // Make sure the lint is triggered inside a macro + let _ = cast_it!(ptr); + + // Do not lint inside macros from external crates + let _ = macro_rules::ptr_as_ptr_cast!(ptr); } fn _msrv_1_37() { diff --git a/tests/ui/ptr_as_ptr.rs b/tests/ui/ptr_as_ptr.rs index b68d4bc0a..ca2616b00 100644 --- a/tests/ui/ptr_as_ptr.rs +++ b/tests/ui/ptr_as_ptr.rs @@ -1,8 +1,17 @@ // run-rustfix +// aux-build:macro_rules.rs #![warn(clippy::ptr_as_ptr)] #![feature(custom_inner_attributes)] +extern crate macro_rules; + +macro_rules! cast_it { + ($ptr: ident) => { + $ptr as *const i32 + }; +} + fn main() { let ptr: *const u32 = &42_u32; let mut_ptr: *mut u32 = &mut 42_u32; @@ -28,6 +37,12 @@ fn main() { // Ensure the lint doesn't produce unnecessary turbofish for inferred types. let _: *const i32 = ptr as *const _; let _: *mut i32 = mut_ptr as _; + + // Make sure the lint is triggered inside a macro + let _ = cast_it!(ptr); + + // Do not lint inside macros from external crates + let _ = macro_rules::ptr_as_ptr_cast!(ptr); } fn _msrv_1_37() { diff --git a/tests/ui/ptr_as_ptr.stderr b/tests/ui/ptr_as_ptr.stderr index 854906dc1..c58c55cfd 100644 --- a/tests/ui/ptr_as_ptr.stderr +++ b/tests/ui/ptr_as_ptr.stderr @@ -1,5 +1,5 @@ error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:10:13 + --> $DIR/ptr_as_ptr.rs:19:13 | LL | let _ = ptr as *const i32; | ^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast::()` @@ -7,40 +7,51 @@ LL | let _ = ptr as *const i32; = note: `-D clippy::ptr-as-ptr` implied by `-D warnings` error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:11:13 + --> $DIR/ptr_as_ptr.rs:20:13 | LL | let _ = mut_ptr as *mut i32; | ^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `mut_ptr.cast::()` error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:16:17 + --> $DIR/ptr_as_ptr.rs:25:17 | LL | let _ = *ptr_ptr as *const i32; | ^^^^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `(*ptr_ptr).cast::()` error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:29:25 + --> $DIR/ptr_as_ptr.rs:38:25 | LL | let _: *const i32 = ptr as *const _; | ^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast()` error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:30:23 + --> $DIR/ptr_as_ptr.rs:39:23 | LL | let _: *mut i32 = mut_ptr as _; | ^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `mut_ptr.cast()` error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:48:13 + --> $DIR/ptr_as_ptr.rs:11:9 + | +LL | $ptr as *const i32 + | ^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `$ptr.cast::()` +... +LL | let _ = cast_it!(ptr); + | ------------- in this macro invocation + | + = note: this error originates in the macro `cast_it` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: `as` casting between raw pointers without changing its mutability + --> $DIR/ptr_as_ptr.rs:63:13 | LL | let _ = ptr as *const i32; | ^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast::()` error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:49:13 + --> $DIR/ptr_as_ptr.rs:64:13 | LL | let _ = mut_ptr as *mut i32; | ^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `mut_ptr.cast::()` -error: aborting due to 7 previous errors +error: aborting due to 8 previous errors diff --git a/tests/ui/recursive_format_impl.rs b/tests/ui/recursive_format_impl.rs new file mode 100644 index 000000000..9241bf7ed --- /dev/null +++ b/tests/ui/recursive_format_impl.rs @@ -0,0 +1,321 @@ +#![warn(clippy::recursive_format_impl)] +#![allow( + clippy::inherent_to_string_shadow_display, + clippy::to_string_in_format_args, + clippy::deref_addrof +)] + +use std::fmt; + +struct A; +impl A { + fn fmt(&self) { + self.to_string(); + } +} + +trait B { + fn fmt(&self) {} +} + +impl B for A { + fn fmt(&self) { + self.to_string(); + } +} + +impl fmt::Display for A { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.to_string()) + } +} + +fn fmt(a: A) { + a.to_string(); +} + +struct C; + +impl C { + // Doesn't trigger if to_string defined separately + // i.e. not using ToString trait (from Display) + fn to_string(&self) -> String { + String::from("I am C") + } +} + +impl fmt::Display for C { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.to_string()) + } +} + +enum D { + E(String), + F, +} + +impl std::fmt::Display for D { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match &self { + Self::E(string) => write!(f, "E {}", string.to_string()), + Self::F => write!(f, "F"), + } + } +} + +// Check for use of self as Display, in Display impl +// Triggers on direct use of self +struct G {} + +impl std::fmt::Display for G { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self) + } +} + +// Triggers on reference to self +struct H {} + +impl std::fmt::Display for H { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", &self) + } +} + +impl std::fmt::Debug for H { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{:?}", &self) + } +} + +// Triggers on multiple reference to self +struct H2 {} + +impl std::fmt::Display for H2 { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", &&&self) + } +} + +// Doesn't trigger on correct deref +struct I {} + +impl std::ops::Deref for I { + type Target = str; + + fn deref(&self) -> &Self::Target { + "test" + } +} + +impl std::fmt::Display for I { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", &**self) + } +} + +impl std::fmt::Debug for I { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{:?}", &**self) + } +} + +// Doesn't trigger on multiple correct deref +struct I2 {} + +impl std::ops::Deref for I2 { + type Target = str; + + fn deref(&self) -> &Self::Target { + "test" + } +} + +impl std::fmt::Display for I2 { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", **&&&**self) + } +} + +// Doesn't trigger on multiple correct deref +struct I3 {} + +impl std::ops::Deref for I3 { + type Target = str; + + fn deref(&self) -> &Self::Target { + "test" + } +} + +impl std::fmt::Display for I3 { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", &&**&&&**self) + } +} + +// Does trigger when deref resolves to self +struct J {} + +impl std::ops::Deref for J { + type Target = str; + + fn deref(&self) -> &Self::Target { + "test" + } +} + +impl std::fmt::Display for J { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", &*self) + } +} + +impl std::fmt::Debug for J { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{:?}", &*self) + } +} + +struct J2 {} + +impl std::ops::Deref for J2 { + type Target = str; + + fn deref(&self) -> &Self::Target { + "test" + } +} + +impl std::fmt::Display for J2 { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", *self) + } +} + +struct J3 {} + +impl std::ops::Deref for J3 { + type Target = str; + + fn deref(&self) -> &Self::Target { + "test" + } +} + +impl std::fmt::Display for J3 { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", **&&*self) + } +} + +struct J4 {} + +impl std::ops::Deref for J4 { + type Target = str; + + fn deref(&self) -> &Self::Target { + "test" + } +} + +impl std::fmt::Display for J4 { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", &&**&&*self) + } +} + +// Doesn't trigger on Debug from Display +struct K {} + +impl std::fmt::Debug for K { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "test") + } +} + +impl std::fmt::Display for K { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{:?}", self) + } +} + +// Doesn't trigger on Display from Debug +struct K2 {} + +impl std::fmt::Debug for K2 { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", self) + } +} + +impl std::fmt::Display for K2 { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "test") + } +} + +// Doesn't trigger on struct fields +struct L { + field1: u32, + field2: i32, +} + +impl std::fmt::Display for L { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{},{}", self.field1, self.field2) + } +} + +impl std::fmt::Debug for L { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{:?},{:?}", self.field1, self.field2) + } +} + +// Doesn't trigger on nested enum matching +enum Tree { + Leaf, + Node(Vec), +} + +impl std::fmt::Display for Tree { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Tree::Leaf => write!(f, "*"), + Tree::Node(children) => { + write!(f, "(")?; + for child in children.iter() { + write!(f, "{},", child)?; + } + write!(f, ")") + }, + } + } +} + +impl std::fmt::Debug for Tree { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Tree::Leaf => write!(f, "*"), + Tree::Node(children) => { + write!(f, "(")?; + for child in children.iter() { + write!(f, "{:?},", child)?; + } + write!(f, ")") + }, + } + } +} + +fn main() { + let a = A; + a.to_string(); + a.fmt(); + fmt(a); + + let c = C; + c.to_string(); +} diff --git a/tests/ui/recursive_format_impl.stderr b/tests/ui/recursive_format_impl.stderr new file mode 100644 index 000000000..6171696ed --- /dev/null +++ b/tests/ui/recursive_format_impl.stderr @@ -0,0 +1,91 @@ +error: using `self.to_string` in `fmt::Display` implementation will cause infinite recursion + --> $DIR/recursive_format_impl.rs:29:25 + | +LL | write!(f, "{}", self.to_string()) + | ^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::recursive-format-impl` implied by `-D warnings` + +error: unnecessary use of `to_string` + --> $DIR/recursive_format_impl.rs:61:50 + | +LL | Self::E(string) => write!(f, "E {}", string.to_string()), + | ^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::unnecessary-to-owned` implied by `-D warnings` + = note: this error originates in the macro `$crate::format_args` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: using `self` as `Display` in `impl Display` will cause infinite recursion + --> $DIR/recursive_format_impl.rs:73:9 + | +LL | write!(f, "{}", self) + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `write` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: using `self` as `Display` in `impl Display` will cause infinite recursion + --> $DIR/recursive_format_impl.rs:82:9 + | +LL | write!(f, "{}", &self) + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `write` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: using `self` as `Debug` in `impl Debug` will cause infinite recursion + --> $DIR/recursive_format_impl.rs:88:9 + | +LL | write!(f, "{:?}", &self) + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `write` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: using `self` as `Display` in `impl Display` will cause infinite recursion + --> $DIR/recursive_format_impl.rs:97:9 + | +LL | write!(f, "{}", &&&self) + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `write` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: using `self` as `Display` in `impl Display` will cause infinite recursion + --> $DIR/recursive_format_impl.rs:171:9 + | +LL | write!(f, "{}", &*self) + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `write` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: using `self` as `Debug` in `impl Debug` will cause infinite recursion + --> $DIR/recursive_format_impl.rs:177:9 + | +LL | write!(f, "{:?}", &*self) + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `write` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: using `self` as `Display` in `impl Display` will cause infinite recursion + --> $DIR/recursive_format_impl.rs:193:9 + | +LL | write!(f, "{}", *self) + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `write` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: using `self` as `Display` in `impl Display` will cause infinite recursion + --> $DIR/recursive_format_impl.rs:209:9 + | +LL | write!(f, "{}", **&&*self) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `write` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: using `self` as `Display` in `impl Display` will cause infinite recursion + --> $DIR/recursive_format_impl.rs:225:9 + | +LL | write!(f, "{}", &&**&&*self) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `write` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 11 previous errors + diff --git a/tests/ui/redundant_slicing.fixed b/tests/ui/redundant_slicing.fixed new file mode 100644 index 000000000..8dd8d3092 --- /dev/null +++ b/tests/ui/redundant_slicing.fixed @@ -0,0 +1,46 @@ +// run-rustfix + +#![allow(unused, clippy::deref_by_slicing)] +#![warn(clippy::redundant_slicing)] + +use std::io::Read; + +fn main() { + let slice: &[u32] = &[0]; + let _ = slice; // Redundant slice + + let v = vec![0]; + let _ = &v[..]; // Ok, results in `&[_]` + let _ = (&*v); // Outer borrow is redundant + + static S: &[u8] = &[0, 1, 2]; + let _ = &mut &S[..]; // Ok, re-borrows slice + + let mut vec = vec![0]; + let mut_slice = &mut vec[..]; // Ok, results in `&mut [_]` + let _ = &mut mut_slice[..]; // Ok, re-borrows slice + + let ref_vec = &vec; + let _ = &ref_vec[..]; // Ok, results in `&[_]` + + macro_rules! m { + ($e:expr) => { + $e + }; + } + let _ = slice; + + macro_rules! m2 { + ($e:expr) => { + &$e[..] + }; + } + let _ = m2!(slice); // Don't lint in a macro + + let slice_ref = &slice; + let _ = &slice_ref[..]; // Ok, derefs slice + + // Issue #7972 + let bytes: &[u8] = &[]; + let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap(); // Ok, re-borrows slice +} diff --git a/tests/ui/redundant_slicing.rs b/tests/ui/redundant_slicing.rs index 554b6ba36..51c16dd8d 100644 --- a/tests/ui/redundant_slicing.rs +++ b/tests/ui/redundant_slicing.rs @@ -1,20 +1,27 @@ -#![allow(unused)] +// run-rustfix + +#![allow(unused, clippy::deref_by_slicing)] #![warn(clippy::redundant_slicing)] +use std::io::Read; + fn main() { let slice: &[u32] = &[0]; - let _ = &slice[..]; + let _ = &slice[..]; // Redundant slice let v = vec![0]; - let _ = &v[..]; // Changes the type - let _ = &(&v[..])[..]; // Outer borrow is redundant + let _ = &v[..]; // Ok, results in `&[_]` + let _ = &(&*v)[..]; // Outer borrow is redundant static S: &[u8] = &[0, 1, 2]; - let err = &mut &S[..]; // Should reborrow instead of slice + let _ = &mut &S[..]; // Ok, re-borrows slice let mut vec = vec![0]; - let mut_slice = &mut *vec; - let _ = &mut mut_slice[..]; // Should reborrow instead of slice + let mut_slice = &mut vec[..]; // Ok, results in `&mut [_]` + let _ = &mut mut_slice[..]; // Ok, re-borrows slice + + let ref_vec = &vec; + let _ = &ref_vec[..]; // Ok, results in `&[_]` macro_rules! m { ($e:expr) => { @@ -29,4 +36,11 @@ fn main() { }; } let _ = m2!(slice); // Don't lint in a macro + + let slice_ref = &slice; + let _ = &slice_ref[..]; // Ok, derefs slice + + // Issue #7972 + let bytes: &[u8] = &[]; + let _ = (&bytes[..]).read_to_end(&mut vec![]).unwrap(); // Ok, re-borrows slice } diff --git a/tests/ui/redundant_slicing.stderr b/tests/ui/redundant_slicing.stderr index bbd10eafb..82367143c 100644 --- a/tests/ui/redundant_slicing.stderr +++ b/tests/ui/redundant_slicing.stderr @@ -1,34 +1,22 @@ error: redundant slicing of the whole range - --> $DIR/redundant_slicing.rs:6:13 + --> $DIR/redundant_slicing.rs:10:13 | -LL | let _ = &slice[..]; +LL | let _ = &slice[..]; // Redundant slice | ^^^^^^^^^^ help: use the original value instead: `slice` | = note: `-D clippy::redundant-slicing` implied by `-D warnings` error: redundant slicing of the whole range - --> $DIR/redundant_slicing.rs:10:13 + --> $DIR/redundant_slicing.rs:14:13 | -LL | let _ = &(&v[..])[..]; // Outer borrow is redundant - | ^^^^^^^^^^^^^ help: use the original value instead: `(&v[..])` +LL | let _ = &(&*v)[..]; // Outer borrow is redundant + | ^^^^^^^^^^ help: use the original value instead: `(&*v)` error: redundant slicing of the whole range - --> $DIR/redundant_slicing.rs:13:20 - | -LL | let err = &mut &S[..]; // Should reborrow instead of slice - | ^^^^^^ help: reborrow the original value instead: `&*S` - -error: redundant slicing of the whole range - --> $DIR/redundant_slicing.rs:17:13 - | -LL | let _ = &mut mut_slice[..]; // Should reborrow instead of slice - | ^^^^^^^^^^^^^^^^^^ help: reborrow the original value instead: `&mut *mut_slice` - -error: redundant slicing of the whole range - --> $DIR/redundant_slicing.rs:24:13 + --> $DIR/redundant_slicing.rs:31:13 | LL | let _ = &m!(slice)[..]; | ^^^^^^^^^^^^^^ help: use the original value instead: `slice` -error: aborting due to 5 previous errors +error: aborting due to 3 previous errors diff --git a/tests/ui/rename.fixed b/tests/ui/rename.fixed index 8bddec576..24a0c8122 100644 --- a/tests/ui/rename.fixed +++ b/tests/ui/rename.fixed @@ -20,6 +20,7 @@ #![allow(clippy::match_result_ok)] #![allow(clippy::disallowed_types)] #![allow(clippy::disallowed_methods)] +#![allow(clippy::recursive_format_impl)] // uplifted lints #![allow(invalid_value)] #![allow(array_into_iter)] @@ -55,6 +56,7 @@ #![warn(clippy::disallowed_types)] #![warn(clippy::disallowed_methods)] #![warn(clippy::needless_borrow)] +#![warn(clippy::recursive_format_impl)] // uplifted lints #![warn(invalid_value)] #![warn(array_into_iter)] diff --git a/tests/ui/rename.rs b/tests/ui/rename.rs index d2010d71d..ea64234c6 100644 --- a/tests/ui/rename.rs +++ b/tests/ui/rename.rs @@ -20,6 +20,7 @@ #![allow(clippy::match_result_ok)] #![allow(clippy::disallowed_types)] #![allow(clippy::disallowed_methods)] +#![allow(clippy::recursive_format_impl)] // uplifted lints #![allow(invalid_value)] #![allow(array_into_iter)] @@ -55,6 +56,7 @@ #![warn(clippy::disallowed_type)] #![warn(clippy::disallowed_method)] #![warn(clippy::ref_in_deref)] +#![warn(clippy::to_string_in_display)] // uplifted lints #![warn(clippy::invalid_ref)] #![warn(clippy::into_iter_on_array)] diff --git a/tests/ui/rename.stderr b/tests/ui/rename.stderr index 45cb8b786..8b132a783 100644 --- a/tests/ui/rename.stderr +++ b/tests/ui/rename.stderr @@ -1,5 +1,5 @@ error: lint `clippy::stutter` has been renamed to `clippy::module_name_repetitions` - --> $DIR/rename.rs:34:9 + --> $DIR/rename.rs:35:9 | LL | #![warn(clippy::stutter)] | ^^^^^^^^^^^^^^^ help: use the new name: `clippy::module_name_repetitions` @@ -7,196 +7,202 @@ LL | #![warn(clippy::stutter)] = note: `-D renamed-and-removed-lints` implied by `-D warnings` error: lint `clippy::new_without_default_derive` has been renamed to `clippy::new_without_default` - --> $DIR/rename.rs:35:9 + --> $DIR/rename.rs:36:9 | LL | #![warn(clippy::new_without_default_derive)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::new_without_default` error: lint `clippy::const_static_lifetime` has been renamed to `clippy::redundant_static_lifetimes` - --> $DIR/rename.rs:36:9 + --> $DIR/rename.rs:37:9 | LL | #![warn(clippy::const_static_lifetime)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::redundant_static_lifetimes` error: lint `clippy::cyclomatic_complexity` has been renamed to `clippy::cognitive_complexity` - --> $DIR/rename.rs:37:9 + --> $DIR/rename.rs:38:9 | LL | #![warn(clippy::cyclomatic_complexity)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::cognitive_complexity` error: lint `clippy::option_and_then_some` has been renamed to `clippy::bind_instead_of_map` - --> $DIR/rename.rs:38:9 + --> $DIR/rename.rs:39:9 | LL | #![warn(clippy::option_and_then_some)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::bind_instead_of_map` error: lint `clippy::box_vec` has been renamed to `clippy::box_collection` - --> $DIR/rename.rs:39:9 + --> $DIR/rename.rs:40:9 | LL | #![warn(clippy::box_vec)] | ^^^^^^^^^^^^^^^ help: use the new name: `clippy::box_collection` error: lint `clippy::block_in_if_condition_expr` has been renamed to `clippy::blocks_in_if_conditions` - --> $DIR/rename.rs:40:9 + --> $DIR/rename.rs:41:9 | LL | #![warn(clippy::block_in_if_condition_expr)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::blocks_in_if_conditions` error: lint `clippy::block_in_if_condition_stmt` has been renamed to `clippy::blocks_in_if_conditions` - --> $DIR/rename.rs:41:9 + --> $DIR/rename.rs:42:9 | LL | #![warn(clippy::block_in_if_condition_stmt)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::blocks_in_if_conditions` error: lint `clippy::option_map_unwrap_or` has been renamed to `clippy::map_unwrap_or` - --> $DIR/rename.rs:42:9 + --> $DIR/rename.rs:43:9 | LL | #![warn(clippy::option_map_unwrap_or)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::map_unwrap_or` error: lint `clippy::option_map_unwrap_or_else` has been renamed to `clippy::map_unwrap_or` - --> $DIR/rename.rs:43:9 + --> $DIR/rename.rs:44:9 | LL | #![warn(clippy::option_map_unwrap_or_else)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::map_unwrap_or` error: lint `clippy::result_map_unwrap_or_else` has been renamed to `clippy::map_unwrap_or` - --> $DIR/rename.rs:44:9 + --> $DIR/rename.rs:45:9 | LL | #![warn(clippy::result_map_unwrap_or_else)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::map_unwrap_or` error: lint `clippy::option_unwrap_used` has been renamed to `clippy::unwrap_used` - --> $DIR/rename.rs:45:9 + --> $DIR/rename.rs:46:9 | LL | #![warn(clippy::option_unwrap_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::unwrap_used` error: lint `clippy::result_unwrap_used` has been renamed to `clippy::unwrap_used` - --> $DIR/rename.rs:46:9 + --> $DIR/rename.rs:47:9 | LL | #![warn(clippy::result_unwrap_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::unwrap_used` error: lint `clippy::option_expect_used` has been renamed to `clippy::expect_used` - --> $DIR/rename.rs:47:9 + --> $DIR/rename.rs:48:9 | LL | #![warn(clippy::option_expect_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::expect_used` error: lint `clippy::result_expect_used` has been renamed to `clippy::expect_used` - --> $DIR/rename.rs:48:9 + --> $DIR/rename.rs:49:9 | LL | #![warn(clippy::result_expect_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::expect_used` error: lint `clippy::for_loop_over_option` has been renamed to `clippy::for_loops_over_fallibles` - --> $DIR/rename.rs:49:9 + --> $DIR/rename.rs:50:9 | LL | #![warn(clippy::for_loop_over_option)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::for_loops_over_fallibles` error: lint `clippy::for_loop_over_result` has been renamed to `clippy::for_loops_over_fallibles` - --> $DIR/rename.rs:50:9 + --> $DIR/rename.rs:51:9 | LL | #![warn(clippy::for_loop_over_result)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::for_loops_over_fallibles` error: lint `clippy::identity_conversion` has been renamed to `clippy::useless_conversion` - --> $DIR/rename.rs:51:9 + --> $DIR/rename.rs:52:9 | LL | #![warn(clippy::identity_conversion)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::useless_conversion` error: lint `clippy::zero_width_space` has been renamed to `clippy::invisible_characters` - --> $DIR/rename.rs:52:9 + --> $DIR/rename.rs:53:9 | LL | #![warn(clippy::zero_width_space)] | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::invisible_characters` error: lint `clippy::single_char_push_str` has been renamed to `clippy::single_char_add_str` - --> $DIR/rename.rs:53:9 + --> $DIR/rename.rs:54:9 | LL | #![warn(clippy::single_char_push_str)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::single_char_add_str` error: lint `clippy::if_let_some_result` has been renamed to `clippy::match_result_ok` - --> $DIR/rename.rs:54:9 + --> $DIR/rename.rs:55:9 | LL | #![warn(clippy::if_let_some_result)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::match_result_ok` error: lint `clippy::disallowed_type` has been renamed to `clippy::disallowed_types` - --> $DIR/rename.rs:55:9 + --> $DIR/rename.rs:56:9 | LL | #![warn(clippy::disallowed_type)] | ^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::disallowed_types` error: lint `clippy::disallowed_method` has been renamed to `clippy::disallowed_methods` - --> $DIR/rename.rs:56:9 + --> $DIR/rename.rs:57:9 | LL | #![warn(clippy::disallowed_method)] | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::disallowed_methods` error: lint `clippy::ref_in_deref` has been renamed to `clippy::needless_borrow` - --> $DIR/rename.rs:57:9 + --> $DIR/rename.rs:58:9 | LL | #![warn(clippy::ref_in_deref)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::needless_borrow` -error: lint `clippy::invalid_ref` has been renamed to `invalid_value` +error: lint `clippy::to_string_in_display` has been renamed to `clippy::recursive_format_impl` --> $DIR/rename.rs:59:9 | +LL | #![warn(clippy::to_string_in_display)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::recursive_format_impl` + +error: lint `clippy::invalid_ref` has been renamed to `invalid_value` + --> $DIR/rename.rs:61:9 + | LL | #![warn(clippy::invalid_ref)] | ^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_value` error: lint `clippy::into_iter_on_array` has been renamed to `array_into_iter` - --> $DIR/rename.rs:60:9 + --> $DIR/rename.rs:62:9 | LL | #![warn(clippy::into_iter_on_array)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `array_into_iter` error: lint `clippy::unused_label` has been renamed to `unused_labels` - --> $DIR/rename.rs:61:9 + --> $DIR/rename.rs:63:9 | LL | #![warn(clippy::unused_label)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `unused_labels` error: lint `clippy::drop_bounds` has been renamed to `drop_bounds` - --> $DIR/rename.rs:62:9 + --> $DIR/rename.rs:64:9 | LL | #![warn(clippy::drop_bounds)] | ^^^^^^^^^^^^^^^^^^^ help: use the new name: `drop_bounds` error: lint `clippy::temporary_cstring_as_ptr` has been renamed to `temporary_cstring_as_ptr` - --> $DIR/rename.rs:63:9 + --> $DIR/rename.rs:65:9 | LL | #![warn(clippy::temporary_cstring_as_ptr)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `temporary_cstring_as_ptr` error: lint `clippy::panic_params` has been renamed to `non_fmt_panics` - --> $DIR/rename.rs:64:9 + --> $DIR/rename.rs:66:9 | LL | #![warn(clippy::panic_params)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `non_fmt_panics` error: lint `clippy::unknown_clippy_lints` has been renamed to `unknown_lints` - --> $DIR/rename.rs:65:9 + --> $DIR/rename.rs:67:9 | LL | #![warn(clippy::unknown_clippy_lints)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `unknown_lints` error: lint `clippy::invalid_atomic_ordering` has been renamed to `invalid_atomic_ordering` - --> $DIR/rename.rs:66:9 + --> $DIR/rename.rs:68:9 | LL | #![warn(clippy::invalid_atomic_ordering)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_atomic_ordering` error: lint `clippy::mem_discriminant_non_enum` has been renamed to `enum_intrinsics_non_enums` - --> $DIR/rename.rs:67:9 + --> $DIR/rename.rs:69:9 | LL | #![warn(clippy::mem_discriminant_non_enum)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `enum_intrinsics_non_enums` -error: aborting due to 33 previous errors +error: aborting due to 34 previous errors diff --git a/tests/ui/single_match.rs b/tests/ui/single_match.rs index bd3718880..dd148edf5 100644 --- a/tests/ui/single_match.rs +++ b/tests/ui/single_match.rs @@ -234,4 +234,12 @@ macro_rules! single_match { fn main() { single_match!(5); + + // Don't lint + let _ = match Some(0) { + #[cfg(feature = "foo")] + Some(10) => 11, + Some(x) => x, + _ => 0, + }; } diff --git a/tests/ui/temporary_assignment.rs b/tests/ui/temporary_assignment.rs index b4a931043..ac4c1bc65 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 4cc32c79f..7d79901a2 100644 --- a/tests/ui/temporary_assignment.stderr +++ b/tests/ui/temporary_assignment.stderr @@ -1,5 +1,5 @@ error: assignment to temporary - --> $DIR/temporary_assignment.rs:48:5 + --> $DIR/temporary_assignment.rs:47:5 | LL | Struct { field: 0 }.field = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -7,7 +7,7 @@ LL | Struct { field: 0 }.field = 1; = note: `-D clippy::temporary-assignment` implied by `-D warnings` error: assignment to temporary - --> $DIR/temporary_assignment.rs:49:5 + --> $DIR/temporary_assignment.rs:48:5 | LL | / MultiStruct { LL | | structure: Struct { field: 0 }, @@ -17,13 +17,13 @@ LL | | .field = 1; | |______________^ error: assignment to temporary - --> $DIR/temporary_assignment.rs:54:5 + --> $DIR/temporary_assignment.rs:53:5 | LL | ArrayStruct { array: [0] }.array[0] = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: assignment to temporary - --> $DIR/temporary_assignment.rs:55:5 + --> $DIR/temporary_assignment.rs:54:5 | LL | (0, 0).0 = 1; | ^^^^^^^^^^^^ diff --git a/tests/ui/to_string_in_display.rs b/tests/ui/to_string_in_display.rs deleted file mode 100644 index 3ccdcd111..000000000 --- a/tests/ui/to_string_in_display.rs +++ /dev/null @@ -1,69 +0,0 @@ -#![warn(clippy::to_string_in_display)] -#![allow(clippy::inherent_to_string_shadow_display, clippy::to_string_in_format_args)] - -use std::fmt; - -struct A; -impl A { - fn fmt(&self) { - self.to_string(); - } -} - -trait B { - fn fmt(&self) {} -} - -impl B for A { - fn fmt(&self) { - self.to_string(); - } -} - -impl fmt::Display for A { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", self.to_string()) - } -} - -fn fmt(a: A) { - a.to_string(); -} - -struct C; - -impl C { - fn to_string(&self) -> String { - String::from("I am C") - } -} - -impl fmt::Display for C { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", self.to_string()) - } -} - -enum D { - E(String), - F, -} - -impl std::fmt::Display for D { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match &self { - Self::E(string) => write!(f, "E {}", string.to_string()), - Self::F => write!(f, "F"), - } - } -} - -fn main() { - let a = A; - a.to_string(); - a.fmt(); - fmt(a); - - let c = C; - c.to_string(); -} diff --git a/tests/ui/to_string_in_display.stderr b/tests/ui/to_string_in_display.stderr deleted file mode 100644 index 80189ca1f..000000000 --- a/tests/ui/to_string_in_display.stderr +++ /dev/null @@ -1,19 +0,0 @@ -error: using `to_string` in `fmt::Display` implementation might lead to infinite recursion - --> $DIR/to_string_in_display.rs:25:25 - | -LL | write!(f, "{}", self.to_string()) - | ^^^^^^^^^^^^^^^^ - | - = note: `-D clippy::to-string-in-display` implied by `-D warnings` - -error: unnecessary use of `to_string` - --> $DIR/to_string_in_display.rs:55:50 - | -LL | Self::E(string) => write!(f, "E {}", string.to_string()), - | ^^^^^^^^^^^^^^^^^^ - | - = note: `-D clippy::unnecessary-to-owned` implied by `-D warnings` - = note: this error originates in the macro `$crate::format_args` (in Nightly builds, run with -Z macro-backtrace for more info) - -error: aborting due to 2 previous errors - diff --git a/tests/ui/transmute_undefined_repr.rs b/tests/ui/transmute_undefined_repr.rs index 71539940f..b163d6056 100644 --- a/tests/ui/transmute_undefined_repr.rs +++ b/tests/ui/transmute_undefined_repr.rs @@ -1,5 +1,8 @@ #![warn(clippy::transmute_undefined_repr)] -#![allow(clippy::unit_arg)] +#![allow(clippy::unit_arg, clippy::transmute_ptr_to_ref)] + +use core::ffi::c_void; +use core::mem::{size_of, transmute}; fn value() -> T { unimplemented!() @@ -14,31 +17,75 @@ struct Ty2C(T, U); fn main() { unsafe { - let _: () = core::mem::transmute(value::()); - let _: Empty = core::mem::transmute(value::<()>()); + let _: () = transmute(value::()); + let _: Empty = transmute(value::<()>()); - let _: Ty = core::mem::transmute(value::()); - let _: Ty = core::mem::transmute(value::()); + let _: Ty = transmute(value::()); + let _: Ty = transmute(value::()); - let _: Ty2C = core::mem::transmute(value::>()); // Lint, Ty2 is unordered - let _: Ty2 = core::mem::transmute(value::>()); // Lint, Ty2 is unordered + let _: Ty2C = transmute(value::>()); // Lint, Ty2 is unordered + let _: Ty2 = transmute(value::>()); // Lint, Ty2 is unordered - let _: Ty2 = core::mem::transmute(value::>>()); // Ok, Ty2 types are the same - let _: Ty> = core::mem::transmute(value::>()); // Ok, Ty2 types are the same + let _: Ty2 = transmute(value::>>()); // Ok, Ty2 types are the same + let _: Ty> = transmute(value::>()); // Ok, Ty2 types are the same - let _: Ty2 = core::mem::transmute(value::>>()); // Lint, different Ty2 instances - let _: Ty> = core::mem::transmute(value::>()); // Lint, different Ty2 instances + let _: Ty2 = transmute(value::>>()); // Lint, different Ty2 instances + let _: Ty> = transmute(value::>()); // Lint, different Ty2 instances - let _: Ty<&()> = core::mem::transmute(value::<&()>()); - let _: &() = core::mem::transmute(value::>()); + let _: Ty<&()> = transmute(value::<&()>()); + let _: &() = transmute(value::>()); - let _: &Ty2 = core::mem::transmute(value::>>()); // Lint, different Ty2 instances - let _: Ty<&Ty2> = core::mem::transmute(value::<&Ty2>()); // Lint, different Ty2 instances + let _: &Ty2 = transmute(value::>>()); // Lint, different Ty2 instances + let _: Ty<&Ty2> = transmute(value::<&Ty2>()); // Lint, different Ty2 instances - let _: Ty = core::mem::transmute(value::<&Ty2>()); // Ok, pointer to usize conversion - let _: &Ty2 = core::mem::transmute(value::>()); // Ok, pointer to usize conversion + let _: Ty = transmute(value::<&Ty2>()); // Ok, pointer to usize conversion + let _: &Ty2 = transmute(value::>()); // Ok, pointer to usize conversion - let _: Ty<[u8; 8]> = core::mem::transmute(value::>()); // Ok, transmute to byte array - let _: Ty2 = core::mem::transmute(value::>()); // Ok, transmute from byte array + let _: Ty<[u8; 8]> = transmute(value::>()); // Ok, transmute to byte array + let _: Ty2 = transmute(value::>()); // Ok, transmute from byte array + + // issue #8417 + let _: Ty2C, ()> = transmute(value::>()); // Ok, Ty2 types are the same + let _: Ty2 = transmute(value::, ()>>()); // Ok, Ty2 types are the same + + let _: &'static mut Ty2 = transmute(value::>>()); // Ok, Ty2 types are the same + let _: Box> = transmute(value::<&'static mut Ty2>()); // Ok, Ty2 types are the same + let _: *mut Ty2 = transmute(value::>>()); // Ok, Ty2 types are the same + let _: Box> = transmute(value::<*mut Ty2>()); // Ok, Ty2 types are the same + + let _: &'static mut Ty2 = transmute(value::>>()); // Lint, different Ty2 instances + let _: Box> = transmute(value::<&'static mut Ty2>()); // Lint, different Ty2 instances + + let _: *const () = transmute(value::>>()); // Ok, type erasure + let _: Ty<&Ty2> = transmute(value::<*const ()>()); // Ok, reverse type erasure + + let _: *const c_void = transmute(value::>>()); // Ok, type erasure + let _: Ty<&Ty2> = transmute(value::<*const c_void>()); // Ok, reverse type erasure + + enum Erase {} + let _: *const Erase = transmute(value::>>()); // Ok, type erasure + let _: Ty<&Ty2> = transmute(value::<*const Erase>()); // Ok, reverse type erasure + + struct Erase2( + [u8; 0], + core::marker::PhantomData<(*mut u8, core::marker::PhantomPinned)>, + ); + let _: *const Erase2 = transmute(value::>>()); // Ok, type erasure + let _: Ty<&Ty2> = transmute(value::<*const Erase2>()); // Ok, reverse type erasure + + let _: *const () = transmute(value::<&&[u8]>()); // Ok, type erasure + let _: &&[u8] = transmute(value::<*const ()>()); // Ok, reverse type erasure + + let _: *mut c_void = transmute(value::<&mut &[u8]>()); // Ok, type erasure + let _: &mut &[u8] = transmute(value::<*mut c_void>()); // Ok, reverse type erasure + + let _: [u8; size_of::<&[u8]>()] = transmute(value::<&[u8]>()); // Ok, transmute to byte array + let _: &[u8] = transmute(value::<[u8; size_of::<&[u8]>()]>()); // Ok, transmute from byte array + + let _: [usize; 2] = transmute(value::<&[u8]>()); // Ok, transmute to int array + let _: &[u8] = transmute(value::<[usize; 2]>()); // Ok, transmute from int array + + let _: *const [u8] = transmute(value::>()); // Ok + let _: Box<[u8]> = transmute(value::<*mut [u8]>()); // Ok } } diff --git a/tests/ui/transmute_undefined_repr.stderr b/tests/ui/transmute_undefined_repr.stderr index 040c63c7a..42d544fc9 100644 --- a/tests/ui/transmute_undefined_repr.stderr +++ b/tests/ui/transmute_undefined_repr.stderr @@ -1,44 +1,64 @@ error: transmute from `Ty2` which has an undefined layout - --> $DIR/transmute_undefined_repr.rs:23:33 + --> $DIR/transmute_undefined_repr.rs:26:33 | -LL | let _: Ty2C = core::mem::transmute(value::>()); // Lint, Ty2 is unordered - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _: Ty2C = transmute(value::>()); // Lint, Ty2 is unordered + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::transmute-undefined-repr` implied by `-D warnings` error: transmute into `Ty2` which has an undefined layout - --> $DIR/transmute_undefined_repr.rs:24:32 + --> $DIR/transmute_undefined_repr.rs:27:32 | -LL | let _: Ty2 = core::mem::transmute(value::>()); // Lint, Ty2 is unordered - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _: Ty2 = transmute(value::>()); // Lint, Ty2 is unordered + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: transmute from `Ty>` to `Ty2`, both of which have an undefined layout - --> $DIR/transmute_undefined_repr.rs:29:32 + --> $DIR/transmute_undefined_repr.rs:32:32 | -LL | let _: Ty2 = core::mem::transmute(value::>>()); // Lint, different Ty2 instances - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _: Ty2 = transmute(value::>>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: two instances of the same generic type (`Ty2`) may have different layouts error: transmute from `Ty2` to `Ty>`, both of which have an undefined layout - --> $DIR/transmute_undefined_repr.rs:30:36 + --> $DIR/transmute_undefined_repr.rs:33:36 | -LL | let _: Ty> = core::mem::transmute(value::>()); // Lint, different Ty2 instances - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _: Ty> = transmute(value::>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: two instances of the same generic type (`Ty2`) may have different layouts -error: transmute to `&Ty2` which has an undefined layout - --> $DIR/transmute_undefined_repr.rs:35:33 +error: transmute from `Ty<&Ty2>` to `&Ty2`, both of which have an undefined layout + --> $DIR/transmute_undefined_repr.rs:38:33 | -LL | let _: &Ty2 = core::mem::transmute(value::>>()); // Lint, different Ty2 instances - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: transmute from `&Ty2` which has an undefined layout - --> $DIR/transmute_undefined_repr.rs:36:37 +LL | let _: &Ty2 = transmute(value::>>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -LL | let _: Ty<&Ty2> = core::mem::transmute(value::<&Ty2>()); // Lint, different Ty2 instances - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: two instances of the same generic type (`Ty2`) may have different layouts -error: aborting due to 6 previous errors +error: transmute from `&Ty2` to `Ty<&Ty2>`, both of which have an undefined layout + --> $DIR/transmute_undefined_repr.rs:39:37 + | +LL | let _: Ty<&Ty2> = transmute(value::<&Ty2>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: two instances of the same generic type (`Ty2`) may have different layouts + +error: transmute from `std::boxed::Box>` to `&mut Ty2`, both of which have an undefined layout + --> $DIR/transmute_undefined_repr.rs:56:45 + | +LL | let _: &'static mut Ty2 = transmute(value::>>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: two instances of the same generic type (`Ty2`) may have different layouts + +error: transmute from `&mut Ty2` to `std::boxed::Box>`, both of which have an undefined layout + --> $DIR/transmute_undefined_repr.rs:57:37 + | +LL | let _: Box> = transmute(value::<&'static mut Ty2>()); // Lint, different Ty2 instances + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: two instances of the same generic type (`Ty2`) may have different layouts + +error: aborting due to 8 previous errors