From 0a021d5900a9ee55875fb247614a0671b71d3271 Mon Sep 17 00:00:00 2001 From: Labelray Date: Fri, 6 Aug 2021 22:46:20 +0800 Subject: [PATCH] Add new lints `negative_feature_names` and `redundant_feature_names` --- CHANGELOG.md | 2 + clippy_lints/src/feature_name.rs | 164 ++++++++++++++++++ clippy_lints/src/lib.rs | 6 + tests/ui-cargo/feature_name/fail/Cargo.toml | 21 +++ tests/ui-cargo/feature_name/fail/src/main.rs | 7 + .../feature_name/fail/src/main.stderr | 44 +++++ tests/ui-cargo/feature_name/pass/Cargo.toml | 9 + tests/ui-cargo/feature_name/pass/src/main.rs | 7 + tests/ui/missing-doc-crate.stderr | 0 .../missing_const_for_fn/cant_be_const.stderr | 0 tests/ui/rc_buffer_redefined_string.stderr | 0 11 files changed, 260 insertions(+) create mode 100644 clippy_lints/src/feature_name.rs create mode 100644 tests/ui-cargo/feature_name/fail/Cargo.toml create mode 100644 tests/ui-cargo/feature_name/fail/src/main.rs create mode 100644 tests/ui-cargo/feature_name/fail/src/main.stderr create mode 100644 tests/ui-cargo/feature_name/pass/Cargo.toml create mode 100644 tests/ui-cargo/feature_name/pass/src/main.rs delete mode 100644 tests/ui/missing-doc-crate.stderr delete mode 100644 tests/ui/missing_const_for_fn/cant_be_const.stderr delete mode 100644 tests/ui/rc_buffer_redefined_string.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index ac745793dda5..1c182c05f509 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2828,6 +2828,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 @@ -2881,6 +2882,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 diff --git a/clippy_lints/src/feature_name.rs b/clippy_lints/src/feature_name.rs new file mode 100644 index 000000000000..eef1407a80cf --- /dev/null +++ b/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); +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f49b382c5ea3..ce671be069af 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -213,6 +213,7 @@ 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; @@ -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, @@ -1781,6 +1784,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), ]); @@ -2105,6 +2110,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] diff --git a/tests/ui-cargo/feature_name/fail/Cargo.toml b/tests/ui-cargo/feature_name/fail/Cargo.toml new file mode 100644 index 000000000000..97d51462a946 --- /dev/null +++ b/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 = [] diff --git a/tests/ui-cargo/feature_name/fail/src/main.rs b/tests/ui-cargo/feature_name/fail/src/main.rs new file mode 100644 index 000000000000..64f01a98c90e --- /dev/null +++ b/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 +} diff --git a/tests/ui-cargo/feature_name/fail/src/main.stderr b/tests/ui-cargo/feature_name/fail/src/main.stderr new file mode 100644 index 000000000000..b9e6cb49bc98 --- /dev/null +++ b/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 + diff --git a/tests/ui-cargo/feature_name/pass/Cargo.toml b/tests/ui-cargo/feature_name/pass/Cargo.toml new file mode 100644 index 000000000000..cf947312bd47 --- /dev/null +++ b/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] diff --git a/tests/ui-cargo/feature_name/pass/src/main.rs b/tests/ui-cargo/feature_name/pass/src/main.rs new file mode 100644 index 000000000000..64f01a98c90e --- /dev/null +++ b/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 +} diff --git a/tests/ui/missing-doc-crate.stderr b/tests/ui/missing-doc-crate.stderr deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/tests/ui/missing_const_for_fn/cant_be_const.stderr b/tests/ui/missing_const_for_fn/cant_be_const.stderr deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/tests/ui/rc_buffer_redefined_string.stderr b/tests/ui/rc_buffer_redefined_string.stderr deleted file mode 100644 index e69de29bb2d1..000000000000