//! lint on enum variants that are prefixed or suffixed by the same characters
-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 clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start};
+use rustc_hir::{EnumDef, Item, ItemKind, Variant};
+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;
declare_clippy_lint! {
- /// **What it does:** Detects enumeration variants that are prefixed or suffixed
+ /// ### What it does
+ /// Detects enumeration variants that are prefixed or suffixed
/// by the same characters.
///
- /// **Why is this bad?** Enumeration variant names should specify their variant,
+ /// ### Why is this bad?
+ /// Enumeration variant names should specify their variant,
/// not repeat the enumeration name.
///
- /// **Known problems:** None.
+ /// ### Limitations
+ /// Characters with no casing will be considered when comparing prefixes/suffixes
+ /// This applies to numbers and non-ascii characters without casing
+ /// e.g. `Foo1` and `Foo2` is considered to have different prefixes
+ /// (the prefixes are `Foo1` and `Foo2` respectively), as also `Bar螃`, `Bar蟹`
///
- /// **Example:**
+ /// ### Example
/// ```rust
/// enum Cake {
/// BlackForestCake,
/// Battenberg,
/// }
/// ```
+ #[clippy::version = "pre 1.29.0"]
pub ENUM_VARIANT_NAMES,
style,
"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
+ /// ### What it does
+ /// Detects type names that are prefixed or suffixed by the
/// containing module's name.
///
- /// **Why is this bad?** It requires the user to type the module name twice.
- ///
- /// **Known problems:** None.
+ /// ### Why is this bad?
+ /// It requires the user to type the module name twice.
///
- /// **Example:**
+ /// ### Example
/// ```rust
/// mod cake {
/// struct BlackForestCake;
/// struct BlackForest;
/// }
/// ```
+ #[clippy::version = "1.33.0"]
pub MODULE_NAME_REPETITIONS,
pedantic,
"type names prefixed/postfixed with their containing module's name"
}
declare_clippy_lint! {
- /// **What it does:** Checks for modules that have the same name as their
+ /// ### What it does
+ /// Checks for modules that have the same name as their
/// parent module
///
- /// **Why is this bad?** A typical beginner mistake is to have `mod foo;` and
+ /// ### Why is this bad?
+ /// A typical beginner mistake is to have `mod foo;` and
/// again `mod foo { ..
/// }` in `foo.rs`.
/// The expectation is that items inside the inner `mod foo { .. }` are then
/// If this is done on purpose, it would be better to choose a more
/// representative module name.
///
- /// **Known problems:** None.
- ///
- /// **Example:**
+ /// ### Example
/// ```ignore
/// // lib.rs
/// mod foo;
/// ...
/// }
/// ```
+ #[clippy::version = "pre 1.29.0"]
pub MODULE_INCEPTION,
style,
"modules that have the same name as their parent module"
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
]);
-/// Returns the number of chars that match from the start
-#[must_use]
-fn partial_match(pre: &str, name: &str) -> usize {
- let mut name_iter = name.chars();
- let _ = name_iter.next_back(); // make sure the name is never fully matched
- pre.chars().zip(name_iter).take_while(|&(l, r)| l == r).count()
+fn check_enum_start(cx: &LateContext<'_>, item_name: &str, variant: &Variant<'_>) {
+ let name = variant.ident.name.as_str();
+ let item_name_chars = item_name.chars().count();
+
+ if count_match_start(item_name, name).char_count == 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,
+ ENUM_VARIANT_NAMES,
+ variant.span,
+ "variant name starts with the enum's name",
+ );
+ }
}
-/// Returns the number of chars that match from the end
-#[must_use]
-fn partial_rmatch(post: &str, name: &str) -> usize {
- let mut name_iter = name.chars();
- let _ = name_iter.next(); // make sure the name is never fully matched
- post.chars()
- .rev()
- .zip(name_iter.rev())
- .take_while(|&(l, r)| l == r)
- .count()
+fn check_enum_end(cx: &LateContext<'_>, item_name: &str, variant: &Variant<'_>) {
+ let name = variant.ident.name.as_str();
+ let item_name_chars = item_name.chars().count();
+
+ if count_match_end(item_name, name).char_count == item_name_chars {
+ span_lint(
+ cx,
+ ENUM_VARIANT_NAMES,
+ variant.span,
+ "variant name ends with the enum's name",
+ );
+ }
}
-fn check_variant(
- cx: &EarlyContext<'_>,
- threshold: u64,
- def: &EnumDef,
- item_name: &str,
- item_name_chars: usize,
- span: Span,
- lint: &'static Lint,
-) {
+fn check_variant(cx: &LateContext<'_>, threshold: u64, def: &EnumDef<'_>, item_name: &str, span: Span) {
if (def.variants.len() as u64) < threshold {
return;
}
- 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");
- }
- if partial_rmatch(item_name, &name) == item_name_chars {
- span_lint(cx, lint, 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 {
+ let mut pre = camel_case_split(first);
+ let mut post = pre.clone();
+ post.reverse();
+ for var in def.variants {
+ check_enum_start(cx, item_name, var);
+ check_enum_end(cx, item_name, var);
let name = var.ident.name.as_str();
- let pre_match = partial_match(pre, &name);
- pre = &pre[..pre_match];
- let pre_camel = camel_case::until(pre);
- pre = &pre[..pre_camel];
- while let Some((next, last)) = name[pre.len()..].chars().zip(pre.chars().rev()).next() {
- if next.is_numeric() {
- return;
- }
- if next.is_lowercase() {
- let last = pre.len() - last.len_utf8();
- let last_camel = camel_case::until(&pre[..last]);
- pre = &pre[..last_camel];
- } else {
- break;
- }
+ let variant_split = camel_case_split(name);
+ if variant_split.len() == 1 {
+ return;
}
- let post_match = partial_rmatch(post, &name);
- let post_end = post.len() - post_match;
- post = &post[post_end..];
- let post_camel = camel_case::from(post);
- post = &post[post_camel..];
+ pre = pre
+ .iter()
+ .zip(variant_split.iter())
+ .take_while(|(a, b)| a == b)
+ .map(|e| *e.0)
+ .collect();
+ post = post
+ .iter()
+ .zip(variant_split.iter().rev())
+ .take_while(|(a, b)| a == b)
+ .map(|e| *e.0)
+ .collect();
}
let (what, value) = match (pre.is_empty(), post.is_empty()) {
(true, true) => return,
- (false, _) => ("pre", pre),
- (true, false) => ("post", post),
+ (false, _) => ("pre", pre.join("")),
+ (true, false) => {
+ post.reverse();
+ ("post", post.join(""))
+ },
};
span_lint_and_help(
cx,
- lint,
+ ENUM_VARIANT_NAMES,
span,
&format!("all variants have the same {}fix: `{}`", what, value),
None,
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);
+ let item_camel = to_camel_case(item_name);
if !item.span.from_expansion() && is_present_in_source(cx, item.span) {
if let Some(&(ref mod_name, ref mod_camel)) = self.modules.last() {
// constants don't have surrounding modules
);
}
}
- if item.vis.kind.is_pub() {
- let matching = partial_match(mod_camel, &item_camel);
- let rmatching = partial_rmatch(mod_camel, &item_camel);
+ // The `module_name_repetitions` lint should only trigger if the item has the module in its
+ // name. Having the same name is accepted.
+ if item.vis.node.is_pub() && item_camel.len() > mod_camel.len() {
+ let matching = count_match_start(mod_camel, &item_camel);
+ let rmatching = count_match_end(mod_camel, &item_camel);
let nchars = mod_camel.chars().count();
let is_word_beginning = |c: char| c == '_' || c.is_uppercase() || c.is_numeric();
- if matching == nchars {
+ if matching.char_count == nchars {
match item_camel.chars().nth(nchars) {
Some(c) if is_word_beginning(c) => span_lint(
cx,
_ => (),
}
}
- if rmatching == nchars {
+ if rmatching.char_count == nchars {
span_lint(
cx,
MODULE_NAME_REPETITIONS,
}
}
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.def_id)) {
+ check_variant(cx, self.threshold, def, item_name, item.span);
+ }
}
self.modules.push((item.ident.name, item_camel));
}