diff options
| -rw-r--r-- | clippy_lints/src/cargo/common_metadata.rs | 54 | ||||
| -rw-r--r-- | clippy_lints/src/cargo/feature_name.rs | 92 | ||||
| -rw-r--r-- | clippy_lints/src/cargo/mod.rs | 221 | ||||
| -rw-r--r-- | clippy_lints/src/cargo/multiple_crate_versions.rs | 63 | ||||
| -rw-r--r-- | clippy_lints/src/cargo/wildcard_dependencies.rs | 27 | ||||
| -rw-r--r-- | clippy_lints/src/cargo_common_metadata.rs | 118 | ||||
| -rw-r--r-- | clippy_lints/src/feature_name.rs | 166 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_cargo.rs | 10 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_lints.rs | 10 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 16 | ||||
| -rw-r--r-- | clippy_lints/src/multiple_crate_versions.rs | 101 | ||||
| -rw-r--r-- | clippy_lints/src/wildcard_dependencies.rs | 57 | ||||
| -rw-r--r-- | clippy_utils/src/lib.rs | 18 |
13 files changed, 474 insertions, 479 deletions
diff --git a/clippy_lints/src/cargo/common_metadata.rs b/clippy_lints/src/cargo/common_metadata.rs new file mode 100644 index 00000000000..e0442dda479 --- /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<T: AsRef<std::ffi::OsStr>>(value: &Option<T>) -> 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 00000000000..79a469a4258 --- /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 00000000000..abe95c6663f --- /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 00000000000..76fd0819a39 --- /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 00000000000..7fa6acbf557 --- /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 23f79fdc682..00000000000 --- 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<T: AsRef<std::ffi::OsStr>>(value: &Option<T>) -> 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/feature_name.rs b/clippy_lints/src/feature_name.rs deleted file mode 100644 index dc6bef52ddd..00000000000 --- 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/lib.register_cargo.rs b/clippy_lints/src/lib.register_cargo.rs index 1809f2cc7d4..c890523fe5a 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_lints.rs b/clippy_lints/src/lib.register_lints.rs index a80320a578f..7a62a99f5ff 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -61,7 +61,11 @@ 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_LOSSLESS, casts::CAST_POSSIBLE_TRUNCATION, @@ -139,8 +143,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 +355,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, @@ -520,7 +521,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.rs b/clippy_lints/src/lib.rs index 3bd7699792a..e78f6187359 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -178,7 +178,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; @@ -220,7 +220,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; @@ -290,7 +289,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; @@ -399,7 +397,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; @@ -724,10 +721,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 +835,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; @@ -864,6 +856,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: 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` } diff --git a/clippy_lints/src/multiple_crate_versions.rs b/clippy_lints/src/multiple_crate_versions.rs deleted file mode 100644 index 1f9db39cf8c..00000000000 --- 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/wildcard_dependencies.rs b/clippy_lints/src/wildcard_dependencies.rs deleted file mode 100644 index 80d7b8a1b6d..00000000000 --- 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 42955080c96..73d91550693 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -2042,24 +2042,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 { |