Skip to content

Add a lint for starts_with #566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 20, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
[Jump to usage instructions](#usage)

##Lints
There are 94 lints included in this crate:
There are 95 lints included in this crate:

name | default | meaning
---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Expand All @@ -20,6 +20,7 @@ name
[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`
[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`
[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`
[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
[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)
[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()`
[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() }`
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
matches::MATCH_OVERLAPPING_ARM,
matches::MATCH_REF_PATS,
matches::SINGLE_MATCH,
methods::CHARS_NEXT_CMP,
methods::FILTER_NEXT,
methods::OK_EXPECT,
methods::OPTION_MAP_UNWRAP_OR,
Expand Down
120 changes: 93 additions & 27 deletions src/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@ use std::borrow::Cow;
use syntax::ptr::P;
use syntax::codemap::Span;

use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, method_chain_args, match_trait_method,
walk_ptrs_ty_depth, walk_ptrs_ty, get_trait_def_id, implements_trait};
use utils::{
BTREEMAP_ENTRY_PATH, DEFAULT_TRAIT_PATH, HASHMAP_ENTRY_PATH, OPTION_PATH,
RESULT_PATH, STRING_PATH
get_trait_def_id, implements_trait, in_external_macro, in_macro, match_path,
match_trait_method, match_type, method_chain_args, snippet, span_lint, span_lint_and_then,
span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth,
};
use utils::{
BTREEMAP_ENTRY_PATH, DEFAULT_TRAIT_PATH, HASHMAP_ENTRY_PATH, OPTION_PATH, RESULT_PATH,
STRING_PATH
};
use utils::MethodArgs;
use rustc::middle::cstore::CrateStore;
Expand Down Expand Up @@ -176,6 +179,17 @@ declare_lint!(pub SEARCH_IS_SOME, Warn,
"using an iterator search followed by `is_some()`, which is more succinctly \
expressed as a call to `any()`");

/// **What it does:** This lint `Warn`s on using `.chars().next()` on a `str` to check if it
/// starts with a given char.
///
/// **Why is this bad?** Readability, this can be written more concisely as `_.starts_with(_)`.
///
/// **Known problems:** None.
///
/// **Example:** `name.chars().next() == Some('_')`
declare_lint!(pub CHARS_NEXT_CMP, Warn,
"using `.chars().next()` to check if a string starts with a char");

/// **What it does:** This lint checks for calls to `.or(foo(..))`, `.unwrap_or(foo(..))`, etc., and
/// suggests to use `or_else`, `unwrap_or_else`, etc., or `unwrap_or_default` instead.
///
Expand Down Expand Up @@ -210,39 +224,56 @@ impl LintPass for MethodsPass {
OK_EXPECT,
OPTION_MAP_UNWRAP_OR,
OPTION_MAP_UNWRAP_OR_ELSE,
OR_FUN_CALL)
OR_FUN_CALL,
CHARS_NEXT_CMP)
}
}

impl LateLintPass for MethodsPass {
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
if let ExprMethodCall(name, _, ref args) = expr.node {
// Chain calls
if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
lint_unwrap(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["to_string"]) {
lint_to_string(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["ok", "expect"]) {
lint_ok_expect(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or"]) {
lint_map_unwrap_or(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or_else"]) {
lint_map_unwrap_or_else(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["filter", "next"]) {
lint_filter_next(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["find", "is_some"]) {
lint_search_is_some(cx, expr, "find", arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["position", "is_some"]) {
lint_search_is_some(cx, expr, "position", arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["rposition", "is_some"]) {
lint_search_is_some(cx, expr, "rposition", arglists[0], arglists[1]);
}
if in_macro(cx, expr.span) {
return;
}

match expr.node {
ExprMethodCall(name, _, ref args) => {
// Chain calls
if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
lint_unwrap(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["to_string"]) {
lint_to_string(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["ok", "expect"]) {
lint_ok_expect(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or"]) {
lint_map_unwrap_or(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or_else"]) {
lint_map_unwrap_or_else(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["filter", "next"]) {
lint_filter_next(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["find", "is_some"]) {
lint_search_is_some(cx, expr, "find", arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["position", "is_some"]) {
lint_search_is_some(cx, expr, "position", arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["rposition", "is_some"]) {
lint_search_is_some(cx, expr, "rposition", arglists[0], arglists[1]);
}

lint_or_fun_call(cx, expr, &name.node.as_str(), &args);
lint_or_fun_call(cx, expr, &name.node.as_str(), &args);
}
ExprBinary(op, ref lhs, ref rhs) if op.node == BiEq || op.node == BiNe => {
if !lint_chars_next(cx, expr, lhs, rhs, op.node == BiEq) {
lint_chars_next(cx, expr, rhs, lhs, op.node == BiEq);
}
}
_ => (),
}
}

fn check_item(&mut self, cx: &LateContext, item: &Item) {
if in_external_macro(cx, item.span) {
return;
}

if let ItemImpl(_, _, _, None, ref ty, ref items) = item.node {
for implitem in items {
let name = implitem.name;
Expand Down Expand Up @@ -570,6 +601,41 @@ fn lint_search_is_some(cx: &LateContext, expr: &Expr, search_method: &str, searc
}
}

/// Checks for the `CHARS_NEXT_CMP` lint.
fn lint_chars_next(cx: &LateContext, expr: &Expr, chain: &Expr, other: &Expr, eq: bool) -> bool {
if_let_chain! {[
let Some(args) = method_chain_args(chain, &["chars", "next"]),
let ExprCall(ref fun, ref arg_char) = other.node,
arg_char.len() == 1,
let ExprPath(None, ref path) = fun.node,
path.segments.len() == 1 && path.segments[0].identifier.name.as_str() == "Some"
], {
let self_ty = walk_ptrs_ty(cx.tcx.expr_ty_adjusted(&args[0][0]));

if self_ty.sty != ty::TyStr {
return false;
}

span_lint_and_then(cx,
CHARS_NEXT_CMP,
expr.span,
"you should use the `starts_with` method",
|db| {
let sugg = format!("{}{}.starts_with({})",
if eq { "" } else { "!" },
snippet(cx, args[0][0].span, "_"),
snippet(cx, arg_char[0].span, "_")
);

db.span_suggestion(expr.span, "like this", sugg);
});

return true;
}}

false
}

// Given a `Result<T, E>` type, return its error type (`E`)
fn get_error_type<'a>(cx: &LateContext, ty: ty::Ty<'a>) -> Option<ty::Ty<'a>> {
if !match_type(cx, ty, &RESULT_PATH) {
Expand Down
9 changes: 5 additions & 4 deletions src/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,13 +389,14 @@ impl LateLintPass for UsedUnderscoreBinding {
.last()
.expect("path should always have at least one segment")
.identifier;
ident.name.as_str().chars().next() == Some('_') && // starts with '_'
ident.name.as_str().chars().skip(1).next() != Some('_') && // doesn't start with "__"
ident.name != ident.unhygienic_name && is_used(cx, expr) // not in bang macro
ident.name.as_str().starts_with('_') &&
!ident.name.as_str().starts_with("__") &&
ident.name != ident.unhygienic_name &&
is_used(cx, expr) // not in bang macro
}
ExprField(_, spanned) => {
let name = spanned.node.as_str();
name.chars().next() == Some('_') && name.chars().skip(1).next() != Some('_')
name.starts_with('_') && !name.starts_with("__")
}
_ => false,
};
Expand Down
12 changes: 12 additions & 0 deletions tests/compile-fail/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,3 +292,15 @@ struct MyError(()); // doesn't implement Debug
struct MyErrorWithParam<T> {
x: T
}

fn starts_with() {
"".chars().next() == Some(' ');
//~^ ERROR starts_with
//~| HELP like this
//~| SUGGESTION "".starts_with(' ')

Some(' ') != "".chars().next();
//~^ ERROR starts_with
//~| HELP like this
//~| SUGGESTION !"".starts_with(' ')
}