Auto merge of #7539 - Labelray:master, r=camsteffen

Add new lint `negative_feature_names` and `redundant_feature_names`

Add new lint [`negative_feature_names`] to detect feature names with prefixes `no-` or `not-` and new lint [`redundant_feature_names`] to detect feature names with prefixes `use-`, `with-` or suffix `-support`
changelog: Add new lint [`negative_feature_names`] and [`redundant_feature_names`]

CHANGELOG.md

@@ -2829,6 +2829,7 @@ Released 2018-09-13
 [`needless_update`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_update
 [`neg_cmp_op_on_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#neg_cmp_op_on_partial_ord
 [`neg_multiply`]: https://rust-lang.github.io/rust-clippy/master/index.html#neg_multiply
+[`negative_feature_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#negative_feature_names
 [`never_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#never_loop
 [`new_ret_no_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#new_ret_no_self
 [`new_without_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default
@@ -2882,6 +2883,7 @@ Released 2018-09-13
 [`redundant_closure_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call
 [`redundant_closure_for_method_calls`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_for_method_calls
 [`redundant_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_else
+[`redundant_feature_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_feature_names
 [`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
 [`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern
 [`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching

clippy_lints/src/feature_name.rs

@@ -0,0 +1,164 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::{diagnostics::span_lint, is_lint_allowed};
+use rustc_hir::{Crate, 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 = []
+    /// ```
+    ///
+    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 = []
+    ///
+    /// ```
+    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<'_>, _: &Crate<'_>) {
+        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);
+}

clippy_lints/src/lib.rs

@@ -213,6 +213,7 @@ macro_rules! declare_clippy_lint {
 mod exit;
 mod explicit_write;
 mod fallible_impl_from;
+mod feature_name;
 mod float_equality_without_abs;
 mod float_literal;
 mod floating_point_arithmetic;
@@ -628,6 +629,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         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,
@@ -1786,6 +1789,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
 
     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),
     ]);
@@ -2116,6 +2121,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_early_pass(move || box disallowed_script_idents::DisallowedScriptIdents::new(&scripts));
     store.register_late_pass(|| box strlen_on_c_strings::StrlenOnCStrings);
     store.register_late_pass(move || box self_named_constructors::SelfNamedConstructors);
+    store.register_late_pass(move || box feature_name::FeatureName);
 }
 
 #[rustfmt::skip]

tests/ui-cargo/feature_name/fail/Cargo.toml

@@ -0,0 +1,21 @@
+
+# Content that triggers the lint goes here
+
+[package]
+name = "feature_name"
+version = "0.1.0"
+publish = false
+
+[workspace]
+
+[features]
+use-qwq = []
+use_qwq = []
+with-owo = []
+with_owo = []
+qvq-support = []
+qvq_support = []
+no-qaq = []
+no_qaq = []
+not-orz = []
+not_orz = []

tests/ui-cargo/feature_name/fail/src/main.rs

@@ -0,0 +1,7 @@
+// compile-flags: --crate-name=feature_name
+#![warn(clippy::redundant_feature_names)]
+#![warn(clippy::negative_feature_names)]
+
+fn main() {
+    // test code goes here
+}

tests/ui-cargo/feature_name/fail/src/main.stderr

@@ -0,0 +1,44 @@
+error: the "no-" prefix in the feature name "no-qaq" is negative
+   |
+   = note: `-D clippy::negative-feature-names` implied by `-D warnings`
+   = help: consider renaming the feature to "qaq", but make sure the feature adds functionality
+
+error: the "no_" prefix in the feature name "no_qaq" is negative
+   |
+   = help: consider renaming the feature to "qaq", but make sure the feature adds functionality
+
+error: the "not-" prefix in the feature name "not-orz" is negative
+   |
+   = help: consider renaming the feature to "orz", but make sure the feature adds functionality
+
+error: the "not_" prefix in the feature name "not_orz" is negative
+   |
+   = help: consider renaming the feature to "orz", but make sure the feature adds functionality
+
+error: the "-support" suffix in the feature name "qvq-support" is redundant
+   |
+   = note: `-D clippy::redundant-feature-names` implied by `-D warnings`
+   = help: consider renaming the feature to "qvq"
+
+error: the "_support" suffix in the feature name "qvq_support" is redundant
+   |
+   = help: consider renaming the feature to "qvq"
+
+error: the "use-" prefix in the feature name "use-qwq" is redundant
+   |
+   = help: consider renaming the feature to "qwq"
+
+error: the "use_" prefix in the feature name "use_qwq" is redundant
+   |
+   = help: consider renaming the feature to "qwq"
+
+error: the "with-" prefix in the feature name "with-owo" is redundant
+   |
+   = help: consider renaming the feature to "owo"
+
+error: the "with_" prefix in the feature name "with_owo" is redundant
+   |
+   = help: consider renaming the feature to "owo"
+
+error: aborting due to 10 previous errors
+

tests/ui-cargo/feature_name/pass/Cargo.toml

@@ -0,0 +1,9 @@
+
+# This file should not trigger the lint
+
+[package]
+name = "feature_name"
+version = "0.1.0"
+publish = false
+
+[workspace]

tests/ui-cargo/feature_name/pass/src/main.rs

@@ -0,0 +1,7 @@
+// compile-flags: --crate-name=feature_name
+#![warn(clippy::redundant_feature_names)]
+#![warn(clippy::negative_feature_names)]
+
+fn main() {
+    // test code goes here
+}