Skip to content

Commit 0625183

Browse files
committed
Auto merge of #12903 - Jarcho:issue_12284, r=y21
Fix ICE in `upper_case_acronyms` fixes #12284 The logic has been rewritten to avoid allocations. The old version allocated multiple vecs and strings for each identifier. The new logic allocates a single string only when the lint triggers. This also no longer lints on strings which don't start with an uppercase letter (e.g. `something_FOO`). changelog: none
2 parents 9550481 + 059eaf1 commit 0625183

File tree

2 files changed

+69
-43
lines changed

2 files changed

+69
-43
lines changed

clippy_lints/src/upper_case_acronyms.rs

+59-43
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_hir_and_then;
2-
use itertools::Itertools;
2+
use core::mem::replace;
33
use rustc_errors::Applicability;
44
use rustc_hir::{HirId, Item, ItemKind};
55
use rustc_lint::{LateContext, LateLintPass, LintContext};
@@ -56,55 +56,71 @@ impl UpperCaseAcronyms {
5656

5757
impl_lint_pass!(UpperCaseAcronyms => [UPPER_CASE_ACRONYMS]);
5858

59-
fn correct_ident(ident: &str) -> String {
60-
let ident = ident.chars().rev().collect::<String>();
61-
let fragments = ident
62-
.split_inclusive(|x: char| !x.is_ascii_lowercase())
63-
.rev()
64-
.map(|x| x.chars().rev().collect::<String>());
65-
66-
let mut ident = fragments.clone().next().unwrap();
67-
for (ref prev, ref curr) in fragments.tuple_windows() {
68-
if <[&String; 2]>::from((prev, curr))
69-
.iter()
70-
.all(|s| s.len() == 1 && s.chars().next().unwrap().is_ascii_uppercase())
71-
{
72-
ident.push_str(&curr.to_ascii_lowercase());
59+
fn contains_acronym(s: &str) -> bool {
60+
let mut count = 0;
61+
for c in s.chars() {
62+
if c.is_ascii_uppercase() {
63+
count += 1;
64+
if count == 3 {
65+
return true;
66+
}
7367
} else {
74-
ident.push_str(curr);
68+
count = 0;
7569
}
7670
}
77-
ident
71+
count == 2
7872
}
7973

8074
fn check_ident(cx: &LateContext<'_>, ident: &Ident, hir_id: HirId, be_aggressive: bool) {
81-
let span = ident.span;
82-
let ident = ident.as_str();
83-
let corrected = correct_ident(ident);
84-
// warn if we have pure-uppercase idents
85-
// assume that two-letter words are some kind of valid abbreviation like FP for false positive
86-
// (and don't warn)
87-
if (ident.chars().all(|c| c.is_ascii_uppercase()) && ident.len() > 2)
88-
// otherwise, warn if we have SOmeTHING lIKE THIs but only warn with the aggressive
89-
// upper-case-acronyms-aggressive config option enabled
90-
|| (be_aggressive && ident != corrected)
75+
let s = ident.as_str();
76+
77+
// By default, only warn for upper case identifiers with at least 3 characters.
78+
let replacement = if s.len() > 2 && s.bytes().all(|c| c.is_ascii_uppercase()) {
79+
let mut r = String::with_capacity(s.len());
80+
let mut s = s.chars();
81+
r.push(s.next().unwrap());
82+
r.extend(s.map(|c| c.to_ascii_lowercase()));
83+
r
84+
} else if be_aggressive
85+
// Only lint if the ident starts with an upper case character.
86+
&& let unprefixed = s.trim_start_matches('_')
87+
&& unprefixed.starts_with(|c: char| c.is_ascii_uppercase())
88+
&& contains_acronym(unprefixed)
9189
{
92-
span_lint_hir_and_then(
93-
cx,
94-
UPPER_CASE_ACRONYMS,
95-
hir_id,
96-
span,
97-
format!("name `{ident}` contains a capitalized acronym"),
98-
|diag| {
99-
diag.span_suggestion(
100-
span,
101-
"consider making the acronym lowercase, except the initial letter",
102-
corrected,
103-
Applicability::MaybeIncorrect,
104-
);
105-
},
106-
);
107-
}
90+
let mut r = String::with_capacity(s.len());
91+
let mut s = s.chars();
92+
let mut prev_upper = false;
93+
while let Some(c) = s.next() {
94+
r.push(
95+
if replace(&mut prev_upper, c.is_ascii_uppercase())
96+
&& s.clone().next().map_or(true, |c| c.is_ascii_uppercase())
97+
{
98+
c.to_ascii_lowercase()
99+
} else {
100+
c
101+
},
102+
);
103+
}
104+
r
105+
} else {
106+
return;
107+
};
108+
109+
span_lint_hir_and_then(
110+
cx,
111+
UPPER_CASE_ACRONYMS,
112+
hir_id,
113+
ident.span,
114+
format!("name `{ident}` contains a capitalized acronym"),
115+
|diag| {
116+
diag.span_suggestion(
117+
ident.span,
118+
"consider making the acronym lowercase, except the initial letter",
119+
replacement,
120+
Applicability::MaybeIncorrect,
121+
);
122+
},
123+
);
108124
}
109125

110126
impl LateLintPass<'_> for UpperCaseAcronyms {

tests/ui/crashes/ice-12284.rs

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#![allow(incomplete_features)]
2+
#![feature(unnamed_fields)]
3+
4+
#[repr(C)]
5+
struct Foo {
6+
_: struct {
7+
},
8+
}
9+
10+
fn main() {}

0 commit comments

Comments
 (0)