Skip to content

Commit 7c0d145

Browse files
euclioeuclio
committed
improve non_snake_case diagnostics
Use a structured suggestion and tighten the span to just the identifier.
1 parent 3dfe36d commit 7c0d145

25 files changed

+191
-179
lines changed

‎src/librustc_lint/nonstandard_style.rs

Lines changed: 88 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ use lint::{EarlyContext, LateContext, LintContext, LintArray};
77
use lint::{EarlyLintPass, LintPass, LateLintPass};
88
use syntax::ast;
99
use syntax::attr;
10-
use syntax_pos::Span;
10+
use syntax::errors::Applicability;
11+
use syntax_pos::{BytePos, symbol::Ident, Span};
1112

1213
#[derive(PartialEq)]
1314
pub enum MethodLateContext {
@@ -179,7 +180,8 @@ impl NonSnakeCase {
179180
words.join("_")
180181
}
181182

182-
fn check_snake_case(&self, cx: &LateContext, sort: &str, name: &str, span: Option<Span>) {
183+
/// Checks if a given identifier is snake case, and reports a diagnostic if not.
184+
fn check_snake_case(&self, cx: &LateContext, sort: &str, ident: &Ident) {
183185
fn is_snake_case(ident: &str) -> bool {
184186
if ident.is_empty() {
185187
return true;
@@ -201,20 +203,28 @@ impl NonSnakeCase {
201203
})
202204
}
203205

206+
let name = &ident.name.as_str();
207+
204208
if !is_snake_case(name) {
205209
let sc = NonSnakeCase::to_snake_case(name);
206-
let msg = if sc != name {
207-
format!("{} `{}` should have a snake case name such as `{}`",
208-
sort,
209-
name,
210-
sc)
210+
211+
let msg = format!("{} `{}` should have a snake case name", sort, name);
212+
let mut err = cx.struct_span_lint(NON_SNAKE_CASE, ident.span, &msg);
213+
214+
// We have a valid span in almost all cases, but we don't have one when linting a crate
215+
// name provided via the command line.
216+
if !ident.span.is_dummy() {
217+
err.span_suggestion_with_applicability(
218+
ident.span,
219+
"convert the identifier to snake case",
220+
sc,
221+
Applicability::MaybeIncorrect,
222+
);
211223
} else {
212-
format!("{} `{}` should have a snake case name", sort, name)
213-
};
214-
match span {
215-
Some(span) => cx.span_lint(NON_SNAKE_CASE, span, &msg),
216-
None => cx.lint(NON_SNAKE_CASE, &msg),
224+
err.help(&format!("convert the identifier to snake case: `{}`", sc));
217225
}
226+
227+
err.emit();
218228
}
219229
}
220230
}
@@ -227,87 +237,111 @@ impl LintPass for NonSnakeCase {
227237

228238
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonSnakeCase {
229239
fn check_crate(&mut self, cx: &LateContext, cr: &hir::Crate) {
230-
let attr_crate_name = attr::find_by_name(&cr.attrs, "crate_name")
231-
.and_then(|at| at.value_str().map(|s| (at, s)));
232-
if let Some(ref name) = cx.tcx.sess.opts.crate_name {
233-
self.check_snake_case(cx, "crate", name, None);
234-
} else if let Some((attr, name)) = attr_crate_name {
235-
self.check_snake_case(cx, "crate", &name.as_str(), Some(attr.span));
240+
let crate_ident = if let Some(name) = &cx.tcx.sess.opts.crate_name {
241+
Some(Ident::from_str(name))
242+
} else {
243+
attr::find_by_name(&cr.attrs, "crate_name")
244+
.and_then(|attr| attr.meta())
245+
.and_then(|meta| {
246+
meta.name_value_literal().and_then(|lit| {
247+
if let ast::LitKind::Str(name, ..) = lit.node {
248+
// Discard the double quotes surrounding the literal.
249+
let sp = cx.sess().source_map().span_to_snippet(lit.span)
250+
.ok()
251+
.and_then(|snippet| {
252+
let left = snippet.find('"')?;
253+
let right = snippet.rfind('"').map(|pos| snippet.len() - pos)?;
254+
255+
Some(
256+
lit.span
257+
.with_lo(lit.span.lo() + BytePos(left as u32 + 1))
258+
.with_hi(lit.span.hi() - BytePos(right as u32)),
259+
)
260+
})
261+
.unwrap_or_else(|| lit.span);
262+
263+
Some(Ident::new(name, sp))
264+
} else {
265+
None
266+
}
267+
})
268+
})
269+
};
270+
271+
if let Some(ident) = &crate_ident {
272+
self.check_snake_case(cx, "crate", ident);
236273
}
237274
}
238275

239276
fn check_generic_param(&mut self, cx: &LateContext, param: &hir::GenericParam) {
240-
match param.kind {
241-
GenericParamKind::Lifetime { .. } => {
242-
let name = param.name.ident().as_str();
243-
self.check_snake_case(cx, "lifetime", &name, Some(param.span));
244-
}
245-
GenericParamKind::Type { .. } => {}
277+
if let GenericParamKind::Lifetime { .. } = param.kind {
278+
self.check_snake_case(cx, "lifetime", &param.name.ident());
246279
}
247280
}
248281

249-
fn check_fn(&mut self,
250-
cx: &LateContext,
251-
fk: FnKind,
252-
_: &hir::FnDecl,
253-
_: &hir::Body,
254-
span: Span,
255-
id: ast::NodeId) {
256-
match fk {
257-
FnKind::Method(name, ..) => {
282+
fn check_fn(
283+
&mut self,
284+
cx: &LateContext,
285+
fk: FnKind,
286+
_: &hir::FnDecl,
287+
_: &hir::Body,
288+
_: Span,
289+
id: ast::NodeId,
290+
) {
291+
match &fk {
292+
FnKind::Method(ident, ..) => {
258293
match method_context(cx, id) {
259294
MethodLateContext::PlainImpl => {
260-
self.check_snake_case(cx, "method", &name.as_str(), Some(span))
295+
self.check_snake_case(cx, "method", ident);
261296
}
262297
MethodLateContext::TraitAutoImpl => {
263-
self.check_snake_case(cx, "trait method", &name.as_str(), Some(span))
298+
self.check_snake_case(cx, "trait method", ident);
264299
}
265300
_ => (),
266301
}
267302
}
268-
FnKind::ItemFn(name, _, header, _, attrs) => {
303+
FnKind::ItemFn(ident, _, header, _, attrs) => {
269304
// Skip foreign-ABI #[no_mangle] functions (Issue #31924)
270-
if header.abi != Abi::Rust && attr::find_by_name(attrs, "no_mangle").is_some() {
305+
if header.abi != Abi::Rust && attr::contains_name(attrs, "no_mangle") {
271306
return;
272307
}
273-
self.check_snake_case(cx, "function", &name.as_str(), Some(span))
308+
self.check_snake_case(cx, "function", ident);
274309
}
275310
FnKind::Closure(_) => (),
276311
}
277312
}
278313

279314
fn check_item(&mut self, cx: &LateContext, it: &hir::Item) {
280315
if let hir::ItemKind::Mod(_) = it.node {
281-
self.check_snake_case(cx, "module", &it.ident.as_str(), Some(it.span));
316+
self.check_snake_case(cx, "module", &it.ident);
282317
}
283318
}
284319

285320
fn check_trait_item(&mut self, cx: &LateContext, item: &hir::TraitItem) {
286-
if let hir::TraitItemKind::Method(_, hir::TraitMethod::Required(ref pnames)) = item.node {
287-
self.check_snake_case(cx,
288-
"trait method",
289-
&item.ident.as_str(),
290-
Some(item.span));
321+
if let hir::TraitItemKind::Method(_, hir::TraitMethod::Required(pnames)) = &item.node {
322+
self.check_snake_case(cx, "trait method", &item.ident);
291323
for param_name in pnames {
292-
self.check_snake_case(cx, "variable", &param_name.as_str(), Some(param_name.span));
324+
self.check_snake_case(cx, "variable", param_name);
293325
}
294326
}
295327
}
296328

297329
fn check_pat(&mut self, cx: &LateContext, p: &hir::Pat) {
298-
if let &PatKind::Binding(_, _, ref ident, _) = &p.node {
299-
self.check_snake_case(cx, "variable", &ident.as_str(), Some(p.span));
330+
if let &PatKind::Binding(_, _, ident, _) = &p.node {
331+
self.check_snake_case(cx, "variable", &ident);
300332
}
301333
}
302334

303-
fn check_struct_def(&mut self,
304-
cx: &LateContext,
305-
s: &hir::VariantData,
306-
_: ast::Name,
307-
_: &hir::Generics,
308-
_: ast::NodeId) {
335+
fn check_struct_def(
336+
&mut self,
337+
cx: &LateContext,
338+
s: &hir::VariantData,
339+
_: ast::Name,
340+
_: &hir::Generics,
341+
_: ast::NodeId,
342+
) {
309343
for sf in s.fields() {
310-
self.check_snake_case(cx, "structure field", &sf.ident.as_str(), Some(sf.span));
344+
self.check_snake_case(cx, "structure field", &sf.ident);
311345
}
312346
}
313347
}

‎src/libsyntax/diagnostics/plugin.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,16 +117,18 @@ pub fn expand_register_diagnostic<'cx>(ecx: &'cx mut ExtCtxt,
117117
));
118118
}
119119
});
120-
let sym = Ident::with_empty_ctxt(Symbol::gensym(&format!(
121-
"__register_diagnostic_{}", code
122-
)));
120+
121+
let span = span.apply_mark(ecx.current_expansion.mark);
122+
123+
let sym = Ident::new(Symbol::gensym(&format!("__register_diagnostic_{}", code)), span);
124+
123125
MacEager::items(smallvec![
124126
ecx.item_mod(
125127
span,
126128
span,
127129
sym,
128-
Vec::new(),
129-
Vec::new()
130+
vec![],
131+
vec![],
130132
)
131133
])
132134
}

‎src/test/ui/enable-unstable-lib-feature.stderr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: function `BOGUS` should have a snake case name such as `bogus`
2-
--> $DIR/enable-unstable-lib-feature.rs:12:1
1+
error: function `BOGUS` should have a snake case name
2+
--> $DIR/enable-unstable-lib-feature.rs:12:8
33
|
44
LL | pub fn BOGUS() { } //~ ERROR
5-
| ^^^^^^^^^^^^^^^^^^
5+
| ^^^^^ help: convert the identifier to snake case: `bogus`
66
|
77
note: lint level defined here
88
--> $DIR/enable-unstable-lib-feature.rs:6:9

‎src/test/ui/expr_attr_paren_order.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: variable `X` should have a snake case name such as `x`
1+
error: variable `X` should have a snake case name
22
--> $DIR/expr_attr_paren_order.rs:19:17
33
|
44
LL | let X = 0; //~ ERROR snake case name
5-
| ^
5+
| ^ help: convert the identifier to snake case: `x`
66
|
77
note: lint level defined here
88
--> $DIR/expr_attr_paren_order.rs:17:17

‎src/test/ui/lint/command-line-lint-group-deny.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: variable `_InappropriateCamelCasing` should have a snake case name such as `_inappropriate_camel_casing`
1+
error: variable `_InappropriateCamelCasing` should have a snake case name
22
--> $DIR/command-line-lint-group-deny.rs:4:9
33
|
44
LL | let _InappropriateCamelCasing = true; //~ ERROR should have a snake
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert the identifier to snake case: `_inappropriate_camel_casing`
66
|
77
= note: `-D non-snake-case` implied by `-D bad-style`
88

‎src/test/ui/lint/command-line-lint-group-forbid.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: variable `_InappropriateCamelCasing` should have a snake case name such as `_inappropriate_camel_casing`
1+
error: variable `_InappropriateCamelCasing` should have a snake case name
22
--> $DIR/command-line-lint-group-forbid.rs:4:9
33
|
44
LL | let _InappropriateCamelCasing = true; //~ ERROR should have a snake
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert the identifier to snake case: `_inappropriate_camel_casing`
66
|
77
= note: `-F non-snake-case` implied by `-F bad-style`
88

‎src/test/ui/lint/command-line-lint-group-warn.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
warning: variable `_InappropriateCamelCasing` should have a snake case name such as `_inappropriate_camel_casing`
1+
warning: variable `_InappropriateCamelCasing` should have a snake case name
22
--> $DIR/command-line-lint-group-warn.rs:5:9
33
|
44
LL | let _InappropriateCamelCasing = true;
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert the identifier to snake case: `_inappropriate_camel_casing`
66
|
77
= note: `-W non-snake-case` implied by `-W bad-style`
88

‎src/test/ui/lint/lint-group-nonstandard-style.stderr

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ LL | #![warn(nonstandard_style)]
1111
| ^^^^^^^^^^^^^^^^^
1212
= note: #[warn(non_camel_case_types)] implied by #[warn(nonstandard_style)]
1313

14-
error: function `CamelCase` should have a snake case name such as `camel_case`
15-
--> $DIR/lint-group-nonstandard-style.rs:4:1
14+
error: function `CamelCase` should have a snake case name
15+
--> $DIR/lint-group-nonstandard-style.rs:4:4
1616
|
1717
LL | fn CamelCase() {} //~ ERROR should have a snake
18-
| ^^^^^^^^^^^^^^^^^
18+
| ^^^^^^^^^ help: convert the identifier to snake case: `camel_case`
1919
|
2020
note: lint level defined here
2121
--> $DIR/lint-group-nonstandard-style.rs:1:9
@@ -24,11 +24,11 @@ LL | #![deny(nonstandard_style)]
2424
| ^^^^^^^^^^^^^^^^^
2525
= note: #[deny(non_snake_case)] implied by #[deny(nonstandard_style)]
2626

27-
error: function `CamelCase` should have a snake case name such as `camel_case`
28-
--> $DIR/lint-group-nonstandard-style.rs:12:9
27+
error: function `CamelCase` should have a snake case name
28+
--> $DIR/lint-group-nonstandard-style.rs:12:12
2929
|
3030
LL | fn CamelCase() {} //~ ERROR should have a snake
31-
| ^^^^^^^^^^^^^^^^^
31+
| ^^^^^^^^^ help: convert the identifier to snake case: `camel_case`
3232
|
3333
note: lint level defined here
3434
--> $DIR/lint-group-nonstandard-style.rs:10:14
@@ -50,11 +50,11 @@ LL | #[forbid(nonstandard_style)]
5050
| ^^^^^^^^^^^^^^^^^
5151
= note: #[forbid(non_upper_case_globals)] implied by #[forbid(nonstandard_style)]
5252

53-
warning: function `CamelCase` should have a snake case name such as `camel_case`
54-
--> $DIR/lint-group-nonstandard-style.rs:20:9
53+
warning: function `CamelCase` should have a snake case name
54+
--> $DIR/lint-group-nonstandard-style.rs:20:12
5555
|
5656
LL | fn CamelCase() {} //~ WARN should have a snake
57-
| ^^^^^^^^^^^^^^^^^
57+
| ^^^^^^^^^ help: convert the identifier to snake case: `camel_case`
5858
|
5959
note: lint level defined here
6060
--> $DIR/lint-group-nonstandard-style.rs:18:17

‎src/test/ui/lint/lint-non-snake-case-crate-2.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// compile-flags: --crate-name NonSnakeCase
2-
// error-pattern: crate `NonSnakeCase` should have a snake case name such as `non_snake_case`
2+
// error-pattern: crate `NonSnakeCase` should have a snake case name
33

44
#![deny(non_snake_case)]
55

‎src/test/ui/lint/lint-non-snake-case-crate-2.stderr

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
error: crate `NonSnakeCase` should have a snake case name such as `non_snake_case`
1+
error: crate `NonSnakeCase` should have a snake case name
22
|
33
note: lint level defined here
44
--> $DIR/lint-non-snake-case-crate-2.rs:4:9
55
|
66
LL | #![deny(non_snake_case)]
77
| ^^^^^^^^^^^^^^
8+
= help: convert the identifier to snake case: `non_snake_case`
89

910
error: aborting due to previous error
1011

‎src/test/ui/lint/lint-non-snake-case-crate.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![crate_name = "NonSnakeCase"]
2-
//~^ ERROR crate `NonSnakeCase` should have a snake case name such as `non_snake_case`
2+
//~^ ERROR crate `NonSnakeCase` should have a snake case name
33
#![deny(non_snake_case)]
44

55
fn main() {}

‎src/test/ui/lint/lint-non-snake-case-crate.stderr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: crate `NonSnakeCase` should have a snake case name such as `non_snake_case`
2-
--> $DIR/lint-non-snake-case-crate.rs:1:1
1+
error: crate `NonSnakeCase` should have a snake case name
2+
--> $DIR/lint-non-snake-case-crate.rs:1:18
33
|
44
LL | #![crate_name = "NonSnakeCase"]
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^ help: convert the identifier to snake case: `non_snake_case`
66
|
77
note: lint level defined here
88
--> $DIR/lint-non-snake-case-crate.rs:3:9

0 commit comments

Comments
 (0)