Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new lint negative_feature_names and redundant_feature_names #7539

Merged
merged 1 commit into from
Aug 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
164 changes: 164 additions & 0 deletions clippy_lints/src/feature_name.rs
Original file line number Diff line number Diff line change
@@ -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 {
""
}
),
);
}

camsteffen marked this conversation as resolved.
Show resolved Hide resolved
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();
camsteffen marked this conversation as resolved.
Show resolved Hide resolved
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
}
camsteffen marked this conversation as resolved.
Show resolved Hide resolved
};
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);
}
6 changes: 6 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
]);
Expand Down Expand Up @@ -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]
Expand Down
21 changes: 21 additions & 0 deletions tests/ui-cargo/feature_name/fail/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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 = []
7 changes: 7 additions & 0 deletions tests/ui-cargo/feature_name/fail/src/main.rs
Original file line number Diff line number Diff line change
@@ -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
}
44 changes: 44 additions & 0 deletions tests/ui-cargo/feature_name/fail/src/main.stderr
Original file line number Diff line number Diff line change
@@ -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

9 changes: 9 additions & 0 deletions tests/ui-cargo/feature_name/pass/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

# This file should not trigger the lint

[package]
name = "feature_name"
version = "0.1.0"
publish = false

[workspace]
7 changes: 7 additions & 0 deletions tests/ui-cargo/feature_name/pass/src/main.rs
Original file line number Diff line number Diff line change
@@ -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
}
Empty file removed tests/ui/missing-doc-crate.stderr
Empty file.
Empty file.
Empty file.