Skip to content

Add avoid_breaking_exported_api config option #7187

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

Merged
merged 10 commits into from
May 27, 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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ Some lints can be configured in a TOML file named `clippy.toml` or `.clippy.toml
value` mapping eg.

```toml
avoid-breaking-exported-api = false
blacklisted-names = ["toto", "tata", "titi"]
cognitive-complexity-threshold = 30
```
Expand Down
2 changes: 1 addition & 1 deletion build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ fn main() {
);
println!(
"cargo:rustc-env=RUSTC_RELEASE_CHANNEL={}",
rustc_tools_util::get_channel().unwrap_or_default()
rustc_tools_util::get_channel()
);
}
1 change: 1 addition & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
avoid-breaking-exported-api = false
19 changes: 19 additions & 0 deletions clippy_lints/src/deprecated_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,22 @@ declare_deprecated_lint! {
pub FILTER_MAP,
"this lint has been replaced by `manual_filter_map`, a more specific lint"
}

declare_deprecated_lint! {
/// **What it does:** Nothing. This lint has been deprecated.
///
/// **Deprecation reason:** The `avoid_breaking_exported_api` config option was added, which
/// enables the `enum_variant_names` lint for public items.
/// ```
pub PUB_ENUM_VARIANT_NAMES,
"set the `avoid_breaking_exported_api` config option to `false` to enable the `enum_variant_names` lint for public items"
}

