Skip to content

Commit c570fc6

Browse files
llogiqllogiq
committed
Merge pull request #566 from mcarton/starts_with
Add a lint for starts_with
2 parents fdcd974 + 7a26cfc commit c570fc6

File tree

5 files changed

+113
-32
lines changed

5 files changed

+113
-32
lines changed

‎README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
66
[Jump to usage instructions](#usage)
77

88
##Lints
9-
There are 94 lints included in this crate:
9+
There are 95 lints included in this crate:
1010

1111
name | default | meaning
1212
---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -20,6 +20,7 @@ name
2020
[cast_possible_wrap](https://github.com/Manishearth/rust-clippy/wiki#cast_possible_wrap) | allow | casts that may cause wrapping around the value, e.g `x as i32` where `x: u32` and `x > i32::MAX`
2121
[cast_precision_loss](https://github.com/Manishearth/rust-clippy/wiki#cast_precision_loss) | allow | casts that cause loss of precision, e.g `x as f32` where `x: u64`
2222
[cast_sign_loss](https://github.com/Manishearth/rust-clippy/wiki#cast_sign_loss) | allow | casts from signed types to unsigned types, e.g `x as u32` where `x: i32`
23+
[chars_next_cmp](https://github.com/Manishearth/rust-clippy/wiki#chars_next_cmp) | warn | using `.chars().next()` to check if a string starts with a char
2324
[cmp_nan](https://github.com/Manishearth/rust-clippy/wiki#cmp_nan) | deny | comparisons to NAN (which will always return false, which is probably not intended)
2425
[cmp_owned](https://github.com/Manishearth/rust-clippy/wiki#cmp_owned) | warn | creating owned instances for comparing with others, e.g. `x == "foo".to_string()`
2526
[collapsible_if](https://github.com/Manishearth/rust-clippy/wiki#collapsible_if) | warn | two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` can be written as `if x && y { foo() }`

‎src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
191191
matches::MATCH_OVERLAPPING_ARM,
192192
matches::MATCH_REF_PATS,
193193
matches::SINGLE_MATCH,
194+
methods::CHARS_NEXT_CMP,
194195
methods::FILTER_NEXT,
195196
methods::OK_EXPECT,
196197
methods::OPTION_MAP_UNWRAP_OR,

‎src/methods.rs

Lines changed: 93 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,14 @@ use std::borrow::Cow;
77
use syntax::ptr::P;
88
use syntax::codemap::Span;
99

10-
use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, method_chain_args, match_trait_method,
11-
walk_ptrs_ty_depth, walk_ptrs_ty, get_trait_def_id, implements_trait};
1210
use utils::{
13-
BTREEMAP_ENTRY_PATH, DEFAULT_TRAIT_PATH, HASHMAP_ENTRY_PATH, OPTION_PATH,
14-
RESULT_PATH, STRING_PATH
11+
get_trait_def_id, implements_trait, in_external_macro, in_macro, match_path,
12+
match_trait_method, match_type, method_chain_args, snippet, span_lint, span_lint_and_then,
13+
span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth,
14+
};
15+
use utils::{
16+
BTREEMAP_ENTRY_PATH, DEFAULT_TRAIT_PATH, HASHMAP_ENTRY_PATH, OPTION_PATH, RESULT_PATH,
17+
STRING_PATH
1518
};
1619
use utils::MethodArgs;
1720
use rustc::middle::cstore::CrateStore;
@@ -176,6 +179,17 @@ declare_lint!(pub SEARCH_IS_SOME, Warn,
176179
"using an iterator search followed by `is_some()`, which is more succinctly \
177180
expressed as a call to `any()`");
178181

182+
/// **What it does:** This lint `Warn`s on using `.chars().next()` on a `str` to check if it
183+
/// starts with a given char.
184+
///
185+
/// **Why is this bad?** Readability, this can be written more concisely as `_.starts_with(_)`.
186+
///
187+
/// **Known problems:** None.
188+
///
189+
/// **Example:** `name.chars().next() == Some('_')`
190+
declare_lint!(pub CHARS_NEXT_CMP, Warn,
191+
"using `.chars().next()` to check if a string starts with a char");
192+
179193
/// **What it does:** This lint checks for calls to `.or(foo(..))`, `.unwrap_or(foo(..))`, etc., and
180194
/// suggests to use `or_else`, `unwrap_or_else`, etc., or `unwrap_or_default` instead.
181195
///
@@ -210,39 +224,56 @@ impl LintPass for MethodsPass {
210224
OK_EXPECT,
211225
OPTION_MAP_UNWRAP_OR,
212226
OPTION_MAP_UNWRAP_OR_ELSE,
213-
OR_FUN_CALL)
227+
OR_FUN_CALL,
228+
CHARS_NEXT_CMP)
214229
}
215230
}
216231

217232
impl LateLintPass for MethodsPass {
218233
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
219-
if let ExprMethodCall(name, _, ref args) = expr.node {
220-
// Chain calls
221-
if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
222-
lint_unwrap(cx, expr, arglists[0]);
223-
} else if let Some(arglists) = method_chain_args(expr, &["to_string"]) {
224-
lint_to_string(cx, expr, arglists[0]);
225-
} else if let Some(arglists) = method_chain_args(expr, &["ok", "expect"]) {
226-
lint_ok_expect(cx, expr, arglists[0]);
227-
} else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or"]) {
228-
lint_map_unwrap_or(cx, expr, arglists[0], arglists[1]);
229-
} else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or_else"]) {
230-
lint_map_unwrap_or_else(cx, expr, arglists[0], arglists[1]);
231-
} else if let Some(arglists) = method_chain_args(expr, &["filter", "next"]) {
232-
lint_filter_next(cx, expr, arglists[0]);
233-
} else if let Some(arglists) = method_chain_args(expr, &["find", "is_some"]) {
234-
lint_search_is_some(cx, expr, "find", arglists[0], arglists[1]);
235-
} else if let Some(arglists) = method_chain_args(expr, &["position", "is_some"]) {
236-
lint_search_is_some(cx, expr, "position", arglists[0], arglists[1]);
237-
} else if let Some(arglists) = method_chain_args(expr, &["rposition", "is_some"]) {
238-
lint_search_is_some(cx, expr, "rposition", arglists[0], arglists[1]);
239-
}
234+
if in_macro(cx, expr.span) {
235+
return;
236+
}
237+
238+
match expr.node {
239+
ExprMethodCall(name, _, ref args) => {
240+
// Chain calls
241+
if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
242+
lint_unwrap(cx, expr, arglists[0]);
243+
} else if let Some(arglists) = method_chain_args(expr, &["to_string"]) {
244+
lint_to_string(cx, expr, arglists[0]);
245+
} else if let Some(arglists) = method_chain_args(expr, &["ok", "expect"]) {
246+
lint_ok_expect(cx, expr, arglists[0]);
247+
} else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or"]) {
248+
lint_map_unwrap_or(cx, expr, arglists[0], arglists[1]);
249+
} else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or_else"]) {
250+
lint_map_unwrap_or_else(cx, expr, arglists[0], arglists[1]);
251+
} else if let Some(arglists) = method_chain_args(expr, &["filter", "next"]) {
252+
lint_filter_next(cx, expr, arglists[0]);
253+
} else if let Some(arglists) = method_chain_args(expr, &["find", "is_some"]) {
254+
lint_search_is_some(cx, expr, "find", arglists[0], arglists[1]);
255+
} else if let Some(arglists) = method_chain_args(expr, &["position", "is_some"]) {
256+
lint_search_is_some(cx, expr, "position", arglists[0], arglists[1]);
257+
} else if let Some(arglists) = method_chain_args(expr, &["rposition", "is_some"]) {
258+
lint_search_is_some(cx, expr, "rposition", arglists[0], arglists[1]);
259+
}
240260

241-
lint_or_fun_call(cx, expr, &name.node.as_str(), &args);
261+
lint_or_fun_call(cx, expr, &name.node.as_str(), &args);
262+
}
263+
ExprBinary(op, ref lhs, ref rhs) if op.node == BiEq || op.node == BiNe => {
264+
if !lint_chars_next(cx, expr, lhs, rhs, op.node == BiEq) {
265+
lint_chars_next(cx, expr, rhs, lhs, op.node == BiEq);
266+
}
267+
}
268+
_ => (),
242269
}
243270
}
244271

