Auto merge of #8127 - dswij:8090, r=xFrednet
Fix `enum_variants` FP on prefixes that are not camel-case closes #8090 Fix FP on `enum_variants` when prefixes are only a substring of a camel-case word. Also adds some util helpers on `str_utils` to help parsing camel-case strings. This changes how the lint behaves: 1. previously if the Prefix is only a length of 1, it's going to get ignored, i.e. these were previously ignored and now is warned ```rust enum Foo { cFoo, cBar, cBaz, } enum Something { CCall, CCreate, CCryogenize, } ``` 2. non-ascii characters that doesn't have casing will not be split, ```rust enum NonCaps { PrefixXXX, PrefixTea, PrefixCake, } ``` will be considered as `PrefixXXX`, `Prefix`, `Prefix`, so this won't lint as opposed to fired previously. changelog: [`enum_variant_names`] Fix FP when first prefix are only a substring of a camel-case word. --- (Edited by `@xFrednet` removed some non ascii characters)
This commit is contained in:
commit
56ccd30a27
|
@ -2,8 +2,8 @@
|
||||||
|
|
||||||
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
|
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
|
||||||
use clippy_utils::source::is_present_in_source;
|
use clippy_utils::source::is_present_in_source;
|
||||||
use clippy_utils::str_utils::{self, count_match_end, count_match_start};
|
use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start};
|
||||||
use rustc_hir::{EnumDef, Item, ItemKind};
|
use rustc_hir::{EnumDef, Item, ItemKind, Variant};
|
||||||
use rustc_lint::{LateContext, LateLintPass};
|
use rustc_lint::{LateContext, LateLintPass};
|
||||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||||
use rustc_span::source_map::Span;
|
use rustc_span::source_map::Span;
|
||||||
|
@ -18,6 +18,12 @@ declare_clippy_lint! {
|
||||||
/// Enumeration variant names should specify their variant,
|
/// Enumeration variant names should specify their variant,
|
||||||
/// not repeat the enumeration name.
|
/// not repeat the enumeration name.
|
||||||
///
|
///
|
||||||
|
/// ### 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
|
/// ```rust
|
||||||
/// enum Cake {
|
/// enum Cake {
|
||||||
|
@ -120,72 +126,73 @@ impl_lint_pass!(EnumVariantNames => [
|
||||||
MODULE_INCEPTION
|
MODULE_INCEPTION
|
||||||
]);
|
]);
|
||||||
|
|
||||||
fn check_variant(
|
fn check_enum_start(cx: &LateContext<'_>, item_name: &str, variant: &Variant<'_>) {
|
||||||
cx: &LateContext<'_>,
|
let name = variant.ident.name.as_str();
|
||||||
threshold: u64,
|
let item_name_chars = item_name.chars().count();
|
||||||
def: &EnumDef<'_>,
|
|
||||||
item_name: &str,
|
if count_match_start(item_name, &name).char_count == item_name_chars
|
||||||
item_name_chars: usize,
|
&& name.chars().nth(item_name_chars).map_or(false, |c| !c.is_lowercase())
|
||||||
span: Span,
|
&& 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",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
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: &LateContext<'_>, threshold: u64, def: &EnumDef<'_>, item_name: &str, span: Span) {
|
||||||
if (def.variants.len() as u64) < threshold {
|
if (def.variants.len() as u64) < threshold {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
for var in def.variants {
|
|
||||||
let name = var.ident.name.as_str();
|
|
||||||
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,
|
|
||||||
var.span,
|
|
||||||
"variant name starts with the enum's name",
|
|
||||||
);
|
|
||||||
}
|
|
||||||
if count_match_end(item_name, &name).char_count == item_name_chars {
|
|
||||||
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 first = &def.variants[0].ident.name.as_str();
|
||||||
let mut pre = &first[..str_utils::camel_case_until(&*first).byte_index];
|
let mut pre = camel_case_split(first);
|
||||||
let mut post = &first[str_utils::camel_case_start(&*first).byte_index..];
|
let mut post = pre.clone();
|
||||||
|
post.reverse();
|
||||||
for var in def.variants {
|
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 name = var.ident.name.as_str();
|
||||||
|
|
||||||
let pre_match = count_match_start(pre, &name).byte_count;
|
let variant_split = camel_case_split(&name);
|
||||||
pre = &pre[..pre_match];
|
|
||||||
let pre_camel = str_utils::camel_case_until(pre).byte_index;
|
|
||||||
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 = str_utils::camel_case_until(&pre[..last]);
|
|
||||||
pre = &pre[..last_camel.byte_index];
|
|
||||||
} else {
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
let post_match = count_match_end(post, &name);
|
pre = pre
|
||||||
let post_end = post.len() - post_match.byte_count;
|
.iter()
|
||||||
post = &post[post_end..];
|
.zip(variant_split.iter())
|
||||||
let post_camel = str_utils::camel_case_start(post);
|
.take_while(|(a, b)| a == b)
|
||||||
post = &post[post_camel.byte_index..];
|
.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()) {
|
let (what, value) = match (pre.is_empty(), post.is_empty()) {
|
||||||
(true, true) => return,
|
(true, true) => return,
|
||||||
(false, _) => ("pre", pre),
|
(false, _) => ("pre", pre.join("")),
|
||||||
(true, false) => ("post", post),
|
(true, false) => {
|
||||||
|
post.reverse();
|
||||||
|
("post", post.join(""))
|
||||||
|
},
|
||||||
};
|
};
|
||||||
span_lint_and_help(
|
span_lint_and_help(
|
||||||
cx,
|
cx,
|
||||||
|
@ -233,7 +240,6 @@ impl LateLintPass<'_> for EnumVariantNames {
|
||||||
#[allow(clippy::similar_names)]
|
#[allow(clippy::similar_names)]
|
||||||
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
|
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
|
||||||
let item_name = item.ident.name.as_str();
|
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 !item.span.from_expansion() && is_present_in_source(cx, item.span) {
|
||||||
if let Some(&(ref mod_name, ref mod_camel)) = self.modules.last() {
|
if let Some(&(ref mod_name, ref mod_camel)) = self.modules.last() {
|
||||||
|
@ -283,7 +289,7 @@ impl LateLintPass<'_> for EnumVariantNames {
|
||||||
}
|
}
|
||||||
if let ItemKind::Enum(ref def, _) = item.kind {
|
if let ItemKind::Enum(ref def, _) = item.kind {
|
||||||
if !(self.avoid_breaking_exported_api && cx.access_levels.is_exported(item.def_id)) {
|
if !(self.avoid_breaking_exported_api && cx.access_levels.is_exported(item.def_id)) {
|
||||||
check_variant(cx, self.threshold, def, &item_name, item_name_chars, item.span);
|
check_variant(cx, self.threshold, def, &item_name, item.span);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
self.modules.push((item.ident.name, item_camel));
|
self.modules.push((item.ident.name, item_camel));
|
||||||
|
|
|
@ -68,6 +68,20 @@ pub fn camel_case_until(s: &str) -> StrIndex {
|
||||||
/// ```
|
/// ```
|
||||||
#[must_use]
|
#[must_use]
|
||||||
pub fn camel_case_start(s: &str) -> StrIndex {
|
pub fn camel_case_start(s: &str) -> StrIndex {
|
||||||
|
camel_case_start_from_idx(s, 0)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Returns `StrIndex` of the last camel-case component of `s[idx..]`.
|
||||||
|
///
|
||||||
|
/// ```
|
||||||
|
/// # use clippy_utils::str_utils::{camel_case_start_from_idx, StrIndex};
|
||||||
|
/// assert_eq!(camel_case_start_from_idx("AbcDef", 0), StrIndex::new(0, 0));
|
||||||
|
/// assert_eq!(camel_case_start_from_idx("AbcDef", 1), StrIndex::new(3, 3));
|
||||||
|
/// assert_eq!(camel_case_start_from_idx("AbcDefGhi", 0), StrIndex::new(0, 0));
|
||||||
|
/// assert_eq!(camel_case_start_from_idx("AbcDefGhi", 1), StrIndex::new(3, 3));
|
||||||
|
/// assert_eq!(camel_case_start_from_idx("Abcdefg", 1), StrIndex::new(7, 7));
|
||||||
|
/// ```
|
||||||
|
pub fn camel_case_start_from_idx(s: &str, start_idx: usize) -> StrIndex {
|
||||||
let char_count = s.chars().count();
|
let char_count = s.chars().count();
|
||||||
let range = 0..char_count;
|
let range = 0..char_count;
|
||||||
let mut iter = range.rev().zip(s.char_indices().rev());
|
let mut iter = range.rev().zip(s.char_indices().rev());
|
||||||
|
@ -78,9 +92,13 @@ pub fn camel_case_start(s: &str) -> StrIndex {
|
||||||
} else {
|
} else {
|
||||||
return StrIndex::new(char_count, s.len());
|
return StrIndex::new(char_count, s.len());
|
||||||
}
|
}
|
||||||
|
|
||||||
let mut down = true;
|
let mut down = true;
|
||||||
let mut last_index = StrIndex::new(char_count, s.len());
|
let mut last_index = StrIndex::new(char_count, s.len());
|
||||||
for (char_index, (byte_index, c)) in iter {
|
for (char_index, (byte_index, c)) in iter {
|
||||||
|
if byte_index < start_idx {
|
||||||
|
break;
|
||||||
|
}
|
||||||
if down {
|
if down {
|
||||||
if c.is_uppercase() {
|
if c.is_uppercase() {
|
||||||
down = false;
|
down = false;
|
||||||
|
@ -98,9 +116,55 @@ pub fn camel_case_start(s: &str) -> StrIndex {
|
||||||
return last_index;
|
return last_index;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
last_index
|
last_index
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Get the indexes of camel case components of a string `s`
|
||||||
|
///
|
||||||
|
/// ```
|
||||||
|
/// # use clippy_utils::str_utils::{camel_case_indices, StrIndex};
|
||||||
|
/// assert_eq!(
|
||||||
|
/// camel_case_indices("AbcDef"),
|
||||||
|
/// vec![StrIndex::new(0, 0), StrIndex::new(3, 3), StrIndex::new(6, 6)]
|
||||||
|
/// );
|
||||||
|
/// assert_eq!(
|
||||||
|
/// camel_case_indices("abcDef"),
|
||||||
|
/// vec![StrIndex::new(3, 3), StrIndex::new(6, 6)]
|
||||||
|
/// );
|
||||||
|
/// ```
|
||||||
|
pub fn camel_case_indices(s: &str) -> Vec<StrIndex> {
|
||||||
|
let mut result = Vec::new();
|
||||||
|
let mut str_idx = camel_case_start(s);
|
||||||
|
|
||||||
|
while str_idx.byte_index < s.len() {
|
||||||
|
let next_idx = str_idx.byte_index + 1;
|
||||||
|
result.push(str_idx);
|
||||||
|
str_idx = camel_case_start_from_idx(s, next_idx);
|
||||||
|
}
|
||||||
|
result.push(str_idx);
|
||||||
|
|
||||||
|
result
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Split camel case string into a vector of its components
|
||||||
|
///
|
||||||
|
/// ```
|
||||||
|
/// # use clippy_utils::str_utils::{camel_case_split, StrIndex};
|
||||||
|
/// assert_eq!(camel_case_split("AbcDef"), vec!["Abc", "Def"]);
|
||||||
|
/// ```
|
||||||
|
pub fn camel_case_split(s: &str) -> Vec<&str> {
|
||||||
|
let mut offsets = camel_case_indices(s)
|
||||||
|
.iter()
|
||||||
|
.map(|e| e.byte_index)
|
||||||
|
.collect::<Vec<usize>>();
|
||||||
|
if offsets[0] != 0 {
|
||||||
|
offsets.insert(0, 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
offsets.windows(2).map(|w| &s[w[0]..w[1]]).collect()
|
||||||
|
}
|
||||||
|
|
||||||
/// Dealing with sting comparison can be complicated, this struct ensures that both the
|
/// Dealing with sting comparison can be complicated, this struct ensures that both the
|
||||||
/// character and byte count are provided for correct indexing.
|
/// character and byte count are provided for correct indexing.
|
||||||
#[derive(Debug, Default, PartialEq, Eq)]
|
#[derive(Debug, Default, PartialEq, Eq)]
|
||||||
|
@ -231,4 +295,31 @@ mod test {
|
||||||
fn until_caps() {
|
fn until_caps() {
|
||||||
assert_eq!(camel_case_until("ABCD"), StrIndex::new(0, 0));
|
assert_eq!(camel_case_until("ABCD"), StrIndex::new(0, 0));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn camel_case_start_from_idx_full() {
|
||||||
|
assert_eq!(camel_case_start_from_idx("AbcDef", 0), StrIndex::new(0, 0));
|
||||||
|
assert_eq!(camel_case_start_from_idx("AbcDef", 1), StrIndex::new(3, 3));
|
||||||
|
assert_eq!(camel_case_start_from_idx("AbcDef", 4), StrIndex::new(6, 6));
|
||||||
|
assert_eq!(camel_case_start_from_idx("AbcDefGhi", 0), StrIndex::new(0, 0));
|
||||||
|
assert_eq!(camel_case_start_from_idx("AbcDefGhi", 1), StrIndex::new(3, 3));
|
||||||
|
assert_eq!(camel_case_start_from_idx("Abcdefg", 1), StrIndex::new(7, 7));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn camel_case_indices_full() {
|
||||||
|
assert_eq!(camel_case_indices("Abc\u{f6}\u{f6}DD"), vec![StrIndex::new(7, 9)]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn camel_case_split_full() {
|
||||||
|
assert_eq!(camel_case_split("A"), vec!["A"]);
|
||||||
|
assert_eq!(camel_case_split("AbcDef"), vec!["Abc", "Def"]);
|
||||||
|
assert_eq!(camel_case_split("Abc"), vec!["Abc"]);
|
||||||
|
assert_eq!(camel_case_split("abcDef"), vec!["abc", "Def"]);
|
||||||
|
assert_eq!(
|
||||||
|
camel_case_split("\u{f6}\u{f6}AabABcd"),
|
||||||
|
vec!["\u{f6}\u{f6}", "Aab", "A", "Bcd"]
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -145,4 +145,10 @@ enum HIDataRequest {
|
||||||
DeleteUnpubHIData(String),
|
DeleteUnpubHIData(String),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
enum North {
|
||||||
|
Normal,
|
||||||
|
NoLeft,
|
||||||
|
NoRight,
|
||||||
|
}
|
||||||
|
|
||||||
fn main() {}
|
fn main() {}
|
||||||
|
|
|
@ -6,6 +6,18 @@ LL | cFoo,
|
||||||
|
|
|
|
||||||
= note: `-D clippy::enum-variant-names` implied by `-D warnings`
|
= note: `-D clippy::enum-variant-names` implied by `-D warnings`
|
||||||
|
|
||||||
|
error: all variants have the same prefix: `c`
|
||||||
|
--> $DIR/enum_variants.rs:14:1
|
||||||
|
|
|
||||||
|
LL | / enum Foo {
|
||||||
|
LL | | cFoo,
|
||||||
|
LL | | cBar,
|
||||||
|
LL | | cBaz,
|
||||||
|
LL | | }
|
||||||
|
| |_^
|
||||||
|
|
|
||||||
|
= help: remove the prefixes and use full paths to the variants instead of glob imports
|
||||||
|
|
||||||
error: variant name starts with the enum's name
|
error: variant name starts with the enum's name
|
||||||
--> $DIR/enum_variants.rs:26:5
|
--> $DIR/enum_variants.rs:26:5
|
||||||
|
|
|
|
||||||
|
@ -60,6 +72,18 @@ LL | | }
|
||||||
|
|
|
|
||||||
= help: remove the prefixes and use full paths to the variants instead of glob imports
|
= help: remove the prefixes and use full paths to the variants instead of glob imports
|
||||||
|
|
||||||
|
error: all variants have the same prefix: `C`
|
||||||
|
--> $DIR/enum_variants.rs:59:1
|
||||||
|
|
|
||||||
|
LL | / enum Something {
|
||||||
|
LL | | CCall,
|
||||||
|
LL | | CCreate,
|
||||||
|
LL | | CCryogenize,
|
||||||
|
LL | | }
|
||||||
|
| |_^
|
||||||
|
|
|
||||||
|
= help: remove the prefixes and use full paths to the variants instead of glob imports
|
||||||
|
|
||||||
error: all variants have the same prefix: `WithOut`
|
error: all variants have the same prefix: `WithOut`
|
||||||
--> $DIR/enum_variants.rs:81:1
|
--> $DIR/enum_variants.rs:81:1
|
||||||
|
|
|
|
||||||
|
@ -72,18 +96,6 @@ LL | | }
|
||||||
|
|
|
|
||||||
= help: remove the prefixes and use full paths to the variants instead of glob imports
|
= help: remove the prefixes and use full paths to the variants instead of glob imports
|
||||||
|
|
||||||
error: all variants have the same prefix: `Prefix`
|
|
||||||
--> $DIR/enum_variants.rs:87:1
|
|
||||||
|
|
|
||||||
LL | / enum NonCaps {
|
|
||||||
LL | | Prefix的,
|
|
||||||
LL | | PrefixTea,
|
|
||||||
LL | | PrefixCake,
|
|
||||||
LL | | }
|
|
||||||
| |_^
|
|
||||||
|
|
|
||||||
= help: remove the prefixes and use full paths to the variants instead of glob imports
|
|
||||||
|
|
||||||
error: all variants have the same postfix: `IData`
|
error: all variants have the same postfix: `IData`
|
||||||
--> $DIR/enum_variants.rs:136:1
|
--> $DIR/enum_variants.rs:136:1
|
||||||
|
|
|
|
||||||
|
@ -108,5 +120,5 @@ LL | | }
|
||||||
|
|
|
|
||||||
= help: remove the postfixes and use full paths to the variants instead of glob imports
|
= help: remove the postfixes and use full paths to the variants instead of glob imports
|
||||||
|
|
||||||
error: aborting due to 11 previous errors
|
error: aborting due to 12 previous errors
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue