mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-23 05:03:21 +00:00
use a multispan for MANY_SINGLE_CHAR_NAMES
This commit is contained in:
parent
17e04ac751
commit
0d50d44ea6
5 changed files with 142 additions and 45 deletions
|
@ -88,7 +88,33 @@ struct SimilarNamesLocalVisitor<'a, 'tcx: 'a> {
|
|||
names: Vec<ExistingName>,
|
||||
cx: &'a EarlyContext<'tcx>,
|
||||
lint: &'a NonExpressiveNames,
|
||||
single_char_names: Vec<char>,
|
||||
|
||||
/// A stack of scopes containing the single-character bindings in each scope.
|
||||
single_char_names: Vec<Vec<Ident>>,
|
||||
}
|
||||
|
||||
impl<'a, 'tcx: 'a> SimilarNamesLocalVisitor<'a, 'tcx> {
|
||||
fn check_single_char_names(&self) {
|
||||
let num_single_char_names = self.single_char_names.iter().flatten().count();
|
||||
let threshold = self.lint.single_char_binding_names_threshold;
|
||||
if num_single_char_names as u64 >= threshold {
|
||||
let span = self
|
||||
.single_char_names
|
||||
.iter()
|
||||
.flatten()
|
||||
.map(|ident| ident.span)
|
||||
.collect::<Vec<_>>();
|
||||
span_lint(
|
||||
self.cx,
|
||||
MANY_SINGLE_CHAR_NAMES,
|
||||
span,
|
||||
&format!(
|
||||
"{} bindings with single-character names in scope",
|
||||
num_single_char_names
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// this list contains lists of names that are allowed to be similar
|
||||
|
@ -109,7 +135,7 @@ struct SimilarNamesNameVisitor<'a: 'b, 'tcx: 'a, 'b>(&'b mut SimilarNamesLocalVi
|
|||
impl<'a, 'tcx: 'a, 'b> Visitor<'tcx> for SimilarNamesNameVisitor<'a, 'tcx, 'b> {
|
||||
fn visit_pat(&mut self, pat: &'tcx Pat) {
|
||||
match pat.node {
|
||||
PatKind::Ident(_, ident, _) => self.check_name(ident.span, ident.name),
|
||||
PatKind::Ident(_, ident, _) => self.check_ident(ident),
|
||||
PatKind::Struct(_, ref fields, _) => {
|
||||
for field in fields {
|
||||
if !field.node.is_shorthand {
|
||||
|
@ -140,27 +166,24 @@ fn whitelisted(interned_name: &str, list: &[&str]) -> bool {
|
|||
}
|
||||
|
||||
impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
|
||||
fn check_short_name(&mut self, c: char, span: Span) {
|
||||
// make sure we ignore shadowing
|
||||
if self.0.single_char_names.contains(&c) {
|
||||
fn check_short_ident(&mut self, ident: Ident) {
|
||||
// Ignore shadowing
|
||||
if self
|
||||
.0
|
||||
.single_char_names
|
||||
.iter()
|
||||
.flatten()
|
||||
.any(|id| id.name == ident.name)
|
||||
{
|
||||
return;
|
||||
}
|
||||
self.0.single_char_names.push(c);
|
||||
if self.0.single_char_names.len() as u64 >= self.0.lint.single_char_binding_names_threshold {
|
||||
span_lint(
|
||||
self.0.cx,
|
||||
MANY_SINGLE_CHAR_NAMES,
|
||||
span,
|
||||
&format!(
|
||||
"{}th binding whose name is just one char",
|
||||
self.0.single_char_names.len()
|
||||
),
|
||||
);
|
||||
} else if let Some(scope) = &mut self.0.single_char_names.last_mut() {
|
||||
scope.push(ident);
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_lines)]
|
||||
fn check_name(&mut self, span: Span, name: Name) {
|
||||
let interned_name = name.as_str();
|
||||
fn check_ident(&mut self, ident: Ident) {
|
||||
let interned_name = ident.name.as_str();
|
||||
if interned_name.chars().any(char::is_uppercase) {
|
||||
return;
|
||||
}
|
||||
|
@ -168,7 +191,7 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
|
|||
span_lint(
|
||||
self.0.cx,
|
||||
JUST_UNDERSCORES_AND_DIGITS,
|
||||
span,
|
||||
ident.span,
|
||||
"consider choosing a more descriptive name",
|
||||
);
|
||||
return;
|
||||
|
@ -176,8 +199,7 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
|
|||
let count = interned_name.chars().count();
|
||||
if count < 3 {
|
||||
if count == 1 {
|
||||
let c = interned_name.chars().next().expect("already checked");
|
||||
self.check_short_name(c, span);
|
||||
self.check_short_ident(ident);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
@ -247,13 +269,13 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
|
|||
span_lint_and_then(
|
||||
self.0.cx,
|
||||
SIMILAR_NAMES,
|
||||
span,
|
||||
ident.span,
|
||||
"binding's name is too similar to existing binding",
|
||||
|diag| {
|
||||
diag.span_note(existing_name.span, "existing binding defined here");
|
||||
if let Some(split) = split_at {
|
||||
diag.span_help(
|
||||
span,
|
||||
ident.span,
|
||||
&format!(
|
||||
"separate the discriminating character by an \
|
||||
underscore like: `{}_{}`",
|
||||
|
@ -269,7 +291,7 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
|
|||
self.0.names.push(ExistingName {
|
||||
whitelist: get_whitelist(&interned_name).unwrap_or(&[]),
|
||||
interned: interned_name,
|
||||
span,
|
||||
span: ident.span,
|
||||
len: count,
|
||||
});
|
||||
}
|
||||
|
@ -297,15 +319,25 @@ impl<'a, 'tcx> Visitor<'tcx> for SimilarNamesLocalVisitor<'a, 'tcx> {
|
|||
SimilarNamesNameVisitor(self).visit_pat(&*local.pat);
|
||||
}
|
||||
fn visit_block(&mut self, blk: &'tcx Block) {
|
||||
self.single_char_names.push(vec![]);
|
||||
|
||||
self.apply(|this| walk_block(this, blk));
|
||||
|
||||
self.check_single_char_names();
|
||||
self.single_char_names.pop();
|
||||
}
|
||||
fn visit_arm(&mut self, arm: &'tcx Arm) {
|
||||
self.single_char_names.push(vec![]);
|
||||
|
||||
self.apply(|this| {
|
||||
// just go through the first pattern, as either all patterns
|
||||
// bind the same bindings or rustc would have errored much earlier
|
||||
SimilarNamesNameVisitor(this).visit_pat(&arm.pats[0]);
|
||||
this.apply(|this| walk_expr(this, &arm.body));
|
||||
});
|
||||
|
||||
self.check_single_char_names();
|
||||
self.single_char_names.pop();
|
||||
}
|
||||
fn visit_item(&mut self, _: &Item) {
|
||||
// do not recurse into inner items
|
||||
|
@ -335,14 +367,17 @@ fn do_check(lint: &mut NonExpressiveNames, cx: &EarlyContext<'_>, attrs: &[Attri
|
|||
names: Vec::new(),
|
||||
cx,
|
||||
lint,
|
||||
single_char_names: Vec::new(),
|
||||
single_char_names: vec![vec![]],
|
||||
};
|
||||
|
||||
// initialize with function arguments
|
||||
for arg in &decl.inputs {
|
||||
SimilarNamesNameVisitor(&mut visitor).visit_pat(&arg.pat);
|
||||
}
|
||||
// walk all other bindings
|
||||
walk_block(&mut visitor, blk);
|
||||
|
||||
visitor.check_single_char_names();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -5,7 +5,7 @@ use rustc::lint::{LateContext, Lint, LintContext};
|
|||
use rustc_errors::{Applicability, CodeSuggestion, Substitution, SubstitutionPart, SuggestionStyle};
|
||||
use std::env;
|
||||
use syntax::errors::DiagnosticBuilder;
|
||||
use syntax::source_map::Span;
|
||||
use syntax::source_map::{MultiSpan, Span};
|
||||
|
||||
/// Wrapper around `DiagnosticBuilder` that adds a link to Clippy documentation for the emitted lint
|
||||
struct DiagnosticWrapper<'a>(DiagnosticBuilder<'a>);
|
||||
|
@ -48,7 +48,7 @@ impl<'a> DiagnosticWrapper<'a> {
|
|||
/// 17 | std::mem::forget(seven);
|
||||
/// | ^^^^^^^^^^^^^^^^^^^^^^^
|
||||
/// ```
|
||||
pub fn span_lint<'a, T: LintContext<'a>>(cx: &T, lint: &'static Lint, sp: Span, msg: &str) {
|
||||
pub fn span_lint<'a, T: LintContext<'a>>(cx: &T, lint: &'static Lint, sp: impl Into<MultiSpan>, msg: &str) {
|
||||
DiagnosticWrapper(cx.struct_span_lint(lint, sp, msg)).docs_link(lint);
|
||||
}
|
||||
|
||||
|
|
|
@ -49,6 +49,45 @@ fn bla() {
|
|||
}
|
||||
}
|
||||
|
||||
fn bindings(a: i32, b: i32, c: i32, d: i32, e: i32, f: i32, g: i32, h: i32) {}
|
||||
|
||||
fn bindings2() {
|
||||
let (a, b, c, d, e, f, g, h) = unimplemented!();
|
||||
}
|
||||
|
||||
fn shadowing() {
|
||||
let a = 0i32;
|
||||
let a = 0i32;
|
||||
let a = 0i32;
|
||||
let a = 0i32;
|
||||
let a = 0i32;
|
||||
let a = 0i32;
|
||||
{
|
||||
let a = 0i32;
|
||||
}
|
||||
}
|
||||
|
||||
fn patterns() {
|
||||
enum Z {
|
||||
A(i32),
|
||||
B(i32),
|
||||
C(i32),
|
||||
D(i32),
|
||||
E(i32),
|
||||
F(i32),
|
||||
}
|
||||
|
||||
// These should not trigger a warning, since the pattern bindings are a new scope.
|
||||
match Z::A(0) {
|
||||
Z::A(a) => {},
|
||||
Z::B(b) => {},
|
||||
Z::C(c) => {},
|
||||
Z::D(d) => {},
|
||||
Z::E(e) => {},
|
||||
Z::F(f) => {},
|
||||
}
|
||||
}
|
||||
|
||||
fn underscores_and_numbers() {
|
||||
let _1 = 1; //~ERROR Consider a more descriptive name
|
||||
let ____1 = 1; //~ERROR Consider a more descriptive name
|
||||
|
|
|
@ -1,31 +1,54 @@
|
|||
error: 5th binding whose name is just one char
|
||||
--> $DIR/non_expressive_names.rs:35:17
|
||||
error: 5 bindings with single-character names in scope
|
||||
--> $DIR/non_expressive_names.rs:27:9
|
||||
|
|
||||
LL | let a: i32;
|
||||
| ^
|
||||
LL | let (b, c, d): (i32, i64, i16);
|
||||
| ^ ^ ^
|
||||
...
|
||||
LL | let e: i32;
|
||||
| ^
|
||||
|
|
||||
= note: `-D clippy::many-single-char-names` implied by `-D warnings`
|
||||
|
||||
error: 5th binding whose name is just one char
|
||||
--> $DIR/non_expressive_names.rs:38:17
|
||||
error: 6 bindings with single-character names in scope
|
||||
--> $DIR/non_expressive_names.rs:27:9
|
||||
|
|
||||
LL | let a: i32;
|
||||
| ^
|
||||
LL | let (b, c, d): (i32, i64, i16);
|
||||
| ^ ^ ^
|
||||
...
|
||||
LL | let e: i32;
|
||||
| ^
|
||||
|
||||
error: 6th binding whose name is just one char
|
||||
--> $DIR/non_expressive_names.rs:39:17
|
||||
|
|
||||
LL | let f: i32;
|
||||
| ^
|
||||
|
||||
error: 5th binding whose name is just one char
|
||||
--> $DIR/non_expressive_names.rs:43:13
|
||||
error: 5 bindings with single-character names in scope
|
||||
--> $DIR/non_expressive_names.rs:27:9
|
||||
|
|
||||
LL | let a: i32;
|
||||
| ^
|
||||
LL | let (b, c, d): (i32, i64, i16);
|
||||
| ^ ^ ^
|
||||
...
|
||||
LL | e => panic!(),
|
||||
| ^
|
||||
|
||||
error: 8 bindings with single-character names in scope
|
||||
--> $DIR/non_expressive_names.rs:52:13
|
||||
|
|
||||
LL | fn bindings(a: i32, b: i32, c: i32, d: i32, e: i32, f: i32, g: i32, h: i32) {}
|
||||
| ^ ^ ^ ^ ^ ^ ^ ^
|
||||
|
||||
error: 8 bindings with single-character names in scope
|
||||
--> $DIR/non_expressive_names.rs:55:10
|
||||
|
|
||||
LL | let (a, b, c, d, e, f, g, h) = unimplemented!();
|
||||
| ^ ^ ^ ^ ^ ^ ^ ^
|
||||
|
||||
error: consider choosing a more descriptive name
|
||||
--> $DIR/non_expressive_names.rs:53:9
|
||||
--> $DIR/non_expressive_names.rs:92:9
|
||||
|
|
||||
LL | let _1 = 1; //~ERROR Consider a more descriptive name
|
||||
| ^^
|
||||
|
@ -33,34 +56,34 @@ LL | let _1 = 1; //~ERROR Consider a more descriptive name
|
|||
= note: `-D clippy::just-underscores-and-digits` implied by `-D warnings`
|
||||
|
||||
error: consider choosing a more descriptive name
|
||||
--> $DIR/non_expressive_names.rs:54:9
|
||||
--> $DIR/non_expressive_names.rs:93:9
|
||||
|
|
||||
LL | let ____1 = 1; //~ERROR Consider a more descriptive name
|
||||
| ^^^^^
|
||||
|
||||
error: consider choosing a more descriptive name
|
||||
--> $DIR/non_expressive_names.rs:55:9
|
||||
--> $DIR/non_expressive_names.rs:94:9
|
||||
|
|
||||
LL | let __1___2 = 12; //~ERROR Consider a more descriptive name
|
||||
| ^^^^^^^
|
||||
|
||||
error: consider choosing a more descriptive name
|
||||
--> $DIR/non_expressive_names.rs:75:13
|
||||
--> $DIR/non_expressive_names.rs:114:13
|
||||
|
|
||||
LL | let _1 = 1;
|
||||
| ^^
|
||||
|
||||
error: consider choosing a more descriptive name
|
||||
--> $DIR/non_expressive_names.rs:76:13
|
||||
--> $DIR/non_expressive_names.rs:115:13
|
||||
|
|
||||
LL | let ____1 = 1;
|
||||
| ^^^^^
|
||||
|
||||
error: consider choosing a more descriptive name
|
||||
--> $DIR/non_expressive_names.rs:77:13
|
||||
--> $DIR/non_expressive_names.rs:116:13
|
||||
|
|
||||
LL | let __1___2 = 12;
|
||||
| ^^^^^^^
|
||||
|
||||
error: aborting due to 10 previous errors
|
||||
error: aborting due to 11 previous errors
|
||||
|
||||
|
|
0
tests/ui/non_expressive_names.stdout
Normal file
0
tests/ui/non_expressive_names.stdout
Normal file
Loading…
Reference in a new issue