245272
fn check_item(&mut self, cx: &LateContext, item: &Item) {
273+
if in_external_macro(cx, item.span) {
274+
return;
275+
}
276+
246277
if let ItemImpl(_, _, _, None, ref ty, ref items) = item.node {
247278
for implitem in items {
248279
let name = implitem.name;
@@ -570,6 +601,41 @@ fn lint_search_is_some(cx: &LateContext, expr: &Expr, search_method: &str, searc
570601
}
571602
}
572603

604+
/// Checks for the `CHARS_NEXT_CMP` lint.
605+
fn lint_chars_next(cx: &LateContext, expr: &Expr, chain: &Expr, other: &Expr, eq: bool) -> bool {
606+
if_let_chain! {[
607+
let Some(args) = method_chain_args(chain, &["chars", "next"]),
608+
let ExprCall(ref fun, ref arg_char) = other.node,
609+
arg_char.len() == 1,
610+
let ExprPath(None, ref path) = fun.node,
611+
path.segments.len() == 1 && path.segments[0].identifier.name.as_str() == "Some"
612+
], {
613+
let self_ty = walk_ptrs_ty(cx.tcx.expr_ty_adjusted(&args[0][0]));
614+
615+
if self_ty.sty != ty::TyStr {
616+
return false;
617+
}
618+
619+
span_lint_and_then(cx,
620+
CHARS_NEXT_CMP,
621+
expr.span,
622+
"you should use the `starts_with` method",
623+
|db| {
624+
let sugg = format!("{}{}.starts_with({})",
625+
if eq { "" } else { "!" },
626+
snippet(cx, args[0][0].span, "_"),
627+
snippet(cx, arg_char[0].span, "_")
628+
);
629+
630+
db.span_suggestion(expr.span, "like this", sugg);
631+
});
632+
633+
return true;
634+
}}
635+
636+
false
637+
}
638+
573639
// Given a `Result<T, E>` type, return its error type (`E`)
574640
fn get_error_type<'a>(cx: &LateContext, ty: ty::Ty<'a>) -> Option<ty::Ty<'a>> {
575641
if !match_type(cx, ty, &RESULT_PATH) {

‎src/misc.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -389,13 +389,14 @@ impl LateLintPass for UsedUnderscoreBinding {
389389
.last()
390390
.expect("path should always have at least one segment")
391391
.identifier;
392-
ident.name.as_str().chars().next() == Some('_') && // starts with '_'
393-
ident.name.as_str().chars().skip(1).next() != Some('_') && // doesn't start with "__"
394-
ident.name != ident.unhygienic_name && is_used(cx, expr) // not in bang macro
392+
ident.name.as_str().starts_with('_') &&
393+
!ident.name.as_str().starts_with("__") &&
394+
ident.name != ident.unhygienic_name &&
395+
is_used(cx, expr) // not in bang macro
395396
}
396397
ExprField(_, spanned) => {
397398
let name = spanned.node.as_str();
398-
name.chars().next() == Some('_') && name.chars().skip(1).next() != Some('_')
399+
name.starts_with('_') && !name.starts_with("__")
399400
}
400401
_ => false,
401402
};

‎tests/compile-fail/methods.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,3 +292,15 @@ struct MyError(()); // doesn't implement Debug
292292
struct MyErrorWithParam<T> {
293293
x: T
294294
}
295+
296+
fn starts_with() {
297+
"".chars().next() == Some(' ');
298+
//~^ ERROR starts_with
299+
//~| HELP like this
300+
//~| SUGGESTION "".starts_with(' ')
301+
302+
Some(' ') != "".chars().next();
303+
//~^ ERROR starts_with
304+
//~| HELP like this
305+
//~| SUGGESTION !"".starts_with(' ')
306+
}

0 commit comments

Comments
 (0)