declare_deprecated_lint! {
/// **What it does:** Nothing. This lint has been deprecated.
///
/// **Deprecation reason:** The `avoid_breaking_exported_api` config option was added, which
/// enables the `wrong_self_conversion` lint for public items.
pub WRONG_PUB_SELF_CONVENTION,
"set the `avoid_breaking_exported_api` config option to `false` to enable the `wrong_self_convention` lint for public items"
}
80 changes: 29 additions & 51 deletions clippy_lints/src/enum_variants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
use clippy_utils::camel_case;
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
use clippy_utils::source::is_present_in_source;
use rustc_ast::ast::{EnumDef, Item, ItemKind, VisibilityKind};
use rustc_lint::{EarlyContext, EarlyLintPass, Lint};
use rustc_hir::{EnumDef, Item, ItemKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::Span;
use rustc_span::symbol::Symbol;
Expand Down Expand Up @@ -39,36 +39,6 @@ declare_clippy_lint! {
"enums where all variants share a prefix/postfix"
}

declare_clippy_lint! {
/// **What it does:** Detects public enumeration variants that are
/// prefixed or suffixed by the same characters.
///
/// **Why is this bad?** Public enumeration variant names should specify their variant,
/// not repeat the enumeration name.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// pub enum Cake {
/// BlackForestCake,
/// HummingbirdCake,
/// BattenbergCake,
/// }
/// ```
/// Could be written as:
/// ```rust
/// pub enum Cake {
/// BlackForest,
/// Hummingbird,
/// Battenberg,
/// }
/// ```
pub PUB_ENUM_VARIANT_NAMES,
pedantic,
"public enums where all variants share a prefix/postfix"
}

declare_clippy_lint! {
/// **What it does:** Detects type names that are prefixed or suffixed by the
/// containing module's name.
Expand Down Expand Up @@ -127,21 +97,22 @@ declare_clippy_lint! {
pub struct EnumVariantNames {
modules: Vec<(Symbol, String)>,
threshold: u64,
avoid_breaking_exported_api: bool,
}

impl EnumVariantNames {
#[must_use]
pub fn new(threshold: u64) -> Self {
pub fn new(threshold: u64, avoid_breaking_exported_api: bool) -> Self {
Self {
modules: Vec::new(),
threshold,
avoid_breaking_exported_api,
}
}
}

impl_lint_pass!(EnumVariantNames => [
ENUM_VARIANT_NAMES,
PUB_ENUM_VARIANT_NAMES,
MODULE_NAME_REPETITIONS,
MODULE_INCEPTION
]);
Expand All @@ -167,33 +138,42 @@ fn partial_rmatch(post: &str, name: &str) -> usize {
}

fn check_variant(
cx: &EarlyContext<'_>,
cx: &LateContext<'_>,
threshold: u64,
def: &EnumDef,
def: &EnumDef<'_>,
item_name: &str,
item_name_chars: usize,
span: Span,
lint: &'static Lint,
) {
if (def.variants.len() as u64) < threshold {
return;
}
for var in &def.variants {
for var in def.variants {
let name = var.ident.name.as_str();
if partial_match(item_name, &name) == item_name_chars
&& name.chars().nth(item_name_chars).map_or(false, |c| !c.is_lowercase())
&& name.chars().nth(item_name_chars + 1).map_or(false, |c| !c.is_numeric())
{
span_lint(cx, lint, var.span, "variant name starts with the enum's name");
span_lint(
cx,
ENUM_VARIANT_NAMES,
var.span,
"variant name starts with the enum's name",
);
}
if partial_rmatch(item_name, &name) == item_name_chars {
span_lint(cx, lint, var.span, "variant name ends with the enum's name");
span_lint(
cx,
ENUM_VARIANT_NAMES,
var.span,
"variant name ends with the enum's name",
);
}
}
let first = &def.variants[0].ident.name.as_str();
let mut pre = &first[..camel_case::until(&*first)];
let mut post = &first[camel_case::from(&*first)..];
for var in &def.variants {
for var in def.variants {
let name = var.ident.name.as_str();

let pre_match = partial_match(pre, &name);
Expand Down Expand Up @@ -226,7 +206,7 @@ fn check_variant(
};
span_lint_and_help(
cx,
lint,
ENUM_VARIANT_NAMES,
span,
&format!("all variants have the same {}fix: `{}`", what, value),
None,
Expand Down Expand Up @@ -261,14 +241,14 @@ fn to_camel_case(item_name: &str) -> String {
s
}

impl EarlyLintPass for EnumVariantNames {
fn check_item_post(&mut self, _cx: &EarlyContext<'_>, _item: &Item) {
impl LateLintPass<'_> for EnumVariantNames {
fn check_item_post(&mut self, _cx: &LateContext<'_>, _item: &Item<'_>) {
let last = self.modules.pop();
assert!(last.is_some());
}

#[allow(clippy::similar_names)]
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
let item_name = item.ident.name.as_str();
let item_name_chars = item_name.chars().count();
let item_camel = to_camel_case(&item_name);
Expand All @@ -286,7 +266,7 @@ impl EarlyLintPass for EnumVariantNames {
);
}
}
if item.vis.kind.is_pub() {
if item.vis.node.is_pub() {
let matching = partial_match(mod_camel, &item_camel);
let rmatching = partial_rmatch(mod_camel, &item_camel);
let nchars = mod_camel.chars().count();
Expand Down Expand Up @@ -317,11 +297,9 @@ impl EarlyLintPass for EnumVariantNames {
}
}
if let ItemKind::Enum(ref def, _) = item.kind {
let lint = match item.vis.kind {
VisibilityKind::Public => PUB_ENUM_VARIANT_NAMES,
_ => ENUM_VARIANT_NAMES,
};
check_variant(cx, self.threshold, def, &item_name, item_name_chars, item.span, lint);
if !(self.avoid_breaking_exported_api && cx.access_levels.is_exported(item.hir_id())) {
check_variant(cx, self.threshold, def, &item_name, item_name_chars, item.span);
}
}
self.modules.push((item.ident.name, item_camel));
}
Expand Down
33 changes: 14 additions & 19 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,6 @@ pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore) {

#[doc(hidden)]
pub fn read_conf(sess: &Session) -> Conf {
use std::path::Path;
let file_name = match utils::conf::lookup_conf_file() {
Ok(Some(path)) => path,
Ok(None) => return Conf::default(),
Expand All @@ -416,16 +415,6 @@ pub fn read_conf(sess: &Session) -> Conf {
},
};

let file_name = if file_name.is_relative() {
sess.local_crate_source_file
.as_deref()
.and_then(Path::parent)
.unwrap_or_else(|| Path::new(""))
.join(file_name)
} else {
file_name
};

let TryConf { conf, errors } = utils::conf::read(&file_name);
// all conf errors are non-fatal, we just use the default conf in case of error
for error in errors {
Expand Down Expand Up @@ -505,6 +494,14 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
"clippy::filter_map",
"this lint has been replaced by `manual_filter_map`, a more specific lint",
);
store.register_removed(
"clippy::pub_enum_variant_names",
"set the `avoid_breaking_exported_api` config option to `false` to enable the `enum_variant_names` lint for public items",
);
store.register_removed(
"clippy::wrong_pub_self_convention",
"set the `avoid_breaking_exported_api` config option to `false` to enable the `wrong_self_convention` lint for public items",
);
// end deprecated lints, do not remove this comment, it’s used in `update_lints`

// begin register lints, do not remove this comment, it’s used in `update_lints`
Expand Down Expand Up @@ -618,7 +615,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
enum_variants::ENUM_VARIANT_NAMES,
enum_variants::MODULE_INCEPTION,
enum_variants::MODULE_NAME_REPETITIONS,
enum_variants::PUB_ENUM_VARIANT_NAMES,
eq_op::EQ_OP,
eq_op::OP_REF,
erasing_op::ERASING_OP,
Expand Down Expand Up @@ -802,7 +798,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
methods::UNNECESSARY_LAZY_EVALUATIONS,
methods::UNWRAP_USED,
methods::USELESS_ASREF,
methods::WRONG_PUB_SELF_CONVENTION,
methods::WRONG_SELF_CONVENTION,
methods::ZST_OFFSET,
minmax::MIN_MAX,
Expand Down Expand Up @@ -1026,7 +1021,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(methods::FILETYPE_IS_FILE),
LintId::of(methods::GET_UNWRAP),
LintId::of(methods::UNWRAP_USED),
LintId::of(methods::WRONG_PUB_SELF_CONVENTION),
LintId::of(misc::FLOAT_CMP_CONST),
LintId::of(misc_early::UNNEEDED_FIELD_PATTERN),
LintId::of(missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS),
Expand Down Expand Up @@ -1078,7 +1072,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(doc::MISSING_PANICS_DOC),
LintId::of(empty_enum::EMPTY_ENUM),
LintId::of(enum_variants::MODULE_NAME_REPETITIONS),
LintId::of(enum_variants::PUB_ENUM_VARIANT_NAMES),
LintId::of(eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS),
LintId::of(excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS),
LintId::of(excessive_bools::STRUCT_EXCESSIVE_BOOLS),
Expand Down Expand Up @@ -1862,7 +1855,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
})
});

store.register_late_pass(move || box methods::Methods::new(msrv));
let avoid_breaking_exported_api = conf.avoid_breaking_exported_api;
store.register_late_pass(move || box methods::Methods::new(avoid_breaking_exported_api, msrv));
store.register_late_pass(move || box matches::Matches::new(msrv));
store.register_early_pass(move || box manual_non_exhaustive::ManualNonExhaustive::new(msrv));
store.register_late_pass(move || box manual_strip::ManualStrip::new(msrv));
Expand Down Expand Up @@ -1944,6 +1938,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
let pass_by_ref_or_value = pass_by_ref_or_value::PassByRefOrValue::new(
conf.trivial_copy_size_limit,
conf.pass_by_value_size_limit,
conf.avoid_breaking_exported_api,
&sess.target,
);
store.register_late_pass(move || box pass_by_ref_or_value);
Expand All @@ -1970,7 +1965,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box redundant_clone::RedundantClone);
store.register_late_pass(|| box slow_vector_initialization::SlowVectorInit);
store.register_late_pass(|| box unnecessary_sort_by::UnnecessarySortBy);
store.register_late_pass(|| box unnecessary_wraps::UnnecessaryWraps);
store.register_late_pass(move || box unnecessary_wraps::UnnecessaryWraps::new(avoid_breaking_exported_api));
store.register_late_pass(|| box assertions_on_constants::AssertionsOnConstants);
store.register_late_pass(|| box transmuting_null::TransmutingNull);
store.register_late_pass(|| box path_buf_push_overwrite::PathBufPushOverwrite);
Expand Down Expand Up @@ -2011,10 +2006,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
let literal_representation_threshold = conf.literal_representation_threshold;
store.register_early_pass(move || box literal_representation::DecimalLiteralRepresentation::new(literal_representation_threshold));
let enum_variant_name_threshold = conf.enum_variant_name_threshold;
store.register_early_pass(move || box enum_variants::EnumVariantNames::new(enum_variant_name_threshold));
store.register_late_pass(move || box enum_variants::EnumVariantNames::new(enum_variant_name_threshold, avoid_breaking_exported_api));
store.register_early_pass(|| box tabs_in_doc_comments::TabsInDocComments);
let upper_case_acronyms_aggressive = conf.upper_case_acronyms_aggressive;
store.register_early_pass(move || box upper_case_acronyms::UpperCaseAcronyms::new(upper_case_acronyms_aggressive));
store.register_late_pass(move || box upper_case_acronyms::UpperCaseAcronyms::new(avoid_breaking_exported_api, upper_case_acronyms_aggressive));
store.register_late_pass(|| box default::Default::default());
store.register_late_pass(|| box unused_self::UnusedSelf);
store.register_late_pass(|| box mutable_debug_assertion::DebugAssertWithMutCall);
Expand Down
Loading