Auto merge of #8427 - Jarcho:merge_cargo_passes, r=llogiq

Merge cargo lints

changelog: None

Author: bors

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)
+}

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);
+}

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));
+                    }
+                },
+            }
+        }
+    }
+}